From dceff4861fb87f36f78be1fb779ad2e77f107770 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 3 Mar 2023 08:54:27 -0500 Subject: [PATCH 1/6] r/aws_lambda_function: Add 'TestAccLambdaFunction_skipDestroyInconsistentPlan'. --- internal/service/lambda/function_test.go | 37 ++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/internal/service/lambda/function_test.go b/internal/service/lambda/function_test.go index 1ce6d7dc782..9e24a8c24e0 100644 --- a/internal/service/lambda/function_test.go +++ b/internal/service/lambda/function_test.go @@ -2127,6 +2127,43 @@ func TestAccLambdaFunction_skipDestroy(t *testing.T) { }) } +// https://github.com/hashicorp/terraform-provider-aws/issues/29777. +func TestAccLambdaFunction_skipDestroyInconsistentPlan(t *testing.T) { + ctx := acctest.Context(t) + var conf lambda.GetFunctionOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_lambda_function.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, names.LambdaEndpointID), + CheckDestroy: testAccCheckFunctionDestroy(ctx), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "aws": { + Source: "hashicorp/aws", + VersionConstraint: "4.56.0", + }, + }, + Config: testAccFunctionConfig_basic(rName, rName, rName, rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckFunctionExists(ctx, resourceName, &conf), + resource.TestCheckNoResourceAttr(resourceName, "skip_destroy"), + ), + }, + { + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + Config: testAccFunctionConfig_basic(rName, rName, rName, rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckFunctionExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "skip_destroy", "false"), + ), + }, + }, + }) +} + func testAccCheckFunctionDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).LambdaClient() From 4219262f378ebd3c0aa18c402af78f82319b6d90 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 6 Mar 2023 13:52:02 -0500 Subject: [PATCH 2/6] r/aws_lambda_function: Set 'skip_destroy' in resource Read. --- internal/service/lambda/function.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/service/lambda/function.go b/internal/service/lambda/function.go index 18b289e1f6d..cd8a70bf201 100644 --- a/internal/service/lambda/function.go +++ b/internal/service/lambda/function.go @@ -638,6 +638,8 @@ func resourceFunctionRead(ctx context.Context, d *schema.ResourceData, meta inte d.Set("runtime", function.Runtime) d.Set("signing_job_arn", function.SigningJobArn) d.Set("signing_profile_version_arn", function.SigningProfileVersionArn) + // Support in-place update of non-refreshable attribute. + d.Set("skip_destroy", d.Get("skip_destroy")) if err := d.Set("snap_start", flattenSnapStart(function.SnapStart)); err != nil { return sdkdiag.AppendErrorf(diags, "setting snap_start: %s", err) } From 1295773750ca33f94e3a0214e7c0860bc9dcdd39 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 6 Mar 2023 13:58:04 -0500 Subject: [PATCH 3/6] Add CHANGELOG entry. --- .changelog/29812.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/29812.txt diff --git a/.changelog/29812.txt b/.changelog/29812.txt new file mode 100644 index 00000000000..a3a4769eccf --- /dev/null +++ b/.changelog/29812.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_lambda_function: Prevent `Provider produced inconsistent final plan` errors produced by null `skip_destroy` attribute value. NOTE: Because the maintainers have been unable to reproduce the reported problem, the fix is best effort and we ask for community help in testing +``` \ No newline at end of file From 9537e467f665624b05b06f07dbe1a9d8808aa916 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 6 Mar 2023 14:24:58 -0500 Subject: [PATCH 4/6] r/aws_lambda_function: Remove 'skip_destroy' from acceptance test 'ImportStateVerifyIgnore'. --- internal/service/lambda/function_test.go | 80 ++++++++++++------------ 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/internal/service/lambda/function_test.go b/internal/service/lambda/function_test.go index 9e24a8c24e0..0173e908a4a 100644 --- a/internal/service/lambda/function_test.go +++ b/internal/service/lambda/function_test.go @@ -76,7 +76,7 @@ func TestAccLambdaFunction_basic(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, }, }) @@ -130,7 +130,7 @@ func TestAccLambdaFunction_tags(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_tags2(rName, "key1", "value1updated", "key2", "value2"), @@ -207,7 +207,7 @@ func TestAccLambdaFunction_unpublishedCodeUpdate(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, }, }) @@ -242,7 +242,7 @@ func TestAccLambdaFunction_codeSigning(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_cscUpdate(rName), @@ -255,7 +255,7 @@ func TestAccLambdaFunction_codeSigning(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_cscDelete(rName), @@ -295,7 +295,7 @@ func TestAccLambdaFunction_concurrency(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_concurrencyUpdate(rName), @@ -335,7 +335,7 @@ func TestAccLambdaFunction_concurrencyCycle(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_concurrencyUpdate(rName), @@ -400,7 +400,7 @@ func TestAccLambdaFunction_envVariables(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_envVariables(rName), @@ -455,7 +455,7 @@ func TestAccLambdaFunction_EnvironmentVariables_noValue(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, }, }) @@ -491,7 +491,7 @@ func TestAccLambdaFunction_encryptedEnvVariables(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_encryptedEnvVariablesKey2(rName), @@ -562,7 +562,7 @@ func TestAccLambdaFunction_versioned(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, }, }) @@ -640,7 +640,7 @@ func TestAccLambdaFunction_versionedUpdate(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, }, }) @@ -689,7 +689,7 @@ func TestAccLambdaFunction_enablePublish(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { // No changes, `publish` is true. This should not publish a new version. @@ -747,7 +747,7 @@ func TestAccLambdaFunction_disablePublish(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, }, }) @@ -782,7 +782,7 @@ func TestAccLambdaFunction_deadLetter(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, // Ensure configuration can be removed { @@ -832,7 +832,7 @@ func TestAccLambdaFunction_deadLetterUpdated(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, }, }) @@ -894,7 +894,7 @@ func TestAccLambdaFunction_fileSystem(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, // Ensure lambda file system configuration can be updated { @@ -967,7 +967,7 @@ func TestAccLambdaFunction_image(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, // Ensure lambda image code can be updated { @@ -1023,7 +1023,7 @@ func TestAccLambdaFunction_architectures(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, // Ensure function's "architectures" attribute can be removed. The actual architecture remains unchanged. { @@ -1074,7 +1074,7 @@ func TestAccLambdaFunction_architecturesUpdate(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, // Ensure function architecture can be updated { @@ -1125,7 +1125,7 @@ func TestAccLambdaFunction_architecturesWithLayer(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, // Ensure function architecture can be updated { @@ -1168,7 +1168,7 @@ func TestAccLambdaFunction_ephemeralStorage(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_updateEphemeralStorage(rName), @@ -1210,7 +1210,7 @@ func TestAccLambdaFunction_tracing(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_tracingUpdated(rName), @@ -1256,7 +1256,7 @@ func TestAccLambdaFunction_KMSKeyARN_noEnvironmentVariables(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, }, }) @@ -1290,7 +1290,7 @@ func TestAccLambdaFunction_layers(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, }, }) @@ -1324,7 +1324,7 @@ func TestAccLambdaFunction_layersUpdate(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_layersUpdated(rName), @@ -1368,7 +1368,7 @@ func TestAccLambdaFunction_vpc(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, }, }) @@ -1401,7 +1401,7 @@ func TestAccLambdaFunction_vpcRemoval(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_basic(rName, rName, rName, rName), @@ -1444,7 +1444,7 @@ func TestAccLambdaFunction_vpcUpdate(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_vpcUpdated(rName), @@ -1490,7 +1490,7 @@ func TestAccLambdaFunction_VPC_withInvocation(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, }, }) @@ -1525,7 +1525,7 @@ func TestAccLambdaFunction_VPCPublishNo_changes(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_vpcPublish(rName), @@ -1568,7 +1568,7 @@ func TestAccLambdaFunction_VPCPublishHas_changes(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_vpcUpdatedPublish(rName), @@ -1703,7 +1703,7 @@ func TestAccLambdaFunction_emptyVPC(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, }, }) @@ -1732,7 +1732,7 @@ func TestAccLambdaFunction_s3(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"publish", "s3_bucket", "s3_key", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"publish", "s3_bucket", "s3_key"}, }, }, }) @@ -1778,7 +1778,7 @@ func TestAccLambdaFunction_localUpdate(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { PreConfig: func() { @@ -1844,7 +1844,7 @@ func TestAccLambdaFunction_LocalUpdate_nameOnly(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { PreConfig: func() { @@ -1899,7 +1899,7 @@ func TestAccLambdaFunction_S3Update_basic(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "s3_bucket", "s3_key", "s3_object_version", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish", "s3_bucket", "s3_key", "s3_object_version"}, }, { PreConfig: func() { @@ -1955,7 +1955,7 @@ func TestAccLambdaFunction_S3Update_unversioned(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "s3_bucket", "s3_key", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish", "s3_bucket", "s3_key"}, }, { PreConfig: func() { @@ -1998,7 +1998,7 @@ func TestAccLambdaFunction_snapStart(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }, { Config: testAccFunctionConfig_snapStartDisabled(rName), @@ -2070,7 +2070,7 @@ func TestAccLambdaFunction_runtimes(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"filename", "publish", "skip_destroy"}, + ImportStateVerifyIgnore: []string{"filename", "publish"}, }) resource.ParallelTest(t, resource.TestCase{ From a572a764be66308d59837393f857627956f9b168 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 6 Mar 2023 14:26:11 -0500 Subject: [PATCH 5/6] Tweak CHANGELOG entry. --- .changelog/29812.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/29812.txt b/.changelog/29812.txt index a3a4769eccf..c4caa974853 100644 --- a/.changelog/29812.txt +++ b/.changelog/29812.txt @@ -1,3 +1,3 @@ ```release-note:bug -resource/aws_lambda_function: Prevent `Provider produced inconsistent final plan` errors produced by null `skip_destroy` attribute value. NOTE: Because the maintainers have been unable to reproduce the reported problem, the fix is best effort and we ask for community help in testing +resource/aws_lambda_function: Prevent `Provider produced inconsistent final plan` errors produced by null `skip_destroy` attribute value. NOTE: Because the maintainers have been unable to reproduce the reported problem, the fix is best effort and we ask for community community support in verifying the fix ``` \ No newline at end of file From 6c88865c6cf3c78240f565fcacdfee16e06423c1 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 6 Mar 2023 14:38:58 -0500 Subject: [PATCH 6/6] Update .changelog/29812.txt Co-authored-by: Justin Retzolk <44710313+justinretzolk@users.noreply.github.com> --- .changelog/29812.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/29812.txt b/.changelog/29812.txt index c4caa974853..c5a4ef5121c 100644 --- a/.changelog/29812.txt +++ b/.changelog/29812.txt @@ -1,3 +1,3 @@ ```release-note:bug -resource/aws_lambda_function: Prevent `Provider produced inconsistent final plan` errors produced by null `skip_destroy` attribute value. NOTE: Because the maintainers have been unable to reproduce the reported problem, the fix is best effort and we ask for community community support in verifying the fix +resource/aws_lambda_function: Prevent `Provider produced inconsistent final plan` errors produced by null `skip_destroy` attribute value. NOTE: Because the maintainers have been unable to reproduce the reported problem, the fix is best effort and we ask for community support in verifying the fix. ``` \ No newline at end of file