From 0a5f1ea76b19811bf0ad9a71d4dac69d5c55249b Mon Sep 17 00:00:00 2001 From: Jaylon McShan Date: Wed, 25 Sep 2024 10:10:44 -0500 Subject: [PATCH 1/4] Fix deletion not found handling and add related tests --- manifest/provider/apply.go | 27 ++++--- .../test/acceptance/delete_not_found_test.go | 77 +++++++++++++++++++ .../testdata/DeleteNotFoundTest/resource.tf | 14 ++++ .../testdata/DeleteNotFoundTest/variables.tf | 19 +++++ 4 files changed, 128 insertions(+), 9 deletions(-) create mode 100644 manifest/test/acceptance/delete_not_found_test.go create mode 100644 manifest/test/acceptance/testdata/DeleteNotFoundTest/resource.tf create mode 100644 manifest/test/acceptance/testdata/DeleteNotFoundTest/variables.tf diff --git a/manifest/provider/apply.go b/manifest/provider/apply.go index 22451dc5ed..f372531fb5 100644 --- a/manifest/provider/apply.go +++ b/manifest/provider/apply.go @@ -540,16 +540,25 @@ func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprot err = rs.Delete(ctxDeadline, rname, metav1.DeleteOptions{}) if err != nil { - rn := types.NamespacedName{Namespace: rnamespace, Name: rname}.String() - resp.Diagnostics = append(resp.Diagnostics, - &tfprotov5.Diagnostic{ - Severity: tfprotov5.DiagnosticSeverityError, - Summary: fmt.Sprintf("Error deleting resource %s: %s", rn, err), - Detail: err.Error(), - }) - return resp, nil - } + if apierrors.IsNotFound(err) { + s.logger.Trace("[ApplyResourceChange][Delete]", "Resource is already deleted") + resp.Diagnostics = append(resp.Diagnostics, + &tfprotov5.Diagnostic{ + Severity: tfprotov5.DiagnosticSeverityWarning, + Summary: fmt.Sprintf("Resource %q was already deleted", rname), + Detail: fmt.Sprintf("The resource %q was not found in the Kubernetes API. This may be due to the resource being already deleted.", rname), + }) + } else { + resp.Diagnostics = append(resp.Diagnostics, + &tfprotov5.Diagnostic{ + Severity: tfprotov5.DiagnosticSeverityError, + Summary: fmt.Sprintf("Error deleting resource %s: %s", rname, err), + Detail: err.Error(), + }) + return resp, nil + } + } // wait for delete for { if time.Now().After(deadline) { diff --git a/manifest/test/acceptance/delete_not_found_test.go b/manifest/test/acceptance/delete_not_found_test.go new file mode 100644 index 0000000000..90f242d097 --- /dev/null +++ b/manifest/test/acceptance/delete_not_found_test.go @@ -0,0 +1,77 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +//go:build acceptance +// +build acceptance + +package acceptance + +import ( + "context" + "testing" + "time" + + "github.com/hashicorp/go-hclog" + "github.com/hashicorp/terraform-provider-kubernetes/manifest/provider" + "github.com/hashicorp/terraform-provider-kubernetes/manifest/test/helper/kubernetes" +) + +func TestKubernetesManifest_DeletionNotFound(t *testing.T) { + ctx := context.Background() + + reattachInfo, err := provider.ServeTest(ctx, hclog.Default(), t) + if err != nil { + t.Fatalf("Failed to create provider instance: %v", err) + } + + name := randName() + namespace := randName() + + tf := tfhelper.RequireNewWorkingDir(ctx, t) + tf.SetReattachInfo(ctx, reattachInfo) + + k8shelper.CreateNamespace(t, namespace) + t.Logf("Verifying if namespace %s exists", namespace) + k8shelper.AssertResourceExists(t, "v1", "namespaces", namespace) + + defer func() { + tf.Destroy(ctx) + tf.Close() + k8shelper.DeleteResource(t, namespace, kubernetes.NewGroupVersionResource("v1", "namespaces")) + k8shelper.AssertResourceDoesNotExist(t, "v1", "namespaces", namespace) + }() + + tfvars := TFVARS{ + "namespace": namespace, + "name": name, + } + + // Load the Terraform config that will create the ConfigMap + tfconfig := loadTerraformConfig(t, "DeleteNotFoundTest/resource.tf", tfvars) + tf.SetConfig(ctx, tfconfig) + + t.Log("Applying Terraform configuration to create ConfigMap") + if err := tf.Apply(ctx); err != nil { + t.Fatalf("Terraform apply failed: %v", err) + } + + state, err := tf.State(ctx) + if err != nil { + t.Fatalf("Failed to retrieve Terraform state: %v", err) + } + t.Logf("Terraform state: %v", state) + + time.Sleep(2 * time.Second) + + t.Logf("Checking if ConfigMap %s in namespace %s was created", name, namespace) + k8shelper.AssertNamespacedResourceExists(t, "v1", "configmaps", namespace, name) + + // Simulating the deletion of the resource outside of Terraform + k8shelper.DeleteNamespacedResource(t, name, namespace, kubernetes.NewGroupVersionResource("v1", "configmaps")) + + // Running tf destroy in order to check if we are handling "404 Not Found" gracefully + tf.Destroy(ctx) + + // Ensuring that the ConfigMap no longer exists + k8shelper.AssertNamespacedResourceDoesNotExist(t, "v1", "configmaps", namespace, name) +} diff --git a/manifest/test/acceptance/testdata/DeleteNotFoundTest/resource.tf b/manifest/test/acceptance/testdata/DeleteNotFoundTest/resource.tf new file mode 100644 index 0000000000..76316832dc --- /dev/null +++ b/manifest/test/acceptance/testdata/DeleteNotFoundTest/resource.tf @@ -0,0 +1,14 @@ +resource "kubernetes_manifest" "test" { + manifest = { + "apiVersion" = "v1" + "kind" = "ConfigMap" + "metadata" = { + "name" = var.name + "namespace" = var.namespace + } + "data" = { + "foo" = "bar" + } + } +} + diff --git a/manifest/test/acceptance/testdata/DeleteNotFoundTest/variables.tf b/manifest/test/acceptance/testdata/DeleteNotFoundTest/variables.tf new file mode 100644 index 0000000000..f526a6a4bf --- /dev/null +++ b/manifest/test/acceptance/testdata/DeleteNotFoundTest/variables.tf @@ -0,0 +1,19 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: MPL-2.0 + +# These variable declarations are only used for interactive testing. +# The test code will template in different variable declarations with a default value when running the test. +# +# To set values for interactive runs, create a var-file and set values in it. +# If the name of the var-file ends in '.auto.tfvars' (e.g. myvalues.auto.tfvars) +# it will be automatically picked up and used by Terraform. +# +# DO NOT check in any files named *.auto.tfvars when making changes to tests. + +variable "name" { + type = string +} + +variable "namespace" { + type = string +} \ No newline at end of file From 821aa1b8866b8d8901e942739ffacf019cce3d87 Mon Sep 17 00:00:00 2001 From: Jaylon McShan Date: Wed, 25 Sep 2024 10:18:59 -0500 Subject: [PATCH 2/4] Adding changelog entry --- .changelog/2592.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/2592.txt diff --git a/.changelog/2592.txt b/.changelog/2592.txt new file mode 100644 index 0000000000..363c08aa2e --- /dev/null +++ b/.changelog/2592.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +handling "404 Not Found" errors during the deletion of Kubernetes resources, particularly in cases where the resource may have already been deleted by an operator managing the CRD before Terraform attempts to delete it. +``` \ No newline at end of file From 297122a0012be6fd97aa0aa92c033ccea10e478e Mon Sep 17 00:00:00 2001 From: Jaylon McShan Date: Wed, 25 Sep 2024 10:20:54 -0500 Subject: [PATCH 3/4] Adding license header --- .../test/acceptance/testdata/DeleteNotFoundTest/resource.tf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/manifest/test/acceptance/testdata/DeleteNotFoundTest/resource.tf b/manifest/test/acceptance/testdata/DeleteNotFoundTest/resource.tf index 76316832dc..8d875bbdd0 100644 --- a/manifest/test/acceptance/testdata/DeleteNotFoundTest/resource.tf +++ b/manifest/test/acceptance/testdata/DeleteNotFoundTest/resource.tf @@ -1,3 +1,5 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: MPL-2.0 resource "kubernetes_manifest" "test" { manifest = { "apiVersion" = "v1" From 5acc32b181ad4a1f15253cb0424d9b7a71bbb6ea Mon Sep 17 00:00:00 2001 From: Jaylon McShan Date: Thu, 26 Sep 2024 11:50:31 -0500 Subject: [PATCH 4/4] Adding refernece to manifest resource / fixing return statement outside of else block --- .changelog/2592.txt | 2 +- manifest/provider/apply.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.changelog/2592.txt b/.changelog/2592.txt index 363c08aa2e..96bf3f30d7 100644 --- a/.changelog/2592.txt +++ b/.changelog/2592.txt @@ -1,3 +1,3 @@ ```release-note:enhancement -handling "404 Not Found" errors during the deletion of Kubernetes resources, particularly in cases where the resource may have already been deleted by an operator managing the CRD before Terraform attempts to delete it. +`kubernetes_manifest` - handling "404 Not Found" errors during the deletion of Kubernetes resources, particularly in cases where the resource may have already been deleted by an operator managing the CRD before Terraform attempts to delete it. ``` \ No newline at end of file diff --git a/manifest/provider/apply.go b/manifest/provider/apply.go index f372531fb5..ec95a8402f 100644 --- a/manifest/provider/apply.go +++ b/manifest/provider/apply.go @@ -556,8 +556,8 @@ func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprot Summary: fmt.Sprintf("Error deleting resource %s: %s", rname, err), Detail: err.Error(), }) - return resp, nil } + return resp, nil } // wait for delete for {