fix(api): correctly manage Remediation Strategy precedence between re…

…po & layer
padok-team · Apr 23, 2024 · c878868 · c878868
1 parent d63a1fc
commit c878868
Show file tree

Hide file tree

Showing 5 changed files with 131 additions and 13 deletions.
diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go
@@ -35,7 +35,7 @@ type RunHistoryPolicy struct {
 }
 
 type RemediationStrategy struct {
-	AutoApply bool                       `json:"autoApply,omitempty"`
+	AutoApply *bool                      `json:"autoApply,omitempty"`
 	OnError   OnErrorRemediationStrategy `json:"onError,omitempty"`
 }
 
@@ -106,17 +106,15 @@ func GetRunHistoryPolicy(repository *TerraformRepository, layer *TerraformLayer)
 	}
 }
 
-func GetRemediationStrategy(repo *TerraformRepository, layer *TerraformLayer) RemediationStrategy {
-	result := RemediationStrategy{
-		AutoApply: false,
-	}
-	if repo.Spec.RemediationStrategy.AutoApply {
-		result.AutoApply = true
+func GetAutoApplyEnabled(repo *TerraformRepository, layer *TerraformLayer) bool {
+	enabled := false
+	if repo.Spec.RemediationStrategy.AutoApply != nil {
+		enabled = *repo.Spec.RemediationStrategy.AutoApply
 	}
-	if layer.Spec.RemediationStrategy.AutoApply {
-		result.AutoApply = true
+	if layer.Spec.RemediationStrategy.AutoApply != nil {
+		enabled = *layer.Spec.RemediationStrategy.AutoApply
 	}
-	return result
+	return enabled
 }
 
 func mergeImagePullSecrets(a, b []corev1.LocalObjectReference) []corev1.LocalObjectReference {

diff --git a/api/v1alpha1/common_test.go b/api/v1alpha1/common_test.go
@@ -230,6 +230,121 @@ func TestGetTerragruntEnabled(t *testing.T) {
 	}
 }
 
+func TestGetAutoApplyEnabled(t *testing.T) {
+	tt := []struct {
+		name       string
+		repository *configv1alpha1.TerraformRepository
+		layer      *configv1alpha1.TerraformLayer
+		expected   bool
+	}{
+		{
+			"EnabledInRepositoryDisabledInLayer",
+			&configv1alpha1.TerraformRepository{
+				Spec: configv1alpha1.TerraformRepositorySpec{
+					RemediationStrategy: configv1alpha1.RemediationStrategy{
+						AutoApply: &[]bool{true}[0],
+					},
+				},
+			},
+			&configv1alpha1.TerraformLayer{
+				Spec: configv1alpha1.TerraformLayerSpec{
+					RemediationStrategy: configv1alpha1.RemediationStrategy{
+						AutoApply: &[]bool{false}[0],
+					},
+				},
+			},
+			false,
+		},
+		{
+			"DisabledInRepositoryEnabledInLayer",
+			&configv1alpha1.TerraformRepository{
+				Spec: configv1alpha1.TerraformRepositorySpec{
+					RemediationStrategy: configv1alpha1.RemediationStrategy{
+						AutoApply: &[]bool{false}[0],
+					},
+				},
+			},
+			&configv1alpha1.TerraformLayer{
+				Spec: configv1alpha1.TerraformLayerSpec{
+					RemediationStrategy: configv1alpha1.RemediationStrategy{
+						AutoApply: &[]bool{true}[0],
+					},
+				},
+			},
+			true,
+		},
+		{
+			"EnabledInRepositoryEnabledInLayer",
+			&configv1alpha1.TerraformRepository{
+				Spec: configv1alpha1.TerraformRepositorySpec{
+					RemediationStrategy: configv1alpha1.RemediationStrategy{
+						AutoApply: &[]bool{true}[0],
+					},
+				},
+			},
+			&configv1alpha1.TerraformLayer{
+				Spec: configv1alpha1.TerraformLayerSpec{
+					RemediationStrategy: configv1alpha1.RemediationStrategy{
+						AutoApply: &[]bool{true}[0],
+					},
+				},
+			},
+			true,
+		},
+		{
+			"DisabledInRepositoryDisabledInLayer",
+			&configv1alpha1.TerraformRepository{
+				Spec: configv1alpha1.TerraformRepositorySpec{
+					RemediationStrategy: configv1alpha1.RemediationStrategy{
+						AutoApply: &[]bool{false}[0],
+					},
+				},
+			},
+			&configv1alpha1.TerraformLayer{
+				Spec: configv1alpha1.TerraformLayerSpec{
+					RemediationStrategy: configv1alpha1.RemediationStrategy{
+						AutoApply: &[]bool{false}[0],
+					},
+				},
+			},
+			false,
+		},
+		{
+			"OnlyRepositoryEnabling",
+			&configv1alpha1.TerraformRepository{
+				Spec: configv1alpha1.TerraformRepositorySpec{
+					RemediationStrategy: configv1alpha1.RemediationStrategy{
+						AutoApply: &[]bool{true}[0],
+					},
+				},
+			},
+			&configv1alpha1.TerraformLayer{},
+			true,
+		},
+		{
+			"OnlyLayerEnabling",
+			&configv1alpha1.TerraformRepository{},
+			&configv1alpha1.TerraformLayer{
+				Spec: configv1alpha1.TerraformLayerSpec{
+					RemediationStrategy: configv1alpha1.RemediationStrategy{
+						AutoApply: &[]bool{true}[0],
+					},
+				},
+			},
+			true,
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			result := configv1alpha1.GetAutoApplyEnabled(tc.repository, tc.layer)
+			if tc.expected != result {
+				t.Errorf("different enabled status computed: expected %t go %t", tc.expected, result)
+			}
+		})
+	}
+}
+
 func TestOverrideRunnerSpec(t *testing.T) {
 	tt := []struct {
 		name         string

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/internal/controllers/terraformlayer/states.go b/internal/controllers/terraformlayer/states.go
@@ -70,8 +70,8 @@ type ApplyNeeded struct{}
 func (s *ApplyNeeded) getHandler() Handler {
 	return func(ctx context.Context, r *Reconciler, layer *configv1alpha1.TerraformLayer, repository *configv1alpha1.TerraformRepository) (ctrl.Result, *configv1alpha1.TerraformRun) {
 		log := log.WithContext(ctx)
-		remediationStrategy := configv1alpha1.GetRemediationStrategy(repository, layer)
-		if !remediationStrategy.AutoApply {
+		autoApply := configv1alpha1.GetAutoApplyEnabled(repository, layer)
+		if !autoApply {
 			log.Infof("layer %s is in dry mode, no action taken", layer.Name)
 			return ctrl.Result{RequeueAfter: r.Config.Controller.Timers.DriftDetection}, nil
 		}

diff --git a/internal/controllers/terraformpullrequest/layer.go b/internal/controllers/terraformpullrequest/layer.go
@@ -92,7 +92,7 @@ func generateTempLayers(pr *configv1alpha1.TerraformPullRequest, layers []config
 				TerraformConfig: layer.Spec.TerraformConfig,
 				Repository:      layer.Spec.Repository,
 				RemediationStrategy: configv1alpha1.RemediationStrategy{
-					AutoApply: false,
+					AutoApply: &[]bool{false}[0],
 				},
 				OverrideRunnerSpec: layer.Spec.OverrideRunnerSpec,
 			},