Skip to content

Add schema.yaml validation for extraFiles.data structure #2095

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

Closed
roelbaz opened this issue Mar 11, 2021 · 17 comments · Fixed by #2198
Closed

Add schema.yaml validation for extraFiles.data structure #2095

roelbaz opened this issue Mar 11, 2021 · 17 comments · Fixed by #2198
Milestone

Comments

@roelbaz
Copy link

roelbaz commented Mar 11, 2021

Bug description

Template parsing error of the extraFile mountPath in:

jupyterhub/templates/_helpers.tpl:382:29: executing "jupyterhub.extraFiles.validate-file" at $file_details.mountPath>: can't evaluate field mountPath in type string

This was introduced in this PR: #2006

This is in versions 0.11.1-n113.hc841983b through 0.11.1-n301.hc86268b1 (Didn't try em all obviously, but those two are the 'bookends')

Expected behaviour

No parsing complaints from the _helpers.tpl

Actual behaviour

ubuntu@jhub-controller:~/jhub/kubernetes$

ubuntu@jhub-controller:~/jhub/kubernetes$ helm upgrade --cleanup-on-fail   --install jhub jupyterhub/jupyterhub   --namespace jhub   --create-namespace   --version=0.11.1-n301.hc86268b1
Error: UPGRADE FAILED: template: jupyterhub/templates/singleuser/secret.yaml:9:9: executing "jupyterhub/templates/singleuser/secret.yaml" at <include "jupyterhub.extraFiles.data" .Values.singleuser.extraFiles>: error calling include: template: jupyterhub/templates/_helpers.tpl:343:8: executing "jupyterhub.extraFiles.data" at <include "jupyterhub.extraFiles.data.withNewLineSuffix" .>: error calling include: template: jupyterhub/templates/_helpers.tpl:336:12: executing "jupyterhub.extraFiles.data.withNewLineSuffix" at <include "jupyterhub.extraFiles.validate-file" (list $file_key $file_details)>: error calling include: template: jupyterhub/templates/_helpers.tpl:382:29: executing "jupyterhub.extraFiles.validate-file" at <$file_details.mountPath>: can't evaluate field mountPath in type string

How to reproduce

Near as I can tell, a standard config.yaml for helm3 is going to provoke this.

  • Note that I did upgrade the cluster from helm 2 to 3. (
  • Basic cluster as described in the install guide with the optional dedicated user pool.

Your personal set up

  • OS:
    ubuntu@jhub-controller:~/jhub/kubernetes$ cat /etc/issue
    Ubuntu 20.04.1 LTS \n \l

  • Configuration

Jupyterhub 1.3.0
Helm chart version: 0.11.1-n301.hc86268b1 (march 11th) but tried all the way back to 0.11.1-n113.hc841983b (feb 1st) when the aforementioned PR got introduced.

jupyterhub_config.py is unmodified from what is in the helm chart.

ubuntu@jhub-controller:~/.config/helm$ cat repositories.yaml 
apiVersion: v1
generated: "2021-02-09T20:21:27.59853531Z"
repositories:
- caFile: ""
  cache: /home/ubuntu/.helm/repository/cache/stable-index.yaml
  certFile: ""
  keyFile: ""
  name: stable
  password: ""
  url: https://charts.helm.sh/stable
  username: ""
- caFile: ""
  cache: /home/ubuntu/.helm/repository/cache/local-index.yaml
  certFile: ""
  keyFile: ""
  name: local
  password: ""
  url: http://127.0.0.1:8879/charts
  username: ""
- caFile: ""
  cache: /home/ubuntu/.helm/repository/cache/jupyterhub-index.yaml
  certFile: ""
  keyFile: ""
  name: jupyterhub
  password: ""
  url: https://jupyterhub.github.io/helm-chart/
  username: ""
  • Logs
Error: UPGRADE FAILED: template: jupyterhub/templates/singleuser/secret.yaml:9:9: executing "jupyterhub/templates/singleuser/secret.yaml" at <include "jupyterhub.extraFiles.data" .Values.singleuser.extraFiles>: error calling include: template: jupyterhub/templates/_helpers.tpl:343:8: executing "jupyterhub.extraFiles.data" at <include "jupyterhub.extraFiles.data.withNewLineSuffix" .>: error calling include: template: jupyterhub/templates/_helpers.tpl:336:12: executing "jupyterhub.extraFiles.data.withNewLineSuffix" at <include "jupyterhub.extraFiles.validate-file" (list $file_key $file_details)>: error calling include: template: jupyterhub/templates/_helpers.tpl:382:29: executing "jupyterhub.extraFiles.validate-file" at <$file_details.mountPath>: can't evaluate field mountPath in type string

@roelbaz roelbaz added the bug Something isn't working label Mar 11, 2021
@welcome
Copy link

welcome bot commented Mar 11, 2021

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@roelbaz
Copy link
Author

roelbaz commented Mar 11, 2021

And prematurely apologies if this an 'obvious' error. Based on my tinkering however I think this is an actual error in _helpers.tpl.

@consideRatio
Copy link
Member

consideRatio commented Mar 11, 2021

@roelbaz I can't reproduce this, and feel quite confident that the helm chart is given some configuration that has been invalid.

It smells to me like an issue caused by for example terraform modifying the config in a faulty way. If it were an issue, it should be able to reproduce like this:

# this works fine!
helm template hub1 jupyterhub --repo=https://jupyterhub.github.io/helm-chart/ --version=0.11.1-n301.hc86268b1

Technically, I think this is an error caused by a config like...

hub:
  extraFiles:
    test: "hi"

I can reproduce it like this...

# this reproduce the error, but provides an invalid config
helm template hub1 jupyterhub --repo=https://jupyterhub.github.io/helm-chart/ --version=0.11.1-n301.hc86268b1 --set hub.extraFiles.test=string-here

So, I assume this is a misconfiguration, and potentially caused by an external tool out of this helm charts control. I'll go ahead and close it for now but do followup if you can pinpoint this as an issue with the Helm chart.

@roelbaz
Copy link
Author

roelbaz commented Mar 11, 2021

First of this is without any reference in the config.yaml to singleuser.extraFiles. (ie, not included whatsoever.)
Also this is a 'plain' zero to jupyterhub install on gke, no fancy/foo-foo/terraform etc. (to be clear, otherwise I would have included that.)

But given your suggestions in your response I'll tinker some and see if I can reproduce. Ultimately I'm after using extraFiles, but at this point I'm not even attempting that.

@roelbaz
Copy link
Author

roelbaz commented Mar 11, 2021

Also thanks a whole bunch replying so quickly

@consideRatio
Copy link
Member

The error you get is associated with Helm template rendering, so it should be reproducible like...

$ helm version
version.BuildInfo{Version:"v3.5.0", GitCommit:"32c22239423b3b4ba6706d450bd044baffdcf9e6", GitTreeState:"clean", GoVersion:"go1.15.6"}

# this should work, and does work for me
helm template hub1 jupyterhub --repo=https://jupyterhub.github.io/helm-chart/ --version=0.11.1-n301.hc86268b1

@roelbaz
Copy link
Author

roelbaz commented Mar 11, 2021

Thanks for the help/suggestions. Template command you gave me renders without error. Obviously have to dig, will let you know what I find.

To be clear the helm config.yaml does work with 0.11.1 and 0.11.1-n082.h437ed29d (ie, just prior to PR #2006)

@consideRatio
Copy link
Member

@roelbaz okay then use --debug helm flag and copy paste the values under hub.extraFiles and singleuser.extraFiles as should be listed before the error shows using --debug.

@roelbaz
Copy link
Author

roelbaz commented Mar 11, 2021

I think I got it... it's the --set-file on the commandline screwing things up. I drop the --set-file and the extraFiles section in the config.yaml, and it loads.

@consideRatio
Copy link
Member

Hmmm... If you are using --set-file, you should probably use something else than data, such as stringData or binaryData. See https://zero-to-jupyterhub.readthedocs.io/en/latest/resources/reference.html#hub-extrafiles

@consideRatio consideRatio changed the title jupyterhub/templates/_helpers.tpl:382:29 <$file_details.mountPath>: can't evaluate field mountPath in type string Add schema.yaml validation for extraFiles.data structure Mar 11, 2021
@consideRatio consideRatio added maintenance and removed bug Something isn't working labels Mar 11, 2021
@consideRatio
Copy link
Member

consideRatio commented Mar 11, 2021

You could have been given a better error message for this misconfiguration, so I'm reopening this issue as an issue about fixing schema.yaml to validate the hub|singleuser.extraFiles.data entries better.

@consideRatio consideRatio reopened this Mar 11, 2021
@roelbaz
Copy link
Author

roelbaz commented Mar 11, 2021

OK, confirmed my 'stupid' here:

--set-file singleuser.extraFiles.sshConfig=./$HELM_CONFIG/files/ssh/config
should be:
--set-file singleuser.extraFiles.sshConfig.stringData=./$HELM_CONFIG/files/ssh/config

@consideRatio Thanks a whole bunch for your patience/help.

@consideRatio
Copy link
Member

consideRatio commented Mar 11, 2021

We have a file called schema.yaml, it is a YAML representation of a JSONSchema, which is a format to validate data structures found in JSON/YAML for example.

Here it is:

extraFiles: &extraFiles
type: object
description: |

To better catch you error, we can trial with...

# this should say that `.data` must not be a string before doing anything else
helm template hub1 jupyterhub --repo=https://jupyterhub.github.io/helm-chart/ --version=0.11.1-n301.hc86268b1 --set hub.extraFiles.data=mystring

To do so, our extraFiles entry should look more like this instead (properties of extraFiles listed, and having some marked as required, and with their type field set):

image: &image-spec
type: object
required: [name, tag]
description: |
Set custom image name, tag, pullPolicy, or pullSecrets for the pod.
properties:
name:
type: string
description: |
The name of the image, without the tag.
```
# example name
gcr.io/my-project/my-image
```
tag:
type: string
description: |
The tag of the image to pull. This is the value following `:` in
complete image specifications.
```
# example tags
v1.11.1
zhy270a
```
pullPolicy:
enum: [null, "", IfNotPresent, Always, Never]
description: |
Configures the Pod's `spec.imagePullPolicy`.
See the [Kubernetes docs](https://kubernetes.io/docs/concepts/containers/images/#updating-images)
for more info.

@roelbaz, would you be interested in contributing a PR to fix this, updating schema.yaml to better help users catch this kind of error in the future?

@roelbaz
Copy link
Author

roelbaz commented Mar 11, 2021

@consideRatio Yes I'll contribute a PR, give me ~24hrs or so.

@consideRatio
Copy link
Member

@roelbaz wiee! Do ping me if you feel stuck!

Also btw I think we are better of by not adding the description fields on the properties of extraFiles such as data, stringData, binaryData, mountPath etc as they are described extensively in the extraFiles description field already so I suggest we avoid repeating ourselves.

@roelbaz
Copy link
Author

roelbaz commented Mar 13, 2021

@consideRatio It'll be a bit more than 24hrs, as usual a bit too optimistic about my actual workload.

@consideRatio
Copy link
Member

❤️ ❤️ @roelbaz no preassure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants