Revert some types from "remove summary objects"

[matthyx]

User description

This partly reverts commit 7461b46.

---

Type

enhancement

---

Description

• Implements storage logic for ConfigurationScanSummary and VulnerabilitySummary with custom business logic.
• Adds unit tests for new storage implementations.
• Generates clientset, conversion, and deepcopy methods for VulnerabilityManifestSummary and related types.
• Enhances informers with interfaces for VulnerabilityManifestSummary and VulnerabilitySummary.

---

Changes walkthrough

	Relevant files
Tests	2 files configurationscansummarystorage_test.go Add Unit Tests for ConfigurationScanSummaryStorage              pkg/registry/file/configurationscansummarystorage_test.go Adds unit tests for ConfigurationScanSummaryStorage methods. Tests cover Create, Delete, Watch, GuaranteedUpdate, Count, Get, and GetList methods. Validates expected errors for unsupported operations. Checks correct behavior for Get and GetList operations. +735/-0  vulnerabilitysummarystorage_test.go Add Unit Tests for VulnerabilitySummaryStorage                      pkg/registry/file/vulnerabilitysummarystorage_test.go Adds unit tests for VulnerabilitySummaryStorage methods. Tests cover Create, Delete, Watch, GuaranteedUpdate, Count, Get, and GetList methods. Validates expected errors for unsupported operations. Checks correct behavior for Get and GetList operations. +453/-0
Enhancement	7 files zz_generated.conversion.go Generate Conversion Functions for VulnerabilityManifestSummary pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go Adds generated conversion functions for VulnerabilityManifestSummary and related types. Supports conversion between internal and v1beta1 versions of VulnerabilityManifestSummary. +252/-0  vulnerabilitymanifestsummary.go Add Clientset for VulnerabilityManifestSummary Resources  pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/vulnerabilitymanifestsummary.go Adds clientset methods for VulnerabilityManifestSummary resources. Supports operations like Create, Update, Delete, Get, List, and Watch. +195/-0  vulnerabilitysummary.go Add Clientset for VulnerabilitySummary Resources                  pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1/vulnerabilitysummary.go Adds clientset methods for VulnerabilitySummary resources. Supports operations like Create, Update, Delete, Get, List, and Watch. +195/-0  configurationscansummarystorage.go Implement ConfigurationScanSummaryStorage with Custom Logic pkg/registry/file/configurationscansummarystorage.go Implements ConfigurationScanSummaryStorage with custom business logic. Provides methods for unsupported operations returning errors. Implements Get and GetList methods to generate summaries on the fly. +215/-0  vulnerabilitysummarystorage.go Implement VulnerabilitySummaryStorage with Custom Logic    pkg/registry/file/vulnerabilitysummarystorage.go Implements VulnerabilitySummaryStorage with custom business logic. Provides methods for unsupported operations returning errors. Implements Get and GetList methods to generate summaries on the fly. +200/-0  zz_generated.deepcopy.go Generate DeepCopy Methods for VulnerabilityManifestSummary Types pkg/apis/softwarecomposition/v1beta1/zz_generated.deepcopy.go Adds deep copy methods for VulnerabilityManifestSummary and related types. Supports deep copying of VulnerabilityManifestSummary, VulnerabilityManifestSummaryList, and VulnerabilityManifestSummarySpec. +178/-0  interface.go Add Informer Interfaces for New Vulnerability Summary Types pkg/generated/informers/externalversions/softwarecomposition/v1beta1/interface.go Adds informer interfaces for VulnerabilityManifestSummary and VulnerabilitySummary. Enables watching and listing of these resources in informers. +14/-0

---

✨ PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-free]

PR Description updated to latest commit (7531312)

[codiumai-pr-agent-free]

PR Review

⏱️ Estimated effort to review [1-5]	5, due to the extensive changes across multiple files, including the addition of new APIs, tests, and storage logic implementations. The PR introduces significant functionality, affecting core components and necessitating a thorough review of code quality, design patterns, and potential impacts on existing functionalities.
🧪 Relevant tests	Yes
🔍 Possible issues	Possible Bug: Lack of validation for new API objects might lead to issues. It's crucial to ensure that the newly introduced API objects (VulnerabilityManifestSummary, VulnerabilitySummary, etc.) have proper validation mechanisms in place to prevent invalid data from being accepted.
	Performance Concern: The on-the-fly generation of ConfigurationScanSummary and VulnerabilitySummary objects could impact performance, especially in large clusters. It's important to assess the performance implications of these operations and consider caching or other optimization strategies.
	Data Consistency: The reliance on aggregating existing stored objects to generate summaries could lead to data consistency issues, particularly in dynamic environments where the underlying data changes frequently. Ensuring data consistency and timely updates to the generated summaries is essential.
🔒 Security concerns	No direct security concerns identified in the PR code diff. However, it's important to ensure that access to the new APIs (VulnerabilityManifestSummary, VulnerabilitySummary, etc.) is properly secured and that RBAC policies are correctly applied to prevent unauthorized access.

---

✨ Review tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[codiumai-pr-agent-free]

PR Code Suggestions

Category	Suggestions
Possible issue	Replace unsafe pointer conversions with safe manual iterations for slice conversions. Consider handling potential errors when converting between VulnerabilityManifestSummaryList and softwarecomposition.VulnerabilityManifestSummaryList using unsafe.Pointer. This direct memory access can lead to runtime panics if the underlying types are not fully compatible or if the slice headers change in future Go versions. A safer approach would be to manually iterate over the slice and convert each item individually, ensuring type safety and error handling. pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go [5835] -out.Items = *(*[]softwarecomposition.VulnerabilityManifestSummary)(unsafe.Pointer(&in.Items)) +out.Items = make([]softwarecomposition.VulnerabilityManifestSummary, len(in.Items)) +for i, item := range in.Items { + if err := Convert_v1beta1_VulnerabilityManifestSummary_To_softwarecomposition_VulnerabilityManifestSummary(&item, &out.Items[i], s); err != nil { + return err + } +}
	Use manual iteration for slice conversion to ensure type safety and error handling. Similar to the previous suggestion, consider handling potential errors when converting between VulnerabilitySummaryList and softwarecomposition.VulnerabilitySummaryList using unsafe.Pointer. Implement a safer approach by manually iterating over the slice and converting each item individually to ensure type safety and proper error handling. pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go [5975] -out.Items = *(*[]softwarecomposition.VulnerabilitySummary)(unsafe.Pointer(&in.Items)) +out.Items = make([]softwarecomposition.VulnerabilitySummary, len(in.Items)) +for i, item := range in.Items { + if err := Convert_v1beta1_VulnerabilitySummary_To_softwarecomposition_VulnerabilitySummary(&item, &out.Items[i], s); err != nil { + return err + } +}
	Ensure type safety in slice conversions by avoiding direct unsafe.Pointer usage. When converting VulnerabilitySummarySpec to softwarecomposition.VulnerabilitySummarySpec, the direct conversion of WorkloadVulnerabilitiesObj using unsafe.Pointer is risky and could lead to runtime issues. Instead, consider implementing a type-safe conversion function that properly converts between the types, ensuring the integrity and safety of the data. pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go [5999] -out.WorkloadVulnerabilitiesObj = *(*[]softwarecomposition.VulnerabilitiesObjScope)(unsafe.Pointer(&in.WorkloadVulnerabilitiesObj)) +// Assuming a conversion function exists for VulnerabilitiesObjScope to its softwarecomposition counterpart +out.WorkloadVulnerabilitiesObj = make([]softwarecomposition.VulnerabilitiesObjScope, len(in.WorkloadVulnerabilitiesObj)) +for i, obj := range in.WorkloadVulnerabilitiesObj { + if err := Convert_v1beta1_VulnerabilitiesObjScope_To_softwarecomposition_VulnerabilitiesObjScope(&obj, &out.WorkloadVulnerabilitiesObj[i], s); err != nil { + return err + } +}
Maintainability	Implement or document the reason for no-op conversion functions. For the conversion functions that currently have no implementation and simply return nil, it's important to either implement the necessary conversion logic or, if the conversion is intentionally a no-op, document the reason. This ensures future maintainers understand the decision and can assess if changes are needed as the code evolves. pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go [6022] -return nil +// Implement conversion logic here or document why a no-op is intentional.
	Organize tests using the table-driven pattern for better readability. To improve test readability and maintenance, consider using table-driven tests with subtests for different scenarios. This approach can make it easier to add new test cases in the future and keep the test code organized. pkg/registry/file/vulnerabilitysummarystorage_test.go [24-38] -tests := []struct { - name string - readonly bool - args args - wantErr bool - want runtime.Object -}{ - { - name: "not supported", - args: args{ - key: "/spdx.softwarecomposition.kubescape.io/vulnerabilitysummaries/kubescape/toto", - obj: &v1beta1.VulnerabilitySummary{}, - }, - wantErr: true, - }, -} +// This code already follows the table-driven test pattern. Consider organizing different types of tests into separate functions or files for better readability.
	Add a TODO comment to remind replacing context.TODO(). To ensure that the context.TODO() is appropriately replaced in the future with a real context, consider adding a TODO comment. This can help to remind developers to replace the placeholder with a more specific context tailored to the operation's requirements. pkg/registry/file/vulnerabilitysummarystorage_test.go [45] +// TODO: Replace context.TODO() with a more specific context as needed. err := s.Create(context.TODO(), tt.args.key, tt.args.obj, tt.args.out, tt.args.in4)
	Define a constant for repeated string paths. To avoid magic strings and improve code maintainability, consider defining a constant for the repeated string "/spdx.softwarecomposition.kubescape.io/vulnerabilitysummaries/kubescape/toto". This makes the code cleaner and easier to update if the base path changes. pkg/registry/file/vulnerabilitysummarystorage_test.go [34] -key: "/spdx.softwarecomposition.kubescape.io/vulnerabilitysummaries/kubescape/toto", +const basePath = "/spdx.softwarecomposition.kubescape.io/vulnerabilitysummaries/kubescape/toto" +key: basePath,
	Define property schemas outside of map initialization for clarity. For the Properties map in the SchemaProps, consider defining the property schemas outside of the map initialization for better readability and maintainability. This approach allows for easier modifications and clearer structure. pkg/generated/openapi/zz_generated.openapi.go [7116-7122] +kindSchema := spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds", + Type: []string{"string"}, + Format: "", + }, +} Properties: map[string]spec.Schema{ - "kind": { - SchemaProps: spec.SchemaProps{ - Description: "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds", - Type: []string{"string"}, - Format: "", - }, - }, + "kind": kindSchema,
	Create a helper function for common spec.SchemaProps fields. To avoid repetition and improve maintainability, consider creating a helper function for creating spec.SchemaProps with common fields like Description, Type, and Format. This function can then be reused across different schema definitions. pkg/generated/openapi/zz_generated.openapi.go [7117-7122] +func createSchemaProps(description, typeStr, formatStr string) spec.SchemaProps { + return spec.SchemaProps{ + Description: description, + Type: []string{typeStr}, + Format: formatStr, + } +} "kind": { - SchemaProps: spec.SchemaProps{ - Description: "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds", - Type: []string{"string"}, - Format: "", - }, + SchemaProps: createSchemaProps("Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds", "string", ""), },
	Define common dependencies as a constant slice for reuse. For the Dependencies slice, consider defining a constant slice for common dependencies that are reused across different schema definitions to reduce duplication and improve maintainability. pkg/generated/openapi/zz_generated.openapi.go [7152-7153] -Dependencies: []string{ - "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1.VulnerabilityManifestStatus", "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1.VulnerabilityManifestSummarySpec", "k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta"}, +var commonDependencies = []string{ + "k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta", +} +Dependencies: append(commonDependencies, "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1.VulnerabilityManifestStatus", "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1.VulnerabilityManifestSummarySpec"),
Best practice	Ensure comprehensive field conversion in struct conversion functions. The conversion functions between VulnerabilityManifestSummarySpec and softwarecomposition.VulnerabilityManifestSummarySpec should ensure all fields are converted, including newly added fields in future versions. It's a good practice to review these functions periodically or when the structs are updated to ensure all relevant fields are included in the conversion. pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go [5856-5860] +// Ensure all fields are converted, including any that may be added in the future. if err := Convert_v1beta1_SeveritySummary_To_softwarecomposition_SeveritySummary(&in.Severities, &out.Severities, s); err != nil { return err } if err := Convert_v1beta1_VulnerabilitiesComponents_To_softwarecomposition_VulnerabilitiesComponents(&in.Vulnerabilities, &out.Vulnerabilities, s); err != nil { return err } +// Add conversion for any new fields here.
	Use assert.IsType for more accurate error type checking. Instead of using assert.Equal for error checking, consider using assert.IsType to ensure the error type is as expected. This provides a more accurate test condition by verifying the specific type of error returned, rather than just comparing error messages. pkg/registry/file/vulnerabilitysummarystorage_test.go [48] -assert.Equal(t, err, storage.NewInvalidObjError(tt.args.key, operationNotSupportedMsg)) +assert.IsType(t, &storage.InvalidObjError{}, err)
	Create a new realStorage instance inside each subtest for isolation. For better test isolation and to avoid potential side effects, consider creating a new instance of realStorage inside each subtest rather than sharing a single instance across multiple tests. pkg/registry/file/vulnerabilitysummarystorage_test.go [40] +// Inside each subtest realStorage := NewStorageImpl(afero.NewMemMapFs(), "/")
	Use more descriptive variable names. Consider using a more descriptive variable name instead of ref for the common.ReferenceCallback parameter to improve code readability. The current name ref is too generic and does not convey the purpose or usage of the parameter within the function. pkg/generated/openapi/zz_generated.openapi.go [7110] -func schema_pkg_apis_softwarecomposition_v1beta1_VulnerabilityManifestSummary(ref common.ReferenceCallback) common.OpenAPIDefinition { +func schema_pkg_apis_softwarecomposition_v1beta1_VulnerabilityManifestSummary(referenceCallback common.ReferenceCallback) common.OpenAPIDefinition {
	Review the use of Default field in SchemaProps. Ensure that the Default field in SchemaProps is appropriately used. Setting an empty map as default for fields like metadata may not be necessary if these fields are expected to be populated with specific values. Consider removing the default or ensuring it aligns with the intended use. pkg/generated/openapi/zz_generated.openapi.go [7131-7135] "metadata": { SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta"), + Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta"), }, },
	Replace hardcoded strings with constants for better maintainability. For better maintainability and to avoid magic strings, consider defining "WorkloadConfigurationScanSummary" as a constant instead of hardcoding it. This makes it easier to manage and change the value in the future if necessary. pkg/registry/file/configurationscansummarystorage.go [206] -Kind: "WorkloadConfigurationScanSummary", +Kind: WorkloadConfigurationScanSummaryKind,
Bug	Check for nil directly on the pointer to avoid incorrect behavior. Consider checking for nil directly on the pointer workloadScanSummaryListObjPtr instead of taking its address. This avoids a common mistake where checking the address of a variable, which will never be nil, leads to incorrect behavior. pkg/registry/file/configurationscansummarystorage.go [73] -if &workloadScanSummaryListObjPtr == nil { +if workloadScanSummaryListObjPtr == nil {
Performance	Use a buffer pool for JSON operations to improve performance. To improve performance and reduce memory allocations, consider using a buffer pool for JSON marshaling and unmarshaling operations. This can be particularly beneficial in high-throughput scenarios. pkg/registry/file/configurationscansummarystorage.go [83-91] -data, err := json.Marshal(configurationScanSummaryObj) -if err = json.Unmarshal(data, objPtr); err != nil { +var buf bytes.Buffer +enc := json.NewEncoder(&buf) +if err := enc.Encode(configurationScanSummaryObj); err != nil { + return err +} +dec := json.NewDecoder(&buf) +if err := dec.Decode(objPtr); err != nil { + return err +}
Enhancement	Simplify slice appending in the loop for cleaner code. Instead of manually appending items to the slice inside the loop, consider using the append function with a slice argument to append all items at once. This can make the code cleaner and potentially improve performance by reducing the number of allocations. pkg/registry/file/configurationscansummarystorage.go [152-156] for _, wlSummary := range wlConfigurationScanSummaryList.Items { - if _, ok := mapNamespaceToSummaries[wlSummary.Namespace]; !ok { - mapNamespaceToSummaries[wlSummary.Namespace] = make([]softwarecomposition.WorkloadConfigurationScanSummary, 0) - } mapNamespaceToSummaries[wlSummary.Namespace] = append(mapNamespaceToSummaries[wlSummary.Namespace], wlSummary) }
	Use more descriptive error messages for JSON operations. Consider using a more descriptive error message when logging JSON marshal and unmarshal failures. Including the type of the object that failed to marshal or unmarshal can help in debugging. pkg/registry/file/configurationscansummarystorage.go [85-91] -logger.L().Ctx(ctx).Error("json marshal failed", helpers.Error(err), helpers.String("key", key)) -logger.L().Ctx(ctx).Error("json unmarshal failed", helpers.Error(err), helpers.String("key", key)) +logger.L().Ctx(ctx).Error("json marshal failed for ConfigurationScanSummary", helpers.Error(err), helpers.String("key", key)) +logger.L().Ctx(ctx).Error("json unmarshal failed for ConfigurationScanSummary", helpers.Error(err), helpers.String("key", key))

---

✨ Improve tool usage guide:

---

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

[github-actions]

Summary:

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