OCPBUGS-11228: Check for duplicate variables in TailoredProfile

Implement duplicate detection for setValues in TailoredProfile and add warning event
[OCPBUGS-11228](https://issues.redhat.com/browse/OCPBUGS-11228)

Author: Vincent056

pkg/controller/tailoredprofile/tailoredprofile_controller.go

@@ -527,14 +527,37 @@ func (r *ReconcileTailoredProfile) getRulesFromSelections(tp *cmpv1alpha1.Tailor
 	}
 	return rules, nil
 }
-
 func (r *ReconcileTailoredProfile) getVariablesFromSelections(tp *cmpv1alpha1.TailoredProfile, pb *cmpv1alpha1.ProfileBundle) ([]*cmpv1alpha1.Variable, error) {
-	variableList := []*cmpv1alpha1.Variable{}
-	for _, setValues := range tp.Spec.SetValues {
+	// 1) First pass over tp.Spec.SetValues to detect duplicates, warn, and decide which value to keep
+	//    We choose to keep the last occurrence. The map "duplicates" maps variableName -> finalValue.
+	//    The slice "uniqueVarOrder" tracks the first time we saw the variable (for stable iteration).
+	duplicates := make(map[string]string)
+	uniqueVarOrder := []string{}
+
+	for i, setValue := range tp.Spec.SetValues {
+		if oldVal, seen := duplicates[setValue.Name]; seen {
+			// We found a duplicate. Warn that we will ignore the previous usage.
+			r.Eventf(tp, corev1.EventTypeWarning,
+				"DuplicatedSetValue",
+				"Variable '%s' appears multiple times in setValues. The operator will keep the last usage with value '%s' (position %d), ignoring the previous value '%s'. This will fail in a future release. Please remove duplicates from setValues.",
+				setValue.Name, setValue.Value, i, oldVal)
+		} else {
+			// Only record order the first time we see the variable
+			uniqueVarOrder = append(uniqueVarOrder, setValue.Name)
+		}
+		// Overwrite with the newest/last usage
+		duplicates[setValue.Name] = setValue.Value
+	}
+
+	// 2) Second pass to actually retrieve the Variables and set their final values
+	variableList := make([]*cmpv1alpha1.Variable, 0, len(uniqueVarOrder))
+	for _, varName := range uniqueVarOrder {
+		finalValue := duplicates[varName]
+
+		// Grab the Variable from the cluster
 		variable := &cmpv1alpha1.Variable{}
-		varKey := types.NamespacedName{Name: setValues.Name, Namespace: tp.Namespace}
-		err := r.Client.Get(context.TODO(), varKey, variable)
-		if err != nil {
+		varKey := types.NamespacedName{Name: varName, Namespace: tp.Namespace}
+		if err := r.Client.Get(context.TODO(), varKey, variable); err != nil {
 			if kerrors.IsNotFound(err) {
 				return nil, common.NewNonRetriableCtrlError("fetching variable: %w", err)
 			}
@@ -548,8 +571,7 @@ func (r *ReconcileTailoredProfile) getVariablesFromSelections(tp *cmpv1alpha1.Ta
 		}
 
 		// try setting the variable, this also validates the value
-		err = variable.SetValue(setValues.Value)
-		if err != nil {
+		if err := variable.SetValue(finalValue); err != nil {
 			return nil, common.NewNonRetriableCtrlError("setting variable: %s", err)
 		}
 

pkg/controller/tailoredprofile/tailoredprofile_controller_test.go

@@ -837,6 +837,94 @@ var _ = Describe("TailoredprofileController", func() {
 			})
 		})
 
+		Context("with duplicated setValues for the same variable", func() {
+			var tpName = "tailoring-duplicate-vars"
+
+			BeforeEach(func() {
+				// Create a TailoredProfile with duplicate entries for "var-1"
+				// Note "var-1" is owned by pb1 in your test setup if i < 5
+				tp := &compv1alpha1.TailoredProfile{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      tpName,
+						Namespace: namespace,
+					},
+					Spec: compv1alpha1.TailoredProfileSpec{
+						// No extends here, so it picks up the PB from the first rule/variable it finds
+						EnableRules: []compv1alpha1.RuleReferenceSpec{
+							{
+								Name:      "rule-1",
+								Rationale: "Ensure ownership detection picks pb1",
+							},
+						},
+						SetValues: []compv1alpha1.VariableValueSpec{
+							{
+								Name:  "var-1",
+								Value: "1111",
+							},
+							{
+								Name:  "var-1",
+								Value: "2222", // Duplicate #2
+							},
+							{
+								Name:  "var-2",
+								Value: "3333",
+							},
+							{
+								Name:  "var-1",
+								Value: "4444", // Duplicate #3 (final mention)
+							},
+						},
+					},
+				}
+
+				createErr := r.Client.Create(ctx, tp)
+				Expect(createErr).To(BeNil())
+			})
+
+			It("keeps the last mention of the duplicated variable and raises a warning", func() {
+				tpKey := types.NamespacedName{
+					Name:      tpName,
+					Namespace: namespace,
+				}
+				tpReq := reconcile.Request{}
+				tpReq.Name = tpName
+				tpReq.Namespace = namespace
+
+				By("Reconciling the TailoredProfile the first time (to set ownership)")
+				_, err := r.Reconcile(context.TODO(), tpReq)
+				Expect(err).To(BeNil())
+
+				By("Reconciling the TailoredProfile the second time (to process setValues)")
+				_, err = r.Reconcile(context.TODO(), tpReq)
+				Expect(err).To(BeNil())
+
+				By("Ensuring the TailoredProfile ends in the Ready state")
+				tp := &compv1alpha1.TailoredProfile{}
+				geterr := r.Client.Get(ctx, tpKey, tp)
+				Expect(geterr).To(BeNil())
+				Expect(tp.Status.State).To(Equal(compv1alpha1.TailoredProfileStateReady))
+				Expect(tp.Status.ErrorMessage).To(BeEmpty())
+
+				By("Ensuring the final configMap uses the last mention of var-1 (4444)")
+				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"]
+				// The final mention for var-1 must be "4444" in the rendered tailoring
+				Expect(data).To(ContainSubstring(`idref="var_1"`))
+				Expect(data).To(ContainSubstring(`4444`))
+				// Ensure that 2222 or 1111 never appear, verifying the earlier duplicates are discarded
+				Expect(data).NotTo(ContainSubstring("1111"))
+				Expect(data).NotTo(ContainSubstring("2222"))
+				Expect(data).To(ContainSubstring(`idref="var_2"`))
+				Expect(data).To(ContainSubstring(`3333`))
+			})
+		})
+
 		Context("with no rules nor variables", func() {
 			BeforeEach(func() {
 				tp := &compv1alpha1.TailoredProfile{