Skip to content
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

Allow Reading Secrets from Files #2618

Closed
wants to merge 3 commits into from

Conversation

sinkingpoint
Copy link
Contributor

As discussed in #2498 , it would be nice to be able to read secrets from files. It's a rainy day here, so in this PR we introduce that support for all secrets in the global and individual notifier configs, based on the existing support for this in the SlackAPIKeyFile config var.

On a sort of personal node: This is useful for any alertmanager deployed in K8s as you can now use Kubernetes Secrets to manage individual config components rather than the whole config file

Comments/Criticism welcome - It's been a really interesting traipse around the codebase

This commit adds the fields to all the config entries that will enable
us to load Secret and SecretURL values from a file. It also validates
that you can't specify both the value and the value_file for any given
entry

Signed-off-by: sinkingpoint <colin@quirl.co.nz>
This commit adds actually using the _file config vars, resolving them at
nofication time similar to the existing slack file config

Signed-off-by: sinkingpoint <colin@quirl.co.nz>
@sinkingpoint
Copy link
Contributor Author

Note that I mimicked the Slack precedent for this that loads the secret at notify time - in high turn over alertmanager instances this feels like overkill for a value that might never change. We should probably make this load the secrets at config unmarshal time and handle reloads nicely

This commit adds tests to each notifier and the config unmarshalling
that loading a secret from a file is valid

Signed-off-by: sinkingpoint <colin@quirl.co.nz>
@roidelapluie
Copy link
Member

The values should be read from file every time they are used. I wonder if we really need to have all this custom code or if we can pass http client credentials to achieve a lot of them.

I will have a look, but it will take a long time. What about we split this pull request per receiver? Maybe start with one to see if we have the right approach?

@sinkingpoint
Copy link
Contributor Author

Happy to close this and reopen with just one notifier. Probably Pagerduty because that's the one I care about the most. It seems that most every provider handles Auth differently, so as much as we can hope for a unified solution I don't think it's going to be that easy (as much as I would love to add a bearer token and call this done)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants