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

enable transit secret engine at another path #67

Merged
merged 1 commit into from
May 27, 2021
Merged

enable transit secret engine at another path #67

merged 1 commit into from
May 27, 2021

Conversation

developer-guy
Copy link
Member

Signed-off-by: Batuhan Apaydın batuhan.apaydin@trendyol.com

I have a problem with storing keys in the Vault transit engine. cosign waits for me to enable Vault Transit Secret Engine at path transit/, but in Vault, I don’t have to enable secret engines at its default paths, so, I can override this behavior by using -path option while enabling it. So, in the scenario I have been enabled Transit Secret Engine at a different path, cosign throws an error like the following:
Screen Shot 2021-05-26 at 20 32 19

if you want to reproduce that error above, you can type the following commands:

$ vault server -dev -dev-root-token-id root
$ export VAULT_ADDR="http://localhost:8200"
$ export VAULT_TOKEN="root"
$ vault secrets enable -path=keystore transit
$ cosign generate-key-pair -kms hashivault://test-key

This PR is aiming to solve this problem that I mentioned above.

Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com>
@developer-guy
Copy link
Member Author

cc: @dlorenc @Dentrax

@dlorenc
Copy link
Member

dlorenc commented May 27, 2021

Hmm, maybe one idea - what if we put this in the "vault_addr" env var rather than adding a new one? If there's no "suffix", we can default to "/transit", otherwise we can just use the full value. Does that make sense?

@developer-guy
Copy link
Member Author

developer-guy commented May 27, 2021

Hmm, maybe one idea - what if we put this in the "vault_addr" env var rather than adding a new one? If there's no "suffix", we can default to "/transit", otherwise we can just use the full value. Does that make sense?

Hello @dlorenc, thank you for the idea but the VAULT_ADDR environment variable is kind of a special one for Vault, it is better not to change this in order to keep consistent with Vault environment variables, what do you think?. Btw, this is why I don't put the VAULT_ prefix to my environment variable to show that this is not a special environment variable that Vault needs.

@dlorenc
Copy link
Member

dlorenc commented May 27, 2021

Ah! Makes sense :)

@dlorenc
Copy link
Member

dlorenc commented May 27, 2021

One last thing - we've been trying to get environment variables out of the "library" and "package" code for the most part - that usually belongs in the "application" code that will eventually call this library. What do you think about leaving the environment variable out of here, and setting it in cosign or the other clients?

@dlorenc
Copy link
Member

dlorenc commented May 27, 2021

Nevermind, we discussed this would be too tough to get right.

@cpanato cpanato added this to the 0.1.0 milestone May 27, 2021
@cpanato cpanato added this to In Progress in Sigstore Release board May 27, 2021
@dlorenc dlorenc merged commit ec2a7bf into sigstore:main May 27, 2021
Sigstore Release board automation moved this from In Progress to Done May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants