status update checks

[amirmalka]

User description

---

Type

enhancement, bug_fix

---

Description

• Introduced validation and rejection logic for status updates in both ApplicationProfile and NetworkNeighbors to prevent invalid status transitions.
• Added utility functions in pkg/utils/validations.go for validating completion and status annotations with predefined valid values.

---

Changes walkthrough

Relevant files

Enhancement

strategy.go Enhance ApplicationProfile Status Update Validation            pkg/registry/softwarecomposition/applicationprofile/strategy.go Added validation and rejection logic for status updates in ApplicationProfile. Prevents status transition from 'complete' to 'partial'. Validates completion and status annotations during creation and update. +37/-2    strategy.go Implement NetworkNeighbors Status Update Validation            pkg/registry/softwarecomposition/networkneighbors/strategy.go Implemented validation and rejection logic for status updates in NetworkNeighbors. Blocks status transition from 'complete' to 'partial'. Validates completion and status annotations during creation and update. +38/-3    validations.go Add Utility Functions for Annotations Validation                  pkg/utils/validations.go Introduced utility functions for validating completion and status annotations. Supports a set of predefined valid values for each annotation. +32/-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 (9c8ab02)

[codiumai-pr-agent-free]

PR Review

⏱️ Estimated effort to review [1-5]	2, because the changes are mostly focused on adding validation logic and annotations handling, which are straightforward to review. The logic is not deeply complex but requires understanding of the existing annotation and status management system.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: The validation logic in both PrepareForUpdate methods (for ApplicationProfile and NetworkNeighbors) directly modifies the annotations of the new object to match the old object if a transition from 'complete' to 'partial' is attempted. This approach might not be thread-safe and could lead to race conditions in a concurrent environment. Consider using a more robust method to handle this validation and rejection logic.
	Inconsistency: The validation functions ValidateCompletionAnnotation and ValidateStatusAnnotation in pkg/utils/validations.go return a single error. In scenarios where multiple annotations are invalid, it would only report the first encountered error. This might not provide full visibility into all issues with the annotations to the caller. Consider aggregating all errors before returning.
🔒 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
Maintainability	Refactor annotation checks and settings into a utility function for better code reuse. Instead of manually checking and setting the annotations for completion and status in PrepareForUpdate, consider creating a utility function that encapsulates this logic. This will improve code reuse and maintainability. pkg/registry/softwarecomposition/applicationprofile/strategy.go [67-69] -if oldAP.Annotations[helpers.CompletionMetadataKey] == helpers.Complete && newAP.Annotations[helpers.CompletionMetadataKey] == helpers.Partial { - newAP.Annotations[helpers.CompletionMetadataKey] = helpers.Complete - newAP.Annotations[helpers.StatusMetadataKey] = oldAP.Annotations[helpers.StatusMetadataKey] -} +utils.EnforceCompletionAndStatusAnnotations(oldAP.Annotations, newAP.Annotations)
	Use a loop to append validation errors for improved readability. To improve code readability and maintainability, consider using a loop to append errors from validation functions in Validate and ValidateUpdate methods, similar to the previous suggestion. pkg/registry/softwarecomposition/networkneighbors/strategy.go [75-80] -if err := utils.ValidateCompletionAnnotation(nn.Annotations); err != nil { - allErrors = append(allErrors, err) +validationFuncs := []func(map[string]string) *field.Error{ + utils.ValidateCompletionAnnotation, + utils.ValidateStatusAnnotation, } +for _, validate := range validationFuncs { + allErrors = append(allErrors, validate(nn.Annotations)) +} +// Filter out nil errors before returning +var validErrors field.ErrorList +for _, err := range allErrors { + if err != nil { + validErrors = append(validErrors, err) + } +} +return validErrors
Bug	Add nil check for annotations map to prevent potential nil pointer dereferences. Consider adding error handling for the case where the annotations map is nil to prevent potential nil pointer dereferences in ValidateCompletionAnnotation and ValidateStatusAnnotation. pkg/utils/validations.go [9] +if annotations == nil { + return field.Required(field.NewPath("metadata").Child("annotations"), "annotations cannot be nil") +} if v, ok := annotations[helpers.CompletionMetadataKey]; ok {
Enhancement	Aggregate validation errors for comprehensive feedback. The error handling in Validate and ValidateUpdate methods could be enhanced by aggregating errors instead of returning immediately upon encountering the first error. This provides a more comprehensive validation feedback. pkg/registry/softwarecomposition/applicationprofile/strategy.go [78-83] -if err := utils.ValidateCompletionAnnotation(ap.Annotations); err != nil { - allErrors = append(allErrors, err) +allErrors = append(allErrors, utils.ValidateCompletionAnnotation(ap.Annotations)) +allErrors = append(allErrors, utils.ValidateStatusAnnotation(ap.Annotations)) +// Filter out nil errors before returning +var validErrors field.ErrorList +for _, err := range allErrors { + if err != nil { + validErrors = append(validErrors, err) + } } +return validErrors

---

✨ 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

[matthyx]

@amirmalka can you check you test failures?

[github-actions]

Summary:

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

[matthyx]

some nits in tests

[matthyx]

thanks!

[github-actions]

Summary:

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