Skip to content

Commit

Permalink
Merge pull request #15473 from ryandeivert/b-aws_organizations_organi…
Browse files Browse the repository at this point in the history
…zation-15462

adding fix for aws org feature_set update issue from #15462
  • Loading branch information
anGie44 authored Nov 13, 2020
2 parents c5ee2b7 + 5b418bb commit 405133c
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 18 deletions.
36 changes: 21 additions & 15 deletions aws/resource_aws_organizations_organization.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package aws

import (
"context"
"fmt"
"log"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/organizations"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
Expand All @@ -24,6 +26,13 @@ func resourceAwsOrganizationsOrganization() *schema.Resource {
State: schema.ImportStatePassthrough,
},

CustomizeDiff: customdiff.Sequence(
customdiff.ForceNewIfChange("feature_set", func(_ context.Context, old, new, meta interface{}) bool {
// Only changes from ALL to CONSOLIDATED_BILLING for feature_set should force a new resource
return old.(string) == organizations.OrganizationFeatureSetAll && new.(string) == organizations.OrganizationFeatureSetConsolidatedBilling
}),
),

Schema: map[string]*schema.Schema{
"arn": {
Type: schema.TypeString,
Expand Down Expand Up @@ -142,24 +151,15 @@ func resourceAwsOrganizationsOrganization() *schema.Resource {
Type: schema.TypeSet,
Optional: true,
Elem: &schema.Schema{
Type: schema.TypeString,
ValidateFunc: validation.StringInSlice([]string{
organizations.PolicyTypeAiservicesOptOutPolicy,
organizations.PolicyTypeBackupPolicy,
organizations.PolicyTypeServiceControlPolicy,
organizations.PolicyTypeTagPolicy,
}, false),
Type: schema.TypeString,
ValidateFunc: validation.StringInSlice(organizations.PolicyType_Values(), false),
},
},
"feature_set": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Default: organizations.OrganizationFeatureSetAll,
ValidateFunc: validation.StringInSlice([]string{
organizations.OrganizationFeatureSetAll,
organizations.OrganizationFeatureSetConsolidatedBilling,
}, true),
Type: schema.TypeString,
Optional: true,
Default: organizations.OrganizationFeatureSetAll,
ValidateFunc: validation.StringInSlice(organizations.OrganizationFeatureSet_Values(), true),
},
},
}
Expand Down Expand Up @@ -400,6 +400,12 @@ func resourceAwsOrganizationsOrganizationUpdate(d *schema.ResourceData, meta int
}
}

if d.HasChange("feature_set") {
if _, err := conn.EnableAllFeatures(&organizations.EnableAllFeaturesInput{}); err != nil {
return fmt.Errorf("error enabling all features in Organization (%s): %w", d.Id(), err)
}
}

return resourceAwsOrganizationsOrganizationRead(d, meta)
}

Expand Down
92 changes: 90 additions & 2 deletions aws/resource_aws_organizations_organization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,76 @@ func testAccAwsOrganizationsOrganization_FeatureSet(t *testing.T) {
})
}

func testAccAwsOrganizationsOrganization_FeatureSetForcesNew(t *testing.T) {
var beforeValue, afterValue organizations.Organization
resourceName := "aws_organizations_organization.test"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAwsOrganizationsOrganizationDestroy,
Steps: []resource.TestStep{
{
Config: testAccAwsOrganizationsOrganizationConfigFeatureSet(organizations.OrganizationFeatureSetAll),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsOrganizationsOrganizationExists(resourceName, &beforeValue),
resource.TestCheckResourceAttr(resourceName, "feature_set", organizations.OrganizationFeatureSetAll),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccAwsOrganizationsOrganizationConfigFeatureSet(organizations.OrganizationFeatureSetConsolidatedBilling),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsOrganizationsOrganizationExists(resourceName, &afterValue),
resource.TestCheckResourceAttr(resourceName, "feature_set", organizations.OrganizationFeatureSetConsolidatedBilling),
testAccAwsOrganizationsOrganizationRecreated(&beforeValue, &afterValue),
),
},
},
})
}

func testAccAwsOrganizationsOrganization_FeatureSetUpdate(t *testing.T) {
var beforeValue, afterValue organizations.Organization
resourceName := "aws_organizations_organization.test"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAwsOrganizationsOrganizationDestroy,
Steps: []resource.TestStep{
{
Config: testAccAwsOrganizationsOrganizationConfigFeatureSet(organizations.OrganizationFeatureSetConsolidatedBilling),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsOrganizationsOrganizationExists(resourceName, &beforeValue),
resource.TestCheckResourceAttr(resourceName, "feature_set", organizations.OrganizationFeatureSetConsolidatedBilling),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccAwsOrganizationsOrganizationConfigFeatureSet(organizations.OrganizationFeatureSetAll),
ExpectNonEmptyPlan: true, // See note below on this perpetual difference
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsOrganizationsOrganizationExists(resourceName, &afterValue),
// The below check cannot be performed here because the user must confirm the change
// via Console. Until then, the FeatureSet will not actually be toggled to ALL
// and will continue to show as CONSOLIDATED_BILLING when calling DescribeOrganization
// resource.TestCheckResourceAttr(resourceName, "feature_set", organizations.OrganizationFeatureSetAll),
testAccAwsOrganizationsOrganizationNotRecreated(&beforeValue, &afterValue),
),
},
},
})
}

func testAccCheckAwsOrganizationsOrganizationDestroy(s *terraform.State) error {
conn := testAccProvider.Meta().(*AWSClient).organizationsconn

Expand Down Expand Up @@ -232,7 +302,7 @@ func testAccCheckAwsOrganizationsOrganizationDestroy(s *terraform.State) error {
return nil
}

func testAccCheckAwsOrganizationsOrganizationExists(n string, a *organizations.Organization) resource.TestCheckFunc {
func testAccCheckAwsOrganizationsOrganizationExists(n string, org *organizations.Organization) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
if !ok {
Expand All @@ -256,7 +326,7 @@ func testAccCheckAwsOrganizationsOrganizationExists(n string, a *organizations.O
return fmt.Errorf("Organization %q does not exist", rs.Primary.ID)
}

a = resp.Organization
*org = *resp.Organization

return nil
}
Expand Down Expand Up @@ -354,3 +424,21 @@ func testFlattenOrganizationsRootPolicyTypes(t *testing.T, index int, result []m
}
}
}

func testAccAwsOrganizationsOrganizationRecreated(before, after *organizations.Organization) resource.TestCheckFunc {
return func(s *terraform.State) error {
if aws.StringValue(before.Id) == aws.StringValue(after.Id) {
return fmt.Errorf("Organization (%s) not recreated", aws.StringValue(before.Id))
}
return nil
}
}

func testAccAwsOrganizationsOrganizationNotRecreated(before, after *organizations.Organization) resource.TestCheckFunc {
return func(s *terraform.State) error {
if aws.StringValue(before.Id) != aws.StringValue(after.Id) {
return fmt.Errorf("Organization (%s) recreated", aws.StringValue(before.Id))
}
return nil
}
}
4 changes: 3 additions & 1 deletion aws/resource_aws_organizations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ func TestAccAWSOrganizations_serial(t *testing.T) {
"basic": testAccAwsOrganizationsOrganization_basic,
"AwsServiceAccessPrincipals": testAccAwsOrganizationsOrganization_AwsServiceAccessPrincipals,
"EnabledPolicyTypes": testAccAwsOrganizationsOrganization_EnabledPolicyTypes,
"FeatureSet": testAccAwsOrganizationsOrganization_FeatureSet,
"FeatureSet_Basic": testAccAwsOrganizationsOrganization_FeatureSet,
"FeatureSet_Update": testAccAwsOrganizationsOrganization_FeatureSetUpdate,
"FeatureSet_ForcesNew": testAccAwsOrganizationsOrganization_FeatureSetForcesNew,
"DataSource": testAccDataSourceAwsOrganizationsOrganization_basic,
},
"Account": {
Expand Down
2 changes: 2 additions & 0 deletions website/docs/r/organizations_organization.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ description: |-

Provides a resource to create an organization.

!> **WARNING:** When migrating from a `feature_set` of `CONSOLIDATED_BILLING` to `ALL`, the Organization account owner will received an email stating the following: "You started the process to enable all features for your AWS organization. As part of that process, all member accounts that joined your organization by invitation must approve the change. You don’t need approval from member accounts that you directly created from within your AWS organization." After all member accounts have accepted the invitation, the Organization account owner must then finalize the changes via the [AWS Console](https://console.aws.amazon.com/organizations/home#/organization/settings/migration-progress). Until these steps are performed, Terraform will perpetually show a difference, and the `DescribeOrganization` API will continue to show the `FeatureSet` as `CONSOLIDATED_BILLING`. See the [AWS Organizations documentation](https://docs.aws.amazon.com/organizations/latest/userguide/orgs_manage_org_support-all-features.html) for more information.

## Example Usage

```hcl
Expand Down

0 comments on commit 405133c

Please sign in to comment.