From 82ff95dfb859da0eb5a502ad393e12645efeab0d Mon Sep 17 00:00:00 2001 From: grahamhar Date: Sun, 14 Feb 2021 11:11:46 +0000 Subject: [PATCH 1/4] Fix change detection relating to vpc_config As reported in https://github.com/hashicorp/terraform-provider-aws/issues/17385 a change i detected in vpc_config when there are no changes, this is caused by an issue in https://github.com/hashicorp/terraform-plugin-sdk/issues/617 The PR to provide a fix is not getting any traction. On further debuging the issue is caused by the nested elements being of type set which needs the use of Equal rather than reflect.DeepEqual to test for differences. We can work around this bug by testing for changes in the two fields within vpc_config independantly as when the item passed to HasChanges is a Set it is tested correctly. --- aws/resource_aws_lambda_function.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/aws/resource_aws_lambda_function.go b/aws/resource_aws_lambda_function.go index 4872f036966..32f716f41c9 100644 --- a/aws/resource_aws_lambda_function.go +++ b/aws/resource_aws_lambda_function.go @@ -364,7 +364,8 @@ func hasConfigChanges(d resourceDiffer) bool { d.HasChange("layers") || d.HasChange("dead_letter_config") || d.HasChange("tracing_config") || - d.HasChange("vpc_config") || + d.HasChange("vpc_config.0.security_group_ids") || + d.HasChange("vpc_config.0.subnet_ids") || d.HasChange("runtime") || d.HasChange("environment") } From ee8fb4523f5e658a8906e5b1cb11f581362841b2 Mon Sep 17 00:00:00 2001 From: grahamhar Date: Mon, 15 Feb 2021 07:02:20 +0000 Subject: [PATCH 2/4] Add acceptance tests --- aws/resource_aws_lambda_function.go | 2 +- aws/resource_aws_lambda_function_test.go | 117 +++++++++++++++++++++++ 2 files changed, 118 insertions(+), 1 deletion(-) diff --git a/aws/resource_aws_lambda_function.go b/aws/resource_aws_lambda_function.go index 32f716f41c9..0971177621f 100644 --- a/aws/resource_aws_lambda_function.go +++ b/aws/resource_aws_lambda_function.go @@ -998,7 +998,7 @@ func resourceAwsLambdaFunctionUpdate(d *schema.ResourceData, meta interface{}) e } } } - if d.HasChange("vpc_config") { + if d.HasChanges("vpc_config.0.security_group_ids", "vpc_config.0.subnet_ids") { configReq.VpcConfig = &lambda.VpcConfig{ SecurityGroupIds: []*string{}, SubnetIds: []*string{}, diff --git a/aws/resource_aws_lambda_function_test.go b/aws/resource_aws_lambda_function_test.go index 9d4b0e4462d..ef0d4c5c711 100644 --- a/aws/resource_aws_lambda_function_test.go +++ b/aws/resource_aws_lambda_function_test.go @@ -1309,6 +1309,89 @@ func TestAccAWSLambdaFunction_VPC_withInvocation(t *testing.T) { }) } +// See https://github.com/hashicorp/terraform-provider-aws/issues/17385 +// When the vpc config doesn't change the version shouldn't change +func TestAccAWSLambdaFunction_VPC_publish_No_Changes(t *testing.T) { + var conf lambda.GetFunctionOutput + + rString := acctest.RandString(8) + funcName := fmt.Sprintf("tf_acc_lambda_func_vpc_w_invc_%s", rString) + policyName := fmt.Sprintf("tf_acc_policy_lambda_func_vpc_w_invc_%s", rString) + roleName := fmt.Sprintf("tf_acc_role_lambda_func_vpc_w_invc_%s", rString) + sgName := fmt.Sprintf("tf_acc_sg_lambda_func_vpc_w_invc_%s", rString) + resourceName := "aws_lambda_function.test" + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ErrorCheck: testAccErrorCheck(t, lambda.EndpointsID), + Providers: testAccProviders, + CheckDestroy: testAccCheckLambdaFunctionDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSLambdaConfigWithVPCPublish(funcName, policyName, roleName, sgName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsLambdaFunctionExists(resourceName, funcName, &conf), + resource.TestCheckResourceAttr(resourceName, "version", "1"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"filename", "publish"}, + }, + { + Config: testAccAWSLambdaConfigWithVPCPublish(funcName, policyName, roleName, sgName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsLambdaFunctionExists(resourceName, funcName, &conf), + resource.TestCheckResourceAttr(resourceName, "version", "1"), + ), + }, + }, + }) +} + +// See https://github.com/hashicorp/terraform-provider-aws/issues/17385 +// When the vpc config changes the version should change +func TestAccAWSLambdaFunction_VPC_publish_Has_Changes(t *testing.T) { + var conf lambda.GetFunctionOutput + + rString := acctest.RandString(8) + funcName := fmt.Sprintf("tf_acc_lambda_func_vpc_w_invc_%s", rString) + policyName := fmt.Sprintf("tf_acc_policy_lambda_func_vpc_w_invc_%s", rString) + roleName := fmt.Sprintf("tf_acc_role_lambda_func_vpc_w_invc_%s", rString) + sgName := fmt.Sprintf("tf_acc_sg_lambda_func_vpc_w_invc_%s", rString) + resourceName := "aws_lambda_function.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ErrorCheck: testAccErrorCheck(t, lambda.EndpointsID), + Providers: testAccProviders, + CheckDestroy: testAccCheckLambdaFunctionDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSLambdaConfigWithVPCPublish(funcName, policyName, roleName, sgName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsLambdaFunctionExists(resourceName, funcName, &conf), + resource.TestCheckResourceAttr(resourceName, "version", "1"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"filename", "publish"}, + }, + { + Config: testAccAWSLambdaConfigWithVPCUpdatedPublish(funcName, policyName, roleName, sgName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsLambdaFunctionExists(resourceName, funcName, &conf), + resource.TestCheckResourceAttr(resourceName, "version", "2"), + ), + }, + }, + }) +} + // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/10044 func TestAccAWSLambdaFunction_VpcConfig_ProperIamDependencies(t *testing.T) { var function lambda.GetFunctionOutput @@ -2865,6 +2948,40 @@ resource "aws_lambda_function" "test" { `, funcName) } +func testAccAWSLambdaConfigWithVPCPublish(funcName, policyName, roleName, sgName string) string { + return fmt.Sprintf(baseAccAWSLambdaConfig(policyName, roleName, sgName)+` +resource "aws_lambda_function" "test" { + filename = "test-fixtures/lambdatest.zip" + function_name = "%s" + role = aws_iam_role.iam_for_lambda.arn + handler = "exports.example" + runtime = "nodejs12.x" + publish = true + vpc_config { + subnet_ids = [aws_subnet.subnet_for_lambda.id] + security_group_ids = [aws_security_group.sg_for_lambda.id] + } +} +`, funcName) +} + +func testAccAWSLambdaConfigWithVPCUpdatedPublish(funcName, policyName, roleName, sgName string) string { + return fmt.Sprintf(baseAccAWSLambdaConfig(policyName, roleName, sgName)+` +resource "aws_lambda_function" "test" { + filename = "test-fixtures/lambdatest.zip" + function_name = "%s" + role = aws_iam_role.iam_for_lambda.arn + handler = "exports.example" + runtime = "nodejs12.x" + publish = true + vpc_config { + subnet_ids = [aws_subnet.subnet_for_lambda.id, aws_subnet.subnet_for_lambda_az2.id] + security_group_ids = [aws_security_group.sg_for_lambda.id] + } +} +`, funcName) +} + func testAccAWSLambdaConfigWithVPCUpdated(funcName, policyName, roleName, sgName, sgName2 string) string { return fmt.Sprintf(baseAccAWSLambdaConfig(policyName, roleName, sgName)+` resource "aws_lambda_function" "test" { From 622a2f4fa3789fa7834850722eb026a046bd499f Mon Sep 17 00:00:00 2001 From: grahamhar Date: Sat, 27 Mar 2021 14:23:50 +0000 Subject: [PATCH 3/4] Refactor test to work around ENI deletion issue Removing the VPC config is still a valid vpc config change and works around the ENI deltion issue. --- aws/resource_aws_lambda_function_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_lambda_function_test.go b/aws/resource_aws_lambda_function_test.go index ef0d4c5c711..9d855479f0d 100644 --- a/aws/resource_aws_lambda_function_test.go +++ b/aws/resource_aws_lambda_function_test.go @@ -1367,6 +1367,7 @@ func TestAccAWSLambdaFunction_VPC_publish_Has_Changes(t *testing.T) { ErrorCheck: testAccErrorCheck(t, lambda.EndpointsID), Providers: testAccProviders, CheckDestroy: testAccCheckLambdaFunctionDestroy, + Steps: []resource.TestStep{ { Config: testAccAWSLambdaConfigWithVPCPublish(funcName, policyName, roleName, sgName), @@ -2975,8 +2976,8 @@ resource "aws_lambda_function" "test" { runtime = "nodejs12.x" publish = true vpc_config { - subnet_ids = [aws_subnet.subnet_for_lambda.id, aws_subnet.subnet_for_lambda_az2.id] - security_group_ids = [aws_security_group.sg_for_lambda.id] + security_group_ids = [] + subnet_ids = [] } } `, funcName) From 5fa4a7a094ec74c71caee6879700709f158ebf36 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 4 Jun 2021 14:11:38 -0700 Subject: [PATCH 4/4] Adds CHANGELOG --- .changelog/17610.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/17610.txt diff --git a/.changelog/17610.txt b/.changelog/17610.txt new file mode 100644 index 00000000000..7f3bcaf1cbf --- /dev/null +++ b/.changelog/17610.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_lambda_function: Prevents perpetual diff in `vpc_config` +```