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

cloudflare_dns: Update SRV record handling for Cloudflare API changes #8948

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

saltydk
Copy link
Contributor

@saltydk saltydk commented Sep 30, 2024

SUMMARY

Brings SRV API up to date with the changes from here: https://developers.cloudflare.com/fundamentals/api/reference/deprecations/#2024-05-31

Fixes #8679

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

cloudflare_dns

ADDITIONAL INFORMATION
- name: Create an SRV record _foo._tcp.example.net
  community.general.cloudflare_dns:
    domain: example.net
    service: foo
    proto: tcp
    port: 3500
    priority: 10
    weight: 20
    type: SRV
    value: fooserver.example.net

@ansibullbot

This comment was marked as outdated.

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added bug This issue/PR relates to a bug ci_verified Push fixes to PR branch to re-run CI module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR new_contributor Help guide this first time contributor plugins plugin (any type) labels Sep 30, 2024
@ansibullbot ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Sep 30, 2024
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-8 Automatically create a backport for the stable-8 branch backport-9 Automatically create a backport for the stable-9 branch labels Oct 1, 2024
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! I've added some first comments below.

plugins/modules/cloudflare_dns.py Outdated Show resolved Hide resolved
changelogs/fragments/8679-fix-cloudflare-srv.yml Outdated Show resolved Hide resolved
plugins/modules/cloudflare_dns.py Outdated Show resolved Hide resolved
@saltydk
Copy link
Contributor Author

saltydk commented Oct 1, 2024

Done, although the lines I edited didn't contain that ending comma.

@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Oct 1, 2024
@ansibullbot ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Oct 1, 2024
@russoz
Copy link
Collaborator

russoz commented Oct 2, 2024

Hi @saltydk , thanks for the contribution! :-)

I am not familiar with Cloudflare's policies/procedures regarding APIs, so I saw the link to the deprecation notice in their website but it is not clear to me whether the deprecated API remains available for longer (if so then how long for) and, most importantly, whether the users have any choice in accessing one or the other version of the API.

I am trying to ensure that we're not going to have users hitting the old version of the API (while it lasts) and getting errors because the payload is formatted differently now. Conversely, if there is no versioning of the API endpoints and that payload structure change is already in place (it appears to be the case from the deprecation date in May), then the next question is: do those endpoints accept the payload in any of the two structures? Because if they only accept the new structure, then we have to merge this yesterday.

Just trying to make sure we're covering all the angles here. Thanks once again

@saltydk
Copy link
Contributor Author

saltydk commented Oct 2, 2024

The deprecation date was May 31, 2024, this change has been live before my original issue, I have a custom Ansible module in my own project with this fix and have been using it for months now. The version of the API Ansible hits is the v4 which has this change live.

cf_api_endpoint = 'https://api.cloudflare.com/client/v4'

@felixfontein
Copy link
Collaborator

That sounds like Cloudflare means "removal date" when saying "deprecation date". Deprecation usually means that you shouldn't use it anymore, but you still can. Here it seems you cannot use it anymore.

@saltydk
Copy link
Contributor Author

saltydk commented Oct 2, 2024

That does appear to be what the initial description says:

Cloudflare occasionally makes updates to our APIs that result in behavior changes or deprecations. When this happens, we will communicate when the API will no longer be available and whether there will be a replacement.

@felixfontein
Copy link
Collaborator

Sounds good to me. If nobody objects, I'll merge this next week.

Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

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

lgtm

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Oct 7, 2024
@felixfontein felixfontein merged commit 1bdf8fc into ansible-collections:main Oct 7, 2024
141 checks passed
Copy link

patchback bot commented Oct 7, 2024

Backport to stable-8: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-8/1bdf8fc02545acd73faf988088ea0f187f3f909f/pr-8948

Backported as #9000

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Oct 7, 2024
@felixfontein
Copy link
Collaborator

@saltydk thanks for your contribution!
@russoz thanks for reviewing!

Copy link

patchback bot commented Oct 7, 2024

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/1bdf8fc02545acd73faf988088ea0f187f3f909f/pr-8948

Backported as #9001

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Oct 7, 2024
felixfontein pushed a commit that referenced this pull request Oct 7, 2024
…ord handling for Cloudflare API changes (#9000)

cloudflare_dns: Update SRV record handling for Cloudflare API changes (#8948)

(cherry picked from commit 1bdf8fc)

Co-authored-by: salty <[email protected]>
felixfontein pushed a commit that referenced this pull request Oct 7, 2024
…ord handling for Cloudflare API changes (#9001)

cloudflare_dns: Update SRV record handling for Cloudflare API changes (#8948)

(cherry picked from commit 1bdf8fc)

Co-authored-by: salty <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8 Automatically create a backport for the stable-8 branch backport-9 Automatically create a backport for the stable-9 branch bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot add SRV records with cloudflare_dns
4 participants