fix(testdata): unblock e2e fresh creates for webhooks and audit_logs logpush#316
Open
vaishakdinesh wants to merge 4 commits into
Open
fix(testdata): unblock e2e fresh creates for webhooks and audit_logs logpush#316vaishakdinesh wants to merge 4 commits into
vaishakdinesh wants to merge 4 commits into
Conversation
…logpush notification_policy_webhooks: switch from https://www.cloudflare.com/cdn-cgi/trace (GET-only; Cloudflare's webhook validator does POST and gets 4xx) to https://httpbin.cfdata.org/post, which accepts POST. Verified end-to-end by creating a webhook via the Notifications API directly. logpush_job (audit_logs dataset): replace ClientIP/EdgeStartTimestamp field references (which belong to the http_requests dataset) with valid audit_logs fields (ActorEmail, ActionType, When). Clears the "no valid fields for dataset audit_logs" (1004) errors from the Cloudflare Logs API. Both errors only surface when e2e tests create resources fresh (rather than refreshing existing state). The pre-existing v4-provider create-time checks never triggered while state was intact from prior successful runs, which is why these silently regressed. Integration tests pass for both modules.
The notification_policy module defines its own webhook resources for use as notification targets. Missed these in the previous commit — all three still pointed at cloudflare.com/cdn-cgi/trace and failed the Notifications API POST validation.
ssicard
approved these changes
Jul 1, 2026
The pinned target v5 provider was 5.19.0-beta.5 (April), which pre-dates: - v5.20.0: load_balancer_pool state upgrader fix for early-v5 object-shape state at schema_version=0. Fixes the 8 'Value Conversion Error … flatten_cname' failures on cloudflare_load_balancer_pool during v5 plan. - v5.21.0: 'add missing no-op state upgraders for version 0 -> 500', which includes handling for the ruleset asset_name attribute added to v4 in v4.52.8 (June). Fixes the 9 'Failed to unmarshal v4 ruleset state: unsupported attribute asset_name' failures on cloudflare_ruleset. With this bump, the remaining failures from the last e2e run should be resolved. v4 apply, migration, and v5 plan should now all pass end-to-end.
…t-branch workflow
Rationale — the previous setup ran e2e-tests.yml against the provider's
next branch, a moving target. Any breaking change on next (e.g. a state
upgrader schema going out of sync with a new v4 attribute) would silently
regress tf-migrate CI and block unrelated PRs. Diagnosis for a single
upstream schema drift took ~2 days of investigation last week.
This change:
- e2e-tests.yml: pin provider ref to v${TARGET_PROVIDER_VERSION} (5.21.1),
and reuse the same variable for --target-provider-version so the pin is
written in exactly one place. Bumping the pin is now an explicit, single-
line PR that carries the compatibility check.
- e2e-tests-next.yml: new workflow_dispatch-only workflow that builds
and tests against the provider's next branch (or any user-supplied ref/
SHA). Advisory only — failures do not block main PRs but do surface
regressions before they land in a stable release.
The two workflows share the same concurrency group so they can't overlap
on the shared Cloudflare test account and R2 state.
Note: pinning to v5.21.1 relies on the provider PR that adds asset_name
handling to the ruleset state upgrader and removes flatten_cname from
load_balancer_pool.migration.v500.SourceOriginsModel. That fix is not yet
in v5.21.1, so the next scheduled provider release (with those fixes)
should be the new pin. Until then, either land those fixes as part of a
v5.21.2 patch, or temporarily set the pin to a commit that includes them.
Member
Author
|
Added workflow split (commit e4327d1):
Same concurrency group so they can't clash on the shared test account. Important compatibility note
Two ways forward once PR #7236 is merged upstream:
Either way the merge order matters: land provider PR #7236 first, then merge this tf-migrate PR (with the appropriate pin), so main CI is green from the moment this ships. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
The e2e workflow has been failing since ~May 12 because prior "green" runs were
no-op refreshes against pre-existing state. When state was destroyed and CI
started attempting fresh creates, two long-latent testdata bugs surfaced:
1. notification_policy_webhooks (22 errors per run)
Testdata used
https://www.cloudflare.com/cdn-cgi/traceas the webhook URL.That endpoint accepts
GET(returns 200) but Cloudflare's Notifications APIdoes a POST to validate webhooks on create — and gets 4xx back. All
terraform applycreate calls fail with:Verified directly against the Notifications API:
Switching to
https://httpbin.cfdata.org/postsucceeds:2. logpush_job with dataset
audit_logs(2 errors per run)The testdata's audit_logs jobs used
ClientIP/EdgeStartTimestampinfield_namesandlogpull_options. Those arehttp_requestsdataset fields,not
audit_logs. The API rejects the create:The current audit_logs schema fields (from the datasets API) are:
ActionResult, ActionType, ActorEmail, ActorID, ActorIP, ActorType, ID, Interface, Metadata, NewValue, OldValue, OwnerID, ResourceID, ResourceType, When.Testdata updated to use
ActorEmail, ActionType, When.Why this only started failing now
The pre-existing v4-provider create-time checks never fired while state was
intact from earlier successful applies — every subsequent run was a no-op
refresh. Once state and reality diverged (out-of-band deletions, state wipes),
CI had to recreate from scratch and immediately hit these two latent issues.
Verification
go test ./integration/... -run TestV4ToV5Migration/notification_policy_webhooks— passgo test ./integration/... -run TestV4ToV5Migration/logpush_job— passgo test -race ./integration/...(full suite) — passGET /accounts/<acct>/logpush/datasets/audit_logs/fieldsNot in this PR
Provider gets non-JSON when creating a Custom Hostname cert. Needs separate
investigation; may be a v4 provider bug or flaky Custom Hostnames endpoint.