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
[Doc] Update README with how to set default value in schema or alm-examples #4304
[Doc] Update README with how to set default value in schema or alm-examples #4304
Conversation
Hi @tlwu2013. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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/test-infra repository. |
Hey @madorn , can you help review this PR and see if this makes sense to you? Thanks! |
The `value` field is an optional field. If present, the value of this spec is the same for all instances of the CRD and can be found here instead of on the CR. You can specify the default value of the field on CRD in [OpenAPI v3 validation schema](https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#defaulting). | ||
The `value` field is an optional field. If present, the value of this spec is the same for all instances of the CRD and can be found here instead of on the CR. | ||
|
||
You can assign the default value of the field on CRD in OpenAPI v3 validation schema (see [Defaulting](https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#defaulting) feature in Kubernetes 1.17). Alternatively, you can specify the value of a field in [CRD Templates](https://github.com/operator-framework/operator-lifecycle-manager/blob/master/doc/design/building-your-csv.md#crd-templates) in the `ClusterServiceVersion` to set (or override) the default value on the CRD. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @tlwu2013!
I think adding a hint here as to why the suggested alternatives to defaulting are included in this documentation will help. Something similar to ...
This should not be used to apply a default value to a given custom resource for consumption by the console. You can assign...
I can go either way on whether or not this is needed. Your discretion. LGTM either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me! I'll include your suggestions to the changes, thanks!
546a38c
to
4d7d08a
Compare
Hey @benjaminapetersen , could you help review/approve this? thanks! |
/ok-to-test |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: benjaminapetersen, komish, tlwu2013 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
/retest Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
10 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
@tlwu2013: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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/test-infra repository. I understand the commands that are listed here. |
Add detailed info about:
Current:
This PR: