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

Add test for k8s version recency (#288) #318

Merged
merged 2 commits into from
Aug 15, 2023
Merged

Add test for k8s version recency (#288) #318

merged 2 commits into from
Aug 15, 2023

Conversation

cah-hbaum
Copy link
Contributor

@cah-hbaum cah-hbaum commented Jun 29, 2023

This PR adds a test for the K8s version recency standard.
A user can execute the test script and provide his Kubernetes Cluster config file in order to check if the cluster version
is still in line with the versions demanded by the standard.

The following requirements are set:

  • Minor version is not older then 4 months
  • Patch version is not older then 1 week
  • Versions with CVEs need to be patched faster

This PR also adds a configuration file for the test script, which allows setting different logging parameters as well as the Github Token required to access the latest K8s release information in a readable format.

Closes #288

@cah-hbaum cah-hbaum added IaaS Issues or pull requests relevant for Team1: IaaS Container Issues or pull requests relevant for Team 2: Container Infra and Tooling SCS is standardized SCS is standardized SCS-VP10 Related to tender lot SCS-VP10 labels Jun 29, 2023
@cah-hbaum cah-hbaum self-assigned this Jun 29, 2023
@cah-hbaum
Copy link
Contributor Author

The things we also might want to think about here:

  • a Github Token needs to be used in order to access the API calls to Github
  • the recency variables could also be set/changed in the config, if we want to be flexible with respect for the standard
  • Versions with CVEs need to be patched faster this section should probably defined better

Some improvements I also thought about:

  • this test could be sped up by either
    a) caching the CVE data for repeated uses somehow, since they take the most time or
    b) adding a --disable-cve option, which would disable it for the requested runs (but this would go against the Standard)
  • test multiple clusters by providing multiple files (or just call the test multiple times, but this would require the caching solution from before)

@cah-hbaum
Copy link
Contributor Author

Please add more/other reviewers if required!

Copy link
Contributor

@mbuechse mbuechse left a comment

Choose a reason for hiding this comment

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

I think this implementation does get the job done, and I also think that a lot of important research went into it (for finding the right endpoints for the Kubernetes releases and CVE data). Nevertheless, from my point of view, it needs a bit of polish here and there.

  • the naming of the functions should better reflect what they do
  • the functions also seem to mix levels of abstraction and concerns that I would prefer to see separated
  • async should be capitalized on more
  • some of the logic seems bit convoluted

@mbuechse
Copy link
Contributor

mbuechse commented Jul 3, 2023

One thing that I forgot: This PR should also replace the TBD in the standard by just one paragraph explaining the name of the script and how to invoke it properly (this wouldn't warrant a dedicated issue in my opinion).

@cah-hbaum cah-hbaum force-pushed the issue/288 branch 4 times, most recently from 996ed5b to 928be2a Compare July 4, 2023 11:24
@cah-hbaum cah-hbaum force-pushed the issue/288 branch 2 times, most recently from eb6028f to 0d4929e Compare July 5, 2023 11:00
@cah-hbaum cah-hbaum force-pushed the issue/288 branch 3 times, most recently from fab867e to 007b023 Compare July 6, 2023 06:41
@cah-hbaum
Copy link
Contributor Author

As per suggestion of @jschoone, I looked into Trivy.
In general, it looks like a nice tool with many possibilities, which can provide an overview for all vulnerabilities.

The only two problems I see is that trivy doesn't provide an interface/API for other programs (at least I didn't find one during checking). Maybe their CI pipeline integration could be useful here, but that would need further investigation from my side.
If we then would need to use the output given by the trivy application, we would again need to parse this. But at least this would be standardized.

If someone else has opinions on this, I'm open for discussions/suggestions/etc.

@jschoone jschoone removed their request for review August 1, 2023 08:45
@cah-hbaum
Copy link
Contributor Author

I think, I fixed/changed most of the problems/things found, please provide more feedback if anymore problems arise. Thank you for your contribution.

@fdobrovolny fdobrovolny self-requested a review August 2, 2023 13:59
@fdobrovolny
Copy link
Contributor

Approved by mistake. Mostly LGTM.

* add initial test script
* check k8s release for releases
* check cluster with kubeconfig for version
* check version against each other
* if not matching, check for cadence time

Signed-off-by: Hannes Baum <[email protected]>
* updates to test after reviews from @chess-knight and @fdobrovolny
* changed math for weeks and months, since they were faulty in the previous implementation
* changed way how the k8s versions are compared to a simpler loop which also causes less problems

Signed-off-by: Hannes Baum <[email protected]>
@cah-hbaum cah-hbaum merged commit 5514957 into main Aug 15, 2023
4 checks passed
@cah-hbaum cah-hbaum deleted the issue/288 branch August 15, 2023 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Container Issues or pull requests relevant for Team 2: Container Infra and Tooling IaaS Issues or pull requests relevant for Team1: IaaS SCS is standardized SCS is standardized SCS-VP10 Related to tender lot SCS-VP10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Write tests for standard "Offered K8s version recency"
4 participants