reject updates on complete app profile, remove omitempty

amirmalka commented 5 months ago

User description

Type

enhancement, bug_fix

Description

Removed 'omitempty' from several fields in ApplicationProfileContainer to ensure they are always serialized.
Updated OpenAPI schema to mark 'capabilities', 'execs', 'opens', 'syscalls' as required.
Added logic to reject updates on ApplicationProfiles that are marked as complete and completed.
Enhanced test coverage for update scenarios of ApplicationProfiles.

Changes walkthrough

Relevant files

Enhancement

types.go `Ensure Required Fields are Always Serialized` pkg/apis/softwarecomposition/v1beta1/types.go Removed 'omitempty' from JSON tags for 'Capabilities', 'Execs', 'Opens', and 'Syscalls' in ApplicationProfileContainer. Ensured these fields are always present in the JSON output.	+4/-4
zz_generated.openapi.go `Update OpenAPI Schema to Reflect Required Fields` pkg/generated/openapi/zz_generated.openapi.go Added 'capabilities', 'execs', 'opens', 'syscalls' to the list of required fields in the OpenAPI schema.	+1/-0
strategy.go `Reject Updates on Completed Application Profiles` pkg/registry/softwarecomposition/applicationprofile/strategy.go Added logging and conditions to reject updates to ApplicationProfiles marked as complete and completed. Prevent status transition from 'complete' to 'partial' for ApplicationProfiles.	+15/-0

Tests

strategy_test.go

Extend Tests for ApplicationProfile Update Logic

pkg/registry/softwarecomposition/applicationprofile/strategy_test.go

Expanded tests to cover rejection of updates on completed
ApplicationProfiles.

Added new test scenarios for handling updates on ApplicationProfiles.

+177/-1

✨ 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[bot] commented 5 months ago

PR Description updated to latest commit (https://github.com/kubescape/storage/commit/53e2738ebef0c46f4db6da7a8e71b75fe681e803)

codiumai-pr-agent-free[bot] commented 5 months ago

PR Review

⏱️ Estimated effort to review [1-5]	3, because the PR involves changes across multiple files including Go structs, OpenAPI specifications, and update strategies which require a good understanding of the existing codebase and the implications of the changes on the system's behavior.
🧪 Relevant tests	Yes
🔍 Possible issues	Possible Bug: The logic to prevent updates on completed application profiles might not handle cases where annotations are missing or malformed. It assumes the presence and correctness of specific annotation keys and values.
🔒 Security concerns	No

✨ 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](https://pr-agent-docs.codium.ai/tools/review/#general-configurations) by configuring the tool. The tool can be triggered [automatically](https://pr-agent-docs.codium.ai/usage-guide/automations_and_usage/#github-app-automatic-tools-when-a-new-pr-is-opened) every time a new PR is opened, or can be invoked manually by commenting on any PR. - When commenting, to edit [configurations](https://github.com/Codium-ai/pr-agent/blob/main/pr_agent/settings/configuration.toml#L23) 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](https://pr-agent-docs.codium.ai/usage-guide/configuration_options/), use the following template: ``` [pr_reviewer] some_config1=... some_config2=... ``` See the review [usage page](https://pr-agent-docs.codium.ai/tools/review/) for a comprehensive guide on using this tool.

codiumai-pr-agent-free[bot] commented 5 months ago

PR Code Suggestions

Category	Suggestions
Enhancement	Add the `omitempty` option to JSON tags to omit empty fields in JSON output. ___ Consider adding the `omitempty` JSON tag option to the `Capabilities`, `Execs`, `Opens`, and `Syscalls` fields in the `ApplicationProfileContainer` struct to allow these fields to be omitted from the JSON output if they are empty. This can reduce the output size and avoid sending empty arrays or objects in JSON, which is particularly useful in APIs where optional fields may not always have data. [pkg/apis/softwarecomposition/v1beta1/types.go [258-265]](https://github.com/kubescape/storage/pull/110/files#diff-c5af80b7e6424e8b4a947dfd8a99a11c5374bc01d4874e3765fb25205fa42158R258-R265) ```diff -Capabilities []string `json:"capabilities"` -Execs []ExecCalls `json:"execs" patchStrategy:"merge" patchMergeKey:"path"` -Opens []OpenCalls `json:"opens" patchStrategy:"merge" patchMergeKey:"path"` -Syscalls []string `json:"syscalls"` +Capabilities []string `json:"capabilities,omitempty"` +Execs []ExecCalls `json:"execs,omitempty" patchStrategy:"merge" patchMergeKey:"path"` +Opens []OpenCalls `json:"opens,omitempty" patchStrategy:"merge" patchMergeKey:"path"` +Syscalls []string `json:"syscalls,omitempty"` ```
Enhancement	Implement error handling when rejecting updates for completed application profiles. ___ When rejecting updates due to the application profile being marked as complete and completed, consider adding an error or exception handling mechanism instead of just logging and resetting the object. This could involve throwing a custom error that can be caught and handled appropriately by the calling function, providing clearer feedback to the user or system about why the update was rejected. [pkg/registry/softwarecomposition/applicationprofile/strategy.go [69-72]](https://github.com/kubescape/storage/pull/110/files#diff-31c3884639bb3a8bb6f8f2968b8573d46eeeb993e9e5579aa8a835c030402a01R69-R72) ```diff logger.L().Debug("application profile is marked as complete and completed, rejecting update", logHelpers.String("name", oldAP.Name), logHelpers.String("namespace", oldAP.Namespace)) -newAP = oldAP // reset the new object to the old object +return fmt.Errorf("update rejected: application profile is marked as complete and completed") ```
Best practice	Update the `Required` fields in the OpenAPI schema to include all mandatory fields. ___ Ensure that the `Required` field in the OpenAPI schema definition includes all necessary fields. If there are other fields in `ApplicationProfileContainer` that are essential for the API operation, they should also be listed in the `Required` array. This ensures that the API consumers are aware of all mandatory fields when interacting with the API. [pkg/generated/openapi/zz_generated.openapi.go [620]](https://github.com/kubescape/storage/pull/110/files#diff-9f4c1466e676f9e733cae72d369ffd5ff37f446116c98562807cf3bfb872ae95R620-R620) ```diff -Required: []string{"capabilities", "execs", "opens", "syscalls"} +Required: []string{"name", "capabilities", "execs", "opens", "syscalls"} ```
Best practice	Enhance test assertions to check the entire object state when updates are rejected. ___ In the test case for rejecting updates when the application profile is marked as complete and completed, ensure that the test asserts not just the state of annotations but also the overall state of the object to confirm that no unintended changes have occurred. This can be done by deep comparing the entire object state, not just the annotations. [pkg/registry/softwarecomposition/applicationprofile/strategy_test.go [87-91]](https://github.com/kubescape/storage/pull/110/files#diff-50d29e8cedb48c7fb1afb5410d84712023b460583d4154c13415d202b429624dR87-R91) ```diff -expected: map[string]string{ - helpers.CompletionMetadataKey: "complete", - helpers.StatusMetadataKey: "completed", +expected: &softwarecomposition.ApplicationProfile{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + helpers.CompletionMetadataKey: "complete", + helpers.StatusMetadataKey: "completed", + }, + }, + // Include checks for other fields to ensure no changes } ```

✨ 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](https://pr-agent-docs.codium.ai/usage-guide/automations_and_usage/#github-app-automatic-tools-when-a-new-pr-is-opened) every time a new PR is opened, or can be invoked manually by commenting on a PR. - When commenting, to edit [configurations](https://github.com/Codium-ai/pr-agent/blob/main/pr_agent/settings/configuration.toml#L78) 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](https://pr-agent-docs.codium.ai/usage-guide/configuration_options/), use the following template: ``` [pr_code_suggestions] some_config1=... some_config2=... ``` See the improve [usage page](https://pr-agent-docs.codium.ai/tools/improve/) for a comprehensive guide on using this tool.

github-actions[bot] commented 5 months ago

Summary:

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

kubescape / storage