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

Should we allow optional secret provider registrations? #200

Closed
stijnmoreels opened this issue Oct 14, 2020 · 13 comments
Closed

Should we allow optional secret provider registrations? #200

stijnmoreels opened this issue Oct 14, 2020 · 13 comments
Labels
proposal All current proposals secret-provider All issues related to secret providers secret-store All issues related to our secret store specs-required All issues where the specifications are still being defined and implementation should be halted

Comments

@stijnmoreels
Copy link
Member

Is your feature request related to a problem? Please describe.
As proposed here, @fgheysels makes the point that we could include optional registrations for the secret providers in the secret store (like provided in the configuration providers).

Describe the solution you'd like
What needs to be figured out is:

  1. Should we provide the option to register all secret providers as optional registrations?
  2. If so, should we ignore/log the exceptions when the secret is not found instead of blocking?
  3. If so, should we allow invalid/incorrect setup of the secret provider with possible unauthorized exceptions to be ignored when the secret provider was registered as optional?

Additional context
#171 (comment)

@stijnmoreels stijnmoreels added proposal All current proposals specs-required All issues where the specifications are still being defined and implementation should be halted secret-store All issues related to our secret store secret-provider All issues related to secret providers labels Oct 14, 2020
@tomkerkhove
Copy link
Contributor

Isn't that how it works today? If it can't find it in the first provider, it will go to the second, then to the third, etc.

Or am I not getting the proposal correctly?

@stijnmoreels
Copy link
Member Author

No, it's more about, if the 'setup' of the provider fails, should we block the secret store or should we ignore it (optional).

@tomkerkhove
Copy link
Contributor

So it would be to not block on configuration issues then, is that correct?

Interesting 🤔

So @fgheysels his initial comment (#171 (comment)) was:

I've changed this, but now this means that if you specify a non-existing directory, the container will fail to start. That is a good example of failing fast but I don't know if you really want that.

What if you have a program which has multiple secret providers configured. In some cases, your program might not be running in a container and needs to retrieve secrets from another location. In that case, you still need to have that directory present.

In this case, wouldn't it be more suitable to make the folder name configurable? I'm just not sure what the exact scenario would be other than this 🤔

@stijnmoreels
Copy link
Member Author

It's I think more about aligning with the configuration provider. Where also mostly have an option to make it optional. Like when adding the app settings and adding an additional app settings file for local development.

@fgheysels
Copy link
Member

True.
Or, in this case for a code base which is deployed as a container but that same codebase can also be deployed on an Azure AppService (without a container). When we register a secret provider like the 'Docker Secrets Provider', and the 'secretsFolder' that is passed to that secret provider doesn't exist, an exception is thrown. When we run the same codebase in an Azure App Service, that folder will most like not be there.

@tomkerkhove
Copy link
Contributor

tomkerkhove commented Oct 20, 2020

Makes sense, thanks for explaining further! Let's make it so 😎

@stijnmoreels
Copy link
Member Author

We still have to decide on the questions described in the issue here. We should have a clear distinction on what this means for the secret store.

@stijnmoreels
Copy link
Member Author

According to this, both connection issues and configuration issues are ignored by optional. But Azure related uses sometimes different defaults.

optional
This setting is a boolean that specified whether to avoid throwing exceptions when the backing configuration source cannot be found or connected. The default default value is true, though some config builders (such as the Azure-based builders) will use a different default.

https://github.com/aspnet/MicrosoftConfigurationBuilders#optional

@stijnmoreels
Copy link
Member Author

We can wait for #211 to complete.

@tomkerkhove
Copy link
Contributor

@fgheysels would it make sense to have it more on a per-provider level so that the docker secrets can coop with this then in case the file is not there for example?

@stijnmoreels
Copy link
Member Author

Should we include this too in the v1.4.0 milestone? And if not, the next?

@tomkerkhove
Copy link
Contributor

Let's wait until we know what to do

@stijnmoreels
Copy link
Member Author

Closed in favor of #241

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal All current proposals secret-provider All issues related to secret providers secret-store All issues related to our secret store specs-required All issues where the specifications are still being defined and implementation should be halted
Projects
None yet
Development

No branches or pull requests

3 participants