Handling secrets in drone v0.5+

[stephansnyt]

Problem

Secrets handling has to change from v0.4 to v0.5 plugin style and there isn't an obvious solution that's equal to or better the current one.

Currently secrets are passed to this plugin in the drone v0.4 style like so:

Secrets map[string]string `json:"secrets"`

i.e. a json object (map) with string values under the key secrets, i.e. a "named map". secrets_base64 behaves much the same way so no need to call it out every time.

In drone v0.5+, secrets are passed to plugins as environment variables. From the docs:

The secrets in the above are exposed to the plugin as uppercase environment variables. The variable names are therefore important.

It turns out this makes it impossible for new style plugins to handle secrets by the same convention, a "named map".

Implications

Within drone-gke it's easy to select all secrets for processing/iteration because there's a very clear convention that they all belong under plugin vargs.Secrets. This makes it pretty safe to do something like exclude vargs.Secrets from verbose/debug output. This convention doesn't stop pipeline authors from injecting drone secrets outside of vargs.Secrets, so it's not perfect, but it's a little better than what is possible in drone v0.5.

Solutions

token is the only named secret in use in this plugin and this issue doesn't really explore that in depth.

Secret Variable Name Prefix

A similar convention could be adopted in drone v0.5, but I think it's less obvious how it works and it makes pipeline definitions slightly more repetitive. The convention is to have all secret targets start with secret_, then the plugin iterates over all environment variables and selects those with this prefix to process as secrets, meaning they are excluded from output and passed to secret templates for interpolation.

deploy:
  gke:
    ...
    secrets:
-      foo: $$FOO_PRD
-      bar: $$BAR_PRD
+      - source: FOO_PRD
+        target: secret_foo
+      - source: BAR_PRD
+        target: secret_bar

Here is a PR that implements this idea: https://github.com/stephansnyt/drone-gke/compare/feature/remove-drone-0.4-code...stephansnyt:secretmap?expand=1

Provide An Explicit List Of Secret Names

It's doubtful this adds value to the solution given above, because this is counting on pipeline authors to the right thing twice instead of just once.

deploy:
  gke:
    ...
    secrets:
-      foo: $$FOO_PRD
-      bar: $$BAR_PRD
+      - source: FOO_PRD
+        target: foo
+      - source: BAR_PRD
+        target: bar
+    secret_variable_names:
+      - foo
+      - bar

Compare Environment Variables Against Keys In Secret Templates

The plugin could get the list of keys from secret templates, and look for corresponding environment variables. Then the plugin could select those to process as secrets, e.g. to exclude them from verbose/debug output.

This isn't perfect either because secret templates could contain non-secret keys like app and env, to be able to reuse that template in multiple envs, along with actual secret values.

[msuterski]

I don't think I understand what the problem is exactly. Why would we want to manage the secrets ourselves?

In drome 0.5+ secrets are pushed as env variable, unless needed, the plugin does not need to read them from there at all.

[stephansnyt]

We're not managing the secrets, we're using them. They have to be handled with care, for example it's not safe to print out all env vars because they may contain drone secrets that you would not want in a build log. It might not be safe to print interpolated templates if secrets were used in interpolation.

Because drone secret handling changed from v0.4 to v0.5, the way the plugin uses those secrets must change.

[fsouza]

@stephansnyt how important is printing the yaml in verbose mode? I feel like this is an issue just if we want to be able to print the yaml.

Alternatively, we could ask if drone authors would be interested in supporting the explicit list automatically (by exporting the list of secrets to an environment variable, something like DRONE_SECRETS maybe).

Yet another alternative is to have a list of "safe" environment variables (ending up with some conventions oh how variables should be named on .drone.yml). Then all other environment variables can be treated as secrets.

[tonglil]

Commenting since the implementation update is nearly complete (pending review).

I feel like it is a little convoluted to force the user to remap secret names...

    secrets:
      - source: API_TOKEN
        target: secret_api_token

... and then use their uppercased version in their template ...

data:
  api-token: {{.SECRET_API_TOKEN}}

... because it's not really expected.

However until harness/gitness#2088 is implemented, we can't automatically parse this out.

I don't want to do option 2 (add another field specifying the "list" of secrets being used), since that seems like the equivalent of target:.

---

Ideally it would look something like this:

    secrets: [API_TOKEN, lowercase_key_ok]

data:
  api-token: {{.API_TOKEN}}
  my-secret: {{.lowercase_key_ok}}

Note: to achieve lowercase_key_ok (key case sensitivity) would require harness/gitness#2070 as well.