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

DEVPROD-12947: use hash of project ID in parameter name #8500

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

Conversation

Kimchelly
Copy link
Contributor

@Kimchelly Kimchelly commented Nov 21, 2024

DEVPROD-12947

Description

I made an assumption that nobody would choose project IDs that have special characters (e.g. my cool project (v1.2)) when they could do that in the display name or identifier. Unfortunately this isn't true - a couple projects in production chose project IDs that have special characters in them. The project ID is used in parameter paths to give each project their own unique namespace, but if they have special characters, that means we can't use the project ID as-is. For example, /evergreen/my cool project (v1.2)/myVariable is an invalid parameter path because Parameter Store doesn't allow spaces and parentheses.

To work around this, I changed the parameter path to use the SHA256 hash of the project ID, which transforms all project IDs into 64-character hexadecimal strings. So far, nobody's found a collision in SHA256 hashes, so each project ID maps to a unique hash.

  • Use hash of the project ID in the parameter path.
  • Include /vars/ as part of the parameter path. This isn't necessary, but it does more neatly group the project vars parameters in the hierarchy and makes it easier to identify what secrets are stored where when we use Parameter Store more widely.

Testing

Updated existing unit tests.

Documentation

N/A

@Kimchelly Kimchelly requested a review from a team November 21, 2024 20:38
util/hash.go Outdated
// GetSHA256Hash returns the SHA256 hash of the given string.
func GetSHA256Hash(s string) string {
hasher := utility.NewSHA256Hash()
hasher.Write([]byte(s))
Copy link
Contributor

Choose a reason for hiding this comment

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

We should handle the error here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, I meant to use Add actually. Changed it.

sha256 doesn't actually return an error, it's just conforming to the io.Writer interface.

@@ -1140,9 +1151,14 @@ func convertVarToParam(projectID string, pm ParameterMappings, varName, varValue
return "", "", errors.Errorf("project variable '%s' cannot have an empty value", varName)
}

prefix := GetVarsParameterPath(projectID) + "/"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we fmt.Sprintf("%s/", GetVarsParameterPath(projectID)) instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

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.

2 participants