[WIP] - Allow suppressed_reasons to be an empty list in order to overwrite the default suppressed reasons

[bschaatsbergen]

Closes #28669

Description

Relations

Closes #0000

References

Output from Acceptance Testing

$ make testacc TESTS=TestAccXXX PKG=ec2

...

[github-actions]

Community Note

Voting for Prioritization

• Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
• Please see our prioritization guide for information on how we prioritize.
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

• Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
• For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
• Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

[F21]

I made some attempts to try and allow empty lists for the suppression_reasons attribute in a local build, but it seems to be a lot more involved than removing MinItems here: https://github.com/hashicorp/terraform-provider-aws/blob/main/internal/service/sesv2/configuration_set.go#L110

For some reason, the value from d.GetOk("suppression_options") is [<nil>], rather than [map[suppressed_reasons:[]]] when setting suppression reasons to an empty list here: https://github.com/hashicorp/terraform-provider-aws/blob/main/internal/service/sesv2/configuration_set.go#L163

From testing against my AWS account, just removing MinItems did not override the account level suppression list settings correctly.

[bschaatsbergen]

I've put out the draft and will follow up this week @F21, thanks for the detailed input.

[F21]

Thanks for looking at this @bschaatsbergen! I spent most of yesterday looking at this and it was driving me mad, glad to have a second pair of eyes looking at this.

[F21]

I think the problem is because terraform-plugin-sdk considers zero values and null to be the same. hashicorp/terraform-plugin-sdk#741 offers more info and a workaround.

[bschaatsbergen]

Any suggestions on the above @ewbankkit ?

[ewbankkit]

Try changing expandSuppressionOptions

terraform-provider-aws/internal/service/sesv2/configuration_set.go

Lines 569 to 581 in 5fd8df3

	func expandSuppressionOptions(tfMap map[string]interface{}) *types.SuppressionOptions {
	if tfMap == nil {
	return nil
	}
	a := &types.SuppressionOptions{}
	if v, ok := tfMap["suppressed_reasons"].([]interface{}); ok && len(v) > 0 {
	a.SuppressedReasons = expandSuppressedReasons(v)
	}
	return a
	}

to

func expandSuppressionOptions(tfMap map[string]interface{}) *types.SuppressionOptions {
	if tfMap == nil {
		return nil
	}

	a := &types.SuppressionOptions{}

	if v, ok := tfMap["suppressed_reasons"].([]interface{}); ok {
		if len(v) > 0 {
			a.SuppressedReasons = expandSuppressedReasons(v)
		} else {
			a.SuppressedReasons = []types.SuppressionListReason
		}
	}

	return a
}

[bschaatsbergen]

@ewbankkit - I have the feeling that the SDK checks whether SuppressedReasons is empty or not, as when setting an empty slice it doesn't seem to become part of the CreateConfigurationSet API call.

  "requestParameters": {
        "configurationSetName": "tf-acc-test-4803928077497064113"
    },

[bschaatsbergen]

Hi @F21, I haven't looked at this PR in a while. Have you made any progress on your side with this?

[F21]

@bschaatsbergen My PR #29671 fixes the issue, but I am still waiting for it to be reviewed.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.