Skip to content

Commit

Permalink
Fix secret environment variables being updated with empty string (#1618)
Browse files Browse the repository at this point in the history 
* Fix secret environment variables being updated with empty string

* use nil vals
  • Loading branch information
@vandyliu
vandyliu committed Apr 16, 2024
1 parent d5f167e commit 5cc493d
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 24 deletions.
6 changes: 3 additions & 3 deletions cloud/deployment/fromfile/fromfile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -941,8 +941,8 @@ func checkEnvVars(deploymentFromFile *inspect.FormattedDeployment, action string
missingField := fmt.Sprintf("deployment.environment_variables[%d].key", i)
return fmt.Errorf("%w: %s", errRequiredField, missingField)
}
if action == createAction {
if envVar.Value == "" {
if action == createAction || (action == updateAction && !envVar.IsSecret) {
if envVar.Value == nil {
missingField := fmt.Sprintf("deployment.environment_variables[%d].value", i)
return fmt.Errorf("%w: %s", errRequiredField, missingField)
}
Expand All @@ -957,7 +957,7 @@ func createEnvVarsRequest(deploymentFromFile *inspect.FormattedDeployment) (envV
var envVar astroplatformcore.DeploymentEnvironmentVariableRequest
envVar.IsSecret = deploymentFromFile.Deployment.EnvVars[i].IsSecret
envVar.Key = deploymentFromFile.Deployment.EnvVars[i].Key
envVar.Value = &deploymentFromFile.Deployment.EnvVars[i].Value
envVar.Value = deploymentFromFile.Deployment.EnvVars[i].Value
envVars = append(envVars, envVar)
}
return envVars
Expand Down
64 changes: 47 additions & 17 deletions cloud/deployment/fromfile/fromfile_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ var (
errTest = errors.New("test error")
poolID = "test-pool-id"
poolID2 = "test-pool-id-2"
val1 = "val-1"
val2 = "val-2"
workloadIdentity = "astro-great-release-name@provider-account.iam.gserviceaccount.com"
clusterID = "test-cluster-id"
clusterName = "test-cluster"
Expand Down Expand Up @@ -3017,13 +3019,13 @@ func TestCheckRequiredFields(t *testing.T) {
IsSecret: false,
Key: "",
UpdatedAt: "",
Value: "val-1",
Value: &val1,
},
{
IsSecret: true,
Key: "key-2",
UpdatedAt: "",
Value: "val-2",
Value: &val2,
},
}
input.Deployment.EnvVars = list
Expand Down Expand Up @@ -3252,13 +3254,13 @@ func TestHasEnvVars(t *testing.T) {
IsSecret: false,
Key: "key-1",
UpdatedAt: "",
Value: "val-1",
Value: &val1,
},
{
IsSecret: true,
Key: "key-2",
UpdatedAt: "",
Value: "val-2",
Value: &val2,
},
}
deploymentFromFile.Deployment.EnvVars = list
Expand All @@ -3278,24 +3280,28 @@ func TestCreateEnvVars(t *testing.T) {
deploymentFromFile inspect.FormattedDeployment
err error
)

t.Run("creates env vars if they were requested in a formatted deployment", func(t *testing.T) {
deploymentFromFile = inspect.FormattedDeployment{}
list := []inspect.EnvironmentVariable{
{
IsSecret: false,
Key: "key-1",
UpdatedAt: "",
Value: "val-1",
Value: &val1,
},
{
IsSecret: true,
Key: "key-2",
UpdatedAt: "",
Value: "val-2",
Value: &val2,
},
{
IsSecret: true,
Key: "key-3",
UpdatedAt: "",
},
}
val1 := "val-1"
val2 := "val-2"
expectedList := []astroplatformcore.DeploymentEnvironmentVariableRequest{
{
IsSecret: false,
Expand All @@ -3307,6 +3313,11 @@ func TestCreateEnvVars(t *testing.T) {
Key: "key-2",
Value: &val2,
},
{
IsSecret: true,
Key: "key-3",
Value: nil,
},
}
deploymentFromFile.Deployment.EnvVars = list
actualEnvVars = createEnvVarsRequest(&deploymentFromFile)
Expand Down Expand Up @@ -3716,13 +3727,13 @@ func TestCheckEnvVars(t *testing.T) {
IsSecret: false,
Key: "",
UpdatedAt: "",
Value: "val-1",
Value: &val1,
},
{
IsSecret: true,
Key: "key-2",
UpdatedAt: "",
Value: "val-2",
Value: &val2,
},
}
input.Deployment.EnvVars = list
Expand All @@ -3738,13 +3749,12 @@ func TestCheckEnvVars(t *testing.T) {
IsSecret: false,
Key: "key-1",
UpdatedAt: "",
Value: "val-1",
Value: &val1,
},
{
IsSecret: true,
Key: "key-2",
UpdatedAt: "",
Value: "",
},
}
input.Deployment.EnvVars = list
Expand All @@ -3760,35 +3770,55 @@ func TestCheckEnvVars(t *testing.T) {
IsSecret: false,
Key: "key-1",
UpdatedAt: "",
Value: "val-1",
Value: &val1,
},
{
IsSecret: true,
Key: "",
UpdatedAt: "",
Value: "val-2",
Value: &val2,
},
}
input.Deployment.EnvVars = list
err = checkEnvVars(&input, "update")
assert.ErrorIs(t, err, errRequiredField)
assert.ErrorContains(t, err, "missing required field: deployment.environment_variables[1].key")
})
t.Run("returns nil if env var values are missing on update", func(t *testing.T) {
t.Run("returns an error if env var values are missing on update for non-secret env var", func(t *testing.T) {
input.Deployment.Configuration.Name = "test-deployment"
input.Deployment.Configuration.ClusterName = "test-cluster-id"
list := []inspect.EnvironmentVariable{
{
IsSecret: false,
Key: "key-1",
UpdatedAt: "",
Value: "val-1",
},
}
input.Deployment.EnvVars = list
err = checkEnvVars(&input, "update")
assert.ErrorIs(t, err, errRequiredField)
assert.ErrorContains(t, err, "missing required field: deployment.environment_variables[0].value")
})
t.Run("returns nil if env var values are valid on update", func(t *testing.T) {
input.Deployment.Configuration.Name = "test-deployment"
input.Deployment.Configuration.ClusterName = "test-cluster-id"
list := []inspect.EnvironmentVariable{
{
IsSecret: false,
Key: "key-1",
UpdatedAt: "",
Value: &val1,
},
{
IsSecret: true,
Key: "key-2",
UpdatedAt: "",
Value: "",
Value: &val2,
},
{
IsSecret: true,
Key: "key-3",
UpdatedAt: "",
},
}
input.Deployment.EnvVars = list
Expand Down
8 changes: 4 additions & 4 deletions cloud/deployment/inspect/inspect.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ type Workerq struct {
}

type EnvironmentVariable struct {
IsSecret bool `mapstructure:"is_secret" yaml:"is_secret" json:"is_secret"`
Key string `mapstructure:"key" yaml:"key" json:"key"`
UpdatedAt string `mapstructure:"updated_at,omitempty" yaml:"updated_at,omitempty" json:"updated_at,omitempty"`
Value string `mapstructure:"value" yaml:"value" json:"value"`
IsSecret bool `mapstructure:"is_secret" yaml:"is_secret" json:"is_secret"`
Key string `mapstructure:"key" yaml:"key" json:"key"`
UpdatedAt string `mapstructure:"updated_at,omitempty" yaml:"updated_at,omitempty" json:"updated_at,omitempty"`
Value *string `mapstructure:"value" yaml:"value" json:"value"`
}

type HibernationSchedule struct {
Expand Down

0 comments on commit 5cc493d

Please sign in to comment.