feat: add support for batch v1 cronjob

bluebrown commented 1 year ago

Signed-off-by: Nico Braun rainbowstack@gmail.com

Description

Add support for batch/v1 CronJob

Fixes #520

Type of change

[x] New feature :sparkles:

How Has This Been Tested?

[ ] Test A
[ ] Test B

The test has failed on me.

go test ./internal/k8sinternal/
# throws errors

But building the project and testing it via curl has worked.

make build
curl -fsSL https://raw.githubusercontent.com/kubernetes/website/snapshot-initial-v1.26/content/en/examples/application/job/cronjob.yaml   \
| ./kubeaudit all -f -
# finds issues as desired

I didn't really understand the test suite. Maybe someone can give me a hint.

Checklist:

[ ] I have :tophat: 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
[x] I have signed the appropriate Contributor License Agreement

genevieveluyt commented 1 year ago

Thanks for the quick fix!

I didn't really understand the test suite

Testing is documented here. You will likely want the USE_KIND=false make test if you do not have Kind installed locally. This will only test manifest mode but the other modes will be tested by CI with Kind. CI is currently broken but should hopefully be fixed by https://github.com/Shopify/kubeaudit/pull/521

Please sign the CLA
Please update the cronjob test fixture to make sure we're testing the new version
You will likely have to rebase / merge main into your branch after https://github.com/Shopify/kubeaudit/pull/521 in order for CI to pass, apologies for the blocker!

bluebrown commented 1 year ago

So I figured out why the test is failing. Kind of. Since my PR would support both beta and v1, there is a mismatch in the resource count here and here.

First error ends with ...should have 26 item(s), but has 24 which is 2*1, since doing ns*rsources. And the second one is ...should have 13 item(s), but has 12 for that extra resource, I guess.

I haven't figured out how to teach it, that there are more resources now.

genevieveluyt commented 1 year ago

So for resources in a cluster, the kubernetes API returns all of the resources in the same version, regardless of what version the resource was originally created as. This is why we don't test for multiple versions of the same resource (kubeaudit can't tell them apart once the versions get normalized). So instead of adding a version in the test, I think we just want to replace the old one.

bluebrown commented 1 year ago

I can remove the beta one from the test. But it would be still better to keep the actual thing around so that both versions are supported right?

Maybe we can find a way to have the tests work with multiple versions.

bluebrown commented 1 year ago

I guess this is failing now because the test is still using 1.20 for the kind image. But batch v1 cronjob was only released in 1.21.

Given that kubernetes supports the last 3 versions officially, I think we should use 1.24 as kind image.

bluebrown commented 1 year ago

I tested locally to bump the version. Now there is the next problem. It throws deprecated errors for the beta cronjob. So those tests need be adjusted as well, if we want to bump the version.

genevieveluyt commented 1 year ago

We indeed only support the latest Kubernetes version. If you are bumping the Kind version, here is a reference PR of the last time it was bumped. Unfortunately the test suite is not set up to support backwards compatibility with older Kubernetes versions.

bluebrown commented 1 year ago

Sorry, I think I misread your comment. I have also removed the fixture containg the beta job but it still errors.

Here is one of the errors. They are all from TestAuditDeprecatedAPIs. Since this test is supposed to test deprecated apis, I dont think I should also remove beta from those fixtures.

--- FAIL: TestAuditDeprecatedAPIs (0.00s)
    --- FAIL: TestAuditDeprecatedAPIs/cronjob.yml-1.20-1.21 (16.71s)
        test.go:53: 
                Error Trace:    /home/blue/src/kubeaudit/auditors/deprecatedapis/test.go:53
                                                        /home/blue/src/kubeaudit/auditors/deprecatedapis/test.go:30
                                                        /home/blue/src/kubeaudit/auditors/deprecatedapis/depreceatedapis_test.go:57
                Error:          Not equal: 
                                expected: map[string]bool{"DeprecatedAPIUsed":true}
                                actual  : map[string]bool{}

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1,3 +1,2 @@
                                -(map[string]bool) (len=1) {
                                - (string) (len=17) "DeprecatedAPIUsed": (bool) true
                                +(map[string]bool) {
                                 }
                Test:           TestAuditDeprecatedAPIs/cronjob.yml-1.20-1.21

genevieveluyt commented 1 year ago

Sorry I was out of office last week, will take a look this week

bluebrown commented 1 year ago

Yeah. I didn't find time to look further into this either. I am also not entirely sure how to proceed.

Shopify / kubeaudit