Add warnings field to TailoredProfile CRD

config/crd/bases/compliance.openshift.io_tailoredprofiles.yaml

@@ -156,6 +156,8 @@ spec:
               state:
                 description: The current state of the tailored profile
                 type: string
+              warnings:
+                type: string
             type: object
         type: object
     served: true

pkg/apis/compliance/v1alpha1/tailoredprofile_types.go

@@ -7,19 +7,16 @@ import (
 // FIXME: move name/rationale to a common struct with an interface?
 
 // DisableOutdatedReferenceValidation a label is used to disable validation of outdated references
-const DisableOutdatedReferenceValidation = "compliance.openshift.io/outdated-reference-validation-disable"
+const DisableOutdatedReferenceValidation = "compliance.openshift.io/disable-outdated-reference-validation"
 
-// PurneOutdatedReferencesAnnotationKey is the annotation key used to indicate that the outdated references of rules or variables should be pruned
-const PurneOutdatedReferencesAnnotationKey = "compliance.openshift.io/prune-outdated-references"
+// PruneOutdatedReferencesAnnotationKey is the annotation key used to indicate that the outdated references of rules or variables should be pruned
+const PruneOutdatedReferencesAnnotationKey = "compliance.openshift.io/prune-outdated-references"
 
 // RuleLastCheckTypeChangedAnnotationKey is the annotation key used to indicate that the rule check type has changed, store its previous check type
 const RuleLastCheckTypeChangedAnnotationKey = "compliance.openshift.io/rule-last-check-type"
 
-// VariableDeprecatedAnnotationKey is the annotation key used to indicate that the variable is deprecated
-const VariableDeprecatedAnnotationKey = "compliance.openshift.io/variable-deprecated"
-
-// DeprecatedRuleAnnotationKey is the annotation key used to indicate that the rule is deprecated
-const DeprecatedRuleAnnotationKey = "compliance.openshift.io/deprecated-rule"
+// DeprecatedAnnotationKey is the annotation key used to indicate that the variable or rule is deprecated
+const DeprecatedAnnotationKey = "compliance.openshift.io/deprecated"
 
 // RuleReferenceSpec specifies a rule to be selected/deselected, as well as the reason why
 type RuleReferenceSpec struct {
@@ -89,6 +86,7 @@ type TailoredProfileStatus struct {
 	// The current state of the tailored profile
 	State        TailoredProfileState `json:"state,omitempty"`
 	ErrorMessage string               `json:"errorMessage,omitempty"`
+	Warnings     string               `json:"warnings,omitempty"`
 }
 
 // OutputRef is a reference to the object created from the tailored profile

pkg/controller/tailoredprofile/tailoredprofile_controller.go

@@ -208,26 +208,36 @@ func (r *ReconcileTailoredProfile) Reconcile(ctx context.Context, request reconc
 	if _, ok := ann[cmpv1alpha1.DisableOutdatedReferenceValidation]; ok {
 		reqLogger.Info("Reference validation is disabled, skipping validation")
 	} else if isValidationRequired(instance) {
-		pureOutdated := false
-		if _, ok := ann[cmpv1alpha1.PurneOutdatedReferencesAnnotationKey]; ok {
-			pureOutdated = true
+		pruneOutdated := false
+		if _, ok := ann[cmpv1alpha1.PruneOutdatedReferencesAnnotationKey]; ok {
+			pruneOutdated = true
 		}
 		// remove any deprecated variables or rules from the tailored profile
-		doContinue, err := r.handleDeprecation(instance, reqLogger, pureOutdated)
+		doContinue, variableNeedtoBeDeprecatedList, ruleNeedToBeDeprecatedList, err := r.handleDeprecation(instance, reqLogger, pruneOutdated)
 		if err != nil {
 			return reconcile.Result{}, err
 		}
 		if !doContinue {
 			return reconcile.Result{}, nil
 		}
 
-		doContinue, err = r.handleMigration(instance, reqLogger, pureOutdated)
+		doContinue, ruleNeedToBeMigratedList, err := r.handleMigration(instance, reqLogger, pruneOutdated)
 		if err != nil {
 			return reconcile.Result{}, err
 		}
 		if !doContinue {
 			return reconcile.Result{}, nil
 		}
+
+		warningMsg := generateWarningMessage(variableNeedtoBeDeprecatedList, ruleNeedToBeDeprecatedList, ruleNeedToBeMigratedList)
+		// check if warning message matches the previous warning message
+		// if it does, we don't need to update the tp, if not we need to update it with the new warning message
+		if warningMsg != instance.Status.Warnings {
+			tpCopy := instance.DeepCopy()
+			tpCopy.Status.Warnings = warningMsg
+			r.Client.Update(context.Background(), tpCopy)
+		}
+
 	}
 
 	rules, ruleErr := r.getRulesFromSelections(instance, pb)
@@ -265,24 +275,49 @@ func (r *ReconcileTailoredProfile) Reconcile(ctx context.Context, request reconc
 	return r.ensureOutputObject(instance, tpcm, reqLogger)
 }
 
+// generateWarningMessage generates a warning message for the user
+// based on the list of deprecated variables and rules that are detected
+// as well as the list of migrated rules that are detected that are not
+// migrated yet
+func generateWarningMessage(variableNeedtoBeDeprecatedList []string, ruleNeedToBeDeprecatedList []string, ruleNeedToBeMigratedList []string) string {
+	var warningMessage string
+	if len(variableNeedtoBeDeprecatedList) > 0 {
+		warningMessage = fmt.Sprintf("The following variables are deprecated and need to be removed from the TailoredProfile: %s\n", strings.Join(variableNeedtoBeDeprecatedList, ","))
+	}
+	if len(ruleNeedToBeDeprecatedList) > 0 {
+		if warningMessage != "" {
+			warningMessage = fmt.Sprintf("%sThe following rules are deprecated and need to be removed from the TailoredProfile: %s\n", warningMessage, strings.Join(ruleNeedToBeDeprecatedList, ","))
+		} else {
+			warningMessage = fmt.Sprintf("The following rules are deprecated and need to be removed from the TailoredProfile: %s\n", strings.Join(ruleNeedToBeDeprecatedList, ","))
+		}
+	}
+	if len(ruleNeedToBeMigratedList) > 0 {
+		if warningMessage != "" {
+			warningMessage = fmt.Sprintf("%sThe following rules are migrated and need to be migrated or removed from the TailoredProfile: %s\n", warningMessage, strings.Join(ruleNeedToBeMigratedList, ","))
+		} else {
+			warningMessage = fmt.Sprintf("The following rules are migrated and need to be migrated or removed from the TailoredProfile: %s\n", strings.Join(ruleNeedToBeMigratedList, ","))
+		}
+	}
+	return warningMessage
+}
+
 // handleMigration check if there are any migrated rules in the TailoredProfile
 // and we will handle the migration of the tailored profile accordingly
 func (r *ReconcileTailoredProfile) handleMigration(
-	v1alphaTp *cmpv1alpha1.TailoredProfile, logger logr.Logger, pruneOudated bool) (bool, error) {
-
+	v1alphaTp *cmpv1alpha1.TailoredProfile, logger logr.Logger, pruneOudated bool) (doContinue bool, ruleNeedToBeMigratedList []string, err error) {
+	doContinue = true
 	// Get the list of KubeletConfig rules that are migrated with checkType change
 	migratedRules, err := r.getMigratedRules(v1alphaTp, logger)
 	if err != nil {
-		return false, err
+		return false, nil, err
 	}
 
 	if len(migratedRules) == 0 {
-		return true, nil
+		return true, nil, nil
 	}
 
 	profileType := utils.GetScanType(v1alphaTp.GetAnnotations())
 
-	doContinue := true
 	v1alphaTpCP := v1alphaTp.DeepCopy()
 
 	// check if there are any disabled rules that are migrated
@@ -295,10 +330,11 @@ func (r *ReconcileTailoredProfile) handleMigration(
 				if pruneOudated {
 					doContinue = false
 					logger.Info("Removing migrated rule from disableRules", "rule", rule.Name)
-					r.Eventf(v1alphaTp, corev1.EventTypeWarning, "TailoredProfileMigratedRule", "Removing migrated rule: %s from disableRules", rule.Name)
+					r.Eventf(v1alphaTp, corev1.EventTypeWarning, "TailoredProfileMigratedRule", "Removing migrated rule: %s from disableRules, it has been changed from %s to %s", rule.Name, checkType, profileType)
 				} else {
 					logger.Info("Migrated rule detected in disableRules", "rule", rule.Name)
-					r.Eventf(v1alphaTp, corev1.EventTypeWarning, "TailoredProfileMigratedRule", "Migrated rule detected: %s Please migrate it or remove it from the TailoredProfile", rule.Name)
+					r.Eventf(v1alphaTp, corev1.EventTypeWarning, "TailoredProfileMigratedRule", "%s type changed from %s to %s. Please migrate it or remove it from the TailoredProfile", rule.Name, checkType, profileType)
+					ruleNeedToBeMigratedList = append(ruleNeedToBeMigratedList, rule.Name)
 					newRules = append(newRules, *rule)
 				}
 				continue
@@ -319,10 +355,11 @@ func (r *ReconcileTailoredProfile) handleMigration(
 				if pruneOudated {
 					doContinue = false
 					logger.Info("Removing migrated rule from enableRules", "rule", rule.Name)
-					r.Eventf(v1alphaTp, corev1.EventTypeWarning, "TailoredProfileMigratedRule", "Removing migrated rule %s from enableRules", rule.Name)
+					r.Eventf(v1alphaTp, corev1.EventTypeWarning, "TailoredProfileMigratedRule", "Removing migrated rule %s from enableRules, it has been changed from %s to %s", rule.Name, checkType, profileType)
 				} else {
 					logger.Info("Migrated rule detected in enableRules", "rule", rule.Name)
-					r.Eventf(v1alphaTp, corev1.EventTypeWarning, "TailoredProfileMigratedRule", "Migrated rule detected: %s Please migrate it or remove it from the TailoredProfile", rule.Name)
+					r.Eventf(v1alphaTp, corev1.EventTypeWarning, "TailoredProfileMigratedRule", "%s type changed from %s to %s. Please migrate it or remove it from the TailoredProfile", rule.Name, checkType, profileType)
+					ruleNeedToBeMigratedList = append(ruleNeedToBeMigratedList, rule.Name)
 					newRules = append(newRules, *rule)
 				}
 				continue
@@ -346,35 +383,34 @@ func (r *ReconcileTailoredProfile) handleMigration(
 
 	if !doContinue {
 		err = r.Client.Update(context.TODO(), v1alphaTpCP)
-		logger.Info("Updating TailoredProfile with migrated rules removed")
+		logger.Info("Updating TailoredProfile after handling migration")
 		if err != nil {
-			return false, err
+			return false, nil, err
 		}
 	}
-	return doContinue, nil
+	return doContinue, ruleNeedToBeMigratedList, nil
 }
 
 func isValidationRequired(tp *cmpv1alpha1.TailoredProfile) bool {
 	if tp.Spec.Extends != "" {
 		return tp.Spec.DisableRules != nil || tp.Spec.EnableRules != nil || tp.Spec.ManualRules != nil || tp.Spec.SetValues != nil
 	}
-	return tp.Spec.EnableRules != nil || tp.Spec.ManualRules != nil
+	return tp.Spec.EnableRules != nil || tp.Spec.ManualRules != nil || tp.Spec.SetValues != nil
 }
 
 func (r *ReconcileTailoredProfile) handleDeprecation(
-	v1alphaTp *cmpv1alpha1.TailoredProfile, logger logr.Logger, pruneOudated bool) (bool, error) {
-
+	v1alphaTp *cmpv1alpha1.TailoredProfile, logger logr.Logger, pruneOudated bool) (doContinue bool, variableNeedtoBeDeprecatedList []string, ruleNeedToBeDeprecatedList []string, err error) {
+	doContinue = true
 	deprecatedRules, err := r.getDeprecatedRules(v1alphaTp, logger)
 	if err != nil {
-		return false, err
+		return false, nil, nil, err
 	}
 
 	deprecatedVariables, err := r.getDeprecatedVariables(v1alphaTp, logger)
 	if err != nil {
-		return false, err
+		return false, nil, nil, err
 	}
 
-	doContinue := true
 	v1alphaTpCP := v1alphaTp.DeepCopy()
 
 	// remove any deprecated variables from the tailored profile
@@ -390,6 +426,7 @@ func (r *ReconcileTailoredProfile) handleDeprecation(
 				} else {
 					logger.Info("Deprecated variable detected", "variable", variables.Name)
 					r.Eventf(v1alphaTp, corev1.EventTypeWarning, "TailoredProfileDeprecatedSetting", "Deprecated variable detected: %s Please remove it from the TailoredProfile", variables.Name)
+					variableNeedtoBeDeprecatedList = append(variableNeedtoBeDeprecatedList, variables.Name)
 					newVariables = append(newVariables, *variables)
 				}
 				continue
@@ -415,6 +452,7 @@ func (r *ReconcileTailoredProfile) handleDeprecation(
 				} else {
 					logger.Info("Deprecated rule detected in enableRules", "rule", rule.Name)
 					r.Eventf(v1alphaTp, corev1.EventTypeWarning, "TailoredProfileDeprecatedSetting", "Deprecated rule detected: %s Please remove it from the TailoredProfile", rule.Name)
+					ruleNeedToBeDeprecatedList = append(ruleNeedToBeDeprecatedList, rule.Name)
 					newRules = append(newRules, *rule)
 				}
 				continue
@@ -439,6 +477,7 @@ func (r *ReconcileTailoredProfile) handleDeprecation(
 				} else {
 					logger.Info("Deprecated rule detected in disableRules", "rule", rule.Name)
 					r.Eventf(v1alphaTp, corev1.EventTypeWarning, "TailoredProfileDeprecatedSetting", "Deprecated rule detected: %s Please remove it from the TailoredProfile", rule.Name)
+					ruleNeedToBeDeprecatedList = append(ruleNeedToBeDeprecatedList, rule.Name)
 					newRules = append(newRules, *rule)
 				}
 				continue
@@ -466,10 +505,10 @@ func (r *ReconcileTailoredProfile) handleDeprecation(
 		err = r.Client.Update(context.TODO(), v1alphaTpCP)
 		logger.Info("Updating TailoredProfile with deprecated rules and variables removed")
 		if err != nil {
-			return false, err
+			return false, nil, nil, err
 		}
 	}
-	return doContinue, nil
+	return doContinue, variableNeedtoBeDeprecatedList, ruleNeedToBeDeprecatedList, nil
 }
 
 // getMigratedRules get list of rules and check if it has RuleLastCheckTypeChangedAnnotationKey annotation
@@ -518,7 +557,7 @@ func (r *ReconcileTailoredProfile) getDeprecatedVariables(tp *cmpv1alpha1.Tailor
 	for vi := range variableList.Items {
 		variable := &variableList.Items[vi]
 		if variable.Annotations != nil {
-			if _, ok := variable.Annotations[cmpv1alpha1.VariableDeprecatedAnnotationKey]; ok {
+			if _, ok := variable.Annotations[cmpv1alpha1.DeprecatedAnnotationKey]; ok {
 				deprecatedVariables[variable.GetName()] = ""
 			}
 		}
@@ -542,7 +581,7 @@ func (r *ReconcileTailoredProfile) getDeprecatedRules(tp *cmpv1alpha1.TailoredPr
 	for ri := range ruleList.Items {
 		rule := &ruleList.Items[ri]
 		if rule.Annotations != nil {
-			if _, ok := rule.Annotations[cmpv1alpha1.DeprecatedRuleAnnotationKey]; ok {
+			if _, ok := rule.Annotations[cmpv1alpha1.DeprecatedAnnotationKey]; ok {
 				deprecatedRules[rule.GetName()] = ""
 			}
 		}

tests/e2e/parallel/main_test.go

@@ -2163,6 +2163,7 @@ func TestScanSettingBindingTailoringManyEnablingRulePass(t *testing.T) {
 		moderateProfileName = "moderate"
 		tpMixName           = "many-migrated-mix-tp"
 		tpSingleName        = "migrated-single-tp"
+		tpSingleNoPruneName = "migrated-single-no-prune-tp"
 	)
 	var (
 		baselineImage = fmt.Sprintf("%s:%s", brokenContentImagePath, "kubelet_default")
@@ -2219,7 +2220,7 @@ func TestScanSettingBindingTailoringManyEnablingRulePass(t *testing.T) {
 			Name:      tpMixName,
 			Namespace: f.OperatorNamespace,
 			Annotations: map[string]string{
-				compv1alpha1.PurneOutdatedReferencesAnnotationKey: "",
+				compv1alpha1.PruneOutdatedReferencesAnnotationKey: "",
 			},
 		},
 		Spec: compv1alpha1.TailoredProfileSpec{
@@ -2243,12 +2244,29 @@ func TestScanSettingBindingTailoringManyEnablingRulePass(t *testing.T) {
 			Name:      tpSingleName,
 			Namespace: f.OperatorNamespace,
 			Annotations: map[string]string{
-				compv1alpha1.PurneOutdatedReferencesAnnotationKey: "",
+				compv1alpha1.PruneOutdatedReferencesAnnotationKey: "",
 			},
 		},
 		Spec: compv1alpha1.TailoredProfileSpec{
 			Title:       "TestForManyRules",
 			Description: "TestForManyRules",
+			EnableRules: []compv1alpha1.RuleReferenceSpec{
+				{
+					Name:      changeTypeRuleName,
+					Rationale: "this rule should be removed from the profile",
+				},
+			},
+		},
+	}
+
+	tpSingleNoPrune := &compv1alpha1.TailoredProfile{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      tpSingleNoPruneName,
+			Namespace: f.OperatorNamespace,
+		},
+		Spec: compv1alpha1.TailoredProfileSpec{
+			Title:       "TestForNoPrune",
+			Description: "TestForNoPrune",
 			EnableRules: []compv1alpha1.RuleReferenceSpec{
 				{
 					Name:      changeTypeRuleName,
@@ -2305,6 +2323,27 @@ func TestScanSettingBindingTailoringManyEnablingRulePass(t *testing.T) {
 		t.Fatalf("Expected the tailored profile to have rule: %s", changeTypeRuleName)
 	}
 
+	// tpSingleNoPrune test
+	createTPErr = f.Client.Create(context.TODO(), tpSingleNoPrune, nil)
+	if createTPErr != nil {
+		t.Fatal(createTPErr)
+	}
+	defer f.Client.Delete(context.TODO(), tpSingleNoPrune)
+	// check the status of the TP to make sure it has no errors
+	err = f.WaitForTailoredProfileStatus(f.OperatorNamespace, tpSingleNoPruneName, compv1alpha1.TailoredProfileStateReady)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	hasRule, err = f.EnableRuleExistInTailoredProfile(f.OperatorNamespace, tpSingleNoPruneName, changeTypeRuleName)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if !hasRule {
+		t.Fatalf("Expected the tailored profile to have rule: %s", changeTypeRuleName)
+	}
+
 	// update the image with a new hash
 	modPb := origPb.DeepCopy()
 	if err := f.Client.Get(context.TODO(), types.NamespacedName{Namespace: modPb.Namespace, Name: modPb.Name}, modPb); err != nil {
@@ -2351,7 +2390,7 @@ func TestScanSettingBindingTailoringManyEnablingRulePass(t *testing.T) {
 	}
 
 	// check that the tp has been updated with the removed rule singleTP
-	err = f.WaitForTailoredProfileStatus(f.OperatorNamespace, tpSingleName, compv1alpha1.TailoredProfileStateReady)
+	err = f.WaitForTailoredProfileStatus(f.OperatorNamespace, tpSingleName, compv1alpha1.TailoredProfileStateError)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -2360,10 +2399,40 @@ func TestScanSettingBindingTailoringManyEnablingRulePass(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	if hasRule {
+		t.Fatalf("Expected the tailored profile not to have rule: %s", changeTypeRuleName)
+	}
+
+	// check that the no prune tp still has the rule
+	err = f.WaitForTailoredProfileStatus(f.OperatorNamespace, tpSingleNoPruneName, compv1alpha1.TailoredProfileStateReady)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	hasRule, err = f.EnableRuleExistInTailoredProfile(f.OperatorNamespace, tpSingleNoPruneName, changeTypeRuleName)
+	if err != nil {
+		t.Fatal(err)
+	}
 	if !hasRule {
 		t.Fatalf("Expected the tailored profile to have rule: %s", changeTypeRuleName)
 	}
 
+	// check that we have a warning message in the tailored profile
+	tpSingleNoPruneFetched := &compv1alpha1.TailoredProfile{}
+	key := types.NamespacedName{Namespace: f.OperatorNamespace, Name: tpSingleNoPruneName}
+	if err := f.Client.Get(context.Background(), key, tpSingleNoPruneFetched); err != nil {
+		t.Fatal(err)
+	}
+
+	if len(tpSingleNoPruneFetched.Status.Warnings) == 0 {
+		t.Fatal("Expected the tailored profile to have a warning message")
+	}
+
+	// check that the warning message is about the rule
+	if !strings.Contains(tpSingleNoPruneFetched.Status.Warnings, changeTypeRule) {
+		t.Fatalf("Expected the tailored profile to have a warning message about migrated rule: %s but got: %s", changeTypeRule, tpSingleNoPruneFetched.Status.Warnings)
+	}
+
 }
 
 func TestScanSettingBindingUsesDefaultScanSetting(t *testing.T) {