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

Traffic management using NGINX ingress controller does not support TLS termination at the ingress controller. #1134

Closed
seventieskid opened this issue Apr 30, 2021 · 8 comments
Labels
enhancement New feature or request no-issue-activity

Comments

@seventieskid
Copy link

seventieskid commented Apr 30, 2021

Summary

Traffic management using NGINX ingress controller does not support TLS termination at the ingress controller.

Original ingress yaml:

`apiVersion: extensions/v1beta1
kind: Ingress
metadata:
annotations:
kubernetes.io/ingress.class: nginx
name: colours
spec:
rules:

  • host: www.example.com
    http:
    paths:
    • backend:
      serviceName: service-stable
      servicePort: 80
      path: /
      tls:
  • hosts:

Autogenerated yaml by Argo Rollouts:

`apiVersion: extensions/v1beta1
kind: Ingress
metadata:
annotations:
kubernetes.io/ingress.class: nginx
nginx.ingress.kubernetes.io/canary: "true"
nginx.ingress.kubernetes.io/canary-weight: "0"
name: colours-colours-canary
ownerReferences:

  • apiVersion: argoproj.io/v1alpha1
    blockOwnerDeletion: true
    controller: true
    kind: Rollout
    name: blah
    spec:
    rules:
  • host: www.example.com
    http:
    paths:
    • backend:
      serviceName: service-canary
      servicePort: 80
      path: /
      status:
      loadBalancer: {}`

You can see that the TLS section is missing.

I'd argue that the full ingress spec should be copied from the original ingress to the argo-generated ingress. That would cause the TLS section, and anything in there to be respected during the canary rollout.

Use Cases

TLS termination at the ingress controller during canary rollouts.


Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritize the issues with the most 👍.

@seventieskid seventieskid added the enhancement New feature or request label Apr 30, 2021
@geowalrus4gh
Copy link

geowalrus4gh commented Oct 7, 2021

The recommended approach for a TLS termination when we use NLB + ingress in an AWS EKS deployment is to use TLS termination at the ingress side since NLB doesn't support it straightly. So this issue will be a critical one if you use Argo Rollouts with NLB+Ingress duo in AWS EKS.

@lackofdream
Copy link

TLS configurations of canary ingress are inherited from the corresponding main ingress, please check whether the canary ingress is correctly selected by the ingress controller (spec.ingressClassName for example)

@avg07
Copy link

avg07 commented Oct 27, 2021

Same issue:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: web-ui-ingress
  annotations:
    kubernetes.io/tls-acme: "true"
    cert-manager.io/cluster-issuer: letsencrypt-prod
spec:
  ingressClassName: nginx
  rules:
    - host: stage.example.com
      http:
        paths:
         - path: /api
           pathType: Prefix
           backend:
            service:
              name: main-service-svc
              port:
                name: http
         - path: /
           pathType: Prefix
           backend:
             service:
               name: web-ui-svc
               port:
                 number: 80
  tls:
    - hosts:
        - stage.example.com
      secretName: stage-letsencrypt
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    nginx.ingress.kubernetes.io/canary: "true"
    nginx.ingress.kubernetes.io/canary-weight: "0"
  generation: 1
  name: main-service-rollout-web-ui-ingress-canary
  namespace: stage
  ownerReferences:
  - apiVersion: argoproj.io/v1alpha1
    blockOwnerDeletion: true
    controller: true
    kind: Rollout
    name: main-service-rollout
spec:
  ingressClassName: nginx
  rules:
  - host: stage.example.com
    http:
      paths:
      - backend:
          service:
            name: main-service-svc-canary
            port:
              name: http
        path: /api
        pathType: Prefix
      - backend:
          service:
            name: web-ui-svc
            port:
              number: 80
        path: /
        pathType: Prefix

kubectl version:

Client Version: version.Info{Major:"1", Minor:"22", GitVersion:"v1.22.2", GitCommit:"8b5a19147530eaac9476b0ab82980b4088bbc1b2", GitTreeState:"clean", BuildDate:"2021-09-15T21:38:50Z", GoVersion:"go1.16.8", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"22", GitVersion:"v1.22.2", GitCommit:"8b5a19147530eaac9476b0ab82980b4088bbc1b2", GitTreeState:"clean", BuildDate:"2021-09-15T21:32:41Z", GoVersion:"go1.16.8", Compiler:"gc", Platform:"linux/amd64"}

kubectl argo rollouts version:

kubectl-argo-rollouts: v1.1.0+ff3471a
  BuildDate: 2021-10-11T20:19:55Z
  GitCommit: ff3471a2fc3ccb90dbb1f370d7e399ff3064043a
  GitTreeState: clean
  GoVersion: go1.16.3
  Compiler: gc
  Platform: linux/amd64

ingress-nginx-controller:

image: k8s.gcr.io/ingress-nginx/controller:v1.0.4@sha256:545cff00370f28363dad31e3b59a94ba377854d3a11f18988f5f9e56841ef9ef

@rarecrumb
Copy link

TLS configurations of canary ingress are inherited from the corresponding main ingress, please check whether the canary ingress is correctly selected by the ingress controller (spec.ingressClassName for example)

This doesn't seem to be the case. I'm using cert-manager with CloudFlare Origin Issuer and the TLS values are not inherited by the canary ingress.

Same issue:

# Stable Ingress
apiVersion: v1
items:
- apiVersion: networking.k8s.io/v1
  kind: Ingress
  metadata:
    annotations:
      cert-manager.io/issuer: origin-issuer
      cert-manager.io/issuer-group: cert-manager.k8s.cloudflare.com
      cert-manager.io/issuer-kind: OriginIssuer
      external-dns.alpha.kubernetes.io/cloudflare-proxied: "true"
    creationTimestamp: "2022-07-22T20:10:32Z"
    generation: 1
    labels:
      app.kubernetes.io/instance: the-canary
      app.kubernetes.io/managed-by: Helm
      app.kubernetes.io/name: the-canary
      app.kubernetes.io/version: 1.16.0
      argocd.argoproj.io/instance: the-canary
      helm.sh/chart: the-canary
    name: the-canary
    namespace: test
    resourceVersion: "69952539"
    uid: x
  spec:
    ingressClassName: nginx
    rules:
    - host: myhostname.net
      http:
        paths:
        - backend:
            service:
              name: the-canary
              port:
                number: 1001
          path: /
          pathType: ImplementationSpecific
    tls:
    - hosts:
      - myhostname.org
      secretName: the-canary-origin-cert
  status:
    loadBalancer:
      ingress:
      - hostname: x.elb.amazonaws.com

# Canary Ingress 
- apiVersion: networking.k8s.io/v1
  kind: Ingress
  metadata:
    annotations:
      nginx.ingress.kubernetes.io/canary: "true"
      nginx.ingress.kubernetes.io/canary-by-header: X-Canary
      nginx.ingress.kubernetes.io/canary-by-header-value: "true"
      nginx.ingress.kubernetes.io/canary-weight: "0"
    creationTimestamp: "2022-10-24T02:17:53Z"
    generation: 1
    name: the-canary
    namespace: test
    ownerReferences:
    - apiVersion: argoproj.io/v1alpha1
      blockOwnerDeletion: true
      controller: true
      kind: Rollout
      name: the-canary
      uid: x
    resourceVersion: x
    uid: x
  spec:
    ingressClassName: nginx
    rules:
    - host: myhostname.net
      http:
        paths:
        - backend:
            service:
              name: the-canary
              port:
                number: 1001
          path: /
          pathType: ImplementationSpecific
  status:
    loadBalancer:
      ingress:
      - hostname: x.elb.amazonaws.com
kind: List
metadata:
  resourceVersion: ""

@github-actions
Copy link
Contributor

This issue is stale because it has been open 60 days with no activity.

@rarecrumb
Copy link

TLS configurations of canary ingress are inherited from the corresponding main ingress, please check whether the canary ingress is correctly selected by the ingress controller (spec.ingressClassName for example)

This doesn't seem to be the case. I'm using cert-manager with CloudFlare Origin Issuer and the TLS values are not inherited by the canary ingress.

Same issue:

Still an issue.

@rarecrumb
Copy link

@pfyod
Copy link
Contributor

pfyod commented Mar 23, 2023

@leoluz: since you're the author of the code / comment from @rarecrumb's question...

// Set up canary ingress resource, we do not have to duplicate spec.tls in a canary, only
// spec.rules

May I ask what is the reasoning behind this? Not copying the spec.tls section makes the canary ingress not functional, if the original ingress does have spec.tls.

andyliuliming pushed a commit to andyliuliming/argo-rollouts that referenced this issue Jun 11, 2023
…gration. Fixes argoproj#1134 (argoproj#2679)

* Retain TLS configuration for canary ingresses in the nginx integration

Signed-off-by: Pavels Fjodorovs <[email protected]>

* Retain TLS configuration for canary ingresses in the nginx integration

Signed-off-by: Pavels Fjodorovs <[email protected]>

* fixed tests after multiple nginx ingress merge

Signed-off-by: Pavels Fjodorovs <[email protected]>

---------

Signed-off-by: Pavels Fjodorovs <[email protected]>
Signed-off-by: Pavels Fjodorovs <[email protected]>
andyliuliming pushed a commit to andyliuliming/argo-rollouts that referenced this issue Jul 22, 2023
…gration. Fixes argoproj#1134 (argoproj#2679)

* Retain TLS configuration for canary ingresses in the nginx integration

Signed-off-by: Pavels Fjodorovs <[email protected]>

* Retain TLS configuration for canary ingresses in the nginx integration

Signed-off-by: Pavels Fjodorovs <[email protected]>

* fixed tests after multiple nginx ingress merge

Signed-off-by: Pavels Fjodorovs <[email protected]>

---------

Signed-off-by: Pavels Fjodorovs <[email protected]>
Signed-off-by: Pavels Fjodorovs <[email protected]>
Signed-off-by: Liming Liu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request no-issue-activity
Projects
None yet
Development

No branches or pull requests

6 participants