feat: Implementation of the create_custom_role_trust_policy flag to control custom_role_trust_policy creation

[schniber]

Description

Hello,

This PR objective is to fix #320 (that got closed because of being stale) and an attempt to solve PR #321 as initially raised by @egarbi

Relying on the var.custom_role_trust_policy = "" in the iam_assume_role module under specific circumstances makes terraform plan error our as below:

╷
│ Error: Invalid count argument
│ 
│   on ../../modules/iam-assumable-role/main.tf line 12, in data "aws_iam_policy_document" "assume_role":
│   12:   count = var.custom_role_trust_policy == "" && var.role_requires_mfa ? 0 : 1
│ 
│ The "count" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created. To work around this, use the
│ -target argument to first apply only the resources that the count depends on.
╵
╷
│ Error: Invalid count argument
│ 
│   on ../../modules/iam-assumable-role/main.tf line 62, in data "aws_iam_policy_document" "assume_role_with_mfa":
│   62:   count = var.custom_role_trust_policy == "" && var.role_requires_mfa ? 1 : 0
│ 
│ The "count" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created. To work around this, use the
│ -target argument to first apply only the resources that the count depends on.

An example of code which makes the module error out is as below:

# #########################################
# # IAM assumable role with custom trust policy 2
# #########################################

resource "aws_iam_saml_provider" "idp_saml" {
  name                   = "idp_saml"
  saml_metadata_document = file("saml-metadata.xml")
}

data "aws_iam_policy_document" "custom_trust_policy_2" {
  statement {
    effect  = "Allow"
    actions = ["sts:AssumeRoleWithSAML"]

    principals {
      type        = "Federated"
      identifiers = [
        aws_iam_saml_provider.idp_saml.id
      ]
    }
    condition {
      test     = "StringEquals"
      variable = "SAML:aud"
      values   = [
        "https://signin.aws.amazon.com/saml"
      ]
    }
  }
  statement {
    effect  = "Allow"
    actions = ["sts:TagSession"]

    principals {
      type        = "Federated"
      identifiers = [
        aws_iam_saml_provider.idp_saml.id
      ]
    }
    condition {
      test     = "StringLike"
      variable = "aws:RequestTag/groups"
      values   = ["*"]
    }
  }
}

module "iam_assumable_role_custom_trust_policy_2" {
  source = "../../modules/iam-assumable-role"

  create_role = true

  role_name = "iam_assumable_role_custom_trust_policy_2"

  custom_role_trust_policy = data.aws_iam_policy_document.custom_trust_policy_2.json
  custom_role_policy_arns  = ["arn:aws:iam::aws:policy/AmazonCognitoReadOnly"]
}

Motivation and Context

As of today, the implementation does not allow the use of custom_role_trust_policies in all cases.

Breaking Changes

Nope this feature is not breaking changes since the new bool flag <create_custom_role_trust_policy> is defaulting to false.

How Has This Been Tested?

• I have updated at least one of the examples/* to demonstrate and validate my change(s)
• I have tested and validated these changes using one or more of the provided examples/* projects

• I have executed pre-commit run -a on my pull request

[schniber]

Hello @antonbabenko and @bryantbiggs,
Would it be possible to provide me with a peer review ?

Thanks in advance.

Bests !

[schniber]

Update fork to keep it in sync with upstream.
Thanks a lot for your valuable support.

[schniber]

Update fork to keep it in sync with upstream.
Thanks a lot for your valuable support @bryantbiggs @antonbabenko

[schniber]

Synced Forked with Upstream repository.

[davepattie]

Thanks for your efforts on this PR. I am also hitting this same problem and would benefit from seeing this get merged, cheers

[schniber]

hello @bryantbiggs,

Any luck for the review of this PR ?

Thanks a lot for your valuable support.

Bests.

[github-actions]

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

[schniber]

Hello @antonbabenko
Any luck with the peer review of this PR ? It's solving an issue we face witg the current implementation.

Thanks a lot for your valuable help.

[egarbi]

@schniber my original PR was merged and this is no longer needed. Please check #321

[schniber]

@egarbi : Yes I saw that, but it looks like your PR only fixes this for the submodule iam assumable role.
in this PR, we apply the same logic to other modules where IAM Roles are created to allow users to maintain their own trust policies if need be.

Issue is that I see that some of the changes that @bryantbiggs recommended are conflicting with the merge on #321 as I deleted the variable "create_custom_trust_policy" in this iteration in favor of the variable "assume_role_policy".

Hopefully this helps converge to a single solution for all.

Bests.

[bryantbiggs]

this is a breaking change so we won't be making that here. a lot of these changes and nuanced differences will be resolved at the next breaking change (ref https://github.com/clowdhaus/terraform-aws-iam)

[schniber]

So, I reverted back my changes to remove the breaking chages.
Thanks.

[github-actions]

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

[schniber]

Commenting this to make sure it does not become stale!

Thanks.

[github-actions]

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

[github-actions]

This PR was automatically closed because of stale in 10 days

[github-actions]

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.