-
Notifications
You must be signed in to change notification settings - Fork 226
crossplane: Mark spec as required, status.atProvider as optional #81
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
Conversation
This is required to fix crossplane-contrib/provider-aws#697 In the vast majority of resources a spec is required, so I'd like to mark it as such. I believe that in the edge cases where the spec really contains only optional fields it's possible to write `spec: {}`. We prefer not to mark status fields as required, since resources will always exist at some point without their status and we consider 'requireness' to be more of a human-facing constraint than a software-facing constraint that our managed resource controllers should be subject to. Signed-off-by: Nic Cope <[email protected]>
Hi @negz. Thanks for your PR. I'm waiting for a aws-controllers-k8s 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. |
CC @muvaf |
/ok-to-test |
/test unit-test |
Fixes crossplane-contrib#697 Depends on aws-controllers-k8s/code-generator#81 In the vast majority of resources a spec is required, so I'd like to mark it as such. I believe that in the edge cases where the spec really contains only optional fields it's possible to write spec: {}. We prefer not to mark status fields as required, since resources will always exist at some point without their status and we consider 'requireness' to be more of a human-facing constraint than a software-facing constraint that our managed resource controllers should be subject to. Signed-off-by: Nic Cope <[email protected]>
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.
@negz thanks for catching this!
/assign @a-hilaly |
Thanks @negz ! |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: A-Hilaly, muvaf, negz 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 |
aws-controllers-k8s/code-generator#81 This commit includes the above PR, which makes all extant ACK generated CRDs conformant according to https://github.com/crossplane/conformance. Signed-off-by: Nic Cope <[email protected]>
This is required to fix crossplane-contrib/provider-aws#697
In the vast majority of resources a spec is required, so I'd like to mark it as such. I believe that in the edge cases where the spec really contains only optional fields it's possible to write
spec: {}
.We prefer not to mark status fields as required, since resources will always exist at some point without their status and we consider 'requiredness' to be more of a human-facing constraint than a software-facing constraint that our managed resource controllers should be subject to.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.