Merge pull request #4704 from KnVerey/ns_transformer_options

Option to customize NamespaceTransformer role binding subject handling

api/filters/namespace/namespace.go

@@ -7,6 +7,7 @@ import (
 	"sigs.k8s.io/kustomize/api/filters/filtersutil"
 	"sigs.k8s.io/kustomize/api/filters/fsslice"
 	"sigs.k8s.io/kustomize/api/types"
+	"sigs.k8s.io/kustomize/kyaml/errors"
 	"sigs.k8s.io/kustomize/kyaml/kio"
 	"sigs.k8s.io/kustomize/kyaml/resid"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
@@ -22,9 +23,25 @@ type Filter struct {
 	// UnsetOnly means only blank namespace fields will be set
 	UnsetOnly bool `json:"unsetOnly" yaml:"unsetOnly"`
 
+	// SetRoleBindingSubjects determines which subject fields in RoleBinding and ClusterRoleBinding
+	// objects will have their namespace fields set. Overrides field specs provided for these types, if any.
+	// - defaultOnly (default): namespace will be set only on subjects named "default".
+	// - allServiceAccounts: namespace will be set on all subjects with "kind: ServiceAccount"
+	// - none: all subjects will be skipped.
+	SetRoleBindingSubjects RoleBindingSubjectMode `json:"setRoleBindingSubjects" yaml:"setRoleBindingSubjects"`
+
 	trackableSetter filtersutil.TrackableSetter
 }
 
+type RoleBindingSubjectMode string
+
+const (
+	DefaultSubjectsOnly       RoleBindingSubjectMode = "defaultOnly"
+	SubjectModeUnspecified    RoleBindingSubjectMode = ""
+	AllServiceAccountSubjects RoleBindingSubjectMode = "allServiceAccounts"
+	NoSubjects                RoleBindingSubjectMode = "none"
+)
+
 var _ kio.Filter = Filter{}
 var _ kio.TrackableFilter = &Filter{}
 
@@ -47,10 +64,10 @@ func (ns Filter) run(node *yaml.RNode) (*yaml.RNode, error) {
 		return nil, err
 	}
 
-	// Special handling for (cluster) role binding -- :(
+	// Special handling for (cluster) role binding subjects -- :(
 	if isRoleBinding(gvk.Kind) {
-		ns.FsSlice = ns.removeRoleBindingFieldSpecs(ns.FsSlice)
+		ns.FsSlice = ns.removeRoleBindingSubjectFieldSpecs(ns.FsSlice)
-		if err := ns.roleBindingHack(node, gvk); err != nil {
+		if err := ns.roleBindingHack(node); err != nil {
 			return nil, err
 		}
 	}
@@ -82,12 +99,16 @@ func (ns Filter) metaNamespaceHack(obj *yaml.RNode, gvk resid.Gvk) error {
 	return err
 }
 
-// roleBindingHack is a hack for implementing the transformer's DefaultSubjectsOnly mode
+// roleBindingHack is a hack for implementing the transformer's SetRoleBindingSubjects option
 // for RoleBinding and ClusterRoleBinding resource types.
-// In this mode, RoleBinding and ClusterRoleBinding have namespace set on
+//
+// In NoSubjects mode, it does nothing.
+//
+// In AllServiceAccountSubjects mode, it sets the namespace on subjects with "kind: ServiceAccount".
+//
+// In DefaultSubjectsOnly mode (default mode), RoleBinding and ClusterRoleBinding have namespace set on
 // elements of the "subjects" field if and only if the subject elements
 // "name" is "default".  Otherwise the namespace is not set.
-//
 // Example:
 //
 // kind: RoleBinding
@@ -96,53 +117,65 @@ func (ns Filter) metaNamespaceHack(obj *yaml.RNode, gvk resid.Gvk) error {
 //   ...
 // - name: "something-else" # this will not have the namespace set
 //   ...
-func (ns Filter) roleBindingHack(obj *yaml.RNode, gvk resid.Gvk) error {
+func (ns Filter) roleBindingHack(obj *yaml.RNode) error {
-	if !isRoleBinding(gvk.Kind) {
+	var visitor filtersutil.SetFn
+	switch ns.SetRoleBindingSubjects {
+	case NoSubjects:
 		return nil
+	case DefaultSubjectsOnly, SubjectModeUnspecified:
+		visitor = ns.setSubjectsNamedDefault
+	case AllServiceAccountSubjects:
+		visitor = ns.setServiceAccountNamespaces
+	default:
+		return errors.Errorf("invalid value %q for setRoleBindingSubjects: "+
+			"must be one of %q, %q or %q", ns.SetRoleBindingSubjects,
+			DefaultSubjectsOnly, NoSubjects, AllServiceAccountSubjects)
 	}
 
-	// Lookup the namespace field on all elements.
+	// Lookup the subjects field on all elements.
 	obj, err := obj.Pipe(yaml.Lookup(subjectsField))
 	if err != nil || yaml.IsMissingOrNull(obj) {
 		return err
 	}
-
+	// Use the appropriate visitor to set the namespace field on the correct subset of subjects
-	// add the namespace to each "subject" with name: default
+	return errors.WrapPrefixf(obj.VisitElements(visitor), "setting namespace on (cluster)role binding subjects")
-	err = obj.VisitElements(func(o *yaml.RNode) error {
-		// The only case we need to force the namespace
-		// if for the "service account". "default" is
-		// kind of hardcoded here for right now.
-		name, err := o.Pipe(
-			yaml.Lookup("name"), yaml.Match("default"),
-		)
-		if err != nil || yaml.IsMissingOrNull(name) {
-			return err
-		}
-
-		// set the namespace for the default account
-		node, err := o.Pipe(
-			yaml.LookupCreate(yaml.ScalarNode, "namespace"),
-		)
-		if err != nil {
-			return err
-		}
-
-		return ns.fieldSetter()(node)
-	})
-
-	return err
 }
 
 func isRoleBinding(kind string) bool {
 	return kind == roleBindingKind || kind == clusterRoleBindingKind
 }
 
-// removeRoleBindingFieldSpecs removes from the list fieldspecs that
+func (ns Filter) setServiceAccountNamespaces(o *yaml.RNode) error {
+	name, err := o.Pipe(yaml.Lookup("kind"), yaml.Match("ServiceAccount"))
+	if err != nil || yaml.IsMissingOrNull(name) {
+		return errors.WrapPrefixf(err, "looking up kind on (cluster)role binding subject")
+	}
+	return setNamespaceField(o, ns.fieldSetter())
+}
+
+func (ns Filter) setSubjectsNamedDefault(o *yaml.RNode) error {
+	name, err := o.Pipe(yaml.Lookup("name"), yaml.Match("default"))
+	if err != nil || yaml.IsMissingOrNull(name) {
+		return errors.WrapPrefixf(err, "looking up name on (cluster)role binding subject")
+	}
+	return setNamespaceField(o, ns.fieldSetter())
+}
+
+func setNamespaceField(node *yaml.RNode, setter filtersutil.SetFn) error {
+	node, err := node.Pipe(yaml.LookupCreate(yaml.ScalarNode, "namespace"))
+	if err != nil {
+		return errors.WrapPrefixf(err, "setting namespace field on (cluster)role binding subject")
+	}
+	return setter(node)
+}
+
+// removeRoleBindingSubjectFieldSpecs removes from the list fieldspecs that
 // have hardcoded implementations
-func (ns Filter) removeRoleBindingFieldSpecs(fs types.FsSlice) types.FsSlice {
+func (ns Filter) removeRoleBindingSubjectFieldSpecs(fs types.FsSlice) types.FsSlice {
 	var val types.FsSlice
 	for i := range fs {
-		if isRoleBinding(fs[i].Kind) && fs[i].Path == subjectsField {
+		if isRoleBinding(fs[i].Kind) &&
+			(fs[i].Path == subjectsNamespacePath || fs[i].Path == subjectsField) {
 			continue
 		}
 		val = append(val, fs[i])
@@ -170,6 +203,7 @@ func (ns *Filter) fieldSetter() filtersutil.SetFn {
 
 const (
 	subjectsField          = "subjects"
+	subjectsNamespacePath  = "subjects/namespace"
 	roleBindingKind        = "RoleBinding"
 	clusterRoleBindingKind = "ClusterRoleBinding"
 )

api/internal/builtins/NamespaceTransformer.go

@@ -9,21 +9,37 @@ import (
 	"sigs.k8s.io/kustomize/api/filters/namespace"
 	"sigs.k8s.io/kustomize/api/resmap"
 	"sigs.k8s.io/kustomize/api/types"
+	"sigs.k8s.io/kustomize/kyaml/errors"
 	"sigs.k8s.io/yaml"
 )
 
 // Change or set the namespace of non-cluster level resources.
 type NamespaceTransformerPlugin struct {
-	types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`
+	types.ObjectMeta       `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`
-	FieldSpecs       []types.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"`
+	FieldSpecs             []types.FieldSpec                `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"`
-	UnsetOnly        bool              `json:"unsetOnly" yaml:"unsetOnly"`
+	UnsetOnly              bool                             `json:"unsetOnly" yaml:"unsetOnly"`
+	SetRoleBindingSubjects namespace.RoleBindingSubjectMode `json:"setRoleBindingSubjects" yaml:"setRoleBindingSubjects"`
 }
 
 func (p *NamespaceTransformerPlugin) Config(
 	_ *resmap.PluginHelpers, c []byte) (err error) {
 	p.Namespace = ""
 	p.FieldSpecs = nil
-	return yaml.Unmarshal(c, p)
+	if err := yaml.Unmarshal(c, p); err != nil {
+		return errors.WrapPrefixf(err, "unmarshalling NamespaceTransformer config")
+	}
+	switch p.SetRoleBindingSubjects {
+	case namespace.AllServiceAccountSubjects, namespace.DefaultSubjectsOnly, namespace.NoSubjects:
+		// valid
+	case namespace.SubjectModeUnspecified:
+		p.SetRoleBindingSubjects = namespace.DefaultSubjectsOnly
+	default:
+		return errors.Errorf("invalid value %q for setRoleBindingSubjects: "+
+			"must be one of %q, %q or %q", p.SetRoleBindingSubjects,
+			namespace.DefaultSubjectsOnly, namespace.NoSubjects, namespace.AllServiceAccountSubjects)
+	}
+
+	return nil
 }
 
 func (p *NamespaceTransformerPlugin) Transform(m resmap.ResMap) error {
@@ -37,9 +53,10 @@ func (p *NamespaceTransformerPlugin) Transform(m resmap.ResMap) error {
 		}
 		r.StorePreviousId()
 		if err := r.ApplyFilter(namespace.Filter{
-			Namespace: p.Namespace,
+			Namespace:              p.Namespace,
-			FsSlice:   p.FieldSpecs,
+			FsSlice:                p.FieldSpecs,
-			UnsetOnly: p.UnsetOnly,
+			SetRoleBindingSubjects: p.SetRoleBindingSubjects,
+			UnsetOnly:              p.UnsetOnly,
 		}); err != nil {
 			return err
 		}

api/konfig/builtinpluginconsts/namespace.go

@@ -6,15 +6,9 @@ package builtinpluginconsts
 const (
 	namespaceFieldSpecs = `
 namespace:
-- path: metadata/namespace
-  create: true
 - path: metadata/name
   kind: Namespace
   create: true
-- path: subjects
-  kind: RoleBinding
-- path: subjects
-  kind: ClusterRoleBinding
 - path: spec/service/namespace
   group: apiregistration.k8s.io
   kind: APIService

plugin/builtin/namespacetransformer/NamespaceTransformer.go

@@ -10,14 +10,16 @@ import (
 	"sigs.k8s.io/kustomize/api/filters/namespace"
 	"sigs.k8s.io/kustomize/api/resmap"
 	"sigs.k8s.io/kustomize/api/types"
+	"sigs.k8s.io/kustomize/kyaml/errors"
 	"sigs.k8s.io/yaml"
 )
 
 // Change or set the namespace of non-cluster level resources.
 type plugin struct {
-	types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`
+	types.ObjectMeta       `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`
-	FieldSpecs       []types.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"`
+	FieldSpecs             []types.FieldSpec                `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"`
-	UnsetOnly        bool              `json:"unsetOnly" yaml:"unsetOnly"`
+	UnsetOnly              bool                             `json:"unsetOnly" yaml:"unsetOnly"`
+	SetRoleBindingSubjects namespace.RoleBindingSubjectMode `json:"setRoleBindingSubjects" yaml:"setRoleBindingSubjects"`
 }
 
 //noinspection GoUnusedGlobalVariable
@@ -27,7 +29,21 @@ func (p *plugin) Config(
 	_ *resmap.PluginHelpers, c []byte) (err error) {
 	p.Namespace = ""
 	p.FieldSpecs = nil
-	return yaml.Unmarshal(c, p)
+	if err := yaml.Unmarshal(c, p); err != nil {
+		return errors.WrapPrefixf(err, "unmarshalling NamespaceTransformer config")
+	}
+	switch p.SetRoleBindingSubjects {
+	case namespace.AllServiceAccountSubjects, namespace.DefaultSubjectsOnly, namespace.NoSubjects:
+		// valid
+	case namespace.SubjectModeUnspecified:
+		p.SetRoleBindingSubjects = namespace.DefaultSubjectsOnly
+	default:
+		return errors.Errorf("invalid value %q for setRoleBindingSubjects: "+
+			"must be one of %q, %q or %q", p.SetRoleBindingSubjects,
+			namespace.DefaultSubjectsOnly, namespace.NoSubjects, namespace.AllServiceAccountSubjects)
+	}
+
+	return nil
 }
 
 func (p *plugin) Transform(m resmap.ResMap) error {
@@ -41,9 +57,10 @@ func (p *plugin) Transform(m resmap.ResMap) error {
 		}
 		r.StorePreviousId()
 		if err := r.ApplyFilter(namespace.Filter{
-			Namespace: p.Namespace,
+			Namespace:              p.Namespace,
-			FsSlice:   p.FieldSpecs,
+			FsSlice:                p.FieldSpecs,
-			UnsetOnly: p.UnsetOnly,
+			SetRoleBindingSubjects: p.SetRoleBindingSubjects,
+			UnsetOnly:              p.UnsetOnly,
 		}); err != nil {
 			return err
 		}

plugin/builtin/namespacetransformer/NamespaceTransformer_test.go

@@ -14,6 +14,12 @@ const defaultFieldSpecs = `
 fieldSpecs:
 - path: metadata/namespace
   create: true
+- path: subjects/namespace
+  kind: RoleBinding
+  group: rbac.authorization.k8s.io
+- path: subjects/namespace
+  kind: ClusterRoleBinding
+  group: rbac.authorization.k8s.io
 - path: subjects
   kind: RoleBinding
   group: rbac.authorization.k8s.io
@@ -419,3 +425,278 @@ spec:
     namespace: original
 `)
 }
+
+func TestNamespaceTransformer_RoleBindingSubjectMode(t *testing.T) {
+	th := kusttest_test.MakeEnhancedHarness(t).
+		PrepBuiltin("NamespaceTransformer")
+	defer th.Reset()
+
+	defaultInput := `
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: manager-rolebinding
+subjects:
+- kind: ServiceAccount
+  name: default
+  namespace: system
+- kind: ServiceAccount
+  name: service-account
+  namespace: system
+- kind: ServiceAccount
+  name: another
+  namespace: random
+- kind: ServiceAccount
+  name: without-namespace
+- kind: Unrecognized
+  name: no-namespace
+- apiGroup: rbac.authorization.k8s.io
+  kind: User
+  name: alice@example.com
+- apiGroup: rbac.authorization.k8s.io
+  kind: Group
+  name: frontend-admins
+- apiGroup: rbac.authorization.k8s.io
+  kind: Group
+  name: system:serviceaccounts:qa
+`
+
+	tests := []struct {
+		name              string
+		input             string
+		transformerConfig string
+		expected          string
+	}{
+		{
+			name:  "target ALL service account subjects",
+			input: defaultInput,
+			transformerConfig: `
+apiVersion: builtin
+kind: NamespaceTransformer
+metadata:
+  name: notImportantHere
+  namespace: test
+setRoleBindingSubjects: allServiceAccounts
+` + defaultFieldSpecs,
+			expected: `
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: manager-rolebinding
+subjects:
+- kind: ServiceAccount
+  name: default
+  namespace: test
+- kind: ServiceAccount
+  name: service-account
+  namespace: test
+- kind: ServiceAccount
+  name: another
+  namespace: test
+- kind: ServiceAccount
+  name: without-namespace
+  namespace: test
+- kind: Unrecognized
+  name: no-namespace
+- apiGroup: rbac.authorization.k8s.io
+  kind: User
+  name: alice@example.com
+- apiGroup: rbac.authorization.k8s.io
+  kind: Group
+  name: frontend-admins
+- apiGroup: rbac.authorization.k8s.io
+  kind: Group
+  name: system:serviceaccounts:qa
+`,
+		},
+		{
+			name:  "target ALL subjects, role binding fieldspecs missing",
+			input: defaultInput,
+			transformerConfig: `
+apiVersion: builtin
+kind: NamespaceTransformer
+metadata:
+  name: notImportantHere
+  namespace: test
+setRoleBindingSubjects: allServiceAccounts
+fieldSpecs:
+- path: metadata/namespace
+  create: true
+`,
+			expected: `
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: manager-rolebinding
+subjects:
+- kind: ServiceAccount
+  name: default
+  namespace: test
+- kind: ServiceAccount
+  name: service-account
+  namespace: test
+- kind: ServiceAccount
+  name: another
+  namespace: test
+- kind: ServiceAccount
+  name: without-namespace
+  namespace: test
+- kind: Unrecognized
+  name: no-namespace
+- apiGroup: rbac.authorization.k8s.io
+  kind: User
+  name: alice@example.com
+- apiGroup: rbac.authorization.k8s.io
+  kind: Group
+  name: frontend-admins
+- apiGroup: rbac.authorization.k8s.io
+  kind: Group
+  name: system:serviceaccounts:qa
+`,
+		},
+		{
+			name:  "target ALL UNSET service account subjects",
+			input: defaultInput,
+			transformerConfig: `
+apiVersion: builtin
+kind: NamespaceTransformer
+metadata:
+  name: notImportantHere
+  namespace: test
+setRoleBindingSubjects: allServiceAccounts
+unsetOnly: true
+` + defaultFieldSpecs,
+			expected: `
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: manager-rolebinding
+subjects:
+- kind: ServiceAccount
+  name: default
+  namespace: system
+- kind: ServiceAccount
+  name: service-account
+  namespace: system
+- kind: ServiceAccount
+  name: another
+  namespace: random
+- kind: ServiceAccount
+  name: without-namespace
+  namespace: test
+- kind: Unrecognized
+  name: no-namespace
+- apiGroup: rbac.authorization.k8s.io
+  kind: User
+  name: alice@example.com
+- apiGroup: rbac.authorization.k8s.io
+  kind: Group
+  name: frontend-admins
+- apiGroup: rbac.authorization.k8s.io
+  kind: Group
+  name: system:serviceaccounts:qa
+`,
+		},
+		{
+			name:  "target NO subjects",
+			input: defaultInput,
+			transformerConfig: `
+apiVersion: builtin
+kind: NamespaceTransformer
+metadata:
+  name: notImportantHere
+  namespace: test
+setRoleBindingSubjects: none
+` + defaultFieldSpecs,
+			expected: defaultInput,
+		},
+		{
+			name:  "target subject named 'default' (explicitly)",
+			input: defaultInput,
+			transformerConfig: `
+apiVersion: builtin
+kind: NamespaceTransformer
+metadata:
+  name: notImportantHere
+  namespace: test
+setRoleBindingSubjects: defaultOnly
+` + defaultFieldSpecs,
+			expected: `
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: manager-rolebinding
+subjects:
+- kind: ServiceAccount
+  name: default
+  namespace: test
+- kind: ServiceAccount
+  name: service-account
+  namespace: system
+- kind: ServiceAccount
+  name: another
+  namespace: random
+- kind: ServiceAccount
+  name: without-namespace
+- kind: Unrecognized
+  name: no-namespace
+- apiGroup: rbac.authorization.k8s.io
+  kind: User
+  name: alice@example.com
+- apiGroup: rbac.authorization.k8s.io
+  kind: Group
+  name: frontend-admins
+- apiGroup: rbac.authorization.k8s.io
+  kind: Group
+  name: system:serviceaccounts:qa
+`,
+		},
+		{
+			name:  "target subject named 'default' (mode unspecified)",
+			input: defaultInput,
+			transformerConfig: `
+apiVersion: builtin
+kind: NamespaceTransformer
+metadata:
+  name: notImportantHere
+  namespace: test
+` + defaultFieldSpecs,
+			expected: `
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: manager-rolebinding
+subjects:
+- kind: ServiceAccount
+  name: default
+  namespace: test
+- kind: ServiceAccount
+  name: service-account
+  namespace: system
+- kind: ServiceAccount
+  name: another
+  namespace: random
+- kind: ServiceAccount
+  name: without-namespace
+- kind: Unrecognized
+  name: no-namespace
+- apiGroup: rbac.authorization.k8s.io
+  kind: User
+  name: alice@example.com
+- apiGroup: rbac.authorization.k8s.io
+  kind: Group
+  name: frontend-admins
+- apiGroup: rbac.authorization.k8s.io
+  kind: Group
+  name: system:serviceaccounts:qa
+`,
+		},
+	}
+
+	for i := range tests {
+		test := tests[i]
+		t.Run(test.name, func(t *testing.T) {
+			th.RunTransformerAndCheckResult(test.transformerConfig, test.input, test.expected)
+		})
+	}
+}