✨ feature: Add Ignore Fields selector options and error reason

[ncr38]

Summary

This PR is aimed to add more advanced field selector options like JSONPointer and JQPathExpressions to the ignoreField config in case of ServerSideApply strategy.

Related Enhancement Proposal(s)

open-cluster-management-io/enhancements#173

Summary by CodeRabbit

• New Features
  • Added additional ways to specify ignore patterns (JSON Pointers and jq expressions) and made the existing JSONPath option optional for more flexible ignore rules.
  • Added new manifest status reasons to indicate applied manifest completion, failure, and ignore-field errors for clearer outcome visibility.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ncr38
Once this PR has been reviewed and has the lgtm label, please assign deads2k for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Walkthrough

IgnoreField gained two optional path-specifier fields (JSONPointers, JQPathExpressions) and JSONPaths became optional; CRD schemas were updated accordingly. deepcopy logic was extended to copy new fields. Three new applied-condition constants were added: AppliedManifestComplete, AppliedManifestFailed, AppliedManifestSSAIgnoreFieldError.

Changes

Cohort / File(s)	Summary
IgnoreField type and applied-condition constants work/v1/types.go	Made JSONPaths optional (omitempty); added JSONPointers []string and JQPathExpressions []string to IgnoreField; added AppliedManifestComplete, AppliedManifestFailed, and AppliedManifestSSAIgnoreFieldError constants and a new applied-condition comment grouping.
Deepcopy updates work/v1/zz_generated.deepcopy.go	Extended deepcopy to allocate and copy JSONPointers and JQPathExpressions in addition to JSONPaths.
CRD: AddOnTemplate addon/v1alpha1/0000_03_addon.open-cluster-management.io_addontemplates.crd.yaml	Added jqPathExpressions and jsonPointers arrays to spec.agentsSpec.deleteOption.ignoreFields; removed jsonPaths from the required list; adjusted descriptions.
CRD: ManifestWork (work/v1) work/v1/0000_00_work.open-cluster-management.io_manifestworks.crd.yaml	Added jqPathExpressions and jsonPointers arrays alongside jsonPaths; removed minItems and required constraint on jsonPaths; updated descriptions to reference JSONPath and RFC 6901 where applicable.
CRD: ManifestWorkReplicaSet (work/v1alpha1) work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml	Added jqPathExpressions and jsonPointers to condition/deleteOption.ignoreFields; removed minItems and jsonPaths from required; updated descriptions and wording.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The description provides a summary of the feature and references the related enhancement proposal, but lacks detail about the error reason mentioned in the title and omits the Related issue(s) section from the template.	Clarify what the error reason feature entails and include the Related issue(s) section (e.g., Fixes # or References #) to complete the template structure.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: adding new ignore field selector options (JSONPointer, JQPathExpressions) and error reason constants to support ServerSideApply strategy.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (3)

work/v1/0000_00_work.open-cluster-management.io_manifestworks.crd.yaml (1)

340-366: Consider adding a validation rule requiring at least one path field to be non-empty.

With jsonPaths no longer required and no constraint on the new fields, a user can create an ignoreFields entry with only condition set and all three path arrays empty/absent. This would be a valid but meaningless no-op entry. Consider adding an x-kubernetes-validations rule like:

x-kubernetes-validations:
- message: "At least one of jsonPaths, jsonPointers, or jqPathExpressions must be specified"
  rule: "has(self.jsonPaths) || has(self.jsonPointers) || has(self.jqPathExpressions)"

This would apply to the ignoreFields item object (around Line 366, before the closing type: object).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@work/v1/0000_00_work.open-cluster-management.io_manifestworks.crd.yaml`
around lines 340 - 366, Add an x-kubernetes-validations block to the
ignoreFields item schema so at least one of the path arrays is present;
specifically, update the object that defines ignoreFields (the schema containing
jsonPaths, jsonPointers, jqPathExpressions and condition) to include an
x-kubernetes-validations entry with a message like "At least one of jsonPaths,
jsonPointers, or jqPathExpressions must be specified" and a rule such as
has(self.jsonPaths) || has(self.jsonPointers) || has(self.jqPathExpressions) so
empty/no-op ignoreFields are rejected.

work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml (1)

376-402: Same validation gap as the ManifestWork CRD — no minimum-one-path constraint.

Same concern as flagged in the ManifestWork CRD: an ignoreFields entry can be created with condition only and all path arrays absent, resulting in a no-op. If you add a validation rule to the ManifestWork CRD, apply it here as well for consistency.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml`
around lines 376 - 402, The ignoreFields object allows creating entries with
only condition set and no path arrays; add an OpenAPI validation requiring at
least one path array to be non-empty by adding an anyOf (or oneOf) under the
same schema that checks jqPathExpressions, jsonPaths, and jsonPointers each with
minItems: 1; reference the ignoreFields object and its properties (condition,
jqPathExpressions, jsonPaths, jsonPointers) and ensure the new anyOf rule is
present in this CRD the same way you added it to the ManifestWork CRD so an
entry must include at least one non-empty path array.

addon/v1alpha1/0000_03_addon.open-cluster-management.io_addontemplates.crd.yaml (1)

357-383: Consistent with the other CRD changes — same validation gap applies.

Same as the other two CRDs: consider adding an x-kubernetes-validations rule to ensure at least one of the three path fields is specified. All three CRDs should stay in sync.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@addon/v1alpha1/0000_03_addon.open-cluster-management.io_addontemplates.crd.yaml`
around lines 357 - 383, Add an x-kubernetes-validations entry on the same object
that contains jqPathExpressions, jsonPaths and jsonPointers (the "condition"
object) to enforce that at least one of those three fields is present; implement
a validation rule referencing jqPathExpressions, jsonPaths and jsonPointers
(e.g. a CEL/validation expression that returns true if any of
.jqPathExpressions, .jsonPaths or .jsonPointers is present) and include a clear
message like "At least one of jqPathExpressions, jsonPaths or jsonPointers must
be specified." Ensure this new validation mirrors the other CRDs so all three
remain in sync.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@addon/v1alpha1/0000_03_addon.open-cluster-management.io_addontemplates.crd.yaml`:
- Around line 357-383: Add an x-kubernetes-validations entry on the same object
that contains jqPathExpressions, jsonPaths and jsonPointers (the "condition"
object) to enforce that at least one of those three fields is present; implement
a validation rule referencing jqPathExpressions, jsonPaths and jsonPointers
(e.g. a CEL/validation expression that returns true if any of
.jqPathExpressions, .jsonPaths or .jsonPointers is present) and include a clear
message like "At least one of jqPathExpressions, jsonPaths or jsonPointers must
be specified." Ensure this new validation mirrors the other CRDs so all three
remain in sync.

In `@work/v1/0000_00_work.open-cluster-management.io_manifestworks.crd.yaml`:
- Around line 340-366: Add an x-kubernetes-validations block to the ignoreFields
item schema so at least one of the path arrays is present; specifically, update
the object that defines ignoreFields (the schema containing jsonPaths,
jsonPointers, jqPathExpressions and condition) to include an
x-kubernetes-validations entry with a message like "At least one of jsonPaths,
jsonPointers, or jqPathExpressions must be specified" and a rule such as
has(self.jsonPaths) || has(self.jsonPointers) || has(self.jqPathExpressions) so
empty/no-op ignoreFields are rejected.

In
`@work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml`:
- Around line 376-402: The ignoreFields object allows creating entries with only
condition set and no path arrays; add an OpenAPI validation requiring at least
one path array to be non-empty by adding an anyOf (or oneOf) under the same
schema that checks jqPathExpressions, jsonPaths, and jsonPointers each with
minItems: 1; reference the ignoreFields object and its properties (condition,
jqPathExpressions, jsonPaths, jsonPointers) and ensure the new anyOf rule is
present in this CRD the same way you added it to the ManifestWork CRD so an
entry must include at least one non-empty path array.