Refine CVE check in check script for k8s version policy #779
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
initial codes pushed to the git, the rest will be upcoming after tests |
Signed-off-by: Piotr <[email protected]>
Signed-off-by: Piotr <[email protected]>
mbuechse
reviewed
Oct 16, 2024
Comment on lines
70
to
82
| 3. CI Integration | ||
| * Trivy | ||
| - Providers should integrate Trivy into their CI pipeline to automatically scan Kubernetes cluster components, | ||
| including kubelet, apiserver, and others. | ||
| - The CI job MUST fail if critical vulnerabilities (CVSS >= 8) are detected in the cluster components. | ||
| - JSON reports from Trivy scans should be reviewed, and Trivy's experimental status should be monitored for changes | ||
| in output formats. | ||
| * nvdlib (Fallback): | ||
| - If Trivy fails or cannot meet requierements, nvdlib MUST be used as a fallback to query CVE data for Kubernetes | ||
| versions, laveraging CPE-based searches to track vunerabilities for specific versions. | ||
| - Providers using nvdlib MUST periodically query for critical cunerabilities affecting the Kubernetes version in production. | ||
|
|
||
| 4. TBD |
There was a problem hiding this comment.
This standard has been stabilized already, for better or worse. New requirements can only be introduced in a new major version (then v3). However, I'm not sure that this was the original objective of this PR; here, we mainly wanted some tooling for the compliance check, and the providers are free to use whatever tools they want. (We can put these items into the implementation notes though, but only as non-authoritative recommendation!)
There was a problem hiding this comment.
does that mean that I should restore original version of standard?
There was a problem hiding this comment.
It would be good to make your research results available. We should just reframe them as guidelines for operators. We could write a blog post. I would then ask you to get feedback from Team Container. It would be good to talk to people who already use Trivy.
There was a problem hiding this comment.
What I have done right now is restore the original standard text and drop the changes.
According to the code, there were several changes made:
- Integrated Trivy for scanning Kubernetes pod images for security vulnerabilities.
- Fixed issue with ClusterInfo object being incorrectly passed where kubeconfig path was expected.
- Added logging improvements to provide clearer insights during version compliance checks.
- Refined the code structure to handle K8s image scanning and cluster versioning in an async manner.
There was a problem hiding this comment.
What I have done right now is restore the original standard text and drop the changes
This is not what I see.
There was a problem hiding this comment.
I would then ask you to get feedback from Team Container
Have you done that?
There was a problem hiding this comment.
What I have done right now is restore the original standard text and drop the changes
@mbuechse I do apologize, I have reverted it now, it was lost somewhere on my git in the mess with the branches
There was a problem hiding this comment.
I would then ask you to get feedback from Team Container
Have you done that?
I have not, I will bring that topic on the nearest container call(last week there was not a container call at all).
mbuechse
changed the title
|
Sorry, I went ahead and marked this as draft, and I changed the title as well to give context. |
- Integrated Trivy for scanning Kubernetes pod images for security vulnerabilities. - Fixed issue with ClusterInfo object being incorrectly passed where kubeconfig path was expected. - Addressed SSL certificate verification error when making external HTTP requests by adding proper handling. - Updated the compliance check logic to ensure correct validation of Kubernetes cluster versions and vulnerability checks. - Added logging improvements to provide clearer insights during version compliance checks. - Refined the code structure to handle K8s image scanning and cluster versioning in an async manner. Signed-off-by: Piotr <[email protected]>
Signed-off-by: Piotr <[email protected]>
mbuechse
reviewed
Oct 25, 2024
Comment on lines
70
to
82
| 3. CI Integration | ||
| * Trivy | ||
| - Providers should integrate Trivy into their CI pipeline to automatically scan Kubernetes cluster components, | ||
| including kubelet, apiserver, and others. | ||
| - The CI job MUST fail if critical vulnerabilities (CVSS >= 8) are detected in the cluster components. | ||
| - JSON reports from Trivy scans should be reviewed, and Trivy's experimental status should be monitored for changes | ||
| in output formats. | ||
| * nvdlib (Fallback): | ||
| - If Trivy fails or cannot meet requierements, nvdlib MUST be used as a fallback to query CVE data for Kubernetes | ||
| versions, laveraging CPE-based searches to track vunerabilities for specific versions. | ||
| - Providers using nvdlib MUST periodically query for critical cunerabilities affecting the Kubernetes version in production. | ||
|
|
||
| 4. TBD |
There was a problem hiding this comment.
What I have done right now is restore the original standard text and drop the changes
This is not what I see.
Comment on lines
70
to
82
| 3. CI Integration | ||
| * Trivy | ||
| - Providers should integrate Trivy into their CI pipeline to automatically scan Kubernetes cluster components, | ||
| including kubelet, apiserver, and others. | ||
| - The CI job MUST fail if critical vulnerabilities (CVSS >= 8) are detected in the cluster components. | ||
| - JSON reports from Trivy scans should be reviewed, and Trivy's experimental status should be monitored for changes | ||
| in output formats. | ||
| * nvdlib (Fallback): | ||
| - If Trivy fails or cannot meet requierements, nvdlib MUST be used as a fallback to query CVE data for Kubernetes | ||
| versions, laveraging CPE-based searches to track vunerabilities for specific versions. | ||
| - Providers using nvdlib MUST periodically query for critical cunerabilities affecting the Kubernetes version in production. | ||
|
|
||
| 4. TBD |
There was a problem hiding this comment.
I would then ask you to get feedback from Team Container
Have you done that?
|
|
||
| async def get_k8s_pod_images(kubeconfig, context=None) -> list[str]: | ||
| """Get the list of container images used by all the pods in the Kubernetes cluster.""" | ||
| cluster_config = await kubernetes_asyncio.config.load_kube_config(kubeconfig, context) |
There was a problem hiding this comment.
This variable is not used (this fact gets reported by flake8 as well), and this doesn't seem right?!
| connector = aiohttp.TCPConnector(limit=5) | ||
| async with aiohttp.ClientSession(connector=connector) as session: | ||
| cve_affected_ranges = await collect_cve_versions(session) | ||
| releases_data = fetch_k8s_releases_data() | ||
|
|
||
| try: | ||
| logger.info(f"Checking cluster specified by {kubeconfig_path}") |
There was a problem hiding this comment.
This needs some more explanation, because almost the same message will be displayed in line 577 (only better, because there, it contains the context as well).
There was a problem hiding this comment.
would that be satisfying?
f""" Initiating scan on the Kubernetes cluster specified by kubeconfig at '{kubeconfig_path}'
{' with context ' + config.context if config.context else ''}.
Fetching cluster information and verifying access.""")
scanner provides in the output additional info regarding vulnerability.
There was a problem hiding this comment.
My point was that it looks like a duplicate of the other line, and it appears to me that the script now tries to achieve the same objective with two different means, one after the other.
Signed-off-by: Piotr <[email protected]>
…d, providing more specific description for initiating scan on the kubernetes, removing cluster_config variable Signed-off-by: Piotr <[email protected]>
Signed-off-by: Piotr <[email protected]>
Signed-off-by: Piotr <[email protected]>
Co-authored-by: Matthias Büchse <[email protected]> Signed-off-by: Piotr <[email protected]>
Signed-off-by: Piotr <[email protected]>
|
Hi @mbuechse and @piobig2871, It looks like it loops over the path to the kubeconfig and uses every character as image to scan. |
Signed-off-by: Piotr <[email protected]>
Signed-off-by: Piotr <[email protected]>
|
Hi @jschoone there was a problem with one argument, I have fixed the issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.