Skip to content
This repository has been archived by the owner on Oct 30, 2024. It is now read-only.

✨ Adds ability to not filter CRDS, see ISSUE #373 #374

Merged
merged 2 commits into from
Nov 17, 2021

Conversation

nobletrout
Copy link
Contributor

@nobletrout nobletrout commented Nov 5, 2021

Description

Adds new command flag to not filter out generated resources, which unfortunately also can include CRD based resources (like a CRD based on Application)
Fixes #373

Type of change
  • New feature ✨
How Has This Been Tested?

I ran it against my cluster and compared results with the flag on and off and found a lot more issues with the flag, which was my intent.

Checklist:
  • I have 🎩 my changes (A 🎩 specifically includes pulling down changes, setting them up, and manually testing the changed features and potential side effects to make sure nothing is broken)
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • The test coverage did not decrease
  • I have signed the appropriate Contributor License Agreement

It is kind of difficult to test if these changes work or not, since they only apply to querying remote clusters and not local files. If you guys have a suggestion for me to add test cases, I'm happy to help or learn.

@ghost
Copy link

ghost commented Nov 5, 2021

Thanks for opening this pull request! Please check out our contributing guidelines and sign the CLA.

@ghost ghost added the core label Nov 5, 2021
Copy link
Contributor

@genevieveluyt genevieveluyt left a comment

Choose a reason for hiding this comment

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

Thanks for your PR! Could you please document the new flag in the README. Perhaps something like:

"By default, kubeaudit will ignore generated resources (such as Pods generated by Deployments). If you would like kubeaudit to produce results for generated resources (for example if you have custom resources or want to catch orphaned resources where the owner resource no longer exists) you can use this flag."

internal/k8sinternal/client.go Show resolved Hide resolved
cmd/commands/root.go Outdated Show resolved Hide resolved
@ghost ghost added the readme label Nov 12, 2021
@genevieveluyt
Copy link
Contributor

Perfect thanks! As for testing, I can look into adding a test next week.

If you'd like to try it out yourself, I'm thinking of doing something similar to https://github.com/Shopify/kubeaudit/blob/main/internal/test/test.go#L111-L114 with a new test fixture and then seeing if the report results contains the pods or not (or even just comparing the number of results). You can use make test-setup to set up a test cluster using kind so you can run the individual test.

But definitely don't feel like you need to do all that, as I said I'm happy to add the testing portion.

@nobletrout
Copy link
Contributor Author

I'll try to get the unit tests to work, but if you beat me to it go ahead. I'm trying to figure out how to not run ALL the unit tests everytime.
any advice?

@nobletrout
Copy link
Contributor Author

ok, had a look at the unit tests. it appears that the file name for the current loaded objects represent the namespace used to test each object. There's a bit of structural issue here with the testing framework that requires a better familiarity with Go and the unit tests here than I feel comfortable tackling right now. I am interested to see how you solve it though.

Copy link
Contributor

@genevieveluyt genevieveluyt left a comment

Choose a reason for hiding this comment

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

Ok I wrote out a test locally and it passes 🎉 . Since I can't push to your branch let's get your PR merged and I'll add the test as a separate PR.

@genevieveluyt genevieveluyt enabled auto-merge (squash) November 17, 2021 20:41
@genevieveluyt genevieveluyt merged commit b0c9c3c into Shopify:main Nov 17, 2021
@genevieveluyt genevieveluyt mentioned this pull request Nov 19, 2021
13 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support for CRD's that extend common types
2 participants