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

issue #4713 New resource aws_neptune_subnet_group #4782

Merged
merged 7 commits into from
Jun 15, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
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
1 change: 1 addition & 0 deletions aws/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,7 @@ func Provider() terraform.ResourceProvider {
"aws_batch_compute_environment": resourceAwsBatchComputeEnvironment(),
"aws_batch_job_definition": resourceAwsBatchJobDefinition(),
"aws_batch_job_queue": resourceAwsBatchJobQueue(),
"aws_neptune_subnet_group": resourceAwsNeptuneSubnetGroup(),

// ALBs are actually LBs because they can be type `network` or `application`
// To avoid regressions, we will add a new resource for each and they both point
Expand Down
264 changes: 264 additions & 0 deletions aws/resource_aws_neptune_subnet_group.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,264 @@
package aws

import (
"fmt"
"log"
"strings"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/neptune"

"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
)

func resourceAwsNeptuneSubnetGroup() *schema.Resource {
return &schema.Resource{
Create: resourceAwsNeptuneSubnetGroupCreate,
Read: resourceAwsNeptuneSubnetGroupRead,
Update: resourceAwsNeptuneSubnetGroupUpdate,
Delete: resourceAwsNeptuneSubnetGroupDelete,
Importer: &schema.ResourceImporter{
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I missed this before, looks like we're missing import test cases, e.g.

{
  ResourceName:      "aws_neptune_subnet_group.test",
  ImportState:       true,
  ImportStateVerify: true,
},

State: schema.ImportStatePassthrough,
},

Schema: map[string]*schema.Schema{
"arn": {
Type: schema.TypeString,
Computed: true,
},

"name": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ConflictsWith: []string{"name_prefix"},
ValidateFunc: validateNeptuneSubnetGroupName,
},
"name_prefix": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ValidateFunc: validateNeptuneSubnetGroupNamePrefix,
},

"description": {
Type: schema.TypeString,
Optional: true,
Default: "Managed by Terraform",
},

"subnet_ids": {
Type: schema.TypeSet,
Required: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add MinItems: 1 here, Required: true accepts subnet_ids = [] 👍

Elem: &schema.Schema{Type: schema.TypeString},
Set: schema.HashString,
},

"tags": tagsSchema(),
},
}
}

func resourceAwsNeptuneSubnetGroupCreate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).neptuneconn
tags := tagsFromMapNeptune(d.Get("tags").(map[string]interface{}))

subnetIdsSet := d.Get("subnet_ids").(*schema.Set)
subnetIds := make([]*string, subnetIdsSet.Len())
for i, subnetId := range subnetIdsSet.List() {
subnetIds[i] = aws.String(subnetId.(string))
}

var groupName string
if v, ok := d.GetOk("name"); ok {
groupName = v.(string)
} else if v, ok := d.GetOk("name_prefix"); ok {
groupName = resource.PrefixedUniqueId(v.(string))
} else {
groupName = resource.UniqueId()
}

createOpts := neptune.CreateDBSubnetGroupInput{
DBSubnetGroupName: aws.String(groupName),
DBSubnetGroupDescription: aws.String(d.Get("description").(string)),
SubnetIds: subnetIds,
Tags: tags,
}

log.Printf("[DEBUG] Create Neptune Subnet Group: %#v", createOpts)
_, err := conn.CreateDBSubnetGroup(&createOpts)
if err != nil {
return fmt.Errorf("Error creating Neptune Subnet Group: %s", err)
}

d.SetId(aws.StringValue(createOpts.DBSubnetGroupName))
log.Printf("[INFO] Neptune Subnet Group ID: %s", d.Id())
return resourceAwsNeptuneSubnetGroupRead(d, meta)
}

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

describeOpts := neptune.DescribeDBSubnetGroupsInput{
DBSubnetGroupName: aws.String(d.Id()),
}

var subnetGroups []*neptune.DBSubnetGroup
//describeResp, err := conn.DescribeDBSubnetGroups(&describeOpts)
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: comment can be removed 😄

if err := conn.DescribeDBSubnetGroupsPages(&describeOpts, func(resp *neptune.DescribeDBSubnetGroupsOutput, lastPage bool) bool {
for _, v := range resp.DBSubnetGroups {
subnetGroups = append(subnetGroups, v)
}
return !lastPage
}); err != nil {
if neptuneerr, ok := err.(awserr.Error); ok && neptuneerr.Code() == "DBSubnetGroupNotFoundFault" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: We can use the helper function and SDK provided constant here -- we should also provide a log message when removing a resource from the state, e.g.

if isAWSErr(err, neptune.ErrCodeDBSubnetGroupNotFoundFault, "") {
  log.Printf("[WARN] Neptune Subnet Group (%s) not found, removing from state", d.Id())
  d.SetId("")
  return nil
}

// Update state to indicate the neptune subnet no longer exists.
d.SetId("")
return nil
}
return err
}

if len(subnetGroups) == 0 {
return fmt.Errorf("Unable to find Neptune Subnet Group: %#v", subnetGroups)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should provide the log message and remove from state, similar to the above error handling, rather than returning an error here.

}

var subnetGroup *neptune.DBSubnetGroup
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that we force lowercase naming via the validation functions, it should be safe to assume here that we can directly access the first element of the slice instead of this extra logic.

subnetGroup := subnetGroups[0]

for _, s := range subnetGroups {
// AWS is down casing the name provided, so we compare lower case versions
// of the names. We lower case both our name and their name in the check,
// incase they change that someday.
if strings.ToLower(d.Id()) == strings.ToLower(aws.StringValue(s.DBSubnetGroupName)) {
subnetGroup = subnetGroups[0]
}
}

if subnetGroup.DBSubnetGroupName == nil {
return fmt.Errorf("Unable to find Neptune Subnet Group: %#v", subnetGroups)
}

d.Set("name", subnetGroup.DBSubnetGroupName)
d.Set("description", subnetGroup.DBSubnetGroupDescription)

subnets := make([]string, 0, len(subnetGroup.Subnets))
for _, s := range subnetGroup.Subnets {
subnets = append(subnets, aws.StringValue(s.SubnetIdentifier))
}
d.Set("subnet_ids", subnets)
Copy link
Contributor

Choose a reason for hiding this comment

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

For TypeSet attributes, we should perform error checking, e.g.

if err := d.Set("subnet_ids", ...); err != nil {
  return fmt.Errorf("error setting subnet_ids: %s", err)
}

In this case I believe you'll find that setting this attribute needs to be occur with schema.NewSet(), e.g. something like

if err := d.Set("subnet_ids", schema.NewSet(schema.HashString, subnets)); err != nil {
  return fmt.Errorf("error setting subnet_ids: %s", err)
}


// list tags for resource
// set tags

//Amazon Neptune shares the format of Amazon RDS ARNs. Neptune ARNs contain rds and not neptune.
//https://docs.aws.amazon.com/neptune/latest/userguide/tagging.ARN.html
arn := arn.ARN{
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not need to manually construct the ARN, its provided in the Neptune SubnetGroup type as DBSubnetGroupArn

Partition: meta.(*AWSClient).partition,
Service: "rds",
Region: meta.(*AWSClient).region,
AccountID: meta.(*AWSClient).accountid,
Resource: fmt.Sprintf("subgrp:%s", d.Id()),
}.String()
d.Set("arn", arn)
resp, err := conn.ListTagsForResource(&neptune.ListTagsForResourceInput{
ResourceName: aws.String(arn),
})

if err != nil {
log.Printf("[DEBUG] Error retreiving tags for ARN: %s", arn)
}

var dt []*neptune.Tag
if len(resp.TagList) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

This conditional seems extraneous -- either tagsToMapNeptune() should handle an empty list appropriately (seems like it would) or we should adjust that function accordingly so we can directly just pass in resp.TagList

dt = resp.TagList
}
d.Set("tags", tagsToMapNeptune(dt))

return nil
}

func resourceAwsNeptuneSubnetGroupUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).neptuneconn
if d.HasChange("subnet_ids") || d.HasChange("description") {
_, n := d.GetChange("subnet_ids")
if n == nil {
n = new(schema.Set)
}
ns := n.(*schema.Set)

var sIds []*string
for _, s := range ns.List() {
sIds = append(sIds, aws.String(s.(string)))
}

_, err := conn.ModifyDBSubnetGroup(&neptune.ModifyDBSubnetGroupInput{
DBSubnetGroupName: aws.String(d.Id()),
DBSubnetGroupDescription: aws.String(d.Get("description").(string)),
SubnetIds: sIds,
})

if err != nil {
return err
}
}

//Amazon Neptune shares the format of Amazon RDS ARNs. Neptune ARNs contain rds and not neptune.
Copy link
Contributor

Choose a reason for hiding this comment

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

The ARN should be available in the state, rather than manually constructing it again here. e.g.

arn := d.Get("arn").(string)

//https://docs.aws.amazon.com/neptune/latest/userguide/tagging.ARN.html
arn := arn.ARN{
Partition: meta.(*AWSClient).partition,
Service: "rds",
Region: meta.(*AWSClient).region,
AccountID: meta.(*AWSClient).accountid,
Resource: fmt.Sprintf("subgrp:%s", d.Id()),
}.String()
if err := setTagsNeptune(conn, d, arn); err != nil {
return err
} else {
d.SetPartial("tags")
}

return resourceAwsNeptuneSubnetGroupRead(d, meta)
}

func resourceAwsNeptuneSubnetGroupDelete(d *schema.ResourceData, meta interface{}) error {
stateConf := &resource.StateChangeConf{
Copy link
Contributor

Choose a reason for hiding this comment

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

This logic seems extraneous -- nothing appears to be retrying or setting a pending state. Seems like we can just call this normally:

conn := meta.(*AWSClient).neptuneconn

input := neptune.DeleteDBSubnetGroupInput{
  DBSubnetGroupName: aws.String(d.Id()),
}

log.Printf("[DEBUG] Deleting Neptune Subnet Group: %s", d.Id())
_, err := conn.DeleteDBSubnetGroup(&input)
if err != nil {
  if isAWSErr(err, neptune.ErrCodeDBSubnetGroupNotFoundFault, "") {
    return nil
  }
  return fmt.Errorf("error deleting Neptune Subnet Group (%s): %s", d.Id(), err)
}

return nil

Pending: []string{"pending"},
Target: []string{"destroyed"},
Refresh: resourceAwsNeptuneSubnetGroupDeleteRefreshFunc(d, meta),
Timeout: 3 * time.Minute,
MinTimeout: 1 * time.Second,
}
_, err := stateConf.WaitForState()
return err
}

func resourceAwsNeptuneSubnetGroupDeleteRefreshFunc(
d *schema.ResourceData,
meta interface{}) resource.StateRefreshFunc {
conn := meta.(*AWSClient).neptuneconn

return func() (interface{}, string, error) {

deleteOpts := neptune.DeleteDBSubnetGroupInput{
DBSubnetGroupName: aws.String(d.Id()),
}

if _, err := conn.DeleteDBSubnetGroup(&deleteOpts); err != nil {
neptuneerr, ok := err.(awserr.Error)
if !ok {
return d, "error", err
}

if neptuneerr.Code() != "DBSubnetGroupNotFoundFault" {
return d, "error", err
}
}

return d, "destroyed", nil
}
}
Loading