-
Notifications
You must be signed in to change notification settings - Fork 788
fix(authenticated_origin_pulls): handle array response and implement full lifecycle #6711
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
fix(authenticated_origin_pulls): handle array response and implement full lifecycle #6711
Conversation
internal/services/authenticated_origin_pulls_certificate/resource_test.go
Outdated
Show resolved
Hide resolved
Is the openapi definition correct? |
Yes, the OpenAPI spec in api-docs correctly defines the response as hostname_aop_response_collection with result: type: array |
…full lifecycle
SECENG-12970
The API endpoint PUT /zones/{zone_id}/origin_tls_client_auth/hostnames
returns an array of hostname associations, but the auto-generated code
expected a single object. This caused Create/Update operations to fail
silently.
Changes for authenticated_origin_pulls:
- Add custom.go with array envelope type and FindByHostname() helper
- Fix Create/Update methods to parse array response and find matching hostname
- Fix Delete method to include cert_id in payload (API requirement)
- Fix Read method to set PrivateKey to null (write-only field)
- Rewrite resource_test.go with comprehensive lifecycle test
- Remove outdated testdata files
Also includes cherry-picked fixes:
- Certificate normalization from PR cloudflare#6710 (authenticated_origin_pulls_certificate)
- Certificate normalization from PR cloudflare#6703 (authenticated_origin_pulls_hostname_certificate)
Test: PASS TestAccAuthenticatedOriginPulls_FullLifecycle (8.21s)
- Add config length validation requiring exactly one hostname association - Move authenticated_origin_pulls_certificate test config to testdata file - Add updated_at to ImportStateVerifyIgnore list
7e3aa5c to
62598d7
Compare
musa-cf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for providing a great description!
Summary
Fixes the
cloudflare_authenticated_origin_pullsresource which was completely broken due to API response format mismatch.Problem
Primary Issue: Array Response Not Handled
The API endpoint
PUT /zones/{zone_id}/origin_tls_client_auth/hostnamesreturns an array of all hostname associations for the zone, not just the single hostname being created/updated:{ "result": [ {"hostname": "example.com", "cert_id": "abc-123", "enabled": true, ...}, {"hostname": "api.example.com", "cert_id": "def-456", "enabled": true, ...} ], "success": true }The auto-generated provider code expected a single object:
{ "result": {"hostname": "example.com", "cert_id": "abc-123", ...} }This caused Create/Update operations to silently fail - the API call succeeded but the response couldn't be parsed, leaving the resource state empty or incorrect.
Secondary Issues
cert_idin the delete payload, but it wasn't being sentprivate_key- This is a write-only field (sent to API, never returned), causing Terraform to detect drift on every refreshSolution
1. Array Response Handling (
custom.go)Created custom types to properly handle the array response:
2. Fixed CRUD Operations (
resource.go)Create/Update:
Delete:
Read:
3. Comprehensive Lifecycle Test (
resource_test.go)Rewrote tests to cover the full lifecycle:
enabled=trueenabled=falseAPI Documentation Verification
Verified implementation against Cloudflare API docs:
AuthenticatedOriginPullsArrayResultEnvelopewithFindByHostname()AuthenticatedOriginPullsResultEnvelope(single object)enabled: nullto void associationenabled: nilin payloadcert_idrequired in requestprivate_keyis write-onlytypes.StringNull()after API callsFiles Changed
custom.goFindByHostname()helperresource.goresource_test.gotestdata/*.tfCherry-picked Dependencies
This PR includes cherry-picked certificate normalization fixes required for the test to pass (the test creates a certificate first, then associates it with a hostname):
authenticated_origin_pulls_certificate)authenticated_origin_pulls_hostname_certificate)Test Results
JIRA
SECENG-12970