From e007b52bda29d91b67762c620fbb004b7823555b Mon Sep 17 00:00:00 2001
From: mprahl <mprahl@users.noreply.github.com>
Date: Wed, 10 Apr 2024 16:35:35 -0400
Subject: [PATCH] Validate the subscription name

Signed-off-by: mprahl <mprahl@users.noreply.github.com>
---
 controllers/operatorpolicy_controller.go      | 11 ++++
 controllers/operatorpolicy_controller_test.go | 59 +++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go
index e8098969..f8f1d95e 100644
--- a/controllers/operatorpolicy_controller.go
+++ b/controllers/operatorpolicy_controller.go
@@ -348,6 +348,17 @@ func buildSubscription(
 		return nil, fmt.Errorf("the policy spec.subscription is invalid: %w", err)
 	}
 
+	name, ok := sub["name"].(string)
+	if !ok || name == "" {
+		return nil, fmt.Errorf("name is required in spec.subscription")
+	}
+
+	if validationErrs := validation.IsDNS1123Label(name); len(validationErrs) != 0 {
+		return nil, fmt.Errorf(
+			"the name '%v' used for the subscription is invalid: %s", name, strings.Join(validationErrs, ", "),
+		)
+	}
+
 	ns, ok := sub["namespace"].(string)
 	if !ok {
 		if defaultNS == "" {
diff --git a/controllers/operatorpolicy_controller_test.go b/controllers/operatorpolicy_controller_test.go
index 55ef19bb..9e7f8941 100644
--- a/controllers/operatorpolicy_controller_test.go
+++ b/controllers/operatorpolicy_controller_test.go
@@ -50,6 +50,65 @@ func TestBuildSubscription(t *testing.T) {
 	assert.Equal(t, ret.ObjectMeta.Namespace, "default")
 }
 
+func TestBuildSubscriptionInvalidNames(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name     string
+		expected string
+	}{
+		{
+			name:     "",
+			expected: "name is required in spec.subscription",
+		},
+		{
+			name: "wrong$s",
+			expected: "the name 'wrong$s' used for the subscription is invalid: a lowercase RFC 1123 label must " +
+				"consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric " +
+				"character (e.g. 'my-name',  or '123-abc', regex used for validation is " +
+				"'[a-z0-9]([-a-z0-9]*[a-z0-9])?')",
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+
+		t.Run(
+			"name="+test.name,
+			func(t *testing.T) {
+				t.Parallel()
+
+				testPolicy := &policyv1beta1.OperatorPolicy{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "my-policy",
+						Namespace: "default",
+					},
+					Spec: policyv1beta1.OperatorPolicySpec{
+						Severity:          "low",
+						RemediationAction: "enforce",
+						ComplianceType:    "musthave",
+						Subscription: runtime.RawExtension{
+							Raw: []byte(`{
+								"namespace": "default",
+								"source": "my-catalog",
+								"sourceNamespace": "my-ns",
+								"name": "` + test.name + `",
+								"channel": "stable",
+								"startingCSV": "my-operator-v1",
+								"installPlanApproval": "Automatic"
+							}`),
+						},
+					},
+				}
+
+				// Check values are correctly bootstrapped to the Subscription
+				_, err := buildSubscription(testPolicy, "my-operators")
+				assert.Equal(t, err.Error(), test.expected)
+			},
+		)
+	}
+}
+
 func TestBuildOperatorGroup(t *testing.T) {
 	testPolicy := &policyv1beta1.OperatorPolicy{
 		ObjectMeta: metav1.ObjectMeta{