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

New Resource: aws_organizations_organization #903

Merged
merged 18 commits into from
Feb 25, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions aws/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import (
"github.com/aws/aws-sdk-go/service/lambda"
"github.com/aws/aws-sdk-go/service/lightsail"
"github.com/aws/aws-sdk-go/service/opsworks"
"github.com/aws/aws-sdk-go/service/organizations"
"github.com/aws/aws-sdk-go/service/rds"
"github.com/aws/aws-sdk-go/service/redshift"
"github.com/aws/aws-sdk-go/service/route53"
Expand Down Expand Up @@ -165,6 +166,7 @@ type AWSClient struct {
lambdaconn *lambda.Lambda
lightsailconn *lightsail.Lightsail
opsworksconn *opsworks.OpsWorks
orgsconn *organizations.Organizations
glacierconn *glacier.Glacier
codebuildconn *codebuild.CodeBuild
codedeployconn *codedeploy.CodeDeploy
Expand Down Expand Up @@ -369,6 +371,7 @@ func (c *Config) Client() (interface{}, error) {
client.lambdaconn = lambda.New(sess)
client.lightsailconn = lightsail.New(sess)
client.opsworksconn = opsworks.New(sess)
client.orgsconn = organizations.New(sess)
client.r53conn = route53.New(r53Sess)
client.rdsconn = rds.New(awsRdsSess)
client.redshiftconn = redshift.New(sess)
Expand Down
1 change: 1 addition & 0 deletions aws/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ func Provider() terraform.ResourceProvider {
"aws_opsworks_user_profile": resourceAwsOpsworksUserProfile(),
"aws_opsworks_permission": resourceAwsOpsworksPermission(),
"aws_opsworks_rds_db_instance": resourceAwsOpsworksRdsDbInstance(),
"aws_organization": resourceAwsOrganization(),
Copy link
Contributor

@bflad bflad Feb 20, 2018

Choose a reason for hiding this comment

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

Its going to look ugly at first and Naming is Hard, but we should probably also rename this aws_organizations_organization for a few reasons:

  • The AWS Organizations service supports multiple resources we'll want to manage, e.g. aws_organizations_account, aws_organizations_organization_unit, aws_organizations_policy, this will keep everything bundled together nicely naming-wise
  • To prevent confusion with other AWS services that might use the terminology "organization"
  • If AWS ever creates a separate account organization management service/method

Ideally the resource function names will be updated to long form as well: resourceAwsOrganizationsOrganization, resourceAwsOrganizationsOrganizationRead, etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I handled all the other feedback you provided. I'm not against doing this but it seemed like more work than I was willing to put in tonight.

"aws_placement_group": resourceAwsPlacementGroup(),
"aws_proxy_protocol_policy": resourceAwsProxyProtocolPolicy(),
"aws_rds_cluster": resourceAwsRDSCluster(),
Expand Down
104 changes: 104 additions & 0 deletions aws/resource_aws_organization.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package aws

import (
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: can you please structure the imports as:

import (
  // stdlib ones

  // others
)

FYI many folks here are using goimports to automatically add, remove, and format imports 😄

"fmt"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/organizations"
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/helper/validation"
"log"
)

func resourceAwsOrganization() *schema.Resource {
return &schema.Resource{
Create: resourceAwsOrganizationCreate,
Read: resourceAwsOrganizationRead,
Update: resourceAwsOrganizationUpdate,
Copy link
Contributor

Choose a reason for hiding this comment

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

As written, this resource does not support updates. We should remove this line from the resource and its associated function.

Delete: resourceAwsOrganizationDelete,
Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
},

Schema: map[string]*schema.Schema{
"arn": &schema.Schema{
Type: schema.TypeString,
Computed: true,
},
"master_account_arn": &schema.Schema{
Type: schema.TypeString,
Computed: true,
},
"master_account_email": &schema.Schema{
Type: schema.TypeString,
Computed: true,
},
"master_account_id": &schema.Schema{
Type: schema.TypeString,
Computed: true,
},
"feature_set": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please set ForceNew: true on this attribute as updates are not implemented

Type: schema.TypeString,
Optional: true,
Default: "ALL",
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nitpick: we should use the SDK constant here too: organizations.OrganizationFeatureSetAll

ValidateFunc: validation.StringInSlice([]string{"ALL", "CONSOLIDATED_BILLING"}, true),
},
},
}
}

func resourceAwsOrganizationCreate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).orgsconn

// Create the organization
createOpts := &organizations.CreateOrganizationInput{
FeatureSet: aws.String(d.Get("feature_set").(string)),
}
log.Printf("[DEBUG] Organization create config: %#v", createOpts)

resp, err := conn.CreateOrganization(createOpts)
if err != nil {
return fmt.Errorf("Error creating organization: %s", err)
}

// Get the ID and store it
org := resp.Organization
d.SetId(*org.Id)
log.Printf("[INFO] Organization ID: %s", d.Id())
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please remove this line or set it to DEBUG? 👍


return resourceAwsOrganizationUpdate(d, meta)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should go right to calling resourceAwsOrganizationRead(d, meta)

}

func resourceAwsOrganizationRead(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).orgsconn
org, err := conn.DescribeOrganization(&organizations.DescribeOrganizationInput{})
if err != nil {
if orgerr, ok := err.(awserr.Error); ok && orgerr.Code() == "AWSOrganizationsNotInUseException" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nitpick: We can simplify this with if isAWSErr(err, organizations.ErrCodeAWSOrganizationsNotInUseException, "") {

d.SetId("")
return nil
}
return err
}

d.Set("arn", org.Organization.Arn)
d.Set("feature_set", org.Organization.FeatureSet)
d.Set("master_account_arn", org.Organization.MasterAccountArn)
d.Set("master_account_email", org.Organization.MasterAccountEmail)
d.Set("master_account_id", org.Organization.MasterAccountId)
return nil
}

func resourceAwsOrganizationUpdate(d *schema.ResourceData, meta interface{}) error {
return resourceAwsOrganizationRead(d, meta)
}

func resourceAwsOrganizationDelete(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).orgsconn
_, err := conn.DeleteOrganization(&organizations.DeleteOrganizationInput{})
if err != nil {
return err
}

return nil

}
89 changes: 89 additions & 0 deletions aws/resource_aws_organization_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package aws

import (
"fmt"
"testing"

"github.com/aws/aws-sdk-go/service/organizations"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
)

func TestAccAWSOrganization_basic(t *testing.T) {
var organization organizations.Organization

feature_set := "CONSOLIDATED_BILLING"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSOrganizationDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSOrganizationConfig(feature_set),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSOrganizationExists("aws_organization.test", &organization),
),
},
},
})
}

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

for _, rs := range s.RootModule().Resources {
if rs.Type != "aws_organization" {
continue
}

params := &organizations.DescribeOrganizationInput{}

resp, err := conn.DescribeOrganization(params)

if err != nil || resp == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be checking and returning errors:

if err != nil {
  if isAWSErr(err, organizations.ErrCodeAWSOrganizationsNotInUseException, "") {
    return nil
  }
  return err
}

if resp != nil && resp.Organization != nil {
  return fmt.Errorf("Organization still exists: %q", rs.Primary.ID)
}

return nil
}

if resp.Organization != nil {
return fmt.Errorf("Bad: Organization still exists: %q", rs.Primary.ID)
}
}

return nil

Copy link
Contributor

Choose a reason for hiding this comment

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

rm extra NL

}

func testAccCheckAWSOrganizationExists(n string, a *organizations.Organization) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
if !ok {
return fmt.Errorf("Not found: %s", n)
}

conn := testAccProvider.Meta().(*AWSClient).orgsconn
params := &organizations.DescribeOrganizationInput{}

resp, err := conn.DescribeOrganization(params)

if err != nil || resp == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be returning errors:

if err != nil {
  return err
}

if resp == nil || resp.Organization == nil {
  return fmt.Errorf("Organization %q does not exist", rs.Primary.ID)
}

return nil
}

if resp.Organization == nil {
return fmt.Errorf("Bad: Organization %q does not exist", rs.Primary.ID)
}

a = resp.Organization

return nil
}
}

func testAccAWSOrganizationConfig(feature_set string) string {
return fmt.Sprintf(`
resource "aws_organization" "test" {
feature_set = "%s"
}
`, feature_set)
}
Loading