Closed dwertent closed 6 months ago
Summary:
PR Description updated to latest commit (https://github.com/kubescape/storage/commit/5323a413f2ddf649621e5a30ec616ae2d9fe8383)
π― 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
π‘ 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.
relevant file | pkg/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 line | func TestCleanupTask(t *testing.T) { |
relevant file | pkg/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 line | func (h *ResourcesCleanupHandler) StartCleanupTask() { |
relevant file | pkg/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 line | err := afero.Walk(h.appFs, v1beta1ApiVersionPath, func(path string, info os.FileInfo, err error) error { |
relevant file | pkg/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 line | APIVersion: StorageV1Beta1ApiVersion, |
Summary:
@dwertent can you check your commits, I see merge commits there
@matthyx yes, I had some conflicts I needed to solve.
@matthyx yes, I had some conflicts I needed to solve.
so please rebase or squash, this is messy
Summary:
Type
Enhancement, Tests
Description
This PR primarily focuses on enhancing the Syft format and adding cleanup tests. The main changes include:
PR changes walkthrough
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.