schema: Enable map value validation #12638
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed we expect the
Elem
for maps to beValueType
directly per #10203 which probably makes sense for primitive types, because wrapping the type inSchema{}
doesn't provide much benefits as most schema attributes don't make any sense in this context, e.g. Optional/Required/Computed - you cannot have value computed or not present as you'd have to know the key which means the whole pair has to be defined beforehand anyway and HCL has AFAIK no notion of a map with optional/no value.That said we do have some
Type: TypeMap, Elem: &Schema{Type: ValueType}
in the codebase, specifically:aws_kms_secret
:context
consul_agent_self
:advertise_addrs
consul_catalog_nodes
:tagged_addresses
consul_catalog_service
:tagged_addresses
consul_catalog_services
:services
external
:query
external
:result
aws_ssm_document
:permissions
chef_environment
:cookbook_constraints
circonus_check
:check_by_collector
consul_key_prefix
:subkeys
datadog_monitor
:thresholds
ns1_user
:notify
test_resource
:map_that_look_like_set
So all of these are well handled in this new validation, but we don't actually cast to right type at this point, so even if the type is right (passed validation), you'll get string values there anyway.
I'm happy to send a separate PR to tighten the internal validation too and modify the schema of the mentioned data sources and resources ^, but it will be obviously BC for any external/pluggable providers, so I'd like to hear some thoughts or at least 👍 / 👎 on this idea.