terraform-aws-modules / terraform-aws-iam

Terraform module to create AWS IAM resources 🇺🇦
https://registry.terraform.io/modules/terraform-aws-modules/iam/aws
Apache License 2.0
787 stars 996 forks source link

docs: Remove the invalid `provider` argument from the `oidc_providers` variable description #452

Closed AndrewCharlesHay closed 9 months ago

AndrewCharlesHay commented 9 months ago

Description

The provider attribute is never accessed and thus shouldn't be listed. It also changes the type of oidc_providers from any to a map(any) because it is a map.

Motivation and Context

It clutters the requirements for the variable and thus should be removed

Breaking Changes

If someone used to pass something into oidc_providers it will now throw an error at an earlier stage of validation

How Has This Been Tested?

AndrewCharlesHay commented 9 months ago

I didn't mean to commit the SID addition to this PR. I can make a new PR if you want.

AndrewCharlesHay commented 9 months ago

@bryantbiggs Thank you for the merge! I understand if you want them in a separate PR. But what is wrong with changing the type to map(any) instead of just any?

bryantbiggs commented 9 months ago

when you go with a type, other than any, you are forcing users into using the same shape on every input. so for example, if you had something like the following that was acceptable for use by the underlying resource, if you used any it will work, if you used map(any), it will complain that the objects are of different types (one has colors and the other does not):

  foo = {
    one = {
      hot = true
      cups = 2
      colors = ["orange", "blue"]
    }
    two = {
      hot = false
      cups = 1
    }
  }

So we typically just default to any for complex types so that users have the most flexibility and can provide only the values they wish

AndrewCharlesHay commented 9 months ago

Ohh yeah I've run into that before. But we would want to ensure that every object has just provider_arn and namespace_service_accounts right?

AndrewCharlesHay commented 9 months ago

Both attributes are referenced in each iteration so we need both and wouldn't want anything else right? I guess it would somewhat unnecessarily make things not backwards compatible.

antonbabenko commented 9 months ago

This PR is included in version 5.33.1 :tada:

github-actions[bot] commented 8 months ago

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.