-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
New Resource: aws_imagebuilder_distribution_configuration #16180
Conversation
# Conflicts: # aws/provider.go
# Conflicts: # aws/provider.go
# Conflicts: # aws/provider.go
…ing set on create
fixed lint issues and terminate_instance_on_failure
…vider-aws into Dogers-image-builder
Reference: #11084 Reference: #13316 Reference: #13485 Changes: ``` * **New Data Source:** `aws_imagebuilder_distribution_configuration` * **New Resource:** `aws_imagebuilder_distribution_configuration` ``` Output from acceptance testing in AWS Commercial: ``` --- PASS: TestAccAwsImageBuilderDistributionConfiguration_basic (23.95s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Description (39.52s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_disappears (18.57s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution (42.99s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_AmiTags (40.02s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_Description (39.36s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_KmsKeyId (46.31s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_LaunchPermission_UserGroups (23.66s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_LaunchPermission_UserIds (40.12s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_Name (39.12s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_TargetAccountIds (40.12s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_LicenseConfigurationArns (40.75s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Tags (49.75s) --- PASS: TestAccAwsImageBuilderDistributionConfigurationDataSource_Arn (18.52s) ``` Output from acceptance testing in AWS GovCloud (US) (failure information: #16178): ``` --- FAIL: TestAccAwsImageBuilderDistributionConfiguration_Distribution_LicenseConfigurationArns (10.88s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_basic (27.00s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Description (40.22s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_disappears (21.76s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_AmiTags (42.09s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_Description (42.89s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_KmsKeyId (51.80s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_LaunchPermission_UserGroups (26.70s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_LaunchPermission_UserIds (41.44s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_Name (42.32s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_TargetAccountIds (42.42s) --- PASS: TestAccAwsImageBuilderDistributionConfiguration_Tags (55.37s) --- PASS: TestAccAwsImageBuilderDistributionConfigurationDataSource_Arn (25.62s) ``` Output from sweeper in AWS Commercial: ``` 2020/11/13 09:11:34 [DEBUG] Running Sweepers for region (us-west-2): 2020/11/13 09:11:34 [DEBUG] Running Sweeper (aws_imagebuilder_distribution_configuration) in region (us-west-2) 2020/11/13 09:11:37 Sweeper Tests ran successfully: - aws_imagebuilder_distribution_configuration 2020/11/13 09:11:37 [DEBUG] Running Sweepers for region (us-east-1): 2020/11/13 09:11:37 [DEBUG] Running Sweeper (aws_imagebuilder_distribution_configuration) in region (us-east-1) 2020/11/13 09:11:39 Sweeper Tests ran successfully: - aws_imagebuilder_distribution_configuration ok github.com/terraform-providers/terraform-provider-aws/aws 7.724s ``` Output from sweeper in AWS GovCloud (US): ``` 2020/11/13 09:30:11 [DEBUG] Running Sweepers for region (us-gov-west-1): 2020/11/13 09:30:11 [DEBUG] Running Sweeper (aws_imagebuilder_distribution_configuration) in region (us-gov-west-1) 2020/11/13 09:30:14 Sweeper Tests ran successfully: - aws_imagebuilder_distribution_configuration ok github.com/terraform-providers/terraform-provider-aws/aws 4.971s ```
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.
LGTM 🚀
A couple cases of fmt.Errorf("...%s", ..., err)
and one suggestion about documentation
Commercial partition:
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_disappears (63.45s)
--- PASS: TestAccAwsImageBuilderDistributionConfigurationDataSource_Arn (70.07s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_LaunchPermission_UserGroups (72.58s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_basic (80.07s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution (92.29s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Description (113.19s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_Description (113.97s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_AmiTags (114.52s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_KmsKeyId (116.19s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_Name (116.45s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_LaunchPermission_UserIds (116.49s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_TargetAccountIds (116.82s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_LicenseConfigurationArns (116.91s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Tags (127.77s)
GovCloud partition:
--- FAIL: TestAccAwsImageBuilderDistributionConfiguration_Distribution_LicenseConfigurationArns (14.72s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_disappears (35.22s)
--- PASS: TestAccAwsImageBuilderDistributionConfigurationDataSource_Arn (39.57s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_LaunchPermission_UserGroups (41.72s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_basic (43.41s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution (55.15s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_Description (62.69s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_LaunchPermission_UserIds (62.76s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_KmsKeyId (62.74s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_AmiTags (62.78s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Description (63.22s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_TargetAccountIds (63.05s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Distribution_AmiDistributionConfiguration_Name (63.45s)
--- PASS: TestAccAwsImageBuilderDistributionConfiguration_Tags (68.12s)
Failure addressed by #16178
if v, ok := d.GetOk("arn"); ok { | ||
input.DistributionConfigurationArn = aws.String(v.(string)) | ||
} |
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.
Do we need this check, since arn
is required?
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.
Treating all the fields as potentially "optional" in the Create/Update function logic will allow us to reduce code churn in the future should the APIs support other fields and gets us closer towards code generation since it can be agnostic to how the data is coming in. It is just a little awkward right now because we do (or at least should) treat everything as "optional" in the expand functions but the root attribute handling is different with ResourceData
instead of a raw state map.
I'm hoping later this week to submit more definitive documentation on ideal object/"structure" handling in the provider codebase, with an eye towards that sort of code generation in the future, so we can have clear standards where possible. 😄
The following arguments are required: | ||
|
||
* `name` - (Required) Name of the distribution configuration. | ||
* `distribution` - (Required) One or more configuration blocks with distribution settings. Detailed 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 "Detailed below" here and elsewhere should be links to the corresponding sections of the document.
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.
Can certainly add this here, but we will want a tracking issue for the rest of the codebase.
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.
Do you know what the expected anchor link is for these since they are secondary with the same link? #distribution-1
?
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.
Ah looking at https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lex_bot, the anchors all overlap so the header anchor and attribute anchor for #message
are not distinguished. I'd say create an issue but we'll need the registry updated first to not generate overlapping anchors.
This has been released in version 3.16.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks! |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
Community Note
Reference: #11084
Closes #13316
Reference: #13485
Release note for CHANGELOG:
Output from acceptance testing in AWS Commercial:
Output from acceptance testing in AWS GovCloud (US) (failure information: #16178):
Output from sweeper in AWS Commercial:
Output from sweeper in AWS GovCloud (US):