fix(metrics-operator): convert SLI names to valid K8s resource names (#…

…2125)

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Co-authored-by: odubajDT <93584209+odubajDT@users.noreply.github.com>

metrics-operator/converter/common.go

@@ -3,6 +3,8 @@ package converter
 import (
 	"fmt"
 	"math"
+	"regexp"
+	"strings"
 
 	"gopkg.in/inf.v0"
 )
@@ -27,9 +29,21 @@ func NewUnconvertableOperatorCombinationErr(op1, op2 string) error {
 	return fmt.Errorf("unconvertable combination of operators: '%s', '%s'", op1, op2)
 }
 
+func NewUnsupportedResourceNameErr(name string) error {
+	return fmt.Errorf(
+		"unsupported resource name: %s. Provided reosource name must match the pattern %s and must not have more than %d characters.",
+		name,
+		K8sResourceNameRegexp,
+		MaxResourceNameLength,
+	)
+}
+
 const MaxInt = math.MaxInt
 const MinInt = -MaxInt - 1
 
+const MaxResourceNameLength = 253
+const K8sResourceNameRegexp = "^[a-z0-9]([-a-z0-9.]*[a-z0-9])?$"
+
 type Operator struct {
 	Value     *inf.Dec
 	Operation string
@@ -47,3 +61,32 @@ func isGreaterOrEqual(op string) bool {
 func isLessOrEqual(op string) bool {
 	return op == "<" || op == "<="
 }
+
+func ValidateResourceName(name string) error {
+	pattern := "^[a-z0-9]([-a-z0-9.]*[a-z0-9])?$"
+
+	// Compile the regular expression.
+	regex := regexp.MustCompile(pattern)
+
+	// Check if the provided name matches the pattern.
+	if !regex.MatchString(name) || len(name) > MaxResourceNameLength {
+		return NewUnsupportedResourceNameErr(name)
+	}
+	return nil
+}
+
+func ConvertResourceName(name string) string {
+	// Replace non-alphanumeric characters with '-'
+	re := regexp.MustCompile("[^a-z0-9]+")
+	normalized := re.ReplaceAllString(strings.ToLower(name), "-")
+
+	// Remove leading and trailing '-'
+	normalized = strings.Trim(normalized, "-")
+
+	// Ensure the name is no longer than 253 characters
+	if len(normalized) > MaxResourceNameLength {
+		normalized = normalized[:MaxResourceNameLength]
+	}
+
+	return normalized
+}

metrics-operator/converter/common_test.go

@@ -19,3 +19,119 @@ func TestIsGreaterOrEqual(t *testing.T) {
 	require.True(t, isGreaterOrEqual(">"))
 	require.True(t, isGreaterOrEqual(">="))
 }
+
+const resourceNameWith253Characters = "my-sample-app-pod-name-is-a-very-long-example-name-012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890253-my-sample-app-pod-name-is-a-very-long-example-name-my-sample-app-pod-name-is-a-very-lon"
+
+func TestConvertResourceName(t *testing.T) {
+	type args struct {
+		name string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "contains invalid characters",
+			args: args{
+				name: "Invalid_resource",
+			},
+			want: "invalid-resource",
+		},
+		{
+			name: "starts with '-'",
+			args: args{
+				name: "-my-resource",
+			},
+			want: "my-resource",
+		},
+		{
+			name: "ends with '-'",
+			args: args{
+				name: "my-resource-",
+			},
+			want: "my-resource",
+		},
+		{
+			name: "empty string",
+			args: args{
+				name: "",
+			},
+			want: "",
+		},
+		{
+			name: "name is 253 characters long",
+			args: args{
+				name: resourceNameWith253Characters,
+			},
+			want: resourceNameWith253Characters,
+		},
+		{
+			name: "name is 254 characters long",
+			args: args{
+				name: resourceNameWith253Characters + "x",
+			},
+			want: resourceNameWith253Characters,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := ConvertResourceName(tt.args.name)
+			require.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func TestValidateResourceName(t *testing.T) {
+	type args struct {
+		name string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{
+			name: "valid resource name",
+			args: args{
+				name: "my-valid-resource-name-1",
+			},
+			wantErr: false,
+		},
+		{
+			name: "invalid resource name",
+			args: args{
+				name: "My-invalid-resource-name",
+			},
+			wantErr: true,
+		},
+		{
+			name: "invalid resource name containing '_'",
+			args: args{
+				name: "my_invalid-resource-name",
+			},
+			wantErr: true,
+		},
+		{
+			name: "253 characters long",
+			args: args{
+				name: resourceNameWith253Characters,
+			},
+			wantErr: false,
+		},
+		{
+			name: "254 characters long",
+			args: args{
+				name: resourceNameWith253Characters + "x",
+			},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := ValidateResourceName(tt.args.name); (err != nil) != tt.wantErr {
+				t.Errorf("ValidateResourceName() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}

metrics-operator/converter/sli_converter.go

@@ -23,8 +23,8 @@ func NewSLIConverter() *SLIConverter {
 
 func (c *SLIConverter) Convert(fileContent []byte, provider string, namespace string) (string, error) {
 	//check that provider and namespace is set
-	if provider == "" || namespace == "" {
-		return "", fmt.Errorf("missing arguments: 'keptn-provider-name' and 'keptn-provider-namespace' needs to be set for conversion")
+	if err := c.validateInput(provider, namespace); err != nil {
+		return "", err
 	}
 
 	// unmarshall content
@@ -63,7 +63,7 @@ func (c *SLIConverter) convertMapToAnalysisValueTemplate(slis map[string]string,
 				APIVersion: "metrics.keptn.sh/v1alpha3",
 			},
 			ObjectMeta: v1.ObjectMeta{
-				Name: key,
+				Name: ConvertResourceName(key),
 			},
 			Spec: metricsapi.AnalysisValueTemplateSpec{
 				Query: convertQuery(query),
@@ -79,9 +79,24 @@ func (c *SLIConverter) convertMapToAnalysisValueTemplate(slis map[string]string,
 	return result
 }
 
+func (c *SLIConverter) validateInput(provider, namespace string) error {
+	// check that provider and namespace is set
+	if provider == "" || namespace == "" {
+		return fmt.Errorf("missing arguments: 'keptn-provider-name' and 'keptn-provider-namespace' needs to be set for conversion")
+	}
+	if err := ValidateResourceName(provider); err != nil {
+		return err
+	}
+	if err := ValidateResourceName(namespace); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func convertQuery(query string) string {
-	// regex matching string starting with $, then upptercase letter
-	// followed by unlimited occurences of uppercase letters and numbers
+	// regex matching string starting with $, then uppercase letter
+	// followed by unlimited occurrences of uppercase letters and numbers
 	// examples: $LIST, $L, $L2T, $L555
 	re := regexp.MustCompile(`\$\b[A-Z][A-Z0-9]*\b`)
 	//get all substrings matching regex

metrics-operator/converter/sli_converter_test.go

@@ -19,7 +19,7 @@ apiVersion: metrics.keptn.sh/v1alpha3
 kind: AnalysisValueTemplate
 metadata:
   creationTimestamp: null
-  name: response_time_p95
+  name: response-time-p95
 spec:
   provider:
     name: dynatrace
@@ -167,3 +167,57 @@ func TestConvertQuery(t *testing.T) {
 
 	}
 }
+
+//nolint:dupl
+func TestSLIConverter_validateInput(t *testing.T) {
+	type args struct {
+		provider  string
+		namespace string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{
+			name: "no provider name",
+			args: args{
+				provider:  "",
+				namespace: "my-namespace",
+			},
+			wantErr: true,
+		},
+		{
+			name: "no namespace",
+			args: args{
+				provider:  "provider",
+				namespace: "",
+			},
+			wantErr: true,
+		},
+		{
+			name: "invalid provider name",
+			args: args{
+				provider:  "provider_name",
+				namespace: "my-namespace",
+			},
+			wantErr: true,
+		},
+		{
+			name: "invalid namespace",
+			args: args{
+				provider:  "provider",
+				namespace: "my_namespace",
+			},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := &SLIConverter{}
+			if err := c.validateInput(tt.args.provider, tt.args.namespace); (err != nil) != tt.wantErr {
+				t.Errorf("validateInput() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}

metrics-operator/converter/slo_converter.go

@@ -61,9 +61,9 @@ func (o *Objective) hasNotSupportedCriteria() bool {
 }
 
 func (c *SLOConverter) Convert(fileContent []byte, analysisDef string, namespace string) (string, error) {
-	//check that provider and namespace is set
-	if analysisDef == "" || namespace == "" {
-		return "", fmt.Errorf("missing arguments: 'analysis-definition-name' and 'analysis-value-template-namespace' needs to be set for conversion")
+	// validate inputs
+	if err := c.validateInput(analysisDef, namespace); err != nil {
+		return "", err
 	}
 
 	// unmarshall content
@@ -129,7 +129,7 @@ func (c *SLOConverter) convertSLO(sloContent *SLO, name string, namespace string
 		}
 		objective := metricsapi.Objective{
 			AnalysisValueTemplateRef: metricsapi.ObjectReference{
-				Name:      o.Name,
+				Name:      ConvertResourceName(o.Name),
 				Namespace: namespace,
 			},
 			KeyObjective: o.KeySLI,
@@ -142,6 +142,21 @@ func (c *SLOConverter) convertSLO(sloContent *SLO, name string, namespace string
 	return definition, nil
 }
 
+func (c *SLOConverter) validateInput(analysisDef, namespace string) error {
+	// check that provider and namespace is set
+	if analysisDef == "" || namespace == "" {
+		return fmt.Errorf("missing arguments: 'analysis-definition-name' and 'analysis-value-template-namespace' needs to be set for conversion")
+	}
+	if err := ValidateResourceName(analysisDef); err != nil {
+		return err
+	}
+	if err := ValidateResourceName(namespace); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // removes % symbol from the scoring values and converts to numeric value
 func removePercentage(str string) (int, error) {
 	t := strings.ReplaceAll(str, "%", "")