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

Support categories #469

Merged
merged 14 commits into from
Jul 30, 2023
Merged

Support categories #469

merged 14 commits into from
Jul 30, 2023

Conversation

Daniel-GrunbergerCA
Copy link
Contributor

Overview

  • Adding categories for controls. Each control can have a category and a sub-category. Each category has a name and an ID. For readability reasons I have added just the name to the controls json, and the ID is inserted on the export.py script. There is a file which maps the categories names to their respective ids. This way, we can easily change the name for a category, as long as we keep the same ID.
  • Two new frameworks: ClusterScan and WorkloadScan. They will be transparent from the BE and users perspective, and will be used by KS for a cluster and workload scan, respectively.
  • Renaming 3 controls:
    1. "Malicious admission controller (validating)" to “Validate admission controller (validating)“
    2. “Malicious admission controller (mutating)” to Validate admission controller (mutating)”
    3. “Containers mounting Docker socket” to “Container runtime socket mounted”

Daniel Grunberger added 8 commits July 11, 2023 11:24
Signed-off-by: Daniel Grunberger <[email protected]>
Signed-off-by: Daniel Grunberger <[email protected]>
Signed-off-by: Daniel Grunberger <[email protected]>
Signed-off-by: Daniel Grunberger <[email protected]>
Signed-off-by: Daniel Grunberger <[email protected]>
@github-actions
Copy link
Contributor

Summary:

  • License scan: failure
  • Credentials scan: success
  • Vulnerabilities scan: success
  • Unit test: success
  • Go linting: success

@yuleib
Copy link
Contributor

yuleib commented Jul 20, 2023

@Daniel-GrunbergerCA - Hey!

  1. This PR is too large, needs to be splitted into the relevant topics of the PR.
  2. With whom did we review that ? why we need to add those categories ?
  3. How we are going to add those categories to new controls developed ? by which base ?
  4. README file should be updated as well to guide the next developers.

This PR should be approved only after a meeting with regolibrary maintainers, so we can understand which effection we have over other developments now.

yuleib
yuleib previously requested changes Jul 20, 2023
Copy link
Contributor

@yuleib yuleib left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Daniel-GrunbergerCA - Hey!

This PR is too large, needs to be splitted into the relevant topics of the PR.
With whom did we review that ? why we need to add those categories ?
How we are going to add those categories to new controls developed ? by which base ?
README file should be updated as well to guide the next developers.
This PR should be approved only after a meeting with regolibrary maintainers, so we can understand which effection we have over other developments now.

Copy link
Collaborator

@YiscahLevySilas1 YiscahLevySilas1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Daniel-GrunbergerCA - please supply clear documentation of how to add these categories for future controls. Like we disscussed - If it can be automated that would be best and would make our life easier to maintain. If not - it should be very clear to the control contributor what should be the taken into consideration, and we should validate that it was added.

scripts/export.py Outdated Show resolved Hide resolved
scripts/export.py Outdated Show resolved Hide resolved
scripts/export.py Outdated Show resolved Hide resolved
scripts/export.py Outdated Show resolved Hide resolved
@Daniel-GrunbergerCA
Copy link
Contributor Author

Daniel-GrunbergerCA commented Jul 20, 2023

@yuleib @YiscahLevySilas1

There is no clear documentation on how to categorize a control since there is not a straightforward way of doing it. As for now, controls are not required to have a category. The controls which needed a category are being categorized on this PR, and adding new controls without categories won't cause any issues.

For now, the one categorizing controls is @slashben

The reason of adding such categories is that the new KS output will be displaying controls divided by their categories

cc: @dwertent

@github-actions
Copy link
Contributor

Summary:

  • License scan: failure
  • Credentials scan: success
  • Vulnerabilities scan: success
  • Unit test: success
  • Go linting: success

@YiscahLevySilas1
Copy link
Collaborator

The reason of adding such categories is that the new KS output will be displaying controls divided by their categories

@Daniel-GrunbergerCA - How will we handle controls that don't have a category specified?

@YiscahLevySilas1
Copy link
Collaborator

@slashben - Can we try to think of a definition for these categories and guidelines on how to specify this on a new control? (Preferably automaticly! for e.g based on what resources we are requesting for rules in this control) If not we won't be able to maintain this in future controls

@Daniel-GrunbergerCA
Copy link
Contributor Author

The reason of adding such categories is that the new KS output will be displaying controls divided by their categories

@Daniel-GrunbergerCA - How will we handle controls that don't have a category specified?

For a cluster and workload scan, ks will be using the new frameworks provided on this PR. These are the only scans where we are printing different tables for different frameworks. And all controls on this frameworks are categorized. On the worst case, that someone pushes a new uncategorized control to this framework, it just won't be seen on the tables outputs (which is the desired behavior for such case)

@github-actions
Copy link
Contributor

Summary:

  • License scan: failure
  • Credentials scan: success
  • Vulnerabilities scan: success
  • Unit test: success
  • Go linting: success

Signed-off-by: Daniel Grunberger <[email protected]>
@github-actions
Copy link
Contributor

Summary:

  • License scan: failure
  • Credentials scan: success
  • Vulnerabilities scan: success
  • Unit test: success
  • Go linting: success

@github-actions
Copy link
Contributor

Summary:

  • License scan: failure
  • Credentials scan: success
  • Vulnerabilities scan: success
  • Unit test: success
  • Go linting: success

@github-actions
Copy link
Contributor

Summary:

  • License scan: failure
  • Credentials scan: success
  • Vulnerabilities scan: success
  • Unit test: success
  • Go linting: success

@dwertent dwertent dismissed yuleib’s stale review July 30, 2023 12:14

We agreed to merge this PR

@dwertent dwertent merged commit 0688c71 into master Jul 30, 2023
26 checks passed
@dwertent dwertent deleted the support-categories branch August 2, 2023 19:40
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.

4 participants