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

nsupdate: fix 'index out of range' error when no TTL answer is given #7219

Merged
merged 2 commits into from
Sep 10, 2023

Conversation

silkeh
Copy link
Contributor

@silkeh silkeh commented Sep 7, 2023

SUMMARY

Fix a possible list index out of range when no answer is returned in the ttl_changed method by applying the existing workaround for NS records to all record types.

Resolves #836

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

nsupdate

ADDITIONAL INFORMATION

See #836

Fix a possible `list index out of range` when no answer is returned in the `ttl_changed` method
by applying the existing workaround for NS records to all record types.

Resolves ansible-collections#836
@silkeh silkeh force-pushed the fix-dns-update-cname branch from 6f05078 to 7fe4621 Compare September 7, 2023 09:47
@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor plugins plugin (any type) labels Sep 7, 2023
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 a first comment.

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-6 labels Sep 7, 2023
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.

If nobody objects, I'll merge by the beginning of next week.

current_ttl = lookup.answer[0].ttl if lookup.answer else lookup.authority[0].ttl
else:
current_ttl = lookup.answer[0].ttl
current_ttl = lookup.answer[0].ttl if lookup.answer else lookup.authority[0].ttl
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know whether using the SOA TTL as a backup is a correct, but this should definitely help against the reported problems. The worst that can happen is that the module will always report changed instead of failing, which is already a big improvement.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 10, 2023
@felixfontein felixfontein merged commit 208df2c into ansible-collections:main Sep 10, 2023
@patchback
Copy link

patchback bot commented Sep 10, 2023

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/208df2c9e6b7e0c0f6b5a7d8543e135a068c581f/pr-7219

Backported as #7235

🤖 @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 Sep 10, 2023
…7219)

* nsupdate: fix 'index out of range' error when no TTL answer is given

Fix a possible `list index out of range` when no answer is returned in the `ttl_changed` method
by applying the existing workaround for NS records to all record types.

Resolves #836

* fixup! nsupdate: fix 'index out of range' error when no TTL answer is given

(cherry picked from commit 208df2c)
@patchback
Copy link

patchback bot commented Sep 10, 2023

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/208df2c9e6b7e0c0f6b5a7d8543e135a068c581f/pr-7219

Backported as #7236

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

@felixfontein
Copy link
Collaborator

@silkeh thanks for your contribution!

patchback bot pushed a commit that referenced this pull request Sep 10, 2023
…7219)

* nsupdate: fix 'index out of range' error when no TTL answer is given

Fix a possible `list index out of range` when no answer is returned in the `ttl_changed` method
by applying the existing workaround for NS records to all record types.

Resolves #836

* fixup! nsupdate: fix 'index out of range' error when no TTL answer is given

(cherry picked from commit 208df2c)
@silkeh
Copy link
Contributor Author

silkeh commented Sep 10, 2023

@felixfontein: thanks for the quick response! It is much appreciated.

felixfontein pushed a commit that referenced this pull request Sep 11, 2023
…nge' error when no TTL answer is given (#7235)

nsupdate: fix 'index out of range' error when no TTL answer is given (#7219)

* nsupdate: fix 'index out of range' error when no TTL answer is given

Fix a possible `list index out of range` when no answer is returned in the `ttl_changed` method
by applying the existing workaround for NS records to all record types.

Resolves #836

* fixup! nsupdate: fix 'index out of range' error when no TTL answer is given

(cherry picked from commit 208df2c)

Co-authored-by: Silke Hofstra <[email protected]>
felixfontein pushed a commit that referenced this pull request Sep 11, 2023
…nge' error when no TTL answer is given (#7236)

nsupdate: fix 'index out of range' error when no TTL answer is given (#7219)

* nsupdate: fix 'index out of range' error when no TTL answer is given

Fix a possible `list index out of range` when no answer is returned in the `ttl_changed` method
by applying the existing workaround for NS records to all record types.

Resolves #836

* fixup! nsupdate: fix 'index out of range' error when no TTL answer is given

(cherry picked from commit 208df2c)

Co-authored-by: Silke Hofstra <[email protected]>
etrombly pushed a commit to etrombly/community.general that referenced this pull request Oct 25, 2023
…nsible-collections#7219)

* nsupdate: fix 'index out of range' error when no TTL answer is given

Fix a possible `list index out of range` when no answer is returned in the `ttl_changed` method
by applying the existing workaround for NS records to all record types.

Resolves ansible-collections#836

* fixup! nsupdate: fix 'index out of range' error when no TTL answer is given
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

nsupdate-module fails If nothing has to be changed
3 participants