diff --git a/api/filters/fieldspec/fieldspec.go b/api/filters/fieldspec/fieldspec.go index d728e9ebc..09bd2e2dd 100644 --- a/api/filters/fieldspec/fieldspec.go +++ b/api/filters/fieldspec/fieldspec.go @@ -11,6 +11,7 @@ import ( "sigs.k8s.io/kustomize/api/internal/utils" "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/errors" + "sigs.k8s.io/kustomize/kyaml/resid" "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -45,8 +46,8 @@ type Filter struct { func (fltr Filter) Filter(obj *yaml.RNode) (*yaml.RNode, error) { // check if the FieldSpec applies to the object - if match, err := isMatchGVK(fltr.FieldSpec, obj); !match || err != nil { - return obj, errors.Wrap(err) + if match := isMatchGVK(fltr.FieldSpec, obj); !match { + return obj, nil } fltr.path = utils.PathSplitter(fltr.FieldSpec.Path) err := fltr.filter(obj) @@ -158,28 +159,24 @@ func isSequenceField(name string) (string, bool) { } // isMatchGVK returns true if the fs.GVK matches the obj GVK. -func isMatchGVK(fs types.FieldSpec, obj *yaml.RNode) (bool, error) { - meta, err := obj.GetMeta() - if err != nil { - return false, err - } - if fs.Kind != "" && fs.Kind != meta.Kind { +func isMatchGVK(fs types.FieldSpec, obj *yaml.RNode) bool { + if kind := obj.GetKind(); fs.Kind != "" && fs.Kind != kind { // kind doesn't match - return false, err + return false } // parse the group and version from the apiVersion field - group, version := parseGV(meta.APIVersion) + group, version := resid.ParseGroupVersion(obj.GetApiVersion()) if fs.Group != "" && fs.Group != group { // group doesn't match - return false, nil + return false } if fs.Version != "" && fs.Version != version { // version doesn't match - return false, nil + return false } - return true, nil + return true } diff --git a/api/filters/fieldspec/fieldspec_test.go b/api/filters/fieldspec/fieldspec_test.go index cefc5b9be..d416191b5 100644 --- a/api/filters/fieldspec/fieldspec_test.go +++ b/api/filters/fieldspec/fieldspec_test.go @@ -46,10 +46,11 @@ xxx: "empty path": { fieldSpec: ` group: foo +version: v1 kind: Bar `, input: ` -apiVersion: foo +apiVersion: foo/v1 kind: Bar xxx: `, @@ -59,7 +60,7 @@ kind: Bar xxx: `, error: `considering field '' of object -apiVersion: foo +apiVersion: foo/v1 kind: Bar xxx: : cannot set or create an empty field name`, @@ -195,11 +196,14 @@ kind: Bar input: ` a: b: c +`, + expected: ` +a: + b: c `, filter: fieldspec.Filter{ SetValue: filtersutil.SetScalar("e"), }, - error: "missing Resource metadata", }, "miss-match-type": { diff --git a/api/filters/fieldspec/gvk.go b/api/filters/fieldspec/gvk.go deleted file mode 100644 index badd8eb0b..000000000 --- a/api/filters/fieldspec/gvk.go +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2019 The Kubernetes Authors. -// SPDX-License-Identifier: Apache-2.0 -package fieldspec - -import ( - "strings" - - "sigs.k8s.io/kustomize/kyaml/resid" - "sigs.k8s.io/kustomize/kyaml/yaml" -) - -// Return true for 'v' followed by a 1 or 2, and don't look at rest. -// I.e. 'v1', 'v1beta1', 'v2', would return true. -func looksLikeACoreApiVersion(s string) bool { - if len(s) < 2 { - return false - } - if s[0:1] != "v" { - return false - } - return s[1:2] == "1" || s[1:2] == "2" -} - -// parseGV parses apiVersion field into group and version. -func parseGV(apiVersion string) (group, version string) { - // parse the group and version from the apiVersion field - parts := strings.SplitN(apiVersion, "/", 2) - group = parts[0] - if len(parts) > 1 { - version = parts[1] - } - // Special case the original "apiVersion" of what - // we now call the "core" (empty) group. - if version == "" && looksLikeACoreApiVersion(group) { - version = group - group = "" - } - return -} - -// GetGVK parses the metadata into a GVK -func GetGVK(meta yaml.ResourceMeta) resid.Gvk { - group, version := parseGV(meta.APIVersion) - return resid.Gvk{ - Group: group, - Version: version, - Kind: meta.Kind, - } -} diff --git a/api/filters/fieldspec/gvk_test.go b/api/filters/fieldspec/gvk_test.go deleted file mode 100644 index cda85612b..000000000 --- a/api/filters/fieldspec/gvk_test.go +++ /dev/null @@ -1,156 +0,0 @@ -// Copyright 2019 The Kubernetes Authors. -// SPDX-License-Identifier: Apache-2.0 -package fieldspec - -import ( - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "sigs.k8s.io/kustomize/kyaml/resid" - "sigs.k8s.io/kustomize/kyaml/yaml" -) - -func TestParseGV(t *testing.T) { - testCases := map[string]struct { - input string - expectedGroup string - expectedVersion string - }{ - "empty": { - input: "", - expectedGroup: "", - expectedVersion: "", - }, - "certSigning": { - input: "certificates.k8s.io/v1beta1", - expectedGroup: "certificates.k8s.io", - expectedVersion: "v1beta1", - }, - "extensions": { - input: "extensions/v1beta1", - expectedGroup: "extensions", - expectedVersion: "v1beta1", - }, - "normal": { - input: "apps/v1", - expectedGroup: "apps", - expectedVersion: "v1", - }, - "justApps": { - input: "apps", - expectedGroup: "apps", - expectedVersion: "", - }, - "coreV1": { - input: "v1", - expectedGroup: "", - expectedVersion: "v1", - }, - "coreV2": { - input: "v2", - expectedGroup: "", - expectedVersion: "v2", - }, - "coreV2Beta1": { - input: "v2beta1", - expectedGroup: "", - expectedVersion: "v2beta1", - }, - } - - for tn, tc := range testCases { - t.Run(tn, func(t *testing.T) { - group, version := parseGV(tc.input) - if !assert.Equal(t, tc.expectedGroup, group) { - t.FailNow() - } - if !assert.Equal(t, tc.expectedVersion, version) { - t.FailNow() - } - }) - } -} - -func TestGetGVK(t *testing.T) { - testCases := map[string]struct { - input string - expected resid.Gvk - parseError string - metaError string - }{ - "empty": { - input: ` -`, - parseError: "EOF", - }, - "junk": { - input: ` -congress: effective -`, - metaError: "missing Resource metadata", - }, - "normal": { - input: ` -apiVersion: apps/v1 -kind: Deployment -`, - expected: resid.Gvk{Group: "apps", Version: "v1", Kind: "Deployment"}, - }, - "apiVersionOnlyWithSlash": { - input: ` -apiVersion: apps/v1 -`, - expected: resid.Gvk{Group: "apps", Version: "v1", Kind: ""}, - }, - "apiVersionOnlyNoSlash1": { - input: ` -apiVersion: apps -`, - expected: resid.Gvk{Group: "apps", Version: "", Kind: ""}, - }, - "apiVersionOnlyNoSlash2": { - input: ` -apiVersion: v1 -`, - expected: resid.Gvk{Group: "", Version: "v1", Kind: ""}, - }, - } - - for tn, tc := range testCases { - t.Run(tn, func(t *testing.T) { - obj, err := yaml.Parse(tc.input) - if len(tc.parseError) != 0 { - if err == nil { - t.Error("expected parse error") - return - } - if !strings.Contains(err.Error(), tc.parseError) { - t.Errorf("expected parse err '%s', got '%v'", tc.parseError, err) - } - return - } - if !assert.NoError(t, err) { - t.FailNow() - } - meta, err := obj.GetMeta() - if len(tc.metaError) != 0 { - if err == nil { - t.Error("expected meta error") - return - } - if !strings.Contains(err.Error(), tc.metaError) { - t.Errorf("expected meta err '%s', got '%v'", tc.metaError, err) - } - return - } - if !assert.NoError(t, err) { - t.FailNow() - } - gvk := GetGVK(meta) - if !assert.Equal(t, tc.expected, gvk) { - t.FailNow() - } - }) - } -} diff --git a/api/filters/nameref/nameref.go b/api/filters/nameref/nameref.go index fa2372780..247c94dd8 100644 --- a/api/filters/nameref/nameref.go +++ b/api/filters/nameref/nameref.go @@ -257,8 +257,7 @@ func previousIdSelectedByGvk(gvk *resid.Gvk) sieveFunc { // If the we are updating a 'roleRef/name' field, the 'apiGroup' and 'kind' // fields in the same 'roleRef' map must be considered. -// If either object is cluster-scoped (!IsNamespaceableKind), there -// can be a referral. +// If either object is cluster-scoped, there can be a referral. // E.g. a RoleBinding (which exists in a namespace) can refer // to a ClusterRole (cluster-scoped) object. // https://kubernetes.io/docs/reference/access-authn-authz/rbac/#role-and-clusterrole @@ -285,12 +284,12 @@ func prefixSuffixEquals(other resource.ResCtx) sieveFunc { func (f Filter) sameCurrentNamespaceAsReferrer() sieveFunc { referrerCurId := f.Referrer.CurId() - if !referrerCurId.IsNamespaceableKind() { + if referrerCurId.IsClusterScoped() { // If the referrer is cluster-scoped, let anything through. return acceptAll } return func(r *resource.Resource) bool { - if !r.CurId().IsNamespaceableKind() { + if r.CurId().IsClusterScoped() { // Allow cluster-scoped through. return true } diff --git a/api/filters/namespace/namespace.go b/api/filters/namespace/namespace.go index 69458e198..e554600bc 100644 --- a/api/filters/namespace/namespace.go +++ b/api/filters/namespace/namespace.go @@ -4,11 +4,11 @@ package namespace import ( - "sigs.k8s.io/kustomize/api/filters/fieldspec" "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/kio" + "sigs.k8s.io/kustomize/kyaml/resid" "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -54,16 +54,11 @@ func (ns Filter) run(node *yaml.RNode) (*yaml.RNode, error) { // hacks applies the namespace transforms that are hardcoded rather // than specified through FieldSpecs. func (ns Filter) hacks(obj *yaml.RNode) error { - meta, err := obj.GetMeta() - if err != nil { + gvk := resid.GvkFromNode(obj) + if err := ns.metaNamespaceHack(obj, gvk); err != nil { return err } - - if err := ns.metaNamespaceHack(obj, meta); err != nil { - return err - } - - return ns.roleBindingHack(obj, meta) + return ns.roleBindingHack(obj, gvk) } // metaNamespaceHack is a hack for implementing the namespace transform @@ -74,9 +69,8 @@ func (ns Filter) hacks(obj *yaml.RNode) error { // This hack should be updated to allow individual resources to specify // if they are cluster scoped through either an annotation on the resources, // or through inlined OpenAPI on the resource as a YAML comment. -func (ns Filter) metaNamespaceHack(obj *yaml.RNode, meta yaml.ResourceMeta) error { - gvk := fieldspec.GetGVK(meta) - if !gvk.IsNamespaceableKind() { +func (ns Filter) metaNamespaceHack(obj *yaml.RNode, gvk resid.Gvk) error { + if gvk.IsClusterScoped() { return nil } f := fsslice.Filter{ @@ -104,8 +98,8 @@ func (ns Filter) metaNamespaceHack(obj *yaml.RNode, meta yaml.ResourceMeta) erro // ... // - name: "something-else" # this will not have the namespace set // ... -func (ns Filter) roleBindingHack(obj *yaml.RNode, meta yaml.ResourceMeta) error { - if meta.Kind != roleBindingKind && meta.Kind != clusterRoleBindingKind { +func (ns Filter) roleBindingHack(obj *yaml.RNode, gvk resid.Gvk) error { + if gvk.Kind != roleBindingKind && gvk.Kind != clusterRoleBindingKind { return nil } diff --git a/api/internal/accumulator/namereferencetransformer_test.go b/api/internal/accumulator/namereferencetransformer_test.go index 707f602d2..39fa9b037 100644 --- a/api/internal/accumulator/namereferencetransformer_test.go +++ b/api/internal/accumulator/namereferencetransformer_test.go @@ -885,9 +885,9 @@ func TestNameReferenceClusterWide(t *testing.T) { }).ResMap() clusterRoleId := resid.NewResId( - resid.Gvk{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRole"}, modifiedname) + resid.NewGvk("rbac.authorization.k8s.io", "v1", "ClusterRole"), modifiedname) clusterRoleBindingId := resid.NewResId( - resid.Gvk{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRoleBinding"}, modifiedname) + resid.NewGvk("rbac.authorization.k8s.io", "v1", "ClusterRoleBinding"), modifiedname) clusterRole, _ := expected.GetByCurrentId(clusterRoleId) clusterRole.AppendRefBy(clusterRoleBindingId) @@ -1012,9 +1012,11 @@ func TestNameReferenceNamespaceTransformation(t *testing.T) { }).ResMap() clusterRoleId := resid.NewResId( - resid.Gvk{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRole"}, modifiedname) + resid.NewGvk("rbac.authorization.k8s.io", "v1", "ClusterRole"), + modifiedname) clusterRoleBindingId := resid.NewResId( - resid.Gvk{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRoleBinding"}, modifiedname) + resid.NewGvk("rbac.authorization.k8s.io", "v1", "ClusterRoleBinding"), + modifiedname) clusterRole, _ := expected.GetByCurrentId(clusterRoleId) clusterRole.AppendRefBy(clusterRoleBindingId) diff --git a/api/internal/accumulator/resaccumulator.go b/api/internal/accumulator/resaccumulator.go index bac5e1fee..1c9b11575 100644 --- a/api/internal/accumulator/resaccumulator.go +++ b/api/internal/accumulator/resaccumulator.go @@ -72,7 +72,7 @@ func (ra *ResAccumulator) MergeVars(incoming []types.Var) error { for _, v := range incoming { targetId := resid.NewResIdWithNamespace(v.ObjRef.GVK(), v.ObjRef.Name, v.ObjRef.Namespace) idMatcher := targetId.GvknEquals - if targetId.Namespace != "" || !targetId.IsNamespaceableKind() { + if targetId.Namespace != "" || targetId.IsClusterScoped() { // Preserve backward compatibility. An empty namespace means // wildcard search on the namespace hence we still use GvknEquals idMatcher = targetId.Equals diff --git a/api/resmap/resmap.go b/api/resmap/resmap.go index 19879550d..766dbb1de 100644 --- a/api/resmap/resmap.go +++ b/api/resmap/resmap.go @@ -168,8 +168,7 @@ type ResMap interface { // GroupedByCurrentNamespace returns a map of namespace // to a slice of *Resource in that namespace. - // Resources for whom IsNamespaceableKind is false are - // are not included at all (see NonNamespaceable). + // Cluster-scoped Resources are not included (see ClusterScoped). // Resources with an empty namespace are placed // in the resid.DefaultNamespace entry. GroupedByCurrentNamespace() map[string][]*resource.Resource @@ -179,10 +178,10 @@ type ResMap interface { // one to perform the grouping. GroupedByOriginalNamespace() map[string][]*resource.Resource - // NonNamespaceable returns a slice of resources that + // ClusterScoped returns a slice of resources that // cannot be placed in a namespace, e.g. // Node, ClusterRole, Namespace itself, etc. - NonNamespaceable() []*resource.Resource + ClusterScoped() []*resource.Resource // AllIds returns all CurrentIds. AllIds() []resid.ResId diff --git a/api/resmap/reswrangler.go b/api/resmap/reswrangler.go index 7dbf92565..17ed49ca9 100644 --- a/api/resmap/reswrangler.go +++ b/api/resmap/reswrangler.go @@ -238,8 +238,8 @@ func (m *resWrangler) GroupedByCurrentNamespace() map[string][]*resource.Resourc return items } -// NonNamespaceable implements ResMap.NonNamespaceable -func (m *resWrangler) NonNamespaceable() []*resource.Resource { +// ClusterScoped implements ResMap.ClusterScoped +func (m *resWrangler) ClusterScoped() []*resource.Resource { return m.groupedByCurrentNamespace()[resid.TotallyNotANamespace] } @@ -388,7 +388,7 @@ func (m *resWrangler) makeCopy(copier resCopier) ResMap { func (m *resWrangler) SubsetThatCouldBeReferencedByResource( referrer *resource.Resource) ResMap { referrerId := referrer.CurId() - if !referrerId.IsNamespaceableKind() { + if referrerId.IsClusterScoped() { // A cluster scoped resource can refer to anything. return m } @@ -396,7 +396,7 @@ func (m *resWrangler) SubsetThatCouldBeReferencedByResource( roleBindingNamespaces := getNamespacesForRoleBinding(referrer) for _, possibleTarget := range m.rList { id := possibleTarget.CurId() - if !id.IsNamespaceableKind() { + if id.IsClusterScoped() { // A cluster-scoped resource can be referred to by anything. result.append(possibleTarget) continue diff --git a/api/resmap/reswrangler_test.go b/api/resmap/reswrangler_test.go index 41fc238be..1e038b3d1 100644 --- a/api/resmap/reswrangler_test.go +++ b/api/resmap/reswrangler_test.go @@ -192,7 +192,7 @@ metadata: } func TestGetMatchingResourcesByCurrentId(t *testing.T) { - cmap := resid.Gvk{Version: "v1", Kind: "ConfigMap"} + cmap := resid.NewGvk("", "v1", "ConfigMap") r1 := rf.FromMap( map[string]interface{}{ diff --git a/api/resource/resource.go b/api/resource/resource.go index b3807507b..456c274d1 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -87,12 +87,7 @@ func (r *Resource) GetBinaryDataMap() map[string]string { } func (r *Resource) GetGvk() resid.Gvk { - meta, err := r.node.GetMeta() - if err != nil { - return resid.GvkFromString("") - } - g, v := resid.ParseGroupVersion(meta.APIVersion) - return resid.Gvk{Group: g, Version: v, Kind: meta.Kind} + return resid.GvkFromNode(r.node) } func (r *Resource) Hash(h ifc.KustHasher) (string, error) { diff --git a/api/resource/resource_test.go b/api/resource/resource_test.go index 6640039ab..05a483aca 100644 --- a/api/resource/resource_test.go +++ b/api/resource/resource_test.go @@ -87,11 +87,13 @@ func TestResourceId(t *testing.T) { { in: testConfigMap, id: resid.NewResIdWithNamespace( - resid.Gvk{Version: "v1", Kind: "ConfigMap"}, "winnie", "hundred-acre-wood"), + resid.NewGvk("", "v1", "ConfigMap"), + "winnie", "hundred-acre-wood"), }, { in: testDeployment, - id: resid.NewResId(resid.Gvk{Group: "apps", Version: "v1", Kind: "Deployment"}, "pooh"), + id: resid.NewResId( + resid.NewGvk("apps", "v1", "Deployment"), "pooh"), }, } for _, test := range tests { diff --git a/kustomize/commands/build/writer.go b/kustomize/commands/build/writer.go index 89e4c2201..44a2e4ebb 100644 --- a/kustomize/commands/build/writer.go +++ b/kustomize/commands/build/writer.go @@ -36,7 +36,7 @@ func (w Writer) WriteIndividualFiles(dirPath string, m resmap.ResMap) error { } } } - for _, res := range m.NonNamespaceable() { + for _, res := range m.ClusterScoped() { err := w.write(dirPath, fileName(res), res) if err != nil { return err diff --git a/kyaml/openapi/openapi.go b/kyaml/openapi/openapi.go index 4bb3e379a..5e34195f6 100644 --- a/kyaml/openapi/openapi.go +++ b/kyaml/openapi/openapi.go @@ -273,6 +273,15 @@ func IsNamespaceScoped(typeMeta yaml.TypeMeta) (bool, bool) { return isNamespaceScoped, found } +// IsCertainlyClusterScoped returns true for Node, Namespace, etc. and +// false for Pod, Deployment, etc. and kinds that aren't recognized in the +// openapi data. See: +// https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces +func IsCertainlyClusterScoped(typeMeta yaml.TypeMeta) bool { + nsScoped, found := IsNamespaceScoped(typeMeta) + return found && !nsScoped +} + // SuppressBuiltInSchemaUse can be called to prevent using the built-in Kubernetes // schema as part of the global schema. // Must be called before the schema is used. diff --git a/kyaml/resid/gvk.go b/kyaml/resid/gvk.go index b112151d3..21dad78d3 100644 --- a/kyaml/resid/gvk.go +++ b/kyaml/resid/gvk.go @@ -16,13 +16,26 @@ type Gvk struct { Group string `json:"group,omitempty" yaml:"group,omitempty"` Version string `json:"version,omitempty" yaml:"version,omitempty"` Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` + // isClusterScoped is true if the object is known, per the openapi + // data in use, to be cluster scoped, and false otherwise. + isClusterScoped bool +} + +func NewGvk(g, v, k string) Gvk { + result := Gvk{Group: g, Version: v, Kind: k} + result.isClusterScoped = + openapi.IsCertainlyClusterScoped(result.AsTypeMeta()) + return result +} + +func GvkFromNode(r *yaml.RNode) Gvk { + g, v := ParseGroupVersion(r.GetApiVersion()) + return NewGvk(g, v, r.GetKind()) } // FromKind makes a Gvk with only the kind specified. func FromKind(k string) Gvk { - return Gvk{ - Kind: k, - } + return NewGvk("", "", k) } // ParseGroupVersion parses a KRM metadata apiVersion field. @@ -56,11 +69,7 @@ func GvkFromString(s string) Gvk { if k == noKind { k = "" } - return Gvk{ - Group: g, - Version: v, - Kind: k, - } + return NewGvk(g, v, k) } // Values that are brief but meaningful in logs. @@ -90,10 +99,13 @@ func (x Gvk) String() string { // ApiVersion returns the combination of Group and Version func (x Gvk) ApiVersion() string { - if x.Group == "" { - return x.Version + var sb strings.Builder + if x.Group != "" { + sb.WriteString(x.Group) + sb.WriteString("/") } - return x.Group + "/" + x.Version + sb.WriteString(x.Version) + return sb.String() } // StringWoEmptyField returns a string representation of the GVK. Non-exist @@ -207,26 +219,16 @@ func (x Gvk) IsSelected(selector *Gvk) bool { return true } -// toKyamlTypeMeta returns a yaml.TypeMeta from x's information. -func (x Gvk) toKyamlTypeMeta() yaml.TypeMeta { - var apiVersion strings.Builder - if x.Group != "" { - apiVersion.WriteString(x.Group) - apiVersion.WriteString("/") - } - apiVersion.WriteString(x.Version) +// AsTypeMeta returns a yaml.TypeMeta from x's information. +func (x Gvk) AsTypeMeta() yaml.TypeMeta { return yaml.TypeMeta{ - APIVersion: apiVersion.String(), + APIVersion: x.ApiVersion(), Kind: x.Kind, } } -// IsNamespaceableKind returns true if x is a namespaceable Gvk, -// e.g. instances of Pod and Deployment are namespaceable, -// but instances of Node and Namespace are not namespaceable. -// Alternative name for this method: IsNotClusterScoped. -// Implements https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/#not-all-objects-are-in-a-namespace -func (x Gvk) IsNamespaceableKind() bool { - isNamespaceScoped, found := openapi.IsNamespaceScoped(x.toKyamlTypeMeta()) - return !found || isNamespaceScoped +// IsClusterScoped returns true if the Gvk is certainly cluster scoped +// with respect to the available openapi data. +func (x Gvk) IsClusterScoped() bool { + return x.isClusterScoped } diff --git a/kyaml/resid/gvk_test.go b/kyaml/resid/gvk_test.go index 790cc087e..cb3b9fadd 100644 --- a/kyaml/resid/gvk_test.go +++ b/kyaml/resid/gvk_test.go @@ -259,30 +259,45 @@ func TestSelectByGVK(t *testing.T) { } } -func TestIsNamespaceableKind(t *testing.T) { +func TestIsClusterScoped(t *testing.T) { testCases := []struct { - name string - gvk Gvk - expected bool + name string + gvk Gvk + isClusterScoped bool }{ { - "namespaceable resource", - Gvk{Group: "apps", Version: "v1", Kind: "Deployment"}, - true, - }, - { - "clusterscoped resource", - Gvk{Group: "", Version: "v1", Kind: "Namespace"}, + "deployment is namespaceable", + NewGvk("apps", "v1", "Deployment"), false, }, { - "unknown resource (should default to namespaceable)", - Gvk{Group: "example1.com", Version: "v1", Kind: "Bar"}, + "clusterscoped resource", + NewGvk("", "v1", "Namespace"), true, }, { "unknown resource (should default to namespaceable)", - Gvk{Group: "apps", Version: "v1", Kind: "ClusterRoleBinding"}, + NewGvk("example1.com", "v1", "BoatyMcBoatface"), + false, + }, + { + "node is cluster scoped", + NewGvk("", "v1", "Node"), + true, + }, + { + "Role is namespace scoped", + NewGvk("rbac.authorization.k8s.io", "v1", "Role"), + false, + }, + { + "ClusterRole is cluster scoped", + NewGvk("rbac.authorization.k8s.io", "v1", "ClusterRole"), + true, + }, + { + "ClusterRoleBinding is cluster scoped", + NewGvk("rbac.authorization.k8s.io", "v1", "ClusterRoleBinding"), true, }, } @@ -290,8 +305,7 @@ func TestIsNamespaceableKind(t *testing.T) { for i := range testCases { test := testCases[i] t.Run(test.name, func(t *testing.T) { - isNamespaceable := test.gvk.IsNamespaceableKind() - assert.Equal(t, test.expected, isNamespaceable) + assert.Equal(t, test.isClusterScoped, test.gvk.IsClusterScoped()) }) } } diff --git a/kyaml/resid/resid.go b/kyaml/resid/resid.go index 92219c03b..1700531a2 100644 --- a/kyaml/resid/resid.go +++ b/kyaml/resid/resid.go @@ -12,13 +12,10 @@ type ResId struct { // Gvk of the resource. Gvk `json:",inline,omitempty" yaml:",inline,omitempty"` - // Name of the resource before transformation. + // Name of the resource. Name string `json:"name,omitempty" yaml:"name,omitempty"` - // Namespace the resource belongs to. - // An untransformed resource has no namespace. - // A fully transformed resource has the namespace - // from the top most overlay. + // Namespace the resource belongs to, if it can belong to a namespace. Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"` } @@ -30,12 +27,12 @@ func NewResIdWithNamespace(k Gvk, n, ns string) ResId { // NewResId creates new ResId. func NewResId(k Gvk, n string) ResId { - return ResId{Gvk: k, Name: n} + return NewResIdWithNamespace(k, n, "") } // NewResIdKindOnly creates a new ResId. func NewResIdKindOnly(k string, n string) ResId { - return ResId{Gvk: FromKind(k), Name: n} + return NewResId(FromKind(k), n) } const ( @@ -113,7 +110,7 @@ func (id ResId) IsNsEquals(o ResId) bool { // ResId and the Namespace is either not set or set // to DefaultNamespace. func (id ResId) IsInDefaultNs() bool { - return id.IsNamespaceableKind() && id.isPutativelyDefaultNs() + return !id.IsClusterScoped() && id.isPutativelyDefaultNs() } func (id ResId) isPutativelyDefaultNs() bool { @@ -124,7 +121,7 @@ func (id ResId) isPutativelyDefaultNs() bool { // namespace for use in reporting and equality tests. func (id ResId) EffectiveNamespace() string { // The order of these checks matters. - if !id.IsNamespaceableKind() { + if id.IsClusterScoped() { return TotallyNotANamespace } if id.isPutativelyDefaultNs() { diff --git a/kyaml/resid/resid_test.go b/kyaml/resid/resid_test.go index 908c2bab3..0ad979ce8 100644 --- a/kyaml/resid/resid_test.go +++ b/kyaml/resid/resid_test.go @@ -464,42 +464,42 @@ func TestFromString(t *testing.T) { } func TestEffectiveNamespace(t *testing.T) { - var test = []struct { + var testCases = map[string]struct { id ResId expected string }{ - { + "tst1": { id: ResId{ - Gvk: Gvk{Group: "", Version: "v1", Kind: "Node"}, + Gvk: NewGvk("", "v1", "Node"), Name: "nm", }, expected: TotallyNotANamespace, }, - { + "tst2": { id: ResId{ Namespace: "foo", - Gvk: Gvk{Group: "", Version: "v1", Kind: "Node"}, + Gvk: NewGvk("", "v1", "Node"), Name: "nm", }, expected: TotallyNotANamespace, }, - { + "tst3": { id: ResId{ Namespace: "foo", - Gvk: Gvk{Group: "g", Version: "v", Kind: "k"}, + Gvk: NewGvk("g", "v", "k"), Name: "nm", }, expected: "foo", }, - { + "tst4": { id: ResId{ Namespace: "", - Gvk: Gvk{Group: "g", Version: "v", Kind: "k"}, + Gvk: NewGvk("g", "v", "k"), Name: "nm", }, expected: DefaultNamespace, }, - { + "tst5": { id: ResId{ Gvk: Gvk{Group: "g", Version: "v", Kind: "k"}, Name: "nm", @@ -508,10 +508,12 @@ func TestEffectiveNamespace(t *testing.T) { }, } - for _, tst := range test { - if actual := tst.id.EffectiveNamespace(); actual != tst.expected { - t.Fatalf("EffectiveNamespace was %s, expected %s", - actual, tst.expected) - } + for n, tst := range testCases { + t.Run(n, func(t *testing.T) { + if actual := tst.id.EffectiveNamespace(); actual != tst.expected { + t.Fatalf("EffectiveNamespace was %s, expected %s", + actual, tst.expected) + } + }) } } diff --git a/kyaml/yaml/rnode.go b/kyaml/yaml/rnode.go index 9b1b5ff1b..c5f258ad6 100644 --- a/kyaml/yaml/rnode.go +++ b/kyaml/yaml/rnode.go @@ -337,13 +337,34 @@ func (rn *RNode) SetYNode(node *yaml.Node) { *rn.value = *node } -// GetKind returns the kind. +// GetKind returns the kind, if it exists, else empty string. func (rn *RNode) GetKind() string { - node, err := rn.Pipe(FieldMatcher{Name: KindField}) - if err != nil { - return "" + if node := rn.getMapFieldValue(KindField); node != nil { + return node.Value } - return GetValue(node) + return "" +} + +// GetApiVersion returns the apiversion, if it exists, else empty string. +func (rn *RNode) GetApiVersion() string { + if node := rn.getMapFieldValue(APIVersionField); node != nil { + return node.Value + } + return "" +} + +// getMapFieldValue returns the value (*yaml.Node) of a mapping field. +// The value might be nil. Also, the function returns nil, not an error, +// if this node is not a mapping node, or if this node does not have the +// given field, so this function cannot be used to make distinctions +// between these cases. +func (rn *RNode) getMapFieldValue(field string) *yaml.Node { + for i := 0; i < len(rn.Content()); i = IncrementFieldIndex(i) { + if rn.Content()[i].Value == field { + return rn.Content()[i+1] + } + } + return nil } // GetName returns the name.