Skip to content

Commit 88a6db6

Browse files
committed
Fix: prevent perpetual drift in sub-environment configuration blocks
Sub-environment read overwrote state with API response in API order and clobbered values for sensitive variables, causing plans to never converge. Now merges in schema order, keeps the schema value for sensitive variables (API returns redacted), and still surfaces remote-only variables as drift.
1 parent d3e2204 commit 88a6db6

2 files changed

Lines changed: 158 additions & 11 deletions

File tree

env0/resource_environment.go

Lines changed: 58 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -728,9 +728,8 @@ func resourceEnvironmentRead(ctx context.Context, d *schema.ResourceData, meta a
728728
return nil
729729
}
730730

731-
func setSubEnvironmentSchema(d *schema.ResourceData, apiClient client.ApiClientInterface) any {
731+
func setSubEnvironmentSchema(d *schema.ResourceData, apiClient client.ApiClientInterface) error {
732732
iSubEnvironments, ok := d.GetOk("sub_environment_configuration")
733-
734733
if !ok {
735734
return nil
736735
}
@@ -741,19 +740,19 @@ func setSubEnvironmentSchema(d *schema.ResourceData, apiClient client.ApiClientI
741740
for _, iSubEnvironment := range subEnvironmentsList {
742741
subEnvironment := iSubEnvironment.(map[string]any)
743742

744-
environmentConfigurationVariables, err := apiClient.ConfigurationVariablesByScope(client.ScopeEnvironment, subEnvironment["id"].(string))
745-
if err != nil {
746-
return fmt.Errorf("could not fetch environment configuration variables for sub environment %s: %w", subEnvironment["id"].(string), err)
743+
subEnvId, _ := subEnvironment["id"].(string)
744+
if subEnvId == "" {
745+
newSubEnvironments = append(newSubEnvironments, subEnvironment)
746+
continue
747747
}
748748

749-
newConfiguration := make([]any, 0, len(environmentConfigurationVariables))
750-
751-
for _, variable := range environmentConfigurationVariables {
752-
newConfiguration = append(newConfiguration, createVariable(&variable))
749+
remoteVariables, err := apiClient.ConfigurationVariablesByScope(client.ScopeEnvironment, subEnvId)
750+
if err != nil {
751+
return fmt.Errorf("could not fetch environment configuration variables for sub environment %s: %w", subEnvId, err)
753752
}
754753

755-
subEnvironment["configuration"] = newConfiguration
756-
754+
stateVariables, _ := subEnvironment["configuration"].([]any)
755+
subEnvironment["configuration"] = mergeSubEnvironmentConfiguration(stateVariables, remoteVariables)
757756
newSubEnvironments = append(newSubEnvironments, subEnvironment)
758757
}
759758

@@ -762,6 +761,54 @@ func setSubEnvironmentSchema(d *schema.ResourceData, apiClient client.ApiClientI
762761
return nil
763762
}
764763

764+
// mergeSubEnvironmentConfiguration aligns state variable order to the schema (TF code) and
765+
// appends remote-only variables as drift. Sensitive variable values come back redacted from
766+
// the API, so the schema value is kept to avoid false drift; the API value is never compared.
767+
func mergeSubEnvironmentConfiguration(stateVariables []any, remoteVariables client.ConfigurationChanges) []any {
768+
merged := make([]any, 0, len(stateVariables))
769+
770+
for _, ivariable := range stateVariables {
771+
variable := ivariable.(map[string]any)
772+
variableName := variable["name"].(string)
773+
774+
for i := range remoteVariables {
775+
remote := &remoteVariables[i]
776+
if remote.Name != variableName {
777+
continue
778+
}
779+
780+
newVariable := createVariable(remote).(map[string]any)
781+
782+
if remote.IsSensitive != nil && *remote.IsSensitive {
783+
newVariable["value"] = variable["value"]
784+
}
785+
786+
merged = append(merged, newVariable)
787+
788+
break
789+
}
790+
}
791+
792+
for i := range remoteVariables {
793+
remote := &remoteVariables[i]
794+
795+
found := false
796+
797+
for _, ivariable := range stateVariables {
798+
if ivariable.(map[string]any)["name"].(string) == remote.Name {
799+
found = true
800+
break
801+
}
802+
}
803+
804+
if !found {
805+
merged = append(merged, createVariable(remote))
806+
}
807+
}
808+
809+
return merged
810+
}
811+
765812
func resourceEnvironmentUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
766813
apiClient := meta.(client.ApiClientInterface)
767814

env0/resource_environment_test.go

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3638,6 +3638,106 @@ func TestUnitEnvironmentWithSubEnvironment(t *testing.T) {
36383638
})
36393639
}
36403640

3641+
func TestMergeSubEnvironmentConfiguration(t *testing.T) {
3642+
varType := client.ConfigurationVariableType(0)
3643+
trueVal := true
3644+
3645+
stateVar := func(name, value string) map[string]any {
3646+
return map[string]any{"name": name, "value": value}
3647+
}
3648+
3649+
remoteVar := func(name, value string, sensitive *bool) client.ConfigurationVariable {
3650+
return client.ConfigurationVariable{
3651+
Name: name,
3652+
Value: value,
3653+
Type: &varType,
3654+
IsSensitive: sensitive,
3655+
Schema: &client.ConfigurationVariableSchema{Type: "string"},
3656+
}
3657+
}
3658+
3659+
t.Run("preserves schema order when API returns reversed", func(t *testing.T) {
3660+
state := []any{stateVar("a", "av"), stateVar("b", "bv"), stateVar("c", "cv")}
3661+
remote := client.ConfigurationChanges{
3662+
remoteVar("c", "cv", nil),
3663+
remoteVar("b", "bv", nil),
3664+
remoteVar("a", "av", nil),
3665+
}
3666+
3667+
merged := mergeSubEnvironmentConfiguration(state, remote)
3668+
3669+
if len(merged) != 3 {
3670+
t.Fatalf("expected 3 vars, got %d", len(merged))
3671+
}
3672+
3673+
names := []string{
3674+
merged[0].(map[string]any)["name"].(string),
3675+
merged[1].(map[string]any)["name"].(string),
3676+
merged[2].(map[string]any)["name"].(string),
3677+
}
3678+
expected := []string{"a", "b", "c"}
3679+
3680+
for i, name := range names {
3681+
if name != expected[i] {
3682+
t.Fatalf("order mismatch at %d: got %q want %q", i, name, expected[i])
3683+
}
3684+
}
3685+
})
3686+
3687+
t.Run("preserves schema value for sensitive vars without comparing API value", func(t *testing.T) {
3688+
state := []any{stateVar("secret", "real-secret")}
3689+
remote := client.ConfigurationChanges{remoteVar("secret", "", &trueVal)}
3690+
3691+
merged := mergeSubEnvironmentConfiguration(state, remote)
3692+
3693+
if len(merged) != 1 {
3694+
t.Fatalf("expected 1 var, got %d", len(merged))
3695+
}
3696+
3697+
got := merged[0].(map[string]any)["value"]
3698+
if got != "real-secret" {
3699+
t.Fatalf("sensitive value not preserved: got %q", got)
3700+
}
3701+
})
3702+
3703+
t.Run("appends remote-only vars as drift", func(t *testing.T) {
3704+
state := []any{stateVar("a", "av")}
3705+
remote := client.ConfigurationChanges{
3706+
remoteVar("a", "av", nil),
3707+
remoteVar("b", "remote-only", nil),
3708+
}
3709+
3710+
merged := mergeSubEnvironmentConfiguration(state, remote)
3711+
3712+
if len(merged) != 2 {
3713+
t.Fatalf("expected 2 vars (incl drift), got %d", len(merged))
3714+
}
3715+
3716+
if merged[0].(map[string]any)["name"] != "a" {
3717+
t.Fatalf("expected schema var first, got %v", merged[0])
3718+
}
3719+
3720+
if merged[1].(map[string]any)["name"] != "b" {
3721+
t.Fatalf("expected drift var second, got %v", merged[1])
3722+
}
3723+
})
3724+
3725+
t.Run("schema var missing from API is dropped", func(t *testing.T) {
3726+
state := []any{stateVar("a", "av"), stateVar("gone", "v")}
3727+
remote := client.ConfigurationChanges{remoteVar("a", "av", nil)}
3728+
3729+
merged := mergeSubEnvironmentConfiguration(state, remote)
3730+
3731+
if len(merged) != 1 {
3732+
t.Fatalf("expected 1 var, got %d", len(merged))
3733+
}
3734+
3735+
if merged[0].(map[string]any)["name"] != "a" {
3736+
t.Fatalf("expected only 'a' var, got %v", merged[0])
3737+
}
3738+
})
3739+
}
3740+
36413741
func TestUnitEnvironmentIsRequiredDeprecated(t *testing.T) {
36423742
resourceType := "env0_environment"
36433743
resourceName := "test"

0 commit comments

Comments
 (0)