cloudflare_filter import issue - zone not set #161
Comments
@patryk did mention in an earlier comment that there was a push to go one or the other (zone vs zone ID) however I'm not sure where that is at. I don't think it's worth doing by itself and we should probably wait on that decision as there will need to be shuffling of the internals to achieve this. It's not a make or break thing at the moment and the import processes documented to reflect these key differences.
This feels like a bug. Let me take a look at what we can do to handle setting a zone or zone ID and work out getting the other. |
|
Re: import - yes, we are moving away from using zone names directly, so I'd like to new code use IDs instead. For existing things, we cannot do it without breaking change, so we will retrofit with IDs support. Eventually (sometime 1H2019) we'll do a 2.0 release where we remove all legacy constructs. Re: empty field, yes, it's most definitely a bug. |
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.
|
I've just checked - ditto import for cloudflare_firewall_rule. |
|
@SteveGoldthorpe-Work I've just pushed up #162 which should aim to address this by keeping both in sync. Give it a whirl and let me know if you hit any snags. |
|
@SteveGoldthorpe-Work I'd recommend we open a new issue for the issue above so that we can track it individually and not have it close when this one is resolved. |
SteveGoldthorpe-Work
changed the title
|
I'm seeing similar behavior with a |
|
I suspect the problem exists for anything importing using a zone id rather than a zone name. I think they want a separate ticket for each resource for tracking purposes. |
* Allow `zone_id` to set `zone` and vice versa 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 #161.
During an import of the `cloudflare_firewall_rule` resource, the `zone_id` is defined within the composite ID. This value is then populated and synced within `resourceCloudflareFirewallRuleRead` 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 firewall rule and ends up attempting to recreate the resource due to detecting a change. This is a similar issue to cloudflare#161 and is addressed in a similar manner. Fixes cloudflare#165
During an import of the `cloudflare_access_rule` resource, the `zone_id` is defined within the composite ID. This value is then populated and synced within `resourceCloudflareAccessRuleRead` 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 access rule and ends up attempting to recreate the resource due to detecting a change. This is a similar issue to cloudflare#161 and is addressed in a similar manner. Fixes cloudflare#166
During an import of the `cloudflare_firewall_rule` resource, the `zone_id` is defined within the composite ID. This value is then populated and synced within `resourceCloudflareFirewallRuleRead` 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 firewall rule and ends up attempting to recreate the resource due to detecting a change. This is a similar issue to #161 and is addressed in a similar manner. Fixes #165
* Ensure `zone` and `zone_id` are always set During an import of the `cloudflare_access_rule` resource, the `zone_id` is defined within the composite ID. This value is then populated and synced within `resourceCloudflareAccessRuleRead` 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 access rule and ends up attempting to recreate the resource due to detecting a change. This is a similar issue to #161 and is addressed in a similar manner. Fixes #166
Zone names within Cloudflare are not universally unique which makes performing a lookup based on just the name a little troublesome. Two accounts can have the same zone name and both of these can be returned if the lookups are not restricted to the zone ID or account ID. To rememdy this, `zone` is being deprecated in favour of explicit `zone_id` values. See the discussion[1] on this for full details. Following Terraform's guidelines for deprecations[2], we are putting in a deprecation notice as part of a minor release and then in our next major release, we will actually perform the schema and functionality removal. This update will not prevent people from using `zone` however use will trigger a warning that it's going away and they should migrate themselves. [1]: https://github.com/terraform-providers/terraform-provider-cloudflare/issues/161#issuecomment-441128985 [2]: https://www.terraform.io/docs/extend/best-practices/deprecations.html#provider-attribute-removal
Zone names within Cloudflare are not universally unique which makes performing a lookup based on just the name a little troublesome. Two accounts can have the same zone name and both of these can be returned if the lookups are not restricted to the zone ID or account ID. To rememdy this, `zone` is being deprecated in favour of explicit `zone_id` values. See the discussion[1] on this for full details. Following Terraform's guidelines for deprecations[2], we are putting in a deprecation notice as part of a minor release and then in our next major release, we will actually perform the schema and functionality removal. This update will not prevent people from using `zone` however use will trigger a warning that it's going away and they should migrate themselves. [1]: https://github.com/terraform-providers/terraform-provider-cloudflare/issues/161#issuecomment-441128985 [2]: https://www.terraform.io/docs/extend/best-practices/deprecations.html#provider-attribute-removal
Zone names within Cloudflare are not universally unique which makes performing a lookup based on just the name a little troublesome. Two accounts can have the same zone name and both of these can be returned if the lookups are not restricted to the zone ID or account ID. To rememdy this, `zone` is being deprecated in favour of explicit `zone_id` values. See the discussion[1] on this for full details. Following Terraform's guidelines for deprecations[2], we are putting in a deprecation notice as part of a minor release and then in our next major release, we will actually perform the schema and functionality removal. This update will not prevent people from using `zone` however use will trigger a warning that it's going away and they should migrate themselves. [1]: https://github.com/terraform-providers/terraform-provider-cloudflare/issues/161#issuecomment-441128985 [2]: https://www.terraform.io/docs/extend/best-practices/deprecations.html#provider-attribute-removal
* add rate limiting definitions + functions with tests, examples * handle pagination in listing rate limits * add pointer values where empty values conflict with api defaults + cleanup review comments
terraform import for cloudflare_filter resource forces a new resource on next apply as zone is not added up on import.
zone: "" => "domain.name" (forces new resource)terraform import for cloudflare_filter resource requires a zone_id rather than zone unlike the other resources. We should be consistent.
$ terraform import cloudflare_filter.default d41d8cd98f00b204e9800998ecf8427e/9e107d9d372bb6826bd81d3542a419d6provider.cloudflare 1.9
The text was updated successfully, but these errors were encountered: