ingress-nginx
ingress-nginx copied to clipboard
kubernetes
Helm Chart installs fails with: metadata.name must be no more than 63 characters
NGINX Ingress controller version: 0.47.0
NGINX Ingress Helm Chart version: 3.34.0
Kubernetes version (use kubectl version):
Client Version: version.Info{Major:"1", Minor:"21", GitVersion:"v1.21.1", GitCommit:"5e58841cce77d4bc13713ad2b91fa0d961e69192", GitTreeState:"clean", BuildDate:"2021-05-13T02:40:46Z", GoVersion:"go1.16.3", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"20+", GitVersion:"v1.20.7-eks-d88609", GitCommit:"d886092805d5cc3a47ed5cf0c43de38ce442dfcb", GitTreeState:"clean", BuildDate:"2021-07-31T00:29:12Z", GoVersion:"go1.15.12", Compiler:"gc", Platform:"linux/amd64"}
Environment:
- Cloud provider or hardware configuration:
AWS EKS - OS (e.g. from /etc/os-release):
Amazon Linux 2 - Kernel (e.g.
uname -a):5.4.129-63.229.amzn2.x86_64 - Install tools:
Terraform Helm Provider - Others:
What happened:
Error: Service "public-ssl-ingress-tool-preview-ingress-nginx-controller-metrics" is invalid: metadata.name: Invalid value: "public-ssl-ingress-tool-preview-ingress-nginx-controller-metrics": must be no more than 63 characters
with module.ingress-controller.helm_release.public_ssl_ingress[0],
on ingress-controller/public-controller-ingress-ssl.tf line 5, in resource "helm_release" "public_ssl_ingress":
5: resource "helm_release" "public_ssl_ingress" {
What you expected to happen:
Name gets truncated properly so that suffixes can be added and don't break the deployment.
How to reproduce it:
Set the name of the deployment to something longer than 63 characters and try to install.
Anything else we need to know:
Even though in templates/_helpers.tpl the name gets truncated to 63 characters the metadata.name fields for the 3 service manifests linked below will fail because they append some extra strings to the name.
templates/controller-service-internal.yaml templates/controller-service-metrics.yaml templates/controller-service-webhook.yaml
/kind bug
Hi @olafnorge thanks for raising this I'm looking into this, will get back to you soon🙌
because some Kubernetes name fields are limited to this (by the DNS naming spec) https://github.com/kubernetes/kubernetes/blob/2d08fd4f565d2c349267032c9d6d48af3925b4f0/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go#L30-L34
/remove-kind bug
@tao12345666333: Closing this issue.
In response to this:
you should change your svc name length.
/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.
you should change your svc name length.
Ähhhm, I could do so but still this is not obvious. To what length should I shorten it? I mean, if you introduce other suffixes my truncation may not be sufficient anymore.
I agree this is an easy fix if I have to do it but the underlying issue still persists.
/kind bug /reopen
@olafnorge: Reopened this issue.
In response to this:
you should change your svc name length.
Ähhhm, I could do so but still this is not obvious. To what length should I shorten it? I mean, if you introduce other suffixes my truncation may not be sufficient anymore.
I agree this is an easy fix if I have to do it but the underlying issue still persists.
/kind bug /reopen
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.
I have given you the reason. This restriction is not on ingress-nginx, but on Kubernetes
Deeper, this is because following rfc1123 https://datatracker.ietf.org/doc/html/rfc1123 So there is nothing we can do here.
To what length should I shorten it?
Needs to be shorter than 63
I am not sure if we are on the same page.
In https://github.com/kubernetes/ingress-nginx/blob/helm-chart-3.34.0/charts/ingress-nginx/templates/_helpers.tpl#L38 is the function ingress-nginx.controller.fullname with the content:
{{- printf "%s-%s" (include "ingress-nginx.fullname" .) .Values.controller.name | trunc 63 | trimSuffix "-" -}}
This function is truncating the name to 63 characters already and is being used inside the templates of the helm chart. So far so good.
BUT in the following templates a suffix gets added and this exceeds the max length restriction again. https://github.com/kubernetes/ingress-nginx/blob/helm-chart-3.34.0/charts/ingress-nginx/templates/controller-service-internal.yaml#L15
name: {{ include "ingress-nginx.controller.fullname" . }}-internal
https://github.com/kubernetes/ingress-nginx/blob/helm-chart-3.34.0/charts/ingress-nginx/templates/controller-service-metrics.yaml#L14
name: {{ include "ingress-nginx.controller.fullname" . }}-metrics
https://github.com/kubernetes/ingress-nginx/blob/helm-chart-3.34.0/charts/ingress-nginx/templates/controller-service-webhook.yaml#L11
name: {{ include "ingress-nginx.controller.fullname" . }}-admission
So even if I would make sure not to exceed the limit by truncating down to 63 characters it will continue failing! Please, read the code I provided.
I can't solve this on my end.
@Debanitrkl you wrote that you're going to have a look. Did you have time to do so already?
@tao12345666333 @Debanitrkl sorry for bothering you again. Any updates here? Have been able to check what I posted here?
Hey I took a look at this couldn't find any problems with the ingress-nginx, I suggest you to have a discussion with other developers in the slack.
We see this problem too, maybe I can help you understand why it's an issue.
By default ingress-nginx helm chart will create admission service (and the others also, just admission is the longest at the moment) based on helm release name. For example I want to name my ingress-nginx helm release like:
- development-webshop-backend01
- development-webshop-backend02
- development-webshop-fe01
- development-webshop-fe02
In this case helm try to create services with names like this:
- development-webshop-backend01-ingress-nginx-controller-admission
- development-webshop-backend02-ingress-nginx-controller-admission
- development-webshop-fe01-ingress-nginx-controller-admission
- development-webshop-fe02-ingress-nginx-controller-admission
But it will fail on the backends because those service names are 65 characters long, because -admission and the other service name suffix added AFTER trimming the fullname at max 63 characters. Also another "problem" if you count the character numbers we have 28 chars max from the possible max 63 (less then 45% is usable because the static long names in helm chart).
Found there are 2 values that could help make it longer:
fullnameOverride - this gives the '
so shortening those two to like: fullnameOverride: development-webshop-backend01 controller.name: ingrctrl
service names will be like this:
- development-webshop-backend01-ingrctrl-admission
which is only 49 characters, even could make it shorter a bit, but couldn't change 'admission' (and metric, internal) static names.
So I guess documentation needs to describe bit more these service naming conventions, because there is a 63 character lenght limit which could be reached easily. Also would be nice to set 'admission', 'metrics' and 'internal' naming with variables, so if needed could be override.
The other issue, that if (without -admission etc.) names lenghts over 63 char it will be stripped and not the total generated names checked if it's over 63 chars. So name will be stripped at 63 char but after that got the '-admission' suffix, the install will fail because service name is over 63 chars. Which means there is no point to strip at 63 chars, strip it at 52 chars (so with -admission it's still 63) or better doesn't strip at all but check before install the max lenght so someone who install it will decide how to make it shorter. Why is it problem if helm strip last x chars to make it short enough? Because if someone installing multiple instances with same prefix names that long enough to be the same after stripping, thos will collide and can't install the second one. Like if you say strip name at 52 char (before adding '-admission' suffix) and there are names containing first 52 chars the same and continous with numbers, like:
- development-wordpress-webshop-backend-release-alpha-03
- development-wordpress-webshop-backend-release-alpha-04 both names will be stripped as 'development-wordpress-webshop-backend-release-alpha', so when you try to install the second one it will fail as service names can't be longer than 63 characters and you find yourself in a painful process to figure it out how much is acceptable and how you could make it longer. This could be much easier if instead of stripping long names there is a warning before actually installing anything, that full service names couldn't be longer then 63 chars which is constructed from this and this and this, so try to shorten it with those variables.
We're also seeing this issue, any idea when this will be resolved?
Fundamentally no fields with a max length should be created by concatenating two strings together unless the combined max length of these strings cannot exceed the total max length. To solve this it looks like the arbitrary strings (such as "admission") could be shortened and helper functions could be added to make the strings in question both valid and unique.
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue or PR as fresh with
/remove-lifecycle stale - Mark this issue or PR as rotten with
/lifecycle rotten - Close this issue or PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
/unassign @Debanitrkl /triage accepted /priority important-longterm We are happy for any contributions :)
/help
@iamNoah1: This request has been marked as needing help from a contributor.
Guidelines
Please ensure that the issue body includes answers to the following questions:
- Why are we solving this issue?
- To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
- Does this issue have zero to low barrier of entry?
- How can the assignee reach out to you for help?
For more details on the requirements of such an issue, please see here and ensure that they are met.
If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.
In response to this:
/unassign @Debanitrkl /triage accepted /priority important-longterm We are happy for any contributions :)
/help
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.
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue or PR as fresh with
/remove-lifecycle stale - Mark this issue or PR as rotten with
/lifecycle rotten - Close this issue or PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue or PR as fresh with
/remove-lifecycle rotten - Close this issue or PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten
The issue has been marked as an important bug and triaged. Such issues are automatically marked as frozen when hitting the rotten state to avoid missing important bugs.
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle frozen
@tao12345666333 @iamNoah1 I'm happy to pick this up if you like?
Would an acceptable solution be to add some validation in _helpers.tpl so that if controller.service.enabled, "ingress-nginx.controller.fullname" must be shorter than 52 chars? That way at least users get an error early on.
We could also just do that regardless of controller.service.enabled if you prefer and have a blanket max length for all. Happy to go with whichever you think is most suitable?
@philnichol sure! Thanks!
As @blackluck said https://github.com/kubernetes/ingress-nginx/issues/7442#issuecomment-917402076
- We can set some suffixes as configurable items.
Of course, adding the validation you mentioned is also possible. Determine whether the final generated length exceeds 63
This issue has not been updated in over 1 year, and should be re-triaged.
You can:
- Confirm that this issue is still relevant with
/triage accepted(org members only) - Close this issue with
/close
For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/
/remove-triage accepted