-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
feat(datazone): add domain and environment blueprint configuration resources #36600
feat(datazone): add domain and environment blueprint configuration resources #36600
Conversation
Community NoteVoting for Prioritization
For Submitters
|
376e155
to
867c93e
Compare
3ba66fa
to
fb99040
Compare
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.
Changes requested. I did not have time to review all files.
|
||
func (r *resourceDomain) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) { | ||
resp.Schema = schema.Schema{ | ||
Attributes: map[string]schema.Attribute{ |
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.
Attributes should be defined in alphabetical order (not the order that the AWS API defines them in).
}, | ||
"portal_url": schema.StringAttribute{ | ||
Computed: true, | ||
PlanModifiers: []planmodifier.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.
Can you explain why it was necessary to use stringplanmodifier.UseStateForUnknown(),
?
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.
It's to save explicitly updating the state from the plan for this computed value in Update
- if it's preferred that I do that I'm happy to update it though!
internal/service/datazone/domain.go
Outdated
} | ||
|
||
in := &datazone.CreateDomainInput{ | ||
Name: aws.String(plan.Name.ValueString()), |
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.
Please alphabetize input parameters
internal/service/datazone/domain.go
Outdated
in.SingleSignOn = expandSingleSignOn(tfList) | ||
} | ||
|
||
out, err := retryCreateDomain(ctx, conn, in, CreateNumRetries) |
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 typically prefer the retry logic to be inline, not in another function.
You can see a recent pattern for this here in the Bedrock Custom Model resource.
outputRaw, err := tfresource.RetryWhenAWSErrMessageContains(ctx, propagationTimeout, func() (interface{}, error) { |
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 awesome thanks for pointing me to an example! Much cleaner 😄
internal/service/datazone/domain.go
Outdated
return out, nil | ||
} | ||
|
||
func isResourceMissing(err error) bool { |
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.
Well done! I love the conciseness of this.
internal/service/datazone/domain.go
Outdated
return apiObject | ||
} | ||
|
||
type resourceDomainData struct { |
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.
Naming convention is normally:
Model.
example: customModelResourceModel
type resourceDomainData struct { | |
type domainResourceModel struct { |
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 awesome, updated! :)
// Copyright (c) HashiCorp, Inc. | ||
// SPDX-License-Identifier: MPL-2.0 | ||
|
||
package datazone_test |
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.
Really nice job on the AccTests! The use of _basic for all required attributes, and then individual tests for each other attribute/feature provide lasting maintainability that is forward compatable.
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.
Thank you! 😄
Required: true, | ||
}, | ||
"manage_access_role_arn": schema.StringAttribute{ | ||
Optional: true, |
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.
Optional: true, | |
CustomType: fwtypes.ARNType, | |
Optional: true, |
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.
Thanks! Updated :)
Optional: true, | ||
}, | ||
"provisioning_role_arn": schema.StringAttribute{ | ||
Optional: true, |
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.
Optional: true, | |
CustomType: fwtypes.ARNType, | |
Optional: true, |
…sources Add resources for DataZone: - Domain - Environment Blueprint Configuration Additionally add a data source for Environment Blueprints in order to retrieve the IDs of managed blueprints without hardcoding. Relates hashicorp#33792
fb99040
to
4c9c614
Compare
Looking forward to Terraform support for Amazon Datazone. |
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.
@cogwirrel can you please re-run all of the acceptance tests and provide the outputs?
|
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 🚀
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 🎉
% make t T=TestAccDataZone K=datazone
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.2 test ./internal/service/datazone/... -v -count 1 -parallel 20 -run='TestAccDataZone' -timeout 360m
=== RUN TestAccDataZoneDomain_basic
=== PAUSE TestAccDataZoneDomain_basic
=== RUN TestAccDataZoneDomain_disappears
=== PAUSE TestAccDataZoneDomain_disappears
=== RUN TestAccDataZoneDomain_kms_key_identifier
=== PAUSE TestAccDataZoneDomain_kms_key_identifier
=== RUN TestAccDataZoneDomain_description
=== PAUSE TestAccDataZoneDomain_description
=== RUN TestAccDataZoneDomain_single_sign_on
=== PAUSE TestAccDataZoneDomain_single_sign_on
=== RUN TestAccDataZoneDomain_tags
=== PAUSE TestAccDataZoneDomain_tags
=== RUN TestAccDataZoneEnvironmentBlueprintConfiguration_basic
=== PAUSE TestAccDataZoneEnvironmentBlueprintConfiguration_basic
=== RUN TestAccDataZoneEnvironmentBlueprintConfiguration_disappears
=== PAUSE TestAccDataZoneEnvironmentBlueprintConfiguration_disappears
=== RUN TestAccDataZoneEnvironmentBlueprintConfiguration_enabled_regions
=== PAUSE TestAccDataZoneEnvironmentBlueprintConfiguration_enabled_regions
=== RUN TestAccDataZoneEnvironmentBlueprintConfiguration_manage_access_role_arn
=== PAUSE TestAccDataZoneEnvironmentBlueprintConfiguration_manage_access_role_arn
=== RUN TestAccDataZoneEnvironmentBlueprintConfiguration_provisioning_role_arn
=== PAUSE TestAccDataZoneEnvironmentBlueprintConfiguration_provisioning_role_arn
=== RUN TestAccDataZoneEnvironmentBlueprintConfiguration_regional_parameters
=== PAUSE TestAccDataZoneEnvironmentBlueprintConfiguration_regional_parameters
=== RUN TestAccDataZoneEnvironmentBlueprintDataSource_basic
=== PAUSE TestAccDataZoneEnvironmentBlueprintDataSource_basic
=== CONT TestAccDataZoneDomain_basic
=== CONT TestAccDataZoneEnvironmentBlueprintConfiguration_disappears
=== CONT TestAccDataZoneDomain_single_sign_on
=== CONT TestAccDataZoneDomain_disappears
=== CONT TestAccDataZoneEnvironmentBlueprintConfiguration_manage_access_role_arn
=== CONT TestAccDataZoneEnvironmentBlueprintConfiguration_enabled_regions
=== CONT TestAccDataZoneEnvironmentBlueprintDataSource_basic
=== CONT TestAccDataZoneDomain_kms_key_identifier
=== CONT TestAccDataZoneEnvironmentBlueprintConfiguration_regional_parameters
=== CONT TestAccDataZoneDomain_description
=== CONT TestAccDataZoneEnvironmentBlueprintConfiguration_basic
=== CONT TestAccDataZoneDomain_tags
=== CONT TestAccDataZoneEnvironmentBlueprintConfiguration_provisioning_role_arn
--- PASS: TestAccDataZoneDomain_disappears (43.25s)
--- PASS: TestAccDataZoneEnvironmentBlueprintDataSource_basic (43.40s)
--- PASS: TestAccDataZoneDomain_single_sign_on (45.28s)
--- PASS: TestAccDataZoneDomain_description (46.07s)
--- PASS: TestAccDataZoneEnvironmentBlueprintConfiguration_disappears (47.39s)
--- PASS: TestAccDataZoneDomain_basic (47.78s)
--- PASS: TestAccDataZoneDomain_kms_key_identifier (49.15s)
--- PASS: TestAccDataZoneEnvironmentBlueprintConfiguration_manage_access_role_arn (51.09s)
--- PASS: TestAccDataZoneEnvironmentBlueprintConfiguration_basic (51.34s)
--- PASS: TestAccDataZoneEnvironmentBlueprintConfiguration_provisioning_role_arn (51.52s)
--- PASS: TestAccDataZoneEnvironmentBlueprintConfiguration_regional_parameters (62.79s)
--- PASS: TestAccDataZoneEnvironmentBlueprintConfiguration_enabled_regions (65.12s)
--- PASS: TestAccDataZoneDomain_tags (72.36s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/datazone 74.834s
This functionality has been released in v5.49.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. Thank you! |
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. |
Description
Add resources for DataZone:
Additionally add a data source for Environment Blueprints in order to retrieve the IDs of managed blueprints without hardcoding.
Relations
Relates #33792
References
https://docs.aws.amazon.com/datazone/latest/userguide/datazone-concepts.html
Output from Acceptance Testing