SecureValues: Remove actor prefix from decrypters (#107433)

Co-authored-by: Matheus Macabu <macabu@users.noreply.github.com>

pkg/registry/apis/secret/reststorage/secure_value_rest.go

@@ -13,6 +13,7 @@ import (
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/endpoints/request"
@@ -308,7 +309,7 @@ func validateSecureValueUpdate(sv, oldSv *secretv0alpha1.SecureValue) field.Erro
 	return errs
 }
 
-// validateDecrypters validates that (if populated) the `decrypters` must match "actor_{name}" and must be unique.
+// validateDecrypters validates that (if populated) the `decrypters` must be unique.
 func validateDecrypters(decrypters []string, decryptersAllowList map[string]struct{}) field.ErrorList {
 	errs := make(field.ErrorList, 0)
 
@@ -326,8 +327,17 @@ func validateDecrypters(decrypters []string, decryptersAllowList map[string]stru
 	decrypterNames := make(map[string]struct{}, 0)
 
 	for i, decrypter := range decrypters {
+		decrypter = strings.TrimSpace(decrypter)
+		if decrypter == "" {
+			errs = append(
+				errs,
+				field.Invalid(field.NewPath("spec", "decrypters", "["+strconv.Itoa(i)+"]"), decrypter, "decrypters cannot be empty if specified"),
+			)
+
+			continue
+		}
+
 		// Allow List: decrypters must match exactly and be in the allowed list to be able to decrypt.
-		// This means an allow list item should have the format "actor_{name}" and not just "{name}".
 		if len(decryptersAllowList) > 0 {
 			if _, exists := decryptersAllowList[decrypter]; !exists {
 				errs = append(
@@ -341,17 +351,19 @@ func validateDecrypters(decrypters []string, decryptersAllowList map[string]stru
 			continue
 		}
 
-		actor, name, found := strings.Cut(strings.TrimSpace(decrypter), "_")
-		if !found || actor != "actor" || name == "" {
-			errs = append(
-				errs,
-				field.Invalid(field.NewPath("spec", "decrypters", "["+strconv.Itoa(i)+"]"), decrypter, "a decrypter must have the format `actor_{name}`"),
-			)
+		// Use the same validation as labels for the decrypters.
+		if verrs := validation.IsValidLabelValue(decrypter); len(verrs) > 0 {
+			for _, verr := range verrs {
+				errs = append(
+					errs,
+					field.Invalid(field.NewPath("spec", "decrypters", "["+strconv.Itoa(i)+"]"), decrypter, verr),
+				)
+			}
 
 			continue
 		}
 
-		if _, exists := decrypterNames[name]; exists {
+		if _, exists := decrypterNames[decrypter]; exists {
 			errs = append(
 				errs,
 				field.Invalid(field.NewPath("spec", "decrypters", "["+strconv.Itoa(i)+"]"), decrypter, "decrypters must be unique"),
@@ -360,7 +372,7 @@ func validateDecrypters(decrypters []string, decryptersAllowList map[string]stru
 			continue
 		}
 
-		decrypterNames[name] = struct{}{}
+		decrypterNames[decrypter] = struct{}{}
 	}
 
 	return errs

pkg/registry/apis/secret/reststorage/secure_value_rest_test.go

@@ -22,7 +22,7 @@ func TestValidateSecureValue(t *testing.T) {
 				Description: "description",
 				Value:       "value",
 				Keeper:      &keeper,
-				Decrypters:  []string{"actor_app1", "actor_app2"},
+				Decrypters:  []string{"app1", "app2"},
 			},
 		}
 
@@ -187,8 +187,8 @@ func TestValidateSecureValue(t *testing.T) {
 				Description: "description", Ref: &ref,
 
 				Decrypters: []string{
-					"actor_app1",
-					"actor_app1",
+					"app1",
+					"app1",
 				},
 			},
 		}
@@ -198,33 +198,8 @@ func TestValidateSecureValue(t *testing.T) {
 		require.Equal(t, "spec.decrypters.[1]", errs[0].Field)
 	})
 
-	t.Run("`decrypters` must match the expected format", func(t *testing.T) {
-		ref := "ref"
-		sv := &secretv0alpha1.SecureValue{
-			Spec: secretv0alpha1.SecureValueSpec{
-				Description: "description", Ref: &ref,
-
-				Decrypters: []string{
-					"app1",
-					"_app1",
-					"actr_app1",
-					"actor_ ",
-					"actor_",
-				},
-			},
-		}
-
-		errs := ValidateSecureValue(sv, nil, admission.Create, nil)
-		require.Len(t, errs, len(sv.Spec.Decrypters))
-
-		for i, err := range errs {
-			require.Equal(t, fmt.Sprintf("spec.decrypters.[%d]", i), err.Field)
-			require.Contains(t, err.Error(), "a decrypter must have the format `actor_{name}`")
-		}
-	})
-
 	t.Run("when set, the `decrypters` must be one of the allowed in the allow list", func(t *testing.T) {
-		allowList := map[string]struct{}{"actor_app1": {}, "actor_app2": {}}
+		allowList := map[string]struct{}{"app1": {}, "app2": {}}
 		decrypters := slices.Collect(maps.Keys(allowList))
 
 		t.Run("no matches, returns an error", func(t *testing.T) {
@@ -233,7 +208,7 @@ func TestValidateSecureValue(t *testing.T) {
 				Spec: secretv0alpha1.SecureValueSpec{
 					Description: "description", Ref: &ref,
 
-					Decrypters: []string{"actor_app3"},
+					Decrypters: []string{"app3"},
 				},
 			}
 
@@ -284,10 +259,37 @@ func TestValidateSecureValue(t *testing.T) {
 		})
 	})
 
+	t.Run("`decrypters` must be a valid label value", func(t *testing.T) {
+		decrypters := []string{
+			"",              // invalid
+			"is/this/valid", // invalid
+			"is this valid", // invalid
+			"is.this.valid",
+			"is-this-valid",
+			"is_this_valid",
+			"0isthisvalid9",
+			"isthisvalid9",
+			"0isthisvalid",
+			"isthisvalid",
+		}
+
+		ref := "ref"
+		sv := &secretv0alpha1.SecureValue{
+			Spec: secretv0alpha1.SecureValueSpec{
+				Description: "description", Ref: &ref,
+
+				Decrypters: decrypters,
+			},
+		}
+
+		errs := ValidateSecureValue(sv, nil, admission.Create, nil)
+		require.Len(t, errs, 3)
+	})
+
 	t.Run("`decrypters` cannot have more than 64 items", func(t *testing.T) {
 		decrypters := make([]string, 0, 64+1)
 		for i := 0; i < 64+1; i++ {
-			decrypters = append(decrypters, fmt.Sprintf("actor_app%d", i))
+			decrypters = append(decrypters, fmt.Sprintf("app%d", i))
 		}
 
 		ref := "ref"

pkg/tests/apis/secret/testdata/secure-value-default-generate.yaml

@@ -11,5 +11,5 @@ spec:
   description: This is a secret
   value: this is super duper secure
   decrypters:
-    - actor_k6
-    - actor_synthetic-monitoring
+    - k6
+    - synthetic-monitoring

pkg/tests/apis/secret/testdata/secure-value-generate.yaml

@@ -12,5 +12,5 @@ spec:
   keeper: my-keeper-1
   value: super duper secure
   decrypters:
-    - actor_k6
-    - actor_synthetic-monitoring
+    - k6
+    - synthetic-monitoring