Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
r/aws_backup_selection: Adding resource to manage selections for AWS Backup plans #7382
r/aws_backup_selection: Adding resource to manage selections for AWS Backup plans #7382
Changes from 5 commits
859d751
9584ff2
efb62f3
f6264b4
c33b851
7884809
33efb69
4d2e478
8d4ee52
5132ea5
c80dd6f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should simplify this attribute by removing the custom
Set
function and allowing Terraform to use its default.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it matter if this resource is using these tags as a parameter to make backup plan selections vs. the normal tag/untag operations that Terraform handles? That's what I noticed when I first planned this out but I will admit I didn't try using the default functions first because I thought they wouldn't work in this context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must admit I'm not sure what you're referring to? 😅 All Terraform schema definitions are unrelated and the default
TypeSet
Set
function applies to anyTypeSet
attribute. We happen to use atags
TypeMap
for most Terraform AWS Provider resources as the resource tagging attribute, but that is only a convention to make it simpler for operators.If you would like to clear up any confusion between a
tag
configuration block here and the typicaltags
map argument across the provider, you can always switch the naming here toselection_tag
or something. It won't have any bearing on theTypeSet
Set
function either which way though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I have this working without the custom hash function this all makes sense. I think I misinterpreted your initial comment to imply that I use
tagSchema()
which doesn't make sense. Oops! Anyway, thanks for the clarification :-)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API seems to be unordered for this attribute:
So this can be switched to
TypeSet
👍There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use a
[]interface{}
variable here instead withappend()
below 👍There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The acceptance testing runs in
us-west-2
by default:https://github.com/terraform-providers/terraform-provider-aws/blob/26fea575f2100d68307e063a8b1c590d8de622d6/aws/provider_test.go#L181-L185
We generally prefer using the
aws_partition
andaws_region
data sources in this case to make the testing region and partition agnostic: