Merge branch 'openshift:master' into other-arch-images

CHANGELOG.md

@@ -13,7 +13,14 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixes
 
--
+- When a TailoredProfile transitions from Ready to Error, the corresponding
+  ConfigMap is removed. This prevents the ConfigMap from being reused with
+  obsolete data while the parent object is in fact marked with an error
+- The ScanSettingBinding controller now reconciles TailoredProfile instances
+  related to a ScanSettingBinding. This ensures that the controller can
+  proceed with generating scans in case the binding used to point to 
+  TailoredProfile that had been marked with an error, but was subsequently
+  fixed ([upstream issue 791](https://github.com/openshift/compliance-operator/issues/791))
 
 ### Internal Changes
 

pkg/controller/scansettingbinding/scansettingbinding_controller.go

@@ -72,7 +72,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 		return err
 	}
 
-	// Watch for changes to secordary resource ScanSetting. Since Setting does not link directly to a Binding,
+	// Watch for changes to secondary resource ScanSetting. Since Setting does not link directly to a Binding,
 	// but the other way around, we use a mapper to enqueue requests for Binding(s) used by a Setting
 	err = c.Watch(&source.Kind{Type: &compliancev1alpha1.ScanSetting{}}, &handler.EnqueueRequestsFromMapFunc{
 		ToRequests: &scanSettingMapper{mgr.GetClient()},
@@ -81,6 +81,15 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 		return err
 	}
 
+	// Watch for changes to secondary resource TailoredProfile. Since TailoredProfile does not link directly to a Binding,
+	// but the other way around, we use a mapper to enqueue requests for TailoredProfiles(s) used by a Setting
+	err = c.Watch(&source.Kind{Type: &compliancev1alpha1.TailoredProfile{}}, &handler.EnqueueRequestsFromMapFunc{
+		ToRequests: &tailoredProfileMapper{mgr.GetClient()},
+	})
+	if err != nil {
+		return err
+	}
+
 	// Watch for changes to secondary resource ComplianceScans and requeue the owner ComplianceSuite
 	err = c.Watch(&source.Kind{Type: &compliancev1alpha1.ComplianceSuite{}}, &handler.EnqueueRequestForOwner{
 		IsController: true,

pkg/controller/scansettingbinding/tailored_profile_mapper.go

@@ -0,0 +1,53 @@
+package scansettingbinding
+
+import (
+	"context"
+	"github.com/openshift/compliance-operator/pkg/apis/compliance/v1alpha1"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+)
+
+type tailoredProfileMapper struct {
+	client.Client
+}
+
+func (s *tailoredProfileMapper) Map(obj handler.MapObject) []reconcile.Request {
+	var requests []reconcile.Request
+
+	ssbList := v1alpha1.ScanSettingBindingList{}
+	err := s.List(context.TODO(), &ssbList, &client.ListOptions{})
+	if err != nil {
+		return requests
+	}
+
+	for _, ssb := range ssbList.Items {
+		add := false
+
+		for _, profRef := range ssb.Profiles {
+			if profRef.Kind != "TailoredProfile" {
+				continue
+			}
+
+			if profRef.Name != obj.Meta.GetName() {
+				continue
+			}
+
+			add = true
+			break
+		}
+
+		if add == false {
+			continue
+		}
+
+		objKey := types.NamespacedName{
+			Name:      ssb.GetName(),
+			Namespace: ssb.GetNamespace(),
+		}
+		requests = append(requests, reconcile.Request{NamespacedName: objKey})
+	}
+
+	return requests
+}

pkg/controller/tailoredprofile/tailoredprofile_controller.go

@@ -114,7 +114,7 @@ func (r *ReconcileTailoredProfile) Reconcile(request reconcile.Request) (reconci
 		p, pb, pbgetErr = r.getProfileInfoFromExtends(instance)
 		if pbgetErr != nil && !common.IsRetriable(pbgetErr) {
 			// the Profile or ProfileBundle objects didn't exist. Surface the error.
-			err = r.updateTailoredProfileStatusError(instance, pbgetErr)
+			err = r.handleTailoredProfileStatusError(instance, pbgetErr)
 			return reconcile.Result{}, err
 		} else if pbgetErr != nil {
 			return reconcile.Result{}, pbgetErr
@@ -132,7 +132,7 @@ func (r *ReconcileTailoredProfile) Reconcile(request reconcile.Request) (reconci
 		pb, pbgetErr = r.getProfileBundleFromRulesOrVars(instance)
 		if pbgetErr != nil && !common.IsRetriable(pbgetErr) {
 			// the Profile or ProfileBundle objects didn't exist. Surface the error.
-			err = r.updateTailoredProfileStatusError(instance, pbgetErr)
+			err = r.handleTailoredProfileStatusError(instance, pbgetErr)
 			return reconcile.Result{}, err
 		} else if pbgetErr != nil {
 			return reconcile.Result{}, pbgetErr
@@ -166,22 +166,22 @@ func (r *ReconcileTailoredProfile) Reconcile(request reconcile.Request) (reconci
 	rules, ruleErr := r.getRulesFromSelections(instance, pb)
 	if ruleErr != nil && !common.IsRetriable(ruleErr) {
 		// Surface the error.
-		suerr := r.updateTailoredProfileStatusError(instance, ruleErr)
+		suerr := r.handleTailoredProfileStatusError(instance, ruleErr)
 		return reconcile.Result{}, suerr
 	} else if ruleErr != nil {
 		return reconcile.Result{}, ruleErr
 	}
 
 	if ruleValidErr := assertValidRuleTypes(rules); ruleValidErr != nil {
 		// Surface the error.
-		suerr := r.updateTailoredProfileStatusError(instance, ruleValidErr)
+		suerr := r.handleTailoredProfileStatusError(instance, ruleValidErr)
 		return reconcile.Result{}, suerr
 	}
 
 	variables, varErr := r.getVariablesFromSelections(instance, pb)
 	if varErr != nil && !common.IsRetriable(varErr) {
 		// Surface the error.
-		suerr := r.updateTailoredProfileStatusError(instance, varErr)
+		suerr := r.handleTailoredProfileStatusError(instance, varErr)
 		return reconcile.Result{}, suerr
 	} else if varErr != nil {
 		return reconcile.Result{}, varErr
@@ -195,7 +195,7 @@ func (r *ReconcileTailoredProfile) Reconcile(request reconcile.Request) (reconci
 		return reconcile.Result{}, err
 	}
 
-	return r.ensureOutputObject(instance, tpcm, pb, reqLogger)
+	return r.ensureOutputObject(instance, tpcm, reqLogger)
 }
 
 // getProfileInfoFromExtends gets the Profile and ProfileBundle where the rules come from
@@ -345,6 +345,14 @@ func (r *ReconcileTailoredProfile) updateTailoredProfileStatusReady(tp *cmpv1alp
 	return r.client.Status().Update(context.TODO(), tpCopy)
 }
 
+func (r *ReconcileTailoredProfile) handleTailoredProfileStatusError(tp *cmpv1alpha1.TailoredProfile, err error) error {
+	if delErr := r.deleteOutputObject(tp); delErr != nil {
+		return delErr
+	}
+
+	return r.updateTailoredProfileStatusError(tp, err)
+}
+
 func (r *ReconcileTailoredProfile) updateTailoredProfileStatusError(tp *cmpv1alpha1.TailoredProfile, err error) error {
 	// Never update the original (update the copy)
 	tpCopy := tp.DeepCopy()
@@ -366,7 +374,26 @@ func (r *ReconcileTailoredProfile) getProfileBundleFrom(objtype string, o metav1
 	return &pb, err
 }
 
-func (r *ReconcileTailoredProfile) ensureOutputObject(tp *cmpv1alpha1.TailoredProfile, tpcm *corev1.ConfigMap, pb *cmpv1alpha1.ProfileBundle, logger logr.Logger) (reconcile.Result, error) {
+func (r *ReconcileTailoredProfile) deleteOutputObject(tp *cmpv1alpha1.TailoredProfile) error {
+	// make sure the configMap is removed so that we don't keep using the old one after
+	// breaking the TP
+	tpcm := newTailoredProfileCM(tp)
+	err := r.client.Get(context.TODO(), types.NamespacedName{Name: tpcm.Name, Namespace: tpcm.Namespace}, tpcm)
+	if err != nil && kerrors.IsNotFound(err) {
+		return nil
+	} else if err != nil {
+		return err
+	}
+
+	err = r.client.Delete(context.TODO(), tpcm)
+	if err != nil && !kerrors.IsNotFound(err) {
+		return err
+	}
+
+	return nil
+}
+
+func (r *ReconcileTailoredProfile) ensureOutputObject(tp *cmpv1alpha1.TailoredProfile, tpcm *corev1.ConfigMap, logger logr.Logger) (reconcile.Result, error) {
 	// Set TailoredProfile instance as the owner and controller
 	if err := controllerutil.SetControllerReference(tp, tpcm, r.scheme); err != nil {
 		return reconcile.Result{}, err

pkg/controller/tailoredprofile/tailoredprofile_controller_test.go

@@ -3,6 +3,7 @@ package tailoredprofile
 import (
 	"context"
 	"fmt"
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
 
 	"github.com/openshift/compliance-operator/pkg/controller/metrics"
 	"github.com/openshift/compliance-operator/pkg/controller/metrics/metricsfakes"
@@ -258,6 +259,74 @@ var _ = Describe("TailoredprofileController", func() {
 			Expect(data).To(ContainSubstring(`select idref="rule_3" selected="true"`))
 			Expect(data).NotTo(ContainSubstring(`select idref="rule_2" selected="true"`))
 		})
+		It("Removes a ConfigMap when the TP is updated and doesn't parse anymore", func() {
+			tpKey := types.NamespacedName{
+				Name:      tpName,
+				Namespace: namespace,
+			}
+			tpReq := reconcile.Request{}
+			tpReq.Name = tpName
+			tpReq.Namespace = namespace
+
+			By("Reconciling the first time")
+			_, err := r.Reconcile(tpReq)
+			Expect(err).To(BeNil())
+
+			By("Reconciling a second time")
+			_, err = r.Reconcile(tpReq)
+			Expect(err).To(BeNil())
+
+			tp := &compv1alpha1.TailoredProfile{}
+			geterr := r.client.Get(ctx, tpKey, tp)
+			Expect(geterr).To(BeNil())
+
+			By("Generated an appropriate ConfigMap")
+			cm := &corev1.ConfigMap{}
+			cmKey := types.NamespacedName{
+				Name:      tp.Status.OutputRef.Name,
+				Namespace: tp.Status.OutputRef.Namespace,
+			}
+
+			geterr = r.client.Get(ctx, cmKey, cm)
+			Expect(geterr).To(BeNil())
+			data := cm.Data["tailoring.xml"]
+			Expect(data).To(ContainSubstring(`extends="profile_1"`))
+			Expect(data).To(ContainSubstring(`select idref="rule_3" selected="true"`))
+			Expect(data).To(ContainSubstring(`select idref="rule_2" selected="false"`))
+
+			By("Update the TP")
+			tpUpdate := &compv1alpha1.TailoredProfile{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      tpName,
+					Namespace: namespace,
+				},
+				Spec: compv1alpha1.TailoredProfileSpec{
+					Extends: profileName,
+					EnableRules: []compv1alpha1.RuleReferenceSpec{
+						{
+							Name:      "rule-xxx",
+							Rationale: "Why not",
+						},
+					},
+				},
+			}
+
+			tp.Spec = *tpUpdate.Spec.DeepCopy()
+			updateErr := r.client.Update(ctx, tp)
+			Expect(updateErr).To(BeNil())
+
+			By("Reconcile the updated TP")
+			_, err = r.Reconcile(tpReq)
+			Expect(err).To(BeNil())
+
+			By("Fetch the updated TP")
+			geterr = r.client.Get(ctx, tpKey, tp)
+			Expect(geterr).To(BeNil())
+
+			By("Assert that the CM is now removed")
+			geterr = r.client.Get(ctx, cmKey, cm)
+			Expect(kerrors.IsNotFound(geterr)).To(BeTrue())
+		})
 	})
 
 	When("extending a profile with reference to another bundle", func() {