From 399264e3611475953c3ef2a02206dd0a7c580dce Mon Sep 17 00:00:00 2001 From: Devon Bleak Date: Mon, 8 Jan 2018 09:21:51 -0800 Subject: [PATCH 1/2] #2672: d/aws_iam_policy_document add support for source_json --- aws/data_source_aws_iam_policy_document.go | 17 +- ...ata_source_aws_iam_policy_document_test.go | 208 ++++++++++++++++++ aws/iam_policy_model.go | 51 +++++ .../docs/d/iam_policy_document.html.markdown | 3 + 4 files changed, 276 insertions(+), 3 deletions(-) diff --git a/aws/data_source_aws_iam_policy_document.go b/aws/data_source_aws_iam_policy_document.go index 2c89355d6d0e..6376bd32ac1d 100644 --- a/aws/data_source_aws_iam_policy_document.go +++ b/aws/data_source_aws_iam_policy_document.go @@ -85,6 +85,10 @@ func dataSourceAwsIamPolicyDocument() *schema.Resource { }, }, }, + "source_json": { + Type: schema.TypeString, + Optional: true, + }, "json": { Type: schema.TypeString, Computed: true, @@ -94,17 +98,22 @@ func dataSourceAwsIamPolicyDocument() *schema.Resource { } func dataSourceAwsIamPolicyDocumentRead(d *schema.ResourceData, meta interface{}) error { - doc := &IAMPolicyDoc{ - Version: "2012-10-17", + doc := &IAMPolicyDoc{} + + if sourceJson, hasSourceJson := d.GetOk("source_json"); hasSourceJson { + if err := json.Unmarshal([]byte(sourceJson.(string)), doc); err != nil { + return err + } } + doc.Version = "2012-10-17" + if policyId, hasPolicyId := d.GetOk("policy_id"); hasPolicyId { doc.Id = policyId.(string) } var cfgStmts = d.Get("statement").([]interface{}) stmts := make([]*IAMPolicyStatement, len(cfgStmts)) - doc.Statements = stmts for i, stmtI := range cfgStmts { cfgStmt := stmtI.(map[string]interface{}) stmt := &IAMPolicyStatement{ @@ -148,6 +157,8 @@ func dataSourceAwsIamPolicyDocumentRead(d *schema.ResourceData, meta interface{} stmts[i] = stmt } + doc.Statements = append(doc.Statements, stmts...) + jsonDoc, err := json.MarshalIndent(doc, "", " ") if err != nil { // should never happen if the above code is correct diff --git a/aws/data_source_aws_iam_policy_document_test.go b/aws/data_source_aws_iam_policy_document_test.go index 9b8b43b1ccca..a38d55164b10 100644 --- a/aws/data_source_aws_iam_policy_document_test.go +++ b/aws/data_source_aws_iam_policy_document_test.go @@ -28,6 +28,34 @@ func TestAccAWSDataSourceIAMPolicyDocument_basic(t *testing.T) { }) } +func TestAccAWSDataSourceIAMPolicyDocument_source(t *testing.T) { + // This really ought to be able to be a unit test rather than an + // acceptance test, but just instantiating the AWS provider requires + // some AWS API calls, and so this needs valid AWS credentials to work. + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccAWSIAMPolicyDocumentSourceConfig, + Check: resource.ComposeTestCheckFunc( + testAccCheckStateValue("data.aws_iam_policy_document.test_source", "json", + testAccAWSIAMPolicyDocumentSourceExpectedJSON, + ), + ), + }, + { + Config: testAccAWSIAMPolicyDocumentSourceBlankConfig, + Check: resource.ComposeTestCheckFunc( + testAccCheckStateValue("data.aws_iam_policy_document.test_source_blank", "json", + testAccAWSIAMPolicyDocumentSourceBlankExpectedJSON, + ), + ), + }, + }, + }) +} + func testAccCheckStateValue(id, name, value string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[id] @@ -187,3 +215,183 @@ var testAccAWSIAMPolicyDocumentExpectedJSON = `{ } ] }` + +var testAccAWSIAMPolicyDocumentSourceConfig = ` +data "aws_iam_policy_document" "test" { + policy_id = "policy_id" + statement { + sid = "1" + actions = [ + "s3:ListAllMyBuckets", + "s3:GetBucketLocation", + ] + resources = [ + "arn:aws:s3:::*", + ] + } + + statement { + actions = [ + "s3:ListBucket", + ] + resources = [ + "arn:aws:s3:::foo", + ] + condition { + test = "StringLike" + variable = "s3:prefix" + values = [ + "home/", + "home/&{aws:username}/", + ] + } + + not_principals { + type = "AWS" + identifiers = ["arn:blahblah:example"] + } + } + + statement { + actions = [ + "s3:*", + ] + resources = [ + "arn:aws:s3:::foo/home/&{aws:username}", + "arn:aws:s3:::foo/home/&{aws:username}/*", + ] + principals { + type = "AWS" + identifiers = ["arn:blahblah:example"] + } + } + + statement { + effect = "Deny" + not_actions = ["s3:*"] + not_resources = ["arn:aws:s3:::*"] + } + + # Normalization of wildcard principals + statement { + effect = "Allow" + actions = ["kinesis:*"] + principals { + type = "AWS" + identifiers = ["*"] + } + } + statement { + effect = "Allow" + actions = ["firehose:*"] + principals { + type = "*" + identifiers = ["*"] + } + } + +} + +data "aws_iam_policy_document" "test_source" { + source_json = "${data.aws_iam_policy_document.test.json}" + + statement { + sid = "SourceJSONTest1" + actions = ["*"] + resources = ["*"] + } +} +` + +var testAccAWSIAMPolicyDocumentSourceExpectedJSON = `{ + "Version": "2012-10-17", + "Id": "policy_id", + "Statement": [ + { + "Sid": "1", + "Effect": "Allow", + "Action": [ + "s3:ListAllMyBuckets", + "s3:GetBucketLocation" + ], + "Resource": "arn:aws:s3:::*" + }, + { + "Sid": "", + "Effect": "Allow", + "Action": "s3:ListBucket", + "Resource": "arn:aws:s3:::foo", + "NotPrincipal": { + "AWS": "arn:blahblah:example" + }, + "Condition": { + "StringLike": { + "s3:prefix": [ + "home/${aws:username}/", + "home/" + ] + } + } + }, + { + "Sid": "", + "Effect": "Allow", + "Action": "s3:*", + "Resource": [ + "arn:aws:s3:::foo/home/${aws:username}/*", + "arn:aws:s3:::foo/home/${aws:username}" + ], + "Principal": { + "AWS": "arn:blahblah:example" + } + }, + { + "Sid": "", + "Effect": "Deny", + "NotAction": "s3:*", + "NotResource": "arn:aws:s3:::*" + }, + { + "Sid": "", + "Effect": "Allow", + "Action": "kinesis:*", + "Principal": "*" + }, + { + "Sid": "", + "Effect": "Allow", + "Action": "firehose:*", + "Principal": "*" + }, + { + "Sid": "SourceJSONTest1", + "Effect": "Allow", + "Action": "*", + "Resource": "*" + } + ] +}` + +var testAccAWSIAMPolicyDocumentSourceBlankConfig = ` +data "aws_iam_policy_document" "test_source_blank" { + source_json = "" + + statement { + sid = "SourceJSONTest2" + actions = ["*"] + resources = ["*"] + } +} +` + +var testAccAWSIAMPolicyDocumentSourceBlankExpectedJSON = `{ + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "SourceJSONTest2", + "Effect": "Allow", + "Action": "*", + "Resource": "*" + } + ] +}` diff --git a/aws/iam_policy_model.go b/aws/iam_policy_model.go index 81306971da59..861f2847aba5 100644 --- a/aws/iam_policy_model.go +++ b/aws/iam_policy_model.go @@ -2,6 +2,7 @@ package aws import ( "encoding/json" + "fmt" "sort" ) @@ -75,6 +76,29 @@ func (ps IAMPolicyStatementPrincipalSet) MarshalJSON() ([]byte, error) { return json.Marshal(&raw) } +func (ps *IAMPolicyStatementPrincipalSet) UnmarshalJSON(b []byte) error { + var out IAMPolicyStatementPrincipalSet + + var data interface{} + if err := json.Unmarshal(b, &data); err != nil { + return err + } + + switch t := data.(type) { + case string: + out = append(out, IAMPolicyStatementPrincipal{Type: "*", Identifiers: []string{"*"}}) + case map[string]interface{}: + for key, value := range data.(map[string]interface{}) { + out = append(out, IAMPolicyStatementPrincipal{Type: key, Identifiers: value}) + } + default: + return fmt.Errorf("Unsupported data type %s for IAMPolicyStatementPrincipalSet", t) + } + + *ps = out + return nil +} + func (cs IAMPolicyStatementConditionSet) MarshalJSON() ([]byte, error) { raw := map[string]map[string]interface{}{} @@ -99,6 +123,33 @@ func (cs IAMPolicyStatementConditionSet) MarshalJSON() ([]byte, error) { return json.Marshal(&raw) } +func (cs *IAMPolicyStatementConditionSet) UnmarshalJSON(b []byte) error { + var out IAMPolicyStatementConditionSet + + var data map[string]map[string]interface{} + if err := json.Unmarshal(b, &data); err != nil { + return err + } + + for test_key, test_value := range data { + for var_key, var_values := range test_value { + switch var_values.(type) { + case string: + out = append(out, IAMPolicyStatementCondition{Test: test_key, Variable: var_key, Values: []string{var_values.(string)}}) + case []interface{}: + values := []string{} + for _, v := range var_values.([]interface{}) { + values = append(values, v.(string)) + } + out = append(out, IAMPolicyStatementCondition{Test: test_key, Variable: var_key, Values: values}) + } + } + } + + *cs = out + return nil +} + func iamPolicyDecodeConfigStringList(lI []interface{}) interface{} { if len(lI) == 1 { return lI[0].(string) diff --git a/website/docs/d/iam_policy_document.html.markdown b/website/docs/d/iam_policy_document.html.markdown index bb12823feadc..7264ac3d73f5 100644 --- a/website/docs/d/iam_policy_document.html.markdown +++ b/website/docs/d/iam_policy_document.html.markdown @@ -78,6 +78,9 @@ valid to use literal JSON strings within your configuration, or to use the The following arguments are supported: * `policy_id` (Optional) - An ID for the policy document. +* `source_json` (Optional) - An IAM policy document to import and add the + statements from. Use this to, for example, customize resource policies in + modules. * `statement` (Required) - A nested configuration block (described below) configuring one *statement* to be included in the policy document. From 91862d20de2b3c929c449686c8f353f012712a3f Mon Sep 17 00:00:00 2001 From: Devon Bleak Date: Sun, 4 Feb 2018 21:21:54 -0800 Subject: [PATCH 2/2] Add merging of duplicate SIDs, add override_json support --- aws/data_source_aws_iam_policy_document.go | 30 ++++- ...ata_source_aws_iam_policy_document_test.go | 115 ++++++++++++++++ aws/iam_policy_model.go | 16 +++ .../docs/d/iam_policy_document.html.markdown | 124 +++++++++++++++++- 4 files changed, 278 insertions(+), 7 deletions(-) diff --git a/aws/data_source_aws_iam_policy_document.go b/aws/data_source_aws_iam_policy_document.go index 6376bd32ac1d..cb14494f7d9e 100644 --- a/aws/data_source_aws_iam_policy_document.go +++ b/aws/data_source_aws_iam_policy_document.go @@ -26,10 +26,18 @@ func dataSourceAwsIamPolicyDocument() *schema.Resource { Read: dataSourceAwsIamPolicyDocumentRead, Schema: map[string]*schema.Schema{ + "override_json": { + Type: schema.TypeString, + Optional: true, + }, "policy_id": { Type: schema.TypeString, Optional: true, }, + "source_json": { + Type: schema.TypeString, + Optional: true, + }, "statement": { Type: schema.TypeList, Required: true, @@ -85,10 +93,6 @@ func dataSourceAwsIamPolicyDocument() *schema.Resource { }, }, }, - "source_json": { - Type: schema.TypeString, - Optional: true, - }, "json": { Type: schema.TypeString, Computed: true, @@ -159,6 +163,24 @@ func dataSourceAwsIamPolicyDocumentRead(d *schema.ResourceData, meta interface{} doc.Statements = append(doc.Statements, stmts...) + // merge in our override_json + if overrideJson, hasOverrideJson := d.GetOk("override_json"); hasOverrideJson { + overrideDoc := &IAMPolicyDoc{} + if err := json.Unmarshal([]byte(overrideJson.(string)), overrideDoc); err != nil { + return err + } + + if len(overrideDoc.Id) > 0 { + doc.Id = overrideDoc.Id + } + + if len(overrideDoc.Statements) > 0 { + doc.Statements = append(doc.Statements, overrideDoc.Statements...) + } + } + + doc.DeDupSids() + jsonDoc, err := json.MarshalIndent(doc, "", " ") if err != nil { // should never happen if the above code is correct diff --git a/aws/data_source_aws_iam_policy_document_test.go b/aws/data_source_aws_iam_policy_document_test.go index a38d55164b10..bd7367f62bc0 100644 --- a/aws/data_source_aws_iam_policy_document_test.go +++ b/aws/data_source_aws_iam_policy_document_test.go @@ -56,6 +56,40 @@ func TestAccAWSDataSourceIAMPolicyDocument_source(t *testing.T) { }) } +func TestAccAWSDataSourceIAMPolicyDocument_sourceConflicting(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccAWSIAMPolicyDocumentSourceConflictingConfig, + Check: resource.ComposeTestCheckFunc( + testAccCheckStateValue("data.aws_iam_policy_document.test_source_conflicting", "json", + testAccAWSIAMPolicyDocumentSourceConflictingExpectedJSON, + ), + ), + }, + }, + }) +} + +func TestAccAWSDataSourceIAMPolicyDocument_override(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccAWSIAMPolicyDocumentOverrideConfig, + Check: resource.ComposeTestCheckFunc( + testAccCheckStateValue("data.aws_iam_policy_document.test_override", "json", + testAccAWSIAMPolicyDocumentOverrideExpectedJSON, + ), + ), + }, + }, + }) +} + func testAccCheckStateValue(id, name, value string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[id] @@ -395,3 +429,84 @@ var testAccAWSIAMPolicyDocumentSourceBlankExpectedJSON = `{ } ] }` + +var testAccAWSIAMPolicyDocumentSourceConflictingConfig = ` +data "aws_iam_policy_document" "test_source" { + statement { + sid = "SourceJSONTestConflicting" + actions = ["iam:*"] + resources = ["*"] + } +} + +data "aws_iam_policy_document" "test_source_conflicting" { + source_json = "${data.aws_iam_policy_document.test_source.json}" + + statement { + sid = "SourceJSONTestConflicting" + actions = ["*"] + resources = ["*"] + } +} +` + +var testAccAWSIAMPolicyDocumentSourceConflictingExpectedJSON = `{ + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "SourceJSONTestConflicting", + "Effect": "Allow", + "Action": "*", + "Resource": "*" + } + ] +}` + +var testAccAWSIAMPolicyDocumentOverrideConfig = ` +data "aws_iam_policy_document" "override" { + statement { + sid = "SidToOverwrite" + + actions = ["s3:*"] + resources = ["*"] + } +} + +data "aws_iam_policy_document" "test_override" { + override_json = "${data.aws_iam_policy_document.override.json}" + + statement { + actions = ["ec2:*"] + resources = ["*"] + } + + statement { + sid = "SidToOverwrite" + + actions = ["s3:*"] + + resources = [ + "arn:aws:s3:::somebucket", + "arn:aws:s3:::somebucket/*", + ] + } +} +` + +var testAccAWSIAMPolicyDocumentOverrideExpectedJSON = `{ + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "", + "Effect": "Allow", + "Action": "ec2:*", + "Resource": "*" + }, + { + "Sid": "SidToOverwrite", + "Effect": "Allow", + "Action": "s3:*", + "Resource": "*" + } + ] +}` diff --git a/aws/iam_policy_model.go b/aws/iam_policy_model.go index 861f2847aba5..aa3db834ec9e 100644 --- a/aws/iam_policy_model.go +++ b/aws/iam_policy_model.go @@ -38,6 +38,22 @@ type IAMPolicyStatementCondition struct { type IAMPolicyStatementPrincipalSet []IAMPolicyStatementPrincipal type IAMPolicyStatementConditionSet []IAMPolicyStatementCondition +func (self *IAMPolicyDoc) DeDupSids() { + // de-dupe the statements by traversing backwards and removing duplicate Sids + sidsSeen := map[string]bool{} + l := len(self.Statements) - 1 + for i := range self.Statements { + if sid := self.Statements[l-i].Sid; len(sid) > 0 { + if sidsSeen[sid] { + // we've seen this sid already so remove the duplicate + self.Statements = append(self.Statements[:l-i], self.Statements[l-i+1:]...) + } + // mark this sid seen + sidsSeen[sid] = true + } + } +} + func (ps IAMPolicyStatementPrincipalSet) MarshalJSON() ([]byte, error) { raw := map[string]interface{}{} diff --git a/website/docs/d/iam_policy_document.html.markdown b/website/docs/d/iam_policy_document.html.markdown index 7264ac3d73f5..6f3d48781c18 100644 --- a/website/docs/d/iam_policy_document.html.markdown +++ b/website/docs/d/iam_policy_document.html.markdown @@ -78,9 +78,14 @@ valid to use literal JSON strings within your configuration, or to use the The following arguments are supported: * `policy_id` (Optional) - An ID for the policy document. -* `source_json` (Optional) - An IAM policy document to import and add the - statements from. Use this to, for example, customize resource policies in - modules. +* `source_json` (Optional) - An IAM policy document to import as a base for the + current policy document. Statements with non-blank `sid`s in the current + policy document will overwrite statements with the same `sid` in the source + json. Statements without an `sid` cannot be overwritten. +* `override_json` (Optional) - An IAM policy document to import and override the + current policy document. Statements with non-blank `sid`s in the override + document will overwrite statements with the same `sid` in the current document. + Statements without an `sid` cannot be overwritten. * `statement` (Required) - A nested configuration block (described below) configuring one *statement* to be included in the policy document. @@ -169,3 +174,116 @@ data "aws_iam_policy_document" "event_stream_bucket_role_assume_role_policy" { } } ``` + +## Example with Source and Override + +Showing how you can use `source_json` and `override_json` + +```hcl +data "aws_iam_policy_document" "source" { + statement { + actions = ["ec2:*"] + resources = ["*"] + } + + statement { + sid = "SidToOverwrite" + + actions = ["s3:*"] + resources = ["*"] + } +} + +data "aws_iam_policy_document" "source_json_example" { + source_json = "${data.aws_iam_policy_document.source.json}" + + statement { + sid = "SidToOverwrite" + + actions = ["s3:*"] + + resources = [ + "arn:aws:s3:::somebucket", + "arn:aws:s3:::somebucket/*", + ] + } +} + +data "aws_iam_policy_document" "override" { + statement { + sid = "SidToOverwrite" + + actions = ["s3:*"] + resources = ["*"] + } +} + +data "aws_iam_policy_document" "override_json_example" { + override_json = "${data.aws_iam_policy_document.override.json}" + + statement { + actions = ["ec2:*"] + resources = ["*"] + } + + statement { + sid = "SidToOverwrite" + + actions = ["s3:*"] + + resources = [ + "arn:aws:s3:::somebucket", + "arn:aws:s3:::somebucket/*", + ] + } +} +``` + +`data.aws_iam_policy_document.source_json_example.json` will evaluate to: + +```json +{ + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "", + "Effect": "Allow", + "Action": "ec2:*", + "Resource": "*" + }, + { + "Sid": "SidToOverwrite", + "Effect": "Allow", + "Action": "s3:*", + "Resource": [ + "arn:aws:s3:::somebucket/*", + "arn:aws:s3:::somebucket" + ] + } + ] +} +``` + +`data.aws_iam_policy_document.override_json_example.json` will evaluate to: + +```json +{ + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "", + "Effect": "Allow", + "Action": "ec2:*", + "Resource": "*" + }, + { + "Sid": "SidToOverwrite", + "Effect": "Allow", + "Action": "s3:*", + "Resource": "*" + } + ] +} +``` + +You can also combine `source_json` and `override_json` in the same document.