Skip to content

Conversation

@russoz
Copy link
Collaborator

@russoz russoz commented Nov 13, 2025

SUMMARY

Parameter validation using Ansible constructs, plus a couple of Python idioms adjustments.

ISSUE TYPE
  • Refactoring Pull Request
COMPONENT NAME

snmp_facts

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added module module plugins plugin (any type) labels Nov 13, 2025
@ansibullbot

This comment was marked as resolved.

@ansibullbot ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Nov 13, 2025
@russoz russoz force-pushed the snmp-facts-validation branch from b0e5610 to 1e79961 Compare November 13, 2025 10:50
@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Nov 13, 2025
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-12 Automatically create a backport for the stable-12 branch labels Nov 13, 2025
@felixfontein felixfontein added breaking_change This PR contains a breaking change that MUST NOT be backported backport-12 Automatically create a backport for the stable-12 branch and removed backport-12 Automatically create a backport for the stable-12 branch breaking_change This PR contains a breaking change that MUST NOT be backported labels Nov 15, 2025
@russoz russoz force-pushed the snmp-facts-validation branch from 1e79961 to 269f021 Compare November 21, 2025 04:13
@russoz russoz force-pushed the snmp-facts-validation branch from 269f021 to 073ab41 Compare November 22, 2025 05:53
@russoz russoz force-pushed the snmp-facts-validation branch from 073ab41 to 537392f Compare November 22, 2025 08:25
required_if=[
("version", "v2", ["community"]),
("version", "v2c", ["community"]),
("version", "v3", ["username", "authkey", "level"]),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding level because if it is not passed:

  • in line 305 the validation of param privacy when level=authPriv is not enforced, but ...
  • ... in line 327, because it is a plain else, it creates a silent, undocumented default level=authPriv when level is not passed

This could lead to a blatantly wrong situation. I suppose this still configures a breaking change, but in this case it's one that should be performed, IMO.

@felixfontein felixfontein added backport-12 Automatically create a backport for the stable-12 branch and removed breaking_change This PR contains a breaking change that MUST NOT be backported labels Nov 23, 2025
@felixfontein
Copy link
Collaborator

I think this is OK for 12.x.y, but I wouldn't backport it to older versions.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Nov 23, 2025
@felixfontein felixfontein merged commit 7321ba4 into ansible-collections:main Nov 23, 2025
134 checks passed
@patchback
Copy link

patchback bot commented Nov 23, 2025

Backport to stable-12: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-12/7321ba49903de02e36f8a911f16077392d0fe193/pr-11148

Backported as #11196

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

@felixfontein
Copy link
Collaborator

@russoz thanks for your contribution!

patchback bot pushed a commit that referenced this pull request Nov 23, 2025
* snmp_facts: improvements

* require level if vesion=v3

(cherry picked from commit 7321ba4)
felixfontein pushed a commit that referenced this pull request Nov 23, 2025
…1196)

snmp_facts: improvements (#11148)

* snmp_facts: improvements

* require level if vesion=v3

(cherry picked from commit 7321ba4)

Co-authored-by: Alexei Znamensky <[email protected]>
@russoz russoz deleted the snmp-facts-validation branch November 23, 2025 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-12 Automatically create a backport for the stable-12 branch module module plugins plugin (any type)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants