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

Revert "Merge pull request #494 from armosec/sbom" #503

Merged
merged 1 commit into from
Dec 2, 2024
Merged

Conversation

matthyx
Copy link
Contributor

@matthyx matthyx commented Dec 2, 2024

User description

This reverts commit d356e60, reversing changes made to 50ffcce.


PR Type

enhancement, tests, configuration changes


Description

  • Refactored code to use image IDs for SBOM and CVE retrieval, replacing workload_tags with SBOMKeys and CVEsKeys.
  • Added new methods and updated logic in test scripts for improved vulnerability scanning.
  • Updated multiple Kubernetes deployment configurations to simplify image references by removing unnecessary prefixes.
  • Added new test configurations and updated expected results for CVE and SBOM data.
  • Introduced new JSON structures for vulnerability manifests and SBOM data with updated metadata and specifications.

Changes walkthrough 📝

Relevant files
Enhancement
6 files
relevant_cve.py
Refactor to use image IDs for SBOM and CVE retrieval         

tests_scripts/helm/relevant_cve.py

  • Replaced workload_tags with SBOMKeys and CVEsKeys in function calls.
  • Updated logic to use get_imagesIDs_keys instead of
    get_workloads_images_tags.
  • +21/-21 
    base_k8s.py
    Use image IDs for SBOM and CVE retrieval                                 

    tests_scripts/kubernetes/base_k8s.py

  • Changed image to image_id in get_image_ids method.
  • Refactored get_SBOM_from_storage and get_CVEs_from_storage to use keys
    instead of tags.
  • +30/-40 
    base_vuln_scan.py
    Add method for generating SBOM keys using image IDs           

    tests_scripts/helm/base_vuln_scan.py

  • Removed a condition from validate_expected_SBOM.
  • Added get_imagesIDs_keys method to generate SBOM keys.
  • +24/-1   
    redis-fixed.json
    Added vulnerability manifest for Redis with CVE details. 

    configurations/expected-result/filteredCVEs/redis-fixed.json

  • Added a new JSON object representing a vulnerability manifest for
    Redis.
  • Includes metadata, spec, and status sections with detailed
    vulnerability information.
  • Lists multiple vulnerabilities with CVE identifiers and their details.

  • +538/-1 
    python-simple.json
    Update vulnerability manifest metadata and source details

    configurations/expected-result/CVEs/python-simple.json

  • Updated the metadata section with new uid and creationTimestamp.
  • Adjusted annotations with a new kubescape.io/resource-size.
  • Updated source with a new manifestDigest and imageSize.
  • +1/-1     
    redis_incomplete.json
    Add new SBOMSyftFiltered JSON structure with metadata       

    configurations/expected-result/filteredSBOM/redis_incomplete.json

  • Added a new JSON structure for SBOMSyftFiltered.
  • Updated metadata with new name, uid, and creationTimestamp.
  • Adjusted spec to include new tool version and report creation date.
  • +56/-1   
    Tests
    3 files
    relevant_vuln_scanning_tests.py
    Add test configuration for relevancy storage disabled       

    configurations/system/tests_cases/relevant_vuln_scanning_tests.py

    • Added a new static method relevancy_storage_disabled.
    +17/-0   
    redis-fixed.json
    Update expected CVE results for Redis                                       

    configurations/expected-result/CVEs/redis-fixed.json

    • Replaced the entire JSON content with updated vulnerability data.
    +534/-1 
    system_test_mapping.json
    Added new test case for relevancy storage disabled scenario.

    system_test_mapping.json

    • Added a new test case entry for 'relevancy_storage_disabled'.
    +10/-0   
    Configuration changes
    12 files
    sbomspdxv2p3s.yaml
    Update metadata name in unsupported SBOM CRD                         

    configurations/kubescape-crds/unsupported/sbomspdxv2p3s.yaml

    • Updated the metadata.name field in the YAML configuration.
    +1/-1     
    mariadb.yaml
    Simplify image reference in MariaDB deployment                     

    configurations/k8s_workloads/deployments/viewsv2kev/mariadb.yaml

  • Changed image reference from full Docker URL to a simplified format.
  • +2/-2     
    mariadb.yaml
    Simplified MariaDB image reference in Kubernetes deployment.

    configurations/k8s_workloads/deployments/wikijs/mariadb.yaml

  • Updated the image reference for MariaDB by removing the
    'docker.io/library/' prefix.
  • +2/-2     
    redis-fixed.yaml
    Simplified Redis image reference in Kubernetes deployment.

    configurations/k8s_workloads/deployments/redis/redis-fixed.yaml

  • Updated the image reference for Redis by removing the 'docker.io/'
    prefix.
  • +2/-2     
    python.yaml
    Simplified Python image reference in Kubernetes deployment.

    configurations/k8s_workloads/deployments/java-and-python/python.yaml

  • Updated the image reference for Python by removing the
    'docker.io/library/' prefix.
  • +1/-1     
    nginx-ingress-deployment.yaml
    Simplified Nginx image reference in Kubernetes deployment.

    configurations/k8s_workloads/deployments/wikijs/nginx-ingress-deployment.yaml

  • Updated the image reference for Nginx by removing the
    'docker.io/library/' prefix.
  • +1/-1     
    nginx-vuln-scan-new-image.yaml
    Simplified Nginx image reference in Kubernetes deployment.

    configurations/k8s_workloads/deployments/nginx-vuln-scan-new-image.yaml

  • Updated the image reference for Nginx by removing the
    'docker.io/library/' prefix.
  • +1/-1     
    wikijs.yaml
    Simplified Wiki.js image reference in Kubernetes deployment.

    configurations/k8s_workloads/deployments/wikijs/wikijs.yaml

  • Updated the image reference for Wiki.js by removing the 'docker.io/'
    prefix.
  • +1/-1     
    replicaset.yaml
    Updated image digest for frontend application in Kubernetes
    replicaset.

    configurations/k8s_workloads/synchronizer/replicaset.yaml

  • Updated the image reference for a sample frontend application to a new
    SHA256 digest.
  • +1/-1     
    java.yaml
    Simplified Jetty image reference in Kubernetes deployment.

    configurations/k8s_workloads/deployments/java-and-python/java.yaml

  • Updated the image reference for Jetty by removing the
    'docker.io/library/' prefix.
  • +1/-1     
    java-simple.yaml
    Simplified Jetty image reference in Kubernetes deployment.

    configurations/k8s_workloads/deployments/java-simple/java-simple.yaml

  • Updated the image reference for Jetty by removing the
    'docker.io/library/' prefix.
  • +1/-1     
    python-simple.yaml
    Simplified Python image reference in Kubernetes deployment.

    configurations/k8s_workloads/deployments/python-simple/python-simple.yaml

  • Updated the image reference for Python by removing the
    'docker.io/library/' prefix.
  • +1/-1     
    Additional files (token-limit)
    35 files
    redis.json
    ...                                                                                                           

    configurations/expected-result/SBOM/redis.json

    ...

    +26480/-2
    wikijs.json
    ...                                                                                                           

    configurations/expected-result/CVEs/wikijs.json

    ...

    +1/-2     
    java-simple.json
    ...                                                                                                           

    configurations/expected-result/filteredSBOM/java-simple.json

    ...

    +1/-2     
    redis-fixed.json
    ...                                                                                                           

    configurations/expected-result/SBOM/redis-fixed.json

    ...

    +1/-2     
    redis_incomplete.json
    ...                                                                                                           

    configurations/expected-result/SBOM/redis_incomplete.json

    ...

    +52/-2   
    redis_sleep.json
    ...                                                                                                           

    configurations/expected-result/SBOM/redis_sleep.json

    ...

    +1/-2     
    nginx.json
    ...                                                                                                           

    configurations/expected-result/filteredSBOM/nginx.json

    ...

    +1/-2     
    wikijs.json
    ...                                                                                                           

    configurations/expected-result/filteredSBOM/wikijs.json

    ...

    +1/-2     
    nginx.json
    ...                                                                                                           

    configurations/expected-result/CVEs/nginx.json

    ...

    +1/-2     
    mariadb.json
    ...                                                                                                           

    configurations/expected-result/filteredCVEs/mariadb.json

    ...

    +1/-2     
    python-simple.json
    ...                                                                                                           

    configurations/expected-result/SBOM/python-simple.json

    ...

    +2/-1     
    java-simple.json
    ...                                                                                                           

    configurations/expected-result/CVEs/java-simple.json

    ...

    +1/-2     
    python-client-to-java.json
    ...                                                                                                           

    configurations/expected-result/filteredSBOM/python-client-to-java.json

    ...

    +1/-2     
    wikijs.json
    ...                                                                                                           

    configurations/expected-result/filteredCVEs/wikijs.json

    ...

    +1/-2     
    java-simple.json
    ...                                                                                                           

    configurations/expected-result/filteredCVEs/java-simple.json

    ...

    +1/-2     
    golang-dynamic-simple.json
    ...                                                                                                           

    configurations/expected-result/CVEs/golang-dynamic-simple.json

    ...

    +5261/-2
    nginx.json
    ...                                                                                                           

    configurations/expected-result/filteredCVEs/nginx.json

    ...

    +1/-2     
    redis_sleep.json
    ...                                                                                                           

    configurations/expected-result/CVEs/redis_sleep.json

    ...

    +1576/-2
    redis.json
    ...                                                                                                           

    configurations/expected-result/CVEs/redis.json

    ...

    +1576/-2
    redis.json
    ...                                                                                                           

    configurations/expected-result/filteredCVEs/redis.json

    ...

    +1580/-2
    redis_sleep_updated.json
    ...                                                                                                           

    configurations/expected-result/filteredCVEs/redis_sleep_updated.json

    ...

    +1580/-2
    redis_sleep_updated.json
    ...                                                                                                           

    configurations/expected-result/filteredSBOM/redis_sleep_updated.json

    ...

    +1279/-1
    redis-fixed.json
    ...                                                                                                           

    configurations/expected-result/filteredSBOM/redis-fixed.json

    ...

    +1191/-1
    redis_sleep.json
    ...                                                                                                           

    configurations/expected-result/filteredSBOM/redis_sleep.json

    ...

    +1192/-1
    redis.json
    ...                                                                                                           

    configurations/expected-result/filteredSBOM/redis.json

    ...

    +1181/-1
    redis_sleep.json
    ...                                                                                                           

    configurations/expected-result/filteredCVEs/redis_sleep.json

    ...

    +1162/-1
    python-client-to-java.json
    ...                                                                                                           

    configurations/expected-result/filteredCVEs/python-client-to-java.json

    ...

    +1/-1     
    python-simple.json
    ...                                                                                                           

    configurations/expected-result/filteredSBOM/python-simple.json

    ...

    +274/-1 
    python-simple.json
    ...                                                                                                           

    configurations/expected-result/filteredCVEs/python-simple.json

    ...

    +185/-1 
    golang-dynamic-simple.json
    ...                                                                                                           

    configurations/expected-result/filteredSBOM/golang-dynamic-simple.json

    ...

    +165/-1 
    golang-simple.json
    ...                                                                                                           

    configurations/expected-result/SBOM/golang-simple.json

    ...

    +145/-1 
    golang-simple.json
    ...                                                                                                           

    configurations/expected-result/filteredSBOM/golang-simple.json

    ...

    +148/-1 
    golang-dynamic-simple.json
    ...                                                                                                           

    configurations/expected-result/filteredCVEs/golang-dynamic-simple.json

    ...

    +97/-1   
    golang-simple.json
    ...                                                                                                           

    configurations/expected-result/CVEs/golang-simple.json

    ...

    +84/-1   
    golang-simple.json
    ...                                                                                                           

    configurations/expected-result/filteredCVEs/golang-simple.json

    ...

    +87/-1   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    This reverts commit d356e60, reversing
    changes made to 50ffcce.
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Code Complexity
    The new get_imagesIDs_keys method contains nested loops and complex logic for matching container names and image IDs. This could impact performance and maintainability.

    Breaking Change
    The get_image_ids method now returns image_id instead of image, which could break existing code that depends on the old return value format.

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add null checks when accessing container status attributes to prevent potential NoneType exceptions

    The get_image_ids method should handle cases where container_statuses or
    init_container_statuses are None to avoid potential NoneType errors.

    tests_scripts/kubernetes/base_k8s.py [484-490]

     @staticmethod
     def get_image_ids(pod):
    -    c = [(container_status.name, container_status.image_id) for container_status in
    -            pod.status.container_statuses]
    +    c = []
    +    if pod.status.container_statuses:
    +        c.extend([(container_status.name, container_status.image_id) for container_status in
    +                pod.status.container_statuses])
         if pod.status.init_container_statuses:
             c.extend([(container_status.name, container_status.image_id) for container_status in
                     pod.status.init_container_statuses])
         return c
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion addresses a potential runtime error by adding null checks before accessing container statuses. This is important for robustness as pod.status.container_statuses could be None in certain scenarios, which would cause the current code to crash.

    8

    💡 Need additional feedback ? start a PR chat

    Copy link

    github-actions bot commented Dec 2, 2024

    Failed to generate code suggestions for PR

    @matthyx matthyx merged commit 71c9c67 into master Dec 2, 2024
    3 checks passed
    @matthyx matthyx deleted the revert branch December 2, 2024 19:14
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants