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

Credentials in kedro #1646

Closed
antonymilne opened this issue Jun 23, 2022 · 2 comments
Closed

Credentials in kedro #1646

antonymilne opened this issue Jun 23, 2022 · 2 comments
Labels
Component: Configuration Type: Technical DR 💾 Decision Records (technical decisions made)

Comments

@antonymilne
Copy link
Contributor

antonymilne commented Jun 23, 2022

How do credentials currently work in kedro?

The basic pattern is as follows:

# conf/base/catalog.yml
dataset_name:
  ...
  credentials: credentials_key

# conf/local/credentials.yml
credentials_key: 
  kwarg1: value1
  kwarg2: value2
  ...

To be concrete, here's an example for Azure Blob Storage:

# conf/base/catalog.yml
shuttles:
  type: pandas.CSVDataSet
  filepath: abfs://somewhere/shuttles.csv
  credentials: abs_credentials

# conf/local/credentials.yml
abs_credentials:
  account_name: antonymilne
  account_key: verysecretpassword

The credentials key is injected into the call that instantiates pandas.CSVDataSet when kedro is run. Specifically, here:

ds_config = _resolve_credentials(ds_config, credentials)

Note:

  • credentials is a special reserved keyword. This doesn't work for any other key name
  • this is one of the very few customisations that kedro's ConfigLoader makes to how yaml is parsed. In-file variable injection is (kind of) supported in yaml using anchors, but injecting a variable from another file is not. The mechanism that does the injection here is entirely defined by kedro
  • the reason for enabling this custom behaviour is that credentials should not be committed to source control. Hence they need to be stored in a separate file outside the data catalog that lives in local and injected into the catalog at runtime

Does this work well?

In my experience and from talking to users: in the case that credentials can be stored in a file, yes. Very little confusion is caused by the custom behaviour of injecting credentials.

What are the problems with this?

The biggest problem is that credentials might not be stored in a file. Alternatives are:

Another problem with credentials is that the way they are handled for PartitionedDataSet is pretty complicated. I'm not sure we'll be able to solve that here but would be nice if we could.

Possible solutions

Environment variables

At a bare minimum I think we need a way of directly injecting environment variables into credentials. Given how common this is outside credentials files also (using TemplatedConfigLoader), my opinion is that this mechanism should not be credentials-specific but instead common across all kedro configuration.

e.g. with OmegaConf you'd do this as:

abs_credentials:
  account_name: ${oc.env:ABS_ACCOUNT_NAME}
  account_key: ${oc.env:ABS_ACCOUNT_KEY}

Quotes from #770:

@idanov: [credentials] is obviously environment specific, but what we should consider doing is adding an environment variables support. Unfortunately this has been on the backlog for a while, but doesn’t seem to be such an important issue that cannot be solved by DevOps, so we never got to implementing the environment variables for credentials.
@Galileo-Galilei: I do not understand what you mean by [this]. My point is precisely that many CI/CD tools expect to communicate with the underlying application through environment variables (to my knowledge: I must confess that I am far from being a devops expert), and it is really weird to me that is not "native" in kedro. I must switch to the TemplatedConfigLoader on deployment mode even if I use a credentials.yml file while developping, and it feels uncomfortable to have to change something for deployment (even if it is very easy to change).

Beyond environment variables

So far the best discussion of this is in #1280. From @Galileo-Galilei:

I have no time for now, and it will likely take weeks before I came up with something intelligible, but this is a topic on which I plan to write a "Universal Kedro Deployment" issue. I think there are some adherence with this #770, but credentials have a lot of specificities indeed. In short my idea is that:

  • kedro should have an abstract class (~roughly similar to AbstractDataSet, say CredentialsManager) to implement the _get_credentials() function. It should be able to get credentials from anywhere (e.g. VaultCredentialsManager, GithubCredentialsManager and FileCredentialsManager which would default to current implementation) and return a dict of credentials.
  • This class should leverage the ConfigLoader when possible
  • This class should be parametrized in the settings.py

Also worth noting the factory approach of @daBlesr discussed in #711 (comment) and following comments.

I don't yet have any particular ideas myself here so I'd love to hear what others think and hear @Galileo-Galilei's idea in more detail 🚀 It would be especially great to hear from people who use cloud-native credentials systems like AWS secrets. This is a bit of a blindspot for us at the moment I think.

@astrojuanlu
Copy link
Member

A user recently told me that they struggled a bit with Snowflake authentication in particular. Looks like the externalbrowser method we shipped for the Snowpark dataset could be expanded for more normal ones.

References:

@yetudada yetudada moved this from Delivery - Later 🔧 to Shipped 🚀 in Roadmap Feb 20, 2024
@merelcht merelcht removed this from Roadmap Mar 28, 2024
@merelcht
Copy link
Member

@merelcht merelcht closed this as not planned Won't fix, can't repro, duplicate, stale Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Configuration Type: Technical DR 💾 Decision Records (technical decisions made)
Projects
None yet
Development

No branches or pull requests

4 participants