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

provider/aws: Add Cognito Identity Pool Roles Attachment #12846

Closed
wants to merge 1 commit into from

Conversation

Ninir
Copy link
Contributor

@Ninir Ninir commented Mar 18, 2017

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

Related issues

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

@Ninir Ninir changed the title provider/aws: Add Cognito Identity provider/aws: Add Cognito Identity Pool Mar 18, 2017
@Ninir Ninir changed the title provider/aws: Add Cognito Identity Pool [WIP] provider/aws: Add Cognito Identity Pool Mar 18, 2017
@Ninir Ninir force-pushed the f-cognito branch 6 times, most recently from b66a057 to 72b422d Compare March 24, 2017 13:11
@Ninir Ninir force-pushed the f-cognito branch 4 times, most recently from 1bd37d4 to a700c64 Compare April 3, 2017 21:24
@radeksimko
Copy link
Member

radeksimko commented Apr 12, 2017

Hi @Ninir
thanks for the PR.
Would you mind splitting this into at least 3 PRs to make the review a bit easier? 😅

  • vendor updates - I think we could merge that one pretty much straight away and then you can rebase and work with less LOC in your other PRs
  • aws_cognito_identity_pool
  • aws_cognito_identity_pool_roles_attachment

Thanks.

@radeksimko radeksimko added the waiting-response An issue/pull request is waiting for a response from the community label Apr 12, 2017
@Ninir
Copy link
Contributor Author

Ninir commented Apr 12, 2017 via email

@radeksimko radeksimko removed the waiting-response An issue/pull request is waiting for a response from the community label Apr 12, 2017
@Ninir Ninir changed the title [WIP] provider/aws: Add Cognito Identity Pool [WIP] provider/aws: Add Cognito Identity Pool Roles Attachment Apr 21, 2017
@Ninir Ninir force-pushed the f-cognito branch 4 times, most recently from 4cea061 to 4a54a1b Compare April 24, 2017 17:45
@Ninir Ninir changed the title [WIP] provider/aws: Add Cognito Identity Pool Roles Attachment provider/aws: Add Cognito Identity Pool Roles Attachment Apr 25, 2017
@Ninir
Copy link
Contributor Author

Ninir commented Apr 25, 2017

@radeksimko Kind of ready on my side :)

"ambiguous_role_resolution": {
Type: schema.TypeString,
ValidateFunc: validateCognitoRoleMappingsAmbiguousRoleResolution,
Optional: true, // Required if Type equals Token or Rules.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

@ghost
Copy link

ghost commented Apr 8, 2020

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants