From 5bf205e048615e530ff0222d7b0b8b7ada503e69 Mon Sep 17 00:00:00 2001 From: Philipp Strube Date: Sat, 18 Jun 2022 09:48:30 +0200 Subject: [PATCH] Error when namespace set but not allowed or not set when required Previously the provider would just error with not found, which frequently confused users not closely paying attention if their resource is namespace scoped or not. --- kustomize/manifest.go | 28 +++++++++- kustomize/resource_kustomization.go | 29 +++++++--- kustomize/resource_kustomization_test.go | 54 +++++++++++++++++++ .../invalid_cluster_role_binding.yaml | 14 +++++ .../kustomization.yaml | 5 ++ .../invalid_role_binding.yaml | 13 +++++ .../kustomization.yaml | 5 ++ 7 files changed, 138 insertions(+), 10 deletions(-) create mode 100644 kustomize/test_kustomizations/fail_namespace_not_allowed/invalid_cluster_role_binding.yaml create mode 100644 kustomize/test_kustomizations/fail_namespace_not_allowed/kustomization.yaml create mode 100644 kustomize/test_kustomizations/fail_namespace_required/invalid_role_binding.yaml create mode 100644 kustomize/test_kustomizations/fail_namespace_required/kustomization.yaml diff --git a/kustomize/manifest.go b/kustomize/manifest.go index 10c8d3b..444179b 100644 --- a/kustomize/manifest.go +++ b/kustomize/manifest.go @@ -121,6 +121,19 @@ func (km *kManifest) namespace() string { return km.id().namespace } +func (km *kManifest) isNamespaced() (bool, error) { + m, err := km.mapping() + if err != nil { + return false, km.fmtErr(fmt.Errorf("api error: %s", err)) + } + + if m.Scope.Name() == k8smeta.RESTScopeNameNamespace { + return true, nil + } + + return false, nil +} + func (km *kManifest) name() string { return km.id().name } @@ -137,10 +150,21 @@ func (km *kManifest) id() (id kManifestId) { func (km *kManifest) api() (api k8sdynamic.ResourceInterface, err error) { gvr, err := km.gvr() if err != nil { - return api, nil + return api, km.fmtErr(fmt.Errorf("api error: %s", err)) + } + + api = km.client.Resource(gvr) + + isNamespaced, err := km.isNamespaced() + if err != nil { + return api, km.fmtErr(fmt.Errorf("api error: %s", err)) + } + + if isNamespaced { + api = km.client.Resource(gvr).Namespace(km.namespace()) } - return km.client.Resource(gvr).Namespace(km.namespace()), nil + return api, nil } func (km *kManifest) apiGet(opts k8smetav1.GetOptions) (resp *k8sunstructured.Unstructured, err error) { diff --git a/kustomize/resource_kustomization.go b/kustomize/resource_kustomization.go index 214190f..875f048 100644 --- a/kustomize/resource_kustomization.go +++ b/kustomize/resource_kustomization.go @@ -179,15 +179,28 @@ func kustomizationResourceDiff(ctx context.Context, d *schema.ResourceDiff, m in } setLastAppliedConfig(kmm, gzipLastAppliedConfig) + _, err = kmm.mappings() + if err != nil { + // if there are no mappings we can't dry-run + // this is for CRDs that do not exist yet + return nil + } + + isNamespaced, err := kmm.isNamespaced() + if err != nil { + return logError(err) + } + if isNamespaced && kmm.namespace() == "" { + err = kmm.fmtErr(fmt.Errorf("is namespace scoped and must set metadata.namespace")) + return logError(err) + } + if !isNamespaced && kmm.namespace() != "" { + err = kmm.fmtErr(fmt.Errorf("is not namespace scoped but has metadata.namespace set")) + return logError(err) + } + if do.(string) == "" { // diffing for create - _, err := kmm.mappings() - if err != nil { - // if there are no mappings we can't dry-run - // this is for CRDs that do not exist yet - return nil - } - _, err = kmm.apiCreate(k8smetav1.CreateOptions{DryRun: []string{k8smetav1.DryRunAll}}) if err != nil { if k8serrors.IsAlreadyExists(err) { @@ -334,7 +347,7 @@ func kustomizationResourceDelete(d *schema.ResourceData, m interface{}) error { // the resource can't exist either return nil } - return logError(err) + return logError(km.fmtErr(err)) } err = km.apiDelete(k8smetav1.DeleteOptions{}) diff --git a/kustomize/resource_kustomization_test.go b/kustomize/resource_kustomization_test.go index a24f84b..dfef327 100644 --- a/kustomize/resource_kustomization_test.go +++ b/kustomize/resource_kustomization_test.go @@ -604,6 +604,60 @@ resource "kustomization_resource" "ns" { ` } +// +// +// Fail namespace not allowed +func TestAccResourceKustomization_failPlanInvalidNamespaceNotAllowed(t *testing.T) { + + resource.Test(t, resource.TestCase{ + Providers: testAccProviders, + Steps: []resource.TestStep{ + // + // + // Expect plan to fail due to namespace not allowed + { + Config: testAccResourceKustomizationConfig_failNamespaceNotAllowed("test_kustomizations/fail_namespace_not_allowed"), + ExpectError: regexp.MustCompile("Error: github.com/kbst/terraform-provider-kustomize/kustomize.kustomizationResourceDiff: \"rbac.authorization.k8s.io/ClusterRoleBinding/default/invalid\": is not namespace scoped but has metadata.namespace set"), + }, + }, + }) +} + +func testAccResourceKustomizationConfig_failNamespaceNotAllowed(path string) string { + return testAccDataSourceKustomizationConfig_basic(path) + ` +resource "kustomization_resource" "crb" { + manifest = data.kustomization_build.test.manifests["rbac.authorization.k8s.io/ClusterRoleBinding/default/invalid"] +} +` +} + +// +// +// Fail namespace required +func TestAccResourceKustomization_failNamespaceRequired(t *testing.T) { + + resource.Test(t, resource.TestCase{ + Providers: testAccProviders, + Steps: []resource.TestStep{ + // + // + // Expect plan to fail due to missing namespace + { + Config: testAccResourceKustomizationConfig_failNamespaceRequired("test_kustomizations/fail_namespace_required"), + ExpectError: regexp.MustCompile("Error: github.com/kbst/terraform-provider-kustomize/kustomize.kustomizationResourceDiff: \"rbac.authorization.k8s.io/RoleBinding/_/invalid\": is namespace scoped and must set metadata.namespace"), + }, + }, + }) +} + +func testAccResourceKustomizationConfig_failNamespaceRequired(path string) string { + return testAccDataSourceKustomizationConfig_basic(path) + ` +resource "kustomization_resource" "crb" { + manifest = data.kustomization_build.test.manifests["rbac.authorization.k8s.io/RoleBinding/_/invalid"] +} +` +} + // // // Fail plan invalid manifest diff --git a/kustomize/test_kustomizations/fail_namespace_not_allowed/invalid_cluster_role_binding.yaml b/kustomize/test_kustomizations/fail_namespace_not_allowed/invalid_cluster_role_binding.yaml new file mode 100644 index 0000000..1315424 --- /dev/null +++ b/kustomize/test_kustomizations/fail_namespace_not_allowed/invalid_cluster_role_binding.yaml @@ -0,0 +1,14 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: invalid + # invalid not namespace scoped + namespace: default +subjects: +- kind: Group + name: admins + apiGroup: rbac.authorization.k8s.io +roleRef: + kind: ClusterRole + name: admin + apiGroup: rbac.authorization.k8s.io diff --git a/kustomize/test_kustomizations/fail_namespace_not_allowed/kustomization.yaml b/kustomize/test_kustomizations/fail_namespace_not_allowed/kustomization.yaml new file mode 100644 index 0000000..8514226 --- /dev/null +++ b/kustomize/test_kustomizations/fail_namespace_not_allowed/kustomization.yaml @@ -0,0 +1,5 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +resources: +- invalid_cluster_role_binding.yaml diff --git a/kustomize/test_kustomizations/fail_namespace_required/invalid_role_binding.yaml b/kustomize/test_kustomizations/fail_namespace_required/invalid_role_binding.yaml new file mode 100644 index 0000000..f1cb786 --- /dev/null +++ b/kustomize/test_kustomizations/fail_namespace_required/invalid_role_binding.yaml @@ -0,0 +1,13 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: invalid + # invalid namespace scoped but no namespace +subjects: +- kind: Group + name: admins + apiGroup: rbac.authorization.k8s.io +roleRef: + kind: ClusterRole + name: admin + apiGroup: rbac.authorization.k8s.io diff --git a/kustomize/test_kustomizations/fail_namespace_required/kustomization.yaml b/kustomize/test_kustomizations/fail_namespace_required/kustomization.yaml new file mode 100644 index 0000000..39e122c --- /dev/null +++ b/kustomize/test_kustomizations/fail_namespace_required/kustomization.yaml @@ -0,0 +1,5 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +resources: +- invalid_role_binding.yaml