Skip to content

Use better names for the extra_vars passed to the Ansible job #88

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

Merged
merged 1 commit into from
Jan 20, 2023
Merged

Use better names for the extra_vars passed to the Ansible job #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 9 additions & 10 deletions api/v1beta1/policyautomation_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const (
Disabled PolicyAutomationMode = "disabled"
)

const DefaultPolicyViolationContextLimit = 1000
const DefaultPolicyViolationsLimit = 1000

// AutomationDef defines the automation to invoke
type AutomationDef struct {
Expand All @@ -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
Expand Down
20 changes: 10 additions & 10 deletions controllers/automation/policyautomation_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
Expand All @@ -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.")
}

Expand All @@ -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),
)
Expand Down
6 changes: 3 additions & 3 deletions deploy/crds/policy.open-cluster-management.io_policyautomations.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
28 changes: 14 additions & 14 deletions test/e2e/case5_policy_automation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down Expand Up @@ -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)))

Expand Down Expand Up @@ -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{},
)
Expand All @@ -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() {
Expand Down