[release-4.15] OCPBUGS-30570: Fix for ACM Policy Generator Plugin

controllers/clustergroupupgrade_controller.go

@@ -831,7 +831,7 @@ func (r *ClusterGroupUpgradeReconciler) updatePlacementRules(ctx context.Context
 
 	for index, clusterNames := range policiesToUpdate {
 		placementRuleName := utils.GetResourceName(clusterGroupUpgrade, clusterGroupUpgrade.Status.ManagedPoliciesForUpgrade[index].Name+"-placement")
-		if safeName, ok := clusterGroupUpgrade.Status.SafeResourceNames[placementRuleName]; ok {
+		if safeName, ok := clusterGroupUpgrade.Status.SafeResourceNames[utils.PrefixNameWithNamespace(clusterGroupUpgrade.Status.ManagedPoliciesForUpgrade[index].Namespace, placementRuleName)]; ok {
 			err := r.updatePlacementRuleWithClusters(ctx, clusterGroupUpgrade, clusterNames, safeName)
 			if err != nil {
 				return err
@@ -1120,7 +1120,7 @@ func (r *ClusterGroupUpgradeReconciler) copyManagedInformPolicy(
 	if annotations == nil {
 		annotations = make(map[string]string)
 	}
-	annotations[utils.DesiredResourceName] = name
+	annotations[utils.DesiredResourceName] = utils.PrefixNameWithNamespace(managedPolicy.GetNamespace(), name)
 	newPolicy.SetAnnotations(annotations)
 
 	// Set new policy remediationAction.
@@ -1234,15 +1234,15 @@ func (r *ClusterGroupUpgradeReconciler) updateConfigurationPolicyName(
 
 	metadataContent := metadata.(map[string]interface{})
 	name := utils.GetResourceName(clusterGroupUpgrade, metadataContent["name"].(string))
-	safeName := utils.GetSafeResourceName(name, clusterGroupUpgrade, utils.MaxPolicyNameLength, 0)
+	safeName := utils.GetSafeResourceName(name, "", clusterGroupUpgrade, utils.MaxPolicyNameLengthExcludingTheDot)
 	metadataContent["name"] = safeName
 }
 
 func (r *ClusterGroupUpgradeReconciler) createNewPolicyFromStructure(
 	ctx context.Context, clusterGroupUpgrade *ranv1alpha1.ClusterGroupUpgrade, policy *unstructured.Unstructured) error {
 
 	name := policy.GetName()
-	safeName := utils.GetSafeResourceName(name, clusterGroupUpgrade, utils.MaxPolicyNameLength, len(policy.GetNamespace())+1)
+	safeName := utils.GetSafeResourceName(name, policy.GetNamespace(), clusterGroupUpgrade, utils.MaxPolicyNameLengthExcludingTheDot)
 	policy.SetName(safeName)
 	if err := controllerutil.SetControllerReference(clusterGroupUpgrade, policy, r.Scheme); err != nil {
 		return err
@@ -1280,7 +1280,7 @@ func (r *ClusterGroupUpgradeReconciler) createNewPolicyFromStructure(
 func (r *ClusterGroupUpgradeReconciler) ensureBatchPlacementRule(ctx context.Context, clusterGroupUpgrade *ranv1alpha1.ClusterGroupUpgrade, policyName string, managedPolicy *unstructured.Unstructured) (string, error) {
 
 	name := utils.GetResourceName(clusterGroupUpgrade, managedPolicy.GetName()+"-placement")
-	safeName := utils.GetSafeResourceName(name, clusterGroupUpgrade, utils.MaxObjectNameLength, 0)
+	safeName := utils.GetSafeResourceName(name, managedPolicy.GetNamespace(), clusterGroupUpgrade, utils.MaxObjectNameLength)
 	pr := r.newBatchPlacementRule(clusterGroupUpgrade, policyName, safeName, name)
 
 	if err := controllerutil.SetControllerReference(clusterGroupUpgrade, pr, r.Scheme); err != nil {
@@ -1331,7 +1331,7 @@ func (r *ClusterGroupUpgradeReconciler) newBatchPlacementRule(clusterGroupUpgrad
 				utils.ExcludeFromClusterBackup:                         "true",
 			},
 			"annotations": map[string]interface{}{
-				utils.DesiredResourceName: desiredName,
+				utils.DesiredResourceName: utils.PrefixNameWithNamespace(clusterGroupUpgrade.Namespace, desiredName),
 			},
 		},
 		"spec": map[string]interface{}{
@@ -1447,9 +1447,8 @@ func (r *ClusterGroupUpgradeReconciler) isUpgradeComplete(ctx context.Context, c
 
 func (r *ClusterGroupUpgradeReconciler) ensureBatchPlacementBinding(
 	ctx context.Context, clusterGroupUpgrade *ranv1alpha1.ClusterGroupUpgrade, policyName, placementRuleName string, managedPolicy *unstructured.Unstructured) error {
-
 	name := utils.GetResourceName(clusterGroupUpgrade, managedPolicy.GetName()+"-placement")
-	safeName := utils.GetSafeResourceName(name, clusterGroupUpgrade, utils.MaxObjectNameLength, 0)
+	safeName := utils.GetSafeResourceName(name, managedPolicy.GetNamespace(), clusterGroupUpgrade, utils.MaxObjectNameLength)
 	// Ensure batch placement bindings.
 	pb := r.newBatchPlacementBinding(clusterGroupUpgrade, policyName, placementRuleName, safeName, name)
 
@@ -1510,7 +1509,7 @@ func (r *ClusterGroupUpgradeReconciler) newBatchPlacementBinding(clusterGroupUpg
 				utils.ExcludeFromClusterBackup:                         "true",
 			},
 			"annotations": map[string]interface{}{
-				utils.DesiredResourceName: desiredName,
+				utils.DesiredResourceName: utils.PrefixNameWithNamespace(clusterGroupUpgrade.Namespace, desiredName),
 			},
 		},
 		"placementRef": map[string]interface{}{

controllers/monitoring.go

@@ -206,7 +206,7 @@ func (r *ClusterGroupUpgradeReconciler) processMonitoredObject(
 	// Get the managedClusterView for the monitored object contained in the current managedPolicy.
 	// If missing, then return error.
 	mcvName := utils.GetMultiCloudObjectName(clusterGroupUpgrade, object.Kind, object.Name)
-	safeName := utils.GetSafeResourceName(mcvName, clusterGroupUpgrade, utils.MaxObjectNameLength, 0)
+	safeName := utils.GetSafeResourceName(mcvName, "", clusterGroupUpgrade, utils.MaxObjectNameLength)
 	mcv, err := utils.EnsureManagedClusterView(
 		ctx, r.Client, safeName, mcvName, clusterName, object.Kind+"."+strings.Split(object.APIVersion, "/")[0],
 		object.Name, *object.Namespace, clusterGroupUpgrade.Name, clusterGroupUpgrade.Namespace)

controllers/utils/common.go

@@ -3,6 +3,7 @@ package utils
 import (
 	"fmt"
 	"regexp"
+	"unicode/utf8"
 
 	ranv1alpha1 "github.com/openshift-kni/cluster-group-upgrades-operator/pkg/api/clustergroupupgrades/v1alpha1"
 	"k8s.io/apimachinery/pkg/util/rand"
@@ -32,25 +33,30 @@ func GetMinOf3(number1, number2, number3 int) int {
 }
 
 // GetSafeResourceName returns the safename if already allocated in the map and creates a new one if not
-func GetSafeResourceName(name string, clusterGroupUpgrade *ranv1alpha1.ClusterGroupUpgrade, maxLength, spareLength int) string {
+func GetSafeResourceName(name, namespace string, clusterGroupUpgrade *ranv1alpha1.ClusterGroupUpgrade, maxLength int) string {
 	if clusterGroupUpgrade.Status.SafeResourceNames == nil {
 		clusterGroupUpgrade.Status.SafeResourceNames = make(map[string]string)
 	}
-	safeName, ok := clusterGroupUpgrade.Status.SafeResourceNames[name]
+	safeName, ok := clusterGroupUpgrade.Status.SafeResourceNames[PrefixNameWithNamespace(namespace, name)]
+
 	if !ok {
-		safeName = NewSafeResourceName(name, clusterGroupUpgrade.GetAnnotations()[NameSuffixAnnotation], maxLength, spareLength)
-		clusterGroupUpgrade.Status.SafeResourceNames[name] = safeName
+		safeName = NewSafeResourceName(name, namespace, clusterGroupUpgrade.GetAnnotations()[NameSuffixAnnotation], maxLength)
+		clusterGroupUpgrade.Status.SafeResourceNames[PrefixNameWithNamespace(namespace, name)] = safeName
 	}
 	return safeName
 }
 
+const (
+	finalDashLength = 1
+)
+
 // NewSafeResourceName creates a safe name to use with random suffix and possible truncation based on limits passed in
-func NewSafeResourceName(name, suffix string, maxLength, spareLength int) string {
+func NewSafeResourceName(name, namespace, suffix string, maxLength int) (safename string) {
 	if suffix == "" {
 		suffix = rand.String(RandomNameSuffixLength)
 	}
-	suffixLength := len(suffix)
-	maxGeneratedNameLength := maxLength - suffixLength - spareLength - 1
+	suffixLength := utf8.RuneCountInString(suffix)
+	maxGeneratedNameLength := maxLength - suffixLength - utf8.RuneCountInString(namespace) - finalDashLength
 	var base string
 	if len(name) > maxGeneratedNameLength {
 		base = name[:maxGeneratedNameLength]
@@ -59,12 +65,17 @@ func NewSafeResourceName(name, suffix string, maxLength, spareLength int) string
 	}
 
 	// Make sure base ends in '-' or an alphanumerical character.
-	for !regexp.MustCompile(`^[a-zA-Z0-9-]*$`).MatchString(base[len(base)-1:]) {
-		base = base[:len(base)-1]
+	for !regexp.MustCompile(`^[a-zA-Z0-9-]*$`).MatchString(base[utf8.RuneCountInString(base)-1:]) {
+		base = base[:utf8.RuneCountInString(base)-1]
 	}
 
 	// The newSafeResourceName should match regex
 	// `[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*` as per
 	// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/
 	return fmt.Sprintf("%s-%s", base, suffix)
 }
+
+// PrefixNameWithNamespace Prefixes the passed name with the passed namespace. Use '/' as a separator
+func PrefixNameWithNamespace(namespace, name string) string {
+	return namespace + "/" + name
+}

controllers/utils/common_test.go

@@ -10,42 +10,42 @@ func TestNewSafeResourceNames(t *testing.T) {
 
 	name := "cnfdf18-new-common-cnfdf18-looooong-subscriptions-policy"
 	namespace := "ztp-install"
-	safeName := NewSafeResourceName(name, "", MaxPolicyNameLength, len(namespace)+1)
-	assert.Equal(t, MaxPolicyNameLength-len(namespace)-1, len(safeName))
-	assert.Equal(t, name[:MaxPolicyNameLength-len(namespace)-7]+"-", safeName[:MaxPolicyNameLength-len(namespace)-6])
+	safeName := NewSafeResourceName(name, namespace, "", MaxPolicyNameLengthExcludingTheDot)
+	assert.Equal(t, MaxPolicyNameLengthExcludingTheDot-len(namespace), len(safeName))
+	assert.Equal(t, name[:MaxPolicyNameLengthExcludingTheDot-len(namespace)-6]+"-", safeName[:MaxPolicyNameLengthExcludingTheDot-len(namespace)-5])
 
 	name = "cnfdf18-new-common-cnfdf18-looooong-subscriptions-policy-config"
-	safeName = NewSafeResourceName(name, "", MaxPolicyNameLength, 0)
-	assert.Equal(t, MaxPolicyNameLength, len(safeName))
-	assert.Equal(t, name[:MaxPolicyNameLength-7]+"-", safeName[:MaxPolicyNameLength-6])
+	safeName = NewSafeResourceName(name, "", "", MaxPolicyNameLengthExcludingTheDot)
+	assert.Equal(t, MaxPolicyNameLengthExcludingTheDot, len(safeName))
+	assert.Equal(t, name[:MaxPolicyNameLengthExcludingTheDot-6]+"-", safeName[:MaxPolicyNameLengthExcludingTheDot-5])
 
 	name = "cnfdf18-new-common-cnfdf18-looooong-subscriptions-policy-placement"
-	safeName = NewSafeResourceName(name, "", MaxObjectNameLength, 0)
+	safeName = NewSafeResourceName(name, "", "", MaxObjectNameLength)
 	assert.Equal(t, len(name)+6, len(safeName))
 	assert.Equal(t, name+"-", safeName[:len(name)+1])
 
 	name = "cnfdf18-new-common-cnfdf18-looooong-subscriptions-policy"
-	safeName = NewSafeResourceName(name, "kuttl", MaxPolicyNameLength, len(namespace)+1)
-	assert.Equal(t, MaxPolicyNameLength-len(namespace)-1, len(safeName))
-	assert.Equal(t, name[:MaxPolicyNameLength-len(namespace)-7]+"-kuttl", safeName)
+	safeName = NewSafeResourceName(name, namespace, "kuttl", MaxPolicyNameLengthExcludingTheDot)
+	assert.Equal(t, MaxPolicyNameLengthExcludingTheDot-len(namespace), len(safeName))
+	assert.Equal(t, name[:MaxPolicyNameLengthExcludingTheDot-len(namespace)-6]+"-kuttl", safeName)
 
 	name = "cnfdf18-new-common-cnfdf18-looooong-subscriptions-policy-config"
-	safeName = NewSafeResourceName(name, "kuttl", MaxPolicyNameLength, 0)
-	assert.Equal(t, MaxPolicyNameLength, len(safeName))
-	assert.Equal(t, name[:MaxPolicyNameLength-6]+"-kuttl", safeName)
+	safeName = NewSafeResourceName(name, "", "kuttl", MaxPolicyNameLengthExcludingTheDot)
+	assert.Equal(t, MaxPolicyNameLengthExcludingTheDot, len(safeName))
+	assert.Equal(t, name[:MaxPolicyNameLengthExcludingTheDot-6]+"-kuttl", safeName)
 
 	name = "cnfdf18-new-common-cnfdf18-looooong-subscriptions-policy-placement"
-	safeName = NewSafeResourceName(name, "kuttl", MaxObjectNameLength, 0)
+	safeName = NewSafeResourceName(name, "", "kuttl", MaxObjectNameLength)
 	assert.Equal(t, len(name)+6, len(safeName))
 	assert.Equal(t, name+"-kuttl", safeName)
 
 	name = "cgu-sriov-cloudransno-site9-spree-lb-du-cvslcm-4.14.0-rc.4-config"
-	safeName = NewSafeResourceName(name, "", MaxPolicyNameLength, 0)
-	assert.Equal(t, MaxPolicyNameLength-1, len(safeName))
-	assert.Equal(t, name[:MaxPolicyNameLength-7]+"-", safeName[:MaxPolicyNameLength-6])
+	safeName = NewSafeResourceName(name, "", "", MaxPolicyNameLengthExcludingTheDot)
+	assert.Equal(t, MaxPolicyNameLengthExcludingTheDot, len(safeName))
+	assert.Equal(t, name[:MaxPolicyNameLengthExcludingTheDot-6]+"-", safeName[:MaxPolicyNameLengthExcludingTheDot-5])
 
 	name = "cgu-sriov-cloudransno-site9-spree-lb-du-cvslcm-4.14.0-rc.4"
-	safeName = NewSafeResourceName(name, "", MaxPolicyNameLength, 8)
-	assert.Equal(t, MaxPolicyNameLength-9, len(safeName))
-	assert.Equal(t, name[:MaxPolicyNameLength-15]+"-", safeName[:MaxPolicyNameLength-14])
+	safeName = NewSafeResourceName(name, "", "12345678", MaxPolicyNameLengthExcludingTheDot)
+	assert.Equal(t, MaxPolicyNameLengthExcludingTheDot, len(safeName))
+	assert.Equal(t, name[:MaxPolicyNameLengthExcludingTheDot-9]+"-", safeName[:MaxPolicyNameLengthExcludingTheDot-8])
 }

controllers/utils/constants.go

@@ -41,10 +41,12 @@ const (
 
 // CR name length limits and suffix annotation
 const (
-	MaxPolicyNameLength    = 63
-	MaxObjectNameLength    = 253
-	NameSuffixAnnotation   = CsvNamePrefix + "/name-suffix"
-	RandomNameSuffixLength = 5
+	// Maximum length of policy + namespace (not including extra separator dot so 63 -1 = 62)
+	// this is calculated with utf8.RuneCountInString(policy.Name)+utf8.RuneCountInString+utf8.RuneCountInString(policy.Namespace)
+	MaxPolicyNameLengthExcludingTheDot = 62
+	MaxObjectNameLength                = 253
+	NameSuffixAnnotation               = CsvNamePrefix + "/name-suffix"
+	RandomNameSuffixLength             = 5
 )
 
 // Pre-cache constants

controllers/utils/multicloud_util.go

@@ -171,7 +171,7 @@ func EnsureManagedClusterView(
 					"openshift-cluster-group-upgrades/clusterGroupUpgradeNamespace": cguNamespace,
 				},
 				Annotations: map[string]string{
-					DesiredResourceName: name,
+					DesiredResourceName: PrefixNameWithNamespace(namespace, name),
 				},
 			}
 			viewSpec := viewv1beta1.ViewSpec{

tests/kuttl/tests/adjust-max-concurrency/00-assert.yaml

@@ -18,15 +18,15 @@ status:
   conditions:
   - message: All selected clusters are valid
     reason: ClusterSelectionCompleted
-    status: "True"
+    status: 'True'
     type: ClustersSelected
   - message: Completed validation
     reason: ValidationCompleted
-    status: "True"
+    status: 'True'
     type: Validated
   - message: Not enabled
     reason: NotEnabled
-    status: "False"
+    status: 'False'
     type: Progressing
   copiedPolicies:
   - cgu-adjust-max-conc-policy1-common-cluster-versio-kuttl
@@ -50,10 +50,10 @@ status:
   remediationPlan:
   - - spoke1
   safeResourceNames:
-    cgu-adjust-max-conc-common-cluster-version-policy-config: cgu-adjust-max-conc-common-cluster-version-policy-config-kuttl
-    cgu-adjust-max-conc-common-pao-sub-policy-config: cgu-adjust-max-conc-common-pao-sub-policy-config-kuttl
-    cgu-adjust-max-conc-policy1-common-cluster-version-policy: cgu-adjust-max-conc-policy1-common-cluster-versio-kuttl
-    cgu-adjust-max-conc-policy1-common-cluster-version-policy-placement: cgu-adjust-max-conc-policy1-common-cluster-version-policy-placement-kuttl
-    cgu-adjust-max-conc-policy2-common-pao-sub-policy: cgu-adjust-max-conc-policy2-common-pao-sub-policy-kuttl
-    cgu-adjust-max-conc-policy2-common-pao-sub-policy-placement: cgu-adjust-max-conc-policy2-common-pao-sub-policy-placement-kuttl
+    /cgu-adjust-max-conc-common-cluster-version-policy-config: cgu-adjust-max-conc-common-cluster-version-policy-config-kuttl
+    /cgu-adjust-max-conc-common-pao-sub-policy-config: cgu-adjust-max-conc-common-pao-sub-policy-config-kuttl
+    default/cgu-adjust-max-conc-policy1-common-cluster-version-policy: cgu-adjust-max-conc-policy1-common-cluster-versio-kuttl
+    default/cgu-adjust-max-conc-policy1-common-cluster-version-policy-placement: cgu-adjust-max-conc-policy1-common-cluster-version-policy-placement-kuttl
+    default/cgu-adjust-max-conc-policy2-common-pao-sub-policy: cgu-adjust-max-conc-policy2-common-pao-sub-policy-kuttl
+    default/cgu-adjust-max-conc-policy2-common-pao-sub-policy-placement: cgu-adjust-max-conc-policy2-common-pao-sub-policy-placement-kuttl
   status: {}

tests/kuttl/tests/backup-complete/00-assert.yaml

@@ -30,19 +30,19 @@ status:
   conditions:
   - message: All selected clusters are valid
     reason: ClusterSelectionCompleted
-    status: "True"
+    status: 'True'
     type: ClustersSelected
   - message: Completed validation
     reason: ValidationCompleted
-    status: "True"
+    status: 'True'
     type: Validated
   - message: Backup in progress for 4 clusters
     reason: InProgress
-    status: "False"
+    status: 'False'
     type: BackupSuceeded
   - message: Cluster backup is in progress
     reason: NotStarted
-    status: "False"
+    status: 'False'
     type: Progressing
   copiedPolicies:
   - cgu-policy3-common-ptp-sub-policy-kuttl
@@ -73,14 +73,13 @@ status:
     - spoke1
     - spoke5
   safeResourceNames:
-    cgu-common-ptp-sub-policy-config: cgu-common-ptp-sub-policy-config-kuttl
-    cgu-common-sriov-sub-policy-config: cgu-common-sriov-sub-policy-config-kuttl
-    cgu-policy3-common-ptp-sub-policy: cgu-policy3-common-ptp-sub-policy-kuttl
-    cgu-policy3-common-ptp-sub-policy-placement: cgu-policy3-common-ptp-sub-policy-placement-kuttl
-    cgu-policy4-common-sriov-sub-policy: cgu-policy4-common-sriov-sub-policy-kuttl
-    cgu-policy4-common-sriov-sub-policy-placement: cgu-policy4-common-sriov-sub-policy-placement-kuttl
+    /cgu-common-ptp-sub-policy-config: cgu-common-ptp-sub-policy-config-kuttl
+    /cgu-common-sriov-sub-policy-config: cgu-common-sriov-sub-policy-config-kuttl
+    default/cgu-policy3-common-ptp-sub-policy: cgu-policy3-common-ptp-sub-policy-kuttl
+    default/cgu-policy3-common-ptp-sub-policy-placement: cgu-policy3-common-ptp-sub-policy-placement-kuttl
+    default/cgu-policy4-common-sriov-sub-policy: cgu-policy4-common-sriov-sub-policy-kuttl
+    default/cgu-policy4-common-sriov-sub-policy-placement: cgu-policy4-common-sriov-sub-policy-placement-kuttl
 ---
-#MCAs
 apiVersion: action.open-cluster-management.io/v1beta1
 kind: ManagedClusterAction
 metadata:
@@ -123,7 +122,7 @@ spec:
   actionType: Delete
   kube:
     name: backup-agent
-    resource: clusterrolebinding    
+    resource: clusterrolebinding
 ---
 apiVersion: action.open-cluster-management.io/v1beta1
 kind: ManagedClusterAction
@@ -145,7 +144,7 @@ spec:
   actionType: Delete
   kube:
     name: backup-agent
-    resource: clusterrolebinding    
+    resource: clusterrolebinding
 ---
 apiVersion: action.open-cluster-management.io/v1beta1
 kind: ManagedClusterAction
@@ -167,9 +166,8 @@ spec:
   actionType: Delete
   kube:
     name: backup-agent
-    resource: clusterrolebinding    
+    resource: clusterrolebinding
 ---
-#MCVs
 apiVersion: view.open-cluster-management.io/v1beta1
 kind: ManagedClusterView
 metadata: