Allow zone_id to set zone and vice versa
#162
Conversation
During an import of the `clouflare_filter` resource, the `zone_id` is defined within the composite ID. This value is then populated and synced within `resourceCloudflareFilterRead` to match the expected schema resource. However, we don't currently set `zone` (the zone name) anywhere. This becomes problematic on subsequent terraform operations as the `Read` attempts to sync the schema with the filter and ends up attempting to recreate the resource due to detecting a change. Initially I thought we could just iterate over all zones and pull out the one that matched the zone ID but this only works _if_ the zone ID has been provided (which isn't guaranteed since both fields are optional but one is required). Instead, I've opted to ensure we update both the `zone` and `zone_id` properties when we have one of them. I.e when we only have the `zone_id`, use that value to find and set the `zone` value (and vice versa). This will ensure that both values are defined and a change in either will retrigger the update. Fixes cloudflare#161.
ghost
added
the
| } | ||
| } | ||
|
|
||
| if zoneName == "" { |
There was a problem hiding this comment.
this is a safe guard to ensure that if we do get to this point, we really do have a zoneName set
There was a problem hiding this comment.
We should not need to check for zoneID being empty here, will be always populated by Create or Import.
We should also mark zone field as Computed: true.
There was a problem hiding this comment.
OK, that causes a bit of confusion here because zone_id is already marked as computed in the schema.
Should it only be zone that is computed? If that's the case, we won't need majority of this PR as we'll assume we always have the zone_id and can work everything around that.
There was a problem hiding this comment.
Computed here means that the code may set it itself, so it should not be taken into consideration on diff unless explicitly set (yes, Computed fields can be set in HCL).
Because now in Create we set zone_id if we provide zone, but we will be setting zone when we import with zone_id only, we need both to be Computed.
I made a mistake coding in the zone field, I was young and stupid at that time ;)
We really need to clean zone vs zone_id mess in v2.0 😰
There was a problem hiding this comment.
Computedhere means that the code may set it itself, so it should not be taken into consideration on diff unless explicitly set (yes,Computedfields can be set in HCL).
TIL, other uses of this that I have seen have all been for things like UUIDs or generating values at runtime.
We really need to clean
zonevszone_idmess in v2.0 😰
Do you want to start a v2.0 issue where we can keep track of these things so that we have a record of it?
There was a problem hiding this comment.
Yup, good idea. Issue and/or milestone (will need to read GitHub guides on this).
| } | ||
| } | ||
|
|
||
| if zoneName == "" { |
There was a problem hiding this comment.
We should not need to check for zoneID being empty here, will be always populated by Create or Import.
We should also mark zone field as Computed: true.
* Added CheckRegions field to LoadBalancerPool * Fixed TestCreateLoadBalancerPool to wirk with check_regions * Fixed TestDeleteLoadBalancerPool to wirk with check_regions * Added documentation for the CheckRegions field in LoadBalancerPool
During an import of the
clouflare_filterresource, thezone_idisdefined within the composite ID. This value is then populated and synced
within
resourceCloudflareFilterReadto match the expected schemaresource. However, we don't currently set
zone(the zone name)anywhere. This becomes problematic on subsequent terraform operations as
the
Readattempts to sync the schema with the filter and ends upattempting to recreate the resource due to detecting a change.
Initially I thought we could just iterate over all zones and pull out
the one that matched the zone ID but this only works if the zone ID
has been provided (which isn't guaranteed since both fields are optional
but one is required). Instead, I've opted to ensure we update both the
zoneandzone_idproperties when we have one of them. I.e when weonly have the
zone_id, use that value to find and set thezonevalue(and vice versa). This will ensure that both values are defined and a
change in either will retrigger the update.
Fixes #161.