-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
provider/aws: Support tags for AWS redshift cluster #5356
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,6 +104,41 @@ func TestAccAWSRedshiftCluster_updateNodeCount(t *testing.T) { | |
}) | ||
} | ||
|
||
func TestAccAWSRedshiftCluster_tags(t *testing.T) { | ||
var v redshift.Cluster | ||
|
||
ri := rand.New(rand.NewSource(time.Now().UnixNano())).Int() | ||
preConfig := fmt.Sprintf(testAccAWSRedshiftClusterConfig_tags, ri) | ||
postConfig := fmt.Sprintf(testAccAWSRedshiftClusterConfig_updatedTags, ri) | ||
|
||
resource.Test(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, | ||
Providers: testAccProviders, | ||
CheckDestroy: testAccCheckAWSRedshiftClusterDestroy, | ||
Steps: []resource.TestStep{ | ||
resource.TestStep{ | ||
Config: preConfig, | ||
Check: resource.ComposeTestCheckFunc( | ||
testAccCheckAWSRedshiftClusterExists("aws_redshift_cluster.default", &v), | ||
resource.TestCheckResourceAttr( | ||
"aws_redshift_cluster.default", "tags.#", "3"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe a nitpick, but it would be good to also test the actual tags - e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 seems like an easy improvement |
||
resource.TestCheckResourceAttr("aws_redshift_cluster.default", "tags.environment", "Production"), | ||
), | ||
}, | ||
|
||
resource.TestStep{ | ||
Config: postConfig, | ||
Check: resource.ComposeTestCheckFunc( | ||
testAccCheckAWSRedshiftClusterExists("aws_redshift_cluster.default", &v), | ||
resource.TestCheckResourceAttr( | ||
"aws_redshift_cluster.default", "tags.#", "1"), | ||
resource.TestCheckResourceAttr("aws_redshift_cluster.default", "tags.environment", "Production"), | ||
), | ||
}, | ||
}, | ||
}) | ||
} | ||
|
||
func testAccCheckAWSRedshiftClusterDestroy(s *terraform.State) error { | ||
for _, rs := range s.RootModule().Resources { | ||
if rs.Type != "aws_redshift_cluster" { | ||
|
@@ -306,10 +341,6 @@ func TestResourceAWSRedshiftClusterMasterUsernameValidation(t *testing.T) { | |
} | ||
|
||
var testAccAWSRedshiftClusterConfig_updateNodeCount = ` | ||
provider "aws" { | ||
region = "us-west-2" | ||
} | ||
|
||
resource "aws_redshift_cluster" "default" { | ||
cluster_identifier = "tf-redshift-cluster-%d" | ||
availability_zone = "us-west-2a" | ||
|
@@ -324,10 +355,6 @@ resource "aws_redshift_cluster" "default" { | |
` | ||
|
||
var testAccAWSRedshiftClusterConfig_basic = ` | ||
provider "aws" { | ||
region = "us-west-2" | ||
} | ||
|
||
resource "aws_redshift_cluster" "default" { | ||
cluster_identifier = "tf-redshift-cluster-%d" | ||
availability_zone = "us-west-2a" | ||
|
@@ -339,11 +366,41 @@ resource "aws_redshift_cluster" "default" { | |
allow_version_upgrade = false | ||
}` | ||
|
||
var testAccAWSRedshiftClusterConfig_notPubliclyAccessible = ` | ||
provider "aws" { | ||
region = "us-west-2" | ||
} | ||
var testAccAWSRedshiftClusterConfig_tags = ` | ||
resource "aws_redshift_cluster" "default" { | ||
cluster_identifier = "tf-redshift-cluster-%d" | ||
availability_zone = "us-west-2a" | ||
database_name = "mydb" | ||
master_username = "foo" | ||
master_password = "Mustbe8characters" | ||
node_type = "dc1.large" | ||
automated_snapshot_retention_period = 7 | ||
allow_version_upgrade = false | ||
|
||
tags { | ||
environment = "Production" | ||
cluster = "reader" | ||
Type = "master" | ||
} | ||
}` | ||
|
||
var testAccAWSRedshiftClusterConfig_updatedTags = ` | ||
resource "aws_redshift_cluster" "default" { | ||
cluster_identifier = "tf-redshift-cluster-%d" | ||
availability_zone = "us-west-2a" | ||
database_name = "mydb" | ||
master_username = "foo" | ||
master_password = "Mustbe8characters" | ||
node_type = "dc1.large" | ||
automated_snapshot_retention_period = 7 | ||
allow_version_upgrade = false | ||
|
||
tags { | ||
environment = "Production" | ||
} | ||
}` | ||
|
||
var testAccAWSRedshiftClusterConfig_notPubliclyAccessible = ` | ||
resource "aws_vpc" "foo" { | ||
cidr_block = "10.1.0.0/16" | ||
} | ||
|
@@ -402,10 +459,6 @@ resource "aws_redshift_cluster" "default" { | |
}` | ||
|
||
var testAccAWSRedshiftClusterConfig_updatePubliclyAccessible = ` | ||
provider "aws" { | ||
region = "us-west-2" | ||
} | ||
|
||
resource "aws_vpc" "foo" { | ||
cidr_block = "10.1.0.0/16" | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,71 @@ | ||
package aws | ||
|
||
import ( | ||
"log" | ||
|
||
"github.com/aws/aws-sdk-go/aws" | ||
"github.com/aws/aws-sdk-go/service/redshift" | ||
"github.com/hashicorp/terraform/helper/schema" | ||
) | ||
|
||
func setTagsRedshift(conn *redshift.Redshift, d *schema.ResourceData, arn string) error { | ||
if d.HasChange("tags") { | ||
oraw, nraw := d.GetChange("tags") | ||
o := oraw.(map[string]interface{}) | ||
n := nraw.(map[string]interface{}) | ||
create, remove := diffTagsRedshift(tagsFromMapRedshift(o), tagsFromMapRedshift(n)) | ||
|
||
// Set tags | ||
if len(remove) > 0 { | ||
log.Printf("[DEBUG] Removing tags: %#v", remove) | ||
k := make([]*string, len(remove), len(remove)) | ||
for i, t := range remove { | ||
k[i] = t.Key | ||
} | ||
|
||
_, err := conn.DeleteTags(&redshift.DeleteTagsInput{ | ||
ResourceName: aws.String(arn), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this API call fail if you passed name instead of ARN? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure actually TBH - looking now There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it will fail. We need to pass the ARN here |
||
TagKeys: k, | ||
}) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
if len(create) > 0 { | ||
log.Printf("[DEBUG] Creating tags: %#v", create) | ||
_, err := conn.CreateTags(&redshift.CreateTagsInput{ | ||
ResourceName: aws.String(arn), | ||
Tags: create, | ||
}) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func diffTagsRedshift(oldTags, newTags []*redshift.Tag) ([]*redshift.Tag, []*redshift.Tag) { | ||
// First, we're creating everything we have | ||
create := make(map[string]interface{}) | ||
for _, t := range newTags { | ||
create[*t.Key] = *t.Value | ||
} | ||
|
||
// Build the list of what to remove | ||
var remove []*redshift.Tag | ||
for _, t := range oldTags { | ||
old, ok := create[*t.Key] | ||
if !ok || old != *t.Value { | ||
// Delete it! | ||
remove = append(remove, t) | ||
} | ||
} | ||
|
||
return tagsFromMapRedshift(create), remove | ||
} | ||
|
||
func tagsFromMapRedshift(m map[string]interface{}) []*redshift.Tag { | ||
result := make([]*redshift.Tag, 0, len(m)) | ||
for k, v := range m { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate the reduction of the interface to 3 strings! 👍