Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes destroy on cloudflare_authenticated_origin_pulls #4649

Closed
wants to merge 3 commits into from

Conversation

PhilipSkinner
Copy link

According to the documentation the enabled flag must be set to null (…or nil) in order for the association to be destroyed. Setting this to false keeps the association in a disabled state instead of destroying the resource.

Documentation on this API endpoint can be found here:

https://developers.cloudflare.com/api/operations/per-hostname-authenticated-origin-pull-enable-or-disable-a-hostname-for-client-authentication

Referencing issue raised on provider:

#4648

Copy link
Contributor

github-actions bot commented Nov 22, 2024

changelog detected ✅

…or nil)

in order for the association to be destroyed. Setting this to false keeps the
association in a disabled state instead of destroying the resource.

Documentation on this API endpoint can be found here:

https://developers.cloudflare.com/api/operations/per-hostname-authenticated-origin-pull-enable-or-disable-a-hostname-for-client-authentication

Referencing issue raised on provider:

cloudflare#4648
@jacobbednarz
Copy link
Member

thanks for the PR. can you please add test coverage for this change to confirm it actually does what we intend here?

@PhilipSkinner
Copy link
Author

will do, this is blocked by the cloudflare API client library not supporting this API endpoint correctly so I'll raise a PR to fix that first then come back to this once its been released

PhilipSkinner pushed a commit to PhilipSkinner/cloudflare-go that referenced this pull request Nov 27, 2024
the API docs state that in order to delete an instance the enabled
flag must be set to null.

API docs can be found here:

https://developers.cloudflare.com/api/operations/per-hostname-authenticated-origin-pull-enable-or-disable-a-hostname-for-client-authentication

This fix is required to correct an issue with the cloudflare terraform
provider which is currently failing to destroy these resources correctly.

This can lead to a hard lock of mtls certificates and origin pulls configured
on them as the cloudflare API contains validation bugs on the endpoint
documented on the URL above.

Issue raised on the terraform provider:

cloudflare/terraform-provider-cloudflare#4648

PR raised to fix this on the terraform provider:

cloudflare/terraform-provider-cloudflare#4649
PhilipSkinner pushed a commit to PhilipSkinner/cloudflare-go that referenced this pull request Nov 27, 2024
the API docs state that in order to delete an instance the enabled
flag must be set to null.

API docs can be found here:

https://developers.cloudflare.com/api/operations/per-hostname-authenticated-origin-pull-enable-or-disable-a-hostname-for-client-authentication

This fix is required to correct an issue with the cloudflare terraform
provider which is currently failing to destroy these resources correctly.

This can lead to a hard lock of mtls certificates and origin pulls configured
on them as the cloudflare API contains validation bugs on the endpoint
documented on the URL above.

Issue raised on the terraform provider:

cloudflare/terraform-provider-cloudflare#4648

PR raised to fix this on the terraform provider:

cloudflare/terraform-provider-cloudflare#4649
@PhilipSkinner
Copy link
Author

PR raised for fixing the API client: cloudflare/cloudflare-go#3680

Once this has been approved and released I'll update the terraform provider to use the fixed library.

Copy link
Contributor

github-actions bot commented Dec 4, 2024

Oops! It looks like no changelog entry is attached to this PR. Please include a release note as described in https://github.com/cloudflare/terraform-provider-cloudflare/blob/master/contributing/changelog-process.md.

Example:

```release-note:TYPE
Release note
```

If you do not require a release note to be included and you have permission, please add the workflow/skip-changelog-entry label. Otherwise, a maintainer will add the label or ask you for one when they review the PR.

@jacobbednarz
Copy link
Member

handled via #4661

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants