Skip to content
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

OpenAPI spec marking optional v1.Route fields as required #24060

Closed
rblaine95 opened this issue Oct 31, 2019 · 24 comments
Closed

OpenAPI spec marking optional v1.Route fields as required #24060

rblaine95 opened this issue Oct 31, 2019 · 24 comments
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@rblaine95
Copy link

rblaine95 commented Oct 31, 2019

When using Helm v3-rc.1, which includes OpenAPI validation before executing an install or upgrade, Route objects fail if they're missing optional fields (such as spec.host, status, spec.to.weight).

After raising on the Helm Github (helm/helm#6830), I was pointed to the OKD v3.11 OpenAPI spec which incorrectly lists these fields as Required when, according to the Docs and API, they should be optional.

Version
» oc version
oc v3.11.0+0cbc58b
kubernetes v1.11.0+d4cacc0
features: Basic-Auth GSSAPI Kerberos SPNEGO

Server https://okd.example.net:8443
openshift v3.11.0+fef6f83-275
kubernetes v1.11.0+d4cacc0
Steps To Reproduce
  1. Have a Helm Chart that has an OKD Route in the templates/ dir
  2. Using Helm >=v3.beta-5, run helm template ./my-chart-with-route --validate (or helm install ./my-chart-with-route)
  3. See error:
Error: unable to build kubernetes objects from release manifest: error validating "": error validating data: [ValidationError(Route.spec.to): missing required field "weight" in com.github.openshift.api.route.v1.RouteTargetReference, ValidationError(Route.spec): missing required field "host" in com.github.openshift.api.route.v1.RouteSpec, ValidationError(Route): missing required field "status" in com.github.openshift.api.route.v1.Route]
Current Result

OKD v3.11 OpenAPI incorrectly lists some Optional fields as Required.

Expected Result

OKD v3.11 OpenAPI spec should list Required fields as Required and Optional fields as Optional.

Edit:

"v1.RouteSpec": {
  "id": "v1.RouteSpec",
  "description": "...",
  "required": [
    "host", # <--- ?
    "to"
  ],
"v1.RouteStatus": {
  "id": "v1.RouteStatus",
  "description": "...",
  "required": [
    "ingress" # <-- ?
  ],
"v1.RouteTargetReference": {
  "id": "v1.RouteTargetReference",
  "description": "...",
  "required": [
    "kind",
    "name",
    "weight" # <--- ?
  ],
@rblaine95 rblaine95 changed the title OpenAPI spec marking optional route fields as required OpenAPI spec marking optional v1.Route fields as required Oct 31, 2019
@rblaine95
Copy link
Author

This is causing a blocker for using any Helm version >=v3.beta-5, including the recent RC versions and any future versions of Helm v3 (at least until Helm v3.1.0 which will allow you to disable OpenAPI Validation helm/helm#6819)

@rblaine95
Copy link
Author

Now that Helm v3.0.0 has been tagged and released, this bug is preventing anyone using OKD from upgrading to Helm 3.

@CermakM
Copy link

CermakM commented Nov 18, 2019

There are IMHO more resources with fields that shouldn't be marked as required, for example:

   "v1.BuildConfig": {
    "required": [
     "spec",
     "status"  # <--- ???
    ],

or

  "v1.BuildConfigSpec": {
    "required": [
     "triggers",
     "strategy",
     "nodeSelector"  # <--- ???
    ],

@nccurry
Copy link

nccurry commented Nov 21, 2019

I opened a bugzilla for this:
https://bugzilla.redhat.com/show_bug.cgi?id=1773682

@Andrei-Stepanov
Copy link

Got the the similar error:

Error: unable to build kubernetes objects from release manifest: error validating "": error validating data: [ValidationError(BuildConfig.spec): missing required field "nodeSelector" in com.github.openshift.api.build.v1.BuildConfigSpec, ValidationError(BuildConfig.status): missing required field "lastVersion" in com.github.openshift.api.build.v1.BuildConfigStatus]
helm.go:76: [debug] error validating "": error validating data: [ValidationError(BuildConfig.spec): missing required field "nodeSelector" in com.github.openshift.api.build.v1.BuildConfigSpec, ValidationError(BuildConfig.status): missing required field "lastVersion" in com.github.openshift.api.build.v1.BuildConfigStatus]

@scniro
Copy link

scniro commented Nov 25, 2019

I am also seeing the same with helm@3, any plans of yet on this one?

helm.go:76: [debug] error validating "": error validating data: ValidationError(Route): missing required field "status" in [...]

@scniro
Copy link

scniro commented Dec 6, 2019

FWIW I was able to move forward by stubbing out a junk status object

status:
  ingress:
    - conditions:
      - lastTransitionTime: "2019-12-06T03:24:58Z"
        status: "True"
        type: Admitted
      host: <host>
      routerCanonicalHostname: <host>
      routerName: default
      wildcardPolicy: None

rottencandy pushed a commit to rottencandy/findmyrelative-chart that referenced this issue Dec 27, 2019
This is used to satisfy helm,
workaround to bug: openshift/origin#24060
rottencandy pushed a commit to rottencandy/findmyrelative-chart that referenced this issue Dec 27, 2019
This is used to satisfy helm,
workaround for bug openshift/origin#24060
@jkroepke
Copy link

I create a own helper inside _helpers.tpl

{{- define "chart.helmRouteFix" -}}
status:
  ingress:
    - host: ""
{{- end -}}

and at the end of a route I include the helper:

...
  to:
    kind: Service
    name: {{ template "chart.fullname" $ }}-keycloak
    weight: 100
{{ include "chart.helmRouteFix" $ }}

It reduce the amount of waste to a minimum

@yoursmanish
Copy link

Error: unable to build kubernetes objects from release manifest: error validating "": error validating data: ValidationError(Route): missing required field "status" in com.github.openshift.api.route.v1. Route

$ oc version
oc v3.11.0+0cbc58b
kubernetes v1.11.0+d4cacc0
features: Basic-Auth SSPI Kerberos SPNEGO

Server https://ilocxxx-xxxx.xxxx.xxxx.com:8443
openshift v3.11.98
kubernetes v1.11.0+d4cacc0

$ helm version
version.BuildInfo{Version:"v3.1.1", GitCommit:"afe70585407b420d0097d07b21c47dc511525ac8", GitTreeState:"clean", GoVersion:"go1.13.8"}

@helfper
Copy link

helfper commented Apr 7, 2020

If people want to be completely minimalist, this is the minimal status definition that complies with the route spec:

{{- define "chart.helmRouteFix" -}}
status:
  ingress: []
{{- end -}}

@openshift-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 7, 2020
@tomcruise81
Copy link

/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 7, 2020
@openshift-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 25, 2020
@jkroepke
Copy link

/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 25, 2020
@jkroepke
Copy link

/lifecycle frozen

@openshift-ci-robot openshift-ci-robot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Oct 25, 2020
@m-yosefpor
Copy link

/remove-lifecycle frozen

@openshift-ci-robot openshift-ci-robot removed the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Nov 3, 2020
@m-yosefpor
Copy link

m-yosefpor commented Nov 3, 2020

The above workarounds are not useful if Helm chart is being used with a gitops tools such as ArgoCD. It continuously want to reconcile the status section as it changes after deployment. It makes ArgoCD not to be usable with Openshift resources. Disabling validation is not a good idea as it might break other things in future.

@jkroepke
Copy link

jkroepke commented Nov 3, 2020

@m-yosefpor

The latest version of Openshift 3.11 should mark this field as optional

References:

@m-yosefpor
Copy link

@jkroepke We are using Openshift 3.10. Any chance that it could be back ported to that version?

@jkroepke
Copy link

jkroepke commented Nov 3, 2020

I guess no unless you want to build openshift from source. 3.10 is out of support. But an upgrade 3.11 should not be a problem.

@openshift-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 2, 2021
@openshift-bot
Copy link
Contributor

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 4, 2021
@openshift-bot
Copy link
Contributor

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci-robot
Copy link

@openshift-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests