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.

Sign up for GitHub

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

Jump to bottom

refactor: replace rest client with controller-runtime clientset for Trivy analyzers #776

Merged
merged 2 commits into from
Nov 29, 2023
Merged

refactor: replace rest client with controller-runtime clientset for Trivy analyzers #776

merged 2 commits into from
Nov 29, 2023

Conversation

ptyin
Copy link
Contributor

@ptyin ptyin commented Nov 26, 2023

Closes #771

📑 Description

Replace the Trivy analyzer in the same manner as the one called by the Gateway analyzer.

✅ 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

ℹ Additional Information

@ptyin ptyin requested review from a team as code owners November 26, 2023 11:16
arbreezy
arbreezy approved these changes Nov 26, 2023
View reviewed changes
Copy link
Member

@arbreezy arbreezy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

great, lgtm
your PR should comply with DCO though @ptyin , please have a look

refactor: replace rest client with controller-runtime clientset for T…
bc0f281 
…rivy analyzers

Signed-off-by: ptyin <peteryin1604@gmail.com>
@ptyin
Copy link
Contributor Author

ptyin commented Nov 27, 2023

great, lgtm your PR should comply with DCO though @ptyin , please have a look

Done

@arbreezy
Copy link
Member

arbreezy commented Nov 27, 2023 •
edited
Loading

@ptyin I think we can also remove the rest client initialization aa well,
https://github.com/k8sgpt-ai/k8sgpt/blob/main/pkg/kubernetes/kubernetes.go#L71

@ptyin
Copy link
Contributor Author

ptyin commented Nov 27, 2023

@ptyin I think we can also remove the rest client initialization aa well, https://github.com/k8sgpt-ai/k8sgpt/blob/main/pkg/kubernetes/kubernetes.go#L71

Got it.

@ptyin
Copy link
Contributor Author

ptyin commented Nov 27, 2023

We should modify the Client struct so to remove field RestClient and GetRestClient method as well, right?

@arbreezy
Copy link
Member

We should modify the Client struct so to remove field RestClient and GetRestClient method as well, right?

Yes exactly, unless there's anything still using the rest client which IIRC there isn't

refactor: remove rest client
255b5eb 
Signed-off-by: ptyin <peteryin1604@gmail.com>
@ptyin ptyin requested a review from arbreezy November 28, 2023 05:58
@ptyin
Copy link
Contributor Author

ptyin commented Nov 29, 2023

Hi Aris, I have pushed another commit as required. Could you kindly re-review it?

@arbreezy arbreezy merged commit 1d19628 into k8sgpt-ai:main Nov 29, 2023
7 checks passed
@github-actions github-actions bot mentioned this pull request Nov 29, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arbreezy arbreezy arbreezy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Backlog
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature]: Replace rest kubernetes client with controller-runtime clientset for Trivy integration analyzers
2 participants
@ptyin @arbreezy