From 2a0a95c811abf2f71b67eebe50f900b776dff0f2 Mon Sep 17 00:00:00 2001 From: Jaegoo Date: Wed, 10 Jun 2026 09:29:45 +0900 Subject: [PATCH] Migrate secret type immutable (#136886) * Wire up Secret for declarative validation * Migrate Secret.Type to declarative immutable validation * Add +k8s:optional tag to Secret.Type field * Add TestDeclarativeValidate test for CREATE flow * Add immutability test cases for unset->set and set->unset * Fix ValidateDeclarativelyWithMigrationChecks call to include DeclarativeValidationConfig * Fix double declarative validation by removing manual ValidateDeclarativelyWithMigrationChecks calls The secret strategy embedded rest.DeclarativeValidation (which implements DeclarativeValidationStrategy) but also called ValidateDeclarativelyWithMigrationChecks directly inside Validate and ValidateUpdate. The REST handler and test framework call ValidateDeclaratively separately after Validate/ValidateUpdate, causing double execution that broke the AllDeclarativeEnforced test scenario. Fix by returning only handwritten errors from Validate/ValidateUpdate, matching the pattern used by csiDriverStrategy and other correctly-migrated strategies. * Use alpha stability level for +k8s:immutable on Secret.Type The validation-gen tool enforces that Beta-level tags cannot be used in Stable validation. Change +k8s:immutable to +k8s:alpha(since: "1.36")=+k8s:immutable to match other stable-API fields. Regenerate zz_generated.validations.go and update test expected errors with .MarkAlpha() accordingly. * Update alpha stability level version from 1.36 to 1.37 Update +k8s:alpha(since: "1.36") annotations to 1.37 in types.go and generated.proto for Secret.Type immutability and ReplicationController declarative validation tags. * Regenerate zz_generated.validations.go after rebase Rebase onto latest master brought in validation-gen changes that add .MarkShortCircuit() to immutable and optional field validations. * Add generated declarative validation test files for Secret validation-gen generates test/declarative_validation/core/secret/ as part of Secret declarative validation wiring. * Add declarative validation coverage test for Secret.type immutability The coverage checker requires all registered validation rules to be exercised by tests. Add a test that triggers the immutable validation error for Secret.type to satisfy coverage for the generated rule: v1, Kind=Secret: type FieldValueInvalid origin="immutable" * Move Secret declarative validation tests to test/declarative_validation Move all test cases from pkg/registry/core/secret/declarative_validation_test.go to test/declarative_validation/core/secret/declarative_validation_test.go per #138872, and remove the original file. * Revert ReplicationController alpha tags from 1.37 back to 1.36 The since: "1.36" tags on ReplicationController fields track when those tags were originally added (v1.36) and should not have been changed. Only the newly added Secret.Type immutable tag targets 1.37. --- pkg/apis/core/v1/zz_generated.validations.go | 62 ++++++++++ pkg/apis/core/validation/validation.go | 2 +- pkg/registry/core/secret/strategy.go | 7 +- .../src/k8s.io/api/core/v1/generated.proto | 2 + staging/src/k8s.io/api/core/v1/types.go | 2 + .../secret/declarative_validation_test.go | 116 ++++++++++++++++++ .../zz_generated.validations.main_test.go | 43 +++++++ .../zz_generated.validations.v1_test.go | 38 ++++++ 8 files changed, 269 insertions(+), 3 deletions(-) create mode 100644 test/declarative_validation/core/secret/declarative_validation_test.go create mode 100644 test/declarative_validation/core/secret/zz_generated.validations.main_test.go create mode 100644 test/declarative_validation/core/secret/zz_generated.validations.v1_test.go diff --git a/pkg/apis/core/v1/zz_generated.validations.go b/pkg/apis/core/v1/zz_generated.validations.go index 25138ca01f0..90baa118c85 100644 --- a/pkg/apis/core/v1/zz_generated.validations.go +++ b/pkg/apis/core/v1/zz_generated.validations.go @@ -55,6 +55,21 @@ func RegisterValidations(scheme *runtime.Scheme) error { field.InternalError(nil, fmt.Errorf("no validation found for %T, subresource: %v", obj, op.Request.SubresourcePath())), } }) + // type Secret + scheme.AddValidationFunc( + (*corev1.Secret)(nil), + func(ctx context.Context, op operation.Operation, obj, oldObj interface{}) field.ErrorList { + switch op.Request.SubresourcePath() { + case "/": + return Validate_Secret( + ctx, op, nil, /* fldPath */ + obj.(*corev1.Secret), + safe.Cast[*corev1.Secret](oldObj)) + } + return field.ErrorList{ + field.InternalError(nil, fmt.Errorf("no validation found for %T, subresource: %v", obj, op.Request.SubresourcePath())), + } + }) return nil } @@ -195,3 +210,50 @@ func Validate_ReplicationControllerSpec( // field corev1.ReplicationControllerSpec.Template has no validation return errs } + +// Validate_Secret validates an instance of Secret according +// to declarative validation rules in the API schema. +func Validate_Secret( + ctx context.Context, op operation.Operation, fldPath *field.Path, + obj, oldObj *corev1.Secret) (errs field.ErrorList) { + + // field corev1.Secret.TypeMeta has no validation + // field corev1.Secret.ObjectMeta has no validation + // field corev1.Secret.Immutable has no validation + // field corev1.Secret.Data has no validation + // field corev1.Secret.StringData has no validation + + { // field corev1.Secret.Type + fn := func( + fldPath *field.Path, + obj, oldObj *corev1.SecretType, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj).MarkAlpha().MarkShortCircuit(); len(e) != 0 { + errs = append(errs, e...) + earlyReturn = true + } + if e := validate.OptionalValue(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + return + } + oldVal := safe.Field(oldObj, + func(oldObj *corev1.Secret) *corev1.SecretType { + return &oldObj.Type + }) + errs = append(errs, fn(fldPath.Child("type"), &obj.Type, oldVal, oldObj != nil)...) + } + + return errs +} diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 955bf8bbd32..4ce2f2b2b42 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -7834,7 +7834,7 @@ func ValidateSecret(secret *core.Secret) field.ErrorList { func ValidateSecretUpdate(newSecret, oldSecret *core.Secret) field.ErrorList { allErrs := ValidateObjectMetaUpdate(&newSecret.ObjectMeta, &oldSecret.ObjectMeta, field.NewPath("metadata")) - allErrs = append(allErrs, ValidateImmutableField(newSecret.Type, oldSecret.Type, field.NewPath("type"))...) + allErrs = append(allErrs, ValidateImmutableField(newSecret.Type, oldSecret.Type, field.NewPath("type")).WithOrigin("immutable").MarkCoveredByDeclarative()...) if oldSecret.Immutable != nil && *oldSecret.Immutable { if newSecret.Immutable == nil || !*newSecret.Immutable { allErrs = append(allErrs, field.Forbidden(field.NewPath("immutable"), "field is immutable when `immutable` is set")) diff --git a/pkg/registry/core/secret/strategy.go b/pkg/registry/core/secret/strategy.go index 2b02fc80bb5..e2ccae5b602 100644 --- a/pkg/registry/core/secret/strategy.go +++ b/pkg/registry/core/secret/strategy.go @@ -58,7 +58,8 @@ func (strategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { } func (strategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { - return validation.ValidateSecret(obj.(*api.Secret)) + newSecret := obj.(*api.Secret) + return validation.ValidateSecret(newSecret) } // WarningsOnCreate returns warnings for the creation of the given object. @@ -86,7 +87,9 @@ func (strategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { } func (strategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - return validation.ValidateSecretUpdate(obj.(*api.Secret), old.(*api.Secret)) + newSecret := obj.(*api.Secret) + oldSecret := old.(*api.Secret) + return validation.ValidateSecretUpdate(newSecret, oldSecret) } // WarningsOnUpdate returns warnings for the given update. diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index 8e57bb3d32e..4778154d173 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -5849,6 +5849,8 @@ message Secret { // Used to facilitate programmatic handling of secret data. // More info: https://kubernetes.io/docs/concepts/configuration/secret/#secret-types // +optional + // +k8s:optional + // +k8s:alpha(since: "1.37")=+k8s:immutable optional string type = 3; } diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 89828ca64da..a49e8562d25 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -7956,6 +7956,8 @@ type Secret struct { // Used to facilitate programmatic handling of secret data. // More info: https://kubernetes.io/docs/concepts/configuration/secret/#secret-types // +optional + // +k8s:optional + // +k8s:alpha(since: "1.37")=+k8s:immutable Type SecretType `json:"type,omitempty" protobuf:"bytes,3,opt,name=type,casttype=SecretType"` } diff --git a/test/declarative_validation/core/secret/declarative_validation_test.go b/test/declarative_validation/core/secret/declarative_validation_test.go new file mode 100644 index 00000000000..6eea68dc37f --- /dev/null +++ b/test/declarative_validation/core/secret/declarative_validation_test.go @@ -0,0 +1,116 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package secret + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + apitesting "k8s.io/kubernetes/pkg/api/testing" + api "k8s.io/kubernetes/pkg/apis/core" + registry "k8s.io/kubernetes/pkg/registry/core/secret" +) + +func TestDeclarativeValidate(t *testing.T) { + ctx := genericapirequest.WithRequestInfo(genericapirequest.NewDefaultContext(), &genericapirequest.RequestInfo{ + APIGroup: "", + APIVersion: "v1", + }) + testCases := map[string]struct { + input api.Secret + expectedErrs field.ErrorList + }{ + "valid create": { + input: mkValidSecret(), + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + apitesting.VerifyValidationEquivalence(t, ctx, &tc.input, registry.Strategy, tc.expectedErrs) + }) + } +} + +func TestDeclarativeValidateUpdate(t *testing.T) { + ctx := genericapirequest.WithRequestInfo(genericapirequest.NewDefaultContext(), &genericapirequest.RequestInfo{ + APIGroup: "", + APIVersion: "v1", + }) + testCases := map[string]struct { + old api.Secret + update api.Secret + expectedErrs field.ErrorList + }{ + "valid update": { + old: mkValidSecret(), + update: mkValidSecret(), + }, + "type: changed": { + old: mkValidSecret(), + update: mkValidSecret(tweakType(api.SecretType("custom-type"))), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("type"), api.SecretType("custom-type"), "field is immutable").WithOrigin("immutable").MarkAlpha(), + }, + }, + "type: set from unset": { + old: mkValidSecret(tweakType("")), + update: mkValidSecret(tweakType(api.SecretTypeOpaque)), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("type"), api.SecretTypeOpaque, "field is immutable").WithOrigin("immutable").MarkAlpha(), + }, + }, + "type: unset from set": { + old: mkValidSecret(), + update: mkValidSecret(tweakType("")), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("type"), api.SecretType(""), "field is immutable").WithOrigin("immutable").MarkAlpha(), + }, + }, + } + for k, tc := range testCases { + t.Run(k, func(t *testing.T) { + tc.old.ResourceVersion = "1" + tc.update.ResourceVersion = "1" + apitesting.VerifyUpdateValidationEquivalence(t, ctx, &tc.update, &tc.old, registry.Strategy, tc.expectedErrs) + }) + } +} + +func mkValidSecret(tweaks ...func(*api.Secret)) api.Secret { + s := api.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: metav1.NamespaceDefault, + }, + Type: api.SecretTypeOpaque, + Data: map[string][]byte{ + "key": []byte("value"), + }, + } + for _, tweak := range tweaks { + tweak(&s) + } + return s +} + +func tweakType(secretType api.SecretType) func(*api.Secret) { + return func(s *api.Secret) { + s.Type = secretType + } +} diff --git a/test/declarative_validation/core/secret/zz_generated.validations.main_test.go b/test/declarative_validation/core/secret/zz_generated.validations.main_test.go new file mode 100644 index 00000000000..a4f0890e50c --- /dev/null +++ b/test/declarative_validation/core/secret/zz_generated.validations.main_test.go @@ -0,0 +1,43 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by validation-gen. DO NOT EDIT. + +package secret + +import ( + fmt "fmt" + os "os" + testing "testing" + + coverage "k8s.io/apimachinery/pkg/test/coverage" +) + +var apiVersions = []string{"v1"} + +func TestMain(m *testing.M) { + code := m.Run() + if err := coverage.AssertDeclarativeCoverage(); err != nil { + fmt.Fprintln(os.Stderr, err) + if code == 0 { + code = 1 + } + } + os.Exit(code) +} diff --git a/test/declarative_validation/core/secret/zz_generated.validations.v1_test.go b/test/declarative_validation/core/secret/zz_generated.validations.v1_test.go new file mode 100644 index 00000000000..76de0fa92ca --- /dev/null +++ b/test/declarative_validation/core/secret/zz_generated.validations.v1_test.go @@ -0,0 +1,38 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by validation-gen. DO NOT EDIT. + +package secret + +import ( + schema "k8s.io/apimachinery/pkg/runtime/schema" + coverage "k8s.io/apimachinery/pkg/test/coverage" +) + +func init() { + coverage.RegisterDeclaredRules( + schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Secret"}, + coverage.FieldRules{ + "type": { + {ErrorType: "FieldValueInvalid", Origin: "immutable"}, + }, + }, + ) +}