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

Validate input from annotations/configmaps #4047

Closed
vncntvandriessche opened this issue Apr 29, 2019 · 7 comments
Closed

Validate input from annotations/configmaps #4047

vncntvandriessche opened this issue Apr 29, 2019 · 7 comments

Comments

@vncntvandriessche
Copy link
Contributor

We should validate input coming from annotations(/configmaps) if wrong submissions cause unexpected behavior.


FEATURE REQUEST:

There should be input validation (validatingWebhookConfiguration?) on the possible configuration options set using annotations (not sure it applies to ConfigMaps as well).

We set the following annotations in our one of our ingresses:

[...]
  annotations:
    [...]
    kubernetes.io/ingress.class: nginx
    nginx.ingress.kubernetes.io/proxy-body-size: 250m
    nginx.ingress.kubernetes.io/proxy-next-upstream-tries: 10
[...]

NGINX Ingress controller version: 0.23.0

Kubernetes version:

Client Version: version.Info{Major:"1", Minor:"13", GitVersion:"v1.13.4", GitCommit:"c27b913fddd1a6c480c229191a087698aa92f0b1", GitTreeState:"clean", BuildDate:"2019-03-01T23:34:27Z", GoVersion:"go1.12", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"11+", GitVersion:"v1.11.8-gke.6", GitCommit:"394ee507d00f15a63cef577a14026096c310698e", GitTreeState:"clean", BuildDate:"2019-03-30T19:31:43Z", GoVersion:"go1.10.8b4", Compiler:"gc", Platform:"linux/amd64"}

Environment:

  • Cloud provider or hardware configuration: GKE
  • OS (e.g. from /etc/os-release): COS from Google
  • Kernel (e.g. uname -a): 4.14.91+
  • Install tools:
  • Others:

What happened:

The configuration changes where ignored, until we fixed the used notation.

What you expected to happen:

For an error to be shown clearly, or for the input to be parsed even though it wasn't passed as a string.

How to reproduce it (as minimally and precisely as possible):

Create an ingress for the nginx-controller to respond to, and create an annotation-setting that should be passed as a string, but pass it as an integer:

[...]
  annotations:
    [...]
    kubernetes.io/ingress.class: nginx
    nginx.ingress.kubernetes.io/proxy-body-size: 250m
    nginx.ingress.kubernetes.io/proxy-next-upstream-tries: 10
[...]

How to fix it using now

This might be a useful section for people bumping their heads into this if you explicitly specify all settings as strings (when in doubt, match the type in the documentation):

[...]
  annotations:
    [...]
    kubernetes.io/ingress.class: nginx
    nginx.ingress.kubernetes.io/proxy-body-size: 250m
    nginx.ingress.kubernetes.io/proxy-next-upstream-tries: "10"
[...]
@aledbf
Copy link
Member

aledbf commented Apr 29, 2019

@vncntvandriessche we already have that information about types here https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/annotations/
Any suggestion to improve that is welcome

@vncntvandriessche
Copy link
Contributor Author

Would a validationController that double checks the values' type integrity be a suitable suggestion? I'm willing to build a prototype but I might not be the most suitable person to do it!

@aledbf
Copy link
Member

aledbf commented Apr 29, 2019

@vncntvandriessche this should be done here #3802
After that is merged I will check the validation is there

@vncntvandriessche
Copy link
Contributor Author

Sorry I waisted your time, somehow missed that.

@aledbf Thank you so much!

@tjamet
Copy link
Contributor

tjamet commented May 3, 2019

Hi, indeed, the annotations part should be solved by the validating webhook
Though, it does not validate config maps, it's a feature that would need to be added
Happy to help implement the feature if needed

@aledbf
Copy link
Member

aledbf commented Jul 8, 2019

Closing. Please update to 0.25.0 where the validating webhook feature is present.

@aledbf aledbf closed this as completed Jul 8, 2019
@vncntvandriessche
Copy link
Contributor Author

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

No branches or pull requests

3 participants