-
Notifications
You must be signed in to change notification settings - Fork 40k
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
Fix CRD validation error on 'items' field #76124
Fix CRD validation error on 'items' field #76124
Conversation
5333e76
to
16ed9f3
Compare
/assign @liggitt |
16ed9f3
to
050d4c5
Compare
8dfb907
to
1403ba2
Compare
@liggitt this PR is done now, PTAL. |
thanks for the quick update. the dependencies you pulled in have transitive dependencies we need to also bump. |
Signed-off-by: He Xiaoxi <xxhe@alauda.io>
Signed-off-by: He Xiaoxi <xxhe@alauda.io>
Signed-off-by: He Xiaoxi <xxhe@alauda.io>
Signed-off-by: He Xiaoxi <xxhe@alauda.io>
Signed-off-by: He Xiaoxi <xxhe@alauda.io>
Signed-off-by: He Xiaoxi <xxhe@alauda.io>
Signed-off-by: He Xiaoxi <xxhe@alauda.io>
@liggitt all bumps are done, please check again. |
/lgtm |
@@ -349,9 +303,15 @@ func (s *Schema) AddType(tpe, format string) *Schema { | |||
return s | |||
} | |||
|
|||
// AsNullable flags this schema as nullable. | |||
func (s *Schema) AsNullable() *Schema { |
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.
@sttts, fyi that we picked up a version with this
@tossmilestone, thanks for all the work on this |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, tossmilestone 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 |
Unfortunately this fix has not solved the core issue. See go-openapi/validate#108 |
On further investigation I think this is due to See #83778 |
…te-items Fix CRD validation error on 'items' field Kubernetes-commit: 1a80962
Root cause of the issue is that k8s 1.15 / 1.16 clusters seem to have known issues related to CRD fields that invoke `items` in it, such as the `volumes` we introduced as part of the nginx proxy that sits in front of Prometheus in Monitoring V2. This only seems to have been fixed in k8s 1.17+. More context: kubernetes/kubernetes#68466 is an issue that tracks a problem with CRDs that use fields named `items` introduced sometime before k8s 1.16. In k8s 1.16's release notes, this issue seemed to have been resolved as it shows up under `Other Notable Changes` [here](https://v1-16.docs.kubernetes.io/docs/setup/release/notes/#other-notable-changes-7): ``` Fix CRD validation error on ‘items’ field. (#76124, @tossmilestone) ``` However, when looking into the actual [PR](kubernetes/kubernetes#76124) that was executed to fix this issue, it seems like this issue had a followup in kubernetes/kubernetes#85223 that was only resolved in k8s 1.17+. This shows up in the k8s 1.17 release notes under `API Machinery` [here](https://v1-17.docs.kubernetes.io/docs/setup/release/notes/#api-machinery): ``` CRDs can have fields named type with value array and nested array with items fields without validation to fall over this. (#85223, @sttts) ``` Therefore, the quick fix was to just modify the contents provided to the `volumes` field of the `prometheusSpec`; once modified, Monitoring V2 successfully deployed in a k8s 1.16 cluster.
Signed-off-by: He Xiaoxi tossmilestone@gmail.com
What type of PR is this?
/kind bug
What this PR does / why we need it:
Fix CRD validation errror on 'items' field
Which issue(s) this PR fixes:
Fixes #68466
Special notes for your reviewer:
Does this PR introduce a user-facing change?: