Merge pull request #11827 from deads2k/make-permissive-sas

pkg/controller/serviceaccount/serviceaccounts_controller.go

@@ -30,7 +30,6 @@ import (
 	"k8s.io/kubernetes/pkg/controller/framework"
 	"k8s.io/kubernetes/pkg/fields"
 	"k8s.io/kubernetes/pkg/runtime"
-	"k8s.io/kubernetes/pkg/util/sets"
 	"k8s.io/kubernetes/pkg/watch"
 )
 
@@ -45,8 +44,8 @@ func nameIndexFunc(obj interface{}) ([]string, error) {
 
 // ServiceAccountsControllerOptions contains options for running a ServiceAccountsController
 type ServiceAccountsControllerOptions struct {
-	// Names is the set of service account names to ensure exist in every namespace
-	Names sets.String
+	// ServiceAccounts is the list of service accounts to ensure exist in every namespace
+	ServiceAccounts []api.ServiceAccount
 
 	// ServiceAccountResync is the interval between full resyncs of ServiceAccounts.
 	// If non-zero, all service accounts will be re-listed this often.
@@ -60,20 +59,24 @@ type ServiceAccountsControllerOptions struct {
 }
 
 func DefaultServiceAccountsControllerOptions() ServiceAccountsControllerOptions {
-	return ServiceAccountsControllerOptions{Names: sets.NewString("default")}
+	return ServiceAccountsControllerOptions{
+		ServiceAccounts: []api.ServiceAccount{
+			{ObjectMeta: api.ObjectMeta{Name: "default"}},
+		},
+	}
 }
 
 // NewServiceAccountsController returns a new *ServiceAccountsController.
 func NewServiceAccountsController(cl client.Interface, options ServiceAccountsControllerOptions) *ServiceAccountsController {
 	e := &ServiceAccountsController{
-		client: cl,
-		names:  options.Names,
+		client:                  cl,
+		serviceAccountsToEnsure: options.ServiceAccounts,
 	}
 
 	accountSelector := fields.Everything()
-	if len(options.Names) == 1 {
+	if len(options.ServiceAccounts) == 1 {
 		// If we're maintaining a single account, we can scope the accounts we watch to just that name
-		accountSelector = fields.SelectorFromSet(map[string]string{client.ObjectNameField: options.Names.List()[0]})
+		accountSelector = fields.SelectorFromSet(map[string]string{client.ObjectNameField: options.ServiceAccounts[0].Name})
 	}
 	e.serviceAccounts, e.serviceAccountController = framework.NewIndexerInformer(
 		&cache.ListWatch{
@@ -119,8 +122,8 @@ func NewServiceAccountsController(cl client.Interface, options ServiceAccountsCo
 type ServiceAccountsController struct {
 	stopChan chan struct{}
 
-	client client.Interface
-	names  sets.String
+	client                  client.Interface
+	serviceAccountsToEnsure []api.ServiceAccount
 
 	serviceAccounts cache.Indexer
 	namespaces      cache.Indexer
@@ -156,24 +159,26 @@ func (e *ServiceAccountsController) serviceAccountDeleted(obj interface{}) {
 		return
 	}
 	// If the deleted service account is one we're maintaining, recreate it
-	if e.names.Has(serviceAccount.Name) {
-		e.createServiceAccountIfNeeded(serviceAccount.Name, serviceAccount.Namespace)
+	for _, sa := range e.serviceAccountsToEnsure {
+		if sa.Name == serviceAccount.Name {
+			e.createServiceAccountIfNeeded(sa, serviceAccount.Namespace)
+		}
 	}
 }
 
 // namespaceAdded reacts to a Namespace creation by creating a default ServiceAccount object
 func (e *ServiceAccountsController) namespaceAdded(obj interface{}) {
 	namespace := obj.(*api.Namespace)
-	for _, name := range e.names.List() {
-		e.createServiceAccountIfNeeded(name, namespace.Name)
+	for _, sa := range e.serviceAccountsToEnsure {
+		e.createServiceAccountIfNeeded(sa, namespace.Name)
 	}
 }
 
 // namespaceUpdated reacts to a Namespace update (or re-list) by creating a default ServiceAccount in the namespace if needed
 func (e *ServiceAccountsController) namespaceUpdated(oldObj interface{}, newObj interface{}) {
 	newNamespace := newObj.(*api.Namespace)
-	for _, name := range e.names.List() {
-		e.createServiceAccountIfNeeded(name, newNamespace.Name)
+	for _, sa := range e.serviceAccountsToEnsure {
+		e.createServiceAccountIfNeeded(sa, newNamespace.Name)
 	}
 }
 
@@ -181,13 +186,13 @@ func (e *ServiceAccountsController) namespaceUpdated(oldObj interface{}, newObj
 // * the named ServiceAccount does not already exist
 // * the specified namespace exists
 // * the specified namespace is in the ACTIVE phase
-func (e *ServiceAccountsController) createServiceAccountIfNeeded(name, namespace string) {
-	serviceAccount, err := e.getServiceAccount(name, namespace)
+func (e *ServiceAccountsController) createServiceAccountIfNeeded(sa api.ServiceAccount, namespace string) {
+	existingServiceAccount, err := e.getServiceAccount(sa.Name, namespace)
 	if err != nil {
 		glog.Error(err)
 		return
 	}
-	if serviceAccount != nil {
+	if existingServiceAccount != nil {
 		// If service account already exists, it doesn't need to be created
 		return
 	}
@@ -206,16 +211,13 @@ func (e *ServiceAccountsController) createServiceAccountIfNeeded(name, namespace
 		return
 	}
 
-	e.createServiceAccount(name, namespace)
+	e.createServiceAccount(sa, namespace)
 }
 
-// createServiceAccount creates a ServiceAccount with the specified name and namespace
-func (e *ServiceAccountsController) createServiceAccount(name, namespace string) {
-	serviceAccount := &api.ServiceAccount{}
-	serviceAccount.Name = name
-	serviceAccount.Namespace = namespace
-	_, err := e.client.ServiceAccounts(namespace).Create(serviceAccount)
-	if err != nil && !apierrs.IsAlreadyExists(err) {
+// createDefaultServiceAccount creates a default ServiceAccount in the specified namespace
+func (e *ServiceAccountsController) createServiceAccount(sa api.ServiceAccount, namespace string) {
+	sa.Namespace = namespace
+	if _, err := e.client.ServiceAccounts(namespace).Create(&sa); err != nil && !apierrs.IsAlreadyExists(err) {
 		glog.Error(err)
 	}
 }

pkg/controller/serviceaccount/serviceaccounts_controller_test.go

@@ -151,7 +151,10 @@ func TestServiceAccountCreation(t *testing.T) {
 	for k, tc := range testcases {
 		client := testclient.NewSimpleFake(defaultServiceAccount, managedServiceAccount)
 		options := DefaultServiceAccountsControllerOptions()
-		options.Names = sets.NewString(defaultName, managedName)
+		options.ServiceAccounts = []api.ServiceAccount{
+			{ObjectMeta: api.ObjectMeta{Name: defaultName}},
+			{ObjectMeta: api.ObjectMeta{Name: managedName}},
+		}
 		controller := NewServiceAccountsController(client, options)
 
 		if tc.ExistingNamespace != nil {

plugin/pkg/admission/serviceaccount/admission.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"io"
 	"math/rand"
+	"strconv"
 	"time"
 
 	"k8s.io/kubernetes/pkg/admission"
@@ -39,12 +40,19 @@ import (
 // DefaultServiceAccountName is the name of the default service account to set on pods which do not specify a service account
 const DefaultServiceAccountName = "default"
 
+// EnforceMountableSecretsAnnotation is a default annotation that indicates that a service account should enforce mountable secrets.
+// The value must be true to have this annotation take effect
+const EnforceMountableSecretsAnnotation = "kubernetes.io/enforce-mountable-secrets"
+
 // DefaultAPITokenMountPath is the path that ServiceAccountToken secrets are automounted to.
 // The token file would then be accessible at /var/run/secrets/kubernetes.io/serviceaccount
 const DefaultAPITokenMountPath = "/var/run/secrets/kubernetes.io/serviceaccount"
 
+// PluginName is the name of this admission plugin
+const PluginName = "ServiceAccount"
+
 func init() {
-	admission.RegisterPlugin("ServiceAccount", func(client client.Interface, config io.Reader) (admission.Interface, error) {
+	admission.RegisterPlugin(PluginName, func(client client.Interface, config io.Reader) (admission.Interface, error) {
 		serviceAccountAdmission := NewServiceAccount(client)
 		serviceAccountAdmission.Run()
 		return serviceAccountAdmission, nil
@@ -183,7 +191,7 @@ func (s *serviceAccount) Admit(a admission.Attributes) (err error) {
 		return admission.NewForbidden(a, fmt.Errorf("service account %s/%s was not found, retry after the service account is created", a.GetNamespace(), pod.Spec.ServiceAccountName))
 	}
 
-	if s.LimitSecretReferences {
+	if s.enforceMountableSecrets(serviceAccount) {
 		if err := s.limitSecretReferences(serviceAccount, pod); err != nil {
 			return admission.NewForbidden(a, err)
 		}
@@ -203,6 +211,21 @@ func (s *serviceAccount) Admit(a admission.Attributes) (err error) {
 	return nil
 }
 
+// enforceMountableSecrets indicates whether mountable secrets should be enforced for a particular service account
+// A global setting of true will override any flag set on the individual service account
+func (s *serviceAccount) enforceMountableSecrets(serviceAccount *api.ServiceAccount) bool {
+	if s.LimitSecretReferences {
+		return true
+	}
+
+	if value, ok := serviceAccount.Annotations[EnforceMountableSecretsAnnotation]; ok {
+		enforceMountableSecretCheck, _ := strconv.ParseBool(value)
+		return enforceMountableSecretCheck
+	}
+
+	return false
+}
+
 // getServiceAccount returns the ServiceAccount for the given namespace and name if it exists
 func (s *serviceAccount) getServiceAccount(namespace string, name string) (*api.ServiceAccount, error) {
 	key := &api.ServiceAccount{ObjectMeta: api.ObjectMeta{Namespace: namespace}}

plugin/pkg/admission/serviceaccount/admission_test.go

@@ -428,6 +428,36 @@ func TestRejectsUnreferencedSecretVolumes(t *testing.T) {
 	}
 }
 
+func TestAllowUnreferencedSecretVolumesForPermissiveSAs(t *testing.T) {
+	ns := "myns"
+
+	admit := NewServiceAccount(nil)
+	admit.LimitSecretReferences = false
+	admit.RequireAPIToken = false
+
+	// Add the default service account for the ns into the cache
+	admit.serviceAccounts.Add(&api.ServiceAccount{
+		ObjectMeta: api.ObjectMeta{
+			Name:        DefaultServiceAccountName,
+			Namespace:   ns,
+			Annotations: map[string]string{EnforceMountableSecretsAnnotation: "true"},
+		},
+	})
+
+	pod := &api.Pod{
+		Spec: api.PodSpec{
+			Volumes: []api.Volume{
+				{VolumeSource: api.VolumeSource{Secret: &api.SecretVolumeSource{SecretName: "foo"}}},
+			},
+		},
+	}
+	attrs := admission.NewAttributesRecord(pod, "Pod", ns, "myname", string(api.ResourcePods), "", admission.Create, nil)
+	err := admit.Admit(attrs)
+	if err == nil {
+		t.Errorf("Expected rejection for using a secret the service account does not reference")
+	}
+}
+
 func TestAllowsReferencedImagePullSecrets(t *testing.T) {
 	ns := "myns"