zenml icon indicating copy to clipboard operation
zenml copied to clipboard

[FEATURE]: allow `slack_alerter` stack component to use secrets

Open strangemonad opened this issue 3 years ago • 7 comments

Contact Details [Optional]

[email protected]

Describe the feature you'd like

Since stacks should be shareable, we'd like a way to configure stack components (e.g. the slack alerter) without specifying a secret in the component (e.g. slack.yaml currently contains a slack_token).

It would be nice if components had a systematic way of ensuring they never store secrets and instead required lookup in the stack's secret manager.

Option 1

Allow ComponentBase to resolve sensitive config by merging from secrets. This could either be implicit or explicit

implicit If you want to register a slack component, you must have a slack secret with a schema that contains slack_token = str. The slack secret gets merged into the slack.yaml dict and you create a SlackAlerter(**dict) from that merged result.

explicit You allow specifying exactly which secret to resolve e.g.

slack:
  slack_token: ${my_slack_secret.slack_token} # or some other syntax for referencing a secret explicitly

Option 2

Use the Pydantic built in support for resolving config from environment variables and force the secrets to be loaded as env vars. This wouldn't work very well for us.

  1. In many cases, you don't want to mount your secrets as env vars (e.g. often times you might want to mount secrets as a file or folder
  2. You'd have to get this to work gracefully in bot the local context and the orchestrator context.
  3. I'm not a huge fan of how Pydantic implemented this. It's cumbersome for nested objects and you have to subclass the special Config base class (I.e. regular models can't just be configured to resolve env vars).

Is your feature request related to a problem?

Yes, if we add a slack component to our stack, it contains a sensitive secret so we can't share that stack.

How do you solve your current problem with the current status-quo of ZenML?

We create a slack component with an invalid token and create a custom slack alerter that dynamically queries the secrets manager for a slack secret that has a slack_token key-value pair and overrides the SlackAlerter.slack_token property before sending a slack alert.

Any other comments?

No response

strangemonad avatar Jul 04 '22 20:07 strangemonad

Hi @strangemonad, thanks for the issue.

Some of our existing components (e.g. the AzureArtifactStore) already use an authentication mixin. This mixin:

  • adds an authentication_secret attribute to the component which can be used to specify a secret name
  • verifies the secret is of an expected schema

This approach is pretty limited (e.g. only allows a single secret, requires a manual call to resolve it) and only implemented for a few components so far. It however has the upside that it only requires users to specify a single secret name, which might be nice in case a component requires multiple secrets for authentication:

class AuthenticationSecretSchema(BaseSecretSchema):
    key_1: str
    key_2: str
    key_3: str

# using our current solution of explicitly specifying the secret name
class ComplexComponentA(StackComponent):
    authentication_secret: str = "authentication_secret"

    def method_that_uses_secret():
        secret = self.resolve_secret(self.authentication_secret)
        # use the secret values
        do_something(secret.key_1, ...)
    
# specifying secret keys manually (explicit option 1)
class ComplexComponentB(StackComponent):
    key_1: str = "${authentication_secret.key_1}"
    key_2: str = "${authentication_secret.key_2}"
    key_3: str = "${authentication_secret.key_3}"

    def method_that_uses_secret():
        do_something(self.key_1, ...)

While this works for our existing components, we think that it lacks some flexibility and came to a very similar conclusion to your explicit option 1, even with pretty much the same syntax.

If we were to implement explicit option 1, do you think it makes sense to have it coexist with our current solution or switch everything to the new approach?

schustmi avatar Jul 05 '22 15:07 schustmi

That makes sense @schustmi

1. Keep the current solution?

As much as possible, I'd love if the zenml architecture gently coaxes you into security best practices the same way the pipelines and steps API "tricks" you into writing robust production pipelines from the get go.

a) My personal preference would be to always use secrets for sensitive component information so that stacks are always shareable BUT streamline setting all this up so it's more user friendly than, say kubernetes config maps and secrets. For example,

  • when registering a secret-dependent-component on the cli, the registration could check that you have a secret manager already configured and interactively prompt you for component config and then secret fields and automatically create the required secret. This could be pretty easy to streamline if the default stack always had a default local secrets manager?
  • It would be super useful for component validation to check that it can resolve the secret as part of validation.
  • Stack export probably needs some warning message (e.g. "secrets <SecretName> with schema <SecretSchema> were not exported". Stack import can probably leverage the component create cli flow to prompt for missing secrets when importing a stack so that after import, the stack components are valid.

b) if the desire is to keep the current approach, I'd still recommend somehow tagging this pydantic field as a SecretField(str, default="") (or something like that) and warning when you do a pipeline.run() that the current stack is configured in a way that's not suitable for production settings. Maybe this mode is just supported for the local orchestrator? The main thing I anticipate is trouble auditing the various stacks people create and I'd hate for secrets to be passed in plain text to a kubeflow orchestrator pod environment and not easily be able to audit where in the system that's happening.

2. Usability limitations of the explicit approach

It might still be nice to create a secret loading mechanism that has sane defaults so you don't have to do all the explicit manual wiring you need to do with Kubernetes configs e.g. maybe something like slack_token: str = SecretField(type=str, secret_name="slack", secret_key="slack_token")? I didn't think about this API very much and it might require more refining. The main idea is that it encodes a default secret name and schema.

strangemonad avatar Jul 05 '22 23:07 strangemonad

@strangemonad just as a quick reply to your first point, I really like the ideas however there is a bit more complexity hiding here as it the stack component creation does not take place in the context of the eventual stack(s) that the component will end up in. So when I am creating my alerter, i will only be able to add it to an existing stack or create an existing stack after creation of the component. The relevant secret manager component might not even exist yet. And the secret itself will only be creatable once the stack with the alerter and secret manager are active.

However, what we could maybe do is add some component-validation when activating/using the stack that makes sure that the secrets of all stack components are present. This could then prompt us to create the secrets with the correct name and schema. The same would then also apply for stacks that have been imported.

AlexejPenner avatar Jul 06 '22 09:07 AlexejPenner

Ah right. I wasn't thinking about how components can be joined to multiple stacks.

I don't have the code in front of me right now but that would imply you light be able to run the component validator when adding the component to the stack? Maybe that's also the correct point for the Cli to (optionally) interactively prompt for the secrets if they don't exist?

Just thinking further, since there's currently no additional config that lives with the stack-component binding, that means that if the secret-mapping lives with the component, the secret with the same name would need to exist in all stacks you want to add the component to or the secret mapping needs to live with the component-stack binding (that seems more confusing for end users since there's already a lot of concepts with profiles, stacks, and components).

For what it's worth, though this may evolve, we currently treat profiles as environments and I currently just have 1 secret manager per profile

strangemonad avatar Jul 06 '22 14:07 strangemonad

You are totally right about the potential confusion across profiles, stacks, stack components, etc. - the way you are describing it, it sounds like you would prefer to use some stack components (like the secrets manager) as defacto profile component that is shared across stacks - definitely an interesting thought.

Regarding the interactive secret prompt after validation, this should be doable, I'll take this with me into our next Sprint Planning.

AlexejPenner avatar Jul 12 '22 15:07 AlexejPenner

@strangemonad FYI we're introducing secret scoping that will give users better control over how secrets are shared across secrets manager components: https://github.com/zenml-io/zenml/pull/803

stefannica avatar Jul 29 '22 13:07 stefannica

@strangemonad #817 implements a generic way of referencing secrets in configurations of stack components as well as many other details discussed here. It currently does not include any automated prompting for secrets when a component is registered or added to a stack, as even in the second case there might not be a secrets manager. We instead opted for a simple CLI command zenml stack register-secrets which interactively prompts the user for all secrets in a given stack.

schustmi avatar Aug 08 '22 13:08 schustmi