This changes the sso and configuration nodes so it uses Terraform's typeMap type rather than a list.
This PR can be merged into Develop and released since it introduces no new resources prior to review.
Benefits:
Since SSO and Configuration will only occur once per App resource, semantically it doesn't make sense to make it a list. So doing this means a user can refer to something in the SSO node like onelogin_apps.my_app.sso.client_id as opposed to onelogin_apps.my_app.sso[0].client_id. This was brought up in this issue
This also means we can totally drop tracking fields by omitting them from the TF plan file. That is, if we remove provider_arn subsequent terraform apply requests will ignore this field. It also means the importer will stop trying to import the field as "" if it doesn't exist in the App from the API
Drawbacks:
We can't dictate what fields a user brings to the configuration or sso nodes, meaning the API will have to err out if a user sends a wrong/unacceptable field. The API should gracefully handle these types of errors.
Terraform typeMap doesn't really work well with complex types. It also needs all map entries to be of type string so we had to do some manual conversion between int/string to get this to work in configuration
Why not use for everything?
I considered using this same approach to parameters, but because TF typeMap doesn't really support complex types, the actual advice is to do what we're currently doing. See this issue on HashiCorp's Repo
This changes the sso and configuration nodes so it uses Terraform's
typeMaptype rather than a list.This PR can be merged into Develop and released since it introduces no new resources prior to review.
Benefits:
Since SSO and Configuration will only occur once per App resource, semantically it doesn't make sense to make it a list. So doing this means a user can refer to something in the SSO node like
onelogin_apps.my_app.sso.client_idas opposed toonelogin_apps.my_app.sso[0].client_id. This was brought up in this issueThis also means we can totally drop tracking fields by omitting them from the TF plan file. That is, if we remove
provider_arnsubsequent terraform apply requests will ignore this field. It also means the importer will stop trying to import the field as""if it doesn't exist in the App from the APIDrawbacks:
We can't dictate what fields a user brings to the
configurationorssonodes, meaning the API will have to err out if a user sends a wrong/unacceptable field. The API should gracefully handle these types of errors.Terraform
typeMapdoesn't really work well with complex types. It also needs all map entries to be of typestringso we had to do some manual conversion between int/string to get this to work inconfigurationWhy not use for everything?
I considered using this same approach to parameters, but because TF
typeMapdoesn't really support complex types, the actual advice is to do what we're currently doing. See this issue on HashiCorp's Repo