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

Updating the syft format #75

Merged
merged 10 commits into from
Dec 24, 2023
Merged

Updating the syft format #75

merged 10 commits into from
Dec 24, 2023

Conversation

dwertent
Copy link

@dwertent dwertent commented Dec 14, 2023

Type

Enhancement, Tests


Description

This PR primarily focuses on enhancing the Syft format and adding cleanup tests. The main changes include:

  • Updating the ReflectTypeFromJSONName function.
  • Updating the Syft document structure.
  • Adding cleanup tests to ensure proper resource management.
  • Updating dependencies, specifically bumping go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc from 0.42.0 to 0.46.0.
  • Modifying the network policy tests.

PR changes walkthrough

Relevant files                                                                                                                                 
Tests
5 files
cleanup_test.go                                                                                         
    pkg/cleanup/cleanup_test.go

    This file is newly added and contains tests for the cleanup
    task. It includes the creation of a cleanup handler,
    starting the cleanup task, and asserting the expected files
    to delete.
+169/-0
networkpolicy_test.go                                                                             
    pkg/apis/softwarecomposition/networkpolicy/networkpolicy_test.go

    This file contains modifications to the network policy
    tests. The changes include the removal of some test cases
    and the addition of new ones, specifically for handling the
    same ports with different addresses and same ports for pod
    traffic.
+350/-39
discovery_test.go                                                                                     
    pkg/cleanup/discovery_test.go

    This file is newly added and likely contains tests related
    to resource discovery for the cleanup process. It was not
    included in the diff, so the specific changes are not
    detailed.
+25/-0
utils_test.go                                                                                             
    pkg/cleanup/utils_test.go

    This file is newly added and likely contains tests for the
    utility functions used in the cleanup process. It was not
    included in the diff, so the specific changes are not
    detailed.
+27/-0
storage_test.go                                                                                         
    pkg/registry/file/storage_test.go

    This file likely contains modifications related to tests for
    file storage. It was not included in the diff, so the
    specific changes are not detailed.
+4/-4
Enhancement
12 files
cleanup.go                                                                                                   
    pkg/cleanup/cleanup.go

    This file is newly added and contains the main cleanup
    functionality. It was not included in the diff, so the
    specific changes are not detailed.
+211/-0
discovery.go                                                                                               
    pkg/cleanup/discovery.go

    This file is newly added and likely contains functionality
    related to resource discovery for the cleanup process. It
    was not included in the diff, so the specific changes are
    not detailed.
+159/-0
utils.go                                                                                                       
    pkg/cleanup/utils.go

    This file is newly added and likely contains utility
    functions for the cleanup process. It was not included in
    the diff, so the specific changes are not detailed.
+66/-0
zz_generated.openapi.go                                                                         
    pkg/generated/openapi/zz_generated.openapi.go

    This file likely contains generated OpenAPI specifications.
    It was not included in the diff, so the specific changes are
    not detailed.
+1/-105
storage.go                                                                                                   
    pkg/registry/file/storage.go

    This file likely contains modifications related to file
    storage. It was not included in the diff, so the specific
    changes are not detailed.
+17/-17
packagemetadata.go                                                                                   
    pkg/apis/softwarecomposition/packagemetadata/packagemetadata.go

    This file likely contains modifications related to package
    metadata. It was not included in the diff, so the specific
    changes are not detailed.
+104/-6
networkpolicy.go                                                                                       
    pkg/apis/softwarecomposition/networkpolicy/networkpolicy.go

    This file likely contains modifications related to network
    policies. It was not included in the diff, so the specific
    changes are not detailed.
+141/-0
syfttypes.go                                                                                               
    pkg/apis/softwarecomposition/v1beta1/syfttypes.go

    This file likely contains modifications related to Syft
    types. It was not included in the diff, so the specific
    changes are not detailed.
+5/-24
syfttypes.go                                                                                               
    pkg/apis/softwarecomposition/syfttypes.go

    This file likely contains modifications related to Syft
    types. It was not included in the diff, so the specific
    changes are not detailed.
+5/-24
configurationscansummarystorage.go                                                   
    pkg/registry/file/configurationscansummarystorage.go

    This file likely contains modifications related to
    configuration scan summary storage. It was not included in
    the diff, so the specific changes are not detailed.
+3/-3
vulnerabilitysummarystorage.go                                                           
    pkg/registry/file/vulnerabilitysummarystorage.go

    This file likely contains modifications related to
    vulnerability summary storage. It was not included in the
    diff, so the specific changes are not detailed.
+3/-3
main.go                                                                                                         
    main.go

    This file likely contains modifications related to the main
    function of the application. It was not included in the
    diff, so the specific changes are not detailed.
+18/-0

User description

Sorry, we do not accept changes directly against this repository. Please see
CONTRIBUTING.md for information on where and how to contribute instead.

@dwertent dwertent marked this pull request as draft December 14, 2023 13:48
Copy link

Summary:

  • License scan: failure
  • Credentials scan: success
  • Vulnerabilities scan: failure
  • Unit test: success
  • Go linting: success

amirmalka and others added 4 commits December 20, 2023 10:19
Signed-off-by: Amir Malka <[email protected]>
Signed-off-by: Matthias Bertschy <[email protected]>
Signed-off-by: Daniel Grunberger <[email protected]>
Co-authored-by: Daniel Grunberger <[email protected]>
Signed-off-by: Daniel Grunberger <[email protected]>
…golang.org/grpc/otelgrpc

Bumps [go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc](https://github.com/open-telemetry/opentelemetry-go-contrib) from 0.42.0 to 0.46.0.
- [Release notes](https://github.com/open-telemetry/opentelemetry-go-contrib/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go-contrib@zpages/v0.42.0...zpages/v0.46.0)

---
updated-dependencies:
- dependency-name: go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Matthias Bertschy <[email protected]>
@dwertent dwertent marked this pull request as ready for review December 20, 2023 08:34
@codiumai-pr-agent-free codiumai-pr-agent-free bot added enhancement New feature or request Tests labels Dec 20, 2023
Copy link

PR Description updated to latest commit (5323a41)

Copy link

PR Analysis

  • 🎯 Main theme: Enhancement and testing of the Syft format in the Kubescape project
  • 📝 PR summary: This PR primarily focuses on enhancing the Syft format and adding cleanup tests in the Kubescape project. It includes updates to the ReflectTypeFromJSONName function, the Syft document structure, and dependencies. It also adds cleanup tests to ensure proper resource management and modifies the network policy tests.
  • 📌 Type of PR: Enhancement
  • 🧪 Relevant tests added: Yes
  • ⏱️ Estimated effort to review [1-5]: 4, because the PR involves changes in multiple files and includes both enhancements and tests. The changes are spread across different functionalities, making the review process more complex.
  • 🔒 Security concerns: No security concerns found

PR Feedback

  • 💡 General suggestions: The PR seems to be well-structured and the changes are logically grouped. The addition of cleanup tests is a good practice to ensure proper resource management. However, it would be beneficial to provide more context or comments in the code to explain the logic, especially for complex functions. This would make the code more maintainable and easier for other developers to understand.

  • 🤖 Code feedback:
    relevant filepkg/cleanup/cleanup_test.go
    suggestion      Consider breaking down the TestCleanupTask function into smaller, more manageable functions. This can improve readability and maintainability of the code. [important]
    relevant linefunc TestCleanupTask(t *testing.T) {

    relevant filepkg/cleanup/cleanup.go
    suggestion      The StartCleanupTask function seems to be doing a lot of work. It could be beneficial to break it down into smaller functions. This would improve the readability and maintainability of the code. [important]
    relevant linefunc (h *ResourcesCleanupHandler) StartCleanupTask() {

    relevant filepkg/cleanup/cleanup.go
    suggestion      It's a good practice to handle errors as close as possible to where they occur. Consider handling the error returned by the afero.Walk function immediately instead of logging it and continuing the execution. [medium]
    relevant lineerr := afero.Walk(h.appFs, v1beta1ApiVersionPath, func(path string, info os.FileInfo, err error) error {

    relevant filepkg/registry/file/generatednetworkpolicy.go
    suggestion      It's a good practice to use constants for repeated string values. Consider defining a constant for the API version string. [medium]
    relevant lineAPIVersion: StorageV1Beta1ApiVersion,

How to use

Instructions

To invoke the PR-Agent, add a comment using one of the following commands:
/review: Request a review of your Pull Request.
/describe: Update the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback.
/ask <QUESTION>: Ask a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.
/add_docs: Generate docstring for new components introduced in the PR.
/generate_labels: Generate labels for the PR based on the PR's contents.
see the tools guide for more details.

To edit any configuration parameter from the configuration.toml, add --config_path=new_value.
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, add a /config comment.

Copy link

Summary:

  • License scan: failure
  • Credentials scan: success
  • Vulnerabilities scan: failure
  • Unit test: success
  • Go linting: success

@matthyx
Copy link
Contributor

matthyx commented Dec 20, 2023

@dwertent can you check your commits, I see merge commits there

@dwertent
Copy link
Author

@matthyx yes, I had some conflicts I needed to solve.

@matthyx
Copy link
Contributor

matthyx commented Dec 20, 2023

@matthyx yes, I had some conflicts I needed to solve.

so please rebase or squash, this is messy

@dwertent dwertent merged commit 4fedfe4 into main Dec 24, 2023
6 checks passed
Copy link

Summary:

  • License scan: failure
  • Credentials scan: success
  • Vulnerabilities scan: failure
  • Unit test: success
  • Go linting: success

@dwertent dwertent deleted the feat-syft-format branch December 24, 2023 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants