chore(cloudflare): migrate customhostname to v5 #5891

AndrewCharlesHay · 2025-10-06T21:49:49Z

What does it do ?

Migrate customhostname to v5

Motivation

Closes #5540

More

Yes, this PR title follows Conventional Commits
Yes, I added unit tests
Yes, I updated end user documentation accordingly

k8s-ci-robot · 2025-10-06T21:49:57Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign szuecs for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

coveralls · 2025-10-06T21:57:47Z

Pull Request Test Coverage Report for Build 19845553576

Details

0 of 0 changed or added relevant lines in 0 files are covered.
99 unchanged lines in 2 files lost coverage.
Overall coverage decreased (-0.2%) to 78.501%

Files with Coverage Reduction	New Missed Lines	%
cloudflare/cloudflare_regional.go	15	92.06%
cloudflare/cloudflare.go	84	87.74%

Totals
Change from base Build 19831487154:	-0.2%
Covered Lines:	15942
Relevant Lines:	20308

💛 - Coveralls

docs/tutorials/cloudflare.md

mloiseleur · 2025-10-09T06:01:56Z

@vflaux @mrozentsvayg Any comment on this PR ? Do you think you can review it ?

go.mod

provider/cloudflare/cloudflare.go

vflaux · 2025-10-09T15:39:35Z

provider/cloudflare/cloudflare.go

+	if strings.Contains(errStr, "exceeded available rate limit retries") ||
+		strings.Contains(errStr, "rate limit") ||
+		strings.Contains(errStr, "429") {


I think we should handle this according to the doc here: https://github.com/cloudflare/cloudflare-go?tab=readme-ov-file#errors

var apierr *cloudflare.Error if errors.As(err, &apierr) { if apierr.StatusCode == http.StatusTooManyRequests { } }

This was implemented

so why do we have this code here?

Rate limit errors are not returned as ErrorTypeRateLimit cloudflare/cloudflare-go#4155 talks about the v0 version of the library which we don't use after merging your PR.

the first string.Contains is not needed if you have the second one.

Sorry forgot to commit it. It's gone now

provider/cloudflare/cloudflare.go

k8s-ci-robot · 2025-10-09T15:47:49Z

@vflaux: changing LGTM is restricted to collaborators

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

AndrewCharlesHay · 2025-10-10T17:11:58Z

How did broken tests get merged in?

vflaux · 2025-10-13T11:06:27Z

How did broken tests get merged in?

This should be fixed, see #5896.

AndrewCharlesHay · 2025-10-13T15:01:36Z

/retest

AndrewCharlesHay · 2025-10-13T19:55:19Z

@vflaux everything look good now?

vflaux

I've not review all the new tests yet.
Looks like IA generated, some clearly improve test coverage, others seems less relevant. 😄

provider/cloudflare/cloudflare.go

provider/cloudflare/cloudflare_test.go

provider/cloudflare/cloudflare.go

go.mod

AndrewCharlesHay · 2025-10-24T18:44:37Z

The tests just fail because of rate limits to coveralls

provider/cloudflare/cloudflare.go

szuecs · 2025-10-31T09:01:58Z

Did you @AndrewCharlesHay address comments by @vflaux ?

Cleaning up things that are not useful is always great for maintaining the software.

AndrewCharlesHay · 2025-11-04T23:03:46Z

@szuecs yep all updated

mloiseleur · 2025-11-08T11:46:15Z

@AndrewCharlesHay Would you please rebase ?

mloiseleur · 2025-11-11T15:30:38Z

@vflaux @szuecs Anything left on your side ?

Co-authored-by: Michel Loiseleur <[email protected]>

…Flare API - Change CustomHostnames method signature to return autoPager[CustomHostnameListResponse] instead of processed []CustomHostname for better API matching and testability - Keep listAllCustomHostnames helper function for callers that need processed results - Update mock implementation to return autoPager with realistic CloudFlare API behavior - All tests continue to pass with improved interface design Addresses PR feedback about method signatures being closer to CloudFlare API for better mocking.

- Handle CloudFlare v5 SDK errors according to official documentation - Use errors.As() to check for cloudflare.Error type - Convert 429 (rate limit) status codes to SoftError for retry logic - Add comprehensive test coverage for v5 error handling - Use wrapper types in tests to avoid CloudFlare SDK internal nil pointer issues Addresses PR comment on line 376 requesting proper v5 error handling.

Co-authored-by: vflaux <[email protected]>

…rkaround no longer needed, per reviewer feedback)

- Update convertCloudflareError to detect rate limit keywords in error messages - Check for 'rate limit' and '429' in error strings - Fixes failing rate limit error tests

Co-authored-by: vflaux <[email protected]>

…g test

… comments - Add comprehensive test coverage for buildCustomHostnameNewParams function - Improve comment explaining why string-based rate limit detection is needed with v5 SDK (SDK's retry logic can wrap errors hiding structured types) - Addresses reviewer feedback about test coverage and error handling rationale

mloiseleur

@vflaux Anything left on your side ?

ivankatliarchuk · 2025-12-09T07:57:20Z

Was this code tested against cloudflare? Change does look riskie without testing.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 6, 2025

k8s-ci-robot requested a review from ivankatliarchuk October 6, 2025 21:49

k8s-ci-robot added the provider Issues or PRs related to a provider label Oct 6, 2025

k8s-ci-robot requested a review from szuecs October 6, 2025 21:49

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 6, 2025

k8s-ci-robot added the docs label Oct 7, 2025

mloiseleur reviewed Oct 7, 2025

View reviewed changes

docs/tutorials/cloudflare.md Outdated Show resolved Hide resolved

AndrewCharlesHay changed the title ~~Andrewcharleshay/cloudflare migration v5~~ chore(cloudflare): migrate customhostname to v5 Oct 7, 2025

vflaux suggested changes Oct 9, 2025

View reviewed changes

vflaux mentioned this pull request Oct 14, 2025

REQUEST: New membership for @vflaux kubernetes/org#5925

Closed

11 tasks

vflaux suggested changes Oct 17, 2025

View reviewed changes

k8s-ci-robot assigned vflaux Oct 17, 2025

szuecs reviewed Oct 24, 2025

View reviewed changes

go.mod Outdated Show resolved Hide resolved

szuecs reviewed Oct 31, 2025

View reviewed changes

provider/cloudflare/cloudflare.go Outdated Show resolved Hide resolved

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 8, 2025

AndrewCharlesHay force-pushed the andrewcharleshay/cloudflare-migration-v5 branch from e0eb277 to bcad628 Compare November 10, 2025 15:43

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 10, 2025

AndrewCharlesHay and others added 20 commits November 13, 2025 19:16

docs: v0.20 update

c790795

Co-authored-by: Michel Loiseleur <[email protected]>

fix: address comments

a8e96b2

style(cloudflare): make go-lint

28ff9cd

test(cloudflare): add test for ssl params

ec546c2

chore: remove extra files

39fadcd

style: make lint

d2c22ce

docs: update comment

f41f302

Co-authored-by: vflaux <[email protected]>

fix: address comments

e641e5d

fix: remove dead code

0a822b3

fix: update test

5e08563

fix: update tests

d1d3357

fix: update tests

3b05158

fix: update test

8e8a960

refactor(cloudflare): remove redundant rate limit error checks (v0 wo…

562b091

…rkaround no longer needed, per reviewer feedback)

fix: handle rate limit errors in string format

f5f3165

- Update convertCloudflareError to detect rate limit keywords in error messages - Check for 'rate limit' and '429' in error strings - Fixes failing rate limit error tests

chore: remove extra comment

8a3c8b1

Co-authored-by: vflaux <[email protected]>

remove unused test

a976b56

Co-authored-by: vflaux <[email protected]>

WIP: Remove coverage file, update error handling and tests per review

64f8d3f

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 14, 2025

AndrewCharlesHay added 3 commits December 1, 2025 17:16

fix: remove orphaned test code and fix test syntax errors

1538574

fix: resolve test compilation errors and skip flaky error log countin…

c8671be

…g test

chore: apply gofmt formatting

46c5795

AndrewCharlesHay force-pushed the andrewcharleshay/cloudflare-migration-v5 branch from 2bd7e13 to 46c5795 Compare December 2, 2025 02:14

Merge upstream master and resolve go.mod conflicts

47f7fbc

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 2, 2025

mloiseleur reviewed Dec 7, 2025

View reviewed changes

chore(cloudflare): migrate customhostname to v5 #5891

Are you sure you want to change the base?

chore(cloudflare): migrate customhostname to v5 #5891

Conversation

AndrewCharlesHay commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does it do ?

Motivation

More

Uh oh!

k8s-ci-robot commented Oct 6, 2025

Uh oh!

coveralls commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 19845553576

Details

💛 - Coveralls

Uh oh!

Uh oh!

mloiseleur commented Oct 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vflaux Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

AndrewCharlesHay Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

szuecs Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndrewCharlesHay Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

k8s-ci-robot commented Oct 9, 2025

Uh oh!

AndrewCharlesHay commented Oct 10, 2025

Uh oh!

vflaux commented Oct 13, 2025

Uh oh!

AndrewCharlesHay commented Oct 13, 2025

Uh oh!

AndrewCharlesHay commented Oct 13, 2025

Uh oh!

vflaux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AndrewCharlesHay commented Oct 24, 2025

Uh oh!

Uh oh!

szuecs commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndrewCharlesHay commented Nov 4, 2025

Uh oh!

mloiseleur commented Nov 8, 2025

Uh oh!

mloiseleur commented Nov 11, 2025

Uh oh!

mloiseleur left a comment

Choose a reason for hiding this comment

Uh oh!

ivankatliarchuk commented Dec 9, 2025

Uh oh!

Reviewers

Assignees

Labels

AndrewCharlesHay commented Oct 6, 2025 •

edited

Loading

coveralls commented Oct 6, 2025 •

edited

Loading

szuecs Oct 24, 2025 •

edited

Loading

szuecs commented Oct 31, 2025 •

edited

Loading