-
Notifications
You must be signed in to change notification settings - Fork 191
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
Adds IngressController CRD Validation & Generation #183
Conversation
format: int32 | ||
type: integer | ||
conditions: | ||
description: 'conditions is a list of conditions and their status. Available |
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.
I didn't see status.conditions
when running oc get ingresscontrollers.operator.openshift.io/default -n openshift-ingress-operator -o yaml
.
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.
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.
Get it, thank you @danehans
/test e2e-aws |
I think @Miciah at least (maybe also @pravisankar) prefers a single PR for the dep updates and the code which uses the new deps — if so, I'm happy to conform to the majority preference (and then #185 could be brought over into this PR). Up to you guys! |
configures DNS zones. - False if any of those conditions are unsatisfied. * | ||
DNSReady - True if the following conditions are met: * DNS is | ||
managed. * DNS records have been successfully created. - False | ||
if any of those conditions are unsatisfied.' |
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.
Are the line breaks really lost in this translation? 🤨
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.
Yeah, I noticed the generated formatting of descriptions are a bit skewed from the source spec. We can potentially try to fix this upstream. It seems painful to make changes in the api docs to be more generator friendly or manually manage the diffs.
/test unit |
/test e2e-aws-operator /test e2e-aws |
/retest |
Let's get this in to get a foundation established. /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danehans, ironcladlou 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 |
@ironcladlou I created a branch in my repo and layered another commit on top of this one that adds more granular validation for the |
PR #185 is required before merging.
Previously,
IngressController
resource fields were not validated. Additionally, the associated CRD was manually managed. This commit adds:OpenAPI v3 schema validation to the
IngressController
custom resource and partially fixes Bug 1683766.Adds
make crd
andmake verify-crd
to generate and verify crd modification respectively.Note: The schema is based on the currently vendored api definitions (853a7faf74b1c63a56e4010d39057559fa13271f) and not the latest from openshift/api.