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

fix: run all analyzer when no --filter given #413

Closed
wants to merge 4 commits into from

Conversation

panpan0000
Copy link
Contributor

@panpan0000 panpan0000 commented May 15, 2023

Closes #412

πŸ“‘ Description

refer to the #412

βœ… Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

Before fixing:

k8sgpt analyze --filter=VulnerabilityReport| wc -l
18
k8sgpt analyze | wc -l
18

Test Result with newly complied binary:

k8sgpt analyze --filter=VulnerabilityReport| wc -l
18
k8sgpt analyze | wc -l
102

β„Ή Additional Information

@panpan0000 panpan0000 requested review from a team as code owners May 15, 2023 00:50
@panpan0000 panpan0000 changed the title Fix: run all analyzer when no --filter given fix: run all analyzer when no --filter given May 15, 2023
Signed-off-by: Peter Pan <Peter.Pan@daocloud.io>
@AlexsJones
Copy link
Member

Active filters are set in a file, by removing this check those settings will be ignored.
@matthisholleville thoughts?

@panpan0000
Copy link
Contributor Author

panpan0000 commented May 15, 2023

Active filters are set in a file, by removing this check those settings will be ignored. @matthisholleville thoughts?

I think the check will done in https://github.com/k8sgpt-ai/k8sgpt/blob/main/pkg/analysis/analysis.go#L192 by if analyzer, ok := analyzerMap[filter]; ok { ?

@matthisholleville
Copy link
Contributor

Exactly, @AlexsJones. This is incorrect because we would only run all filters if "--filter" is not set and the "active_filters" in the configuration file are empty.

@panpan0000
Copy link
Contributor Author

@matthisholleville @AlexsJones .
Sorry that I didn't catch you: per #412, when after k8sgpt integration activate trivy , there's no way to run the "ALL" analyzers UNTIL you deactivate trivy (that will uninstall trivy.)
Is it a best user experience ?

my understanding of the UX design is as below, but seems it's not. Do you consider the way about below ?

  • user can integrate any plugin then activate them( install the plugin)
  • then user can get analysis for a plugin only ( ex: --filter=VulnerabilityReport)
  • then user can get analysis for ALL analyzers ( ex: by default, no--filter given. )

@AlexsJones
Copy link
Member

k8sgpt integration activate trivy , there's no way to run the "ALL" analyzers UNTIL you deactivate trivy (that will uninstall trivy.)

When you do this, you should be able to run k8sgpt analyze
And all analyzers work

The filters should also show something like this

filters list
Active:
> NetworkPolicy
> Node
> Pod
> CronJob
> PodDisruptionBudget
> VulnerabilityReport
> StatefulSet
> Deployment
> Service
> Ingress
> PersistentVolumeClaim
> ReplicaSet
> HorizontalPodAutoScaler
Unused:

If that is not working currently then there is a bug.

I am not really following what the issue is here.

It's completely possible for k8sgpt analyze to show only a few results if you've disabled analyzers with filters.

@panpan0000
Copy link
Contributor Author

panpan0000 commented May 17, 2023

k8sgpt integration activate trivy , there's no way to run the "ALL" analyzers UNTIL you deactivate trivy (that will uninstall trivy.)

When you do this, you should be able to run k8sgpt analyze And all analyzers work

The filters should also show something like this

filters list
Active:
> NetworkPolicy
> Node
> Pod
> CronJob
> PodDisruptionBudget
> VulnerabilityReport
> StatefulSet
> Deployment
> Service
> Ingress
> PersistentVolumeClaim
> ReplicaSet
> HorizontalPodAutoScaler
Unused:

If that is not working currently then there is a bug.

I am not really following what the issue is here.

It's completely possible for k8sgpt analyze to show only a few results if you've disabled analyzers with filters.

Yes, it's a bug, Alex. @AlexsJones

Detail reproduce steps with 0.3.2.

(1) before integration, everything is fine.

#  k8sgpt  filter list
Active:
> PersistentVolumeClaim
> Service
> StatefulSet
> CronJob
> Pod
> ReplicaSet
> Ingress
> Node
> Deployment
Unused:
> PodDisruptionBudget
> NetworkPolicy
> HorizontalPodAutoScaler

(2) activate trivy

#  k8sgpt  integration activate trivy
2023/05/17 08:48:54 creating 1 resource(s)
2023/05/17 08:48:54 CRD clustercompliancereports.aquasecurity.github.io is already present. Skipping.
2023/05/17 08:48:54 creating 1 resource(s)
.......
2023/05/17 08:48:56 release installed successfully: trivy-operator-k8sgpt/trivy-operator-0.13.2
Activated integration trivy

(3) ALL others are REMOVED from Active filter list, ONLY Trivy left (I believe it's unexpected and the root cause )
I think the expected behavior is adding VulnerabilityReport into the original Active List instead of exclude them all.

#  k8sgpt  filter list
Active:
> VulnerabilityReport (integration)
Unused:
> Pod
> ReplicaSet
> PersistentVolumeClaim
> Node
> Deployment
> Service
> Ingress
> StatefulSet
> CronJob
> HorizontalPodAutoScaler
> PodDisruptionBudget
> NetworkPolicy

(4) k8sgpt analyze only shows Trivy reports.

#  k8sgpt  analyze
AI Provider: openai

0 mspider-system/mspider-gsc-controller-7f99674b5f(Deployment/mspider-gsc-controller)
- Error: critical Vulnerability found ID: CVE-2022-31045 (learn more at: https://avd.aquasec.com/nvd/cve-2022-31045)

1 mspider-system/mspider-work-api-795bb68cb9(Deployment/mspider-work-api)
- Error: critical Vulnerability found ID: CVE-2022-31045 (learn more at: https://avd.aquasec.com/nvd/cve-2022-31045)

2 skoala-system/hive-d5bc5bb58(Deployment/hive)
- Error: critical Vulnerability found ID: CVE-2022-31045 (learn more at: https://avd.aquasec.com/nvd/cve-2022-31045)

3 skoala-system/sesame-686ffc4bc9(Deployment/sesame)
- Error: critical Vulnerability found ID: CVE-2022-31045 (learn more at: https://avd.aquasec.com/nvd/cve-2022-31045)

@panpan0000
Copy link
Contributor Author

panpan0000 commented May 17, 2023

I think the problem actually in pkg/integration/integration.go
when the FIRST time invoking Integration.Activate():

        // HERE:
        // the "active_filters" in viper(`~/.config/k8sgpt/k8sgpt.yaml`) is blank, 
        // because ONLY `coreAnalyzerMap` active so far,  but they are NOT in config file.

        activeFilters := viper.GetStringSlice("active_filters")

        // So the `activeFilters` variable will be BLANK.

        // then  the `mergedFilters` will only have `VulnerabilityReport ` after run `k8sgpt  integration activate trivy`

        mergedFilters := append(activeFilters, integrations[name].GetAnalyzerName())

The best I can figure out to fix the problem is below

@@ -73,6 +74,11 @@ func (*Integration) Activate(name string, namespace string) error {

        // Update filters
        activeFilters := viper.GetStringSlice("active_filters")
+        if len(activeFilters) == 0 {
+               coreFilters, _, _ := analyzer.ListFilters()
+                activeFilters = coreFilters
+        }

But it will introduce complier error of import cycle not allowed.
Hope you experts can shield some light.

@panpan0000
Copy link
Contributor Author

panpan0000 commented May 17, 2023

move the discussion to issue #412

@panpan0000
Copy link
Contributor Author

close it , #441 has fixed it

@panpan0000 panpan0000 closed this May 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug] After integration activate, k8sgpt analyze will never run the Core Analyzers.
3 participants