Skip to content

✨ New kubebuilder:schemaModifier CRD marker #1140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yaroslavborbat
Copy link

Overview

Closes #441

This pull request introduces a new marker kubebuilder:schemaModifier, which allows modifying specific fields in JSONSchemaProps for CustomResourceDefinitions (CRDs). This marker supports flexible rules for targeting specific paths in the CRD schema and allows setting multiple properties at once.

Details

New Marker: kubebuilder:schemaModifier

The schemaModifier marker operates on CRD schemas and enables users to apply modifications to specific fields. The targeting of fields is achieved using the PathPattern attribute, which defines a path-matching rule using a JSONPath-like syntax:

  • *: Matches any single field name (e.g., /spec/*/field).
  • **: Matches fields across multiple levels of nesting (e.g., /spec/**/field).

This marker can update multiple properties in JSONSchemaProps, such as Description, Format, Maximum, Nullable, and others.

Example:

// +kubebuilder:schemaModifier:pathPattern=/spec/exampleObject/*,description=""

The above example applies the empty description to all fields matching /spec/exampleObject/*.

How This Was Tested

Currently, no tests have been added for this feature. However, once the structure and approach are approved, I plan to add integration tests to verify the functionality of the kubebuilder:schemaModifier marker.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 30, 2025
@k8s-ci-robot
Copy link
Contributor

Welcome @yaroslavborbat!

It looks like this is your first PR to kubernetes-sigs/controller-tools 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/controller-tools has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 30, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @yaroslavborbat. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 30, 2025
Signed-off-by: Yaroslav Borbat <[email protected]>
@yaroslavborbat yaroslavborbat force-pushed the add-marker-schemaModifier branch from 71a1237 to d0371a8 Compare February 4, 2025 07:15
Copy link
Member

@chrischdi chrischdi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have unit-test coverage.

I'd like to understand the use-case some more: Can't this be done by just not setting markers at some fields?

I guess this does not work e.g. for description, but I don't get it for other fields.

@JoelSpeed
Copy link
Contributor

Where there are lists, for example an enum, would you need to specify the entire enum with this marker, and how would that look? What separators are used?

Does this encourage the use of/give folks the ability to create schemas that aren't valid? I wonder if we want to encourage patching schemas in ways that could be foot guns? WDYT?

@yaroslavborbat
Copy link
Author

Hi @chrischdi! I'll add test coverage for this soon.

This approach works for description and should also work for other fields. CRD markers are processed last, when the full schema has already been generated, so we can traverse it recursively and modify fields as needed.

One key use case is when defining a CRD that includes imported types. Since we cannot modify or extend annotations on those types directly, this marker allows us to adjust their schema properties where necessary.

@yaroslavborbat
Copy link
Author

Hi @JoelSpeed!
For lists like enums, you would need to specify the entire list when modifying it. The marker does not perform partial updates to list elements—it replaces the entire field.

Here’s an example of how I replaced the required list and the description:

// +kubebuilder:schemaModifier:pathPattern=/spec/testObject,required={"field1", "field2", "field3", "field4"},description="new my description"

Regarding schema validity, this marker does allow users to generate an invalid schema if used incorrectly, so it should be used with caution. At the same time, it provides additional flexibility in CRD generation. Personally, I found such a marker useful, which is why I created this PR 🙃.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 10, 2025
@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@yaroslavborbat yaroslavborbat marked this pull request as draft February 28, 2025 22:14
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to skip descriptions per field or per package
4 participants