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

[bug] trivy creates Clusterinfraassessmentreports without node-selector, but doesn't really work with node-selector #1911

Open
cwrau opened this issue Mar 14, 2024 · 11 comments
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed.

Comments

@cwrau
Copy link

cwrau commented Mar 14, 2024

          > @cwrau the param for [use node-selector](https://github.com/aquasecurity/trivy-operator/blob/main/deploy/helm/values.yaml?rgh-link-date=2024-03-12T09%3A52%3A49Z#L620) is enable by default. you can choose not to use it (by configuration) so in-term of scan every node will be collected by node-collector

Ah, perfect, then the only missing part would be the tolerations.

Or, if trivy doesn't want to add tolerations by itself, it shouldn't try to schedule jobs for nodes with taints (that aren't covered by the tolerations)

Originally posted by @cwrau in #1610 (comment)


Currently, the node-selector is enabled by default but doesn't work if the nodes have taints, for example the control-plane.

#1780 introduced a way to disable the node-selector, but that doesn't fix the problem, quite the opposite, now scans for node A might be run on node B, producing incorrect results and creating confusion.

First, disabling the node-selector shouldn't be an option, as otherwise, the Clusterinfraassessmentreports are unreliable. So one should instead disable the Clusterinfraassessmentreports.

A solution to the core problem would be to "calculate" the required tolerations to run on the specified node or just operator: Exists tolerate everything.

@chen-keinan
Copy link
Collaborator

@cwrau 1st option (calculate toleration) should be interfere with user settings
2nd option is a configuration that can be applied today on existing config, correct?

@cwrau
Copy link
Author

cwrau commented Mar 14, 2024

@cwrau 1st option (calculate toleration) should be interfere with user settings

That's currently correct, but we could remove this configuration option in that case.

2nd option is a configuration that can be applied today on existing config, correct?

Yes!

@chen-keinan
Copy link
Collaborator

@cwrau this is not a bug , the information on Clusterinfraassessmentreports is not depend on node-collector only

@cwrau
Copy link
Author

cwrau commented Mar 17, 2024

@cwrau this is not a bug , the information on Clusterinfraassessmentreports is not depend on node-collector only

Ah, ok, so trivy automatically skips the checks that use the mounted directories?

@chen-keinan
Copy link
Collaborator

@cwrau this is not a bug , the information on Clusterinfraassessmentreports is not depend on node-collector only

Ah, ok, so trivy automatically skips the checks that use the mounted directories?

Clusterinfraassessmentreports some checks are taken also from other CRDs (checks which are related to apiserver, etcd, controller-manager and etc)

@chen-keinan
Copy link
Collaborator

@cwrau do you still want to keep this issue open ?
it will be great to add this input to troubleshooting docs

A solution to the core problem would be to "calculate" the required tolerations to run on the specified node or just operator: Exists tolerate everything.

@cwrau
Copy link
Author

cwrau commented Mar 25, 2024

@cwrau this is not a bug , the information on Clusterinfraassessmentreports is not depend on node-collector only

Ah, ok, so trivy automatically skips the checks that use the mounted directories?

Clusterinfraassessmentreports some checks are taken also from other CRDs (checks which are related to apiserver, etcd, controller-manager and etc)

Ok, but are the checks requiring the mounted directories skipped? To me it looks like no, as the job just ran on another node but the Clusterinfraassessmentreports shows the following;

- category: Kubernetes Security Check                                                     
  checkID: KCV0077                                                                        
  description: Ensure that if the kubelet refers to a configuration file with the         
    --config argument, that file has permissions of 600 or more restrictive.              
  messages:                                                                               
  - Ensure that if the kubelet refers to a configuration file with the --config           
    argument, that file has permissions of 600 or more restrictive.                       
  remediation: Change the kubelet config yaml permissions to 600 or more restrictive      
    if exist                                                                              
  severity: HIGH                                                                          
  success: false                                                                          
  title: If the kubelet config.yaml configuration file is being used validate permissions 
    set to 600 or more restrictive                                                        

@cwrau do you still want to keep this issue open ? it will be great to add this input to troubleshooting docs

Definitely,

A solution to the core problem would be to "calculate" the required tolerations to run on the specified node or just operator: Exists tolerate everything.

this would be a great improvement to the operator core itself, not the docs

@chen-keinan
Copy link
Collaborator

chen-keinan commented Mar 27, 2024

@cwrau this is not a bug , the information on Clusterinfraassessmentreports is not depend on node-collector only

Ah, ok, so trivy automatically skips the checks that use the mounted directories?

Clusterinfraassessmentreports some checks are taken also from other CRDs (checks which are related to apiserver, etcd, controller-manager and etc)

Ok, but are the checks requiring the mounted directories skipped? To me it looks like no, as the job just ran on another node but the Clusterinfraassessmentreports shows the following;

- category: Kubernetes Security Check                                                     
  checkID: KCV0077                                                                        
  description: Ensure that if the kubelet refers to a configuration file with the         
    --config argument, that file has permissions of 600 or more restrictive.              
  messages:                                                                               
  - Ensure that if the kubelet refers to a configuration file with the --config           
    argument, that file has permissions of 600 or more restrictive.                       
  remediation: Change the kubelet config yaml permissions to 600 or more restrictive      
    if exist                                                                              
  severity: HIGH                                                                          
  success: false                                                                          
  title: If the kubelet config.yaml configuration file is being used validate permissions 
    set to 600 or more restrictive                                                        

@cwrau do you still want to keep this issue open ? it will be great to add this input to troubleshooting docs

Definitely,

A solution to the core problem would be to "calculate" the required tolerations to run on the specified node or just operator: Exists tolerate everything.

this would be a great improvement to the operator core itself, not the docs

as mention above, the 1st option "calculate" can be intrusive to user config and 2nd option can be easily fix by config, you mean you want to make operator: Exists as default config ?

@cwrau
Copy link
Author

cwrau commented Mar 27, 2024

@cwrau this is not a bug , the information on Clusterinfraassessmentreports is not depend on node-collector only

Ah, ok, so trivy automatically skips the checks that use the mounted directories?

Clusterinfraassessmentreports some checks are taken also from other CRDs (checks which are related to apiserver, etcd, controller-manager and etc)

Ok, but are the checks requiring the mounted directories skipped? To me it looks like no, as the job just ran on another node but the Clusterinfraassessmentreports shows the following;

- category: Kubernetes Security Check                                                     
  checkID: KCV0077                                                                        
  description: Ensure that if the kubelet refers to a configuration file with the         
    --config argument, that file has permissions of 600 or more restrictive.              
  messages:                                                                               
  - Ensure that if the kubelet refers to a configuration file with the --config           
    argument, that file has permissions of 600 or more restrictive.                       
  remediation: Change the kubelet config yaml permissions to 600 or more restrictive      
    if exist                                                                              
  severity: HIGH                                                                          
  success: false                                                                          
  title: If the kubelet config.yaml configuration file is being used validate permissions 
    set to 600 or more restrictive                                                        

@cwrau do you still want to keep this issue open ? it will be great to add this input to troubleshooting docs

Definitely,

A solution to the core problem would be to "calculate" the required tolerations to run on the specified node or just operator: Exists tolerate everything.

this would be a great improvement to the operator core itself, not the docs

as mention above, the 1st option "calculate" can be intrusive to user config and 2nd option can be easily fix by config, you mean you want to make operator: Exists as default config ?

As I said, one of the two options should be implemented by the operator.
I mean, the operator decided,, by itself,, to run this job, why should the user be required to configure stuff.
I would say the default config should just work.

I'm open to implement one of the two options, the question is just, which one?

@chen-keinan
Copy link
Collaborator

@cwrau sure, contributions are welcome

Copy link

This issue is stale because it has been labeled with inactivity.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed. label May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

2 participants