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.

Sign up for GitHub

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

Jump to bottom

Fix secret environment variables being updated with empty string #1618

Merged
merged 2 commits into from
Apr 16, 2024
Merged

Fix secret environment variables being updated with empty string #1618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6dcdf56
Fix secret environment variables being updated with empty string
vandyliu Apr 10, 2024
5960ad1
use nil vals
vandyliu Apr 11, 2024
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
6 changes: 3 additions & 3 deletions cloud/deployment/fromfile/fromfile.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,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