issue #3158 add new argument additional_info in aws_emr_cluster #4590
Conversation
aws/resource_aws_emr_cluster.go
Outdated
| "additional_info": { | ||
| Type: schema.TypeString, | ||
| Optional: true, | ||
| Computed: true, |
There was a problem hiding this comment.
This attribute should not be Computed: true - we want to know when there is drift from the API
| @@ -42,6 +43,17 @@ func resourceAwsEMRCluster() *schema.Resource { | |||
| Optional: true, | |||
| ForceNew: true, | |||
| }, | |||
| "additional_info": { | |||
There was a problem hiding this comment.
We need to:
- Add a new acceptance test and test configuration for this new attribute
- Read it back into the Terraform state via
d.Set("additional_info", XXXX)
As for the schema itself, we most likely need to add DiffSuppressFunc: suppressEquivalentJsonDiffs,
There was a problem hiding this comment.
done all those changes 👍
aws/resource_aws_emr_cluster.go
Outdated
| if v, ok := d.GetOk("additional_info"); ok { | ||
| info, err := structure.NormalizeJsonString(v) | ||
| if err != nil { | ||
| return errwrap.Wrapf("Additional Info contains an invalid JSON: {{err}}", err) |
There was a problem hiding this comment.
We should avoid the usage of errwrap.Wrapf() in resource functions in favor of the standard fmt.Errorf() -- we will be going through and cleaning up all the existing ones at some point.
There was a problem hiding this comment.
changed to fmt.Errorf
bflad
added
enhancement
ghost
added
size/L
|
@bflad I checked the AWS API documentation, looks like there is no option to read back the additional info right now. I tried in AWS CLI too. So I think we can note it in our documentation. please advise how? |
|
A simple sentence with the new argument documentation is fine, something like |
|
@bflad thanks, updated documentation. |
|
|
||
| scale_down_behavior = "TERMINATE_AT_TASK_COMPLETION" | ||
|
|
||
| bootstrap_action { |
There was a problem hiding this comment.
Future note: we should try to only add the required arguments in test configurations to prevent false positives with other arguments/resources (in this case this resource configuration should only have additional_info and the required resource attributes)
There was a problem hiding this comment.
LGTM -- thanks! 🚀
14 tests passed (all tests)
=== RUN TestAccAWSEMRCluster_basic
--- PASS: TestAccAWSEMRCluster_basic (470.52s)
=== RUN TestAccAWSEMRCluster_instance_group
--- PASS: TestAccAWSEMRCluster_instance_group (522.05s)
=== RUN TestAccAWSEMRCluster_security_config
--- PASS: TestAccAWSEMRCluster_security_config (570.45s)
=== RUN TestAccAWSEMRCluster_additionalInfo
--- PASS: TestAccAWSEMRCluster_additionalInfo (577.58s)
=== RUN TestAccAWSEMRCluster_Kerberos_ClusterDedicatedKdc
--- PASS: TestAccAWSEMRCluster_Kerberos_ClusterDedicatedKdc (581.90s)
=== RUN TestAccAWSEMRCluster_bootstrap_ordering
--- PASS: TestAccAWSEMRCluster_bootstrap_ordering (590.39s)
=== RUN TestAccAWSEMRCluster_Step_Multiple
--- PASS: TestAccAWSEMRCluster_Step_Multiple (616.49s)
=== RUN TestAccAWSEMRCluster_custom_ami_id
--- PASS: TestAccAWSEMRCluster_custom_ami_id (647.99s)
=== RUN TestAccAWSEMRCluster_Step_Basic
--- PASS: TestAccAWSEMRCluster_Step_Basic (678.18s)
=== RUN TestAccAWSEMRCluster_terminationProtected
--- PASS: TestAccAWSEMRCluster_terminationProtected (698.93s)
=== RUN TestAccAWSEMRCluster_s3Logging
--- PASS: TestAccAWSEMRCluster_s3Logging (860.46s)
=== RUN TestAccAWSEMRCluster_root_volume_size
--- PASS: TestAccAWSEMRCluster_root_volume_size (924.50s)
=== RUN TestAccAWSEMRCluster_visibleToAllUsers
--- PASS: TestAccAWSEMRCluster_visibleToAllUsers (962.58s)
=== RUN TestAccAWSEMRCluster_tags
--- PASS: TestAccAWSEMRCluster_tags (1030.03s)
|
This has been released in version 1.20.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
|
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! |
ghost
locked and limited conversation to collaborators
Fixes #3158