-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handling secrets in drone v0.5+ #32
Comments
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. |
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. |
@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 Yet another alternative is to have a list of "safe" environment variables (ending up with some conventions oh how variables should be named on |
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/harness#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 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 |
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:
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 pluginvargs.Secrets
. This makes it pretty safe to do something like excludevargs.Secrets
from verbose/debug output. This convention doesn't stop pipeline authors from injecting drone secrets outside ofvargs.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.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.
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
andenv
, to be able to reuse that template in multiple envs, along with actual secret values.The text was updated successfully, but these errors were encountered: