reject updates on complete app profile, remove omitempty

[amirmalka]

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]

PR Description updated to latest commit (53e2738)

[codiumai-pr-agent-free]

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 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
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] -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"`
	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] 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] -Required: []string{"capabilities", "execs", "opens", "syscalls"} +Required: []string{"name", "capabilities", "execs", "opens", "syscalls"}
	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] -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 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: success