Skip to content

refactor(cloudflare): use lib v4 for proxied service #5702

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

refactor(cloudflare): use lib v4 for proxied service #5702

wants to merge 2 commits into from

Conversation

AndrewCharlesHay
Copy link
Contributor

@AndrewCharlesHay AndrewCharlesHay commented Jul 31, 2025 •
edited
Loading

What does it do ?

Updates the Cloudflare provider’s proxied functionality to v4, with related code and documentation updates. Similar to #5654 & #5609

Motivation

Addresses part of #5540

More

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

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. docs labels Jul 31, 2025
@k8s-ci-robot
Copy link
Contributor

[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 raffo 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:

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

@k8s-ci-robot k8s-ci-robot requested a review from mloiseleur July 31, 2025 17:00
@k8s-ci-robot k8s-ci-robot added provider Issues or PRs related to a provider size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 31, 2025
@AndrewCharlesHay AndrewCharlesHay changed the title Andrewhay/proxiedv4 refactor(cloudflare): use lib v4 for proxied service Jul 31, 2025
@ivankatliarchuk
Copy link
Contributor

Hi @vflaux wdyt to be a first reviewer?

@vflaux
Copy link
Contributor

vflaux commented Aug 2, 2025

Why is the distinction based on the proxied flag? I don't see much difference between a proxied record and a not proxied one.

vflaux
vflaux reviewed Aug 2, 2025
View reviewed changes
provider/cloudflare/cloudflare_proxied.go

switch cfc.ResourceRecord.Type {
case "A":
aRecord := dns.ARecordParam{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we need to use those specialized structs since they're all the same except for the type field, which is restricted to a single unique value. Using RecordUpdateParamsBody should work for all types.
I haven't tested this, but I think you still need to assign the type field when using the specialized struct, so there isn't much added value in using them.

@mrozentsvayg
Copy link
Contributor

cloudflare-go v5.0.0 has already been released, in this (early) state of PR it might make sense to switch;

also, is the v4 for custom hostnames in a roadmap or should i start looking into it?

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

PR needs rebase.

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.

@mloiseleur
Copy link
Collaborator

@AndrewCharlesHay Since PR switching to v5 has been merged, I close this one. Feel free to re-open it or open a new one if I missed something.

/close

@k8s-ci-robot
Copy link
Contributor

@mloiseleur: Closed this PR.

In response to this:

@AndrewCharlesHay Since PR switching to v5 has been merged, I close this one. Feel free to re-open it or open a new one if I missed something.

/close

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
Copy link
Contributor Author

Yeah that's fine. Sorry for the late response. I'm on vacation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ivankatliarchuk ivankatliarchuk Awaiting requested review from ivankatliarchuk

@mloiseleur mloiseleur Awaiting requested review from mloiseleur

1 more reviewer

@vflaux vflaux vflaux left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. docs needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. provider Issues or PRs related to a provider size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@AndrewCharlesHay @k8s-ci-robot @ivankatliarchuk @vflaux @mrozentsvayg @mloiseleur