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
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion model/project_ref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,8 @@ func checkAndSetProjectVarsSynced(t *testing.T, projRef *ProjectRef, isRepoRef b
// project vars all include the project ID as a prefix.
func checkParametersNamespacedByProject(t *testing.T, vars ProjectVars) {
projectID := vars.Id
commonAndProjectIDPrefix := fmt.Sprintf("/%s/%s/", strings.TrimSuffix(strings.TrimPrefix(evergreen.GetEnvironment().Settings().Providers.AWS.ParameterStore.Prefix, "/"), "/"), projectID)

commonAndProjectIDPrefix := fmt.Sprintf("/%s/%s/", strings.TrimSuffix(strings.TrimPrefix(evergreen.GetEnvironment().Settings().Providers.AWS.ParameterStore.Prefix, "/"), "/"), GetVarsParameterPath(projectID))
for _, pm := range vars.Parameters {
assert.True(t, strings.HasPrefix(pm.ParameterName, commonAndProjectIDPrefix), "parameter name '%s' should have standard prefix '%s'", pm.ParameterName, commonAndProjectIDPrefix)
}
Expand Down
19 changes: 17 additions & 2 deletions model/project_vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -1126,6 +1126,17 @@ func (projectVars *ProjectVars) MergeWithRepoVars(repoVars *ProjectVars) {
sort.Sort(projectVars.Parameters)
}

// GetVarsParameterPath returns the parameter path for project variables in the
// given project.
func GetVarsParameterPath(projectID string) string {
// Include a hash of the project ID in the parameter name for uniqueness.
// The hashing is necessary because project IDs are unique but some
// existing projects contain characters (e.g. spaces) that are invalid for
// parameter names.
hashedProjectID := util.GetSHA256Hash(projectID)
return fmt.Sprintf("vars/%s", hashedProjectID)
}

// convertVarToParam converts a project variable to its equivalent parameter
// name and value. In particular, it validates that the variable name and value
// fits within parameter constraints and if the name or value doesn't fit in the
Expand All @@ -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.


varsToParams := pm.NameMap()
m, ok := varsToParams[varName]
if ok {
if ok && strings.Contains(m.ParameterName, prefix) {
// Only reuse the existing parameter name if it exists and already
// contains the required prefix. If it doesn't have the required prefix,
// then a new parameter has to be created.
paramName = m.ParameterName
} else {
paramName, err = createParamBasenameForVar(varName)
Expand All @@ -1156,7 +1172,6 @@ func convertVarToParam(projectID string, pm ParameterMappings, varName, varValue
return "", "", errors.Wrapf(err, "getting compressed parameter name and value for project variable '%s'", varName)
}

prefix := fmt.Sprintf("%s/", projectID)
if !strings.Contains(paramName, prefix) {
paramName = fmt.Sprintf("%s%s", prefix, paramName)
}
Expand Down
112 changes: 68 additions & 44 deletions model/project_vars_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -756,60 +756,67 @@ func TestAWSVars(t *testing.T) {
func TestConvertVarToParam(t *testing.T) {
t.Run("ReturnsNewParamNameForValidVarNameAndValue", func(t *testing.T) {
const (
varName = "var_name"
varValue = "var_value"
varName = "var_name"
varValue = "var_value"
projectID = "project_id"
)
paramName, paramValue, err := convertVarToParam("project_id", ParameterMappings{}, varName, varValue)
paramName, paramValue, err := convertVarToParam(projectID, ParameterMappings{}, varName, varValue)
require.NoError(t, err)
assert.Equal(t, "project_id/var_name", paramName, "new parameter name should include project ID prefix")
assert.Equal(t, fmt.Sprintf("%s/%s", GetVarsParameterPath(projectID), varName), paramName, "new parameter name should include project ID prefix")
assert.Equal(t, varValue, paramValue, "variable value is valid and should be unchanged")
})
t.Run("ReturnsValidParamNameForVarContainingDisallowedAWSPrefix", func(t *testing.T) {
const (
varName = "aws_secret"
varValue = "super_secret"
varName = "aws_secret"
varValue = "super_secret"
projectID = "project_id"
)
paramName, paramValue, err := convertVarToParam("project_id", ParameterMappings{}, varName, varValue)
paramName, paramValue, err := convertVarToParam(projectID, ParameterMappings{}, varName, varValue)
require.NoError(t, err)
assert.Equal(t, "project_id/_aws_secret", paramName, "new parameter name should prevent invalid 'aws' prefix from appearing in basename")
assert.Equal(t, fmt.Sprintf("%s/_%s", GetVarsParameterPath(projectID), varName), paramName, "new parameter name should prevent invalid 'aws' prefix from appearing in basename")
assert.Equal(t, varValue, paramValue, "parameter value should match variable value because variable value is valid")
})
t.Run("ReturnsValidParamNameForVarContainingDisallowedSSMPrefix", func(t *testing.T) {
const (
varName = "ssm_secret"
varValue = "super_secret"
varName = "ssm_secret"
varValue = "super_secret"
projectID = "project_id"
)
paramName, paramValue, err := convertVarToParam("project_id", ParameterMappings{}, varName, varValue)
paramName, paramValue, err := convertVarToParam(projectID, ParameterMappings{}, varName, varValue)
require.NoError(t, err)
assert.Equal(t, "project_id/_ssm_secret", paramName, "new parameter name should prevent invalid 'ssm' prefix from appearing in basename")
assert.Equal(t, fmt.Sprintf("%s/_%s", GetVarsParameterPath(projectID), varName), paramName, "new parameter name should prevent invalid 'ssm' prefix from appearing in basename")
assert.Equal(t, varValue, paramValue, "parameter value should match variable value because variable value is valid")
})
t.Run("ReturnsExistingParameterNameAndNewVarValueForVarWithExistingParameter", func(t *testing.T) {
const (
varName = "var_name"
varValue = "var_value"
existingParamName = "/prefix/project_id/var_name"
varName = "var_name"
varValue = "var_value"
projectID = "project_id"
)
existingParamName := fmt.Sprintf("/prefix/%s/%s", GetVarsParameterPath(projectID), varName)
pm := ParameterMappings{
{
Name: varName,
ParameterName: existingParamName,
},
}
paramName, paramValue, err := convertVarToParam("project_id", pm, varName, varValue)
paramName, paramValue, err := convertVarToParam(projectID, pm, varName, varValue)
require.NoError(t, err)
assert.Equal(t, existingParamName, paramName, "should return already-existing parameter name")
assert.Equal(t, varValue, paramValue, "parameter value should match variable value")
})
t.Run("ReturnsNewParameterNameAndCompressedParameterValueWhenVarValueExceedsLimit", func(t *testing.T) {
const varName = "var_name"
const (
varName = "var_name"
projectID = "project_id"
)

longVarValue := strings.Repeat("abc", parameterstore.ParamValueMaxLength)
assert.Greater(t, len(longVarValue), parameterstore.ParamValueMaxLength)

paramName, paramValue, err := convertVarToParam("project_id", ParameterMappings{}, varName, longVarValue)
paramName, paramValue, err := convertVarToParam(projectID, ParameterMappings{}, varName, longVarValue)
require.NoError(t, err)
assert.Equal(t, "project_id/var_name.gz", paramName, "should include project ID prefix and gzip extension to indicate the value was compressed")
assert.Equal(t, fmt.Sprintf("%s/%s.gz", GetVarsParameterPath(projectID), varName), paramName, "should include project ID prefix and gzip extension to indicate the value was compressed")
assert.NotEqual(t, longVarValue, paramValue, "compressed value should not match original variable value")

gzr, err := gzip.NewReader(strings.NewReader(paramValue))
Expand All @@ -820,9 +827,10 @@ func TestConvertVarToParam(t *testing.T) {
})
t.Run("ReturnsNewParameterNameAndCompressedParameterValueWhenLengthOfExistingVarIncreasesBeyondLimit", func(t *testing.T) {
const (
varName = "var_name"
existingParamName = "/prefix/project_id/var_name"
varName = "var_name"
projectID = "project_id"
)
existingParamName := fmt.Sprintf("/prefix/%s/%s", GetVarsParameterPath(projectID), varName)
pm := ParameterMappings{
{
Name: varName,
Expand All @@ -833,10 +841,10 @@ func TestConvertVarToParam(t *testing.T) {
longVarValue := strings.Repeat("abc", parameterstore.ParamValueMaxLength)
assert.Greater(t, len(longVarValue), parameterstore.ParamValueMaxLength)

paramName, paramValue, err := convertVarToParam("project_id", pm, varName, longVarValue)
paramName, paramValue, err := convertVarToParam(projectID, pm, varName, longVarValue)
require.NoError(t, err)
assert.NotEqual(t, existingParamName, paramName)
assert.Equal(t, existingParamName+gzipCompressedParamExtension, paramName, "project variable that was previously short but now is long enoguh to require compression should have its parameter name changed")
assert.Equal(t, fmt.Sprintf("%s.gz", existingParamName), paramName, "project variable that was previously short but now is long enough to require compression should have its parameter name changed")
assert.NotEqual(t, longVarValue, paramValue)

gzr, err := gzip.NewReader(strings.NewReader(paramValue))
Expand All @@ -847,51 +855,58 @@ func TestConvertVarToParam(t *testing.T) {
})
t.Run("ReturnsErrorForEmptyVariableName", func(t *testing.T) {
const (
varName = ""
varValue = "var_value"
varName = ""
varValue = "var_value"
projectID = "project_id"
)
_, _, err := convertVarToParam("project_id", ParameterMappings{}, varName, varValue)
_, _, err := convertVarToParam(projectID, ParameterMappings{}, varName, varValue)
assert.Error(t, err, "should not allow variable with empty name")
})
t.Run("ReturnsErrorForEmptyVariableValue", func(t *testing.T) {
const (
varName = "var_name"
varValue = ""
varName = "var_name"
varValue = ""
projectID = "project_id"
)
_, _, err := convertVarToParam("project_id", ParameterMappings{}, varName, varValue)
_, _, err := convertVarToParam(projectID, ParameterMappings{}, varName, varValue)
assert.Error(t, err, "should not allow variable with empty value")
})
t.Run("ReturnsErrorForVariableNameEndingInGzipExtension", func(t *testing.T) {
const (
varName = "var_name.gz"
varValue = "var_value"
varName = "var_name.gz"
varValue = "var_value"
projectID = "project_id"
)
_, _, err := convertVarToParam("project_id", ParameterMappings{}, varName, varValue)
_, _, err := convertVarToParam(projectID, ParameterMappings{}, varName, varValue)
assert.Error(t, err, "should not allow variable with gzip extension")
})
t.Run("ReturnsErrorForVarValueThatExceedsMaxLengthAfterCompression", func(t *testing.T) {
const varName = "var_name"
const (
varName = "var_name"
projectID = "project_id"
)
// Since this is a purely random string, there's no realistic way to
// compress it to fit within the max length limit.
longVarValue := utility.MakeRandomString(10 * parameterstore.ParamValueMaxLength)
assert.Greater(t, len(longVarValue), parameterstore.ParamValueMaxLength)

_, _, err := convertVarToParam("project_id", ParameterMappings{}, varName, longVarValue)
_, _, err := convertVarToParam(projectID, ParameterMappings{}, varName, longVarValue)
assert.Error(t, err)
})
t.Run("ReturnsErrorForNewParameterThatWouldConflictWithExistingParameter", func(t *testing.T) {
const (
varName = "aws_secret"
varValue = "super_secret"
varName = "aws_secret"
varValue = "super_secret"
projectID = "project_id"
)
pm := ParameterMappings{
{
Name: "_aws_secret",
ParameterName: "/prefix/project_id/_aws_secret",
ParameterName: fmt.Sprintf("/prefix/%s/_%s", GetVarsParameterPath(projectID), varName),
},
}

_, _, err := convertVarToParam("project_id", pm, varName, varValue)
_, _, err := convertVarToParam(projectID, pm, varName, varValue)
assert.Error(t, err, "should not allow creation of new parameter whose name conflicts with an already-existing parameter")
})
}
Expand All @@ -901,8 +916,9 @@ func TestConvertParamToVar(t *testing.T) {
const (
varName = "var_name"
varValue = "var_value"
paramName = "/prefix/project_id/var_name"
projectID = "project_id"
)
paramName := fmt.Sprintf("/prefix/%s/%s", GetVarsParameterPath(projectID), varName)
pm := ParameterMappings{
{
Name: varName,
Expand All @@ -918,8 +934,9 @@ func TestConvertParamToVar(t *testing.T) {
const (
varName = "aws_secret"
varValue = "super_secret"
paramName = "/prefix/project_id/_aws_secret"
projectID = "project_Id"
)
paramName := fmt.Sprintf("/prefix/%s/_%s", GetVarsParameterPath(projectID), varName)
pm := ParameterMappings{
{
Name: varName,
Expand All @@ -932,10 +949,14 @@ func TestConvertParamToVar(t *testing.T) {
assert.Equal(t, varValue, varValueFromParam, "should return original variable value")
})
t.Run("ReturnsErrorForVariableMissingParameterMapping", func(t *testing.T) {
const (
varName = "var_name"
projectID = "project_id"
)
pm := ParameterMappings{
{
Name: "var_name",
ParameterName: "/prefix/project_id/var_name",
Name: varName,
ParameterName: fmt.Sprintf("/prefix/%s/%s", GetVarsParameterPath(projectID), varName),
},
}
varNameFromParam, varValueFromParam, err := convertParamToVar(pm, "some_other_var_name", "some_other_var_value")
Expand All @@ -945,8 +966,10 @@ func TestConvertParamToVar(t *testing.T) {
})
t.Run("ReturnsErrorForParameterThatMapsToEmptyVariable", func(t *testing.T) {
const (
paramName = "/prefix/project_id/var_name"
projectID = "project_id"
varName = "var_name"
)
paramName := fmt.Sprintf("/prefix/%s/%s", GetVarsParameterPath(projectID), varName)
pm := ParameterMappings{
{
ParameterName: paramName,
Expand All @@ -960,8 +983,9 @@ func TestConvertParamToVar(t *testing.T) {
t.Run("DecompressesParameterValueToOriginalVariableValue", func(t *testing.T) {
const (
varName = "var_name"
paramName = "/prefix/project_id/var_name.gz"
projectID = "project_id"
)
paramName := fmt.Sprintf("/prefix/%s/%s.gz", GetVarsParameterPath(projectID), varName)

longVarValue := strings.Repeat("abc", parameterstore.ParamValueMaxLength)
assert.Greater(t, len(longVarValue), parameterstore.ParamValueMaxLength)
Expand Down
2 changes: 1 addition & 1 deletion rest/data/project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ func checkAndSetProjectVarsSynced(t *testing.T, projRef *model.ProjectRef, isRep

func checkParametersNamespacedByProject(t *testing.T, vars model.ProjectVars) {
projectID := vars.Id
commonAndProjectIDPrefix := fmt.Sprintf("/%s/%s/", strings.TrimSuffix(strings.TrimPrefix(evergreen.GetEnvironment().Settings().Providers.AWS.ParameterStore.Prefix, "/"), "/"), projectID)
commonAndProjectIDPrefix := fmt.Sprintf("/%s/%s/", strings.TrimSuffix(strings.TrimPrefix(evergreen.GetEnvironment().Settings().Providers.AWS.ParameterStore.Prefix, "/"), "/"), model.GetVarsParameterPath(projectID))
for _, pm := range vars.Parameters {
assert.True(t, strings.HasPrefix(pm.ParameterName, commonAndProjectIDPrefix), "parameter name '%s' should have standard prefix '%s'", pm.ParameterName, commonAndProjectIDPrefix)
}
Expand Down
10 changes: 10 additions & 0 deletions util/hash.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package util

import "github.com/evergreen-ci/utility"

// 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.

return hasher.Sum()
}
Loading