Skip to content
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

Add tags argument to resource/aws_aws_organizations_organizational_unit (#15476) #15615

Closed
wants to merge 4 commits into from

Conversation

ryno75
Copy link
Contributor

@ryno75 ryno75 commented Oct 13, 2020

Community Note

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

Closes #15476

Release note for CHANGELOG:

ENHANCEMENTS

* resource/aws_organizations_organizational_unit: Add `tags` argument ([#15476](https:/terraform-providers/terraform-provider-aws/issues/15476))

Output from acceptance testing:

❯ make testacc TESTARGS='-run=TestAccAWSOrganizations/OrganizationalUnit'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSOrganizations/OrganizationalUnit -timeout 120m
=== RUN   TestAccAWSOrganizations_serial
=== RUN   TestAccAWSOrganizations_serial/OrganizationalUnit
=== RUN   TestAccAWSOrganizations_serial/OrganizationalUnit/Tags
=== RUN   TestAccAWSOrganizations_serial/OrganizationalUnit/basic
=== RUN   TestAccAWSOrganizations_serial/OrganizationalUnit/Name
=== RUN   TestAccAWSOrganizations_serial/OrganizationalUnits
=== RUN   TestAccAWSOrganizations_serial/OrganizationalUnits/DataSource
=== PAUSE TestAccAWSOrganizations_serial/OrganizationalUnits/DataSource
=== CONT  TestAccAWSOrganizations_serial/OrganizationalUnits/DataSource
--- PASS: TestAccAWSOrganizations_serial (275.65s)
    --- PASS: TestAccAWSOrganizations_serial/OrganizationalUnit (233.46s)
        --- PASS: TestAccAWSOrganizations_serial/OrganizationalUnit/Tags (110.02s)
        --- PASS: TestAccAWSOrganizations_serial/OrganizationalUnit/basic (46.11s)
        --- PASS: TestAccAWSOrganizations_serial/OrganizationalUnit/Name (77.33s)
    --- PASS: TestAccAWSOrganizations_serial/OrganizationalUnits (0.00s)
        --- PASS: TestAccAWSOrganizations_serial/OrganizationalUnits/DataSource (42.19s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	277.660s

@ryno75 ryno75 requested a review from a team October 13, 2020 07:20
@ghost ghost added size/M Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. service/organizations Issues and PRs that pertain to the organizations service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Oct 13, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Oct 13, 2020
@anGie44 anGie44 added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Oct 13, 2020
@dzoeteman
Copy link

Could you add the tags to the Create action? This would allow the use of aws:RequestTag condition for example on IAM policies for terraform.

@ryno75
Copy link
Contributor Author

ryno75 commented Nov 30, 2020

Could you add the tags to the Create action? This would allow the use of aws:RequestTag condition for example on IAM policies for terraform.

Not sure I understand. The tags are part of the Create action under this PR...

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.OrganizationsUpdateTags(conn, d.Id(), nil, v); err != nil {
return fmt.Errorf("Error adding Organizational Unit (%s) tags: %s", d.Id(), err)
}
}

Are you referring to something else?

Base automatically changed from master to main January 23, 2021 00:59
@breathingdust breathingdust requested a review from a team as a code owner January 23, 2021 00:59
@anGie44 anGie44 self-assigned this Apr 14, 2021
Copy link
Contributor

@anGie44 anGie44 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ryno75 , thank you for this PR! changes overall look great just needs a rebase and my only note is that when setting tags on create, we can directly set those in the CreateOrganizationalUnitInput struct rather than call the UpdateTags (as originally suggested by @dzoeteman i believe 😄 ), tho that certainly works as well. Additionally, could you please add a .changelog/15615.txt file w/an entry similar to https:/hashicorp/terraform-provider-aws/blob/main/docs/contributing/pullrequest-submission-and-lifecycle.md#resource-and-provider-enhancements ?

}
log.Printf("[DEBUG] Organizational Unit create response: %#v", resp)

// Store the ID
ouId := resp.OrganizationalUnit.Id
d.SetId(*ouId)

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
Copy link
Contributor

@anGie44 anGie44 Apr 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should alternatively add tags w/in the CreateInput struct above e.g.

Suggested change
if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
// Create the organizational unit
createOpts := &organizations.CreateOrganizationalUnitInput{
Name: aws.String(d.Get("name").(string)),
ParentId: aws.String(d.Get("parent_id").(string)),
Tags: keyvaluetags.New(d.Get("tags").(map[string]interface{})).IgnoreAws().OrganizationsTags(),
}

@anGie44 anGie44 added the waiting-response Maintainers are waiting on response from community or contributor. label Apr 14, 2021
@anGie44
Copy link
Contributor

anGie44 commented Apr 22, 2021

Hi @ryno75, thank you again for this PR! Since time has passed since my last review, I've continued your work in #18861 to get the PR reviewed by another maintainer and into a future provider release, it includes your commits and addresses the comment above.

@anGie44 anGie44 closed this Apr 22, 2021
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 22, 2021
@breathingdust breathingdust removed the waiting-response Maintainers are waiting on response from community or contributor. label Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/organizations Issues and PRs that pertain to the organizations service. size/M Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tags attribute support for aws_organizations_organizational_unit
4 participants