Merge pull request #2464 from milescrabill/bug-1591476-registerworker-returns-worker-config

bug 1591476: registerWorker() returns workerConfig

Author: Miles Crabill

changelog/bug-1591476.md

@@ -0,0 +1,4 @@
+level: patch
+reference: bug 1591476
+---
+worker-manager's `registerWorker()` now returns worker config, and worker-runner (for Azure and static providers, others coming soon) merges that configuration with other configuration sources.  This allows worker pools to include configuration for static workers, and allows Azure workers to fetch their config without referencing the non-functional customData instance metadata.

clients/client-go/tcworkermanager/types.go

@@ -6,3 +6,13 @@ additionalProperties: false
 required: []
 properties:
   lifecycle: {$ref: 'worker-lifecycle.json#'}
+  workerConfig:
+    title: Worker Config
+    type: object
+    additionalProperties: true
+    description: |
+      This value is supplied unchanged to the worker from the provider configuration.
+      The expectation is that the worker will merge this information with configuration from other sources,
+      and this is precisely what [taskcluster-worker-runner](https://github.com/taskcluster/taskcluster-worker-runner) does.
+      This property must not be used for secret configuration, as it is visible both in the worker pool configuration and in the worker instance's metadata.
+      Instead, put secret configuration in the [secrets service](https://github.com/taskcluster/taskcluster-worker-runner#secrets).

generated/references.json

@@ -36,7 +36,18 @@ properties:
     required:
       - accessToken
       - clientId
+  workerConfig:
+    type: object
+    title: Worker Config
+    additionalProperties: true
+    description: |
+      This value is supplied unchanged to the worker from the worker-pool configuration.
+      The expectation is that the worker will merge this information with configuration from other sources,
+      and this is precisely what [taskcluster-worker-runner](https://github.com/taskcluster/taskcluster-worker-runner) does.
+      This property must not be used for secret configuration, as it is visible both in the worker pool configuration and in the worker instance's metadata.
+      Instead, put secret configuration in the [secrets service](https://github.com/taskcluster/taskcluster-worker-runner#secrets).
 additionalProperties: false
 required:
   - expires
   - credentials
+  - workerConfig

services/worker-manager/schemas/v1/config-static.yml

@@ -647,10 +647,11 @@ builder.declare({
       `Worker ${workerGroup}/${workerId} does not have provider ${providerId}`, {});
   }
 
-  let expires;
+  let expires, workerConfig;
   try {
     const reg = await provider.registerWorker({worker, workerPool, workerIdentityProof});
     expires = reg.expires;
+    workerConfig = reg.workerConfig;
   } catch (err) {
     if (!(err instanceof ApiError)) {
       throw err;
@@ -682,5 +683,5 @@ builder.declare({
     credentials: this.cfg.taskcluster.credentials,
   });
 
-  return res.reply({expires: expires.toJSON(), credentials});
+  return res.reply({expires: expires.toJSON(), credentials, workerConfig});
 });

services/worker-manager/schemas/v1/register-worker-response.yml

@@ -225,6 +225,7 @@ class AwsProvider extends Provider {
             stateReason: i.StateReason.Message,
             terminateAfter,
             reregistrationTimeout,
+            workerConfig: config.workerConfig || {},
           },
         });
       }));
@@ -275,7 +276,8 @@ class AwsProvider extends Provider {
       w.providerData.terminateAfter = expires.getTime();
     });
 
-    return {expires};
+    const workerConfig = worker.providerData.workerConfig || {};
+    return {expires, workerConfig};
   }
 
   async checkWorker({worker}) {

services/worker-manager/src/api.js

@@ -200,6 +200,7 @@ class AzureProvider extends Provider {
       let providerData = {
         location: cfg.location,
         resourceGroupName: this.providerConfig.resourceGroupName,
+        workerConfig: cfg.workerConfig,
         vm: {
           name: virtualMachineName,
           computerName,
@@ -375,8 +376,8 @@ class AzureProvider extends Provider {
       w.state = this.Worker.states.RUNNING;
       w.providerData.terminateAfter = expires.getTime();
     });
-
-    return {expires};
+    const workerConfig = worker.providerData.workerConfig || {};
+    return {expires, workerConfig};
   }
 
   async scanPrepare() {

services/worker-manager/src/providers/aws.js

@@ -158,7 +158,8 @@ class GoogleProvider extends Provider {
     });
 
     // assume for the moment that workers self-terminate before 96 hours
-    return {expires};
+    const workerConfig = worker.providerData.workerConfig || {};
+    return {expires, workerConfig};
   }
 
   async deprovision({workerPool}) {
@@ -324,6 +325,7 @@ class GoogleProvider extends Provider {
           },
           terminateAfter,
           reregistrationTimeout, // Record this for later reregistrations so that we can recalculate deadline
+          workerConfig: cfg.workerConfig || {},
         },
       });
     }));

services/worker-manager/src/providers/azure.js

@@ -27,7 +27,7 @@ class StaticProvider extends Provider {
       expires: new Date(input.expires),
       capacity: input.capacity,
       state: Worker.states.RUNNING,
-      providerData: {staticSecret},
+      providerData: {staticSecret, workerConfig: workerPool.config.workerConfig},
     };
 
     let worker;
@@ -71,8 +71,8 @@ class StaticProvider extends Provider {
     } else {
       expires = taskcluster.fromNow('96 hours');
     }
-
-    return {expires};
+    const workerConfig = worker.providerData.workerConfig || {};
+    return {expires, workerConfig};
   }
 }
 

services/worker-manager/src/providers/google.js

@@ -56,7 +56,8 @@ class TestingProvider extends Provider {
     if (worker.providerData.noExpiry) {
       return {};
     }
-    return {expires: taskcluster.fromNow('1 hour')};
+    const workerConfig = worker.providerData.workerConfig || {};
+    return {expires: taskcluster.fromNow('1 hour'), workerConfig};
   }
 
   async createWorker({workerPool, workerGroup, workerId, input}) {

services/worker-manager/src/providers/static.js

@@ -1050,6 +1050,11 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
       });
       await helper.Worker.create({
         ...defaultWorker,
+        providerData: {
+          workerConfig: {
+            "someKey": "someValue",
+          },
+        },
       });
       const res = await helper.workerManager.registerWorker({
         ...defaultRegisterWorker,
@@ -1058,6 +1063,8 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
       assert.equal(res.credentials.clientId,
         `worker/${providerId}/${workerPoolId}/${workerGroup}/${workerId}`);
 
+      assert.equal(res.workerConfig.someKey, "someValue");
+
       // cheat a little and look in the certificate to check the scopes
       const scopes = new Set(JSON.parse(res.credentials.certificate).scopes);
       const msg = `got scopes ${[...scopes].join(', ')}`;

services/worker-manager/src/providers/testing.js

@@ -402,6 +402,9 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
           availabilityZone: 'us-west-2a',
           privateIp: '172.31.23.159',
           owner: actualWorkerIid.accountId,
+          workerConfig: {
+            "someConfig": "someConfigValue",
+          },
         },
       });
 
@@ -413,6 +416,7 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
       const resp = await provider.registerWorker({worker: runningWorker, workerPool, workerIdentityProof});
       assert(resp.expires - new Date() + 10000 > 96 * 3600 * 1000);
       assert(resp.expires - new Date() - 10000 < 96 * 3600 * 1000);
+      assert.equal(resp.workerConfig.someConfig, 'someConfigValue');
 
       sinon.restore();
     });
@@ -430,6 +434,9 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
           availabilityZone: 'us-west-2a',
           privateIp: '172.31.23.159',
           owner: actualWorkerIid.accountId,
+          workerConfig: {
+            'someKey': 'someValue',
+          },
         },
       });
 
@@ -441,7 +448,7 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
       const resp = await provider.registerWorker({worker: runningWorker, workerPool, workerIdentityProof});
       assert(resp.expires - new Date() + 10000 > 10 * 3600 * 1000);
       assert(resp.expires - new Date() - 10000 < 10 * 3600 * 1000);
-
+      assert.equal(resp.workerConfig.someKey, 'someValue');
       sinon.restore();
     });
   });

services/worker-manager/test/api_test.js

@@ -505,18 +505,39 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
     test('sweet success', async function() {
       const worker = await helper.Worker.create({
         ...defaultWorker,
+        providerData: {
+          ...baseProviderData,
+          vm: {
+            name: 'some-vm',
+            vmId: vmId,
+          },
+          workerConfig: {
+            "someKey": "someValue",
+          },
+        },
       });
       const document = fs.readFileSync(path.resolve(__dirname, 'fixtures/azure_signature_good')).toString();
       const workerIdentityProof = {document};
       const res = await provider.registerWorker({workerPool, worker, workerIdentityProof});
       // allow +- 10 seconds since time passes while the test executes
       assert(res.expires - new Date() + 10000 > 96 * 3600 * 1000, res.expires);
       assert(res.expires - new Date() - 10000 < 96 * 3600 * 1000, res.expires);
+      assert.equal(res.workerConfig.someKey, 'someValue');
     });
 
     test('sweet success (different reregister)', async function() {
       const worker = await helper.Worker.create({
         ...defaultWorker,
+        providerData: {
+          ...baseProviderData,
+          vm: {
+            name: 'some-vm',
+            vmId: vmId,
+          },
+          workerConfig: {
+            "someKey": "someValue",
+          },
+        },
       });
       await worker.modify(w => {
         w.providerData.reregistrationTimeout = 10 * 3600 * 1000;
@@ -527,6 +548,7 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
       // allow +- 10 seconds since time passes while the test executes
       assert(res.expires - new Date() + 10000 > 10 * 3600 * 1000, res.expires);
       assert(res.expires - new Date() - 10000 < 10 * 3600 * 1000, res.expires);
+      assert.equal(res.workerConfig.someKey, 'someValue');
     });
   });
 });

services/worker-manager/test/provider_aws_test.js

@@ -434,26 +434,36 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
     test('sweet success', async function() {
       const worker = await helper.Worker.create({
         ...defaultWorker,
+        providerData: {
+          workerConfig: {
+            "someKey": "someValue",
+          },
+        },
       });
       const workerIdentityProof = {token: 'good'};
       const res = await provider.registerWorker({workerPool, worker, workerIdentityProof});
       // allow +- 10 seconds since time passes while the test executes
       assert(res.expires - new Date() + 10000 > 96 * 3600 * 1000, res.expires);
       assert(res.expires - new Date() - 10000 < 96 * 3600 * 1000, res.expires);
+      assert.equal(res.workerConfig.someKey, 'someValue');
     });
 
     test('sweet success (different reregister)', async function() {
       const worker = await helper.Worker.create({
         ...defaultWorker,
         providerData: {
           reregistrationTimeout: 3600 * 10 * 1000,
+          workerConfig: {
+            "someKey": "someValue",
+          },
         },
       });
       const workerIdentityProof = {token: 'good'};
       const res = await provider.registerWorker({workerPool, worker, workerIdentityProof});
       // allow +- 10 seconds since time passes while the test executes
       assert(res.expires - new Date() + 10000 > 10 * 3600 * 1000, res.expires);
       assert(res.expires - new Date() - 10000 < 10 * 3600 * 1000, res.expires);
+      assert.equal(res.workerConfig.someKey, 'someValue');
     });
   });
 });

services/worker-manager/test/provider_azure_test.js

@@ -90,13 +90,20 @@ helper.secrets.mockSuite(testing.suiteName(), ['azure'], function(mock, skipping
     test('successful registration', async function() {
       const worker = await helper.Worker.create({
         ...defaultWorker,
+        providerData: {
+          staticSecret: 'good',
+          workerConfig: {
+            "someKey": "someValue",
+          },
+        },
       });
       const workerIdentityProof = {staticSecret: 'good'};
       const res = await provider.registerWorker({workerPool, worker, workerIdentityProof});
       const expectedExpires = new Date(Date.now() + 3600 * 1000);
       // allow +- 10 seconds since time passes while the test executes
       assert(Math.abs(res.expires - expectedExpires) < 10000,
         `${res.expires}, ${expectedExpires}, diff = ${res.expires - expectedExpires} ms`);
+      assert.equal(res.workerConfig.someKey, 'someValue');
     });
   });
 });

services/worker-manager/test/provider_google_test.js

@@ -17,7 +17,7 @@ type ProviderWorkerConfig struct {
 	Files  []files.File  `json:"files,omitempty"`
 }
 
-// ParseProviderWorkerConfig takes a RawMessage represneting the `workerConfig`
+// ParseProviderWorkerConfig takes a RawMessage representing the `workerConfig`
 // field received from worker-manager, and returns the config and files it
 // contains.  This requires the runnerConfig in order to determine the worker
 // implementation name.

services/worker-manager/test/provider_static_test.js

@@ -58,7 +58,10 @@ func (p *AWSProvider) ConfigureRun(state *run.State) error {
 		"signature": interface{}(instanceIdentityDocumentSignature),
 	}
 
-	err = provider.RegisterWorker(
+	// TODO
+	// bug 1591476: we should get workerConfig from RegisterWorker()
+	// and not from the metadata service
+	_, err = provider.RegisterWorker(
 		state,
 		wm,
 		userData.WorkerPoolId,

tools/taskcluster-worker-runner/cfg/providerworkerconfig.go

@@ -68,7 +68,7 @@ func (p *AzureProvider) ConfigureRun(state *run.State) error {
 		"document": interface{}(document),
 	}
 
-	err = provider.RegisterWorker(
+	workerConfig, err := provider.RegisterWorker(
 		state,
 		wm,
 		customData.WorkerPoolId,
@@ -97,7 +97,7 @@ func (p *AzureProvider) ConfigureRun(state *run.State) error {
 
 	state.ProviderMetadata = providerMetadata
 
-	pwc, err := cfg.ParseProviderWorkerConfig(p.runnercfg, customData.ProviderWorkerConfig)
+	pwc, err := cfg.ParseProviderWorkerConfig(p.runnercfg, workerConfig)
 	if err != nil {
 		return err
 	}