From 0c37ee89afe782895af8a14de90b120824f3443d Mon Sep 17 00:00:00 2001 From: Katrina Verey Date: Fri, 8 Jul 2022 14:18:25 -0400 Subject: [PATCH] Option to customize NamespaceTransfomer role binding subject handling --- api/filters/namespace/namespace.go | 108 ++++--- api/internal/builtins/NamespaceTransformer.go | 31 +- api/konfig/builtinpluginconsts/namespace.go | 6 - .../NamespaceTransformer.go | 31 +- .../NamespaceTransformer_test.go | 281 ++++++++++++++++++ 5 files changed, 400 insertions(+), 57 deletions(-) diff --git a/api/filters/namespace/namespace.go b/api/filters/namespace/namespace.go index 1b28d5c34..5173a9554 100644 --- a/api/filters/namespace/namespace.go +++ b/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) - if err := ns.roleBindingHack(node, gvk); err != nil { + ns.FsSlice = ns.removeRoleBindingSubjectFieldSpecs(ns.FsSlice) + 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 { - if !isRoleBinding(gvk.Kind) { +func (ns Filter) roleBindingHack(obj *yaml.RNode) error { + 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 } - - // add the namespace to each "subject" with name: default - 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 + // Use the appropriate visitor to set the namespace field on the correct subset of subjects + return errors.WrapPrefixf(obj.VisitElements(visitor), "setting namespace on (cluster)role binding subjects") } 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" ) diff --git a/api/internal/builtins/NamespaceTransformer.go b/api/internal/builtins/NamespaceTransformer.go index 03a664cff..71b162a79 100644 --- a/api/internal/builtins/NamespaceTransformer.go +++ b/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"` - FieldSpecs []types.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` - UnsetOnly bool `json:"unsetOnly" yaml:"unsetOnly"` + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` + FieldSpecs []types.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` + 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, - FsSlice: p.FieldSpecs, - UnsetOnly: p.UnsetOnly, + Namespace: p.Namespace, + FsSlice: p.FieldSpecs, + SetRoleBindingSubjects: p.SetRoleBindingSubjects, + UnsetOnly: p.UnsetOnly, }); err != nil { return err } diff --git a/api/konfig/builtinpluginconsts/namespace.go b/api/konfig/builtinpluginconsts/namespace.go index a35ef9c6f..35774a7db 100644 --- a/api/konfig/builtinpluginconsts/namespace.go +++ b/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 diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer.go b/plugin/builtin/namespacetransformer/NamespaceTransformer.go index 249fdf1ea..e06e442bd 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer.go +++ b/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"` - FieldSpecs []types.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` - UnsetOnly bool `json:"unsetOnly" yaml:"unsetOnly"` + types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` + FieldSpecs []types.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` + 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, - FsSlice: p.FieldSpecs, - UnsetOnly: p.UnsetOnly, + Namespace: p.Namespace, + FsSlice: p.FieldSpecs, + SetRoleBindingSubjects: p.SetRoleBindingSubjects, + UnsetOnly: p.UnsetOnly, }); err != nil { return err } diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go b/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go index efcc90c92..0c1449082 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go +++ b/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) + }) + } +}