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/ssoadmin_account_assignment: new resource; d/identitystore: new data sources #15322

Merged
merged 55 commits into from
Jan 14, 2021

Conversation

burck1
Copy link
Contributor

@burck1 burck1 commented Sep 23, 2020

Update 2020/11/03

To help us to continue to move forward, please go give a thumbs up on #15808.

We've completed most of the work for supporting the AWS SSO and AWS SSO Identity Store resources and datasources in Terraform. This [WIP] PR encompasses all of that work. But, the contribution guide for this repo recommends submitting small pull requests with the minimum required resources, so we've submitted #15808 as our initial PR with just data.aws_sso_instance, data.aws_sso_permission_set, and aws_sso_permission_set. Once that's merged, we will submit PRs for all of the other resources and data sources since they depend on that initial PR.


Updated 2020/09/29

data "aws_sso_instance" "selected" {}

resource "aws_sso_permission_set" "example" {
  instance_arn = data.aws_sso_instance.selected.arn
  name         = "Example"
  # ..
}
data "aws_caller_identity" "current" {}

data "aws_sso_instance" "selected" { }

data "aws_sso_permission_set" "example" {
  instance_arn = data.aws_sso_instance.selected.arn
  name         = "Example"
}

data "aws_identity_store_group" "example_group" {
  identity_store_id = data.aws_sso_instance.selected.identity_store_id
  display_name      = "ExampleGroup@example.com"
}

resource "aws_sso_assignment" "example_group" {
  instance_arn       = data.aws_sso_instance.selected.arn
  permission_set_arn = data.aws_sso_permission_set.example.arn

  target_type = "AWS_ACCOUNT"
  target_id   = data.aws_caller_identity.current.account_id

  principal_type = "GROUP"
  principal_id   = data.aws_identity_store_group.example_group.group_id
}

data "aws_identity_store_user" "example_user" {
  identity_store_id = data.aws_sso_instance.selected.identity_store_id
  user_name         = "example@example.com"
}

resource "aws_sso_assignment" "example_user" {
  instance_arn       = data.aws_sso_instance.selected.arn
  permission_set_arn = data.aws_sso_permission_set.example.arn

  target_type = "AWS_ACCOUNT"
  target_id   = data.aws_caller_identity.current.account_id

  principal_type = "USER"
  principal_id   = data.aws_identity_store_user.example_user.user_id
}

References

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates #15108

@ghost ghost added the size/M Managed by automation to categorize the size of a PR. label Sep 23, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Sep 23, 2020
@sklarsa
Copy link

sklarsa commented Sep 25, 2020

As discussed #15108, I'm happy to help. Where do you need assistance?

@ghost ghost added size/L Managed by automation to categorize the size of a PR. size/XL Managed by automation to categorize the size of a PR. provider Pertains to the provider itself, rather than any interaction with AWS. and removed size/M Managed by automation to categorize the size of a PR. size/L Managed by automation to categorize the size of a PR. labels Sep 26, 2020
@ghost ghost added documentation Introduces or discusses updates to documentation. size/XXL Managed by automation to categorize the size of a PR. and removed size/XL Managed by automation to categorize the size of a PR. labels Sep 29, 2020
@onitake
Copy link
Contributor

onitake commented Oct 8, 2020

Looks promising!

Where does aws_sso_instance source its data? There is a ListInstances API call, but it may return multiple instances when a user has access to multiple accounts. Can there be more than one instance per account?

For the aws_sso_assignment (or maybe aws_sso_account_assignment is better?), I think the usage could be simplified.
I proposed something like this in #15540 :

resource "aws_sso_account_assignment" "assignment" {
  instance_arn = data.aws_sso_instance.selected.arn
  identity_store_id = data.aws_sso_instance.selected.identity_store_id
  permission_set_arn = aws_sso_permission_set.example.arn
  # principal_group is mutually exclusive with principal_user
  principal_group = "group-name"
  target_account = data.aws_caller_identity.current.account_id
}

There is only one target_type at the moment (AWS_ACCOUNT) and there are only two principal_types: GROUP and USER. The actual group and user IDs could be derived implicitly with a call to ListGroups or ListUsers, as there doesn't seem to be any other use for them at the moment.
On the other hand, excessive amounts of non-dynamic data{} blocks could be avoided with a for_each, which is supported for data sources since Terraform 0.12.6.

There should also be support for managing policy attachments:

resource "aws_sso_managed_policy_attachment" "attachment" {
  instance_arn = data.aws_sso_instance.selected.arn
  policy_arn = "arn:aws:iam::aws:policy/AdministratorAccess"
  permission_set_arn = aws_sso_permission_set.example.arn
}

@burck1
Copy link
Contributor Author

burck1 commented Oct 8, 2020

Hi @onitake. Thanks for your input!

Where does aws_sso_instance source its data? There is a ListInstances API call, but it may return multiple instances when a user has access to multiple accounts. Can there be more than one instance per account?

We're required to make a call to the ListInstances API for two purposes: 1. The CreatePermissionSet API requires the InstanceArn returned from ListInstances and 2. The Identity Store APIs all require the IdentityStoreId returned from ListInstances.

So, in the design I came up with, the aws_sso_instance data source would call the ListInstances API and then users could use the arn and identity_store_id outputs with their aws_sso_permission_set resources, aws_identity_store_group data sources, and aws_identity_store_user data sources.

As for the ListInstances API returning multiple instances, I'm not actually sure when that could happen. At least in my experience with AWS SSO, I've only ever had one SSO Instance. I don't even think there's a way in the AWS Console to create multiple SSO Instances. It'd be great to hear input from anyone who has seen multiple; I.E. If you run aws sso-admin list-instances command, you see multiple instances.

If it's the case that there can be multiple SSO Instances, I'm not sure what to provide users as a filter to the aws_sso_instance data source. That API provides no other info besides the InstanceArn and ListInstances, so I suspect we'll need to require users to provide one of those IDs.

For the aws_sso_assignment (or maybe aws_sso_account_assignment is better?), I think the usage could be simplified.
I proposed something like this in #15540 :

resource "aws_sso_account_assignment" "assignment" {
  instance_arn = data.aws_sso_instance.selected.arn
  identity_store_id = data.aws_sso_instance.selected.identity_store_id
  permission_set_arn = aws_sso_permission_set.example.arn
  # principal_group is mutually exclusive with principal_user
  principal_group = "group-name"
  target_account = data.aws_caller_identity.current.account_id
}

There is only one target_type at the moment (AWS_ACCOUNT) and there are only two principal_types: GROUP and USER. The actual group and user IDs could be derived implicitly with a call to ListGroups or ListUsers, as there doesn't seem to be any other use for them at the moment.
On the other hand, excessive amounts of non-dynamic data{} blocks could be avoided with a for_each, which is supported for data sources since Terraform 0.12.6.

I'd be fine with renaming the aws_sso_assignment resource to aws_sso_account_assignment and removing the target_type variable. I was really just basing the design on the corresponding CloudFormation resource, but I don't know what other target types are planned by AWS for the future.

As for adding the principal_group and principal_user variables to the aws_sso_account_assignment resource rather than having separate aws_identity_store_group and aws_identity_store_user data sources, my concern with that would be that every aws_sso_account_assignment resource would need to make calls to the ListUsers and ListGroups Identity Store APIs. What that means is if your are assigning the same user/group to multiple permission sets then redundant API calls will need to be made to get the corresponding UserId/GroupId. In the interest of minimizing that redundancy, I'd prefer to keep the users & groups as separate data sources for this pull request. Note: This would still allow us to potentially add in those variables to the aws_sso_account_assignment resource in the future as a shortcut, but for keeping backwards compatibility in mind we couldn't go the opposite direction and remove the variables in the future.

There should also be support for managing policy attachments:

resource "aws_sso_managed_policy_attachment" "attachment" {
  instance_arn = data.aws_sso_instance.selected.arn
  policy_arn = "arn:aws:iam::aws:policy/AdministratorAccess"
  permission_set_arn = aws_sso_permission_set.example.arn
}

This is an interesting one. I do plan to add in support for attaching managed policies (and a custom policy), but maybe not in the way you would expect.

If you take a look at the schema I've setup for the aws_sso_permission_set resource, you can see one of the variables it takes is a string list of managed policies (managed_policies). Here's what that would look like:

resource "aws_sso_permission_set" "example" {
  instance_arn = data.aws_sso_instance.selected.arn
  name         = "Example"

  # ..

  managed_policies = [
    "arn:aws:iam::aws:policy/AdministratorAccess",
  ]
}

Now the reason I've designed it like this rather than having a separate aws_sso_managed_policy_attachment resource is the result of an implementation detail of the AWS SSO Admin API for Permission Sets. What AWS requires is that whenever you call CreatePermissionSet, UpdatePermissionSet, AttachManagedPolicyToPermissionSet, PutInlinePolicyToPermissionSet, TagResource, etc, you must then call the ProvisionPermissionSet API in order to actually deploy the permission set to the assigned accounts. So, if we were to create separate terraform resources for managed policy attachments and inline policy attachments, each of those resources would need to call the ProvisionPermissionSet API for their corresponding permission set whenever they are updated. What this means is if you had say just two aws_sso_managed_policy_attachment resources for the same permission set, during a single terraform execution, both of them would potentially need to call the ProvisionPermissionSet API which could result in conflicting provisioning calls (you can only have one active provisioning at a time per account) and even if you don't run into conflicts you would at least have redundant API calls. Alternatively, by having variables on the aws_sso_permission_set resource for inline_policy, managed_policies, and tags, then the aws_sso_permission_set resource is the only terraform resource that ever needs to call the ProvisionPermissionSet API within its Update function.

Note on Account Assignments: The CreateAccountAssignment API states that:

As part of a successful CreateAccountAssignment call, the specified permission set will automatically be provisioned to the account in the form of an IAM policy attached to the SSO-created IAM role. If the permission set is subsequently updated, the corresponding IAM policies attached to roles in your accounts will not be updated automatically. In this case, you will need to call ProvisionPermissionSet to make these updates.

So, in order to ensure that all account assignments are updated whenever the connecting aws_sso_permission_set resource is updated, within the Update function for the aws_sso_permission_set resource I plan to call the ProvisionPermissionSet API with a TargetType parameter of ALL_PROVISIONED_ACCOUNTS. What this means is whenever the permission set is updated, it should re-provision all accounts setup with the aws_sso_account_assignment resource.

@onitake
Copy link
Contributor

onitake commented Oct 9, 2020

So, in the design I came up with, the aws_sso_instance data source would call the ListInstances API and then users could use the arn and identity_store_id outputs with their aws_sso_permission_set resources, aws_identity_store_group data sources, and aws_identity_store_user data sources.

Makes sense.

If it's the case that there can be multiple SSO Instances, I'm not sure what to provide users as a filter to the aws_sso_instance data source. That API provides no other info besides the InstanceArn and ListInstances, so I suspect we'll need to require users to provide one of those IDs.

As long as that is always an option, I don't see a problem. There might be an edge case where a user/API key has access to multiple AWS accounts, not sure what would happen then. I think you should report an error when more than one instance is returned, to ensure nothing unexpected happens.

Makes me wonder why AWS has implemented a ListInstances API in the first place, if there can only be one or zero instances per account.

I'd be fine with renaming the aws_sso_assignment resource to aws_sso_account_assignment and removing the target_type variable. I was really just basing the design on the corresponding CloudFormation resource, but I don't know what other target types are planned by AWS for the future.

Both resource names are ok, my suggestion was simply based on the API call.
I also think the target_type option should exist, but be optional. If AWS adds other types later, it can simply be extended.

As for adding the principal_group and principal_user variables to the aws_sso_account_assignment resource rather than having separate aws_identity_store_group and aws_identity_store_user data sources, my concern with that would be that every aws_sso_account_assignment resource would need to make calls to the ListUsers and ListGroups Identity Store APIs. What that means is if your are assigning the same user/group to multiple permission sets then redundant API calls will need to be made to get the corresponding UserId/GroupId. In the interest of minimizing that redundancy, I'd prefer to keep the users & groups as separate data sources for this pull request. Note: This would still allow us to potentially add in those variables to the aws_sso_account_assignment resource in the future as a shortcut, but for keeping backwards compatibility in mind we couldn't go the opposite direction and remove the variables in the future.

Upon further reflection, what you say makes perfect sense.
Separating the lookup from the provisioning is certainly more future-proof.
But there should definitely be an example in the documentation.

Now the reason I've designed it like this rather than having a separate aws_sso_managed_policy_attachment resource is the result of an implementation detail of the AWS SSO Admin API for Permission Sets. What AWS requires is that whenever you call CreatePermissionSet, UpdatePermissionSet, AttachManagedPolicyToPermissionSet, PutInlinePolicyToPermissionSet, TagResource, etc, you must then call the ProvisionPermissionSet API in order to actually deploy the permission set to the assigned accounts. So, if we were to create separate terraform resources for managed policy attachments and inline policy attachments, each of those resources would need to call the ProvisionPermissionSet API for their corresponding permission set whenever they are updated. What this means is if you had say just two aws_sso_managed_policy_attachment resources for the same permission set, during a single terraform execution, both of them would potentially need to call the ProvisionPermissionSet API which could result in conflicting provisioning calls (you can only have one active provisioning at a time per account) and even if you don't run into conflicts you would at least have redundant API calls. Alternatively, by having variables on the aws_sso_permission_set resource for inline_policy, managed_policies, and tags, then the aws_sso_permission_set resource is the only terraform resource that ever needs to call the ProvisionPermissionSet API within its Update function.

There's certainly no need for a 1:1 mapping of all the API calls. Updating a list in hcl with API magic happening in the background is much more practical.

So, in order to ensure that all account assignments are updated whenever the connecting aws_sso_permission_set resource is updated, within the Update function for the aws_sso_permission_set resource I plan to call the ProvisionPermissionSet API with a TargetType parameter of ALL_PROVISIONED_ACCOUNTS. What this means is whenever the permission set is updated, it should re-provision all accounts setup with the aws_sso_account_assignment resource.

The downside here is that this will also trigger updates on all assignments that are not Terraform managed, as a sideeffect. On the other hand, this is probably what most people would want anyway...

@burck1
Copy link
Contributor Author

burck1 commented Oct 9, 2020

Update

Did some initial testing. All data sources seem to be functioning as expected! Working on the aws_sso_permission_set and aws_sso_assignment resources today.

data_source_aws_sso_instance

provider "aws" {
  region = "us-east-1"
}

data "aws_sso_instance" "selected" {}

output "arn" {
  value = data.aws_sso_instance.selected.arn
}

output "identity_store_id" {
  value = data.aws_sso_instance.selected.identity_store_id
}
> terraform apply
data.aws_sso_instance.selected: Refreshing state...

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

arn = arn:aws:sso:::instance/ssoins-0123456789abcdef
identity_store_id = d-0123456789

Note: Sensitive info has been obfuscated

data_source_aws_sso_permission_set

provider "aws" {
  region = "us-east-1"
}

data "aws_sso_instance" "selected" {}

data "aws_sso_permission_set" "example" {
  instance_arn = data.aws_sso_instance.selected.arn
  name         = "AWSReadOnlyAccess"
}

output "arn" {
  value = data.aws_sso_permission_set.example.arn
}

output "created_date" {
  value = data.aws_sso_permission_set.example.created_date
}

output "instance_arn" {
  value = data.aws_sso_permission_set.example.instance_arn
}

output "name" {
  value = data.aws_sso_permission_set.example.name
}

output "description" {
  value = data.aws_sso_permission_set.example.description
}

output "session_duration" {
  value = data.aws_sso_permission_set.example.session_duration
}

output "relay_state" {
  value = data.aws_sso_permission_set.example.relay_state
}

output "inline_policy" {
  value = data.aws_sso_permission_set.example.inline_policy
}

output "managed_policies" {
  value = data.aws_sso_permission_set.example.managed_policies
}

output "tags" {
  value = data.aws_sso_permission_set.example.tags
}
> terraform apply
data.aws_sso_instance.selected: Refreshing state...
data.aws_sso_permission_set.example: Refreshing state...

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

arn = arn:aws:sso:::permissionSet/ssoins-0123456789abcdef/ps-0123456789abcdef
created_date = 2020-08-15T16:19:38Z
description = This policy grants permissions to view resources and basic metadata across all AWS services
inline_policy = {"Version":"2012-10-17","Statement":[{"Sid":"","Effect":"Allow","Action":["sqs:List*","sqs:Get*"],"Resource":"*"}]}
instance_arn = arn:aws:sso:::instance/ssoins-0123456789abcdef
managed_policies = [
  {
    "arn" = "arn:aws:iam::aws:policy/AWSSupportAccess"
    "name" = "AWSSupportAccess"
  },
  {
    "arn" = "arn:aws:iam::aws:policy/job-function/ViewOnlyAccess"
    "name" = "ViewOnlyAccess"
  },
]
name = AWSReadOnlyAccess
relay_state =
session_duration = PT1H
tags = {}

Note: Sensitive info has been obfuscated

data_source_aws_identity_store_group

provider "aws" {
  region = "us-east-1"
}

data "aws_sso_instance" "selected" {}

data "aws_identity_store_group" "example_group" {
  identity_store_id = data.aws_sso_instance.selected.identity_store_id
  display_name      = "Example Group@example.com"
}

output "identity_store_id" {
  value = data.aws_identity_store_group.example_group.identity_store_id
}

output "group_id" {
  value = data.aws_identity_store_group.example_group.group_id
}

output "display_name" {
  value = data.aws_identity_store_group.example_group.display_name
}
> terraform apply
data.aws_sso_instance.selected: Refreshing state...
data.aws_identity_store_group.example_group: Refreshing state...

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

display_name = Example Group@example.com
group_id = 51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96
identity_store_id = d-0123456789

Note: Sensitive info has been obfuscated

data_source_aws_identity_store_user

provider "aws" {
  region = "us-east-1"
}

data "aws_sso_instance" "selected" {}

data "aws_identity_store_user" "example_user" {
  identity_store_id = data.aws_sso_instance.selected.identity_store_id
  user_name         = "euser@example.com"
}

output "identity_store_id" {
  value = data.aws_identity_store_user.example_user.identity_store_id
}

output "user_id" {
  value = data.aws_identity_store_user.example_user.user_id
}

output "user_name" {
  value = data.aws_identity_store_user.example_user.user_name
}
> terraform apply
data.aws_sso_instance.selected: Refreshing state...
data.aws_identity_store_user.example_user: Refreshing state...

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

identity_store_id = d-0123456789
user_id = 3ecb19c3b7-4a4ffa92-c025-bc69-9e3e-a27352aba8f4
user_name = euser@example.com

Note: Sensitive info has been obfuscated

@rfarro82
Copy link

Hello, I am curious if there is a timeline for when these resources will be added to terraform? I see you are making great progress and I'm really excited!

@burck1
Copy link
Contributor Author

burck1 commented Oct 15, 2020

Update

aws_sso_assignment is now functioning as expected!

provider "aws" {
  region = "us-east-1"
}

data "aws_sso_instance" "selected" {}

data "aws_sso_permission_set" "example" {
  instance_arn = data.aws_sso_instance.selected.arn
  name         = "AWSReadOnlyAccess"
}

data "aws_identity_store_group" "example_group" {
  identity_store_id = data.aws_sso_instance.selected.identity_store_id
  display_name      = "Example Group@example.com"
}

resource "aws_sso_assignment" "example" {
  instance_arn       = data.aws_sso_instance.selected.arn
  permission_set_arn = data.aws_sso_permission_set.example.arn

  target_type = "AWS_ACCOUNT"
  target_id   = "012347678910"

  principal_type = "GROUP"
  principal_id   = data.aws_identity_store_group.example_group.group_id
}

output "instance_arn" {
  value = aws_sso_assignment.example.instance_arn
}

output "permission_set_arn" {
  value = aws_sso_assignment.example.permission_set_arn
}

output "target_type" {
  value = aws_sso_assignment.example.target_type
}

output "target_id" {
  value = aws_sso_assignment.example.target_id
}

output "principal_type" {
  value = aws_sso_assignment.example.principal_type
}

output "principal_id" {
  value = aws_sso_assignment.example.principal_id
}

output "id" {
  value = aws_sso_assignment.example.id
}

output "created_date" {
  value = aws_sso_assignment.example.created_date
}

terraform apply

> terraform apply
data.aws_sso_instance.selected: Refreshing state...
data.aws_sso_permission_set.example: Refreshing state...
data.aws_identity_store_group.example_group: Refreshing state...

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # aws_sso_assignment.example will be created
  + resource "aws_sso_assignment" "example" {
      + created_date       = (known after apply)
      + id                 = (known after apply)
      + instance_arn       = "arn:aws:sso:::instance/ssoins-0123456789abcdef"
      + permission_set_arn = "arn:aws:sso:::permissionSet/ssoins-0123456789abcdef/ps-0123456789abcdef"
      + principal_id       = "51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96"
      + principal_type     = "GROUP"
      + target_id          = "012347678910"
      + target_type        = "AWS_ACCOUNT"
    }

Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

aws_sso_assignment.example: Creating...
aws_sso_assignment.example: Creation complete after 6s [id=ssoins-0123456789abcdef/ps-0123456789abcdef/AWS_ACCOUNT/012347678910/GROUP/51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

Outputs:

created_date = 2020-10-15T02:34:42Z
id = ssoins-0123456789abcdef/ps-0123456789abcdef/AWS_ACCOUNT/012347678910/GROUP/51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96
instance_arn = arn:aws:sso:::instance/ssoins-0123456789abcdef
permission_set_arn = arn:aws:sso:::permissionSet/ssoins-0123456789abcdef/ps-0123456789abcdef
principal_id = 51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96
principal_type = GROUP
target_id = 012347678910
target_type = AWS_ACCOUNT

Note: Sensitive info has been obfuscated

terraform destroy

> terraform destroy
data.aws_sso_instance.selected: Refreshing state...
data.aws_sso_permission_set.example: Refreshing state...
data.aws_identity_store_group.example_group: Refreshing state...
aws_sso_assignment.example: Refreshing state... [id=ssoins-0123456789abcdef/ps-0123456789abcdef/AWS_ACCOUNT/012347678910/GROUP/51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96]

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  - destroy

Terraform will perform the following actions:

  # aws_sso_assignment.example will be destroyed
  - resource "aws_sso_assignment" "example" {
      - created_date       = "2020-10-15T02:34:42Z" -> null
      - id                 = "ssoins-0123456789abcdef/ps-0123456789abcdef/AWS_ACCOUNT/012347678910/GROUP/51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96" -> null
      - instance_arn       = "arn:aws:sso:::instance/ssoins-0123456789abcdef" -> null
      - permission_set_arn = "arn:aws:sso:::permissionSet/ssoins-0123456789abcdef/ps-0123456789abcdef" -> null
      - principal_id       = "51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96" -> null
      - principal_type     = "GROUP" -> null
      - target_id          = "012347678910" -> null
      - target_type        = "AWS_ACCOUNT" -> null
    }

Plan: 0 to add, 0 to change, 1 to destroy.

Do you really want to destroy all resources?
  Terraform will destroy all your managed infrastructure, as shown above.
  There is no undo. Only 'yes' will be accepted to confirm.

  Enter a value: yes

aws_sso_assignment.example: Destroying... [id=ssoins-0123456789abcdef/ps-0123456789abcdef/AWS_ACCOUNT/012347678910/GROUP/51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96]
aws_sso_assignment.example: Destruction complete after 5s

Destroy complete! Resources: 1 destroyed.

Note: Sensitive info has been obfuscated

terraform apply (resource already exists)

> terraform apply
data.aws_sso_instance.selected: Refreshing state...
data.aws_identity_store_group.example_group: Refreshing state...
data.aws_sso_permission_set.example: Refreshing state...

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # aws_sso_assignment.example will be created
  + resource "aws_sso_assignment" "example" {
      + created_date       = (known after apply)
      + id                 = (known after apply)
      + instance_arn       = "arn:aws:sso:::instance/ssoins-0123456789abcdef"
      + permission_set_arn = "arn:aws:sso:::permissionSet/ssoins-0123456789abcdef/ps-0123456789abcdef"
      + principal_id       = "51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96"
      + principal_type     = "GROUP"
      + target_id          = "012347678910"
      + target_type        = "AWS_ACCOUNT"
    }

Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

aws_sso_assignment.example: Creating...

Error: AWS SSO Assignment already exists. Import the resource by calling: terraform import <resource_address> ssoins-0123456789abcdef/ps-0123456789abcdef/AWS_ACCOUNT/012347678910/GROUP/51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96

Note: Sensitive info has been obfuscated

terraform import

> terraform import aws_sso_assignment.example ssoins-0123456789abcdef/ps-0123456789abcdef/AWS_ACCOUNT/012347678910/GROUP/51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96
aws_sso_assignment.example: Importing from ID "ssoins-0123456789abcdef/ps-0123456789abcdef/AWS_ACCOUNT/012347678910/GROUP/51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96"...
aws_sso_assignment.example: Import prepared!
  Prepared aws_sso_assignment for import
aws_sso_assignment.example: Refreshing state... [id=ssoins-0123456789abcdef/ps-0123456789abcdef/AWS_ACCOUNT/012347678910/GROUP/51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96]

Import successful!

The resources that were imported are shown above. These resources are now in
your Terraform state and will henceforth be managed by Terraform.

Note: Sensitive info has been obfuscated

terraform plan (after import)

> terraform plan
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

data.aws_sso_instance.selected: Refreshing state...
data.aws_sso_permission_set.example: Refreshing state...
data.aws_identity_store_group.example_group: Refreshing state...
aws_sso_assignment.example: Refreshing state... [id=ssoins-0123456789abcdef/ps-0123456789abcdef/AWS_ACCOUNT/012347678910/GROUP/51b3755f39-e945c18b-e449-4a93-3e95-12231cb7ef96]

------------------------------------------------------------------------

No changes. Infrastructure is up-to-date.

This means that Terraform did not detect any differences between your
configuration and real physical resources that exist. As a result, no
actions need to be performed.

Note: Sensitive info has been obfuscated

@burck1
Copy link
Contributor Author

burck1 commented Oct 15, 2020

Hi @rfarro82. Thanks for your interest!

With regards to the progress of this PR; we've completed all of the desired data sources and the aws_sso_assignment resource. We have a few tweaks left for the aws_sso_permission_set resource that we're hoping to get through today. After that, we have some tests and documentation that we need to write. Once we have that, I'll update this as a non-work in progress [WIP] PR and then one of the maintainers of this repo will need to review.

As for a timeline, we're hoping to finish the primary work for this PR this week or early next week. After that I'm sure there'll be some back-and-fourth with the repo maintainers before they we can actually merge and schedule this for release. I'm not sure how long that typically takes.

@ghost ghost added the tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. label Oct 16, 2020
@burck1
Copy link
Contributor Author

burck1 commented Oct 23, 2020

Hi @maryelizbeth / @bflad / @gdavison / @anGie44 / @breathingdust / @ksatirli (pulled from FAQ). We think we are mostly done with the work for this PR. We went through the contributing guides and we think we've got everything needed. Right now in this [WIP] PR we've got these new resources and data sources:

  • data.aws_sso_instance
  • data.aws_sso_permission_set
  • aws_sso_permission_set
  • aws_sso_assignment
  • data.aws_identity_store_group
  • data.aws_identity_store_user

My question is what would be the best way for us to split this PR up into multiple? Based on the new service docs, "We prefer you to submit the necessary minimum in a single PR ...", we are planning on creating an initial PR with data.aws_sso_instance, data.aws_sso_permission_set, and aws_sso_permission_set. Then submitting individual PRs for aws_sso_assignment, data.aws_identity_store_group, and data.aws_identity_store_user. Does that line up with your expectations?

@burck1
Copy link
Contributor Author

burck1 commented Oct 27, 2020

My question is what would be the best way for us to split this PR up into multiple?

@ewbankkit and @DrFaust92; You've both had a bunch of PRs merged recently. Are you able to provide us with any advice for how to best setup these PRs?

@gdavison
Copy link
Contributor

Hi @burck1, thank you for all of your work on this service. The breakdown for your initial PR, data.aws_sso_instance, data.aws_sso_permission_set, and aws_sso_permission_set looks good, though you could also submit data.aws_sso_permission_set as a separate PR, so that the initial PR is even smaller.

@seddarj
Copy link

seddarj commented Nov 3, 2020

Hello, I'm really interested by the support of AWS SSO in terraform. Do you have any updates on this PR? Any way we can help move this forward?

@burck1
Copy link
Contributor Author

burck1 commented Nov 3, 2020

Hello, I'm really interested by the support of AWS SSO in terraform. Do you have any updates on this PR? Any way we can help move this forward?

Hi @seddarj. We've completed most of the work for supporting AWS SSO in Terraform. This [WIP] PR encompasses all of that work. But, the contribution guide for this repo recommends submitting small pull requests with the minimum required resources, so we've submitted #15808 as our initial PR with just data.aws_sso_instance, data.aws_sso_permission_set, and aws_sso_permission_set. Once that's merged, we will submit PRs for all of the other resources and data sources since they depend on that initial PR.

To help us move forward, please just go give a thumbs up on #15808.

@seddarj
Copy link

seddarj commented Nov 3, 2020

Hi @burck1, thanks for the details, I had completely missed that PR. You have my thumbs up. Thanks for implementing all this!

@anGie44 anGie44 added new-data-source Introduces a new data source. new-resource Introduces a new resource. and removed waiting-response Maintainers are waiting on response from community or contributor. labels Jan 12, 2021
@anarsen
Copy link

anarsen commented Jan 12, 2021

Proposal for new PR title: r/aws_sso_assignment: New Resource

@anGie44
Copy link
Contributor

anGie44 commented Jan 12, 2021

Proposal for new PR title: r/aws_sso_assignment: New Resource

Good catch @anarsen -- going to rename to reflect the changes here 👍

@anGie44 anGie44 changed the title [WIP] r/aws_sso_permission_set: New Resource [WIP] r/ssoadmin_account_assignment: new resource; d/identitystore: new data sources Jan 12, 2021
@burck1
Copy link
Contributor Author

burck1 commented Jan 12, 2021

Hi @anGie44. Thank you for all of your work on moving this forward! We've been a bit preoccupied the past few weeks, but we have been paying attention to the updates.

Definitely feel free to update this PR as necessary, or split into separate PRs, or however you want to handle the changes. Please let us know if you are blocked on anything or have any questions.

Happy New Year!

@plynch-magnolia
Copy link

plynch-magnolia commented Jan 13, 2021

Was wondering how feasible it would be if in the "aws_ssoadmin_managed_policy_attachment" resource you could pass in a list of "managed_policy_arn" values to avoid creating a new resource for each attached policy?

@anGie44
Copy link
Contributor

anGie44 commented Jan 13, 2021

Was wondering how feasible it would be if in the "aws_ssoadmin_managed_policy_attachment" resource you could pass in a list of "managed_policy_arn" values to avoid creating a new resource for each attached policy?

Hi @plynch-magnolia 👋 since the resource represents the singular attachment, I don't believe we would generally add an argument that would represent the management of multiple attachments; however, that is something we can open up for more discussion/find an alternative if feasible. do you mind creating a new issue to follow-up on your feature request/question?

@anGie44 anGie44 marked this pull request as ready for review January 14, 2021 06:46
@anGie44 anGie44 requested a review from a team as a code owner January 14, 2021 06:46
@anGie44 anGie44 changed the title [WIP] r/ssoadmin_account_assignment: new resource; d/identitystore: new data sources r/ssoadmin_account_assignment: new resource; d/identitystore: new data sources Jan 14, 2021
@ghost ghost added the service/identitystore Issues and PRs that pertain to the identitystore service. label Jan 14, 2021
@anarsen
Copy link

anarsen commented Jan 14, 2021

I really appreciate the increased traction on integrating AWS SSO Admin APIs into the provider. I realize this comment notifies a bunch of people, but seriously... I love the work you're all doing to make this happen.

Thank you.

@ghost ghost added the service/ssoadmin Issues and PRs that pertain to the ssoadmin service. label Jan 14, 2021
@plynch-magnolia
Copy link

plynch-magnolia commented Jan 14, 2021

Was wondering how feasible it would be if in the "aws_ssoadmin_managed_policy_attachment" resource you could pass in a list of "managed_policy_arn" values to avoid creating a new resource for each attached policy?

Hi @plynch-magnolia 👋 since the resource represents the singular attachment, I don't believe we would generally add an argument that would represent the management of multiple attachments; however, that is something we can open up for more discussion/find an alternative if feasible. do you mind creating a new issue to follow-up on your feature request/question?

@anGie44 I have added new issue #17108 to support this request

@anGie44 anGie44 added this to the v3.24.0 milestone Jan 14, 2021
Copy link
Contributor

@anGie44 anGie44 left a comment

Choose a reason for hiding this comment

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

Thanks again for all your contributions @burck1 @lawdhavmercy -- really great work here! Happy New year 🎉

Output of acceptances tests:

--- PASS: TestAccAWSIdentityStoreUserDataSource_NonExistent (3.88s)
--- PASS: TestAccAWSIdentityStoreGroupDataSource_NonExistent (3.89s)
--- PASS: TestAccAWSIdentityStoreUserDataSource_UserName (13.79s)
--- PASS: TestAccAWSIdentityStoreUserDataSource_UserID (13.79s)
--- PASS: TestAccAWSIdentityStoreGroupDataSource_DisplayName (13.80s)
--- PASS: TestAccAWSIdentityStoreGroupDataSource_GroupID (13.99s)

--- PASS: TestAccAWSSSOAdminAccountAssignment_Disappears (29.17s)
--- PASS: TestAccAWSSSOAdminAccountAssignment_Basic_Group (31.40s)
--- PASS: TestAccAWSSSOAdminAccountAssignment_Basic_User (62.27s)

@anGie44 anGie44 merged commit 8800edb into hashicorp:master Jan 14, 2021
anGie44 added a commit that referenced this pull request Jan 14, 2021
@burck1 burck1 deleted the aws-sso branch January 14, 2021 23:01
@ghost
Copy link

ghost commented Jan 15, 2021

This has been released in version 3.24.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented Feb 14, 2021

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!

@ghost ghost locked as resolved and limited conversation to collaborators Feb 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. new-data-source Introduces a new data source. new-resource Introduces a new resource. provider Pertains to the provider itself, rather than any interaction with AWS. service/identitystore Issues and PRs that pertain to the identitystore service. service/ssoadmin Issues and PRs that pertain to the ssoadmin service. size/XXL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.