Cognito: Added Identity Pool Roles Attachment

[Ninir]

Description

This adds the AWS Cognito Identity Roles association resource, based on the following API: http://docs.aws.amazon.com/cognitoidentity/latest/APIReference/API_SetIdentityPoolRoles.html

(Migrated from hashicorp/terraform#12846)

Related issues

• Missing AWS Cognito Support terraform#8309

Tests

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCognitoIdentityPoolRolesAttachment_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/04/21 20:27:48 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCognitoIdentityPoolRolesAttachment_ -timeout 120m
=== RUN   TestAccAWSCognitoIdentityPoolRolesAttachment_basic
--- PASS: TestAccAWSCognitoIdentityPoolRolesAttachment_basic (59.64s)
=== RUN   TestAccAWSCognitoIdentityPoolRolesAttachment_roleMappings
--- PASS: TestAccAWSCognitoIdentityPoolRolesAttachment_roleMappings (103.70s)
=== RUN   TestAccAWSCognitoIdentityPoolRolesAttachment_roleMappingsWithError
--- PASS: TestAccAWSCognitoIdentityPoolRolesAttachment_roleMappingsWithError (17.80s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    181.171s

TODOs

• Add the Cognito Identity Roles Resource
• Add Cognito Identity Roles Resource validation
• Add the Cognito Identity Roles Resource documentation
• Add Cognito Identity Roles Resource test cases
• Compile & play with it, in order to ensure every cases are handled & guards are present

[Ninir]

At the moment, as token is required and has only 2 possibles values (Token or Rules), this seems required too.
The API states that this is Optional though.

[radeksimko]

Hey @Ninir
Sorry it took me so long to get to this!

I looked at the API structs and the schema to understand the reasoning behind such deeply nested schema, which I think is/was the primary blocker for this PR.

How would you feel about flattening the schema like this?

map[string]*schema.Schema{
	"identity_pool_id": {
		Type:     schema.TypeString,
		Required: true,
		ForceNew: true,
	},

	"role_mapping": {
		Type:     schema.TypeSet,
		Optional: true,
		MaxItems: 1,
		Set:      nil,
		Elem: &schema.Resource{
			Schema: map[string]*schema.Schema{
				"identity_provider": {
					Type:     schema.TypeString,
					Required: true,
				},
				"ambiguous_role_resolution": {
					Type:         schema.TypeString,
					ValidateFunc: validateCognitoRoleMappingsAmbiguousRoleResolution,
					Optional:     true, // Required if Type equals Token or Rules.
				},
				"type": {
					Type:         schema.TypeString,
					Required:     true,
					ValidateFunc: validateCognitoRoleMappingsType,
				},
				"mapping_rule": {
					Type:     schema.TypeList,
					Required: true,
					MaxItems: 25,
					Elem: &schema.Resource{
						Schema: map[string]*schema.Schema{
							"claim": {
								Type:         schema.TypeString,
								Required:     true,
								ValidateFunc: validateCognitoRoleMappingsRulesClaim,
							},
							"match_type": {
								Type:         schema.TypeString,
								Required:     true,
								ValidateFunc: validateCognitoRoleMappingsRulesMatchType,
							},
							"role_arn": {
								Type:         schema.TypeString,
								Required:     true,
								ValidateFunc: validateArn,
							},
							"value": {
								Type:         schema.TypeString,
								Required:     true,
								ValidateFunc: validateCognitoRoleMappingsRulesValue,
							},
						},
					},
				},
			},
		},
	},

	"roles": {
		Type:     schema.TypeMap,
		Required: true,
		ForceNew: true,
		Elem: &schema.Resource{
			Schema: map[string]*schema.Schema{
				"authenticated": {
					Type:         schema.TypeString,
					ValidateFunc: validateArn,
					Optional:     true, // Required if unauthenticated isn't defined.
				},
				"unauthenticated": {
					Type:         schema.TypeString,
					ValidateFunc: validateArn,
					Optional:     true, // Required if authenticated isn't defined.
				},
			},
		},
	},
}

then the HCL config could look something like this

resource "aws_cognito_identity_pool_roles_attachment" "main" {
  identity_pool_id = "${aws_cognito_identity_pool.main.id}"

  role_mapping {
    identity_provider         = "graph.facebook.com"
    ambiguous_role_resolution = "AuthenticatedRole"
    type                      = "Rules"

    mapping_rule {
      claim      = "isAdmin"
      match_type = "Equals"
      role_arn   = "${aws_iam_role.authenticated.arn}"
      value      = "paid"
    }
  }

  roles {
    "authenticated" = "${aws_iam_role.authenticated.arn}"
  }
}

eventually I'm thinking identity_provider could be shortened to idp, but that's really a nitpick in this context - the main goal is to reduce the nesting here.

Let me know what you think - then we can proceed with the rest of the PR as the schema change is the probably the most significant thing here.

[Ninir]

@radeksimko This is a much better approach, thank you for the suggestion :)

While tests are passing, I would like to check another time that everything is ok on my side before merging the work:

make testacc TEST=./aws TESTARGS='-run=TestAccAWSCognitoIdentityPoolRolesAttachment_roleMappings'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSCognitoIdentityPoolRolesAttachment_roleMappings -timeout 120m
=== RUN   TestAccAWSCognitoIdentityPoolRolesAttachment_roleMappings
--- PASS: TestAccAWSCognitoIdentityPoolRolesAttachment_roleMappings (47.47s)
=== RUN   TestAccAWSCognitoIdentityPoolRolesAttachment_roleMappingsWithAmbiguousRoleResolutionError
--- PASS: TestAccAWSCognitoIdentityPoolRolesAttachment_roleMappingsWithAmbiguousRoleResolutionError (11.74s)
=== RUN   TestAccAWSCognitoIdentityPoolRolesAttachment_roleMappingsWithRulesTypeError
--- PASS: TestAccAWSCognitoIdentityPoolRolesAttachment_roleMappingsWithRulesTypeError (11.12s)
=== RUN   TestAccAWSCognitoIdentityPoolRolesAttachment_roleMappingsWithTokenTypeError
--- PASS: TestAccAWSCognitoIdentityPoolRolesAttachment_roleMappingsWithTokenTypeError (11.02s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	81.410s

[radeksimko]

I left you one comment, but this LGTM once you fix the Printf vet errors as reported in Travis.

Thanks for bringing it to the finish line! 👍

[radeksimko]

Nitpick, but how do you feel about exposing this as a wrapper around fmt.Sprinf so that we get the validation-at-compile-time benefit?

[Ninir]

Isn't it the case as this constant is concatenated to another string but wrapped into a fmt.Sprintf line https://github.com/terraform-providers/terraform-provider-aws/pull/863/files/4716be981696dcd13458a0e660bfe19b1ab15860#diff-213dd3d721447149c2fa3d3ddfb5e5b1R305 ?

[radeksimko]

The link isn't valid anymore, but yeah - I thought you could do something like this:

func testAccAWSCognitoIdentityPoolRolesAttachmentConfig_basic(name string) string {
    return fmt.Sprintf(baseAWSCognitoIdentityPoolRolesAttachmentConfig(name)+`
resource "aws_cognito_identity_pool_roles_attachment" "main" {

the benefit you get there is you may change baseAWSCognitoIdentityPoolRolesAttachmentConfig template and add/remove modifiers as necessary without having to refactor all references.

It's not a big deal though - so feel free to ignore.

[Ninir]

Done :) thanks for the advice!

[Ninir]

@radeksimko Ready to go on my side :)

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!