From bc42fb758a4dc5edf8fa1bc16ac7df1a1968f191 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Mon, 11 Apr 2022 14:26:04 +0200 Subject: [PATCH] Fix label key truncation for subscription annotations (#2731) Signed-off-by: Per Goncalves da Silva Co-authored-by: Per Goncalves da Silva Upstream-repository: operator-lifecycle-manager Upstream-commit: 1bdf969e6dff317e975de21078d60dd1406235e0 --- .../operators/decorators/operator.go | 22 ++++++- .../operators/decorators/operator_test.go | 58 +++++++++++++++++++ .../operators/decorators/operator.go | 22 ++++++- 3 files changed, 98 insertions(+), 4 deletions(-) diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go b/staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go index 4046f7ee09..64868f8b89 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go +++ b/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 diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator_test.go b/staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator_test.go index 3c6ee8dbf9..b438680ce1 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator_test.go +++ b/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) + } +} diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go index 4046f7ee09..64868f8b89 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go +++ b/vendor/github.com/operator-framework/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