Skip to content
This repository has been archived by the owner on Jul 26, 2022. It is now read-only.

feat(ibm): Support looking up secrets in IBM Cloud Secret Manager by name #850

Merged
merged 1 commit into from
Nov 17, 2021

Conversation

davesteinberg
Copy link
Contributor

This PR adds support for using names, instead of IDs, as the keys in ExternalSecrets with IBM Cloud Secrets Manager.

With the current backend implementation, the key on each data item is assumed to be an ID, which is assigned to each secret by Secrets Manager on creation. Secrets Manager's API and CLI are totally oriented around these IDs, which are generally not meaningful or useful to users. Users assign a name, not an ID, to secrets, and they should be able to reference them by that name. Fortunately, Secrets Manager's API does allow you to search for a secret by name and get its ID, so clients can use that to support referencing secrets by name.

If I am defining ExternalSecrets in YAML and committing it to Git, I definitely want to put the names of the secrets in those YAMLs, not the IDs, which are specific to a particular instance of Secrets Manager (so they will always be different across different environments) and even a particular instance of the secret (if I have to delete a secret and create a new one with the same name, it will have a new ID). If I can't use names, I will have to update my YAMLs every time after adding the secrets to Secrets Manager, and I will be unable to share the same YAMLs across environments.

My approach was not to change the default behaviour, since it has been released and is probably in use already. Rather, I added support for a new keyByNameproperty on the spec. If true, keys will be treated as names, and the backend will do the two-step lookup to find the ID by name and then the secret value by ID.

While I was developing this, I really wanted the ability to test the backend code against a real instance of Secrets Manager, so I modified the unit tests to allow them to run with our without the stubs. A developer can set a few environment variables to disable the stubs and talk to a real Secrets Manager instance. By default, they'll run with the stubs for a fast, dependency-free unit test. I also added a few missing test cases to cover secret types that are supported by the existing code (arbitrary and iam_credentials).

I am currently using this successfully in a project, so I am confident that it works, at least.

…stead of id, as the key

* Look up secrets based on names when keyByName is true.
* Allow unit tests for the backend running without mocking in development.
* Add missing unit tests for generic secrets and IAM credentials.
* Add keyByName property to CRD.
* Document keyByName in README.
@Flydiverny
Copy link
Member

Thanks for the contribution! This does indeed look nice.
It would be interesting to have a look on this from ESO (https://github.com/external-secrets/external-secrets) perspective, since I'm not an IBM user I'm not sure if a similar contribution could be valuable there.

I'm a bit hesitant to add this to KES since it would make migration to ESO harder if it doesn't support the same feature, and with KES on mainly life support I think its important to consider :)

See #864

@davesteinberg
Copy link
Contributor Author

@Flydiverny Thanks for the feedback!

I wasn't even aware of ESO when I submitted this PR, but now that I am, I do believe that this sort of enhancement would be very valuable there, as well, for the same reasons that I outlined above. I see that ESO isn't at 1.0 yet, so I wonder if it would be possible to get name support in before then. What's the best way to get the attention of the folks who are working on that backend?

If I were going to try to contribute it, I would certainly like to get some buy-in first, since I don't know Go at all, so it would be quite a bit more a time investment.

@Flydiverny
Copy link
Member

This slipped past me, I think the best way to get in contact would be to reach out on slack https://kubernetes.slack.com/archives/C017BF84G2Y

@Flydiverny Flydiverny merged commit 20496ab into external-secrets:master Nov 17, 2021
@Flydiverny
Copy link
Member

Released as 8.4.0

@davesteinberg davesteinberg deleted the ibm-names branch December 15, 2021 19:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants