Fix label key truncation for subscription annotations (#2731)

Signed-off-by: Per Goncalves da Silva <[email protected]>

Co-authored-by: Per Goncalves da Silva <[email protected]>
Upstream-repository: operator-lifecycle-manager
Upstream-commit: 1bdf969e6dff317e975de21078d60dd1406235e0

Author: perdasilva

staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go

@@ -128,14 +128,32 @@ func (o *Operator) ComponentLabelKey() (string, error) {
 		return o.componentLabelKey, nil
 	}
 
-	if o.GetName() == "" {
+	name := o.GetName()
+	if name == "" {
 		return "", fmt.Errorf(componentLabelKeyError, "empty name field")
 	}
 
-	name := o.GetName()
 	if len(name) > 63 {
 		// Truncate
 		name = name[0:63]
+
+		// Remove trailing illegal characters
+		idx := len(name) - 1
+		for ; idx >= 0; idx-- {
+			lastChar := name[idx]
+			if lastChar != '.' && lastChar != '_' && lastChar != '-' {
+				break
+			}
+		}
+
+		// Just being defensive. This is unlikely to happen since the operator name should
+		// be compatible kubernetes naming constraints
+		if idx < 0 {
+			return "", fmt.Errorf(componentLabelKeyError, "unsupported name field")
+		}
+
+		// Update Label
+		name = name[0 : idx+1]
 	}
 	o.componentLabelKey = ComponentLabelKeyPrefix + name
 

staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator_test.go

@@ -231,3 +231,61 @@ func TestAddComponents(t *testing.T) {
 		})
 	}
 }
+
+func TestComponentLabelKey(t *testing.T) {
+	tests := []struct {
+		description       string
+		componentLabelKey string
+		name              string
+		expectedLabel     string
+		returnsError      bool
+	}{
+		{
+			description:       "when componentLabelKey is set then return it",
+			componentLabelKey: "my-component-label",
+			name:              "my-operator",
+			expectedLabel:     "my-component-label",
+			returnsError:      false,
+		},
+		{
+			description:       "when componentLabelKey is not set and operator name is less than 63 characters then do not truncate",
+			componentLabelKey: "",
+			name:              "my-operator",
+			expectedLabel:     ComponentLabelKeyPrefix + "my-operator",
+			returnsError:      false,
+		},
+		{
+			description:   "when componentLabelKey is not set and operator name is more than 63 characters truncate",
+			name:          "this-is-my-operator-its-the-coolest-you-got-a-problem-with-that-come-at-me-bro",
+			expectedLabel: ComponentLabelKeyPrefix + "this-is-my-operator-its-the-coolest-you-got-a-problem-with-that",
+			returnsError:  false,
+		},
+		{
+			description:   "when componentLabelKey is not set and operator name is more than 63 characters truncate and drop trailing illegal characters",
+			name:          "this-is-my-operator-its-the-coolest-you-got-a-problem-with----...---___...---",
+			expectedLabel: ComponentLabelKeyPrefix + "this-is-my-operator-its-the-coolest-you-got-a-problem-with",
+			returnsError:  false,
+		},
+		{
+			description:   "when componentLabelKey is not set and operator name is more than 63 characters and is made up of illegal characters then return error",
+			name:          "----...---___...-------...---___...-------...---___...-------...---___...---",
+			expectedLabel: "",
+			returnsError:  true,
+		},
+	}
+
+	for _, tt := range tests {
+		operator := &Operator{
+			Operator: &operatorsv1.Operator{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: tt.name,
+				},
+			},
+			componentLabelKey: tt.componentLabelKey,
+		}
+
+		actualLabel, actualErr := operator.ComponentLabelKey()
+		require.Equal(t, tt.returnsError, actualErr != nil, actualErr)
+		require.Equal(t, tt.expectedLabel, actualLabel)
+	}
+}