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

VAULT-28317: Fix issue with lowercasing of HCP resources #38

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

VioletHynes
Copy link
Contributor

@VioletHynes VioletHynes commented Jul 2, 2024

Overview

This fixes an issue where the names of HCP resources would be lowercased. Since HCP resources are case sensitive, this means that if you have e.g. "foo" and "FOO", you would be unable to access one of them.

Likewise, if you had a project called "FOO", you could previously only access it via typing "foo", which was a pain point forsome users.

This could affect a customer that was previously accessing "FOO" by typing "foo", but ultimately that's on them, since these resources are case sensitive.

I tested this in Vault to fix issues found in vault hcp connect using the following:
replace github.com/hashicorp/vault-hcp-lib => ../vault-hcp-lib

Once merged, I'll update this in Vault, and backport it etc.

Contributor Checklist

  • [Will be added after] Add relevant docs to upstream Vault repository, or sufficient reasoning why docs won’t be added yet
  • [N/A] Add output for any tests not ran in CI to the PR description (eg, acceptance tests)
  • [It has a minor incompatibility if the user was writing the name of their uppercase project in lowercase but fixes a bug] Backwards compatible

@VioletHynes VioletHynes requested a review from a team as a code owner July 2, 2024 20:35
@@ -382,7 +382,7 @@ func Test_getProject(t *testing.T) {
// Test multiple projects
// UI interaction required
"multiple projects": {
userInputProjectName: "mock-project-2\n",
userInputProjectName: "MOCK_-project-2\n",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests would fail before the fix, and are valid project names.

Copy link
Contributor

@ccapurso ccapurso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL the resource names are case sensitive. Looks good!

@VioletHynes VioletHynes requested a review from divyaac July 3, 2024 18:03
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.

4 participants