SecureValues: make decrypters an optional field (#100731)

SecureValues: make decrypters an optional field

pkg/apis/secret/v0alpha1/secure_value.go

@@ -32,7 +32,8 @@ type SecureValueSpec struct {
 	// Name of the keeper, being the actual storage of the secure value.
 	Keeper string `json:"keeper,omitempty"`
 
-	// The Decrypters that are allowed to decrypt this secret
+	// The Decrypters that are allowed to decrypt this secret.
+	// An empty list means no service can decrypt it.
 	// Support and behavior is still TBD, but could likely look like:
 	// * testdata.grafana.app/{name1}
 	// * testdata.grafana.app/{name2}
@@ -41,6 +42,7 @@ type SecureValueSpec struct {
 	// [{ group:"testdata.grafana.app", name="name1"},
 	//  { group:"runner.k6.grafana.app"}]
 	// +listType=atomic
+	// +optional
 	Decrypters []string `json:"decrypters"`
 }
 

pkg/apis/secret/v0alpha1/zz_generated.openapi.go

@@ -681,7 +681,7 @@ func schema_pkg_apis_secret_v0alpha1_SecureValueSpec(ref common.ReferenceCallbac
 							},
 						},
 						SchemaProps: spec.SchemaProps{
-							Description: "The Decrypters that are allowed to decrypt this secret Support and behavior is still TBD, but could likely look like: * testdata.grafana.app/{name1} * testdata.grafana.app/{name2} * runner.k6.grafana.app/*  -- allow any k6 test runner Rather than a string pattern, we may want a more explicit object: [{ group:\"testdata.grafana.app\", name=\"name1\"},\n { group:\"runner.k6.grafana.app\"}]",
+							Description: "The Decrypters that are allowed to decrypt this secret. An empty list means no service can decrypt it. Support and behavior is still TBD, but could likely look like: * testdata.grafana.app/{name1} * testdata.grafana.app/{name2} * runner.k6.grafana.app/*  -- allow any k6 test runner Rather than a string pattern, we may want a more explicit object: [{ group:\"testdata.grafana.app\", name=\"name1\"},\n { group:\"runner.k6.grafana.app\"}]",
 							Type:        []string{"array"},
 							Items: &spec.SchemaOrArray{
 								Schema: &spec.Schema{
@@ -695,7 +695,7 @@ func schema_pkg_apis_secret_v0alpha1_SecureValueSpec(ref common.ReferenceCallbac
 						},
 					},
 				},
-				Required: []string{"title", "decrypters"},
+				Required: []string{"title"},
 			},
 		},
 	}

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

@@ -207,10 +207,9 @@ func ValidateSecureValue(sv, oldSv *secretv0alpha1.SecureValue, operation admiss
 	}
 
 	// General validations.
-
 	decrypterGroups := make(map[string]map[string]int, 0)
 
-	// Decrypters must match "{group}/{name OR *}" and must be unique.
+	// If populated, `Decrypters` must match "{group}/{name OR *}" and must be unique.
 	for i, decrypter := range sv.Spec.Decrypters {
 		group, name, found := strings.Cut(decrypter, "/")
 		if !found {
@@ -305,10 +304,6 @@ func validateSecureValueCreate(sv *secretv0alpha1.SecureValue) field.ErrorList {
 		errs = append(errs, field.Required(field.NewPath("spec"), "only one of `value` or `ref` can be set"))
 	}
 
-	if len(sv.Spec.Decrypters) == 0 {
-		errs = append(errs, field.Required(field.NewPath("spec", "decrypters"), "`decrypters` is required"))
-	}
-
 	return errs
 }
 

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

@@ -54,15 +54,6 @@ func TestValidateSecureValue(t *testing.T) {
 			require.Len(t, errs, 1)
 			require.Equal(t, "spec", errs[0].Field)
 		})
-
-		t.Run("`decrypters` must be present", func(t *testing.T) {
-			sv := validSecureValue.DeepCopy()
-			sv.Spec.Decrypters = make([]string, 0)
-
-			errs := ValidateSecureValue(sv, nil, admission.Create)
-			require.Len(t, errs, 1)
-			require.Equal(t, "spec.decrypters", errs[0].Field)
-		})
 	})
 
 	t.Run("when updating a securevalue", func(t *testing.T) {

pkg/storage/secret/metadata/secure_value_model.go

@@ -29,7 +29,7 @@ type secureValueDB struct {
 	// Spec
 	Title      string  `xorm:"title"`
 	Keeper     string  `xorm:"keeper"`
-	Decrypters string  `xorm:"decrypters"`
+	Decrypters *string `xorm:"decrypters"`
 	Ref        *string `xorm:"ref"`
 	ExternalID string  `xorm:"external_id"`
 }
@@ -55,8 +55,8 @@ func (sv *secureValueDB) toKubernetes() (*secretv0alpha1.SecureValue, error) {
 	}
 
 	decrypters := make([]string, 0)
-	if sv.Decrypters != "" {
-		if err := json.Unmarshal([]byte(sv.Decrypters), &decrypters); err != nil {
+	if sv.Decrypters != nil && *sv.Decrypters != "" {
+		if err := json.Unmarshal([]byte(*sv.Decrypters), &decrypters); err != nil {
 			return nil, fmt.Errorf("failed to unmarshal decrypters: %w", err)
 		}
 	}
@@ -157,14 +157,15 @@ func toRow(sv *secretv0alpha1.SecureValue, externalID string) (*secureValueDB, e
 		labels = string(encodedLabels)
 	}
 
-	var decrypters string
+	var decrypters *string
 	if len(sv.Spec.Decrypters) > 0 {
 		encodedDecrypters, err := json.Marshal(sv.Spec.Decrypters)
 		if err != nil {
 			return nil, fmt.Errorf("failed to encode decrypters: %w", err)
 		}
 
-		decrypters = string(encodedDecrypters)
+		rawDecrypters := string(encodedDecrypters)
+		decrypters = &rawDecrypters
 	}
 
 	meta, err := utils.MetaAccessor(sv)

pkg/storage/secret/migrator/migrator.go

@@ -49,7 +49,7 @@ func initSecretStore(mg *migrator.Migrator) string {
 			// Spec
 			{Name: "title", Type: migrator.DB_Text, Nullable: false},
 			{Name: "keeper", Type: migrator.DB_Text, Nullable: false},
-			{Name: "decrypters", Type: migrator.DB_Text, Nullable: false},
+			{Name: "decrypters", Type: migrator.DB_Text, Nullable: true},
 			{Name: "ref", Type: migrator.DB_Text, Nullable: true}, // Reference to third-party storage secret path.
 			{Name: "external_id", Type: migrator.DB_Text, Nullable: false},
 		},