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

r/aws_s3_bucket: support SSE-KMS replication configuration #2625

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
109 changes: 105 additions & 4 deletions aws/resource_aws_s3_bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,36 @@ func resourceAwsS3Bucket() *schema.Resource {
Optional: true,
ValidateFunc: validateS3BucketReplicationDestinationStorageClass,
},
"replica_kms_key_id": {
Type: schema.TypeString,
Optional: true,
},
},
},
},
"source_selection_criteria": {
Type: schema.TypeSet,
Optional: true,
MinItems: 1,
MaxItems: 1,
Set: sourceSelectionCriteriaHash,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"sse_kms_encrypted_objects": {
Type: schema.TypeSet,
Optional: true,
MinItems: 1,
MaxItems: 1,
Set: sourceSseKmsObjectsHash,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"enabled": {
Type: schema.TypeBool,
Required: true,
},
},
},
},
},
},
},
Expand Down Expand Up @@ -1680,17 +1710,37 @@ func resourceAwsS3BucketReplicationConfigurationUpdate(s3conn *s3.S3, d *schema.
}

ruleDestination := &s3.Destination{}
if destination, ok := rr["destination"]; ok {
dest := destination.(*schema.Set).List()

bd := dest[0].(map[string]interface{})
if dest, ok := rr["destination"].(*schema.Set); ok && dest.Len() > 0 {
bd := dest.List()[0].(map[string]interface{})
ruleDestination.Bucket = aws.String(bd["bucket"].(string))

if storageClass, ok := bd["storage_class"]; ok && storageClass != "" {
ruleDestination.StorageClass = aws.String(storageClass.(string))
}

if replicaKmsKeyId, ok := bd["replica_kms_key_id"]; ok && replicaKmsKeyId != "" {
ruleDestination.EncryptionConfiguration = &s3.EncryptionConfiguration{
ReplicaKmsKeyID: aws.String(replicaKmsKeyId.(string)),
}
}
}
rcRule.Destination = ruleDestination

if ssc, ok := rr["source_selection_criteria"].(*schema.Set); ok && ssc.Len() > 0 {
sscValues := ssc.List()[0].(map[string]interface{})
ruleSsc := &s3.SourceSelectionCriteria{}
if sseKms, ok := sscValues["sse_kms_encrypted_objects"].(*schema.Set); ok && sseKms.Len() > 0 {
sseKmsValues := sseKms.List()[0].(map[string]interface{})
sseKmsEncryptedObjects := &s3.SseKmsEncryptedObjects{}
if sseKmsValues["enabled"].(bool) {
sseKmsEncryptedObjects.Status = aws.String(s3.SseKmsEncryptedObjectsStatusEnabled)
} else {
sseKmsEncryptedObjects.Status = aws.String(s3.SseKmsEncryptedObjectsStatusDisabled)
}
ruleSsc.SseKmsEncryptedObjects = sseKmsEncryptedObjects
}
rcRule.SourceSelectionCriteria = ruleSsc
}
rules = append(rules, rcRule)
}

Expand Down Expand Up @@ -1913,6 +1963,11 @@ func flattenAwsS3BucketReplicationConfiguration(r *s3.ReplicationConfiguration)
if v.Destination.StorageClass != nil {
rd["storage_class"] = *v.Destination.StorageClass
}
if v.Destination.EncryptionConfiguration != nil {
if v.Destination.EncryptionConfiguration.ReplicaKmsKeyID != nil {
rd["replica_kms_key_id"] = *v.Destination.EncryptionConfiguration.ReplicaKmsKeyID
}
}
t["destination"] = schema.NewSet(destinationHash, []interface{}{rd})
}

Expand All @@ -1925,6 +1980,19 @@ func flattenAwsS3BucketReplicationConfiguration(r *s3.ReplicationConfiguration)
if v.Status != nil {
t["status"] = *v.Status
}
if vssc := v.SourceSelectionCriteria; vssc != nil {
tssc := make(map[string]interface{})
if vssc.SseKmsEncryptedObjects != nil {
tSseKms := make(map[string]interface{})
if *vssc.SseKmsEncryptedObjects.Status == s3.SseKmsEncryptedObjectsStatusEnabled {
tSseKms["enabled"] = true
} else if *vssc.SseKmsEncryptedObjects.Status == s3.SseKmsEncryptedObjectsStatusDisabled {
tSseKms["enabled"] = false
}
tssc["sse_kms_encrypted_objects"] = schema.NewSet(sourceSseKmsObjectsHash, []interface{}{tSseKms})
}
t["source_selection_criteria"] = schema.NewSet(sourceSelectionCriteriaHash, []interface{}{tssc})
}
rules = append(rules, t)
}
m["rules"] = schema.NewSet(rulesHash, rules)
Expand Down Expand Up @@ -2086,6 +2154,12 @@ func rulesHash(v interface{}) int {
if v, ok := m["status"]; ok {
buf.WriteString(fmt.Sprintf("%s-", v.(string)))
}
if v, ok := m["destination"].(*schema.Set); ok && v.Len() > 0 {
buf.WriteString(fmt.Sprintf("%d-", destinationHash(v.List()[0])))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Given this is adding an existing field to the hashcode function - this will appear as a diff to existing resources, I believe that should be fine, but @radeksimko / @bflad can confirm for sure

Copy link
Member

Choose a reason for hiding this comment

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

In that case I think we should add state migration, so the diff doesn't appear for existing users who just upgraded and not use any new functionality.

Copy link
Contributor Author

@modax modax Feb 20, 2018

Choose a reason for hiding this comment

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

hi, @radeksimko,

I have just confirmed that neither of these:

  1. terraform apply
  2. terraform apply -refresh=false
  3. terraform refresh

report any changes after provider upgrade. Rule hash value changes in the state file after all of them though. Frankly, I have also expected 2) to report changes but for some reason it does not happen (must be related to how set hash is compared probably, i.e. always upfront or something).

Here is the tf code if you want to test yourself: https://gist.github.com/modax/9e13db22565ab2f1a26821e5ee5995e5

Copy link
Contributor

Choose a reason for hiding this comment

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

confirmed that scenario ^ - LGTM 👍

if v, ok := m["source_selection_criteria"].(*schema.Set); ok && v.Len() > 0 && v.List()[0] != nil {
buf.WriteString(fmt.Sprintf("%d-", sourceSelectionCriteriaHash(v.List()[0])))
}
return hashcode.String(buf.String())
}

Expand All @@ -2099,6 +2173,33 @@ func destinationHash(v interface{}) int {
if v, ok := m["storage_class"]; ok {
buf.WriteString(fmt.Sprintf("%s-", v.(string)))
}
if v, ok := m["replica_kms_key_id"]; ok {
buf.WriteString(fmt.Sprintf("%s-", v.(string)))
}
return hashcode.String(buf.String())
}

func sourceSelectionCriteriaHash(v interface{}) int {
// v is nil if empty source_selection_criteria is given.
if v == nil {
return 0
}
var buf bytes.Buffer
m := v.(map[string]interface{})

if v, ok := m["sse_kms_encrypted_objects"].(*schema.Set); ok && v.Len() > 0 {
buf.WriteString(fmt.Sprintf("%d-", sourceSseKmsObjectsHash(v.List()[0])))
}
return hashcode.String(buf.String())
}

func sourceSseKmsObjectsHash(v interface{}) int {
var buf bytes.Buffer
m := v.(map[string]interface{})

if v, ok := m["enabled"]; ok {
buf.WriteString(fmt.Sprintf("%t-", v.(bool)))
}
return hashcode.String(buf.String())
}

Expand Down
139 changes: 136 additions & 3 deletions aws/resource_aws_s3_bucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -767,11 +767,56 @@ func TestAccAWSS3Bucket_Replication(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSS3BucketExistsWithProvider("aws_s3_bucket.bucket", testAccAwsRegionProviderFunc("us-west-2", &providers)),
resource.TestCheckResourceAttr("aws_s3_bucket.bucket", "replication_configuration.#", "1"),
resource.TestMatchResourceAttr("aws_s3_bucket.bucket", "replication_configuration.0.role", regexp.MustCompile(fmt.Sprintf("^arn:aws:iam::[\\d+]+:role/tf-iam-role-replication-%d", rInt))),
resource.TestCheckResourceAttr("aws_s3_bucket.bucket", "replication_configuration.0.rules.#", "1"),
resource.TestCheckResourceAttr("aws_s3_bucket.bucket", "replication_configuration.0.rules.2229345141.id", "foobar"),
resource.TestCheckResourceAttr("aws_s3_bucket.bucket", "replication_configuration.0.rules.2229345141.prefix", "foo"),
resource.TestCheckResourceAttr("aws_s3_bucket.bucket", "replication_configuration.0.rules.2229345141.status", s3.ReplicationRuleStatusEnabled),
testAccCheckAWSS3BucketExistsWithProvider("aws_s3_bucket.destination", testAccAwsRegionProviderFunc("eu-west-1", &providers)),
testAccCheckAWSS3BucketReplicationRules(
"aws_s3_bucket.bucket",
testAccAwsRegionProviderFunc("us-west-2", &providers),
[]*s3.ReplicationRule{
{
ID: aws.String("foobar"),
Destination: &s3.Destination{
Bucket: aws.String(fmt.Sprintf("arn:aws:s3:::tf-test-bucket-destination-%d", rInt)),
StorageClass: aws.String(s3.ObjectStorageClassStandard),
},
Prefix: aws.String("foo"),
Status: aws.String(s3.ReplicationRuleStatusEnabled),
},
},
),
),
},
{
Config: testAccAWSS3BucketConfigReplicationWithSseKmsEncryptedObjects(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSS3BucketExistsWithProvider("aws_s3_bucket.bucket", testAccAwsRegionProviderFunc("us-west-2", &providers)),
resource.TestCheckResourceAttr("aws_s3_bucket.bucket", "replication_configuration.#", "1"),
resource.TestMatchResourceAttr("aws_s3_bucket.bucket", "replication_configuration.0.role", regexp.MustCompile(fmt.Sprintf("^arn:aws:iam::[\\d+]+:role/tf-iam-role-replication-%d", rInt))),
resource.TestCheckResourceAttr("aws_s3_bucket.bucket", "replication_configuration.0.rules.#", "1"),
testAccCheckAWSS3BucketReplicationRules(
"aws_s3_bucket.bucket",
testAccAwsRegionProviderFunc("us-west-2", &providers),
[]*s3.ReplicationRule{
{
ID: aws.String("foobar"),
Destination: &s3.Destination{
Bucket: aws.String(fmt.Sprintf("arn:aws:s3:::tf-test-bucket-destination-%d", rInt)),
StorageClass: aws.String(s3.ObjectStorageClassStandard),
EncryptionConfiguration: &s3.EncryptionConfiguration{
ReplicaKmsKeyID: aws.String("${aws_kms_key.replica.arn}"),
},
},
Prefix: aws.String("foo"),
Status: aws.String(s3.ReplicationRuleStatusEnabled),
SourceSelectionCriteria: &s3.SourceSelectionCriteria{
SseKmsEncryptedObjects: &s3.SseKmsEncryptedObjects{
Status: aws.String(s3.SseKmsEncryptedObjectsStatusEnabled),
},
},
},
},
),
),
},
},
Expand Down Expand Up @@ -1198,6 +1243,43 @@ func testAccCheckAWSS3BucketLogging(n, b, p string) resource.TestCheckFunc {
}
}

func testAccCheckAWSS3BucketReplicationRules(n string, providerF func() *schema.Provider, rules []*s3.ReplicationRule) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, _ := s.RootModule().Resources[n]
for _, rule := range rules {
if dest := rule.Destination; dest != nil {
if ec := dest.EncryptionConfiguration; ec != nil {
if ec.ReplicaKmsKeyID != nil {
key_arn := s.RootModule().Resources["aws_kms_key.replica"].Primary.Attributes["arn"]
ec.ReplicaKmsKeyID = aws.String(strings.Replace(*ec.ReplicaKmsKeyID, "${aws_kms_key.replica.arn}", key_arn, -1))
}
}
}
}

provider := providerF()

conn := provider.Meta().(*AWSClient).s3conn
out, err := conn.GetBucketReplication(&s3.GetBucketReplicationInput{
Bucket: aws.String(rs.Primary.ID),
})
if err != nil {
if isAWSErr(err, s3.ErrCodeNoSuchBucket, "") {
return fmt.Errorf("S3 bucket not found")
}
if rules == nil {
return nil
}
return fmt.Errorf("GetReplicationConfiguration error: %v", err)
}
if !reflect.DeepEqual(out.ReplicationConfiguration.Rules, rules) {
return fmt.Errorf("bad replication rules, expected: %v, got %v", rules, out.ReplicationConfiguration.Rules)
}

return nil
}
}

// These need a bit of randomness as the name can only be used once globally
// within AWS
func testAccBucketName(randInt int) string {
Expand Down Expand Up @@ -1799,6 +1881,57 @@ resource "aws_s3_bucket" "destination" {
`, randInt, randInt, randInt)
}

func testAccAWSS3BucketConfigReplicationWithSseKmsEncryptedObjects(randInt int) string {
return fmt.Sprintf(testAccAWSS3BucketConfigReplicationBasic+`
resource "aws_kms_key" "replica" {
provider = "aws.euwest"
description = "TF Acceptance Test S3 repl KMS key"
deletion_window_in_days = 7
}

resource "aws_s3_bucket" "bucket" {
provider = "aws.uswest2"
bucket = "tf-test-bucket-%d"
acl = "private"

versioning {
enabled = true
}

replication_configuration {
role = "${aws_iam_role.role.arn}"
rules {
id = "foobar"
prefix = "foo"
status = "Enabled"

destination {
bucket = "${aws_s3_bucket.destination.arn}"
storage_class = "STANDARD"
replica_kms_key_id = "${aws_kms_key.replica.arn}"
}

source_selection_criteria {
sse_kms_encrypted_objects {
enabled = true
}
}
}
}
}

resource "aws_s3_bucket" "destination" {
provider = "aws.euwest"
bucket = "tf-test-bucket-destination-%d"
region = "eu-west-1"

versioning {
enabled = true
}
}
`, randInt, randInt, randInt)
}

func testAccAWSS3BucketConfigReplicationWithoutStorageClass(randInt int) string {
return fmt.Sprintf(testAccAWSS3BucketConfigReplicationBasic+`
resource "aws_s3_bucket" "bucket" {
Expand Down
12 changes: 12 additions & 0 deletions website/docs/r/s3_bucket.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -405,13 +405,25 @@ The `rules` object supports the following:

* `id` - (Optional) Unique identifier for the rule.
* `destination` - (Required) Specifies the destination for the rule (documented below).
* `source_selection_criteria` - (Optional) Specifies special object selection criteria (documented below).
* `prefix` - (Required) Object keyname prefix identifying one or more objects to which the rule applies. Set as an empty string to replicate the whole bucket.
* `status` - (Required) The status of the rule. Either `Enabled` or `Disabled`. The rule is ignored if status is not Enabled.

The `destination` object supports the following:

* `bucket` - (Required) The ARN of the S3 bucket where you want Amazon S3 to store replicas of the object identified by the rule.
* `storage_class` - (Optional) The class of storage used to store the object.
* `replica_kms_key_id` - (Optional) Destination KMS encryption key ID for SSE-KMS replication. Must be used in conjunction with
`sse_kms_encrypted_objects` source selection criteria.

The `source_selection_criteria` object supports the following:

* `sse_kms_encrypted_objects` - (Optional) Match SSE-KMS encrypted objects (documented below). If specified, `replica_kms_key_id`
in `destination` must be specified as well.

The `sse_kms_encrypted_objects` object supports the following:

* `enabled` - (Required) Boolean which indicates if this criteria is enabled.

The `server_side_encryption_configuration` object supports the following:

Expand Down