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

fix(vaultsecrets.secrets.create): parse correct secret-type key #216

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

syaghoubi00
Copy link

@syaghoubi00 syaghoubi00 commented Feb 25, 2025

πŸ› οΈ Description

Parses correct key from hcp vault-secrets secrets create --secret-type

Parses the literal string static from flag arg

πŸ”— Additional Link

Closes: #215

πŸ—οΈ Local Testing

podman run --rm -it fedora
dnf install -y go
git clone ${REPO}
make go/test
make go/build
echo test | ./bin/hcp vault-secrets secrets create --secret-type=static --data-file=- test_secret

πŸ‘ Checklist

  • The PR has a descriptive title.
  • Input validation updated
  • Unit tests updated
  • Documentation updated
  • Major architecture changes have a corresponding RFC
  • Tests added if applicable
  • CHANGELOG entry added or label 'pr/no-changelog' added to PR

    Run CHANGELOG_PR=<PR number> make changelog/new-entry for guidance
    in authoring a changelog entry, and commit the resulting file, which should
    have a name matching your PR number. Entries should use imperative present
    tense (e.g. Add support for...)

@syaghoubi00 syaghoubi00 requested a review from a team as a code owner February 25, 2025 02:40
Copy link

hashicorp-cla-app bot commented Feb 25, 2025

CLA assistant check
All committers have signed the CLA.

Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

Copy link

This PR is more than 2 weeks old. Please remove the stale label, update, or comment if this PR is still valid and relevant, otherwise it will be closed in 7 days.

@github-actions github-actions bot added the stale label Mar 13, 2025
@syaghoubi00
Copy link
Author

Added changelog entry. Is anything else required?

@github-actions github-actions bot removed the stale label Mar 16, 2025
@dhuckins
Copy link
Contributor

hi @syaghoubi00 thanks for the contribution!

this would mean a breaking change for anyone who uses hcp vault-secrets secrets create --secret-type kv, to prevent that from breaking would it be possible to allow both "kv" and "static" as acceptable to create a kv secret?

@syaghoubi00
Copy link
Author

@dhuckins I went ahead and preserved the kv opt. This is actually the better fix, because the secretTypeKV is used in more places than I found at first. This will allow the interface to the secretTypeKV to remain consistent in code, while still parsing static from the arg opts.

It might be a good idea to normalize all the opts input, since they are being parsed as string literals. eg. =Static and =STATIC are both 'valid' opts, so adding something like input.ToLowercase might be better for doing the case evals.

const (
secretTypeKV = "kv"
secretTypeDynamic = "dynamic"
secretTypeRotating = "rotating"
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hcp vault-secrets secrets create --secret-type does not accept the correct key
2 participants