Skip to content

Commit

Permalink
Add CRD reporting to OperatorPolicy
Browse files Browse the repository at this point in the history
Also adjusts CSV reporting to handle when the CSV exists but the
subscription does not, by using a label that OLM adds to these objects.

The authorino operator was chosen because it has more than one CRD, but
not too many to make a concise test.

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
  • Loading branch information
JustinKuli authored and openshift-merge-bot[bot] committed Apr 10, 2024
1 parent 0c17093 commit 6b1db91
Show file tree
Hide file tree
Showing 4 changed files with 250 additions and 21 deletions.
101 changes: 82 additions & 19 deletions controllers/operatorpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ var (
Version: "v1alpha1",
Kind: "ClusterServiceVersion",
}
customResourceDefinitionGVK = schema.GroupVersionKind{
Group: "apiextensions.k8s.io",
Version: "v1",
Kind: "CustomResourceDefinition",
}
deploymentGVK = schema.GroupVersionKind{
Group: "apps",
Version: "v1",
Expand Down Expand Up @@ -251,6 +256,16 @@ func (r *OperatorPolicyReconciler) handleResources(ctx context.Context, policy *
return earlyComplianceEvents, condChanged, err
}

earlyConds, changed, err = r.handleCRDs(ctx, policy, subscription)
earlyComplianceEvents = append(earlyComplianceEvents, earlyConds...)
condChanged = condChanged || changed

if err != nil {
OpLog.Error(err, "Error handling CustomResourceDefinitions")

return earlyComplianceEvents, condChanged, err
}

changed, err = r.handleDeployment(ctx, policy, csv)
condChanged = condChanged || changed

Expand Down Expand Up @@ -940,37 +955,38 @@ func (r *OperatorPolicyReconciler) handleCSV(
}

watcher := opPolIdentifier(policy.Namespace, policy.Name)
selector := subLabelSelector(sub)

// case where subscription status has not been populated yet
if sub.Status.InstalledCSV == "" {
return nil, updateStatus(policy, noCSVCond, noExistingCSVObj), nil
csvList, err := r.DynamicWatcher.List(watcher, clusterServiceVersionGVK, sub.Namespace, selector)
if err != nil {
return nil, false, fmt.Errorf("error listing CSVs: %w", err)
}

// Get the CSV related to the object
foundCSV, err := r.DynamicWatcher.Get(watcher, clusterServiceVersionGVK, sub.Namespace,
sub.Status.InstalledCSV)
if err != nil {
return nil, false, err
var foundCSV *operatorv1alpha1.ClusterServiceVersion

for _, csv := range csvList {
if csv.GetName() == sub.Status.InstalledCSV {
matchedCSV := operatorv1alpha1.ClusterServiceVersion{}

err = runtime.DefaultUnstructuredConverter.FromUnstructured(csv.UnstructuredContent(), &matchedCSV)
if err != nil {
return nil, false, err
}

foundCSV = &matchedCSV
}
}


// CSV has not yet been created by OLM
if foundCSV == nil {
changed := updateStatus(policy,
missingWantedCond("ClusterServiceVersion"), missingCSVObj(sub.Name, sub.Namespace))

return nil, changed, nil
}

// Check CSV most recent condition
unstructured := foundCSV.UnstructuredContent()
var csv operatorv1alpha1.ClusterServiceVersion

err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructured, &csv)
if err != nil {
return nil, false, err
return foundCSV, changed, nil
}

return &csv, updateStatus(policy, buildCSVCond(&csv), existingCSVObj(&csv)), nil
return foundCSV, updateStatus(policy, buildCSVCond(foundCSV), existingCSVObj(foundCSV)), nil
}

func (r *OperatorPolicyReconciler) handleDeployment(
Expand Down Expand Up @@ -1029,6 +1045,36 @@ func (r *OperatorPolicyReconciler) handleDeployment(
return updateStatus(policy, buildDeploymentCond(depNum > 0, unavailableDeployments), relatedObjects...), nil
}

func (r *OperatorPolicyReconciler) handleCRDs(
_ context.Context,
policy *policyv1beta1.OperatorPolicy,
sub *operatorv1alpha1.Subscription,
) ([]metav1.Condition, bool, error) {
if sub == nil {
return nil, updateStatus(policy, noCRDCond, noExistingCRDObj), nil
}

watcher := opPolIdentifier(policy.Namespace, policy.Name)
selector := subLabelSelector(sub)

crdList, err := r.DynamicWatcher.List(watcher, customResourceDefinitionGVK, sub.Namespace, selector)
if err != nil {
return nil, false, fmt.Errorf("error listing CRDs: %w", err)
}

if len(crdList) == 0 {
return nil, updateStatus(policy, noCRDCond, noExistingCRDObj), nil
}

relatedCRDs := make([]policyv1.RelatedObject, len(crdList))

for i := range crdList {
relatedCRDs[i] = matchedObj(&crdList[i])
}

return nil, updateStatus(policy, crdFoundCond, relatedCRDs...), nil
}

func (r *OperatorPolicyReconciler) handleCatalogSource(
policy *policyv1beta1.OperatorPolicy,
subscription *operatorv1alpha1.Subscription,
Expand Down Expand Up @@ -1136,3 +1182,20 @@ func (r *OperatorPolicyReconciler) mergeObjects(

return updateNeeded, false, nil
}

// subLabelSelector returns a selector that matches a label that OLM adds to resources
// that are related to a Subscription. It can be used to find those resources even
// after the Subscription or CSV is deleted.
func subLabelSelector(sub *operatorv1alpha1.Subscription) labels.Selector {
sel, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{{
Key: fmt.Sprintf("operators.coreos.com/%v.%v", sub.Name, sub.Namespace),
Operator: metav1.LabelSelectorOpExists,
}},
})
if err != nil {
panic(err)
}

return sel
}
46 changes: 46 additions & 0 deletions controllers/operatorpolicy_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,18 @@ func calculateComplianceCondition(policy *policyv1beta1.OperatorPolicy) metav1.C
}
}

idx, cond = policy.Status.GetCondition(crdConditionType)
if idx == -1 {
messages = append(messages, "the status of the CustomResourceDefinitions is unknown")
foundNonCompliant = true
} else {
messages = append(messages, cond.Message)

if cond.Status != metav1.ConditionTrue {
foundNonCompliant = true
}
}

idx, cond = policy.Status.GetCondition(deploymentConditionType)
if idx == -1 {
messages = append(messages, "the status of the Deployments are unknown")
Expand Down Expand Up @@ -354,6 +366,7 @@ const (
opGroupConditionType = "OperatorGroupCompliant"
subConditionType = "SubscriptionCompliant"
csvConditionType = "ClusterServiceVersionCompliant"
crdConditionType = "CustomResourceDefinitionCompliant"
deploymentConditionType = "DeploymentCompliant"
catalogSrcConditionType = "CatalogSourcesUnhealthy"
installPlanConditionType = "InstallPlanCompliant"
Expand All @@ -369,6 +382,8 @@ func condType(kind string) string {
return installPlanConditionType
case "ClusterServiceVersion":
return csvConditionType
case "CustomResourceDefinition":
return crdConditionType
case "Deployment":
return deploymentConditionType
case "CatalogSource":
Expand Down Expand Up @@ -618,6 +633,22 @@ var noCSVCond = metav1.Condition{
Message: "A relevant installed ClusterServiceVersion could not be found",
}

// noCRDCond is a Compliant condition for when no CRDs are found
var noCRDCond = metav1.Condition{
Type: crdConditionType,
Status: metav1.ConditionTrue,
Reason: "RelevantCRDNotFound",
Message: "No CRDs were found for the operator",
}

// crdFoundCond is a Compliant condition for when CRDs are found
var crdFoundCond = metav1.Condition{
Type: crdConditionType,
Status: metav1.ConditionTrue,
Reason: "RelevantCRDFound",
Message: "There are CRDs present for the operator",
}

// buildDeploymentCond creates a Condition for deployments. If any are not at their
// minimum availability, the condition will be NonCompliant, and the message will
// list the unavailable deployments.
Expand Down Expand Up @@ -889,6 +920,21 @@ var noExistingCSVObj = policyv1.RelatedObject{
Reason: "No relevant ClusterServiceVersion found",
}

// noExistingCRDObj is a Compliant RelatedObject for CustomResourceDefinitions,
// with Reason 'No relevant CustomResourceDefinitions found'. It is considered
// compliant because not all operators will have CRDs.
var noExistingCRDObj = policyv1.RelatedObject{
Object: policyv1.ObjectResource{
Kind: customResourceDefinitionGVK.Kind,
APIVersion: customResourceDefinitionGVK.GroupVersion().String(),
Metadata: policyv1.ObjectMetadata{
Name: "-",
},
},
Compliant: string(policyv1.Compliant),
Reason: "No relevant CustomResourceDefinitions found",
}

// missingDeploymentObj returns a NonCompliant RelatedObject for a Deployment,
// with Reason 'Resource not found but should exist'
func missingDeploymentObj(name string, namespace string) policyv1.RelatedObject {
Expand Down
101 changes: 99 additions & 2 deletions test/e2e/case38_install_operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun
) {
var debugMessage string

defer func() {
DeferCleanup(func() {
if CurrentSpecReport().Failed() {
GinkgoWriter.Println(debugMessage)
}
}()
})

checkFunc := func(g Gomega) {
GinkgoHelper()
Expand Down Expand Up @@ -1200,6 +1200,103 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun
)
})
})
Describe("Testing CustomResourceDefinition reporting", Ordered, func() {
const (
opPolYAML = "../resources/case38_operator_install/operator-policy-authorino.yaml"
opPolName = "oppol-authorino"
)
BeforeAll(func() {
utils.Kubectl("delete", "crd", "--selector=olm.managed=true")
utils.Kubectl("create", "ns", opPolTestNS)
DeferCleanup(func() {
utils.Kubectl("delete", "ns", opPolTestNS)
})

createObjWithParent(parentPolicyYAML, parentPolicyName,
opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy)
})

It("Should initially not report on CRDs because they won't exist yet", func(ctx SpecContext) {
check(
opPolName,
false,
[]policyv1.RelatedObject{{
Object: policyv1.ObjectResource{
Kind: "CustomResourceDefinition",
APIVersion: "apiextensions.k8s.io/v1",
Metadata: policyv1.ObjectMetadata{
Name: "-",
},
},
Compliant: "Compliant",
Reason: "No relevant CustomResourceDefinitions found",
}},
metav1.Condition{
Type: "CustomResourceDefinitionCompliant",
Status: metav1.ConditionTrue,
Reason: "RelevantCRDNotFound",
Message: "No CRDs were found for the operator",
},
"No CRDs were found for the operator",
)
})

It("Should generate conditions and relatedobjects of CRD", func(ctx SpecContext) {
utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p",
`[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`)
By("Waiting for a CRD to appear, which should indicate the operator is installing")
Eventually(func(ctx SpecContext) *unstructured.Unstructured {
crd, _ := clientManagedDynamic.Resource(gvrCRD).Get(ctx,
"authconfigs.authorino.kuadrant.io", metav1.GetOptions{})

return crd
}, olmWaitTimeout, 5, ctx).ShouldNot(BeNil())

By("Waiting for the policy to become compliant, indicating the operator is installed")
Eventually(func(g Gomega) string {
pol := utils.GetWithTimeout(clientManagedDynamic, gvrOperatorPolicy, opPolName,
opPolTestNS, true, eventuallyTimeout)
compliance, found, err := unstructured.NestedString(pol.Object, "status", "compliant")
g.Expect(err).NotTo(HaveOccurred())
g.Expect(found).To(BeTrue())

return compliance
}, olmWaitTimeout, 5, ctx).Should(Equal("Compliant"))

check(
opPolName,
false,
[]policyv1.RelatedObject{{
Object: policyv1.ObjectResource{
Kind: "CustomResourceDefinition",
APIVersion: "apiextensions.k8s.io/v1",
Metadata: policyv1.ObjectMetadata{
Name: "authconfigs.authorino.kuadrant.io",
},
},
Compliant: "Compliant",
Reason: "Resource found as expected",
}, {
Object: policyv1.ObjectResource{
Kind: "CustomResourceDefinition",
APIVersion: "apiextensions.k8s.io/v1",
Metadata: policyv1.ObjectMetadata{
Name: "authorinos.operator.authorino.kuadrant.io",
},
},
Compliant: "Compliant",
Reason: "Resource found as expected",
}},
metav1.Condition{
Type: "CustomResourceDefinitionCompliant",
Status: metav1.ConditionTrue,
Reason: "RelevantCRDFound",
Message: "There are CRDs present for the operator",
},
"There are CRDs present for the operator",
)
})
})
Describe("Testing OperatorPolicy validation messages", Ordered, func() {
const (
opPolYAML = "../resources/case38_operator_install/operator-policy-validity-test.yaml"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
apiVersion: policy.open-cluster-management.io/v1beta1
kind: OperatorPolicy
metadata:
name: oppol-authorino
annotations:
policy.open-cluster-management.io/parent-policy-compliance-db-id: "124"
policy.open-cluster-management.io/policy-compliance-db-id: "64"
ownerReferences:
- apiVersion: policy.open-cluster-management.io/v1
kind: Policy
name: parent-policy
uid: 12345678-90ab-cdef-1234-567890abcdef # must be replaced before creation
spec:
remediationAction: inform
severity: medium
complianceType: musthave
subscription:
channel: stable
name: authorino-operator
namespace: operator-policy-testns
installPlanApproval: Automatic
source: operatorhubio-catalog
sourceNamespace: olm

0 comments on commit 6b1db91

Please sign in to comment.