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.

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

nsupdate: issues/4657 #5377

Merged
merged 7 commits into from Nov 2, 2022
Merged

nsupdate: issues/4657 #5377

merged 7 commits into from Nov 2, 2022

Conversation

Simon-TheUser
Copy link
Contributor

SUMMARY

Fixes #4657

The "modify_record" method of the nsupdate module uses a delete-first approach to DNS updates. This prevents any updates to the NS records against Bind9 DNS servers.

This MR changes this behavior where updates to NS records will insert and update existing records, then perform the deletes.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

plugins/modules/net_tools/nsupdate.py

ADDITIONAL INFORMATION

I have been using this patch in my infrastructure for 2 months without any side issues.

@ansibullbot

This comment was marked as outdated.

Sign in to view
@ansibullbot ansibullbot added bug This issue/PR relates to a bug merge_commit This PR contains at least one merge commit. Please resolve! module module needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html new_contributor Help guide this first time contributor labels Oct 17, 2022
@ansibullbot

This comment was marked as outdated.

Sign in to view
@ansibullbot ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Oct 17, 2022
@ansibullbot

This comment was marked as outdated.

Sign in to view
@felixfontein felixfontein changed the title Issues/4657 nsupdate: issues/4657 Oct 17, 2022
@felixfontein
Copy link
Collaborator

Thanks for your contribution! Could you please add a changelog fragment? Thanks.

@ansibullbot

This comment was marked as outdated.

Sign in to view
@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI and removed ci_verified Push fixes to PR branch to re-run CI labels Oct 18, 2022
@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Oct 18, 2022
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-4 labels Oct 21, 2022
@felixfontein
Copy link
Collaborator

Please note that the issues in #5377 (comment) are still waiting to be resolved.

@Simon-TheUser
Copy link
Contributor Author

I hope I did the rebase correctly.

@ansibullbot
Copy link
Collaborator

cc @nerzhul
click here for bot help

@ansibullbot ansibullbot added net_tools plugins plugin (any type) and removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Oct 25, 2022
@felixfontein
Copy link
Collaborator

@Simon-TheUser looks good, thank you! Now also the bot informed the module maintainer.

@felixfontein
Copy link
Collaborator

If nobody complains, I'll merge this by the end of this week.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Nov 2, 2022
@felixfontein
Copy link
Collaborator

Sorry, defining now as the end of week ;)

@felixfontein felixfontein merged commit 5cb9a9e into ansible-collections:main Nov 2, 2022
@patchback
Copy link

patchback bot commented Nov 2, 2022 •
edited

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/5cb9a9e4f0ff68670e5aa285263c01227b03d8af/pr-5377

Backported as #5460

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

@felixfontein
Copy link
Collaborator

@Simon-TheUser thanks for your contribution!

patchback bot pushed a commit that referenced this pull request Nov 2, 2022
@Simon-TheUser @patchback
nsupdate: issues/4657 (#5377)
f53eb6c 
* Insert new entries before deleting old ones.
resolves #4657

* Slight wording changes.

* lint fix

* Address lint

* Added changelog
Fixed lint

* More linting

* Update changelogs/fragments/5377-nsupdate-ns-records-with-bind.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 5cb9a9e)
@patchback patchback bot mentioned this pull request Nov 2, 2022
Merged
@github-actions
Copy link

github-actions bot commented Nov 2, 2022

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

felixfontein pushed a commit that referenced this pull request Nov 2, 2022
@patchback @Simon-TheUser
nsupdate: issues/4657 (#5377) (#5460)
b871963 
* Insert new entries before deleting old ones.
resolves #4657

* Slight wording changes.

* lint fix

* Address lint

* Added changelog
Fixed lint

* More linting

* Update changelogs/fragments/5377-nsupdate-ns-records-with-bind.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 5cb9a9e)

Co-authored-by: Simon-TheUser <35318753+Simon-TheUser@users.noreply.github.com>
rekup pushed a commit to rekup/community.general that referenced this pull request Nov 3, 2022
@Simon-TheUser @felixfontein @rekup
nsupdate: issues/4657 (ansible-collections#5377)
0d4a332 
* Insert new entries before deleting old ones.
resolves ansible-collections#4657

* Slight wording changes.

* lint fix

* Address lint

* Added changelog
Fixed lint

* More linting

* Update changelogs/fragments/5377-nsupdate-ns-records-with-bind.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
russoz pushed a commit to russoz-ansible/community.general that referenced this pull request Nov 5, 2022
@Simon-TheUser @felixfontein @russoz
nsupdate: issues/4657 (ansible-collections#5377)
24bebf4 
* Insert new entries before deleting old ones.
resolves ansible-collections#4657

* Slight wording changes.

* lint fix

* Address lint

* Added changelog
Fixed lint

* More linting

* Update changelogs/fragments/5377-nsupdate-ns-records-with-bind.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
bratwurzt pushed a commit to bratwurzt/community.general that referenced this pull request Nov 7, 2022
@Simon-TheUser @felixfontein
nsupdate: issues/4657 (ansible-collections#5377)
fdfa5d3 
* Insert new entries before deleting old ones.
resolves ansible-collections#4657

* Slight wording changes.

* lint fix

* Address lint

* Added changelog
Fixed lint

* More linting

* Update changelogs/fragments/5377-nsupdate-ns-records-with-bind.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
bratwurzt pushed a commit to bratwurzt/community.general that referenced this pull request Nov 7, 2022
@Simon-TheUser @felixfontein
nsupdate: issues/4657 (ansible-collections#5377)
435216c 
* Insert new entries before deleting old ones.
resolves ansible-collections#4657

* Slight wording changes.

* lint fix

* Address lint

* Added changelog
Fixed lint

* More linting

* Update changelogs/fragments/5377-nsupdate-ns-records-with-bind.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
russoz pushed a commit to russoz-ansible/community.general that referenced this pull request Nov 10, 2022
@Simon-TheUser @felixfontein @russoz
nsupdate: issues/4657 (ansible-collections#5377)
6e2d717 
* Insert new entries before deleting old ones.
resolves ansible-collections#4657

* Slight wording changes.

* lint fix

* Address lint

* Added changelog
Fixed lint

* More linting

* Update changelogs/fragments/5377-nsupdate-ns-records-with-bind.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@felixfontein felixfontein felixfontein left review comments

Assignees
No one assigned
Labels
bug This issue/PR relates to a bug module module net_tools new_contributor Help guide this first time contributor plugins plugin (any type)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot update "NS" types entries from Bind9 managed DNS zone
3 participants
@Simon-TheUser @ansibullbot @felixfontein