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

Proposal: allow alert manager to read from secret resources #3108

Open
qinxx108 opened this issue Oct 13, 2022 · 13 comments · May be fixed by prometheus/prometheus#13955
Open

Proposal: allow alert manager to read from secret resources #3108

qinxx108 opened this issue Oct 13, 2022 · 13 comments · May be fixed by prometheus/prometheus#13955

Comments

@qinxx108
Copy link
Contributor

qinxx108 commented Oct 13, 2022

Problem

The prometheus alert manager (open source) right now support read password from file for part of the receivers, see issue (prometheus/prometheus#8551). And also besides read from file, we want to also be able to read from the other password provider for example AWS secret manager, Azure Key Vault provider etc.

We do have the secrets store CSI driver (https://secrets-store-csi-driver.sigs.k8s.io/getting-started/installation.html) to mount the secrets to pod so that we can use the receivers’s {KEY_NAME}_file to read the password. However that require user’s to maintain the additional CSI inside of the same cluster.

The ideal solution will be the alert manager itself can resolve the secrets in itself and cache that for a configurable time, re-fetch the secrets after timeout

Proposed configuration

In <pagerduty_config>

# The following two options are mutually exclusive.
# The PagerDuty integration key (when using PagerDuty integration type `Events API v2`).
routing_key: [<tmpl_secret>]
# The PagerDuty integration key (when using PagerDuty integration type `Prometheus`).
service_key: [<tmpl_secret>]

Will accept two new configuration

# The following two options are mutually exclusive.
# The PagerDuty integration key (when using PagerDuty integration type `Events API v2`).
routing_key_config: [<secret_config>]
# The PagerDuty integration key (when using PagerDuty integration type `Prometheus`).
service_key_config: [<secret_config>]

A new configuration type of <secret_config> will be introduced, the config will have the following fields so that users will use this to configure the type and id of the secrets to fetch from.

# "file", "aws_secret_manager", "azure_key_vault", etc.
type: <string>

aws_secret_manager: 
  
  # The arn of the secrets to fetch from
  secret_arn: <tmpl_string>
  
  # How long secrets stay in the memory. After timeout the secret will be refetch from the source
  [ timeout: <duration> | default = 5s ]
  
  # Configures AWS's Signature Verification 4 signing process to sign requests.
  sigv4:
    [ <sigv4_config> ]
  
file: 
  path: <filepath>

Alert manager Examples

aws_secret_manager type example

global:
  resolve_timeout: 5s
route:
  receiver: 'pager-duty-notifications'
  group_by: [alertname, datacenter, app]

receivers:
- name: 'pager-duty-notifications'
  pagerduty_configs:
    - service_key_config:
        type: aws_secret_manager
        aws_secret_manager: 
          secret_arn: 'arn:aws:secretsmanager:us-west-2:111:secret:test-123'
          sigv4:
            region: us-west-2

Prometheus example

basic_auth:
  [ username: <string>]
  [ password_config: <secret_config>]
basic_auth:
  username: test
  password_config: 
    type: aws_secret_manager
    aws_secret_manager: 
      secret_arn: 'arn:aws:secretsmanager:us-west-2:111:secret:test-123'
      sigv4:
        region: us-west-2

Please let me know what do you think

@simonpasquier
Copy link
Member

It sounds interesting but it needs to be consistent across all Prometheus projects (first and foremost Prometheus itself). Did you file a similar issue in github.com/prometheus/prometheus?

@qinxx108
Copy link
Contributor Author

A corresponding issue cut in prometheus: prometheus/prometheus#11477

@qinxx108
Copy link
Contributor Author

Merge the prometheus issue into this one as discussion on the kubecon Contribfest

@davidpellcb
Copy link

davidpellcb commented Oct 27, 2022

can we please just get support for environment variables in the config files 😭 😭

@bwplotka
Copy link
Member

Thanks for this! @qinxx108 - we spoke on ContribFest, and I think it makes total sense.

Indeed we could revisit env vars - but you can do the same by reloading the secret file, so I don't think the environment variable helps here.

I think it makes sense to support more popular secret providers by default - the same as we have for service discovery. I think we should have some entry in the authorization section, perhaps, with aws_secret_manager as an option. Thoughts @simonpasquier @roidelapluie ?

@simonpasquier
Copy link
Member

can we please just get support for environment variables in the config files

I think that it's a bit different from the original request so I wouldn't conflate the 2 issues. As @bwplotka said, maybe it should be revisited but it's a different discussion to have.

I think we should have some entry in the authorization section, perhaps, with aws_secret_manager as an option.

I believe that integration with popular password managers is wider than just for HTTP authentication. In @qinxx108's example, the use case is to inject the PagerDuty key. For Prometheus, it could be useful for many service discoveries too (e.g. inject token into Consul SD configuration).

@roidelapluie
Copy link
Member

roidelapluie commented Nov 25, 2022

I agree with @simonpasquier , I think we should aim for more than http. probably storing all the secrets configuration in a top level section would make sense as well, then just refering to a secret name at the localtion we need access to the secret.

secrets:
- name: hello
  source: vault
  path: secret/data/hello
basic auth:
   username: bar
   password_secret_name: <string>

I am however more against secrets in templates. Templates should not get access to secret data, this seems to bring a higher risk.

@jeremych1000
Copy link

Envvar support would be a great first step. Don't think there would be much pushback against envvars?

@juliusv
Copy link
Member

juliusv commented Mar 23, 2023

Without diving fully into this discussion, just a side link to a previous exploration of env vars in config files Prometheus:

@bwplotka
Copy link
Member

bwplotka commented Mar 23, 2023

We have discussion about this in the DevSummit just now. The consensus is that we are open to secret provider support given:

  • It's the same for Alertmanager and Prometheus
  • It reuses mechanisms and ideas from ServiceDiscovery
  • There is a small design document that answers questions like the YAML syntax, implementation direction (service discovery like plugins or static), provider interface guarantee (will it need caching or not etc).

Help wanted for the design doc! 🤗

@lstellway
Copy link

lstellway commented Aug 25, 2023

Not very elegant, but I found a pretty neat example in the Loki documentation for how to write a file in the entrypoint (allowing for using environment variables).

I was able to get this working by:

  1. Creating an environment of the file contents
  2. Dumping the environment variable into the configuration file location in the entrypoint

Here is an example docker-compose.yml:

# docker-compose.yml
version: "3.8"
services:
  alertmanager:
    container_name: alertmanager
    # Configuration
    # @see https://prometheus.io/docs/alerting/latest/configuration/
    entrypoint:
      - sh
      - -euc
      - |
        # mkdir -p /etc/alertmanager
        echo "$${ALERTMANAGER_CONFIG}" > /etc/alertmanager/alertmanager.yml
        /bin/alertmanager --config.file=/etc/alertmanager/alertmanager.yml --storage.path=/alertmanager
    environment:
      ALERTMANAGER_CONFIG: |
        # Place the entire contents of your alertmanager.yml file here.
        # Reference environment variables as usual with ${VARIABLE_NAME}
        # ...

        global:
          # The smarthost and SMTP sender used for mail notifications.
          smtp_smarthost: smtp.sendgrid.net:465
          smtp_from: alertmanager@example.com
          smtp_auth_username: apikey
          smtp_auth_password: ${SENDGRID_API_KEY}

    image: prom/alertmanager:v0.26.0
    ports:
      - 9093:9093
    restart: unless-stopped

Notice the double $$ for referencing the $ALERTMANAGER_CONFIG variable .. this is required so the variable is not replaced by Docker.

@TheSpiritXIII
Copy link

Hi, I would love for this to move forward. As such, I put together a quick proposal: https://docs.google.com/document/d/1EqHd2EwQxf9SYD8-gl3sgkwaU6A10GhiN7aw-2kx7NU/edit?usp=sharing

Thank you @bwplotka for the DevSummit discussion link and thanks to @roidelapluie for the configuration suggestion!

I could make a new issue for this proposal, if needed. Thanks!

@alvinlin123
Copy link
Contributor

I think using this issue for your proposal is better to avoid duplicate issues :)

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 a pull request may close this issue.

10 participants