From acd8ef62e464463f40d3aeb072d5b0d6c4ae74e1 Mon Sep 17 00:00:00 2001 From: Loren Gordon Date: Mon, 22 May 2023 10:14:00 -0700 Subject: [PATCH 1/2] Uses *only* `permission_set_arn` when available Also removes the `depends_on` in the account-assignment module. The depends_on was causing resource cycles when adding new account assignments. --- main.tf | 26 ++++++++++++++++++++------ modules/account-assignment/main.tf | 10 ---------- tests/test-account-assignment/main.tf | 12 ++++++++---- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/main.tf b/main.tf index ee59f21..5544b2c 100644 --- a/main.tf +++ b/main.tf @@ -3,11 +3,11 @@ module "permission_sets" { for_each = { for permission_set in var.sso_admin.permission_sets : permission_set.name => permission_set } permission_set = merge( + each.value, { instance_arn = local.sso_instance_arn partition = local.partition }, - each.value ) } @@ -16,15 +16,29 @@ module "account_assignments" { for_each = { for account_assignment in var.sso_admin.account_assignments : account_assignment.name => account_assignment } account_assignment = merge( + each.value, { identity_store_id = local.identity_store_id instance_arn = local.sso_instance_arn - permission_set_arn = try( - module.permission_sets[each.value.permission_set_name].permission_set.arn, - null, - ) + + permission_set_arn = ( + try(each.value.permission_set_arn, null) != null || + contains( + var.sso_admin.permission_sets[*].name, + try(each.value.permission_set_name, null) + ) + ) ? module.permission_sets[each.value.permission_set_name].permission_set.arn : null + + # Null `permission_set_name` if `permission_set_arn` is provided or can + # be looked up from the permission-set module + permission_set_name = ( + try(each.value.permission_set_arn, null) != null || + contains( + var.sso_admin.permission_sets[*].name, + try(each.value.permission_set_name, null) + ) + ) ? null : each.value.permission_set_name }, - each.value ) } diff --git a/modules/account-assignment/main.tf b/modules/account-assignment/main.tf index b33fafc..18bae55 100644 --- a/modules/account-assignment/main.tf +++ b/modules/account-assignment/main.tf @@ -14,16 +14,6 @@ data "aws_ssoadmin_permission_set" "this" { name = var.account_assignment.permission_set_name instance_arn = local.sso_instance_arn - - depends_on = [ - # When the permission set is managed in the same tfstate, it will not exist - # during the plan phase until after the first apply. This depends_on forces - # terraform to wait to evaluate the inputs until after the permission set is - # created, even on the first apply, ensuring it will exist when this data source - # is evaluated. See `tests/test-account-assignment` for the setup that will - # trigger the error condition, if this is removed. - var.account_assignment - ] } data "aws_identitystore_group" "this" { diff --git a/tests/test-account-assignment/main.tf b/tests/test-account-assignment/main.tf index 64cee14..2a9122c 100644 --- a/tests/test-account-assignment/main.tf +++ b/tests/test-account-assignment/main.tf @@ -2,10 +2,14 @@ module "test_account_assignment" { source = "../../modules/account-assignment" account_assignment = { - principal_name = var.principal_name - principal_type = "GROUP" - permission_set_name = module.test_permission_set.permission_set.name - target_id = local.account_id + principal_name = var.principal_name + principal_type = "GROUP" + target_id = local.account_id + + # It is not supported to use `permission_set_name` in this scenario. When the + # permission set is created in the same config, the `permission_set_arn` must + # be used instead. If you use `permission_set_name`, you will get an error. + permission_set_arn = module.test_permission_set.permission_set.arn } } From 670fe6a679170fd629b2c5951bc7c168768440c0 Mon Sep 17 00:00:00 2001 From: Loren Gordon Date: Mon, 22 May 2023 10:17:20 -0700 Subject: [PATCH 2/2] Bumps version to 2.0.1 --- .bumpversion.cfg | 2 +- CHANGELOG.md | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/.bumpversion.cfg b/.bumpversion.cfg index b49e513..b3432d3 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 2.0.0 +current_version = 2.0.1 commit = True message = Bumps version to {new_version} tag = False diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f2b80d..d98d248 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,18 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +### [2.0.1](https://github.com/plus3it/terraform-aws-tardigrade-sso-admin/releases/tag/2.0.1) + +**Commit Delta**: n/a + +**Released**: 2023.05.22 + +**Summary**: + +* Uses *only* `permission_set_arn` when available to create the account assignment +* Fixes resource cycle when state already exists and new account assignments are + added to the list + ### 2.0.0 **Commit Delta**: n/a @@ -23,5 +35,5 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p **Summary**: -* Provides initial capability for managing AWS SSO Permission Sets and Account - Assignments +* Provides initial capability for managing AWS SSO Permission Sets and Account + Assignments