1391205 - generic worker panics and crashes when cot key location (signingKeyLocation) is incorrectly set

Status: RESOLVED DUPLICATE of bug 1308054

(Taskcluster :: Workers, defect)

Description

[Rob Thijssen [:grenade (EET/UTC+0300)]]

while testing ebs changes, i came across some gw behaviour that ended up costing a lot of time to debug. i needed a green build on a beta worker in order to promote ebs changes to level 1/try but despite the build in question [1] not needing cot keys, and the build log explicitly stating "signing disabled because MOZ_SIGNING_SERVERS is not set" [2], and proclaiming a successful build, both treeherder and task inspector show the build as an exception/retry because gw crashes after uploading the last artifact with error message: "

Aug 17 10:00:29 i-0e8c8beea49e34551.gecko-1-b-win2012-beta.use1.mozilla.com generic-worker: 2017/08/17 09:00:29  *********** PANIC occurred! ***********  
Aug 17 10:00:29 i-0e8c8beea49e34551.gecko-1-b-win2012-beta.use1.mozilla.com generic-worker: 2017/08/17 09:00:29 open C:\generic-worker\generic-worker-gpg-signing-key.key: The system cannot find the file specified. 
Aug 17 10:00:30 i-0e8c8beea49e34551.gecko-1-b-win2012-beta.use1.mozilla.com generic-worker: 2017/08/17 09:00:30 Exiting worker with exit code 69 
Aug 17 10:00:30 i-0e8c8beea49e34551.gecko-1-b-win2012-beta.use1.mozilla.com generic-worker: 2017/08/17 09:00:30 Immediate shutdown being issued... 
Aug 17 10:00:30 i-0e8c8beea49e34551.gecko-1-b-win2012-beta.use1.mozilla.com generic-worker: 2017/08/17 09:00:30 generic-worker internal error " [3]

i realise that the simplest fix is just to have signingKeyLocation correctly set in aws provisioner configuration, but i've raised this bug because i believe the current behaviour in g-w of crashing after a successful build because configuration that was not required, did not exist, wasted a lot of time (waiting for builds to successfully complete before burning the job on this exception) in debugging a non-problem. i think that g-w should have handled this better but i don't know what "better" looks like, tbh.

i will also update occ to set signingKeyLocation when a cot key has been created on the golden ami instance.

[1]: https://tools.taskcluster.net/groups/VGpGbe9vQimEB8RTqjPTEQ/tasks/L0IDWfrLS1W0tYlNzkVS8g/runs/1
[2]: https://public-artifacts.taskcluster.net/L0IDWfrLS1W0tYlNzkVS8g/1/public/logs/live_backing.log
[3]: https://papertrailapp.com/systems/1086771181/events?focus=834763833217306648&selected=834763833217306648

Comment 1

[Rob Thijssen [:grenade (EET/UTC+0300)]]

Attachment: https://github.com/mozilla-releng/OpenCloudConfig/pull/87

Comment 2

[Greg Arndt [:garndt]]

I agree, I think that waiting until the end of the task to find out the worker type was not configured properly is not ideal.  

In this case, the feature is enabled in the payload, so the worker could determine that the worker is configured to support that feature (including having a valid key that can be read) and if not, fail the task (I don't think the worker needs to panic and shutdown, but maybe I do not know the nuances with that decision).  I think this is can be done in the Initialize method for the chain of trust feature in the worker (probably a simple os.Stat())

Comment 3

[Pete Moore [:pmoore][:pete]]

Comment on attachment 8898229 [details] [diff] [review]
https://github.com/mozilla-releng/OpenCloudConfig/pull/87

Review of attachment 8898229 [details] [diff] [review]:
-----------------------------------------------------------------

Signing key is not allowed to be an empty string, it needs to be a valid private key. If we don't care what the key is, the best is to just generate one on the fly with:

```
generic-worker new-openpgp-keypair --file C:\generic-worker\cot.key
```

This way, you can use `C:\generic-worker\cot.key` regardless of which windows worker it is.

See [how we do this](https://github.com/taskcluster/generic-worker/blob/a262293216c8e5a39b15f87a1e2b5783c923c2d1/worker_types/nss-win2012r2/userdata#L144-L145) for NSS windows 2012 workers.

Comment 4

[Pete Moore [:pmoore][:pete]]

At the moment, a valid PGP signing key is a prerequisite for running the worker, since the worker may potentially claim tasks which require it to sign an artifact. The reasoning behind this was, even if it is a try job, somebody might want to test something that runs in production which requires signing, and the signing artifact to be generated.

Rather than prevent such tasks running e.g. on try, the preferred approach was to make it very simple to create a signing key (just a generic-worker command) so that it is trivial and inexpensive to set up, and then whoever provisions the environment can decide if they want to create a throwaway signing key, and not care what it is, use a shared private key that isn't very secure, or jump through lots of security hoops to have something very secure. I wanted to avoid that at runtime, a job comes through, months after a worker type got deployed, and suddenly someone discovers they can't run a job that requires signing. I wanted to make it a trivial but required step to ensure *some* signing key existed, so that you don't have to worry about it later, when a job comes in (i.e. take the hit upfront).

Anyways, I could probably improve this by adding an upfront check that a valid file has been specified, when the worker first starts up, rather than failing at the end of a task that tried to sign something. However, note, this could mean with our current setup, that the provisioner would just keep provisioning instances, and all of them would shut down, and then we'd have another problem on our hands - so it isn't ideal either. :/

Comment 5

[Greg Arndt [:garndt]]

If the cot key cannot be found, or is not a valid key, I think terminating the worker is the best option (like we do now) because clearly something is in a state it should not be in.  Could we propagate that up to the task level to repo what happened? Or should we create a sentry alert and monitor that?

Also, any value in detecting this problem earlier in the task lifecycle rather than at the end of trying to generate the cot?

Comment 6

[Greg Arndt [:garndt]]

Ah, I didn't see your previous comment when submitting and ignored the warning it gave when submitting :)

I think failing the task early is nice because then you do not have to spend time doing anything known to fail.  However, you do hit the problem you're describing here if this is a problem with a worker type configuration rather than a specific worker gone bad.

Comment 8

[Pete Moore [:pmoore][:pete]]

(In reply to Rob Thijssen (:grenade - UTC+3) from comment #0)
> despite the build in question [1] not needing cot keys

This is not correct, the build had chain of trust feature enabled:

  "features": {
    "chainOfTrust": true
  }

See https://tools.taskcluster.net/groups/VGpGbe9vQimEB8RTqjPTEQ/tasks/L0IDWfrLS1W0tYlNzkVS8g/details

> the build log explicitly stating "signing disabled because MOZ_SIGNING_SERVERS is not set" [2]

This is not related to chain-of-trust signing - that is related to whether the builds are signed or not

> and proclaiming a successful build, both treeherder and task inspector show the
> build as an exception/retry because gw crashes after uploading the last
> artifact with error message:

This is expected and correct behaviour, since the worker has been misconfigured to use a key that does not exist. The task needs to use the key, the worker attempts to open the key file, and it does not exist, and therefore resolves the task as exception.