Merge pull request kubernetes#30257 from timstclair/aa-psp-annotations

Automatic merge from submit-queue Add annotations to the PodSecurityPolicy Provider interface @pweil- is this what you were thinking in terms of API changes? I really like to avoid functions with more than 2 return values, but couldn't think of a cleaner approach in this case.
lavalamp · Aug 18, 2016 · dc588e4 · dc588e4
2 parents b5a85eb + c99d7fd
commit dc588e4
Show file tree

Hide file tree

Showing 7 changed files with 80 additions and 20 deletions.
diff --git a/hack/.linted_packages b/hack/.linted_packages
@@ -197,3 +197,4 @@ test/integration/openshift
 test/soak/cauldron
 test/soak/serve_hostnames
 third_party/forked/golang/expansion
+pkg/util/maps
diff --git a/pkg/security/podsecuritypolicy/provider.go b/pkg/security/podsecuritypolicy/provider.go
@@ -22,6 +22,7 @@ import (
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/apis/extensions"
 	psputil "k8s.io/kubernetes/pkg/security/podsecuritypolicy/util"
+	"k8s.io/kubernetes/pkg/util/maps"
 	"k8s.io/kubernetes/pkg/util/validation/field"
 )
 
@@ -67,7 +68,7 @@ func NewSimpleProvider(psp *extensions.PodSecurityPolicy, namespace string, stra
 //
 // NOTE: this method works on a copy of the PodSecurityContext.  It is up to the caller to
 // apply the PSC if validation passes.
-func (s *simpleProvider) CreatePodSecurityContext(pod *api.Pod) (*api.PodSecurityContext, error) {
+func (s *simpleProvider) CreatePodSecurityContext(pod *api.Pod) (*api.PodSecurityContext, map[string]string, error) {
 	var sc *api.PodSecurityContext = nil
 	if pod.Spec.SecurityContext != nil {
 		// work with a copy
@@ -76,32 +77,33 @@ func (s *simpleProvider) CreatePodSecurityContext(pod *api.Pod) (*api.PodSecurit
 	} else {
 		sc = &api.PodSecurityContext{}
 	}
+	annotations := maps.CopySS(pod.Annotations)
 
 	if len(sc.SupplementalGroups) == 0 {
 		supGroups, err := s.strategies.SupplementalGroupStrategy.Generate(pod)
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		sc.SupplementalGroups = supGroups
 	}
 
 	if sc.FSGroup == nil {
 		fsGroup, err := s.strategies.FSGroupStrategy.GenerateSingle(pod)
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		sc.FSGroup = fsGroup
 	}
 
 	if sc.SELinuxOptions == nil {
 		seLinux, err := s.strategies.SELinuxStrategy.Generate(pod, nil)
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		sc.SELinuxOptions = seLinux
 	}
 
-	return sc, nil
+	return sc, annotations, nil
 }
 
 // Create a SecurityContext based on the given constraints.  If a setting is already set on the
@@ -110,7 +112,7 @@ func (s *simpleProvider) CreatePodSecurityContext(pod *api.Pod) (*api.PodSecurit
 //
 // NOTE: this method works on a copy of the SC of the container.  It is up to the caller to apply
 // the SC if validation passes.
-func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container *api.Container) (*api.SecurityContext, error) {
+func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container *api.Container) (*api.SecurityContext, map[string]string, error) {
 	var sc *api.SecurityContext = nil
 	if container.SecurityContext != nil {
 		// work with a copy of the original
@@ -119,18 +121,20 @@ func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container
 	} else {
 		sc = &api.SecurityContext{}
 	}
+	annotations := maps.CopySS(pod.Annotations)
+
 	if sc.RunAsUser == nil {
 		uid, err := s.strategies.RunAsUserStrategy.Generate(pod, container)
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		sc.RunAsUser = uid
 	}
 
 	if sc.SELinuxOptions == nil {
 		seLinux, err := s.strategies.SELinuxStrategy.Generate(pod, container)
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		sc.SELinuxOptions = seLinux
 	}
@@ -150,7 +154,7 @@ func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container
 
 	caps, err := s.strategies.CapabilitiesStrategy.Generate(pod, container)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	sc.Capabilities = caps
 
@@ -161,7 +165,7 @@ func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container
 		sc.ReadOnlyRootFilesystem = &readOnlyRootFS
 	}
 
-	return sc, nil
+	return sc, annotations, nil
 }
 
 // Ensure a pod's SecurityContext is in compliance with the given constraints.

diff --git a/pkg/security/podsecuritypolicy/provider_test.go b/pkg/security/podsecuritypolicy/provider_test.go
@@ -78,7 +78,7 @@ func TestCreatePodSecurityContextNonmutating(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unable to create provider %v", err)
 	}
-	sc, err := provider.CreatePodSecurityContext(pod)
+	sc, _, err := provider.CreatePodSecurityContext(pod)
 	if err != nil {
 		t.Fatalf("unable to create psc %v", err)
 	}
@@ -148,7 +148,7 @@ func TestCreateContainerSecurityContextNonmutating(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unable to create provider %v", err)
 	}
-	sc, err := provider.CreateContainerSecurityContext(pod, &pod.Spec.Containers[0])
+	sc, _, err := provider.CreateContainerSecurityContext(pod, &pod.Spec.Containers[0])
 	if err != nil {
 		t.Fatalf("unable to create container security context %v", err)
 	}
@@ -698,7 +698,7 @@ func TestGenerateContainerSecurityContextReadOnlyRootFS(t *testing.T) {
 			t.Errorf("%s unable to create provider %v", k, err)
 			continue
 		}
-		sc, err := provider.CreateContainerSecurityContext(v.pod, &v.pod.Spec.Containers[0])
+		sc, _, err := provider.CreateContainerSecurityContext(v.pod, &v.pod.Spec.Containers[0])
 		if err != nil {
 			t.Errorf("%s unable to create container security context %v", k, err)
 			continue

diff --git a/pkg/security/podsecuritypolicy/types.go b/pkg/security/podsecuritypolicy/types.go
@@ -29,10 +29,12 @@ import (
 // Provider provides the implementation to generate a new security
 // context based on constraints or validate an existing security context against constraints.
 type Provider interface {
-	// Create a PodSecurityContext based on the given constraints.
-	CreatePodSecurityContext(pod *api.Pod) (*api.PodSecurityContext, error)
-	// Create a container SecurityContext based on the given constraints
-	CreateContainerSecurityContext(pod *api.Pod, container *api.Container) (*api.SecurityContext, error)
+	// Create a PodSecurityContext based on the given constraints. Also returns an updated set
+	// of Pod annotations for alpha feature support.
+	CreatePodSecurityContext(pod *api.Pod) (*api.PodSecurityContext, map[string]string, error)
+	// Create a container SecurityContext based on the given constraints. Also returns an updated set
+	// of Pod annotations for alpha feature support.
+	CreateContainerSecurityContext(pod *api.Pod, container *api.Container) (*api.SecurityContext, map[string]string, error)
 	// Ensure a pod's SecurityContext is in compliance with the given constraints.
 	ValidatePodSecurityContext(pod *api.Pod, fldPath *field.Path) field.ErrorList
 	// Ensure a container's SecurityContext is in compliance with the given constraints

diff --git a/pkg/util/maps/doc.go b/pkg/util/maps/doc.go
@@ -0,0 +1,18 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package maps has common methods for dealing with common map types.
+package maps
diff --git a/pkg/util/maps/string.go b/pkg/util/maps/string.go
@@ -0,0 +1,29 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package maps
+
+// CopySS makes a shallow copy of a map.
+func CopySS(m map[string]string) map[string]string {
+	if m == nil {
+		return nil
+	}
+	copy := make(map[string]string, len(m))
+	for k, v := range m {
+		copy[k] = v
+	}
+	return copy
+}
diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission.go b/plugin/pkg/admission/security/podsecuritypolicy/admission.go
@@ -35,6 +35,7 @@ import (
 	psputil "k8s.io/kubernetes/pkg/security/podsecuritypolicy/util"
 	sc "k8s.io/kubernetes/pkg/securitycontext"
 	"k8s.io/kubernetes/pkg/serviceaccount"
+	"k8s.io/kubernetes/pkg/util/maps"
 	"k8s.io/kubernetes/pkg/util/validation/field"
 	"k8s.io/kubernetes/pkg/watch"
 )
@@ -190,7 +191,7 @@ func assignSecurityContext(provider psp.Provider, pod *api.Pod, fldPath *field.P
 
 	errs := field.ErrorList{}
 
-	psc, err := provider.CreatePodSecurityContext(pod)
+	psc, pscAnnotations, err := provider.CreatePodSecurityContext(pod)
 	if err != nil {
 		errs = append(errs, field.Invalid(field.NewPath("spec", "securityContext"), pod.Spec.SecurityContext, err.Error()))
 	}
@@ -200,6 +201,8 @@ func assignSecurityContext(provider psp.Provider, pod *api.Pod, fldPath *field.P
 	// validation.
 	originalPSC := pod.Spec.SecurityContext
 	pod.Spec.SecurityContext = psc
+	originalAnnotations := maps.CopySS(pod.Annotations)
+	pod.Annotations = pscAnnotations
 	errs = append(errs, provider.ValidatePodSecurityContext(pod, field.NewPath("spec", "securityContext"))...)
 
 	// Note: this is not changing the original container, we will set container SCs later so long
@@ -211,14 +214,15 @@ func assignSecurityContext(provider psp.Provider, pod *api.Pod, fldPath *field.P
 		// overriding pod level settings.
 		containerCopy.SecurityContext = sc.DetermineEffectiveSecurityContext(pod, &containerCopy)
 
-		sc, err := provider.CreateContainerSecurityContext(pod, &containerCopy)
+		sc, scAnnotations, err := provider.CreateContainerSecurityContext(pod, &containerCopy)
 		if err != nil {
 			errs = append(errs, field.Invalid(field.NewPath("spec", "initContainers").Index(i).Child("securityContext"), "", err.Error()))
 			continue
 		}
 		generatedInitSCs = append(generatedInitSCs, sc)
 
 		containerCopy.SecurityContext = sc
+		pod.Annotations = scAnnotations
 		errs = append(errs, provider.ValidateContainerSecurityContext(pod, &containerCopy, field.NewPath("spec", "initContainers").Index(i).Child("securityContext"))...)
 	}
 
@@ -231,20 +235,22 @@ func assignSecurityContext(provider psp.Provider, pod *api.Pod, fldPath *field.P
 		// overriding pod level settings.
 		containerCopy.SecurityContext = sc.DetermineEffectiveSecurityContext(pod, &containerCopy)
 
-		sc, err := provider.CreateContainerSecurityContext(pod, &containerCopy)
+		sc, scAnnotations, err := provider.CreateContainerSecurityContext(pod, &containerCopy)
 		if err != nil {
 			errs = append(errs, field.Invalid(field.NewPath("spec", "containers").Index(i).Child("securityContext"), "", err.Error()))
 			continue
 		}
 		generatedSCs[i] = sc
 
 		containerCopy.SecurityContext = sc
+		pod.Annotations = scAnnotations
 		errs = append(errs, provider.ValidateContainerSecurityContext(pod, &containerCopy, field.NewPath("spec", "containers").Index(i).Child("securityContext"))...)
 	}
 
 	if len(errs) > 0 {
 		// ensure psc is not mutated if there are errors
 		pod.Spec.SecurityContext = originalPSC
+		pod.Annotations = originalAnnotations
 		return errs
 	}