feat(anta): Added the test case to verify SNMP groups #886

vitthalmagadum · 2024-10-16T18:19:29Z

Description

Verifies the SNMP group configurations for specified version(s).

- Verifies that the valid group name and security model version.
- Ensures that the SNMP views, the read, write and notify settings aligning with version-specific requirements.

Expected Results
----------------
* Success: The test will pass if the provided SNMP group and all specified parameters are correctly configured.
* Failure: The test will fail if the provided SNMP group is not configured or specified parameters are not correctly configured.

Fixes #853

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my code
I have run pre-commit for code linting and typing (pre-commit run)
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes (tox -e testenv)

for more information, see https://pre-commit.ci

github-actions · 2024-12-02T20:54:19Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions · 2025-01-16T01:26:13Z

Conflicts have been resolved. A maintainer will review the pull request shortly.

codspeed-hq · 2025-01-16T10:49:54Z

CodSpeed Performance Report

Merging #886 will not alter performance

_{Comparing vitthalmagadum:issue_853 (8d3d1ab) with main (995943a)}

Summary

✅ 22 untouched benchmarks

carl-baillargeon · 2025-01-20T23:36:52Z

anta/input_models/snmp.py

+    """Model for a SNMP group."""
+
+    group_name: str
+    """SNMP group for the user."""


Suggested change

"""SNMP group for the user."""

"""SNMP group name."""

carl-baillargeon · 2025-01-20T23:44:51Z

anta/input_models/snmp.py

+    authentication: Literal["v3Auth", "v3Priv", "v3NoAuth"] | None = None
+    """Advanced authentication in v3 SNMP version. Defaults to None.
+    - v3Auth: Group using authentication but not privacy
+    - v3Priv: Group using both authentication and privacy
+    - v3NoAuth: Group using neither authentication nor privacy


This is not being rendered properly in the doc:

I know we provide the default everywhere, e.g., "Defaults to None" but it's not necessary since we have a Default column in our documentation.

Also, since this field is required with SNMPv3, we should update the docstring with something like "SNMPv3 authentication settings. Required when version = v3"

Hi, Carl, I agree with your point. Since we already have a default column, I've removed the string 'Defaults to None.'

As for the content rendering in the document, I've made several attempts to modify it, but it’s still not rendering as expected. Therefore, for the time being, I’ve removed the informative strings from the description to avoid confusion. Please share your thoughts/suggestion on this approach.

Thanks
Geetanjali

carl-baillargeon · 2025-01-20T23:47:42Z

anta/tests/snmp.py

+
+      1. Verifies that the SNMP group is configured for the specified version.
+      2. For SNMP version 3, verify that the security model matches the expected value.
+      3. Ensures that the SNMP views, the read, write and notify settings aligning with version-specific requirements.


Suggested change

3. Ensures that the SNMP views, the read, write and notify settings aligning with version-specific requirements.

3. Ensures that SNMP group configurations, including read, write, and notify views, align with version-specific requirements.

carl-baillargeon · 2025-01-20T23:51:13Z

anta/tests/snmp.py

+            """Validate the inputs provided to the SnmpGroup class."""
+            for snmp_group in snmp_groups:
+                if snmp_group.version == "v3" and snmp_group.authentication is None:
+                    msg = f"{snmp_group}; advanced `authentication` is required."


Suggested change

msg = f"{snmp_group}; advanced `authentication` is required."

msg = f"{snmp_group}; `authentication` field is required for `version: v3`"

carl-baillargeon · 2025-01-21T00:05:15Z

anta/tests/snmp.py

+            # Verify SNMP views, the read, write and notify settings aligning with version-specific requirements.
+            if group.read_view and not all(
+                [(act_view := group_details.get("readView")) == group.read_view, (view_configured := group_details.get("readViewConfig"))]
+            ):
+                self.result.is_failure(f"{group}, ReadView: {group.read_view} - View configuration mismatch - ReadView: {act_view}, Configured: {view_configured}")
+
+            if group.write_view and not all(
+                [(act_view := group_details.get("writeView")) == group.write_view, (view_configured := group_details.get("writeViewConfig"))]
+            ):
+                self.result.is_failure(
+                    f"{group}, WriteView: {group.write_view} - View configuration mismatch - WriteView: {act_view}, Configured: {view_configured}"
+                )
+
+            if group.notify_view and not all(
+                [(act_view := group_details.get("notifyView")) == group.notify_view, (view_configured := group_details.get("notifyViewConfig"))]
+            ):
+                self.result.is_failure(
+                    f"{group}, NotifyView: {group.notify_view} - View configuration mismatch - NotifyView: {act_view}, Configured: {view_configured}"
+                )


You could simplify this logic by looping over read, write, notify. Something like:

for view_type in ["read", "write", "notify"]: expected_view = getattr(group, f"{view_type}_view") if expected_view and not all([ (act_view := group_details.get(f"{view_type}View")) == expected_view, (view_configured := group_details.get(f"{view_type}ViewConfig")) ]): self.result.is_failure( f"{group} {view_type.title()} View: {expected_view} - " f"View configuration mismatch - {view_type.title()} View: {act_view}, " f"Configured: {view_configured}" )

Also, since group_details.get() could return an empty string, we should handle it in the failure message to avoid having something empty. You should also add a unit test for an output with it:

{ "groups": { "ARISTA_SNMPV3_RO_GROUP": { "versions": { "v3": { "secModel": "v3Priv", "readView": "ARISTA_FULL_SNMP_TREE_VIEW", "readViewConfig": true, "writeView": "", # Empty string "notifyView": "" } } } } }

Yeah!..Thanks a lot for the suggestion, @carl-baillargeon! I've updated the test case with the new logic and added unit tests for the empty string check.

… issue

sonarqubecloud · 2025-01-21T10:36:53Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

gmuloc

@carl-baillargeon let us know if you disagree with splitting the error messages

gmuloc · 2025-01-21T22:00:56Z

anta/tests/snmp.py

+    Expected Results
+    ----------------
+    * Success: The test will pass if the provided SNMP group and all specified parameters are correctly configured.
+    * Failure: The test will fail if the provided SNMP group is not configured or specified parameters are not correctly configured.


Suggested change

* Failure: The test will fail if the provided SNMP group is not configured or specified parameters are not correctly configured.

* Failure: The test will fail if the provided SNMP group is not configured or if any specified parameter is not correctly configured.

gmuloc · 2025-01-21T22:01:20Z

anta/tests/snmp.py

+    anta.tests.snmp:
+      - VerifySnmpGroup:
+          snmp_groups:
+            - group_name: Group1


show other versions in example - especially with the security model for v3

gmuloc · 2025-01-21T22:01:56Z

anta/tests/snmp.py

+            """Validate the inputs provided to the SnmpGroup class."""
+            for snmp_group in snmp_groups:
+                if snmp_group.version == "v3" and snmp_group.authentication is None:
+                    msg = f"{snmp_group}; `authentication` field is required for `version: v3`"


Please don't use semi columns

Suggested change

msg = f"{snmp_group}; `authentication` field is required for `version: v3`"

msg = f"{snmp_group}: `authentication` field is required for `version: v3`"

gmuloc · 2025-01-21T22:06:27Z

anta/tests/snmp.py

+                elif expected_view and not all(
+                    [(act_view := group_details.get(f"{view_type}View")) == expected_view, (view_configured := group_details.get(f"{view_type}ViewConfig"))]
+                ):
+                    self.result.is_failure(
+                        f"{group} {view_type.title()} View: {expected_view} - "
+                        f"View configuration mismatch - {view_type.title()} View: {act_view}, "
+                        f"Configured: {view_configured}"
+                    )


I would rather make two error messages, 1 to indicate the view name is not correct (matcging the expecting one) and 2. to indicate that when the view name is matching, it is not properly configured (Configured: False)

issue_853 Added TC for SNMP groups

fea9674

github-actions bot assigned vitthalmagadum Oct 16, 2024

vitthalmagadum marked this pull request as draft October 16, 2024 18:21

vitthalmagadum and others added 2 commits November 29, 2024 13:49

Merge branch 'main' into issue_853

212147d

[pre-commit.ci] auto fixes from pre-commit.com hooks

cb1d3d7

for more information, see https://pre-commit.ci

github-actions bot added the state: conflict label Dec 2, 2024

geetanjalimanegslab assigned geetanjalimanegslab and unassigned vitthalmagadum Jan 15, 2025

geetanjalimanegslab force-pushed the issue_853 branch from 827047c to cb1d3d7 Compare January 16, 2025 01:17

Merge branch 'main' into issue_853

de48bd8

github-actions bot removed the state: conflict label Jan 16, 2025

Issue_853: Refactored testcase with input model changes

c383c57

geetanjalimanegslab force-pushed the issue_853 branch from 239ea7d to c383c57 Compare January 16, 2025 10:41

geetanjalimanegslab added 2 commits January 17, 2025 11:12

Issue_853: UUpdated docstrings,test failure message and optimized code

68a5acb

Issue_853: Updated test failure message

7886dbd

geetanjalimanegslab requested a review from carl-baillargeon January 17, 2025 11:29

geetanjalimanegslab marked this pull request as ready for review January 17, 2025 11:29

carl-baillargeon requested changes Jan 21, 2025

View reviewed changes

Merge branch 'main' into issue_853

819c92b

geetanjalimanegslab force-pushed the issue_853 branch from 6ceb261 to 819c92b Compare January 21, 2025 01:34

geetanjalimanegslab added 4 commits January 21, 2025 07:27

Issue_853: Optimized code, added UT for view configuration check

c12f970

Issue_853: Updated testcase docstrings and fixed Cognitive Complexity…

69308b9

… issue

Issue_853: fixed Cognitive Complexity issue

05f1c99

Issue_853: optimized code to fix the congnitive complexity issues

8d3d1ab

geetanjalimanegslab requested a review from carl-baillargeon January 21, 2025 11:04

gmuloc requested changes Jan 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(anta): Added the test case to verify SNMP groups #886

feat(anta): Added the test case to verify SNMP groups #886

vitthalmagadum commented Oct 16, 2024

github-actions bot commented Dec 2, 2024

github-actions bot commented Jan 16, 2025

codspeed-hq bot commented Jan 16, 2025 •

edited

Loading

carl-baillargeon Jan 20, 2025

geetanjalimanegslab Jan 21, 2025

carl-baillargeon Jan 20, 2025

geetanjalimanegslab Jan 21, 2025 •

edited

Loading

carl-baillargeon Jan 20, 2025

carl-baillargeon Jan 20, 2025

carl-baillargeon Jan 21, 2025

carl-baillargeon Jan 21, 2025

geetanjalimanegslab Jan 21, 2025 •

edited

Loading

sonarqubecloud bot commented Jan 21, 2025

gmuloc left a comment

gmuloc Jan 21, 2025

gmuloc Jan 21, 2025

gmuloc Jan 21, 2025

gmuloc Jan 21, 2025

carl-baillargeon Jan 22, 2025

	3. Ensures that the SNMP views, the read, write and notify settings aligning with version-specific requirements.
	3. Ensures that SNMP group configurations, including read, write, and notify views, align with version-specific requirements.

	msg = f"{snmp_group}; advanced `authentication` is required."
	msg = f"{snmp_group}; `authentication` field is required for `version: v3`"

	* Failure: The test will fail if the provided SNMP group is not configured or specified parameters are not correctly configured.
	* Failure: The test will fail if the provided SNMP group is not configured or if any specified parameter is not correctly configured.

	msg = f"{snmp_group}; `authentication` field is required for `version: v3`"
	msg = f"{snmp_group}: `authentication` field is required for `version: v3`"

feat(anta): Added the test case to verify SNMP groups #886

Are you sure you want to change the base?

feat(anta): Added the test case to verify SNMP groups #886

Conversation

vitthalmagadum commented Oct 16, 2024

Description

Checklist:

github-actions bot commented Dec 2, 2024

github-actions bot commented Jan 16, 2025

codspeed-hq bot commented Jan 16, 2025 • edited Loading

Merging #886 will not alter performance

Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geetanjalimanegslab Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geetanjalimanegslab Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 21, 2025

Quality Gate passed

gmuloc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codspeed-hq bot commented Jan 16, 2025 •

edited

Loading

geetanjalimanegslab Jan 21, 2025 •

edited

Loading

geetanjalimanegslab Jan 21, 2025 •

edited

Loading