diff --git a/pkg/apis/core/fuzzer/fuzzer.go b/pkg/apis/core/fuzzer/fuzzer.go index 60ee22721a1..b2578df2983 100644 --- a/pkg/apis/core/fuzzer/fuzzer.go +++ b/pkg/apis/core/fuzzer/fuzzer.go @@ -81,6 +81,10 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} { } s.TerminationGracePeriodSeconds = &ttl + // DeprecatedServiceAccount is an alias for ServiceAccountName kept + // in sync by defaulting; valid objects always have them equal. + s.DeprecatedServiceAccount = s.ServiceAccountName //nolint:staticcheck // SA1019 DeprecatedServiceAccount must be fuzzed for backward compatibility + c.Fill(s.SecurityContext) if s.SecurityContext == nil { diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index fc8588b73be..a3c3724e6b7 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -3745,6 +3745,11 @@ type PodSpec struct { // ServiceAccountName is the name of the ServiceAccount to use to run this pod // The pod will be allowed to use secrets referenced by the ServiceAccount ServiceAccountName string + // DeprecatedServiceAccount is a deprecated alias for ServiceAccountName. + // + // Deprecated: Use serviceAccountName instead. + // +optional + DeprecatedServiceAccount string // AutomountServiceAccountToken indicates whether a service account token should be automatically mounted. // +optional AutomountServiceAccountToken *bool diff --git a/pkg/apis/core/v1/conversion.go b/pkg/apis/core/v1/conversion.go index e3b9adbca54..0736aa86571 100644 --- a/pkg/apis/core/v1/conversion.go +++ b/pkg/apis/core/v1/conversion.go @@ -293,19 +293,6 @@ func Convert_core_PodStatus_To_v1_PodStatus(in *core.PodStatus, out *v1.PodStatu return nil } -// The following two v1.PodSpec conversions are done here to support v1.ServiceAccount -// as an alias for ServiceAccountName. -func Convert_core_PodSpec_To_v1_PodSpec(in *core.PodSpec, out *v1.PodSpec, s conversion.Scope) error { - if err := autoConvert_core_PodSpec_To_v1_PodSpec(in, out, s); err != nil { - return err - } - - // DeprecatedServiceAccount is an alias for ServiceAccountName. - out.DeprecatedServiceAccount = in.ServiceAccountName - - return nil -} - func Convert_core_NodeSpec_To_v1_NodeSpec(in *core.NodeSpec, out *v1.NodeSpec, s conversion.Scope) error { if err := autoConvert_core_NodeSpec_To_v1_NodeSpec(in, out, s); err != nil { return err @@ -336,20 +323,6 @@ func Convert_v1_NodeSpec_To_core_NodeSpec(in *v1.NodeSpec, out *core.NodeSpec, s return nil } -func Convert_v1_PodSpec_To_core_PodSpec(in *v1.PodSpec, out *core.PodSpec, s conversion.Scope) error { - if err := autoConvert_v1_PodSpec_To_core_PodSpec(in, out, s); err != nil { - return err - } - - // We support DeprecatedServiceAccount as an alias for ServiceAccountName. - // If both are specified, ServiceAccountName (the new field) wins. - if in.ServiceAccountName == "" { - out.ServiceAccountName = in.DeprecatedServiceAccount - } - - return nil -} - func Convert_v1_Pod_To_core_Pod(in *v1.Pod, out *core.Pod, s conversion.Scope) error { if err := autoConvert_v1_Pod_To_core_Pod(in, out, s); err != nil { return err diff --git a/pkg/apis/core/v1/conversion_test.go b/pkg/apis/core/v1/conversion_test.go index f8a2326c880..701606526b2 100644 --- a/pkg/apis/core/v1/conversion_test.go +++ b/pkg/apis/core/v1/conversion_test.go @@ -138,15 +138,15 @@ func TestPodLogOptions(t *testing.T) { } } -// TestPodSpecConversion tests that v1.ServiceAccount is an alias for -// ServiceAccountName. +// TestPodSpecConversion tests that ServiceAccountName and its deprecated alias +// are preserved verbatim by conversion in both directions (keeping them in +// sync is handled by defaulting and the pod registry strategy, not conversion). func TestPodSpecConversion(t *testing.T) { name, other := "foo", "bar" - // Test internal -> v1. Should have both alias (DeprecatedServiceAccount) - // and new field (ServiceAccountName). i := &core.PodSpec{ - ServiceAccountName: name, + ServiceAccountName: name, + DeprecatedServiceAccount: other, } v := v1.PodSpec{} if err := legacyscheme.Scheme.Convert(i, &v, nil); err != nil { @@ -155,32 +155,19 @@ func TestPodSpecConversion(t *testing.T) { if v.ServiceAccountName != name { t.Fatalf("want v1.ServiceAccountName %q, got %q", name, v.ServiceAccountName) } - if v.DeprecatedServiceAccount != name { - t.Fatalf("want v1.DeprecatedServiceAccount %q, got %q", name, v.DeprecatedServiceAccount) + if v.DeprecatedServiceAccount != other { + t.Fatalf("want v1.DeprecatedServiceAccount %q, got %q", other, v.DeprecatedServiceAccount) } - // Test v1 -> internal. Either DeprecatedServiceAccount, ServiceAccountName, - // or both should translate to ServiceAccountName. ServiceAccountName wins - // if both are set. - testCases := []*v1.PodSpec{ - // New - {ServiceAccountName: name}, - // Alias - {DeprecatedServiceAccount: name}, - // Both: same - {ServiceAccountName: name, DeprecatedServiceAccount: name}, - // Both: different - {ServiceAccountName: name, DeprecatedServiceAccount: other}, + got := core.PodSpec{} + if err := legacyscheme.Scheme.Convert(&v, &got, nil); err != nil { + t.Fatalf("unexpected error: %v", err) } - for k, v := range testCases { - got := core.PodSpec{} - err := legacyscheme.Scheme.Convert(v, &got, nil) - if err != nil { - t.Fatalf("unexpected error for case %d: %v", k, err) - } - if got.ServiceAccountName != name { - t.Fatalf("want core.ServiceAccountName %q, got %q", name, got.ServiceAccountName) - } + if got.ServiceAccountName != name { + t.Fatalf("want core.ServiceAccountName %q, got %q", name, got.ServiceAccountName) + } + if got.DeprecatedServiceAccount != other { //nolint:staticcheck // SA1019 DeprecatedServiceAccount must be tested for backward compatibility + t.Fatalf("want core.DeprecatedServiceAccount %q, got %q", other, got.DeprecatedServiceAccount) //nolint:staticcheck // SA1019 DeprecatedServiceAccount must be tested for backward compatibility } } diff --git a/pkg/apis/core/v1/defaults.go b/pkg/apis/core/v1/defaults.go index 6ec370411e7..dede7cd5811 100644 --- a/pkg/apis/core/v1/defaults.go +++ b/pkg/apis/core/v1/defaults.go @@ -213,6 +213,15 @@ func SetDefaults_PodSpec(obj *v1.PodSpec) { // https://github.com/kubernetes/kubernetes/issues/69445 // In most cases the new defaulted field can added to SetDefaults_Pod instead of here, so // that it only materializes in the Pod object and not all objects with a PodSpec field. + + // DeprecatedServiceAccount is an alias for ServiceAccountName; keep the two + // in sync, with ServiceAccountName winning when both are set. This was + // historically applied during conversion and produces identical + // serializations, so it is exempt from the new-field caution above. + if len(obj.ServiceAccountName) == 0 { + obj.ServiceAccountName = obj.DeprecatedServiceAccount + } + obj.DeprecatedServiceAccount = obj.ServiceAccountName if obj.DNSPolicy == "" { obj.DNSPolicy = v1.DNSClusterFirst } diff --git a/pkg/apis/core/v1/defaults_test.go b/pkg/apis/core/v1/defaults_test.go index 57187106a47..8d86999f212 100644 --- a/pkg/apis/core/v1/defaults_test.go +++ b/pkg/apis/core/v1/defaults_test.go @@ -1777,6 +1777,39 @@ func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { return obj3 } +func TestSetDefaultServiceAccountAlias(t *testing.T) { + tests := []struct { + name string + serviceAccountName string + deprecatedServiceAccount string + expected string + }{ + {name: "neither set", serviceAccountName: "", deprecatedServiceAccount: "", expected: ""}, + {name: "both set to the same value", serviceAccountName: "a", deprecatedServiceAccount: "a", expected: "a"}, + {name: "both set, serviceAccountName wins", serviceAccountName: "a", deprecatedServiceAccount: "b", expected: "a"}, + {name: "only serviceAccountName set", serviceAccountName: "a", deprecatedServiceAccount: "", expected: "a"}, + {name: "only deprecated alias set", serviceAccountName: "", deprecatedServiceAccount: "a", expected: "a"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + pod := &v1.Pod{ + Spec: v1.PodSpec{ + ServiceAccountName: tc.serviceAccountName, + DeprecatedServiceAccount: tc.deprecatedServiceAccount, + }, + } + obj2 := roundTrip(t, runtime.Object(pod)) + pod2 := obj2.(*v1.Pod) + if pod2.Spec.ServiceAccountName != tc.expected { + t.Errorf("expected serviceAccountName %q, got %q", tc.expected, pod2.Spec.ServiceAccountName) + } + if pod2.Spec.DeprecatedServiceAccount != tc.expected { + t.Errorf("expected deprecatedServiceAccount %q, got %q", tc.expected, pod2.Spec.DeprecatedServiceAccount) + } + }) + } +} + func TestSetDefaultReplicationController(t *testing.T) { tests := []struct { rc *v1.ReplicationController diff --git a/pkg/apis/core/v1/zz_generated.conversion.go b/pkg/apis/core/v1/zz_generated.conversion.go index 8b5dfcad05e..ba4de19e119 100644 --- a/pkg/apis/core/v1/zz_generated.conversion.go +++ b/pkg/apis/core/v1/zz_generated.conversion.go @@ -1612,6 +1612,16 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddGeneratedConversionFunc((*corev1.PodSpec)(nil), (*core.PodSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1_PodSpec_To_core_PodSpec(a.(*corev1.PodSpec), b.(*core.PodSpec), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*core.PodSpec)(nil), (*corev1.PodSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_core_PodSpec_To_v1_PodSpec(a.(*core.PodSpec), b.(*corev1.PodSpec), scope) + }); err != nil { + return err + } if err := s.AddGeneratedConversionFunc((*corev1.PodTemplate)(nil), (*core.PodTemplate)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1_PodTemplate_To_core_PodTemplate(a.(*corev1.PodTemplate), b.(*core.PodTemplate), scope) }); err != nil { @@ -2432,11 +2442,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddConversionFunc((*core.PodSpec)(nil), (*corev1.PodSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_core_PodSpec_To_v1_PodSpec(a.(*core.PodSpec), b.(*corev1.PodSpec), scope) - }); err != nil { - return err - } if err := s.AddConversionFunc((*core.PodStatus)(nil), (*corev1.PodStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_core_PodStatus_To_v1_PodStatus(a.(*core.PodStatus), b.(*corev1.PodStatus), scope) }); err != nil { @@ -2467,11 +2472,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddConversionFunc((*corev1.PodSpec)(nil), (*core.PodSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1_PodSpec_To_core_PodSpec(a.(*corev1.PodSpec), b.(*core.PodSpec), scope) - }); err != nil { - return err - } if err := s.AddConversionFunc((*corev1.PodStatus)(nil), (*core.PodStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1_PodStatus_To_core_PodStatus(a.(*corev1.PodStatus), b.(*core.PodStatus), scope) }); err != nil { @@ -7046,7 +7046,7 @@ func autoConvert_v1_PodSpec_To_core_PodSpec(in *corev1.PodSpec, out *core.PodSpe out.DNSPolicy = core.DNSPolicy(in.DNSPolicy) out.NodeSelector = *(*map[string]string)(unsafe.Pointer(&in.NodeSelector)) out.ServiceAccountName = in.ServiceAccountName - // INFO: in.DeprecatedServiceAccount opted out of conversion generation + out.DeprecatedServiceAccount = in.DeprecatedServiceAccount out.AutomountServiceAccountToken = (*bool)(unsafe.Pointer(in.AutomountServiceAccountToken)) out.NodeName = in.NodeName out.HostNetwork = in.HostNetwork @@ -7081,6 +7081,11 @@ func autoConvert_v1_PodSpec_To_core_PodSpec(in *corev1.PodSpec, out *core.PodSpe return nil } +// Convert_v1_PodSpec_To_core_PodSpec is an autogenerated conversion function. +func Convert_v1_PodSpec_To_core_PodSpec(in *corev1.PodSpec, out *core.PodSpec, s conversion.Scope) error { + return autoConvert_v1_PodSpec_To_core_PodSpec(in, out, s) +} + func autoConvert_core_PodSpec_To_v1_PodSpec(in *core.PodSpec, out *corev1.PodSpec, s conversion.Scope) error { out.Volumes = *(*[]corev1.Volume)(unsafe.Pointer(&in.Volumes)) out.InitContainers = *(*[]corev1.Container)(unsafe.Pointer(&in.InitContainers)) @@ -7092,6 +7097,7 @@ func autoConvert_core_PodSpec_To_v1_PodSpec(in *core.PodSpec, out *corev1.PodSpe out.DNSPolicy = corev1.DNSPolicy(in.DNSPolicy) out.NodeSelector = *(*map[string]string)(unsafe.Pointer(&in.NodeSelector)) out.ServiceAccountName = in.ServiceAccountName + out.DeprecatedServiceAccount = in.DeprecatedServiceAccount out.AutomountServiceAccountToken = (*bool)(unsafe.Pointer(in.AutomountServiceAccountToken)) out.NodeName = in.NodeName out.HostNetwork = in.HostNetwork @@ -7126,6 +7132,11 @@ func autoConvert_core_PodSpec_To_v1_PodSpec(in *core.PodSpec, out *corev1.PodSpe return nil } +// Convert_core_PodSpec_To_v1_PodSpec is an autogenerated conversion function. +func Convert_core_PodSpec_To_v1_PodSpec(in *core.PodSpec, out *corev1.PodSpec, s conversion.Scope) error { + return autoConvert_core_PodSpec_To_v1_PodSpec(in, out, s) +} + func autoConvert_v1_PodStatus_To_core_PodStatus(in *corev1.PodStatus, out *core.PodStatus, s conversion.Scope) error { out.ObservedGeneration = in.ObservedGeneration out.Phase = core.PodPhase(in.Phase) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 1ea14c10119..658918e7c7c 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -23555,6 +23555,7 @@ func TestValidateOSFields(t *testing.T) { "RuntimeClassName", "SchedulerName", "SchedulingGates[*].Name", + "DeprecatedServiceAccount", "SecurityContext.RunAsNonRoot", "ServiceAccountName", "SetHostnameAsFQDN", diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go index d3828c1276e..328eccd2f09 100644 --- a/plugin/pkg/admission/serviceaccount/admission.go +++ b/plugin/pkg/admission/serviceaccount/admission.go @@ -152,9 +152,11 @@ func (s *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission. return s.Validate(ctx, a, o) } - // Set the default service account if needed + // Set the default service account if needed, keeping the deprecated alias + // in sync (see SetDefaults_PodSpec) so the result is defaulting-stable. if len(pod.Spec.ServiceAccountName) == 0 { pod.Spec.ServiceAccountName = DefaultServiceAccountName + pod.Spec.DeprecatedServiceAccount = DefaultServiceAccountName //nolint:staticcheck // SA1019 DeprecatedServiceAccount must be set for backward compatibility } serviceAccount, err := s.getServiceAccount(a.GetNamespace(), pod.Spec.ServiceAccountName) diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index 984d38ed3de..7f7fe18f5fe 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -4523,7 +4523,6 @@ message PodSpec { // DeprecatedServiceAccount is a deprecated alias for ServiceAccountName. // Deprecated: Use serviceAccountName instead. - // +k8s:conversion-gen=false // +optional optional string serviceAccount = 9; diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 2e30ee22172..3fa9bbfd9c9 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -4226,7 +4226,6 @@ type PodSpec struct { ServiceAccountName string `json:"serviceAccountName,omitempty" protobuf:"bytes,8,opt,name=serviceAccountName"` // DeprecatedServiceAccount is a deprecated alias for ServiceAccountName. // Deprecated: Use serviceAccountName instead. - // +k8s:conversion-gen=false // +optional DeprecatedServiceAccount string `json:"serviceAccount,omitempty" protobuf:"bytes,9,opt,name=serviceAccount"` // AutomountServiceAccountToken indicates whether a service account token should be automatically mounted.