Use better names for the extra_vars passed to the Ansible job

Relates:
https://issues.redhat.com/browse/ACM-2946

Signed-off-by: mprahl <[email protected]>

Author: mprahl

api/v1beta1/policyautomation_types.go

@@ -44,7 +44,7 @@ const (
 	Disabled   PolicyAutomationMode = "disabled"
 )
 
-const DefaultPolicyViolationContextLimit = 1000
+const DefaultPolicyViolationsLimit = 1000
 
 // AutomationDef defines the automation to invoke
 type AutomationDef struct {
@@ -66,21 +66,20 @@ type AutomationDef struct {
 	JobTTL *int `json:"jobTtl,omitempty"`
 	// +kubebuilder:validation:Minimum=0
 	// The maximum number of violating cluster contexts that will be provided to the Ansible job as extra variables.
-	// When policyViolationContextLimit is set to 0, it means no limit.
+	// When policyViolationsLimit is set to 0, it means no limit.
 	// The default value is 1000.
-	PolicyViolationContextLimit *uint `json:"policyViolationContextLimit,omitempty"`
+	PolicyViolationsLimit *uint `json:"policyViolationsLimit,omitempty"`
 }
 
 // ViolationContext defines the non-compliant replicated policy information
 // that is sent to the AnsibleJob through extra_vars.
 type ViolationContext struct {
-	TargetClusters  []string `json:"targetClusters" ansibleJob:"target_clusters"`
-	PolicyName      string   `json:"policyName" ansibleJob:"policy_name"`
-	PolicyNamespace string   `json:"namespace"`
-	HubCluster      string   `json:"hubCluster" ansibleJob:"hub_cluster"`
-	PolicySets      []string `json:"policySet" ansibleJob:"policy_set"`
-	//nolint: lll
-	PolicyViolationContext map[string]ReplicatedPolicyStatus `json:"policyViolationContext" ansibleJob:"policy_violation_context"`
+	TargetClusters   []string                          `json:"targetClusters" ansibleJob:"target_clusters"`
+	PolicyName       string                            `json:"policyName" ansibleJob:"policy_name"`
+	PolicyNamespace  string                            `json:"policyNamespace" ansibleJob:"policy_namespace"`
+	HubCluster       string                            `json:"hubCluster" ansibleJob:"hub_cluster"`
+	PolicySets       []string                          `json:"policySets" ansibleJob:"policy_sets"`
+	PolicyViolations map[string]ReplicatedPolicyStatus `json:"policyViolations" ansibleJob:"policy_violations"`
 }
 
 // PolicyAutomationStatus defines the observed state of PolicyAutomation

controllers/automation/policyautomation_controller.go

@@ -195,16 +195,16 @@ func (r *PolicyAutomationReconciler) getViolationContext(
 		return violationContext, nil
 	}
 
-	policyViolationContextLimit := policyAutomation.Spec.Automation.PolicyViolationContextLimit
-	if policyViolationContextLimit == nil {
-		policyViolationContextLimit = new(uint)
-		*policyViolationContextLimit = policyv1beta1.DefaultPolicyViolationContextLimit
+	policyViolationsLimit := policyAutomation.Spec.Automation.PolicyViolationsLimit
+	if policyViolationsLimit == nil {
+		policyViolationsLimit = new(uint)
+		*policyViolationsLimit = policyv1beta1.DefaultPolicyViolationsLimit
 	}
 
-	contextLimit := int(*policyViolationContextLimit)
+	contextLimit := int(*policyViolationsLimit)
 
 	targetListMap := getTargetListMap(targetList)
-	violationContext.PolicyViolationContext = make(
+	violationContext.PolicyViolations = make(
 		map[string]policyv1beta1.ReplicatedPolicyStatus,
 		len(replicatedPlcList.Items),
 	)
@@ -219,7 +219,7 @@ func (r *PolicyAutomationReconciler) getViolationContext(
 		rPlcStatus := policyv1beta1.ReplicatedPolicyStatus{}
 		// Convert PolicyStatus to ReplicatedPolicyStatus and skip the unnecessary items
 		err := common.TypeConverter(rPlc.Status, &rPlcStatus)
-		if err != nil { // still assign the empty rPlcStatus to PolicyViolationContext later
+		if err != nil { // still assign the empty rPlcStatus to PolicyViolations later
 			log.Error(err, "The PolicyStatus cannot be converted to the type ReplicatedPolicyStatus.")
 		}
 
@@ -229,10 +229,10 @@ func (r *PolicyAutomationReconciler) getViolationContext(
 			rPlcStatus.ViolationMessage = statusDetails[0].History[0].Message
 		}
 
-		violationContext.PolicyViolationContext[clusterName] = rPlcStatus
-		if contextLimit > 0 && len(violationContext.PolicyViolationContext) == contextLimit {
+		violationContext.PolicyViolations[clusterName] = rPlcStatus
+		if contextLimit > 0 && len(violationContext.PolicyViolations) == contextLimit {
 			log.V(2).Info(
-				"PolicyViolationContextLimit is %s so skipping %s remaining replicated policies violations.",
+				"PolicyViolationsLimit is %s so skipping %s remaining replicated policies violations.",
 				fmt.Sprint(contextLimit),
 				fmt.Sprint(len(replicatedPlcList.Items)-contextLimit),
 			)

deploy/crds/policy.open-cluster-management.io_policyautomations.yaml

@@ -55,11 +55,11 @@ spec:
                       job
                     minLength: 1
                     type: string
-                  policyViolationContextLimit:
+                  policyViolationsLimit:
                     description: The maximum number of violating cluster contexts
                       that will be provided to the Ansible job as extra variables.
-                      When policyViolationContextLimit is set to 0, it means no limit.
-                      The default value is 1000.
+                      When policyViolationsLimit is set to 0, it means no limit. The
+                      default value is 1000.
                     minimum: 0
                     type: integer
                   secret:

test/e2e/case5_policy_automation_test.go

@@ -171,13 +171,13 @@ var _ = Describe("Test policy automation", func() {
 			spec := ansiblejobList.Items[0].Object["spec"]
 			extraVars := spec.(map[string]interface{})["extra_vars"].(map[string]interface{})
 			Expect(extraVars["policy_name"]).To(Equal("case5-test-policy"))
-			Expect(extraVars["namespace"]).To(Equal(testNamespace))
+			Expect(extraVars["policy_namespace"]).To(Equal(testNamespace))
 			Expect(extraVars["hub_cluster"]).To(Equal("millienium-falcon.tatooine.local"))
 			Expect(len(extraVars["target_clusters"].([]interface{}))).To(Equal(1))
 			Expect(extraVars["target_clusters"].([]interface{})[0]).To(Equal("managed1"))
-			Expect(len(extraVars["policy_set"].([]interface{}))).To(Equal(1))
-			Expect(extraVars["policy_set"].([]interface{})[0]).To(Equal("case5-test-policyset"))
-			managed1 := extraVars["policy_violation_context"].(map[string]interface{})["managed1"]
+			Expect(len(extraVars["policy_sets"].([]interface{}))).To(Equal(1))
+			Expect(extraVars["policy_sets"].([]interface{})[0]).To(Equal("case5-test-policyset"))
+			managed1 := extraVars["policy_violations"].(map[string]interface{})["managed1"]
 			compliant := managed1.(map[string]interface{})["compliant"]
 			Expect(compliant).To(Equal(string(policiesv1.NonCompliant)))
 			violationMessage := managed1.(map[string]interface{})["violation_message"]
@@ -807,15 +807,15 @@ var _ = Describe("Test policy automation", func() {
 			spec := ansiblejobList.Items[0].Object["spec"]
 			extraVars := spec.(map[string]interface{})["extra_vars"].(map[string]interface{})
 			Expect(extraVars["policy_name"]).To(Equal("case5-test-policy"))
-			Expect(extraVars["namespace"]).To(Equal(testNamespace))
+			Expect(extraVars["policy_namespace"]).To(Equal(testNamespace))
 			Expect(extraVars["hub_cluster"]).To(Equal("millienium-falcon.tatooine.local"))
 			Expect(len(extraVars["target_clusters"].([]interface{}))).To(Equal(2))
-			Expect(len(extraVars["policy_set"].([]interface{}))).To(Equal(1))
-			Expect(extraVars["policy_set"].([]interface{})[0]).To(Equal("case5-test-policyset"))
-			managed1 := extraVars["policy_violation_context"].(map[string]interface{})["managed1"]
+			Expect(len(extraVars["policy_sets"].([]interface{}))).To(Equal(1))
+			Expect(extraVars["policy_sets"].([]interface{})[0]).To(Equal("case5-test-policyset"))
+			managed1 := extraVars["policy_violations"].(map[string]interface{})["managed1"]
 			compliant := managed1.(map[string]interface{})["compliant"]
 			Expect(compliant).To(Equal(string(policiesv1.NonCompliant)))
-			managed2 := extraVars["policy_violation_context"].(map[string]interface{})["managed2"]
+			managed2 := extraVars["policy_violations"].(map[string]interface{})["managed2"]
 			compliant = managed2.(map[string]interface{})["compliant"]
 			Expect(compliant).To(Equal(string(policiesv1.NonCompliant)))
 
@@ -866,7 +866,7 @@ var _ = Describe("Test policy automation", func() {
 				return len(ansiblejobList.Items)
 			}, 30, 1).Should(Equal(2))
 
-			By("Check the policy_violation_context is mostly empty for the compliant manual run case")
+			By("Check policy_violations is mostly empty for the compliant manual run case")
 			ansiblejobList, err = clientHubDynamic.Resource(gvrAnsibleJob).Namespace(testNamespace).List(
 				context.TODO(), metav1.ListOptions{},
 			)
@@ -880,12 +880,12 @@ var _ = Describe("Test policy automation", func() {
 
 			extraVars = spec.(map[string]interface{})["extra_vars"].(map[string]interface{})
 			Expect(extraVars["policy_name"]).To(Equal("case5-test-policy"))
-			Expect(extraVars["namespace"]).To(Equal(testNamespace))
+			Expect(extraVars["policy_namespace"]).To(Equal(testNamespace))
 			Expect(extraVars["hub_cluster"]).To(Equal("millienium-falcon.tatooine.local"))
 			Expect(len(extraVars["target_clusters"].([]interface{}))).To(Equal(0))
-			Expect(len(extraVars["policy_set"].([]interface{}))).To(Equal(1))
-			Expect(extraVars["policy_set"].([]interface{})[0]).To(Equal("case5-test-policyset"))
-			Expect(extraVars["policy_violation_context"]).To(BeNil())
+			Expect(len(extraVars["policy_sets"].([]interface{}))).To(Equal(1))
+			Expect(extraVars["policy_sets"].([]interface{})[0]).To(Equal("case5-test-policyset"))
+			Expect(extraVars["policy_violations"]).To(BeNil())
 		})
 	})
 	Describe("Clean up", func() {