Merge pull request #13 from lorengordon/fix/resource-cycle

.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

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

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
   )
 }
 

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" {

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
   }
 }