-
Notifications
You must be signed in to change notification settings - Fork 706
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
Fix issue with ambiguity of control product #12454
Fix issue with ambiguity of control product #12454
Conversation
…uct names vs product specific controls When product member is initialised during loading of yaml file it could be ambigously created as a string, when control is specific for only one product or list when multiple products are specified in the yaml. The problem with partial match of product names comes later in the add_references method, where the current product of the build is matched vs the control product, and the condition used is `product not in self.product`. In case of list this condition will check if any of the members of the list is exact match to the product. In case of string though, which is the more common case it will check if the string of the product name, for which we are building is partially matched (contained) in the self.product. The issue was found while analysing complaint from a contributor Joel Njanga(@barbarello), while he was trying to add support for al2 platform and it was conflicting with exsting platform al2023
🤖 A k8s content image for this PR is available at: Click here to see how to deploy itIf you alread have Compliance Operator deployed: Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and: |
Thanks for the pull request. It appears that the CI failures on the Python Unit tests are valid. Please take look. If you need any help please let us know. |
Thanks @Mab879 missed to include the consideration if product remains None |
@teacup-on-rockingchair Excellent catch! I think it was originally designed as a filed that should contain just a single product but then it evolved. Unfortunately, at this moment it isn't well handled. Please update this documentation:
Also, consider renaming it to "products" and enforcing it being a list. |
ssg/controls.py
Outdated
@@ -349,7 +349,11 @@ def load(self): | |||
self.title = ssg.utils.required_key(yaml_contents, "title") | |||
self.source = yaml_contents.get("source", "") | |||
self.reference_type = yaml_contents.get("reference_type", None) | |||
self.product = yaml_contents.get("product", None) | |||
yaml_product = yaml_contents.get("product", None) | |||
if type(yaml_product) is list: |
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.
using isinstance() is preffered over type()
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.
Thanks changed it in 83eaa21
Thanks to @jan-cerny for the note 🙇
Thanks to @jan-cerny for the reminder 🙇
Code Climate has analyzed commit adf59e6 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 60.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 60.9% (1.3% change). View more on Code Climate. |
Description:
Rationale:
product not in self.product
.Fixes: