Merge pull request #690 from yuumasato/handle-deprecated-profiles

CMP-3149: Handle profile deprecation

Author: openshift-merge-bot[bot]

pkg/apis/compliance/v1alpha1/profilebundle_types.go

@@ -16,6 +16,9 @@ const ProfileBundleOwnerLabel = "compliance.openshift.io/profile-bundle"
 // ProfileImageDigestAnnotation is the parsed out digest of the content image
 const ProfileImageDigestAnnotation = "compliance.openshift.io/image-digest"
 
+// ProfileStatusAnnotation is the parsed out status from the data stream
+const ProfileStatusAnnotation = "compliance.openshift.io/profile-status"
+
 // DataStreamStatusType is the type for the data stream status
 type DataStreamStatusType string
 

pkg/controller/compliancescan/compliancescan_controller.go

@@ -14,6 +14,7 @@ import (
 	"github.com/ComplianceAsCode/compliance-operator/pkg/controller/common"
 	"github.com/ComplianceAsCode/compliance-operator/pkg/controller/metrics"
 	"github.com/ComplianceAsCode/compliance-operator/pkg/utils"
+	"github.com/ComplianceAsCode/compliance-operator/pkg/xccdf"
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -282,8 +283,106 @@ func (r *ReconcileComplianceScan) validate(instance *compv1alpha1.ComplianceScan
 	return true, nil
 }
 
+func (r *ReconcileComplianceScan) notifyUseOfDeprecatedProfile(instance *compv1alpha1.ComplianceScan, logger logr.Logger) error {
+	profile := &compv1alpha1.Profile{}
+	tp := &compv1alpha1.TailoredProfile{}
+	var profileName string
+
+	if instance.Spec.TailoringConfigMap != nil {
+		// The ComplianceScan references a TailoredProfile
+		tpName := xccdf.GetNameFromXCCDFTailoredProfileID(instance.Spec.Profile)
+
+		if err := r.Client.Get(context.TODO(), types.NamespacedName{Name: tpName, Namespace: common.GetComplianceOperatorNamespace()}, tp); err != nil {
+			logger.Error(err, "Could not get TailoredProfile", "TailoredProfile", tpName, "ComplianceScan", instance.Name)
+			return err
+		}
+		if tp.GetAnnotations()[compv1alpha1.ProfileStatusAnnotation] == "deprecated" {
+			logger.Info("ComplianceScan is running with a deprecated tailored profile", instance.Name, tp.Name)
+			r.Recorder.Eventf(
+				instance, corev1.EventTypeWarning, "DeprecatedTailoredProfile",
+				"TailoredProfile %s is deprecated and should be avoided. "+
+					"Please consider using another profile", tp.Name)
+			return nil
+		}
+
+		if tp.Spec.Extends == "" {
+			return nil
+		}
+		// The extends field references a profile by its full name
+		profileName = tp.Spec.Extends
+	} else {
+		var pbName string
+		pbs := &compv1alpha1.ProfileBundleList{}
+
+		// We first find the ProfileBundle matching the scan's spec, then we get the profile that matches the scan's Profile ID.
+		// We do this because a profile ID is not unique across all PBs, but is unique withing a PB.
+		// We can, but should not, infer the Profile name based on the ComplianceScan name.
+		// Advanced users still could create Suites and Scans with arbitrary names, and that is exactly what we do in our tests.
+		if err := r.Client.List(context.TODO(), pbs, client.InNamespace(common.GetComplianceOperatorNamespace())); err != nil {
+			logger.Error(err, "Could not list ProfileBundles")
+			return err
+		}
+		for _, pb := range pbs.Items {
+			if pb.Spec.ContentFile == instance.Spec.Content && pb.Spec.ContentImage == instance.Spec.ContentImage {
+				pbName = pb.Name
+				break
+			}
+		}
+		if pbName == "" {
+			err := goerrors.New("Could not find ProfileBundle used by scan")
+			logger.Error(err, "ComplianceScan uses non-existent ProfileBundle", "ComplianceScan", instance.Name, "Profile", instance.Spec.Profile)
+			return err
+		}
+
+		xccdfProfileName := xccdf.GetProfileNameFromID(instance.Spec.Profile)
+
+		// This is the full profile name,
+		// taking into account the possiblity of an 'upstream-' prefix in the PB.
+		profileName = pbName + "-" + xccdfProfileName
+	}
+
+	if err := r.Client.Get(context.TODO(), types.NamespacedName{Name: profileName, Namespace: common.GetComplianceOperatorNamespace()}, profile); err != nil {
+		logger.Error(err, "Could not get Profile", "profile", profileName, "ns", common.GetComplianceOperatorNamespace())
+		return err
+	}
+
+	if profile.GetAnnotations()[compv1alpha1.ProfileStatusAnnotation] == "deprecated" {
+		logger.Info("ComplianceScan is running with a deprecated profile", instance.Name, profile.Name)
+		if instance.Spec.TailoringConfigMap != nil {
+			r.Recorder.Eventf(
+				instance, corev1.EventTypeWarning, "DeprecatedTailoredProfile",
+				"TailoredProfile %s is extending a deprecated Profile (%s) that will be removed in a future version of Compliance Operator. "+
+					"Please consider using a newer version of this profile", tp.Name, profile.Name)
+		} else {
+			r.Recorder.Eventf(
+				instance, corev1.EventTypeWarning, "DeprecatedProfile",
+				"Profile %s is deprecated and will be removed in a future version of Compliance Operator. "+
+					"Please consider using a newer version of this profile", profile.Name)
+		}
+	}
+	return nil
+}
+
 func (r *ReconcileComplianceScan) phasePendingHandler(instance *compv1alpha1.ComplianceScan, logger logr.Logger) (reconcile.Result, error) {
 	logger.Info("Phase: Pending")
+
+	err := r.notifyUseOfDeprecatedProfile(instance, logger)
+	if err != nil {
+		logger.Error(err, "Could not check whether the Profile used by ComplianceScan is deprecated", "ComplianceScan", instance.Name)
+		instanceCopy := instance.DeepCopy()
+		instanceCopy.Status.Phase = compv1alpha1.PhaseDone
+		instanceCopy.Status.Result = compv1alpha1.ResultError
+		instanceCopy.Status.ErrorMessage = "Could not check whether the Profile used by ComplianceScan is deprecated"
+		instanceCopy.Status.StartTimestamp = &metav1.Time{Time: time.Now()}
+		instanceCopy.Status.EndTimestamp = &metav1.Time{Time: time.Now()}
+		err = r.Client.Status().Update(context.TODO(), instanceCopy)
+		if err != nil {
+			logger.Error(err, "Cannot update the status")
+			return reconcile.Result{}, err
+		}
+		return reconcile.Result{}, nil
+	}
+
 	// Remove annotation if needed
 	if instance.NeedsRescan() {
 		instanceCopy := instance.DeepCopy()
@@ -307,7 +406,7 @@ func (r *ReconcileComplianceScan) phasePendingHandler(instance *compv1alpha1.Com
 	instance.Status.Result = compv1alpha1.ResultNotAvailable
 	instance.Status.StartTimestamp = &metav1.Time{Time: time.Now()}
 	instance.Status.EndTimestamp = nil
-	err := r.Client.Status().Update(context.TODO(), instance)
+	err = r.Client.Status().Update(context.TODO(), instance)
 	if err != nil {
 		logger.Error(err, "Cannot update the status")
 		return reconcile.Result{}, err

pkg/controller/compliancescan/compliancescan_controller_test.go

@@ -137,6 +137,9 @@ var _ = Describe("Testing compliancescan controller phases", func() {
 						Size:          compv1alpha1.DefaultRawStorageSize,
 					},
 				},
+				ContentImage: "MyContentImage",
+				Content:      "MyContentFile",
+				Profile:      "xccdf_org.ssgproject.content_profile_unmoderate",
 			},
 		}
 		platformscaninstance = &compv1alpha1.ComplianceScan{
@@ -204,12 +207,44 @@ var _ = Describe("Testing compliancescan controller phases", func() {
 			},
 		}
 
-		objs = append(objs, nodeinstance1, nodeinstance2, caSecret, serverSecret, clientSecret, ns)
+		profileList := &compv1alpha1.ProfileList{}
+		profile := &compv1alpha1.Profile{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Profile",
+				APIVersion: compv1alpha1.SchemeGroupVersion.String(),
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "some-product-unmoderate",
+				Namespace: common.GetComplianceOperatorNamespace(),
+			},
+			ProfilePayload: compv1alpha1.ProfilePayload{
+				Title:       "Unmoderate setteings",
+				Description: "Describes obnoxious recommendations",
+				ID:          "xccdf_org.ssgproject.content_profile_unmoderate",
+			},
+		}
+		profileBundleList := &compv1alpha1.ProfileBundleList{}
+		profileBundle := &compv1alpha1.ProfileBundle{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "ProfileBundle",
+				APIVersion: compv1alpha1.SchemeGroupVersion.String(),
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "some-product",
+				Namespace: common.GetComplianceOperatorNamespace(),
+			},
+			Spec: compv1alpha1.ProfileBundleSpec{
+				ContentImage: "MyContentImage",
+				ContentFile:  "MyContentFile",
+			},
+		}
+
+		objs = append(objs, nodeinstance1, nodeinstance2, caSecret, serverSecret, clientSecret, ns, profile, profileBundle)
 		scheme := scheme.Scheme
-		scheme.AddKnownTypes(compv1alpha1.SchemeGroupVersion, compliancescaninstance, results)
+		scheme.AddKnownTypes(compv1alpha1.SchemeGroupVersion, compliancescaninstance, results, profile, profileList, profileBundle, profileBundleList)
 
 		statusObjs := []runtimeclient.Object{}
-		statusObjs = append(statusObjs, compliancescaninstance)
+		statusObjs = append(statusObjs, compliancescaninstance, profile)
 
 		client := fake.NewClientBuilder().
 			WithScheme(scheme).

pkg/controller/tailoredprofile/tailoredprofile_controller.go

@@ -169,6 +169,15 @@ func (r *ReconcileTailoredProfile) Reconcile(ctx context.Context, request reconc
 			return r.setOwnership(tpCopy, p)
 		}
 
+		// Warn that TailoredProfile extends deprecated Profile
+		if p.GetAnnotations()[cmpv1alpha1.ProfileStatusAnnotation] == "deprecated" {
+			reqLogger.Info("TailoredProfile extends a deprecated profile", instance.Name, p.Name)
+			r.Recorder.Eventf(
+				instance, corev1.EventTypeWarning, "DeprecatedTailoredProfile",
+				"TailoredProfile %s is extending a deprecated Profile (%s) that will be removed in a future version of Compliance Operator. "+
+					"Please consider using a newer version of this profile", instance.Name, p.Name)
+		}
+
 	} else {
 		if !isValidationRequired(instance) {
 			// check if the TailoredProfile is empty without any extends

pkg/profileparser/profileparser.go

@@ -310,6 +310,8 @@ func parseProfileFromNode(profileRoot *xmlquery.Node, pb *cmpv1alpha1.ProfileBun
 		if id == "" {
 			return LogAndReturnError("no id in profile")
 		}
+		// Profile status is an optional field and can be nil
+		status := profileObj.SelectElement("xccdf-1.2:status")
 		title := profileObj.SelectElement("xccdf-1.2:title")
 		if title == nil {
 			return LogAndReturnError("no title in profile")
@@ -387,6 +389,7 @@ func parseProfileFromNode(profileRoot *xmlquery.Node, pb *cmpv1alpha1.ProfileBun
 		}
 
 		annotateWithNonce(&p, nonce)
+		annotateWithStatus(&p, status)
 
 		err := action(&p)
 		if err != nil {
@@ -794,6 +797,17 @@ func annotateWithNonce(o metav1.Object, nonce string) {
 	o.SetAnnotations(annotations)
 }
 
+func annotateWithStatus(o metav1.Object, status *xmlquery.Node) {
+	if status != nil {
+		annotations := o.GetAnnotations()
+		if annotations == nil {
+			annotations = make(map[string]string)
+		}
+		annotations[cmpv1alpha1.ProfileStatusAnnotation] = status.InnerText()
+		o.SetAnnotations(annotations)
+	}
+}
+
 type complianceStandard struct {
 	Name        string
 	hrefMatcher *regexp.Regexp

pkg/xccdf/tailoring.go

@@ -88,6 +88,12 @@ func GetXCCDFProfileID(tp *cmpv1alpha1.TailoredProfile) string {
 	return fmt.Sprintf("xccdf_%s_profile_%s", XCCDFNamespace, tp.Name)
 }
 
+// GetNameFromXCCDFTailoredProfileID gets the name of a tailored profile
+// from the TailoredProfile ID
+func GetNameFromXCCDFTailoredProfileID(id string) string {
+	return strings.TrimPrefix(id, fmt.Sprintf("xccdf_%s_profile_", XCCDFNamespace))
+}
+
 // GetProfileNameFromID gets a profile name from the xccdf ID
 func GetProfileNameFromID(id string) string {
 	trimedName := strings.TrimPrefix(id, profileIDPrefix)

tests/e2e/framework/common.go

@@ -176,6 +176,28 @@ func (f *Framework) PrintROSADebugInfo(t *testing.T) {
 	}
 }
 
+func (f *Framework) CreateProfileBundle(pbName string, baselineImage string, contentFile string) (*compv1alpha1.ProfileBundle, error) {
+	origPb := &compv1alpha1.ProfileBundle{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      pbName,
+			Namespace: f.OperatorNamespace,
+		},
+		Spec: compv1alpha1.ProfileBundleSpec{
+			ContentImage: baselineImage,
+			ContentFile:  contentFile,
+		},
+	}
+	// Pass nil in as the cleanupOptions since so we don't invoke all the
+	// cleanup function code in Create. Use defer to cleanup the
+	// ProfileBundle at the end of the test, instead of at the end of the
+	// suite.
+	log.Printf("Creating ProfileBundle %s", pbName)
+	if err := f.Client.Create(context.TODO(), origPb, nil); err != nil {
+		return nil, err
+	}
+	return origPb, nil
+}
+
 func (f *Framework) cleanUpProfileBundle(p string) error {
 	pb := &compv1alpha1.ProfileBundle{
 		ObjectMeta: metav1.ObjectMeta{
@@ -449,6 +471,55 @@ func (f *Framework) ensureTestNamespaceExists() error {
 
 }
 
+func (f *Framework) WaitForProfileDeprecatedWarning(t *testing.T, scanName string, profileName string) error {
+	polledScan := &compv1alpha1.ComplianceScan{}
+
+	// Wait for profile deprecation warning event
+	err := wait.Poll(RetryInterval, Timeout, func() (bool, error) {
+		getErr := f.Client.Get(context.TODO(), types.NamespacedName{Name: scanName, Namespace: f.OperatorNamespace}, polledScan)
+		if getErr != nil {
+			t.Log(getErr)
+			return false, nil
+		}
+
+		profileEventList, getEventErr := f.KubeClient.CoreV1().Events(f.OperatorNamespace).List(context.TODO(), metav1.ListOptions{
+			FieldSelector: "reason=DeprecatedProfile",
+		})
+		if getEventErr != nil {
+			t.Log(getEventErr)
+			return false, nil
+		}
+
+		tailoredProfileEventList, getEventErr := f.KubeClient.CoreV1().Events(f.OperatorNamespace).List(context.TODO(), metav1.ListOptions{
+			FieldSelector: "reason=DeprecatedTailoredProfile",
+		})
+		if getEventErr != nil {
+			t.Log(getEventErr)
+			return false, nil
+		}
+
+		re := regexp.MustCompile(fmt.Sprintf(".*%s.*", profileName))
+		for _, item := range profileEventList.Items {
+			if item.InvolvedObject.Name == polledScan.Name && re.MatchString(item.Message) {
+				t.Logf("Found ComplianceScan deprecated profile event: %s", item.Message)
+				return true, nil
+			}
+		}
+		for _, item := range tailoredProfileEventList.Items {
+			if item.InvolvedObject.Name == polledScan.Name && re.MatchString(item.Message) {
+				t.Logf("Found ComplianceScan deprecated profile event: %s", item.Message)
+				return true, nil
+			}
+		}
+		return false, nil
+	})
+	if err != nil {
+		t.Fatalf("No ComplianceScan event for profile \"%s\" found", profileName)
+		return err
+	}
+	return nil
+}
+
 // waitForProfileBundleStatus will poll until the compliancescan that we're
 // lookingfor reaches a certain status, or until a timeout is reached.
 func (f *Framework) WaitForProfileBundleStatus(name string, status compv1alpha1.DataStreamStatusType) error {
@@ -1160,7 +1231,7 @@ func (f *Framework) AssertScanIsCompliant(name, namespace string) error {
 		return err
 	}
 	if cs.Status.Result != compv1alpha1.ResultCompliant {
-		return fmt.Errorf("scan result was %s instead of %s", compv1alpha1.ResultCompliant, cs.Status.Result)
+		return fmt.Errorf("scan result was %s instead of %s", cs.Status.Result, compv1alpha1.ResultCompliant)
 	}
 	return nil
 }
@@ -1238,7 +1309,7 @@ func (f *Framework) AssertScanIsNonCompliant(name, namespace string) error {
 		return err
 	}
 	if cs.Status.Result != compv1alpha1.ResultNonCompliant {
-		return fmt.Errorf("scan result was %s instead of %s", compv1alpha1.ResultNonCompliant, cs.Status.Result)
+		return fmt.Errorf("scan result was %s instead of %s", cs.Status.Result, compv1alpha1.ResultNonCompliant)
 	}
 	return nil
 }
@@ -1251,7 +1322,7 @@ func (f *Framework) AssertScanIsNotApplicable(name, namespace string) error {
 		return err
 	}
 	if cs.Status.Result != compv1alpha1.ResultNotApplicable {
-		return fmt.Errorf("scan result was %s instead of %s", compv1alpha1.ResultNotApplicable, cs.Status.Result)
+		return fmt.Errorf("scan result was %s instead of %s", cs.Status.Result, compv1alpha1.ResultNotApplicable)
 	}
 	return nil
 }
@@ -1264,7 +1335,7 @@ func (f *Framework) AssertScanIsInError(name, namespace string) error {
 		return err
 	}
 	if cs.Status.Result != compv1alpha1.ResultError {
-		return fmt.Errorf("scan result was %s instead of %s", compv1alpha1.ResultError, cs.Status.Result)
+		return fmt.Errorf("scan result was %s instead of %s", cs.Status.Result, compv1alpha1.ResultError)
 	}
 	if cs.Status.ErrorMessage == "" {
 		return fmt.Errorf("scan 'errormsg' is empty, but it should be set")