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

Implementing SNMP server community #393

Closed
wants to merge 2 commits into from

Conversation

xaviramon
Copy link
Contributor

Change Summary

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Documentation content changes
  • Other (please describe):

Related Issue(s)

Fixes #201

Component(s) name

Proposed changes

How to test

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have updated molecule CI testing accordingly
  • I have read the CONTRIBUTING document.
  • All new and existing tests passed (pre-commit, make linting and make sanity-lint).

@github-actions github-actions bot added role: eos_cli_config_gen issue related to eos_cli_config_gen role type: documentation Improvements or additions to documentation labels Nov 27, 2020
@titom73 titom73 added this to the v1.1.2 milestone Nov 27, 2020
@titom73 titom73 self-requested a review November 28, 2020 08:37
@titom73
Copy link
Contributor

titom73 commented Nov 28, 2020

Hi @xaviramon

Thanks for the PR. After looking at this first commit, we should update data model like this:

snmp_server:
  communities:
    < community_name_1 >:
      access: < ro | rw >
      access_list_ipv4:
        name: < acl_ipv4_name >
      access_list_ipv6:
        name: < acl_ipv6_name >
      view: <view_name >
    < community_name_2 >:
      access: < ro | rw >
      access_list_ipv4:
        name: < acl_ipv4_name >
      access_list_ipv6:
        name: < acl_ipv6_name >
      view: <view_name >

Then, in template, you might change the following:

  • Loop management: for community in snmp_server.communities | arista.avd.natural_sort
  • Test within your CLI: You should at least test if var is defined. We can then assume that if defined means the user has provided a value.

A potential fix is

diff --git a/ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/eos/snmp-settings.j2 b/ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/eos/snmp-settings.j2
index 9f9c44a4..5e6832df 100644
--- a/ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/eos/snmp-settings.j2
+++ b/ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/eos/snmp-settings.j2
@@ -19,8 +19,8 @@ snmp-server view {{ view['name'] }} {{ view['MIB_family_name'] }} {% if view['in
 {%          endfor %}
 {%    endif %}
 {%    if snmp_server.communities is defined and snmp_server.communities is not none %}
-{%      for community in communities | arista.avd.natural_sort %}
-snmp-server community {{ community }} {% if snmp_server.communities[community].view is not none %}view {{ snmp_server.communities[community].view }}  {% endif %}{% if snmp_server.communities[community].access is defined %} {{ snmp_server.communities[community].access }} {% else %} ro {% endif %} {% if snmp_server.communities[community].access_list_ipv6.name is not none %} ipv6 {{ snmp_server.communities[community].access_list_ipv6.name }} {% endif %}{% if snmp_server.communities[community].access_list_ipv4.name is not none %}{{ snmp_server.communities[community].access_list_ipv4.name }} {% endif %}
+{%      for community in snmp_server.communities | arista.avd.natural_sort %}
+snmp-server community {{ community }} {% if snmp_server.communities[community].view is defined %}view {{ snmp_server.communities[community].view }}  {% endif %}{% if snmp_server.communities[community].access is defined %} {{ snmp_server.communities[community].access }} {% else %} ro {% endif %} {% if snmp_server.communities[community].access_list_ipv6.name is defined %} ipv6 {{ snmp_server.communities[community].access_list_ipv6.name }} {% endif %}{% if snmp_server.communities[community].access_list_ipv4.name is defined %}{{ snmp_server.communities[community].access_list_ipv4.name }} {% endif %}
 {%      endfor %}
 {%     endif %}
 {%     if snmp_server.groups is defined and snmp_server.groups is not none %}

@carlbuchmann carlbuchmann modified the milestones: v1.1.2, v1.1.3 Dec 4, 2020
@onurgashi
Copy link
Contributor

Is it possible to add access-list for snmp-server as well additionally to per community?

snmp-server ipv4 access-list ACL-SNMP vrf default

@xaviramon
Copy link
Contributor Author

@titom73 Changes implemented and pushed.

@titom73 titom73 mentioned this pull request Dec 11, 2020
12 tasks
@titom73
Copy link
Contributor

titom73 commented Dec 11, 2020

Need to be investigated with #447 as both are covering more or less the same perimeter.

@onurgashi
Copy link
Contributor

Please review it with @aphillipps and include the documentation of my change in #377

Thanks

@carlbuchmann carlbuchmann modified the milestones: v1.1.3, v1.2.0 Dec 16, 2020
@titom73 titom73 removed this from the v2.0.0rc1 milestone Feb 12, 2021
@github-actions github-actions bot added the state: conflict PR with conflict label Feb 15, 2021
@github-actions
Copy link

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

@melkypie
Copy link
Contributor

Is this PR abandoned or are there any other issues as to why it's not merged?

@carlbuchmann
Copy link
Member

Closing as PR is stale, and data model changed for snmp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
role: eos_cli_config_gen issue related to eos_cli_config_gen role state: conflict PR with conflict type: documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add snmp-server community to snmp-settings template in eos_cli_config_gen
5 participants