From bada19925e899ff2ac7272212b41405e605f70a1 Mon Sep 17 00:00:00 2001 From: tabito Date: Sun, 18 May 2025 00:25:32 +0900 Subject: [PATCH 1/5] suppress diffs from log levels when publish is true --- internal/service/lambda/function.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/internal/service/lambda/function.go b/internal/service/lambda/function.go index bc24157a1b06..4864f4aa4cb8 100644 --- a/internal/service/lambda/function.go +++ b/internal/service/lambda/function.go @@ -1360,12 +1360,25 @@ func needsFunctionCodeUpdate(d sdkv2.ResourceDiffer) bool { d.HasChange("architectures") } +func hasChangeForLoggingConfigLogLevel(d sdkv2.ResourceDiffer, key string, logFormatHasChange bool) bool { + if d.HasChange(key) { + oldValue, newValue := d.GetChange(key) + suppressed := suppressLoggingConfigUnspecifiedLogLevelsPrimitive(key, oldValue.(string), newValue.(string), logFormatHasChange) + return !suppressed + } else { + return false + } +} + func needsFunctionConfigUpdate(d sdkv2.ResourceDiffer) bool { return d.HasChange(names.AttrDescription) || d.HasChange("handler") || d.HasChange("file_system_config") || d.HasChange("image_config") || - d.HasChange("logging_config") || + d.HasChange("logging_config.0.log_format") || + d.HasChange("logging_config.0.log_group") || + hasChangeForLoggingConfigLogLevel(d, "logging_config.0.application_log_level", d.HasChange("logging_config.0.log_format")) || + hasChangeForLoggingConfigLogLevel(d, "logging_config.0.system_log_level", d.HasChange("logging_config.0.log_format")) || d.HasChange("memory_size") || d.HasChange(names.AttrRole) || d.HasChange(names.AttrTimeout) || @@ -1544,7 +1557,11 @@ func flattenLoggingConfig(apiObject *awstypes.LoggingConfig) []map[string]any { // Suppress diff if log levels have not been specified, unless log_format has changed func suppressLoggingConfigUnspecifiedLogLevels(k, old, new string, d *schema.ResourceData) bool { - if d.HasChanges("logging_config.0.log_format") { + return suppressLoggingConfigUnspecifiedLogLevelsPrimitive(k, old, new, d.HasChanges("logging_config.0.log_format")) +} + +func suppressLoggingConfigUnspecifiedLogLevelsPrimitive(k, old, new string, logFormatHasChanges bool) bool { + if logFormatHasChanges { return false } if old != "" && new == "" { From d64f2277776ad42264afc67a33b447d75d43cf5d Mon Sep 17 00:00:00 2001 From: tabito Date: Sun, 18 May 2025 00:26:54 +0900 Subject: [PATCH 2/5] add acctests --- internal/service/lambda/function_test.go | 157 +++++++++++++++++++++++ 1 file changed, 157 insertions(+) diff --git a/internal/service/lambda/function_test.go b/internal/service/lambda/function_test.go index 2150420af95e..cf8c1b275ab0 100644 --- a/internal/service/lambda/function_test.go +++ b/internal/service/lambda/function_test.go @@ -1251,6 +1251,104 @@ func TestAccLambdaFunction_loggingConfig(t *testing.T) { }) } +func TestAccLambdaFunction_loggingConfigWithPublish(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(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.LambdaServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckFunctionDestroy(ctx), + + Steps: []resource.TestStep{ + { + Config: testAccFunctionConfig_loggingConfigWithPublish(rName, "Text"), + Check: resource.ComposeTestCheckFunc( + testAccCheckFunctionExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "publish", acctest.CtTrue), + resource.TestCheckResourceAttr(resourceName, names.AttrVersion, "1"), + resource.TestCheckResourceAttr(resourceName, "logging_config.#", "1"), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.application_log_level", ""), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.log_format", "Text"), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.system_log_level", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"filename", "publish"}, + }, + { + Config: testAccFunctionConfig_loggingConfigWithPublish(rName, "JSON"), + Check: resource.ComposeTestCheckFunc( + testAccCheckFunctionExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "publish", acctest.CtTrue), + resource.TestCheckResourceAttr(resourceName, names.AttrVersion, "2"), + resource.TestCheckResourceAttr(resourceName, "logging_config.#", "1"), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.application_log_level", "INFO"), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.log_format", "JSON"), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.system_log_level", "INFO"), + ), + }, + { + Config: testAccFunctionConfig_loggingConfigWithPublish(rName, "JSON"), + ExpectNonEmptyPlan: false, + PlanOnly: true, + }, + { + Config: testAccFunctionConfig_loggingConfigWithPublishUpdated1(rName, "JSON", "DEBUG"), + Check: resource.ComposeTestCheckFunc( + testAccCheckFunctionExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "publish", acctest.CtTrue), + resource.TestCheckResourceAttr(resourceName, names.AttrVersion, "3"), + resource.TestCheckResourceAttr(resourceName, "logging_config.#", "1"), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.application_log_level", "DEBUG"), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.log_format", "JSON"), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.system_log_level", "INFO"), + ), + }, + { + Config: testAccFunctionConfig_loggingConfigWithPublishUpdated2(rName, "JSON", "WARN"), + Check: resource.ComposeTestCheckFunc( + testAccCheckFunctionExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "publish", acctest.CtTrue), + resource.TestCheckResourceAttr(resourceName, names.AttrVersion, "4"), + resource.TestCheckResourceAttr(resourceName, "logging_config.#", "1"), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.application_log_level", "DEBUG"), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.log_format", "JSON"), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.system_log_level", "WARN"), + ), + }, + { + Config: testAccFunctionConfig_loggingConfigWithPublish(rName, "JSON"), + ExpectNonEmptyPlan: false, + PlanOnly: true, + }, + { + Config: testAccFunctionConfig_loggingConfigWithPublish(rName, "Text"), + Check: resource.ComposeTestCheckFunc( + testAccCheckFunctionExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "publish", acctest.CtTrue), + resource.TestCheckResourceAttr(resourceName, names.AttrVersion, "5"), + resource.TestCheckResourceAttr(resourceName, "logging_config.#", "1"), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.application_log_level", ""), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.log_format", "Text"), + resource.TestCheckResourceAttr(resourceName, "logging_config.0.system_log_level", ""), + ), + }, + { + Config: testAccFunctionConfig_loggingConfigWithPublish(rName, "Text"), + ExpectNonEmptyPlan: false, + PlanOnly: true, + }, + }, + }) +} + func TestAccLambdaFunction_tracing(t *testing.T) { ctx := acctest.Context(t) if testing.Short() { @@ -3385,6 +3483,65 @@ resource "aws_lambda_function" "test" { `, rName)) } +func testAccFunctionConfig_loggingConfigWithPublish(rName, logFormat string) string { + return acctest.ConfigCompose( + acctest.ConfigLambdaBase(rName, rName, rName), + fmt.Sprintf(` +resource "aws_lambda_function" "test" { + filename = "test-fixtures/lambdatest.zip" + function_name = %[1]q + role = aws_iam_role.iam_for_lambda.arn + handler = "exports.example" + runtime = "nodejs20.x" + publish = true + + logging_config { + log_format = %[2]q + } +} +`, rName, logFormat)) +} + +func testAccFunctionConfig_loggingConfigWithPublishUpdated1(rName, logFormat, appLogLevel string) string { + return acctest.ConfigCompose( + acctest.ConfigLambdaBase(rName, rName, rName), + fmt.Sprintf(` +resource "aws_lambda_function" "test" { + filename = "test-fixtures/lambdatest.zip" + function_name = %[1]q + role = aws_iam_role.iam_for_lambda.arn + handler = "exports.example" + runtime = "nodejs20.x" + publish = true + + logging_config { + log_format = %[2]q + application_log_level = %[3]q + } +} +`, rName, logFormat, appLogLevel)) +} + +func testAccFunctionConfig_loggingConfigWithPublishUpdated2(rName, logFormat, sysLogLevel string) string { + return acctest.ConfigCompose( + acctest.ConfigLambdaBase(rName, rName, rName), + fmt.Sprintf(` +resource "aws_lambda_function" "test" { + filename = "test-fixtures/lambdatest.zip" + function_name = %[1]q + role = aws_iam_role.iam_for_lambda.arn + handler = "exports.example" + runtime = "nodejs20.x" + publish = true + + logging_config { + log_format = %[2]q + system_log_level = %[3]q + } +} +`, rName, logFormat, sysLogLevel)) +} + func testAccFunctionConfig_tracing(rName string) string { return acctest.ConfigCompose( acctest.ConfigLambdaBase(rName, rName, rName), From 1d6bfcfc3e44c7ed61254367dd35acee6a611a27 Mon Sep 17 00:00:00 2001 From: tabito Date: Sun, 18 May 2025 01:02:28 +0900 Subject: [PATCH 3/5] add changelog --- .changelog/42660.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/42660.txt diff --git a/.changelog/42660.txt b/.changelog/42660.txt new file mode 100644 index 000000000000..c2b0f4e8cb9d --- /dev/null +++ b/.changelog/42660.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_lambda_function: Fixed an issue where persistent diffs appeared in log level arguments when `log_format` in `logging_config` was set to `JSON` and `publish = true` +``` From 9a837dc7a8906ac3c37ffcb5ec0c01a45d792eb3 Mon Sep 17 00:00:00 2001 From: tabito Date: Sun, 18 May 2025 01:18:43 +0900 Subject: [PATCH 4/5] add nolint --- internal/service/lambda/function.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/lambda/function.go b/internal/service/lambda/function.go index 4864f4aa4cb8..951dd9b1d614 100644 --- a/internal/service/lambda/function.go +++ b/internal/service/lambda/function.go @@ -1560,7 +1560,7 @@ func suppressLoggingConfigUnspecifiedLogLevels(k, old, new string, d *schema.Res return suppressLoggingConfigUnspecifiedLogLevelsPrimitive(k, old, new, d.HasChanges("logging_config.0.log_format")) } -func suppressLoggingConfigUnspecifiedLogLevelsPrimitive(k, old, new string, logFormatHasChanges bool) bool { +func suppressLoggingConfigUnspecifiedLogLevelsPrimitive(k, old, new string, logFormatHasChanges bool) bool { //nolint:unparam if logFormatHasChanges { return false } From 9d9f547b4bea27e90bf6a4a2bf0967cb91b6c749 Mon Sep 17 00:00:00 2001 From: tabito Date: Sun, 18 May 2025 01:46:42 +0900 Subject: [PATCH 5/5] fix acctest 'versionedUpdate' --- internal/service/lambda/function_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/service/lambda/function_test.go b/internal/service/lambda/function_test.go index cf8c1b275ab0..9188b37f995e 100644 --- a/internal/service/lambda/function_test.go +++ b/internal/service/lambda/function_test.go @@ -635,12 +635,12 @@ func TestAccLambdaFunction_versionedUpdate(t *testing.T) { PreConfig: func() { timeBeforeUpdate = time.Now() }, - Config: testAccFunctionConfig_versionedNodeJs20xRuntime(path, rName), + Config: testAccFunctionConfig_versionedNodeJs22xRuntime(path, rName), Check: resource.ComposeTestCheckFunc( testAccCheckFunctionExists(ctx, resourceName, &conf), resource.TestCheckResourceAttr(resourceName, names.AttrVersion, versionUpdated), acctest.CheckResourceAttrRegionalARN(ctx, resourceName, "qualified_arn", "lambda", fmt.Sprintf("function:%s:%s", rName, versionUpdated)), - resource.TestCheckResourceAttr(resourceName, "runtime", string(awstypes.RuntimeNodejs20x)), + resource.TestCheckResourceAttr(resourceName, "runtime", string(awstypes.RuntimeNodejs22x)), func(s *terraform.State) error { return testAccCheckAttributeIsDateAfter(s, resourceName, "last_modified", timeBeforeUpdate) }, @@ -3071,7 +3071,7 @@ resource "aws_lambda_function" "test" { `, fileName, rName, publish)) } -func testAccFunctionConfig_versionedNodeJs20xRuntime(fileName, rName string) string { +func testAccFunctionConfig_versionedNodeJs22xRuntime(fileName, rName string) string { return acctest.ConfigCompose( acctest.ConfigLambdaBase(rName, rName, rName), fmt.Sprintf(` @@ -3081,7 +3081,7 @@ resource "aws_lambda_function" "test" { publish = true role = aws_iam_role.iam_for_lambda.arn handler = "exports.example" - runtime = "nodejs20.x" + runtime = "nodejs22.x" } `, fileName, rName)) }