-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[caclmgrd]: Add infrastructure to support adding feature specific acls #11367
Conversation
ACLs if required. Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
@@ -201,6 +206,12 @@ class ControlPlaneAclManager(daemon_base.DaemonBase): | |||
tcp_flags_str = tcp_flags_str[:-1] | |||
return tcp_flags_str | |||
|
|||
def update_feature_present(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added unit-test and mock data for the unit-test.
feature_tb_info = self.config_db_map[DEFAULT_NAMESPACE].get_table(self.FEATURE_TABLE) | ||
if feature_tb_info: | ||
for k, v in feature_tb_info.items(): | ||
self.feature_present[k] = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems this is a generic oneshot cache. Suggest leverage common library like cached_property
. ref: https://www.geeksforgeeks.org/python-functools-cached_property/ #WontFix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for the pointer. Currently, we are updating this feature_present map with the features that are present only once during initialization so it is not repeatedlyinvoked.
Also, if we use cached_property, we will invoke the function once, which could be in per-thread context, which might cause issues. So not using cached_property in this case.
Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
#11367) Why I did it Add infrastructure to support adding feature specific acls. If feature specific ACLs has to be added: if feature_name in self.feature_present and self.feature_present.get('feature_name'): add_feature_specific_acls() How I did it Add function to get features present in feature table. How to verify it unit-test passes.
sonic-net#11367) Why I did it Add infrastructure to support adding feature specific acls. If feature specific ACLs has to be added: if feature_name in self.feature_present and self.feature_present.get('feature_name'): add_feature_specific_acls() How I did it Add function to get features present in feature table. How to verify it unit-test passes.
Signed-off-by: Suvarna Meenakshi sumeenak@microsoft.com
Why I did it
Add infrastructure to support adding feature specific acls.
If feature specific ACLs has to be added:
How I did it
Add function to get features present in feature table.
How to verify it
unit-test passes.
Which release branch to backport (provide reason below if selected)
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)