diff --git a/0012-Return-error-for-localhost-seccomp-type-with-no-loca.patch b/0012-Return-error-for-localhost-seccomp-type-with-no-loca.patch new file mode 100644 index 0000000000000000000000000000000000000000..6ef4d4d08ac74d862227d6003b53c34205cf1058 --- /dev/null +++ b/0012-Return-error-for-localhost-seccomp-type-with-no-loca.patch @@ -0,0 +1,516 @@ +From 73174f870735251e7d4240cdc36983d1bef7db5f Mon Sep 17 00:00:00 2001 +From: Craig Ingram +Date: Fri, 24 Feb 2023 15:24:49 -0500 +Subject: [PATCH] Return error for localhost seccomp type with no localhost + profile defined + +--- + pkg/kubelet/kuberuntime/helpers.go | 54 +++--- + pkg/kubelet/kuberuntime/helpers_test.go | 175 ++++-------------- + .../kuberuntime_container_linux.go | 16 +- + .../kuberuntime_container_linux_test.go | 7 +- + pkg/kubelet/kuberuntime/security_context.go | 15 +- + 5 files changed, 98 insertions(+), 169 deletions(-) + +diff --git a/pkg/kubelet/kuberuntime/helpers.go b/pkg/kubelet/kuberuntime/helpers.go +index 6db2698c..99cc6764 100644 +--- a/pkg/kubelet/kuberuntime/helpers.go ++++ b/pkg/kubelet/kuberuntime/helpers.go +@@ -202,21 +202,25 @@ func toKubeRuntimeStatus(status *runtimeapi.RuntimeStatus) *kubecontainer.Runtim + return &kubecontainer.RuntimeStatus{Conditions: conditions} + } + +-func fieldProfile(scmp *v1.SeccompProfile, profileRootPath string) string { ++func fieldProfile(scmp *v1.SeccompProfile, profileRootPath string) (string, error) { + if scmp == nil { +- return "" ++ return "", nil + } + if scmp.Type == v1.SeccompProfileTypeRuntimeDefault { +- return v1.SeccompProfileRuntimeDefault ++ return v1.SeccompProfileRuntimeDefault, nil + } +- if scmp.Type == v1.SeccompProfileTypeLocalhost && scmp.LocalhostProfile != nil && len(*scmp.LocalhostProfile) > 0 { +- fname := filepath.Join(profileRootPath, *scmp.LocalhostProfile) +- return v1.SeccompLocalhostProfileNamePrefix + fname ++ if scmp.Type == v1.SeccompProfileTypeLocalhost { ++ if scmp.LocalhostProfile != nil && len(*scmp.LocalhostProfile) > 0 { ++ fname := filepath.Join(profileRootPath, *scmp.LocalhostProfile) ++ return v1.SeccompLocalhostProfileNamePrefix + fname, nil ++ } else { ++ return "", fmt.Errorf("localhostProfile must be set if seccompProfile type is Localhost.") ++ } + } + if scmp.Type == v1.SeccompProfileTypeUnconfined { +- return v1.SeccompProfileNameUnconfined ++ return v1.SeccompProfileNameUnconfined, nil + } +- return "" ++ return "", nil + } + + func annotationProfile(profile, profileRootPath string) string { +@@ -229,7 +233,7 @@ func annotationProfile(profile, profileRootPath string) string { + } + + func (m *kubeGenericRuntimeManager) getSeccompProfilePath(annotations map[string]string, containerName string, +- podSecContext *v1.PodSecurityContext, containerSecContext *v1.SecurityContext) string { ++ podSecContext *v1.PodSecurityContext, containerSecContext *v1.SecurityContext) (string, error) { + // container fields are applied first + if containerSecContext != nil && containerSecContext.SeccompProfile != nil { + return fieldProfile(containerSecContext.SeccompProfile, m.seccompProfileRoot) +@@ -238,7 +242,7 @@ func (m *kubeGenericRuntimeManager) getSeccompProfilePath(annotations map[string + // if container field does not exist, try container annotation (deprecated) + if containerName != "" { + if profile, ok := annotations[v1.SeccompContainerAnnotationKeyPrefix+containerName]; ok { +- return annotationProfile(profile, m.seccompProfileRoot) ++ return annotationProfile(profile, m.seccompProfileRoot), nil + } + } + +@@ -249,39 +253,43 @@ func (m *kubeGenericRuntimeManager) getSeccompProfilePath(annotations map[string + + // as last resort, try to apply pod annotation (deprecated) + if profile, ok := annotations[v1.SeccompPodAnnotationKey]; ok { +- return annotationProfile(profile, m.seccompProfileRoot) ++ return annotationProfile(profile, m.seccompProfileRoot), nil + } + +- return "" ++ return "", nil + } + +-func fieldSeccompProfile(scmp *v1.SeccompProfile, profileRootPath string) *runtimeapi.SecurityProfile { ++func fieldSeccompProfile(scmp *v1.SeccompProfile, profileRootPath string) (*runtimeapi.SecurityProfile, error) { + // TODO: Move to RuntimeDefault as the default instead of Unconfined after discussion + // with sig-node. + if scmp == nil { + return &runtimeapi.SecurityProfile{ + ProfileType: runtimeapi.SecurityProfile_Unconfined, +- } ++ }, nil + } + if scmp.Type == v1.SeccompProfileTypeRuntimeDefault { + return &runtimeapi.SecurityProfile{ + ProfileType: runtimeapi.SecurityProfile_RuntimeDefault, +- } ++ }, nil + } +- if scmp.Type == v1.SeccompProfileTypeLocalhost && scmp.LocalhostProfile != nil && len(*scmp.LocalhostProfile) > 0 { +- fname := filepath.Join(profileRootPath, *scmp.LocalhostProfile) +- return &runtimeapi.SecurityProfile{ +- ProfileType: runtimeapi.SecurityProfile_Localhost, +- LocalhostRef: fname, ++ if scmp.Type == v1.SeccompProfileTypeLocalhost { ++ if scmp.LocalhostProfile != nil && len(*scmp.LocalhostProfile) > 0 { ++ fname := filepath.Join(profileRootPath, *scmp.LocalhostProfile) ++ return &runtimeapi.SecurityProfile{ ++ ProfileType: runtimeapi.SecurityProfile_Localhost, ++ LocalhostRef: fname, ++ }, nil ++ } else { ++ return nil, fmt.Errorf("localhostProfile must be set if seccompProfile type is Localhost.") + } + } + return &runtimeapi.SecurityProfile{ + ProfileType: runtimeapi.SecurityProfile_Unconfined, +- } ++ }, nil + } + + func (m *kubeGenericRuntimeManager) getSeccompProfile(annotations map[string]string, containerName string, +- podSecContext *v1.PodSecurityContext, containerSecContext *v1.SecurityContext) *runtimeapi.SecurityProfile { ++ podSecContext *v1.PodSecurityContext, containerSecContext *v1.SecurityContext) (*runtimeapi.SecurityProfile, error) { + // container fields are applied first + if containerSecContext != nil && containerSecContext.SeccompProfile != nil { + return fieldSeccompProfile(containerSecContext.SeccompProfile, m.seccompProfileRoot) +@@ -294,7 +302,7 @@ func (m *kubeGenericRuntimeManager) getSeccompProfile(annotations map[string]str + + return &runtimeapi.SecurityProfile{ + ProfileType: runtimeapi.SecurityProfile_Unconfined, +- } ++ }, nil + } + + func ipcNamespaceForPod(pod *v1.Pod) runtimeapi.NamespaceMode { +diff --git a/pkg/kubelet/kuberuntime/helpers_test.go b/pkg/kubelet/kuberuntime/helpers_test.go +index 1628cb3c..6d5ef720 100644 +--- a/pkg/kubelet/kuberuntime/helpers_test.go ++++ b/pkg/kubelet/kuberuntime/helpers_test.go +@@ -176,17 +176,18 @@ func TestFieldProfile(t *testing.T) { + scmpProfile *v1.SeccompProfile + rootPath string + expectedProfile string ++ expectedError string + }{ + { + description: "no seccompProfile should return empty", + expectedProfile: "", + }, + { +- description: "type localhost without profile should return empty", ++ description: "type localhost without profile should return error", + scmpProfile: &v1.SeccompProfile{ + Type: v1.SeccompProfileTypeLocalhost, + }, +- expectedProfile: "", ++ expectedError: "localhostProfile must be set if seccompProfile type is Localhost.", + }, + { + description: "unknown type should return empty", +@@ -213,7 +214,7 @@ func TestFieldProfile(t *testing.T) { + description: "SeccompProfileTypeLocalhost should return unconfined", + scmpProfile: &v1.SeccompProfile{ + Type: v1.SeccompProfileTypeLocalhost, +- LocalhostProfile: utilpointer.StringPtr("profile.json"), ++ LocalhostProfile: utilpointer.String("profile.json"), + }, + rootPath: "/test/", + expectedProfile: "localhost//test/profile.json", +@@ -221,8 +222,13 @@ func TestFieldProfile(t *testing.T) { + } + + for i, test := range tests { +- seccompProfile := fieldProfile(test.scmpProfile, test.rootPath) +- assert.Equal(t, test.expectedProfile, seccompProfile, "TestCase[%d]: %s", i, test.description) ++ seccompProfile, err := fieldProfile(test.scmpProfile, test.rootPath) ++ if test.expectedError != "" { ++ assert.EqualError(t, err, test.expectedError, "TestCase[%d]: %s", i, test.description) ++ } else { ++ assert.NoError(t, err, "TestCase[%d]: %s", i, test.description) ++ assert.Equal(t, test.expectedProfile, seccompProfile, "TestCase[%d]: %s", i, test.description) ++ } + } + } + +@@ -237,6 +243,7 @@ func TestGetSeccompProfilePath(t *testing.T) { + containerSc *v1.SecurityContext + containerName string + expectedProfile string ++ expectedError string + }{ + { + description: "no seccomp should return empty", +@@ -247,91 +254,6 @@ func TestGetSeccompProfilePath(t *testing.T) { + containerName: "container1", + expectedProfile: "", + }, +- { +- description: "annotations: pod runtime/default seccomp profile should return runtime/default", +- annotation: map[string]string{ +- v1.SeccompPodAnnotationKey: v1.SeccompProfileRuntimeDefault, +- }, +- expectedProfile: "runtime/default", +- }, +- { +- description: "annotations: pod docker/default seccomp profile should return docker/default", +- annotation: map[string]string{ +- v1.SeccompPodAnnotationKey: v1.DeprecatedSeccompProfileDockerDefault, +- }, +- expectedProfile: "docker/default", +- }, +- { +- description: "annotations: pod runtime/default seccomp profile with containerName should return runtime/default", +- annotation: map[string]string{ +- v1.SeccompPodAnnotationKey: v1.SeccompProfileRuntimeDefault, +- }, +- containerName: "container1", +- expectedProfile: "runtime/default", +- }, +- { +- description: "annotations: pod docker/default seccomp profile with containerName should return docker/default", +- annotation: map[string]string{ +- v1.SeccompPodAnnotationKey: v1.DeprecatedSeccompProfileDockerDefault, +- }, +- containerName: "container1", +- expectedProfile: "docker/default", +- }, +- { +- description: "annotations: pod unconfined seccomp profile should return unconfined", +- annotation: map[string]string{ +- v1.SeccompPodAnnotationKey: v1.SeccompProfileNameUnconfined, +- }, +- expectedProfile: "unconfined", +- }, +- { +- description: "annotations: pod unconfined seccomp profile with containerName should return unconfined", +- annotation: map[string]string{ +- v1.SeccompPodAnnotationKey: v1.SeccompProfileNameUnconfined, +- }, +- containerName: "container1", +- expectedProfile: "unconfined", +- }, +- { +- description: "annotations: pod localhost seccomp profile should return local profile path", +- annotation: map[string]string{ +- v1.SeccompPodAnnotationKey: "localhost/chmod.json", +- }, +- expectedProfile: "localhost/" + filepath.Join(fakeSeccompProfileRoot, "chmod.json"), +- }, +- { +- description: "annotations: pod localhost seccomp profile with containerName should return local profile path", +- annotation: map[string]string{ +- v1.SeccompPodAnnotationKey: "localhost/chmod.json", +- }, +- containerName: "container1", +- expectedProfile: "localhost/" + filepath.Join(fakeSeccompProfileRoot, "chmod.json"), +- }, +- { +- description: "annotations: container localhost seccomp profile with containerName should return local profile path", +- annotation: map[string]string{ +- v1.SeccompContainerAnnotationKeyPrefix + "container1": "localhost/chmod.json", +- }, +- containerName: "container1", +- expectedProfile: "localhost/" + filepath.Join(fakeSeccompProfileRoot, "chmod.json"), +- }, +- { +- description: "annotations: container localhost seccomp profile should override pod profile", +- annotation: map[string]string{ +- v1.SeccompPodAnnotationKey: v1.SeccompProfileNameUnconfined, +- v1.SeccompContainerAnnotationKeyPrefix + "container1": "localhost/chmod.json", +- }, +- containerName: "container1", +- expectedProfile: "localhost/" + filepath.Join(fakeSeccompProfileRoot, "chmod.json"), +- }, +- { +- description: "annotations: container localhost seccomp profile with unmatched containerName should return empty", +- annotation: map[string]string{ +- v1.SeccompContainerAnnotationKeyPrefix + "container1": "localhost/chmod.json", +- }, +- containerName: "container2", +- expectedProfile: "", +- }, + { + description: "pod seccomp profile set to unconfined returns unconfined", + podSc: &v1.PodSecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeUnconfined}}, +@@ -358,14 +280,14 @@ func TestGetSeccompProfilePath(t *testing.T) { + expectedProfile: "localhost/" + filepath.Join(fakeSeccompProfileRoot, "filename"), + }, + { +- description: "pod seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns empty", +- podSc: &v1.PodSecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}}, +- expectedProfile: "", ++ description: "pod seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns error", ++ podSc: &v1.PodSecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}}, ++ expectedError: "localhostProfile must be set if seccompProfile type is Localhost.", + }, + { +- description: "container seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns empty", +- containerSc: &v1.SecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}}, +- expectedProfile: "", ++ description: "container seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns error", ++ containerSc: &v1.SecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}}, ++ expectedError: "localhostProfile must be set if seccompProfile type is Localhost.", + }, + { + description: "container seccomp profile set to SeccompProfileTypeLocalhost returns 'localhost/' + LocalhostProfile", +@@ -378,41 +300,16 @@ func TestGetSeccompProfilePath(t *testing.T) { + containerSc: &v1.SecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeRuntimeDefault}}, + expectedProfile: "runtime/default", + }, +- { +- description: "prioritise container field over container annotation, pod field and pod annotation", +- podSc: &v1.PodSecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost, LocalhostProfile: getLocal("field-pod-profile.json")}}, +- containerSc: &v1.SecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost, LocalhostProfile: getLocal("field-cont-profile.json")}}, +- annotation: map[string]string{ +- v1.SeccompPodAnnotationKey: "localhost/annota-pod-profile.json", +- v1.SeccompContainerAnnotationKeyPrefix + "container1": "localhost/annota-cont-profile.json", +- }, +- containerName: "container1", +- expectedProfile: "localhost/" + filepath.Join(fakeSeccompProfileRoot, "field-cont-profile.json"), +- }, +- { +- description: "prioritise container annotation over pod field", +- podSc: &v1.PodSecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost, LocalhostProfile: getLocal("field-pod-profile.json")}}, +- annotation: map[string]string{ +- v1.SeccompPodAnnotationKey: "localhost/annota-pod-profile.json", +- v1.SeccompContainerAnnotationKeyPrefix + "container1": "localhost/annota-cont-profile.json", +- }, +- containerName: "container1", +- expectedProfile: "localhost/" + filepath.Join(fakeSeccompProfileRoot, "annota-cont-profile.json"), +- }, +- { +- description: "prioritise pod field over pod annotation", +- podSc: &v1.PodSecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost, LocalhostProfile: getLocal("field-pod-profile.json")}}, +- annotation: map[string]string{ +- v1.SeccompPodAnnotationKey: "localhost/annota-pod-profile.json", +- }, +- containerName: "container1", +- expectedProfile: "localhost/" + filepath.Join(fakeSeccompProfileRoot, "field-pod-profile.json"), +- }, + } + + for i, test := range tests { +- seccompProfile := m.getSeccompProfilePath(test.annotation, test.containerName, test.podSc, test.containerSc) +- assert.Equal(t, test.expectedProfile, seccompProfile, "TestCase[%d]: %s", i, test.description) ++ seccompProfile, err := m.getSeccompProfilePath(test.annotation, test.containerName, test.podSc, test.containerSc) ++ if test.expectedError != "" { ++ assert.EqualError(t, err, test.expectedError, "TestCase[%d]: %s", i, test.description) ++ } else { ++ assert.NoError(t, err, "TestCase[%d]: %s", i, test.description) ++ assert.Equal(t, test.expectedProfile, seccompProfile, "TestCase[%d]: %s", i, test.description) ++ } + } + } + +@@ -435,6 +332,7 @@ func TestGetSeccompProfile(t *testing.T) { + containerSc *v1.SecurityContext + containerName string + expectedProfile *runtimeapi.SecurityProfile ++ expectedError string + }{ + { + description: "no seccomp should return unconfined", +@@ -469,14 +367,14 @@ func TestGetSeccompProfile(t *testing.T) { + }, + }, + { +- description: "pod seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns unconfined", +- podSc: &v1.PodSecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}}, +- expectedProfile: unconfinedProfile, ++ description: "pod seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns error", ++ podSc: &v1.PodSecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}}, ++ expectedError: "localhostProfile must be set if seccompProfile type is Localhost.", + }, + { +- description: "container seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns unconfined", +- containerSc: &v1.SecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}}, +- expectedProfile: unconfinedProfile, ++ description: "container seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns error", ++ containerSc: &v1.SecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}}, ++ expectedError: "localhostProfile must be set if seccompProfile type is Localhost.", + }, + { + description: "container seccomp profile set to SeccompProfileTypeLocalhost returns 'localhost/' + LocalhostProfile", +@@ -505,8 +403,13 @@ func TestGetSeccompProfile(t *testing.T) { + } + + for i, test := range tests { +- seccompProfile := m.getSeccompProfile(test.annotation, test.containerName, test.podSc, test.containerSc) +- assert.Equal(t, test.expectedProfile, seccompProfile, "TestCase[%d]: %s", i, test.description) ++ seccompProfile, err := m.getSeccompProfile(test.annotation, test.containerName, test.podSc, test.containerSc) ++ if test.expectedError != "" { ++ assert.EqualError(t, err, test.expectedError, "TestCase[%d]: %s", i, test.description) ++ } else { ++ assert.NoError(t, err, "TestCase[%d]: %s", i, test.description) ++ assert.Equal(t, test.expectedProfile, seccompProfile, "TestCase[%d]: %s", i, test.description) ++ } + } + } + +diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go b/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go +index 0f4fab34..d7c22c86 100644 +--- a/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go ++++ b/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go +@@ -34,15 +34,23 @@ import ( + + // applyPlatformSpecificContainerConfig applies platform specific configurations to runtimeapi.ContainerConfig. + func (m *kubeGenericRuntimeManager) applyPlatformSpecificContainerConfig(config *runtimeapi.ContainerConfig, container *v1.Container, pod *v1.Pod, uid *int64, username string, nsTarget *kubecontainer.ContainerID) error { +- config.Linux = m.generateLinuxContainerConfig(container, pod, uid, username, nsTarget) ++ cl, err := m.generateLinuxContainerConfig(container, pod, uid, username, nsTarget) ++ if err != nil { ++ return err ++ } ++ config.Linux = cl + return nil + } + + // generateLinuxContainerConfig generates linux container config for kubelet runtime v1. +-func (m *kubeGenericRuntimeManager) generateLinuxContainerConfig(container *v1.Container, pod *v1.Pod, uid *int64, username string, nsTarget *kubecontainer.ContainerID) *runtimeapi.LinuxContainerConfig { ++func (m *kubeGenericRuntimeManager) generateLinuxContainerConfig(container *v1.Container, pod *v1.Pod, uid *int64, username string, nsTarget *kubecontainer.ContainerID) (*runtimeapi.LinuxContainerConfig, error) { ++ sc, err := m.determineEffectiveSecurityContext(pod, container, uid, username) ++ if err != nil { ++ return nil, err ++ } + lc := &runtimeapi.LinuxContainerConfig{ + Resources: &runtimeapi.LinuxContainerResources{}, +- SecurityContext: m.determineEffectiveSecurityContext(pod, container, uid, username), ++ SecurityContext: sc, + } + + if nsTarget != nil && lc.SecurityContext.NamespaceOptions.Pid == runtimeapi.NamespaceMode_CONTAINER { +@@ -89,7 +97,7 @@ func (m *kubeGenericRuntimeManager) generateLinuxContainerConfig(container *v1.C + + lc.Resources.HugepageLimits = GetHugepageLimitsFromResources(container.Resources) + +- return lc ++ return lc, nil + } + + // GetHugepageLimitsFromResources returns limits of each hugepages from resources. +diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go +index 98203fef..3f145938 100644 +--- a/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go ++++ b/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go +@@ -44,6 +44,8 @@ func makeExpectedConfig(m *kubeGenericRuntimeManager, pod *v1.Pod, containerInde + restartCountUint32 := uint32(restartCount) + envs := make([]*runtimeapi.KeyValue, len(opts.Envs)) + ++ l, _ := m.generateLinuxContainerConfig(container, pod, new(int64), "", nil) ++ + expectedConfig := &runtimeapi.ContainerConfig{ + Metadata: &runtimeapi.ContainerMetadata{ + Name: container.Name, +@@ -61,7 +63,7 @@ func makeExpectedConfig(m *kubeGenericRuntimeManager, pod *v1.Pod, containerInde + Stdin: container.Stdin, + StdinOnce: container.StdinOnce, + Tty: container.TTY, +- Linux: m.generateLinuxContainerConfig(container, pod, new(int64), "", nil), ++ Linux: l, + Envs: envs, + } + return expectedConfig +@@ -360,7 +362,8 @@ func TestGenerateLinuxContainerConfigNamespaces(t *testing.T) { + }, + } { + t.Run(tc.name, func(t *testing.T) { +- got := m.generateLinuxContainerConfig(&tc.pod.Spec.Containers[0], tc.pod, nil, "", tc.target) ++ got, err := m.generateLinuxContainerConfig(&tc.pod.Spec.Containers[0], tc.pod, nil, "", tc.target) ++ assert.NoError(t, err) + if diff := cmp.Diff(tc.want, got.SecurityContext.NamespaceOptions); diff != "" { + t.Errorf("%v: diff (-want +got):\n%v", t.Name(), diff) + } +diff --git a/pkg/kubelet/kuberuntime/security_context.go b/pkg/kubelet/kuberuntime/security_context.go +index 45f0d599..aeacbead 100644 +--- a/pkg/kubelet/kuberuntime/security_context.go ++++ b/pkg/kubelet/kuberuntime/security_context.go +@@ -24,7 +24,7 @@ import ( + ) + + // determineEffectiveSecurityContext gets container's security context from v1.Pod and v1.Container. +-func (m *kubeGenericRuntimeManager) determineEffectiveSecurityContext(pod *v1.Pod, container *v1.Container, uid *int64, username string) *runtimeapi.LinuxContainerSecurityContext { ++func (m *kubeGenericRuntimeManager) determineEffectiveSecurityContext(pod *v1.Pod, container *v1.Container, uid *int64, username string) (*runtimeapi.LinuxContainerSecurityContext, error) { + effectiveSc := securitycontext.DetermineEffectiveSecurityContext(pod, container) + synthesized := convertToRuntimeSecurityContext(effectiveSc) + if synthesized == nil { +@@ -36,9 +36,16 @@ func (m *kubeGenericRuntimeManager) determineEffectiveSecurityContext(pod *v1.Po + + // TODO: Deprecated, remove after we switch to Seccomp field + // set SeccompProfilePath. +- synthesized.SeccompProfilePath = m.getSeccompProfilePath(pod.Annotations, container.Name, pod.Spec.SecurityContext, container.SecurityContext) ++ var err error ++ synthesized.SeccompProfilePath, err = m.getSeccompProfilePath(pod.Annotations, container.Name, pod.Spec.SecurityContext, container.SecurityContext) ++ if err != nil { ++ return nil, err ++ } + +- synthesized.Seccomp = m.getSeccompProfile(pod.Annotations, container.Name, pod.Spec.SecurityContext, container.SecurityContext) ++ synthesized.Seccomp, err = m.getSeccompProfile(pod.Annotations, container.Name, pod.Spec.SecurityContext, container.SecurityContext) ++ if err != nil { ++ return nil, err ++ } + + // set ApparmorProfile. + synthesized.ApparmorProfile = apparmor.GetProfileNameFromPodAnnotations(pod.Annotations, container.Name) +@@ -74,7 +81,7 @@ func (m *kubeGenericRuntimeManager) determineEffectiveSecurityContext(pod *v1.Po + synthesized.MaskedPaths = securitycontext.ConvertToRuntimeMaskedPaths(effectiveSc.ProcMount) + synthesized.ReadonlyPaths = securitycontext.ConvertToRuntimeReadonlyPaths(effectiveSc.ProcMount) + +- return synthesized ++ return synthesized, nil + } + + // convertToRuntimeSecurityContext converts v1.SecurityContext to runtimeapi.SecurityContext. +-- +2.25.1 + diff --git a/0013-Validate-etcd-paths.patch b/0013-Validate-etcd-paths.patch new file mode 100644 index 0000000000000000000000000000000000000000..b050b1069a50816a11418e32cf38b81c48d50ab2 --- /dev/null +++ b/0013-Validate-etcd-paths.patch @@ -0,0 +1,1109 @@ +From 54269f2b186ea2a37bb4e9a22e797fdc6ebdcb37 Mon Sep 17 00:00:00 2001 +From: Tim Allclair +Date: Mon, 10 Oct 2022 18:15:22 -0700 +Subject: [PATCH] Validate etcd paths + +--- + .../apiserver/pkg/storage/etcd3/store.go | 146 +++++++---- + .../apiserver/pkg/storage/etcd3/store_test.go | 239 ++++++++++++------ + 2 files changed, 260 insertions(+), 125 deletions(-) + +diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go +index 0cff6b3f..ede07112 100644 +--- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go ++++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go +@@ -89,18 +89,23 @@ func New(c *clientv3.Client, codec runtime.Codec, newFunc func() runtime.Object, + + func newStore(c *clientv3.Client, newFunc func() runtime.Object, pagingEnabled bool, codec runtime.Codec, prefix string, transformer value.Transformer) *store { + versioner := APIObjectVersioner{} ++ // for compatibility with etcd2 impl. ++ // no-op for default prefix of '/registry'. ++ // keeps compatibility with etcd2 impl for custom prefixes that don't start with '/' ++ pathPrefix := path.Join("/", prefix) ++ if !strings.HasSuffix(pathPrefix, "/") { ++ // Ensure the pathPrefix ends in "/" here to simplify key concatenation later. ++ pathPrefix += "/" ++ } + result := &store{ + client: c, + codec: codec, + versioner: versioner, + transformer: transformer, + pagingEnabled: pagingEnabled, +- // for compatibility with etcd2 impl. +- // no-op for default prefix of '/registry'. +- // keeps compatibility with etcd2 impl for custom prefixes that don't start with '/' +- pathPrefix: path.Join("/", prefix), +- watcher: newWatcher(c, codec, newFunc, versioner, transformer), +- leaseManager: newDefaultLeaseManager(c), ++ pathPrefix: pathPrefix, ++ watcher: newWatcher(c, codec, newFunc, versioner, transformer), ++ leaseManager: newDefaultLeaseManager(c), + } + return result + } +@@ -112,9 +117,12 @@ func (s *store) Versioner() storage.Versioner { + + // Get implements storage.Interface.Get. + func (s *store) Get(ctx context.Context, key string, opts storage.GetOptions, out runtime.Object) error { +- key = path.Join(s.pathPrefix, key) ++ preparedKey, err := s.prepareKey(key) ++ if err != nil { ++ return err ++ } + startTime := time.Now() +- getResp, err := s.client.KV.Get(ctx, key) ++ getResp, err := s.client.KV.Get(ctx, preparedKey) + metrics.RecordEtcdRequestLatency("get", getTypeName(out), startTime) + if err != nil { + return err +@@ -127,11 +135,11 @@ func (s *store) Get(ctx context.Context, key string, opts storage.GetOptions, ou + if opts.IgnoreNotFound { + return runtime.SetZeroValue(out) + } +- return storage.NewKeyNotFoundError(key, 0) ++ return storage.NewKeyNotFoundError(preparedKey, 0) + } + kv := getResp.Kvs[0] + +- data, _, err := s.transformer.TransformFromStorage(kv.Value, authenticatedDataString(key)) ++ data, _, err := s.transformer.TransformFromStorage(kv.Value, authenticatedDataString(preparedKey)) + if err != nil { + return storage.NewInternalError(err.Error()) + } +@@ -141,6 +149,11 @@ func (s *store) Get(ctx context.Context, key string, opts storage.GetOptions, ou + + // Create implements storage.Interface.Create. + func (s *store) Create(ctx context.Context, key string, obj, out runtime.Object, ttl uint64) error { ++ preparedKey, err := s.prepareKey(key) ++ if err != nil { ++ return err ++ } ++ + if version, err := s.versioner.ObjectResourceVersion(obj); err == nil && version != 0 { + return errors.New("resourceVersion should not be set on objects to be created") + } +@@ -151,30 +164,29 @@ func (s *store) Create(ctx context.Context, key string, obj, out runtime.Object, + if err != nil { + return err + } +- key = path.Join(s.pathPrefix, key) + + opts, err := s.ttlOpts(ctx, int64(ttl)) + if err != nil { + return err + } + +- newData, err := s.transformer.TransformToStorage(data, authenticatedDataString(key)) ++ newData, err := s.transformer.TransformToStorage(data, authenticatedDataString(preparedKey)) + if err != nil { + return storage.NewInternalError(err.Error()) + } + + startTime := time.Now() + txnResp, err := s.client.KV.Txn(ctx).If( +- notFound(key), ++ notFound(preparedKey), + ).Then( +- clientv3.OpPut(key, string(newData), opts...), ++ clientv3.OpPut(preparedKey, string(newData), opts...), + ).Commit() + metrics.RecordEtcdRequestLatency("create", getTypeName(obj), startTime) + if err != nil { + return err + } + if !txnResp.Succeeded { +- return storage.NewKeyExistsError(key, 0) ++ return storage.NewKeyExistsError(preparedKey, 0) + } + + if out != nil { +@@ -186,12 +198,15 @@ func (s *store) Create(ctx context.Context, key string, obj, out runtime.Object, + + // Delete implements storage.Interface.Delete. + func (s *store) Delete(ctx context.Context, key string, out runtime.Object, preconditions *storage.Preconditions, validateDeletion storage.ValidateObjectFunc) error { ++ preparedKey, err := s.prepareKey(key) ++ if err != nil { ++ return err ++ } + v, err := conversion.EnforcePtr(out) + if err != nil { + return fmt.Errorf("unable to convert output object to pointer: %v", err) + } +- key = path.Join(s.pathPrefix, key) +- return s.conditionalDelete(ctx, key, out, v, preconditions, validateDeletion) ++ return s.conditionalDelete(ctx, preparedKey, out, v, preconditions, validateDeletion) + } + + func (s *store) conditionalDelete(ctx context.Context, key string, out runtime.Object, v reflect.Value, preconditions *storage.Preconditions, validateDeletion storage.ValidateObjectFunc) error { +@@ -239,6 +254,10 @@ func (s *store) conditionalDelete(ctx context.Context, key string, out runtime.O + func (s *store) GuaranteedUpdate( + ctx context.Context, key string, out runtime.Object, ignoreNotFound bool, + preconditions *storage.Preconditions, tryUpdate storage.UpdateFunc, suggestion runtime.Object) error { ++ preparedKey, err := s.prepareKey(key) ++ if err != nil { ++ return err ++ } + trace := utiltrace.New("GuaranteedUpdate etcd3", utiltrace.Field{"type", getTypeName(out)}) + defer trace.LogIfLong(500 * time.Millisecond) + +@@ -246,16 +265,15 @@ func (s *store) GuaranteedUpdate( + if err != nil { + return fmt.Errorf("unable to convert output object to pointer: %v", err) + } +- key = path.Join(s.pathPrefix, key) + + getCurrentState := func() (*objState, error) { + startTime := time.Now() +- getResp, err := s.client.KV.Get(ctx, key) ++ getResp, err := s.client.KV.Get(ctx, preparedKey) + metrics.RecordEtcdRequestLatency("get", getTypeName(out), startTime) + if err != nil { + return nil, err + } +- return s.getState(getResp, key, v, ignoreNotFound) ++ return s.getState(getResp, preparedKey, v, ignoreNotFound) + } + + var origState *objState +@@ -274,9 +292,9 @@ func (s *store) GuaranteedUpdate( + } + trace.Step("initial value restored") + +- transformContext := authenticatedDataString(key) ++ transformContext := authenticatedDataString(preparedKey) + for { +- if err := preconditions.Check(key, origState.obj); err != nil { ++ if err := preconditions.Check(preparedKey, origState.obj); err != nil { + // If our data is already up to date, return the error + if !mustCheckData { + return err +@@ -349,11 +367,11 @@ func (s *store) GuaranteedUpdate( + + startTime := time.Now() + txnResp, err := s.client.KV.Txn(ctx).If( +- clientv3.Compare(clientv3.ModRevision(key), "=", origState.rev), ++ clientv3.Compare(clientv3.ModRevision(preparedKey), "=", origState.rev), + ).Then( +- clientv3.OpPut(key, string(newData), opts...), ++ clientv3.OpPut(preparedKey, string(newData), opts...), + ).Else( +- clientv3.OpGet(key), ++ clientv3.OpGet(preparedKey), + ).Commit() + metrics.RecordEtcdRequestLatency("update", getTypeName(out), startTime) + if err != nil { +@@ -362,8 +380,8 @@ func (s *store) GuaranteedUpdate( + trace.Step("Transaction committed") + if !txnResp.Succeeded { + getResp := (*clientv3.GetResponse)(txnResp.Responses[0].GetResponseRange()) +- klog.V(4).Infof("GuaranteedUpdate of %s failed because of a conflict, going to retry", key) +- origState, err = s.getState(getResp, key, v, ignoreNotFound) ++ klog.V(4).Infof("GuaranteedUpdate of %s failed because of a conflict, going to retry", preparedKey) ++ origState, err = s.getState(getResp, preparedKey, v, ignoreNotFound) + if err != nil { + return err + } +@@ -379,6 +397,11 @@ func (s *store) GuaranteedUpdate( + + // GetToList implements storage.Interface.GetToList. + func (s *store) GetToList(ctx context.Context, key string, listOpts storage.ListOptions, listObj runtime.Object) error { ++ preparedKey, err := s.prepareKey(key) ++ if err != nil { ++ return err ++ } ++ + resourceVersion := listOpts.ResourceVersion + match := listOpts.ResourceVersionMatch + pred := listOpts.Predicate +@@ -400,7 +423,6 @@ func (s *store) GetToList(ctx context.Context, key string, listOpts storage.List + + newItemFunc := getNewItemFunc(listObj, v) + +- key = path.Join(s.pathPrefix, key) + startTime := time.Now() + var opts []clientv3.OpOption + if len(resourceVersion) > 0 && match == metav1.ResourceVersionMatchExact { +@@ -411,7 +433,7 @@ func (s *store) GetToList(ctx context.Context, key string, listOpts storage.List + opts = append(opts, clientv3.WithRev(int64(rv))) + } + +- getResp, err := s.client.KV.Get(ctx, key, opts...) ++ getResp, err := s.client.KV.Get(ctx, preparedKey, opts...) + metrics.RecordEtcdRequestLatency("get", getTypeName(listPtr), startTime) + if err != nil { + return err +@@ -421,7 +443,7 @@ func (s *store) GetToList(ctx context.Context, key string, listOpts storage.List + } + + if len(getResp.Kvs) > 0 { +- data, _, err := s.transformer.TransformFromStorage(getResp.Kvs[0].Value, authenticatedDataString(key)) ++ data, _, err := s.transformer.TransformFromStorage(getResp.Kvs[0].Value, authenticatedDataString(preparedKey)) + if err != nil { + return storage.NewInternalError(err.Error()) + } +@@ -451,18 +473,21 @@ func getNewItemFunc(listObj runtime.Object, v reflect.Value) func() runtime.Obje + } + + func (s *store) Count(key string) (int64, error) { +- key = path.Join(s.pathPrefix, key) ++ preparedKey, err := s.prepareKey(key) ++ if err != nil { ++ return 0, err ++ } + + // We need to make sure the key ended with "/" so that we only get children "directories". + // e.g. if we have key "/a", "/a/b", "/ab", getting keys with prefix "/a" will return all three, + // while with prefix "/a/" will return only "/a/b" which is the correct answer. +- if !strings.HasSuffix(key, "/") { +- key += "/" ++ if !strings.HasSuffix(preparedKey, "/") { ++ preparedKey += "/" + } + + startTime := time.Now() +- getResp, err := s.client.KV.Get(context.Background(), key, clientv3.WithRange(clientv3.GetPrefixRangeEnd(key)), clientv3.WithCountOnly()) +- metrics.RecordEtcdRequestLatency("listWithCount", key, startTime) ++ getResp, err := s.client.KV.Get(context.Background(), preparedKey, clientv3.WithRange(clientv3.GetPrefixRangeEnd(preparedKey)), clientv3.WithCountOnly()) ++ metrics.RecordEtcdRequestLatency("listWithCount", preparedKey, startTime) + if err != nil { + return 0, err + } +@@ -531,6 +556,11 @@ func encodeContinue(key, keyPrefix string, resourceVersion int64) (string, error + + // List implements storage.Interface.List. + func (s *store) List(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error { ++ preparedKey, err := s.prepareKey(key) ++ if err != nil { ++ return err ++ } ++ + resourceVersion := opts.ResourceVersion + match := opts.ResourceVersionMatch + pred := opts.Predicate +@@ -550,16 +580,13 @@ func (s *store) List(ctx context.Context, key string, opts storage.ListOptions, + return fmt.Errorf("need ptr to slice: %v", err) + } + +- if s.pathPrefix != "" { +- key = path.Join(s.pathPrefix, key) +- } + // We need to make sure the key ended with "/" so that we only get children "directories". + // e.g. if we have key "/a", "/a/b", "/ab", getting keys with prefix "/a" will return all three, + // while with prefix "/a/" will return only "/a/b" which is the correct answer. +- if !strings.HasSuffix(key, "/") { +- key += "/" ++ if !strings.HasSuffix(preparedKey, "/") { ++ preparedKey += "/" + } +- keyPrefix := key ++ keyPrefix := preparedKey + + // set the appropriate clientv3 options to filter the returned data set + var paging bool +@@ -595,7 +622,7 @@ func (s *store) List(ctx context.Context, key string, opts storage.ListOptions, + + rangeEnd := clientv3.GetPrefixRangeEnd(keyPrefix) + options = append(options, clientv3.WithRange(rangeEnd)) +- key = continueKey ++ preparedKey = continueKey + + // If continueRV > 0, the LIST request needs a specific resource version. + // continueRV==0 is invalid. +@@ -652,7 +679,7 @@ func (s *store) List(ctx context.Context, key string, opts storage.ListOptions, + var getResp *clientv3.GetResponse + for { + startTime := time.Now() +- getResp, err = s.client.KV.Get(ctx, key, options...) ++ getResp, err = s.client.KV.Get(ctx, preparedKey, options...) + metrics.RecordEtcdRequestLatency("list", getTypeName(listPtr), startTime) + if err != nil { + return interpretListError(err, len(pred.Continue) > 0, continueKey, keyPrefix) +@@ -705,7 +732,7 @@ func (s *store) List(ctx context.Context, key string, opts storage.ListOptions, + if int64(v.Len()) >= pred.Limit { + break + } +- key = string(lastKey) + "\x00" ++ preparedKey = string(lastKey) + "\x00" + if withRev == 0 { + withRev = returnedRV + options = append(options, clientv3.WithRev(withRev)) +@@ -779,12 +806,15 @@ func (s *store) WatchList(ctx context.Context, key string, opts storage.ListOpti + } + + func (s *store) watch(ctx context.Context, key string, opts storage.ListOptions, recursive bool) (watch.Interface, error) { ++ preparedKey, err := s.prepareKey(key) ++ if err != nil { ++ return nil, err ++ } + rev, err := s.versioner.ParseResourceVersion(opts.ResourceVersion) + if err != nil { + return nil, err + } +- key = path.Join(s.pathPrefix, key) +- return s.watcher.Watch(ctx, key, int64(rev), recursive, opts.ProgressNotify, opts.Predicate) ++ return s.watcher.Watch(ctx, preparedKey, int64(rev), recursive, opts.ProgressNotify, opts.Predicate) + } + + func (s *store) getState(getResp *clientv3.GetResponse, key string, v reflect.Value, ignoreNotFound bool) (*objState, error) { +@@ -896,6 +926,30 @@ func (s *store) validateMinimumResourceVersion(minimumResourceVersion string, ac + return nil + } + ++func (s *store) prepareKey(key string) (string, error) { ++ if key == ".." || ++ strings.HasPrefix(key, "../") || ++ strings.HasSuffix(key, "/..") || ++ strings.Contains(key, "/../") { ++ return "", fmt.Errorf("invalid key: %q", key) ++ } ++ if key == "." || ++ strings.HasPrefix(key, "./") || ++ strings.HasSuffix(key, "/.") || ++ strings.Contains(key, "/./") { ++ return "", fmt.Errorf("invalid key: %q", key) ++ } ++ if key == "" || key == "/" { ++ return "", fmt.Errorf("empty key: %q", key) ++ } ++ // We ensured that pathPrefix ends in '/' in construction, so skip any leading '/' in the key now. ++ startIndex := 0 ++ if key[0] == '/' { ++ startIndex = 1 ++ } ++ return s.pathPrefix + key[startIndex:], nil ++} ++ + // decode decodes value of bytes into object. It will also set the object resource version to rev. + // On success, objPtr would be set to the object. + func decode(codec runtime.Codec, versioner storage.Versioner, value []byte, objPtr runtime.Object, rev int64) error { +diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go +index 8496e03a..f3b20bc7 100644 +--- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go ++++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go +@@ -58,6 +58,7 @@ var scheme = runtime.NewScheme() + var codecs = serializer.NewCodecFactory(scheme) + + const defaultTestPrefix = "test!" ++const basePath = "/keybase" + + func init() { + metav1.AddToGroupVersion(scheme, metav1.SchemeGroupVersion) +@@ -260,12 +261,12 @@ func TestGet(t *testing.T) { + rv: fmt.Sprintf("%d", lastUpdatedCurrentRV+1), + }, { // test get on non-existing item with ignoreNotFound=false + name: "get non-existing", +- key: "/non-existing", ++ key: basePath + "/non-existing", + ignoreNotFound: false, + expectNotFoundErr: true, + }, { // test get on non-existing item with ignoreNotFound=true + name: "get non-existing, ignore not found", +- key: "/non-existing", ++ key: basePath + "/non-existing", + ignoreNotFound: true, + expectNotFoundErr: false, + expectedOut: &example.Pod{}, +@@ -493,7 +494,7 @@ func TestGuaranteedUpdate(t *testing.T) { + ctx, store, cluster := testSetup(t) + defer cluster.Terminate(t) + etcdClient := cluster.RandClient() +- key := "/testkey" ++ key := basePath + "/testkey" + + tests := []struct { + key string +@@ -505,14 +506,14 @@ func TestGuaranteedUpdate(t *testing.T) { + transformStale bool + hasSelfLink bool + }{{ // GuaranteedUpdate on non-existing key with ignoreNotFound=false +- key: "/non-existing", ++ key: basePath + "/non-existing", + ignoreNotFound: false, + precondition: nil, + expectNotFoundErr: true, + expectInvalidObjErr: false, + expectNoUpdate: false, + }, { // GuaranteedUpdate on non-existing key with ignoreNotFound=true +- key: "/non-existing", ++ key: basePath + "/non-existing", + ignoreNotFound: true, + precondition: nil, + expectNotFoundErr: false, +@@ -831,13 +832,13 @@ func TestTransformationFailure(t *testing.T) { + obj *example.Pod + storedObj *example.Pod + }{{ +- key: "/one-level/test", ++ key: basePath + "/one-level/test", + obj: &example.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "bar"}, + Spec: storagetesting.DeepEqualSafePodSpec(), + }, + }, { +- key: "/two-level/1/test", ++ key: basePath + "/two-level/1/test", + obj: &example.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "baz"}, + Spec: storagetesting.DeepEqualSafePodSpec(), +@@ -865,7 +866,7 @@ func TestTransformationFailure(t *testing.T) { + + // List should fail + var got example.PodList +- if err := store.List(ctx, "/", storage.ListOptions{Predicate: storage.Everything}, &got); !storage.IsInternalError(err) { ++ if err := store.List(ctx, basePath, storage.ListOptions{Predicate: storage.Everything}, &got); !storage.IsInternalError(err) { + t.Errorf("Unexpected error %v", err) + } + +@@ -928,23 +929,23 @@ func TestList(t *testing.T) { + storedObj *example.Pod + }{ + { +- key: "/one-level/test", ++ key: basePath + "/one-level/test", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, + }, + { +- key: "/two-level/1/test", ++ key: basePath + "/two-level/1/test", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, + }, + { +- key: "/two-level/2/test", ++ key: basePath + "/two-level/2/test", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "bar"}}, + }, + { +- key: "/z-level/3/test", ++ key: basePath + "/z-level/3/test", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "fourth"}}, + }, + { +- key: "/z-level/3/test-2", ++ key: basePath + "/z-level/3/test-2", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "bar"}}, + }, + } +@@ -958,7 +959,7 @@ func TestList(t *testing.T) { + } + + list := &example.PodList{} +- store.List(ctx, "/two-level", storage.ListOptions{ResourceVersion: "0", Predicate: storage.Everything}, list) ++ store.List(ctx, basePath+"/two-level", storage.ListOptions{ResourceVersion: "0", Predicate: storage.Everything}, list) + continueRV, _ := strconv.Atoi(list.ResourceVersion) + secondContinuation, err := encodeContinue("/two-level/2", "/two-level/", int64(continueRV)) + if err != nil { +@@ -986,14 +987,14 @@ func TestList(t *testing.T) { + }{ + { + name: "rejects invalid resource version", +- prefix: "/", ++ prefix: basePath, + pred: storage.Everything, + rv: "abc", + expectError: true, + }, + { + name: "rejects resource version and continue token", +- prefix: "/", ++ prefix: basePath, + pred: storage.SelectionPredicate{ + Label: labels.Everything(), + Field: fields.Everything(), +@@ -1005,26 +1006,26 @@ func TestList(t *testing.T) { + }, + { + name: "rejects resource version set too high", +- prefix: "/", ++ prefix: basePath, + rv: fmt.Sprintf("%d", continueRV+1), + expectRVTooLarge: true, + }, + { + name: "test List on existing key", +- prefix: "/one-level/", ++ prefix: basePath + "/one-level/", + pred: storage.Everything, + expectedOut: []*example.Pod{preset[0].storedObj}, + }, + { + name: "test List on existing key with resource version set to 0", +- prefix: "/one-level/", ++ prefix: basePath + "/one-level/", + pred: storage.Everything, + expectedOut: []*example.Pod{preset[0].storedObj}, + rv: "0", + }, + { + name: "test List on existing key with resource version set to 1, match=Exact", +- prefix: "/one-level/", ++ prefix: basePath + "/one-level/", + pred: storage.Everything, + expectedOut: []*example.Pod{}, + rv: "1", +@@ -1033,7 +1034,7 @@ func TestList(t *testing.T) { + }, + { + name: "test List on existing key with resource version set to 1, match=NotOlderThan", +- prefix: "/one-level/", ++ prefix: basePath + "/one-level/", + pred: storage.Everything, + expectedOut: []*example.Pod{preset[0].storedObj}, + rv: "0", +@@ -1041,7 +1042,7 @@ func TestList(t *testing.T) { + }, + { + name: "test List on existing key with resource version set to 1, match=Invalid", +- prefix: "/one-level/", ++ prefix: basePath + "/one-level/", + pred: storage.Everything, + rv: "0", + rvMatch: "Invalid", +@@ -1049,14 +1050,14 @@ func TestList(t *testing.T) { + }, + { + name: "test List on existing key with resource version set to current resource version", +- prefix: "/one-level/", ++ prefix: basePath + "/one-level/", + pred: storage.Everything, + expectedOut: []*example.Pod{preset[0].storedObj}, + rv: list.ResourceVersion, + }, + { + name: "test List on existing key with resource version set to current resource version, match=Exact", +- prefix: "/one-level/", ++ prefix: basePath + "/one-level/", + pred: storage.Everything, + expectedOut: []*example.Pod{preset[0].storedObj}, + rv: list.ResourceVersion, +@@ -1065,7 +1066,7 @@ func TestList(t *testing.T) { + }, + { + name: "test List on existing key with resource version set to current resource version, match=NotOlderThan", +- prefix: "/one-level/", ++ prefix: basePath + "/one-level/", + pred: storage.Everything, + expectedOut: []*example.Pod{preset[0].storedObj}, + rv: list.ResourceVersion, +@@ -1073,13 +1074,13 @@ func TestList(t *testing.T) { + }, + { + name: "test List on non-existing key", +- prefix: "/non-existing/", ++ prefix: basePath + "/non-existing/", + pred: storage.Everything, + expectedOut: nil, + }, + { + name: "test List with pod name matching", +- prefix: "/one-level/", ++ prefix: basePath + "/one-level/", + pred: storage.SelectionPredicate{ + Label: labels.Everything(), + Field: fields.ParseSelectorOrDie("metadata.name!=foo"), +@@ -1088,7 +1089,7 @@ func TestList(t *testing.T) { + }, + { + name: "test List with limit", +- prefix: "/two-level/", ++ prefix: basePath + "/two-level/", + pred: storage.SelectionPredicate{ + Label: labels.Everything(), + Field: fields.Everything(), +@@ -1100,7 +1101,7 @@ func TestList(t *testing.T) { + }, + { + name: "test List with limit at current resource version", +- prefix: "/two-level/", ++ prefix: basePath + "/two-level/", + pred: storage.SelectionPredicate{ + Label: labels.Everything(), + Field: fields.Everything(), +@@ -1114,7 +1115,7 @@ func TestList(t *testing.T) { + }, + { + name: "test List with limit at current resource version and match=Exact", +- prefix: "/two-level/", ++ prefix: basePath + "/two-level/", + pred: storage.SelectionPredicate{ + Label: labels.Everything(), + Field: fields.Everything(), +@@ -1129,7 +1130,7 @@ func TestList(t *testing.T) { + }, + { + name: "test List with limit at resource version 0", +- prefix: "/two-level/", ++ prefix: basePath + "/two-level/", + pred: storage.SelectionPredicate{ + Label: labels.Everything(), + Field: fields.Everything(), +@@ -1143,7 +1144,7 @@ func TestList(t *testing.T) { + }, + { + name: "test List with limit at resource version 0 match=NotOlderThan", +- prefix: "/two-level/", ++ prefix: basePath + "/two-level/", + pred: storage.SelectionPredicate{ + Label: labels.Everything(), + Field: fields.Everything(), +@@ -1158,7 +1159,7 @@ func TestList(t *testing.T) { + }, + { + name: "test List with limit at resource version 1 and match=Exact", +- prefix: "/two-level/", ++ prefix: basePath + "/two-level/", + pred: storage.SelectionPredicate{ + Label: labels.Everything(), + Field: fields.Everything(), +@@ -1187,7 +1188,7 @@ func TestList(t *testing.T) { + { + name: "test List with limit when paging disabled", + disablePaging: true, +- prefix: "/two-level/", ++ prefix: basePath + "/two-level/", + pred: storage.SelectionPredicate{ + Label: labels.Everything(), + Field: fields.Everything(), +@@ -1198,7 +1199,7 @@ func TestList(t *testing.T) { + }, + { + name: "test List with pregenerated continue token", +- prefix: "/two-level/", ++ prefix: basePath + "/two-level/", + pred: storage.SelectionPredicate{ + Label: labels.Everything(), + Field: fields.Everything(), +@@ -1209,7 +1210,7 @@ func TestList(t *testing.T) { + }, + { + name: "ignores resource version 0 for List with pregenerated continue token", +- prefix: "/two-level/", ++ prefix: basePath + "/two-level/", + pred: storage.SelectionPredicate{ + Label: labels.Everything(), + Field: fields.Everything(), +@@ -1221,13 +1222,13 @@ func TestList(t *testing.T) { + }, + { + name: "test List with multiple levels of directories and expect flattened result", +- prefix: "/two-level/", ++ prefix: basePath + "/two-level/", + pred: storage.Everything, + expectedOut: []*example.Pod{preset[1].storedObj, preset[2].storedObj}, + }, + { + name: "test List with filter returning only one item, ensure only a single page returned", +- prefix: "/", ++ prefix: basePath, + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "fourth"), + Label: labels.Everything(), +@@ -1238,7 +1239,7 @@ func TestList(t *testing.T) { + }, + { + name: "test List with filter returning only one item, covers the entire list", +- prefix: "/", ++ prefix: basePath, + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "fourth"), + Label: labels.Everything(), +@@ -1249,7 +1250,7 @@ func TestList(t *testing.T) { + }, + { + name: "test List with filter returning only one item, covers the entire list, with resource version 0", +- prefix: "/", ++ prefix: basePath, + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "fourth"), + Label: labels.Everything(), +@@ -1261,7 +1262,7 @@ func TestList(t *testing.T) { + }, + { + name: "test List with filter returning two items, more pages possible", +- prefix: "/", ++ prefix: basePath, + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "foo"), + Label: labels.Everything(), +@@ -1272,7 +1273,7 @@ func TestList(t *testing.T) { + }, + { + name: "filter returns two items split across multiple pages", +- prefix: "/", ++ prefix: basePath, + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "bar"), + Label: labels.Everything(), +@@ -1282,7 +1283,7 @@ func TestList(t *testing.T) { + }, + { + name: "filter returns one item for last page, ends on last item, not full", +- prefix: "/", ++ prefix: basePath, + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "bar"), + Label: labels.Everything(), +@@ -1293,7 +1294,7 @@ func TestList(t *testing.T) { + }, + { + name: "filter returns one item for last page, starts on last item, full", +- prefix: "/", ++ prefix: basePath, + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "bar"), + Label: labels.Everything(), +@@ -1304,7 +1305,7 @@ func TestList(t *testing.T) { + }, + { + name: "filter returns one item for last page, starts on last item, partial page", +- prefix: "/", ++ prefix: basePath, + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "bar"), + Label: labels.Everything(), +@@ -1315,7 +1316,7 @@ func TestList(t *testing.T) { + }, + { + name: "filter returns two items, page size equal to total list size", +- prefix: "/", ++ prefix: basePath, + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "bar"), + Label: labels.Everything(), +@@ -1325,7 +1326,7 @@ func TestList(t *testing.T) { + }, + { + name: "filter returns one item, page size equal to total list size", +- prefix: "/", ++ prefix: basePath, + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "fourth"), + Label: labels.Everything(), +@@ -1403,7 +1404,7 @@ func TestListContinuation(t *testing.T) { + ctx := context.Background() + + // Setup storage with the following structure: +- // / ++ // /keybase/ + // - one-level/ + // | - test + // | +@@ -1420,15 +1421,15 @@ func TestListContinuation(t *testing.T) { + storedObj *example.Pod + }{ + { +- key: "/one-level/test", ++ key: basePath + "/one-level/test", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, + }, + { +- key: "/two-level/1/test", ++ key: basePath + "/two-level/1/test", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, + }, + { +- key: "/two-level/2/test", ++ key: basePath + "/two-level/2/test", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "bar"}}, + }, + } +@@ -1455,7 +1456,7 @@ func TestListContinuation(t *testing.T) { + }, + } + } +- if err := store.List(ctx, "/", storage.ListOptions{ResourceVersion: "0", Predicate: pred(1, "")}, out); err != nil { ++ if err := store.List(ctx, basePath, storage.ListOptions{ResourceVersion: "0", Predicate: pred(1, "")}, out); err != nil { + t.Fatalf("Unable to get initial list: %v", err) + } + if len(out.Continue) == 0 { +@@ -1477,14 +1478,14 @@ func TestListContinuation(t *testing.T) { + + // no limit, should get two items + out = &example.PodList{} +- if err := store.List(ctx, "/", storage.ListOptions{ResourceVersion: "0", Predicate: pred(0, continueFromSecondItem)}, out); err != nil { ++ if err := store.List(ctx, basePath, storage.ListOptions{ResourceVersion: "0", Predicate: pred(0, continueFromSecondItem)}, out); err != nil { + t.Fatalf("Unable to get second page: %v", err) + } + if len(out.Continue) != 0 { + t.Fatalf("Unexpected continuation token set") + } + if !reflect.DeepEqual(out.Items, []example.Pod{*preset[1].storedObj, *preset[2].storedObj}) { +- key, rv, err := decodeContinue(continueFromSecondItem, "/") ++ key, rv, err := decodeContinue(continueFromSecondItem, basePath) + t.Logf("continue token was %d %s %v", rv, key, err) + t.Fatalf("Unexpected second page: %#v", out.Items) + } +@@ -1499,7 +1500,7 @@ func TestListContinuation(t *testing.T) { + + // limit, should get two more pages + out = &example.PodList{} +- if err := store.List(ctx, "/", storage.ListOptions{ResourceVersion: "0", Predicate: pred(1, continueFromSecondItem)}, out); err != nil { ++ if err := store.List(ctx, basePath, storage.ListOptions{ResourceVersion: "0", Predicate: pred(1, continueFromSecondItem)}, out); err != nil { + t.Fatalf("Unable to get second page: %v", err) + } + if len(out.Continue) == 0 { +@@ -1520,7 +1521,7 @@ func TestListContinuation(t *testing.T) { + continueFromThirdItem := out.Continue + + out = &example.PodList{} +- if err := store.List(ctx, "/", storage.ListOptions{ResourceVersion: "0", Predicate: pred(1, continueFromThirdItem)}, out); err != nil { ++ if err := store.List(ctx, basePath, storage.ListOptions{ResourceVersion: "0", Predicate: pred(1, continueFromThirdItem)}, out); err != nil { + t.Fatalf("Unable to get second page: %v", err) + } + if len(out.Continue) != 0 { +@@ -1570,26 +1571,26 @@ func TestListContinuationWithFilter(t *testing.T) { + storedObj *example.Pod + }{ + { +- key: "/1", ++ key: basePath + "/1", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, + }, + { +- key: "/2", ++ key: basePath + "/2", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "bar"}}, // this should not match + }, + { +- key: "/3", ++ key: basePath + "/3", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, + }, + { +- key: "/4", ++ key: basePath + "/4", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, + }, + } + + for i, ps := range preset { + preset[i].storedObj = &example.Pod{} +- err := store.Create(ctx, ps.key, ps.obj, preset[i].storedObj, 0) ++ err := store.Create(ctx, basePath+ps.key, ps.obj, preset[i].storedObj, 0) + if err != nil { + t.Fatalf("Set failed: %v", err) + } +@@ -1612,7 +1613,7 @@ func TestListContinuationWithFilter(t *testing.T) { + }, + } + } +- if err := store.List(ctx, "/", storage.ListOptions{ResourceVersion: "0", Predicate: pred(2, "")}, out); err != nil { ++ if err := store.List(ctx, basePath, storage.ListOptions{ResourceVersion: "0", Predicate: pred(2, "")}, out); err != nil { + t.Errorf("Unable to get initial list: %v", err) + } + if len(out.Continue) == 0 { +@@ -1641,7 +1642,7 @@ func TestListContinuationWithFilter(t *testing.T) { + // but since there is only one item left, that is all we should get with no continueValue + // both read counters should be incremented for the singular calls they make in this case + out = &example.PodList{} +- if err := store.List(ctx, "/", storage.ListOptions{ResourceVersion: "0", Predicate: pred(2, cont)}, out); err != nil { ++ if err := store.List(ctx, basePath, storage.ListOptions{ResourceVersion: "0", Predicate: pred(2, cont)}, out); err != nil { + t.Errorf("Unable to get second page: %v", err) + } + if len(out.Continue) != 0 { +@@ -1668,7 +1669,7 @@ func TestListInconsistentContinuation(t *testing.T) { + ctx := context.Background() + + // Setup storage with the following structure: +- // / ++ // /keybase/ + // - one-level/ + // | - test + // | +@@ -1685,15 +1686,15 @@ func TestListInconsistentContinuation(t *testing.T) { + storedObj *example.Pod + }{ + { +- key: "/one-level/test", ++ key: basePath + "/one-level/test", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, + }, + { +- key: "/two-level/1/test", ++ key: basePath + "/two-level/1/test", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, + }, + { +- key: "/two-level/2/test", ++ key: basePath + "/two-level/2/test", + obj: &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "bar"}}, + }, + } +@@ -1720,7 +1721,7 @@ func TestListInconsistentContinuation(t *testing.T) { + } + + out := &example.PodList{} +- if err := store.List(ctx, "/", storage.ListOptions{ResourceVersion: "0", Predicate: pred(1, "")}, out); err != nil { ++ if err := store.List(ctx, basePath, storage.ListOptions{ResourceVersion: "0", Predicate: pred(1, "")}, out); err != nil { + t.Fatalf("Unable to get initial list: %v", err) + } + if len(out.Continue) == 0 { +@@ -1761,7 +1762,7 @@ func TestListInconsistentContinuation(t *testing.T) { + } + + // The old continue token should have expired +- err = store.List(ctx, "/", storage.ListOptions{ResourceVersion: "0", Predicate: pred(0, continueFromSecondItem)}, out) ++ err = store.List(ctx, basePath, storage.ListOptions{ResourceVersion: "0", Predicate: pred(0, continueFromSecondItem)}, out) + if err == nil { + t.Fatalf("unexpected no error") + } +@@ -1778,7 +1779,7 @@ func TestListInconsistentContinuation(t *testing.T) { + } + + out = &example.PodList{} +- if err := store.List(ctx, "/", storage.ListOptions{ResourceVersion: "0", Predicate: pred(1, inconsistentContinueFromSecondItem)}, out); err != nil { ++ if err := store.List(ctx, basePath, storage.ListOptions{ResourceVersion: "0", Predicate: pred(1, inconsistentContinueFromSecondItem)}, out); err != nil { + t.Fatalf("Unable to get second page: %v", err) + } + if len(out.Continue) == 0 { +@@ -1792,7 +1793,7 @@ func TestListInconsistentContinuation(t *testing.T) { + } + continueFromThirdItem := out.Continue + out = &example.PodList{} +- if err := store.List(ctx, "/", storage.ListOptions{ResourceVersion: "0", Predicate: pred(1, continueFromThirdItem)}, out); err != nil { ++ if err := store.List(ctx, basePath, storage.ListOptions{ResourceVersion: "0", Predicate: pred(1, continueFromThirdItem)}, out); err != nil { + t.Fatalf("Unable to get second page: %v", err) + } + if len(out.Continue) != 0 { +@@ -1822,7 +1823,7 @@ func testSetup(t *testing.T) (context.Context, *store, *integration.ClusterV3) { + // keys and stored objects. + func testPropogateStore(ctx context.Context, t *testing.T, store *store, obj *example.Pod) (string, *example.Pod) { + // Setup store with a key and grab the output for returning. +- key := "/testkey" ++ key := basePath + "/testkey" + return key, testPropogateStoreWithKey(ctx, t, store, key, obj) + } + +@@ -1850,9 +1851,9 @@ func TestPrefix(t *testing.T) { + defer cluster.Terminate(t) + transformer := &prefixTransformer{prefix: []byte(defaultTestPrefix)} + testcases := map[string]string{ +- "custom/prefix": "/custom/prefix", +- "/custom//prefix//": "/custom/prefix", +- "/registry": "/registry", ++ "custom/prefix": "/custom/prefix/", ++ "/custom//prefix//": "/custom/prefix/", ++ "/registry": "/registry/", + } + for configuredPrefix, effectivePrefix := range testcases { + store := newStore(cluster.RandClient(), nil, true, codec, configuredPrefix, transformer) +@@ -2045,7 +2046,7 @@ func TestConsistentList(t *testing.T) { + } + + result1 := example.PodList{} +- if err := store.List(context.TODO(), "/", storage.ListOptions{Predicate: predicate}, &result1); err != nil { ++ if err := store.List(context.TODO(), basePath, storage.ListOptions{Predicate: predicate}, &result1); err != nil { + t.Fatalf("failed to list objects: %v", err) + } + +@@ -2057,7 +2058,7 @@ func TestConsistentList(t *testing.T) { + } + + result2 := example.PodList{} +- if err := store.List(context.TODO(), "/", options, &result2); err != nil { ++ if err := store.List(context.TODO(), basePath, options, &result2); err != nil { + t.Fatalf("failed to list objects: %v", err) + } + +@@ -2069,7 +2070,7 @@ func TestConsistentList(t *testing.T) { + options.ResourceVersionMatch = metav1.ResourceVersionMatchNotOlderThan + + result3 := example.PodList{} +- if err := store.List(context.TODO(), "/", options, &result3); err != nil { ++ if err := store.List(context.TODO(), basePath, options, &result3); err != nil { + t.Fatalf("failed to list objects: %v", err) + } + +@@ -2077,7 +2078,7 @@ func TestConsistentList(t *testing.T) { + options.ResourceVersionMatch = metav1.ResourceVersionMatchExact + + result4 := example.PodList{} +- if err := store.List(context.TODO(), "/", options, &result4); err != nil { ++ if err := store.List(context.TODO(), basePath, options, &result4); err != nil { + t.Fatalf("failed to list objects: %v", err) + } + +@@ -2123,3 +2124,83 @@ func TestCount(t *testing.T) { + t.Fatalf("store.Count for resource %s: expected %d but got %d", resourceA, resourceACountExpected, resourceACountGot) + } + } ++ ++func TestValidateKey(t *testing.T) { ++ validKeys := []string{ ++ "/foo/bar/baz/a.b.c/", ++ "/foo", ++ "foo/bar/baz", ++ "/foo/bar..baz/", ++ "/foo/bar..", ++ "foo", ++ "foo/bar", ++ "/foo/bar/", ++ } ++ invalidKeys := []string{ ++ "/foo/bar/../a.b.c/", ++ "..", ++ "/..", ++ "../", ++ "/foo/bar/..", ++ "../foo/bar", ++ "/../foo", ++ "/foo/bar/../", ++ ".", ++ "/.", ++ "./", ++ "/./", ++ "/foo/.", ++ "./bar", ++ "/foo/./bar/", ++ } ++ const ( ++ pathPrefix = "/first/second" ++ expectPrefix = pathPrefix + "/" ++ ) ++ client := testserver.RunEtcd(t, nil) ++ codec := apitesting.TestCodec(codecs, examplev1.SchemeGroupVersion) ++ store := newStore(client, codec, newPod, pathPrefix, &prefixTransformer{prefix: []byte(defaultTestPrefix)}, true, LeaseManagerConfig{ ++ ReuseDurationSeconds: 1, ++ MaxObjectCount: defaultLeaseMaxObjectCount, ++ }) ++ ++ for _, key := range validKeys { ++ k, err := store.prepareKey(key) ++ if err != nil { ++ t.Errorf("key %q should be valid; unexpected error: %v", key, err) ++ } else if !strings.HasPrefix(k, expectPrefix) { ++ t.Errorf("key %q should have prefix %q", k, expectPrefix) ++ } ++ } ++ ++ for _, key := range invalidKeys { ++ _, err := store.prepareKey(key) ++ if err == nil { ++ t.Errorf("key %q should be invalid", key) ++ } ++ } ++} ++ ++func TestInvalidKeys(t *testing.T) { ++ const invalidKey = "/foo/bar/../baz" ++ expectedError := fmt.Sprintf("invalid key: %q", invalidKey) ++ ++ expectInvalidKey := func(methodName string, err error) { ++ if err == nil { ++ t.Errorf("[%s] expected invalid key error; got nil", methodName) ++ } else if err.Error() != expectedError { ++ t.Errorf("[%s] expected invalid key error; got %v", methodName, err) ++ } ++ } ++ ++ ctx, store, _ := testSetup(t) ++ expectInvalidKey("Create", store.Create(ctx, invalidKey, nil, nil, 0)) ++ expectInvalidKey("Delete", store.Delete(ctx, invalidKey, nil, nil, nil, nil)) ++ _, watchErr := store.Watch(ctx, invalidKey, storage.ListOptions{}) ++ expectInvalidKey("Watch", watchErr) ++ expectInvalidKey("Get", store.Get(ctx, invalidKey, storage.GetOptions{}, nil)) ++ expectInvalidKey("GetList", store.List(ctx, invalidKey, storage.ListOptions{}, nil)) ++ expectInvalidKey("GuaranteedUpdate", store.GuaranteedUpdate(ctx, invalidKey, nil, true, nil, nil, nil)) ++ _, countErr := store.Count(invalidKey) ++ expectInvalidKey("Count", countErr) ++} diff --git a/0014-fix-node-address-validation.patch b/0014-fix-node-address-validation.patch new file mode 100644 index 0000000000000000000000000000000000000000..06e4bb8c16e197f8e8d6a22605ee80860be4a2e1 --- /dev/null +++ b/0014-fix-node-address-validation.patch @@ -0,0 +1,245 @@ +From 78d552efe620879599509ff0f9b04b8764e1878f Mon Sep 17 00:00:00 2001 +From: Andrew Sy Kim +Date: Mon, 7 Nov 2022 10:22:44 -0500 +Subject: [PATCH] fix node address validation + +Signed-off-by: Andrew Sy Kim +--- + .../core/node/storage/storage_test.go | 171 +++++++++++++++++- + pkg/registry/core/node/strategy.go | 8 +- + 2 files changed, 174 insertions(+), 5 deletions(-) + +diff --git a/pkg/registry/core/node/storage/storage_test.go b/pkg/registry/core/node/storage/storage_test.go +index af228b8a..89df2edf 100644 +--- a/pkg/registry/core/node/storage/storage_test.go ++++ b/pkg/registry/core/node/storage/storage_test.go +@@ -17,6 +17,7 @@ limitations under the License. + package storage + + import ( ++ "fmt" + "testing" + + "k8s.io/apimachinery/pkg/api/resource" +@@ -24,11 +25,14 @@ import ( + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" ++ genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/apiserver/pkg/registry/generic" + genericregistrytest "k8s.io/apiserver/pkg/registry/generic/testing" ++ "k8s.io/apiserver/pkg/registry/rest" + etcd3testing "k8s.io/apiserver/pkg/storage/etcd3/testing" + api "k8s.io/kubernetes/pkg/apis/core" + kubeletclient "k8s.io/kubernetes/pkg/kubelet/client" ++ proxyutil "k8s.io/kubernetes/pkg/proxy/util" + "k8s.io/kubernetes/pkg/registry/registrytest" + ) + +@@ -40,7 +44,10 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { + DeleteCollectionWorkers: 1, + ResourcePrefix: "nodes", + } +- storage, err := NewStorage(restOptions, kubeletclient.KubeletClientConfig{}, nil) ++ storage, err := NewStorage(restOptions, kubeletclient.KubeletClientConfig{ ++ Port: 10250, ++ PreferredAddressTypes: []string{string(api.NodeInternalIP)}, ++ }, nil) + if err != nil { + t.Fatal(err) + } +@@ -156,3 +163,165 @@ func TestShortNames(t *testing.T) { + expected := []string{"no"} + registrytest.AssertShortNames(t, storage, expected) + } ++ ++func TestResourceLocation(t *testing.T) { ++ testCases := []struct { ++ name string ++ node api.Node ++ query string ++ location string ++ err error ++ }{ ++ { ++ name: "proxyable hostname with default port", ++ node: api.Node{ ++ ObjectMeta: metav1.ObjectMeta{Name: "node0"}, ++ Status: api.NodeStatus{ ++ Addresses: []api.NodeAddress{ ++ { ++ Type: api.NodeInternalIP, ++ Address: "10.0.0.1", ++ }, ++ }, ++ }, ++ }, ++ query: "node0", ++ location: "10.0.0.1:10250", ++ err: nil, ++ }, ++ { ++ name: "proxyable hostname with kubelet port in query", ++ node: api.Node{ ++ ObjectMeta: metav1.ObjectMeta{Name: "node0"}, ++ Status: api.NodeStatus{ ++ Addresses: []api.NodeAddress{ ++ { ++ Type: api.NodeInternalIP, ++ Address: "10.0.0.1", ++ }, ++ }, ++ }, ++ }, ++ query: "node0:5000", ++ location: "10.0.0.1:5000", ++ err: nil, ++ }, ++ { ++ name: "proxyable hostname with kubelet port in status", ++ node: api.Node{ ++ ObjectMeta: metav1.ObjectMeta{Name: "node0"}, ++ Status: api.NodeStatus{ ++ Addresses: []api.NodeAddress{ ++ { ++ Type: api.NodeInternalIP, ++ Address: "10.0.0.1", ++ }, ++ }, ++ DaemonEndpoints: api.NodeDaemonEndpoints{ ++ KubeletEndpoint: api.DaemonEndpoint{ ++ Port: 5000, ++ }, ++ }, ++ }, ++ }, ++ query: "node0", ++ location: "10.0.0.1:5000", ++ err: nil, ++ }, ++ { ++ name: "non-proxyable hostname with default port", ++ node: api.Node{ ++ ObjectMeta: metav1.ObjectMeta{Name: "node0"}, ++ Status: api.NodeStatus{ ++ Addresses: []api.NodeAddress{ ++ { ++ Type: api.NodeInternalIP, ++ Address: "127.0.0.1", ++ }, ++ }, ++ }, ++ }, ++ query: "node0", ++ location: "", ++ err: proxyutil.ErrAddressNotAllowed, ++ }, ++ { ++ name: "non-proxyable hostname with kubelet port in query", ++ node: api.Node{ ++ ObjectMeta: metav1.ObjectMeta{Name: "node0"}, ++ Status: api.NodeStatus{ ++ Addresses: []api.NodeAddress{ ++ { ++ Type: api.NodeInternalIP, ++ Address: "127.0.0.1", ++ }, ++ }, ++ }, ++ }, ++ query: "node0:5000", ++ location: "", ++ err: proxyutil.ErrAddressNotAllowed, ++ }, ++ { ++ name: "non-proxyable hostname with kubelet port in status", ++ node: api.Node{ ++ ObjectMeta: metav1.ObjectMeta{Name: "node0"}, ++ Status: api.NodeStatus{ ++ Addresses: []api.NodeAddress{ ++ { ++ Type: api.NodeInternalIP, ++ Address: "127.0.0.1", ++ }, ++ }, ++ DaemonEndpoints: api.NodeDaemonEndpoints{ ++ KubeletEndpoint: api.DaemonEndpoint{ ++ Port: 443, ++ }, ++ }, ++ }, ++ }, ++ query: "node0", ++ location: "", ++ err: proxyutil.ErrAddressNotAllowed, ++ }, ++ } ++ ++ for _, testCase := range testCases { ++ t.Run(testCase.name, func(t *testing.T) { ++ storage, server := newStorage(t) ++ defer server.Terminate(t) ++ defer storage.Store.DestroyFunc() ++ ++ ctx := genericapirequest.WithNamespace(genericapirequest.NewDefaultContext(), fmt.Sprintf("namespace-%s", testCase.name)) ++ key, _ := storage.KeyFunc(ctx, testCase.node.Name) ++ if err := storage.Storage.Create(ctx, key, &testCase.node, nil, 0, false); err != nil { ++ t.Fatalf("unexpected error: %v", err) ++ } ++ ++ redirector := rest.Redirector(storage) ++ location, _, err := redirector.ResourceLocation(ctx, testCase.query) ++ ++ if err != nil && testCase.err != nil { ++ if err.Error() != testCase.err.Error() { ++ t.Fatalf("Unexpected error: %v, expected: %v", err, testCase.err) ++ } ++ ++ return ++ } ++ ++ if err != nil && testCase.err == nil { ++ t.Fatalf("Unexpected error: %v, expected: %v", err, testCase.err) ++ } else if err == nil && testCase.err != nil { ++ t.Fatalf("Expected error but got none, err: %v", testCase.err) ++ } ++ ++ if location == nil { ++ t.Errorf("Unexpected nil resource location: %v", location) ++ } ++ ++ if location.Host != testCase.location { ++ t.Errorf("Unexpected host: expected %v, but got %v", testCase.location, location.Host) ++ } ++ }) ++ } ++} +diff --git a/pkg/registry/core/node/strategy.go b/pkg/registry/core/node/strategy.go +index aaddb69e..59096e56 100644 +--- a/pkg/registry/core/node/strategy.go ++++ b/pkg/registry/core/node/strategy.go +@@ -242,6 +242,10 @@ func ResourceLocation(getter ResourceGetter, connection client.ConnectionInfoGet + return nil, nil, err + } + ++ if err := proxyutil.IsProxyableHostname(ctx, &net.Resolver{}, info.Hostname); err != nil { ++ return nil, nil, errors.NewBadRequest(err.Error()) ++ } ++ + // We check if we want to get a default Kubelet's transport. It happens if either: + // - no port is specified in request (Kubelet's port is default) + // - the requested port matches the kubelet port for this node +@@ -254,10 +258,6 @@ func ResourceLocation(getter ResourceGetter, connection client.ConnectionInfoGet + nil + } + +- if err := proxyutil.IsProxyableHostname(ctx, &net.Resolver{}, info.Hostname); err != nil { +- return nil, nil, errors.NewBadRequest(err.Error()) +- } +- + // Otherwise, return the requested scheme and port, and the proxy transport + return &url.URL{Scheme: schemeReq, Host: net.JoinHostPort(info.Hostname, portReq)}, proxyTransport, nil + } +-- +2.25.1 + diff --git a/0015-Add-ephemeralcontainer-to-imagepolicy-securityaccoun.patch b/0015-Add-ephemeralcontainer-to-imagepolicy-securityaccoun.patch new file mode 100644 index 0000000000000000000000000000000000000000..d88733aaa18397ca7c5335ac91fae476e1e6d204 --- /dev/null +++ b/0015-Add-ephemeralcontainer-to-imagepolicy-securityaccoun.patch @@ -0,0 +1,554 @@ +From f754a4dee31455a0d7fc0f51cb85348af9ea5e1f Mon Sep 17 00:00:00 2001 +From: Rita Zhang +Date: Tue, 30 May 2023 20:35:33 +0000 +Subject: [PATCH] Add ephemeralcontainer to imagepolicy securityaccount + admission plugin + +Signed-off-by: Rita Zhang +--- + plugin/pkg/admission/imagepolicy/admission.go | 28 ++-- + .../admission/imagepolicy/admission_test.go | 148 +++++++++++++++++- + .../pkg/admission/serviceaccount/admission.go | 57 ++++++- + .../serviceaccount/admission_test.go | 88 +++++++++++ + 4 files changed, 297 insertions(+), 24 deletions(-) + +diff --git a/plugin/pkg/admission/imagepolicy/admission.go b/plugin/pkg/admission/imagepolicy/admission.go +index 80bd4852..34290671 100644 +--- a/plugin/pkg/admission/imagepolicy/admission.go ++++ b/plugin/pkg/admission/imagepolicy/admission.go +@@ -46,6 +46,7 @@ import ( + + // PluginName indicates name of admission plugin. + const PluginName = "ImagePolicyWebhook" ++const ephemeralcontainers = "ephemeralcontainers" + + // AuditKeyPrefix is used as the prefix for all audit keys handled by this + // pluggin. Some well known suffixes are listed below. +@@ -132,8 +133,9 @@ func (a *Plugin) webhookError(pod *api.Pod, attributes admission.Attributes, err + + // Validate makes an admission decision based on the request attributes + func (a *Plugin) Validate(ctx context.Context, attributes admission.Attributes, o admission.ObjectInterfaces) (err error) { +- // Ignore all calls to subresources or resources other than pods. +- if attributes.GetSubresource() != "" || attributes.GetResource().GroupResource() != api.Resource("pods") { ++ // Ignore all calls to subresources other than ephemeralcontainers or calls to resources other than pods. ++ subresource := attributes.GetSubresource() ++ if (subresource != "" && subresource != ephemeralcontainers) || attributes.GetResource().GroupResource() != api.Resource("pods") { + return nil + } + +@@ -144,13 +146,21 @@ func (a *Plugin) Validate(ctx context.Context, attributes admission.Attributes, + + // Build list of ImageReviewContainerSpec + var imageReviewContainerSpecs []v1alpha1.ImageReviewContainerSpec +- containers := make([]api.Container, 0, len(pod.Spec.Containers)+len(pod.Spec.InitContainers)) +- containers = append(containers, pod.Spec.Containers...) +- containers = append(containers, pod.Spec.InitContainers...) +- for _, c := range containers { +- imageReviewContainerSpecs = append(imageReviewContainerSpecs, v1alpha1.ImageReviewContainerSpec{ +- Image: c.Image, +- }) ++ if subresource == "" { ++ containers := make([]api.Container, 0, len(pod.Spec.Containers)+len(pod.Spec.InitContainers)) ++ containers = append(containers, pod.Spec.Containers...) ++ containers = append(containers, pod.Spec.InitContainers...) ++ for _, c := range containers { ++ imageReviewContainerSpecs = append(imageReviewContainerSpecs, v1alpha1.ImageReviewContainerSpec{ ++ Image: c.Image, ++ }) ++ } ++ } else if subresource == ephemeralcontainers { ++ for _, c := range pod.Spec.EphemeralContainers { ++ imageReviewContainerSpecs = append(imageReviewContainerSpecs, v1alpha1.ImageReviewContainerSpec{ ++ Image: c.Image, ++ }) ++ } + } + imageReview := v1alpha1.ImageReview{ + Spec: v1alpha1.ImageReviewSpec{ +diff --git a/plugin/pkg/admission/imagepolicy/admission_test.go b/plugin/pkg/admission/imagepolicy/admission_test.go +index d1f81d51..a9188462 100644 +--- a/plugin/pkg/admission/imagepolicy/admission_test.go ++++ b/plugin/pkg/admission/imagepolicy/admission_test.go +@@ -37,7 +37,6 @@ import ( + api "k8s.io/kubernetes/pkg/apis/core" + + "fmt" +- "io/ioutil" + "os" + "path/filepath" + "text/template" +@@ -67,7 +66,7 @@ imagePolicy: + ` + + func TestNewFromConfig(t *testing.T) { +- dir, err := ioutil.TempDir("", "") ++ dir, err := os.MkdirTemp("", "") + if err != nil { + t.Fatal(err) + } +@@ -92,7 +91,7 @@ func TestNewFromConfig(t *testing.T) { + {data.Key, clientKey}, + } + for _, file := range files { +- if err := ioutil.WriteFile(file.name, file.data, 0400); err != nil { ++ if err := os.WriteFile(file.name, file.data, 0400); err != nil { + t.Fatal(err) + } + } +@@ -196,7 +195,7 @@ current-context: default + // Use a closure so defer statements trigger between loop iterations. + t.Run(tt.msg, func(t *testing.T) { + err := func() error { +- tempfile, err := ioutil.TempFile("", "") ++ tempfile, err := os.CreateTemp("", "") + if err != nil { + return err + } +@@ -211,7 +210,7 @@ current-context: default + return fmt.Errorf("failed to execute test template: %v", err) + } + +- tempconfigfile, err := ioutil.TempFile("", "") ++ tempconfigfile, err := os.CreateTemp("", "") + if err != nil { + return err + } +@@ -359,7 +358,7 @@ func (m *mockService) HTTPStatusCode() int { return m.statusCode } + // newImagePolicyWebhook creates a temporary kubeconfig file from the provided arguments and attempts to load + // a new newImagePolicyWebhook from it. + func newImagePolicyWebhook(callbackURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration, defaultAllow bool) (*Plugin, error) { +- tempfile, err := ioutil.TempFile("", "") ++ tempfile, err := os.CreateTemp("", "") + if err != nil { + return nil, err + } +@@ -381,7 +380,7 @@ func newImagePolicyWebhook(callbackURL string, clientCert, clientKey, ca []byte, + return nil, err + } + +- tempconfigfile, err := ioutil.TempFile("", "") ++ tempconfigfile, err := os.CreateTemp("", "") + if err != nil { + return nil, err + } +@@ -595,17 +594,23 @@ func TestContainerCombinations(t *testing.T) { + test string + pod *api.Pod + wantAllowed, wantErr bool ++ subresource string ++ operation admission.Operation + }{ + { + test: "Single container allowed", + pod: goodPod("good"), + wantAllowed: true, ++ subresource: "", ++ operation: admission.Create, + }, + { + test: "Single container denied", + pod: goodPod("bad"), + wantAllowed: false, + wantErr: true, ++ subresource: "", ++ operation: admission.Create, + }, + { + test: "One good container, one bad", +@@ -627,6 +632,8 @@ func TestContainerCombinations(t *testing.T) { + }, + wantAllowed: false, + wantErr: true, ++ subresource: "", ++ operation: admission.Create, + }, + { + test: "Multiple good containers", +@@ -648,6 +655,8 @@ func TestContainerCombinations(t *testing.T) { + }, + wantAllowed: true, + wantErr: false, ++ subresource: "", ++ operation: admission.Create, + }, + { + test: "Multiple bad containers", +@@ -669,6 +678,8 @@ func TestContainerCombinations(t *testing.T) { + }, + wantAllowed: false, + wantErr: true, ++ subresource: "", ++ operation: admission.Create, + }, + { + test: "Good container, bad init container", +@@ -692,6 +703,8 @@ func TestContainerCombinations(t *testing.T) { + }, + wantAllowed: false, + wantErr: true, ++ subresource: "", ++ operation: admission.Create, + }, + { + test: "Bad container, good init container", +@@ -715,6 +728,8 @@ func TestContainerCombinations(t *testing.T) { + }, + wantAllowed: false, + wantErr: true, ++ subresource: "", ++ operation: admission.Create, + }, + { + test: "Good container, good init container", +@@ -738,6 +753,123 @@ func TestContainerCombinations(t *testing.T) { + }, + wantAllowed: true, + wantErr: false, ++ subresource: "", ++ operation: admission.Create, ++ }, ++ { ++ test: "Good container, good init container, bad ephemeral container when updating ephemeralcontainers subresource", ++ pod: &api.Pod{ ++ Spec: api.PodSpec{ ++ ServiceAccountName: "default", ++ SecurityContext: &api.PodSecurityContext{}, ++ Containers: []api.Container{ ++ { ++ Image: "good", ++ SecurityContext: &api.SecurityContext{}, ++ }, ++ }, ++ InitContainers: []api.Container{ ++ { ++ Image: "good", ++ SecurityContext: &api.SecurityContext{}, ++ }, ++ }, ++ EphemeralContainers: []api.EphemeralContainer{ ++ { ++ EphemeralContainerCommon: api.EphemeralContainerCommon{ ++ Image: "bad", ++ SecurityContext: &api.SecurityContext{}, ++ }, ++ }, ++ }, ++ }, ++ }, ++ wantAllowed: false, ++ wantErr: true, ++ subresource: "ephemeralcontainers", ++ operation: admission.Update, ++ }, ++ { ++ test: "Good container, good init container, bad ephemeral container when updating subresource=='' which sets initContainer and container only", ++ pod: &api.Pod{ ++ Spec: api.PodSpec{ ++ ServiceAccountName: "default", ++ SecurityContext: &api.PodSecurityContext{}, ++ Containers: []api.Container{ ++ { ++ Image: "good", ++ SecurityContext: &api.SecurityContext{}, ++ }, ++ }, ++ InitContainers: []api.Container{ ++ { ++ Image: "good", ++ SecurityContext: &api.SecurityContext{}, ++ }, ++ }, ++ EphemeralContainers: []api.EphemeralContainer{ ++ { ++ EphemeralContainerCommon: api.EphemeralContainerCommon{ ++ Image: "bad", ++ SecurityContext: &api.SecurityContext{}, ++ }, ++ }, ++ }, ++ }, ++ }, ++ wantAllowed: true, ++ wantErr: false, ++ subresource: "", ++ operation: admission.Update, ++ }, ++ ++ { ++ test: "Bad container, good ephemeral container when updating subresource=='ephemeralcontainers' which sets ephemeralcontainers only", ++ pod: &api.Pod{ ++ Spec: api.PodSpec{ ++ ServiceAccountName: "default", ++ SecurityContext: &api.PodSecurityContext{}, ++ Containers: []api.Container{ ++ { ++ Image: "bad", ++ SecurityContext: &api.SecurityContext{}, ++ }, ++ }, ++ EphemeralContainers: []api.EphemeralContainer{ ++ { ++ EphemeralContainerCommon: api.EphemeralContainerCommon{ ++ Image: "good", ++ SecurityContext: &api.SecurityContext{}, ++ }, ++ }, ++ }, ++ }, ++ }, ++ wantAllowed: true, ++ wantErr: false, ++ subresource: "ephemeralcontainers", ++ operation: admission.Update, ++ }, ++ { ++ test: "Good ephemeral container", ++ pod: &api.Pod{ ++ Spec: api.PodSpec{ ++ ServiceAccountName: "default", ++ SecurityContext: &api.PodSecurityContext{}, ++ EphemeralContainers: []api.EphemeralContainer{ ++ { ++ EphemeralContainerCommon: api.EphemeralContainerCommon{ ++ Image: "good", ++ SecurityContext: &api.SecurityContext{}, ++ }, ++ }, ++ }, ++ }, ++ }, ++ wantAllowed: true, ++ wantErr: false, ++ subresource: "ephemeralcontainers", ++ operation: admission.Update, + }, + } + for _, tt := range tests { +@@ -759,7 +891,7 @@ func TestContainerCombinations(t *testing.T) { + return + } + +- attr := admission.NewAttributesRecord(tt.pod, nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, &user.DefaultInfo{}) ++ attr := admission.NewAttributesRecord(tt.pod, nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), tt.subresource, tt.operation, &metav1.CreateOptions{}, false, &user.DefaultInfo{}) + + err = wh.Validate(context.TODO(), attr, nil) + if tt.wantAllowed { +diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go +index 6a003f75..38ac66ab 100644 +--- a/plugin/pkg/admission/serviceaccount/admission.go ++++ b/plugin/pkg/admission/serviceaccount/admission.go +@@ -109,7 +109,7 @@ var _ = genericadmissioninitializer.WantsExternalKubeInformerFactory(&Plugin{}) + // 5. If MountServiceAccountToken is true, it adds a VolumeMount with the pod's ServiceAccount's api token secret to containers + func NewServiceAccount() *Plugin { + return &Plugin{ +- Handler: admission.NewHandler(admission.Create), ++ Handler: admission.NewHandler(admission.Create, admission.Update), + // TODO: enable this once we've swept secret usage to account for adding secret references to service accounts + LimitSecretReferences: false, + // Auto mount service account API token secrets +@@ -163,7 +163,10 @@ func (s *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission. + if shouldIgnore(a) { + return nil + } +- ++ if a.GetOperation() != admission.Create { ++ // we only mutate pods during create requests ++ return nil ++ } + pod := a.GetObject().(*api.Pod) + + // Don't modify the spec of mirror pods. +@@ -180,7 +183,7 @@ func (s *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission. + + serviceAccount, err := s.getServiceAccount(a.GetNamespace(), pod.Spec.ServiceAccountName) + if err != nil { +- return admission.NewForbidden(a, fmt.Errorf("error looking up service account %s/%s: %v", a.GetNamespace(), pod.Spec.ServiceAccountName, err)) ++ return admission.NewForbidden(a, fmt.Errorf("error looking up service account %s/%s: %w", a.GetNamespace(), pod.Spec.ServiceAccountName, err)) + } + if s.MountServiceAccountToken && shouldAutomount(serviceAccount, pod) { + if err := s.mountServiceAccountToken(serviceAccount, pod); err != nil { +@@ -208,6 +211,15 @@ func (s *Plugin) Validate(ctx context.Context, a admission.Attributes, o admissi + + pod := a.GetObject().(*api.Pod) + ++ if a.GetOperation() == admission.Update && a.GetSubresource() == "ephemeralcontainers" { ++ return s.limitEphemeralContainerSecretReferences(pod, a) ++ } ++ ++ if a.GetOperation() != admission.Create { ++ // we only validate pod specs during create requests ++ return nil ++ } ++ + // Mirror pods have restrictions on what they can reference + if _, isMirrorPod := pod.Annotations[api.MirrorPodAnnotationKey]; isMirrorPod { + if len(pod.Spec.ServiceAccountName) != 0 { +@@ -233,6 +245,10 @@ func (s *Plugin) Validate(ctx context.Context, a admission.Attributes, o admissi + return nil + } + ++ // Require container pods to have service accounts ++ if len(pod.Spec.ServiceAccountName) == 0 { ++ return admission.NewForbidden(a, fmt.Errorf("no service account specified for pod %s/%s", a.GetNamespace(), pod.Name)) ++ } + // Ensure the referenced service account exists + serviceAccount, err := s.getServiceAccount(a.GetNamespace(), pod.Spec.ServiceAccountName) + if err != nil { +@@ -249,10 +265,7 @@ func (s *Plugin) Validate(ctx context.Context, a admission.Attributes, o admissi + } + + func shouldIgnore(a admission.Attributes) bool { +- if a.GetResource().GroupResource() != api.Resource("pods") { +- return true +- } +- if a.GetSubresource() != "" { ++ if a.GetResource().GroupResource() != api.Resource("pods") || (a.GetSubresource() != "" && a.GetSubresource() != "ephemeralcontainers") { + return true + } + obj := a.GetObject() +@@ -424,6 +437,36 @@ func (s *Plugin) limitSecretReferences(serviceAccount *corev1.ServiceAccount, po + return nil + } + ++func (s *Plugin) limitEphemeralContainerSecretReferences(pod *api.Pod, a admission.Attributes) error { ++ // Require ephemeral container pods to have service accounts ++ if len(pod.Spec.ServiceAccountName) == 0 { ++ return admission.NewForbidden(a, fmt.Errorf("no service account specified for pod %s/%s", a.GetNamespace(), pod.Name)) ++ } ++ // Ensure the referenced service account exists ++ serviceAccount, err := s.getServiceAccount(a.GetNamespace(), pod.Spec.ServiceAccountName) ++ if err != nil { ++ return admission.NewForbidden(a, fmt.Errorf("error looking up service account %s/%s: %w", a.GetNamespace(), pod.Spec.ServiceAccountName, err)) ++ } ++ if !s.enforceMountableSecrets(serviceAccount) { ++ return nil ++ } ++ // Ensure all secrets the ephemeral containers reference are allowed by the service account ++ mountableSecrets := sets.NewString() ++ for _, s := range serviceAccount.Secrets { ++ mountableSecrets.Insert(s.Name) ++ } ++ for _, container := range pod.Spec.EphemeralContainers { ++ for _, env := range container.Env { ++ if env.ValueFrom != nil && env.ValueFrom.SecretKeyRef != nil { ++ if !mountableSecrets.Has(env.ValueFrom.SecretKeyRef.Name) { ++ return fmt.Errorf("ephemeral container %s with envVar %s referencing secret.secretName=\"%s\" is not allowed because service account %s does not reference that secret", container.Name, env.Name, env.ValueFrom.SecretKeyRef.Name, serviceAccount.Name) ++ } ++ } ++ } ++ } ++ return nil ++} ++ + func (s *Plugin) mountServiceAccountToken(serviceAccount *corev1.ServiceAccount, pod *api.Pod) error { + var ( + // serviceAccountToken holds the name of a secret containing a legacy service account token +diff --git a/plugin/pkg/admission/serviceaccount/admission_test.go b/plugin/pkg/admission/serviceaccount/admission_test.go +index 77ef50c6..6489b391 100644 +--- a/plugin/pkg/admission/serviceaccount/admission_test.go ++++ b/plugin/pkg/admission/serviceaccount/admission_test.go +@@ -630,6 +630,34 @@ func TestAllowsReferencedSecret(t *testing.T) { + if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err != nil { + t.Errorf("Unexpected error: %v", err) + } ++ ++ pod2 = &api.Pod{ ++ Spec: api.PodSpec{ ++ ServiceAccountName: DefaultServiceAccountName, ++ EphemeralContainers: []api.EphemeralContainer{ ++ { ++ EphemeralContainerCommon: api.EphemeralContainerCommon{ ++ Name: "container-2", ++ Env: []api.EnvVar{ ++ { ++ Name: "env-1", ++ ValueFrom: &api.EnvVarSource{ ++ SecretKeyRef: &api.SecretKeySelector{ ++ LocalObjectReference: api.LocalObjectReference{Name: "foo"}, ++ }, ++ }, ++ }, ++ }, ++ }, ++ }, ++ }, ++ }, ++ } ++ // validate enforces restrictions on secret mounts when operation==create and subresource=='' or operation==update and subresource==ephemeralcontainers" ++ attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "ephemeralcontainers", admission.Update, &metav1.UpdateOptions{}, false, nil) ++ if err := admit.Validate(context.TODO(), attrs, nil); err != nil { ++ t.Errorf("Unexpected error: %v", err) ++ } + } + + func TestRejectsUnreferencedSecretVolumes(t *testing.T) { +@@ -708,6 +736,66 @@ func TestRejectsUnreferencedSecretVolumes(t *testing.T) { + if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envVar") { + t.Errorf("Unexpected error: %v", err) + } ++ ++ pod2 = &api.Pod{ ++ Spec: api.PodSpec{ ++ ServiceAccountName: DefaultServiceAccountName, ++ InitContainers: []api.Container{ ++ { ++ Name: "container-1", ++ Env: []api.EnvVar{ ++ { ++ Name: "env-1", ++ ValueFrom: &api.EnvVarSource{ ++ SecretKeyRef: &api.SecretKeySelector{ ++ LocalObjectReference: api.LocalObjectReference{Name: "foo"}, ++ }, ++ }, ++ }, ++ }, ++ }, ++ }, ++ }, ++ } ++ attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Update, &metav1.UpdateOptions{}, false, nil) ++ if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err != nil { ++ t.Errorf("admit only enforces restrictions on secret mounts when operation==create. Unexpected error: %v", err) ++ } ++ attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, nil) ++ if err := admit.Validate(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envVar") { ++ t.Errorf("validate only enforces restrictions on secret mounts when operation==create and subresource==''. Unexpected error: %v", err) ++ } ++ ++ pod2 = &api.Pod{ ++ Spec: api.PodSpec{ ++ ServiceAccountName: DefaultServiceAccountName, ++ EphemeralContainers: []api.EphemeralContainer{ ++ { ++ EphemeralContainerCommon: api.EphemeralContainerCommon{ ++ Name: "container-2", ++ Env: []api.EnvVar{ ++ { ++ Name: "env-1", ++ ValueFrom: &api.EnvVarSource{ ++ SecretKeyRef: &api.SecretKeySelector{ ++ LocalObjectReference: api.LocalObjectReference{Name: "foo"}, ++ }, ++ }, ++ }, ++ }, ++ }, ++ }, ++ }, ++ }, ++ } ++ attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Update, &metav1.UpdateOptions{}, false, nil) ++ if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err != nil { ++ t.Errorf("admit only enforces restrictions on secret mounts when operation==create and subresource==''. Unexpected error: %v", err) ++ } ++ attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "ephemeralcontainers", admission.Update, &metav1.UpdateOptions{}, false, nil) ++ if err := admit.Validate(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envVar") { ++ t.Errorf("validate enforces restrictions on secret mounts when operation==update and subresource==ephemeralcontainers. Unexpected error: %v", err) ++ } + } + + func TestAllowUnreferencedSecretVolumesForPermissiveSAs(t *testing.T) { +-- +2.25.1 + diff --git a/0016-Add-envFrom-to-serviceaccount-admission-plugin.patch b/0016-Add-envFrom-to-serviceaccount-admission-plugin.patch new file mode 100644 index 0000000000000000000000000000000000000000..9af2e71a06ebd0a5dcb69a3ccc5bfa4d0d1c44b3 --- /dev/null +++ b/0016-Add-envFrom-to-serviceaccount-admission-plugin.patch @@ -0,0 +1,236 @@ +From 3f0922513d235d8bdebe79f0d07da769c04211b8 Mon Sep 17 00:00:00 2001 +From: Rita Zhang +Date: Mon, 25 Mar 2024 10:33:41 -0700 +Subject: [PATCH] Add envFrom to serviceaccount admission plugin + +Signed-off-by: Rita Zhang +--- + .../pkg/admission/serviceaccount/admission.go | 21 +++ + .../serviceaccount/admission_test.go | 122 ++++++++++++++++-- + 2 files changed, 132 insertions(+), 11 deletions(-) + +diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go +index c844a051c24..3f4338128e5 100644 +--- a/plugin/pkg/admission/serviceaccount/admission.go ++++ b/plugin/pkg/admission/serviceaccount/admission.go +@@ -337,6 +337,13 @@ func (s *Plugin) limitSecretReferences(serviceAccount *corev1.ServiceAccount, po + } + } + } ++ for _, envFrom := range container.EnvFrom { ++ if envFrom.SecretRef != nil { ++ if !mountableSecrets.Has(envFrom.SecretRef.Name) { ++ return fmt.Errorf("init container %s with envFrom referencing secret.secretName=\"%s\" is not allowed because service account %s does not reference that secret", container.Name, envFrom.SecretRef.Name, serviceAccount.Name) ++ } ++ } ++ } + } + + for _, container := range pod.Spec.Containers { +@@ -347,6 +354,13 @@ func (s *Plugin) limitSecretReferences(serviceAccount *corev1.ServiceAccount, po + } + } + } ++ for _, envFrom := range container.EnvFrom { ++ if envFrom.SecretRef != nil { ++ if !mountableSecrets.Has(envFrom.SecretRef.Name) { ++ return fmt.Errorf("container %s with envFrom referencing secret.secretName=\"%s\" is not allowed because service account %s does not reference that secret", container.Name, envFrom.SecretRef.Name, serviceAccount.Name) ++ } ++ } ++ } + } + + // limit pull secret references as well +@@ -388,6 +402,13 @@ func (s *Plugin) limitEphemeralContainerSecretReferences(pod *api.Pod, a admissi + } + } + } ++ for _, envFrom := range container.EnvFrom { ++ if envFrom.SecretRef != nil { ++ if !mountableSecrets.Has(envFrom.SecretRef.Name) { ++ return fmt.Errorf("ephemeral container %s with envFrom referencing secret.secretName=\"%s\" is not allowed because service account %s does not reference that secret", container.Name, envFrom.SecretRef.Name, serviceAccount.Name) ++ } ++ } ++ } + } + return nil + } +diff --git a/plugin/pkg/admission/serviceaccount/admission_test.go b/plugin/pkg/admission/serviceaccount/admission_test.go +index bf15f870d75..4dba6cd8b13 100644 +--- a/plugin/pkg/admission/serviceaccount/admission_test.go ++++ b/plugin/pkg/admission/serviceaccount/admission_test.go +@@ -521,6 +521,25 @@ func TestAllowsReferencedSecret(t *testing.T) { + t.Errorf("Unexpected error: %v", err) + } + ++ pod2 = &api.Pod{ ++ Spec: api.PodSpec{ ++ Containers: []api.Container{ ++ { ++ Name: "container-1", ++ EnvFrom: []api.EnvFromSource{ ++ { ++ SecretRef: &api.SecretEnvSource{ ++ LocalObjectReference: api.LocalObjectReference{ ++ Name: "foo"}}}}, ++ }, ++ }, ++ }, ++ } ++ attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, nil) ++ if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err != nil { ++ t.Errorf("Unexpected error: %v", err) ++ } ++ + pod2 = &api.Pod{ + Spec: api.PodSpec{ + InitContainers: []api.Container{ +@@ -545,6 +564,25 @@ func TestAllowsReferencedSecret(t *testing.T) { + t.Errorf("Unexpected error: %v", err) + } + ++ pod2 = &api.Pod{ ++ Spec: api.PodSpec{ ++ InitContainers: []api.Container{ ++ { ++ Name: "container-1", ++ EnvFrom: []api.EnvFromSource{ ++ { ++ SecretRef: &api.SecretEnvSource{ ++ LocalObjectReference: api.LocalObjectReference{ ++ Name: "foo"}}}}, ++ }, ++ }, ++ }, ++ } ++ attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, nil) ++ if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err != nil { ++ t.Errorf("Unexpected error: %v", err) ++ } ++ + pod2 = &api.Pod{ + Spec: api.PodSpec{ + ServiceAccountName: DefaultServiceAccountName, +@@ -572,6 +610,28 @@ func TestAllowsReferencedSecret(t *testing.T) { + if err := admit.Validate(context.TODO(), attrs, nil); err != nil { + t.Errorf("Unexpected error: %v", err) + } ++ ++ pod2 = &api.Pod{ ++ Spec: api.PodSpec{ ++ ServiceAccountName: DefaultServiceAccountName, ++ EphemeralContainers: []api.EphemeralContainer{ ++ { ++ EphemeralContainerCommon: api.EphemeralContainerCommon{ ++ Name: "container-2", ++ EnvFrom: []api.EnvFromSource{{ ++ SecretRef: &api.SecretEnvSource{ ++ LocalObjectReference: api.LocalObjectReference{ ++ Name: "foo"}}}}, ++ }, ++ }, ++ }, ++ }, ++ } ++ // validate enforces restrictions on secret mounts when operation==update and subresource==ephemeralcontainers" ++ attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "ephemeralcontainers", admission.Update, &metav1.UpdateOptions{}, false, nil) ++ if err := admit.Validate(context.TODO(), attrs, nil); err != nil { ++ t.Errorf("Unexpected error: %v", err) ++ } + } + + func TestRejectsUnreferencedSecretVolumes(t *testing.T) { +@@ -628,25 +688,20 @@ func TestRejectsUnreferencedSecretVolumes(t *testing.T) { + + pod2 = &api.Pod{ + Spec: api.PodSpec{ +- InitContainers: []api.Container{ ++ Containers: []api.Container{ + { + Name: "container-1", +- Env: []api.EnvVar{ ++ EnvFrom: []api.EnvFromSource{ + { +- Name: "env-1", +- ValueFrom: &api.EnvVarSource{ +- SecretKeyRef: &api.SecretKeySelector{ +- LocalObjectReference: api.LocalObjectReference{Name: "foo"}, +- }, +- }, +- }, +- }, ++ SecretRef: &api.SecretEnvSource{ ++ LocalObjectReference: api.LocalObjectReference{ ++ Name: "foo"}}}}, + }, + }, + }, + } + attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, nil) +- if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envVar") { ++ if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envFrom") { + t.Errorf("Unexpected error: %v", err) + } + +@@ -679,6 +734,30 @@ func TestRejectsUnreferencedSecretVolumes(t *testing.T) { + t.Errorf("validate only enforces restrictions on secret mounts when operation==create and subresource==''. Unexpected error: %v", err) + } + ++ pod2 = &api.Pod{ ++ Spec: api.PodSpec{ ++ ServiceAccountName: DefaultServiceAccountName, ++ InitContainers: []api.Container{ ++ { ++ Name: "container-1", ++ EnvFrom: []api.EnvFromSource{ ++ { ++ SecretRef: &api.SecretEnvSource{ ++ LocalObjectReference: api.LocalObjectReference{ ++ Name: "foo"}}}}, ++ }, ++ }, ++ }, ++ } ++ attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Update, &metav1.UpdateOptions{}, false, nil) ++ if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err != nil { ++ t.Errorf("admit only enforces restrictions on secret mounts when operation==create. Unexpected error: %v", err) ++ } ++ attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, nil) ++ if err := admit.Validate(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envFrom") { ++ t.Errorf("validate only enforces restrictions on secret mounts when operation==create and subresource==''. Unexpected error: %v", err) ++ } ++ + pod2 = &api.Pod{ + Spec: api.PodSpec{ + ServiceAccountName: DefaultServiceAccountName, +@@ -709,6 +788,27 @@ func TestRejectsUnreferencedSecretVolumes(t *testing.T) { + if err := admit.Validate(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envVar") { + t.Errorf("validate enforces restrictions on secret mounts when operation==update and subresource==ephemeralcontainers. Unexpected error: %v", err) + } ++ ++ pod2 = &api.Pod{ ++ Spec: api.PodSpec{ ++ ServiceAccountName: DefaultServiceAccountName, ++ EphemeralContainers: []api.EphemeralContainer{ ++ { ++ EphemeralContainerCommon: api.EphemeralContainerCommon{ ++ Name: "container-2", ++ EnvFrom: []api.EnvFromSource{{ ++ SecretRef: &api.SecretEnvSource{ ++ LocalObjectReference: api.LocalObjectReference{ ++ Name: "foo"}}}}, ++ }, ++ }, ++ }, ++ }, ++ } ++ attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "ephemeralcontainers", admission.Update, &metav1.UpdateOptions{}, false, nil) ++ if err := admit.Validate(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envFrom") { ++ t.Errorf("validate enforces restrictions on secret mounts when operation==update and subresource==ephemeralcontainers. Unexpected error: %v", err) ++ } + } + + func TestAllowUnreferencedSecretVolumesForPermissiveSAs(t *testing.T) { +-- +2.34.1 + diff --git a/kubernetes.spec b/kubernetes.spec index 2beca7ba0e2c03b7a887379e117f7358a343424c..e9ce2d99d39f82456621525ff695a249c5240a69 100644 --- a/kubernetes.spec +++ b/kubernetes.spec @@ -3,7 +3,7 @@ Name: kubernetes Version: 1.20.2 -Release: 16 +Release: 21 Summary: Container cluster management License: ASL 2.0 URL: https://k8s.io/kubernetes @@ -35,6 +35,11 @@ Patch6007: 0008-kubelet-fix-websocket-reference-nil-pointer.patch Patch6008: 0009-timeout-wait-backend-to-frontend-complete.patch Patch6009: 0010-Escape-terminal-special-characters-in-kubectl-112553.patch Patch6010: 0011-Remove-Endpoints-write-access-from-aggregated-edit-r.patch +Patch6011: 0012-Return-error-for-localhost-seccomp-type-with-no-loca.patch +Patch6012: 0013-Validate-etcd-paths.patch +Patch6013: 0014-fix-node-address-validation.patch +Patch6014: 0015-Add-ephemeralcontainer-to-imagepolicy-securityaccoun.patch +Patch6015: 0016-Add-envFrom-to-serviceaccount-admission-plugin.patch %description Container cluster management. @@ -95,7 +100,7 @@ Summary: Help documents for kubernetes Help documents for kubernetes. %prep -%autosetup -n kubernetes-1.20.2 -Sgit -p1 +%autosetup -n kubernetes-1.20.2 -p1 mkdir -p src/k8s.io/kubernetes mv $(ls | grep -v "^src$") src/k8s.io/kubernetes/. @@ -266,6 +271,36 @@ getent passwd kube >/dev/null || useradd -r -g kube -d / -s /sbin/nologin \ %systemd_postun kubelet kube-proxy %changelog +* Mon Apr 29 2024 liuxu - 1.20.2-21 +- Type:bugfix +- CVE:NA +- SUG:NA +- DESC:fix CVE-2024-3177 + +* Tue Jul 04 2023 zhangxiaoyu - 1.20.2-20 +- Type:bugfix +- CVE:NA +- SUG:NA +- DESC:fix CVE-2023-2727 and CVE-2023-2728 + +* Tue Jul 04 2023 zhangxiaoyu - 1.20.2-19 +- Type:bugfix +- CVE:NA +- SUG:NA +- DESC:fix CVE-2022-3294 + +* Thu Jun 29 2023 zhangxiaoyu - 1.20.2-18 +- Type:bugfix +- CVE:NA +- SUG:NA +- DESC:fix CVE-2022-3162 + +* Thu Jun 29 2023 zhangxiaoyu - 1.20.2-17 +- Type:bugfix +- CVE:NA +- SUG:NA +- DESC:fix CVE-2023-2431 + * Thu Dec 08 2022 zhangxiaoyu - 1.20.2-16 - Type:bugfix - CVE:NA