kubernetes / ingress-nginx

Ingress NGINX Controller for Kubernetes
https://kubernetes.github.io/ingress-nginx/
Apache License 2.0
17.47k stars 8.25k forks source link

Better support for Helm installs #11341

Open aceeric opened 6 months ago

aceeric commented 6 months ago

When an application is installed from a Helm chart, Helm creates a list of manifests from the chart and simply sends the manifests into the cluster - leaving it to Kubernetes to handle. The following scenario doesn't currently allow the chart to successfully deploy because of the Nginx admission webhook:

  1. Manifest A is a Job that creates a Secret in the cluster.
  2. Manifest B is an Ingress with annotation nginx.ingress.kubernetes.io/auth-tls-secret: secret-created-by-job.
  3. The Job manifest and the Ingress manifest go into the cluster at essentially the same time.
  4. The Nginx ingress admission webhook sees that the nginx.ingress.kubernmetes.io/auth-tls-secret annotation refs a secret that does not exist yet. And so the webhook rejects the ingress.
  5. The Helm chart then does not fully deploy (no ingresses) and so the app is unusable.
  6. A second later the Job completes and the secret is present.

I can think of a couple ways to address this:

  1. Introduce configurable (via cmdline) exponential backoff to the admission webhook to retry all ingresses
  2. Introduce an annotation to the ingress that causes the admission webhook to retry that specific ingress
  3. Introduce an annotation to the ingress that causes the admission webhook to not validate that specific ingress

If you think any of these approaches are valid I would be happy to look into submitting a PR. Thanks.

k8s-ci-robot commented 6 months ago

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
longwuyuan commented 6 months ago

One example of handling this use-case shows how to simplify the use instead of changing the controller code ;

echo "deploying prometheus.."
helm upgrade --install prometheusgrafana kube-prometheus-stack \
    --repo https://prometheus-community.github.io/helm-charts \
    --namespace observability \
    --create-namespace \
    --set prometheus.prometheusSpec.podMonitorSelectorNilUsesHelmValues=false \
    --set prometheus.prometheusSpec.serviceMonitorSelectorNilUsesHelmValues=false

echo "creating ingress-nginx ns"
kubectl create ns ingress-nginx

echo "creating default-ssl-certificate for ingress-controller"
kubectl -n ingress-nginx create secret tls wildcard.domain.com \
    --cert ~user/letsencrypt/domain.com/fullchain1.pem \
    --key ~user/letsencrypt/domain.com/privkey1.pem

echo "deploying ingress-nginx controller.."
helm upgrade --install ingress-nginx ingress-nginx \
    --repo https://kubernetes.github.io/ingress-nginx \
    --namespace ingress-nginx \
    --set controller.metrics.enabled=true \
    --set controller.metrics.serviceMonitor.enabled=true \
    --set controller.metrics.serviceMonitor.additionalLabels.release="prometheusgrafana" \
    --set controller.service.externalTrafficPolicy=Local \
    --set controller.extraArgs.default-ssl-certificate=ingress-nginx/wildcard.domain.com

echo "creating ingress for grafana.."
ingressnginxPodName=`kubectl -n ingress-nginx get po | grep -i ingress-nginx-controller | cut -f1 -d" "`
kubectl -n ingress-nginx wait --for=condition=Ready pod/$ingressnginxPodName --timeout 120s
kubectl -n observability create ingress grafana \
    --class nginx \
    --rule grafana.domain.com/"*"=prometheusgrafana:80,tls

This example shows not one but multiple charts integration, instead of trying to change the chart itself

aceeric commented 6 months ago

Thanks - this doesn't address the original question which is - how to prevent the webhook from rejecting ingresses when the ingress references a secret created by a job and both the ingress AND the job are deployed in one helm chart.

github-actions[bot] commented 5 months ago

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

bpsizemore commented 3 months ago

Throwing support in for this issue - when bootstrapping new clusters this is a problem I come across frequently where a helm install leaves ingress-nginx in a bad state and prevents our gitops solution from properly bringing up clusters. Looking into manual alternatives now to solve the problem, but would prefer to be able to use the chart as intended.

aceeric commented 2 months ago

All - is there any way to elevate this? Thanks!

longwuyuan commented 2 months ago

Current status is acute shortage of developer time resources and a pivot to improving security as well as focus on sticking closer to the KEP of the Kubernetes Ingress API specs.

I agree that having the kind of changes proposed in the description of this issue is extremely useful. But the instrumentation proposed in this issue is best suited as code that resides outside this project. While the changes would help the use case described, please do not expect any resources to be spent on making the kind of changes proposed here.