Skip to content

[ACL] Add CTRLPLANE table services support#4450

Open
HanumanthKumarMotepalli wants to merge 3 commits intosonic-net:masterfrom
HanumanthKumarMotepalli:acl-ctrlplane-services-support
Open

[ACL] Add CTRLPLANE table services support#4450
HanumanthKumarMotepalli wants to merge 3 commits intosonic-net:masterfrom
HanumanthKumarMotepalli:acl-ctrlplane-services-support

Conversation

@HanumanthKumarMotepalli
Copy link
Copy Markdown

@HanumanthKumarMotepalli HanumanthKumarMotepalli commented Apr 13, 2026

fixes sonic-net/sonic-buildimage#4970

What I did

Added --services / -S option to the config acl add table command to support CTRLPLANE ACL table type. Previously, CTRLPLANE tables could not be properly configured.

How I did it

  • Added ACL_CTRLPLANE_VALID_SERVICES = {"NTP", "SNMP", "SSH", "EXTERNAL_CLIENT", "ANY"} in config/main.py
  • Updated parse_acl_table_info() to accept an optional services parameter
  • For CTRLPLANE tables: validates that at least one service is provided, rejects port bindings, and validates service names against the allowed set
  • For non-CTRLPLANE tables: raises an error if --services is passed
  • Added -S / --services CLI option to the add table command
  • Added 10 new unit tests covering all new code paths in tests/acl_config_test.py

How to verify it

#Should succeed
config acl add table SNMP_ACL CTRLPLANE -S SNMP.
config acl add table ROUTER_ACL CTRLPLANE -S SNMP,SSH,NTP

#Should fail - no service provided
config acl add table SNMP_ACL CTRLPLANE

#Should fail - CTRLPLANE cannot bind ports
config acl add table SNMP_ACL CTRLPLANE -S SNMP -p Ethernet0

#Should fail - invalid service
config acl add table SNMP_ACL CTRLPLANE -S INVALID_SVC

Previous command output (if the output of a command-line utility has changed)

$ config acl add table SNMP_ACL CTRLPLANE
#(no error — silently created table without services field)
#show acl table then crashes with KeyError:

New command output (if the output of a command-line utility has changed)

$ config acl add table SNMP_ACL CTRLPLANE
Failed to parse ACL table config: exception=CTRLPLANE ACL table requires at least one service via -S/--services (e.g. SNMP,SSH,NTP)

$ config acl add table SNMP_ACL CTRLPLANE -S SNMP
#(succeeds, table created with services=["SNMP"])

Signed-off-by: Hanumanth Kumar Motepalli <hanumanth-kumar.motepalli@hpe.com>
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Apr 13, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

* Document new -S/--services option for config acl add table
* Add CTRLPLANE as a valid table_type in parameter description
* Add CTRLPLANE usage examples with -S flag

Signed-off-by: Hanumanth Kumar Motepalli <hanumanth-kumar.motepalli@hpe.com>
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@AnantKishorSharma
Copy link
Copy Markdown

AnantKishorSharma commented Apr 14, 2026

@HanumanthKumarMotepalli - Please fix the pre commit error

config/main.py:7860:32: E711 comparison to None should be 'if cond is not None:'

Signed-off-by: Hanumanth Kumar Motepalli <hanumanth-kumar.motepalli@hpe.com>
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@HanumanthKumarMotepalli
Copy link
Copy Markdown
Author

@HanumanthKumarMotepalli - Please fix the pre commit error

config/main.py:7860:32: E711 comparison to None should be 'if cond is not None:'

Resolved

@HanumanthKumarMotepalli
Copy link
Copy Markdown
Author

Hi @yxieca, could you please suggest the appropriate reviewers for this PR?

@yxieca
Copy link
Copy Markdown
Contributor

yxieca commented Apr 18, 2026

Hi @yxieca, could you please suggest the appropriate reviewers for this PR?

@prsunny are you the right reviewer for this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

config acl add does not support control plane service ACLs

4 participants