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.

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

🤖 Sync from open-cluster-management-io/governance-policy-framework-addon: #86 #166

Merged
merged 2 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion controllers/statussync/policy_status_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
// filter events to current policy instance and build map
eventForPolicyMap := make(map[string]*[]historyEvent)
// panic if regexp invalid
rgx := regexp.MustCompile(`(?i)^policy:\s*(?:([a-z0-9.-]+)\s*\/)?([a-z0-9.-]+)`)
rgx := regexp.MustCompile(`(?i)^policy:\s*(?:([a-z0-9.-]+)\s*\/)?(.+)`)
for _, event := range eventList.Items {
// sample event.Reason -- reason: 'policy: calamari/policy-grc-rbactest-example'
reason := rgx.FindString(event.Reason)
Expand Down
13 changes: 8 additions & 5 deletions controllers/templatesync/template_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,17 +450,16 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
continue
}

// reject if not configuration policy and has template strings
// reject if not configuration policy and has template strings, and don't requeue
if gvk.Kind != "ConfigurationPolicy" {
// if not configuration policies, do a simple check for templates {{hub and reject
// only checking for hub and not {{ as they could be valid cases where they are valid chars.
if strings.Contains(string(policyT.ObjectDefinition.Raw), "{{hub ") {
errMsg := fmt.Sprintf("Templates are not supported for kind : %s", gvk.Kind)
resultError = k8serrors.NewBadRequest(errMsg)

_ = r.emitTemplateError(ctx, instance, tIndex, tName, isClusterScoped, errMsg)

tLogger.Error(resultError, "Failed to process the policy template")
tLogger.Error(k8serrors.NewBadRequest(errMsg), "Failed to process the policy template")

policyUserErrorsCounter.WithLabelValues(instance.Name, tName, "format-error").Inc()

Expand Down Expand Up @@ -559,7 +558,6 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
)
}

resultError = err
errMsg := fmt.Sprintf("Failed to create policy template: %s", err)

_ = r.emitTemplateError(ctx, instance, tIndex, tName, isClusterScoped, errMsg)
Expand All @@ -571,6 +569,9 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
policyUserErrorsCounter.WithLabelValues(instance.Name, tName, "format-error").Inc()
} else {
policySystemErrorsCounter.WithLabelValues(instance.Name, tName, "create-error").Inc()

// Only requeue if the policy template is valid
resultError = err
}

continue
Expand Down Expand Up @@ -757,7 +758,6 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
return reconcile.Result{}, err
}

resultError = err
errMsg := fmt.Sprintf("Failed to update policy template %s: %s", tName, err)

_ = r.emitTemplateError(ctx, instance, tIndex, tName, isClusterScoped, errMsg)
Expand All @@ -769,6 +769,9 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
policyUserErrorsCounter.WithLabelValues(instance.Name, tName, "format-error").Inc()
} else {
policySystemErrorsCounter.WithLabelValues(instance.Name, tName, "patch-error").Inc()

// Only requeue if the policy template is valid
resultError = err
}

continue
Expand Down
31 changes: 31 additions & 0 deletions test/e2e/case10_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import (
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
policiesv1 "open-cluster-management.io/governance-policy-propagator/api/v1"
"open-cluster-management.io/governance-policy-propagator/test/utils"
)

Expand Down Expand Up @@ -110,6 +112,35 @@ var _ = Describe("Test error handling", func() {
1,
).Should(BeTrue())
})
It("should generate creation err event", func() {
policyName := "case10-invalid-name-error"
statusMsg := "template-error; Failed to create policy template:"

hubApplyPolicy(policyName,
yamlBasePath+"invalid-name-error.yaml")

By("Checking for event with creation err on managed cluster in ns:" + clusterNamespace)
Eventually(
checkForEvent(policyName, statusMsg),
defaultTimeoutSeconds,
1,
).Should(BeTrue())
By("Checking if policy status is noncompliant")
hubPlc := utils.GetWithTimeout(
clientHubDynamic,
gvrPolicy,
policyName,
clusterNamespaceOnHub,
true,
defaultTimeoutSeconds)
var plc *policiesv1.Policy
err := runtime.DefaultUnstructuredConverter.FromUnstructured(hubPlc.Object, &plc)
Expect(err).ToNot(HaveOccurred())
Expect(plc.Status.Details).To(HaveLen(1))
Expect(plc.Status.Details[0].History).To(HaveLen(1))
Expect(plc.Status.Details[0].TemplateMeta.GetName()).To(Equal("case10_invalid-name"))
Expect(plc.Status.Details[0].History[0].Message).To(ContainSubstring(statusMsg))
})
It("should generate unsupported object err event", func() {
hubApplyPolicy("case10-unsupported-object",
yamlBasePath+"unsupported-object-error.yaml")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
apiVersion: policy.open-cluster-management.io/v1
kind: Policy
metadata:
name: case10-invalid-name-error
labels:
policy.open-cluster-management.io/cluster-name: managed
policy.open-cluster-management.io/cluster-namespace: managed
policy.open-cluster-management.io/root-policy: case10-template-name-error
spec:
remediationAction: inform
disabled: false
policy-templates:
- objectDefinition:
apiVersion: policy.open-cluster-management.io/v1
kind: ConfigurationPolicy
metadata:
name: case10_invalid-name
spec:
object-templates:
- complianceType: musthave
objectDefinition:
apiVersion: v1
kind: Pod
metadata:
name: nginx-pod-e2e
namespace: default
spec:
containers:
- name: nginx

Loading