From 49dced2e013ac6e6e252810831cd397ca0083e38 Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Fri, 23 Oct 2020 11:45:51 -0700 Subject: [PATCH] removed hardcoded list of namespaceable resources --- api/filters/namespace/namespace_test.go | 16 ++-- api/krusty/namespaces_test.go | 14 +++- api/resid/gvk.go | 4 +- api/resid/gvk_test.go | 39 ++++++++++ api/resid/resid_test.go | 6 +- api/resmap/reswrangler_test.go | 6 +- kyaml/yaml/types.go | 40 ---------- kyaml/yaml/types_test.go | 75 ------------------- .../NamespaceTransformer_test.go | 4 + 9 files changed, 70 insertions(+), 134 deletions(-) diff --git a/api/filters/namespace/namespace_test.go b/api/filters/namespace/namespace_test.go index 74bba6f6d..e9854811e 100644 --- a/api/filters/namespace/namespace_test.go +++ b/api/filters/namespace/namespace_test.go @@ -194,47 +194,47 @@ metadata: { name: "update-clusterrolebinding", input: ` -apiVersion: example.com/v1 +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding subjects: - name: default --- -apiVersion: example.com/v1 +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding subjects: - name: default namespace: foo --- -apiVersion: example.com/v1 +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding subjects: - name: something --- -apiVersion: example.com/v1 +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding subjects: - name: something namespace: foo `, expected: ` -apiVersion: example.com/v1 +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding subjects: - name: default namespace: bar --- -apiVersion: example.com/v1 +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding subjects: - name: default namespace: bar --- -apiVersion: example.com/v1 +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding subjects: - name: something --- -apiVersion: example.com/v1 +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding subjects: - name: something diff --git a/api/krusty/namespaces_test.go b/api/krusty/namespaces_test.go index 01183588d..97304eb2b 100644 --- a/api/krusty/namespaces_test.go +++ b/api/krusty/namespaces_test.go @@ -159,7 +159,7 @@ subjects: name: default namespace: irrelevant --- -apiVersion: admissionregistration.k8s.io/v1beta1 +apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: name: example @@ -180,15 +180,17 @@ webhooks: name: svc3 namespace: random --- -apiVersion: apiextensions.k8s.io/v1beta1 +apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: name: crds.my.org --- +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: name: cr1 --- +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: name: crb1 @@ -197,6 +199,7 @@ subjects: name: default namespace: irrelevant --- +apiVersion: v1 kind: PersistentVolume metadata: name: pv1 @@ -257,7 +260,7 @@ subjects: name: default namespace: newnamespace --- -apiVersion: admissionregistration.k8s.io/v1beta1 +apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: name: p1-example-s1 @@ -278,15 +281,17 @@ webhooks: namespace: random name: example3 --- -apiVersion: apiextensions.k8s.io/v1beta1 +apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: name: crds.my.org --- +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: name: p1-cr1-s1 --- +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: name: p1-crb1-s1 @@ -295,6 +300,7 @@ subjects: name: default namespace: newnamespace --- +apiVersion: v1 kind: PersistentVolume metadata: name: p1-pv1-s1 diff --git a/api/resid/gvk.go b/api/resid/gvk.go index 1e7b5f8d4..17f00db2c 100644 --- a/api/resid/gvk.go +++ b/api/resid/gvk.go @@ -6,6 +6,7 @@ package resid import ( "strings" + "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -207,5 +208,6 @@ func (x Gvk) toKyamlTypeMeta() yaml.TypeMeta { // IsNamespaceableKind returns true if x is a namespaceable Gvk // Implements https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/#not-all-objects-are-in-a-namespace func (x Gvk) IsNamespaceableKind() bool { - return x.toKyamlTypeMeta().IsNamespaceable() + isNamespaceScoped, found := openapi.IsNamespaceScoped(x.toKyamlTypeMeta()) + return !found || isNamespaceScoped } diff --git a/api/resid/gvk_test.go b/api/resid/gvk_test.go index 53f53618f..de2a11895 100644 --- a/api/resid/gvk_test.go +++ b/api/resid/gvk_test.go @@ -18,6 +18,8 @@ package resid import ( "testing" + + "github.com/stretchr/testify/assert" ) var equalsTests = []struct { @@ -255,3 +257,40 @@ func TestSelectByGVK(t *testing.T) { } } } + +func TestIsNamespaceableKind(t *testing.T) { + testCases := []struct { + name string + gvk Gvk + expected bool + }{ + { + "namespaceable resource", + Gvk{Group: "apps", Version: "v1", Kind: "Deployment"}, + true, + }, + { + "clusterscoped resource", + Gvk{Group: "", Version: "v1", Kind: "Namespace"}, + false, + }, + { + "unknown resource (should default to namespaceable)", + Gvk{Group: "example1.com", Version: "v1", Kind: "Bar"}, + true, + }, + { + "unknown resource (should default to namespaceable)", + Gvk{Group: "apps", Version: "v1", Kind: "ClusterRoleBinding"}, + true, + }, + } + + 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) + }) + } +} diff --git a/api/resid/resid_test.go b/api/resid/resid_test.go index ec2cf8404..b2e8570ff 100644 --- a/api/resid/resid_test.go +++ b/api/resid/resid_test.go @@ -265,7 +265,7 @@ func TestResIdEquals(t *testing.T) { Name: "nm", }, gVknResult: false, - nsEquals: false, + nsEquals: true, equals: false, }, { @@ -376,7 +376,7 @@ func TestEffectiveNamespace(t *testing.T) { }{ { id: ResId{ - Gvk: Gvk{Group: "g", Version: "v", Kind: "Node"}, + Gvk: Gvk{Group: "", Version: "v1", Kind: "Node"}, Name: "nm", }, expected: TotallyNotANamespace, @@ -384,7 +384,7 @@ func TestEffectiveNamespace(t *testing.T) { { id: ResId{ Namespace: "foo", - Gvk: Gvk{Group: "g", Version: "v", Kind: "Node"}, + Gvk: Gvk{Group: "", Version: "v1", Kind: "Node"}, Name: "nm", }, expected: TotallyNotANamespace, diff --git a/api/resmap/reswrangler_test.go b/api/resmap/reswrangler_test.go index 35d8050b8..7468c9170 100644 --- a/api/resmap/reswrangler_test.go +++ b/api/resmap/reswrangler_test.go @@ -355,7 +355,7 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { }) r4 := rf.FromMap( map[string]interface{}{ - "apiVersion": "v1", + "apiVersion": "apps/v1", "kind": "Deployment", "metadata": map[string]interface{}{ "name": "charlie", @@ -374,7 +374,7 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { r5.AddNamePrefix("little-") r6 := rf.FromMap( map[string]interface{}{ - "apiVersion": "v1", + "apiVersion": "apps/v1", "kind": "Deployment", "metadata": map[string]interface{}{ "name": "domino", @@ -384,7 +384,7 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { r6.AddNamePrefix("little-") r7 := rf.FromMap( map[string]interface{}{ - "apiVersion": "v1", + "apiVersion": "rbac.authorization.k8s.io/v1", "kind": "ClusterRoleBinding", "metadata": map[string]interface{}{ "name": "meh", diff --git a/kyaml/yaml/types.go b/kyaml/yaml/types.go index 1690d53ef..4adf0bda7 100644 --- a/kyaml/yaml/types.go +++ b/kyaml/yaml/types.go @@ -114,46 +114,6 @@ type TypeMeta struct { Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` } -// Hardcoded list. -// TODO(#2861): replace this with data acquired from openapi. -var notNamespaceableKinds = []string{ - "APIService", - "CSIDriver", - "CSINode", - "CertificateSigningRequest", - "Cluster", - "ClusterRole", - "ClusterRoleBinding", - "ComponentStatus", - "CustomResourceDefinition", - "MutatingWebhookConfiguration", - "Namespace", - "Node", - "PersistentVolume", - "PodSecurityPolicy", - "PriorityClass", - "RuntimeClass", - "SelfSubjectAccessReview", - "SelfSubjectRulesReview", - "StorageClass", - "SubjectAccessReview", - "TokenReview", - "ValidatingWebhookConfiguration", - "VolumeAttachment", -} - -// IsNamespaceable returns true if this TypeMeta is for an object -// that can be placed in a namespace. -// Implements https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/#not-all-objects-are-in-a-namespace -func (tm TypeMeta) IsNamespaceable() bool { - for _, k := range notNamespaceableKinds { - if k == tm.Kind { - return false - } - } - return true -} - // NameMeta contains name information. type NameMeta struct { // Name is the metadata.name field of a Resource diff --git a/kyaml/yaml/types_test.go b/kyaml/yaml/types_test.go index 7182cac86..6da832003 100644 --- a/kyaml/yaml/types_test.go +++ b/kyaml/yaml/types_test.go @@ -112,78 +112,3 @@ func TestIsYNodeEmptySeq(t *testing.T) { t.Fatalf("a node with content isn't empty") } } - -func TestIsNameSpaceable1(t *testing.T) { - testCases := []struct { - tm TypeMeta - isNamespaceable bool - }{ - { - tm: TypeMeta{Kind: "ClusterRole"}, - isNamespaceable: false, - }, - { - tm: TypeMeta{Kind: "Namespace"}, - isNamespaceable: false, - }, - { - tm: TypeMeta{Kind: "Deployment"}, - isNamespaceable: true, - }, - } - for _, tc := range testCases { - if tc.tm.IsNamespaceable() { - if !tc.isNamespaceable { - t.Fatalf("%v is namespaceable, but shouldn't be", tc.tm) - } - } else { - if tc.isNamespaceable { - t.Fatalf("%v is not namespaceable, but should be", tc.tm) - } - } - } -} - -func TestIsNameSpaceable2(t *testing.T) { - testCases := []struct { - yammy string - isNamespaceable bool - }{ - { - yammy: `apiVersion: apps/v1 -kind: Deployment -metadata: - name: bilbo - namespace: middleEarth -`, - isNamespaceable: true, - }, - { - yammy: `apiVersion: v1 -kind: Namespace -metadata: - name: middleEarth -`, - isNamespaceable: false, - }, - } - for _, tc := range testCases { - rn, err := Parse(tc.yammy) - if err != nil { - t.Fatalf("unexpected parse error %v from json: %s", err, tc.yammy) - } - meta, err := rn.GetMeta() - if err != nil { - t.Fatalf("unexpected meta error %v", err) - } - if meta.IsNamespaceable() { - if !tc.isNamespaceable { - t.Fatalf("%v is namespaceable, but shouldn't be", meta) - } - } else { - if tc.isNamespaceable { - t.Fatalf("%v is not namespaceable, but should be", meta) - } - } - } -} diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go b/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go index 1b57ed1bf..01e36e80d 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go +++ b/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go @@ -193,18 +193,22 @@ kind: Namespace metadata: name: ns1 --- +apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: name: crd1 --- +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: name: cr1 --- +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: name: crb1 --- +apiVersion: v1 kind: PersistentVolume metadata: name: pv1