knative / serving

Kubernetes-based, scale-to-zero, request-driven compute
https://knative.dev/docs/serving/
Apache License 2.0
5.54k stars 1.15k forks source link

Serving with autoTLS and auto redirect to HTTPS does not work #12527

Open dharsanb opened 2 years ago

dharsanb commented 2 years ago

What version of Knative?

1.1

Expected Behavior

When a new service is deployed, service URL should be served in HTTP and once HTTPS certificate validation is done, HTTPS and auto redirect to HTTPS should be enabled.

Actual Behavior

When autoTLS with auto redirect to HTTPS is enabled, the service does not work because before autoTLS verification could be done, auto redirect redirects all requests to HTTPS which does not work because verification for the HTTPS is not yet done. The service does not become active and certificate validation goes in a infinite loop and it also hits lets-encrypt rate limit.

Steps to Reproduce the Problem

The following Knative deployment is done on DigitalOcean Managed Kubernetes cluster The Domain DNS records are present in DigitalOcean domains

1. Install Knative serving using yaml

kubectl apply -f https://github.com/knative/serving/releases/download/knative-v1.1.0/serving-crds.yaml
kubectl apply -f https://github.com/knative/serving/releases/download/knative-v1.1.0/serving-core.yaml

2. Install Kourier network layer

kubectl apply -f https://github.com/knative/net-kourier/releases/download/knative-v1.1.0/kourier.yaml

kubectl patch configmap/config-network \
  --namespace knative-serving \
  --type merge \
  --patch '{"data":{"ingress-class":"kourier.ingress.networking.knative.dev"}}'

kubectl --namespace kourier-system get service kourier

3. Direct Knative to use the domain

kubectl patch configmap/config-domain \
  --namespace knative-serving \
  --type merge \
  --patch '{"data":{"test.voiceintern.com":""}}'

4. Enabling TLS with HTTP01

kubectl apply -f https://github.com/knative/net-http01/releases/download/knative-v1.1.0/release.yaml

kubectl patch configmap/config-network \
  --namespace knative-serving \
  --type merge \
  --patch '{"data":{"certificate-class":"net-http01.certificate.networking.knative.dev", "auto-tls":"Enabled", "http-protocol":  "redirected"}}'

5. Deploying sample app and testing

kn service create hello \
  --image gcr.io/knative-samples/helloworld-go \
  --port 8080 \
  --env TARGET=World \
  --revision-name=world

After this step, the output is always

$ kn services list -A
NAMESPACE   NAME    URL                                          LATEST        AGE   CONDITIONS   READY     REASON
default     hello   https://hello.default.test.voiceintern.com   hello-world   10m   1 OK / 3     Unknown   CertificateNotReady : Certificate route-139798f4-02d6-42b9-b7a0-5fdf596dbe5a is not ready.
skonto commented 2 years ago

/cc @nak3

sidharthramesh commented 2 years ago

Any idea of how to fix this issue?

nak3 commented 2 years ago

Does this happen only on HTTP01? If so, there seems to be a limitation as described in https://knative.dev/docs/serving/using-auto-tls/#turn-on-auto-tls

When using HTTP-01 challenge, http-protocol field has to be set to Enabled to make sure HTTP-01 challenge requests can be accepted by the cluster.

sidharthramesh commented 2 years ago

Hey @nak3. Yes, this happens only on HTTP01. However, I think the solution is simple - Just wait for the certificates to renew, and once the status.url changes to "https", start redirecting all traffic to HTTPS.

I tried to write a controller that will do this and it's working (at least on my local cluster, need to test in production). Here is the code: https://github.com/medblocks/knative-https-controller/blob/bd7c5509f6345c7b78bafdb6060944425dfe5403/controllers/service_controller.go#L58-L65

My main question is, is this something that is part of the knative serving spec, or something that happens at the level of the ingress - i.e Kourier?

nak3 commented 2 years ago

Sorry I am not familiar with http01 stuff. cc to Zhimin who implemented it.

/cc @ZhiminXiang

ZhiminXiang commented 2 years ago

Good catch. autoTLS + HTTPS redirection is definitely an issue.

I think the logic in the https://github.com/knative/serving/issues/12527#issuecomment-1112944359 is right. This logic should happens at Route controller and DomainMapping controller when deciding the HTTP option.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

sidharthramesh commented 1 year ago

Can we please not mark this as stale?

sidharthramesh commented 1 year ago

/reopen

knative-prow[bot] commented 1 year ago

@sidharthramesh: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to [this](https://github.com/knative/serving/issues/12527#issuecomment-1311305899): >/reopen 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.
sidharthramesh commented 1 year ago

@DharsanB please reopen this?

dharsanb commented 1 year ago

/reopen

knative-prow[bot] commented 1 year ago

@DharsanB: Reopened this issue.

In response to [this](https://github.com/knative/serving/issues/12527#issuecomment-1311307290): >/reopen 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.
dprotaso commented 1 year ago

/lifecycle frozen

dprotaso commented 1 year ago

/triage accepted

mjka-sclable commented 1 year ago

I came across the same issue using Knative Serving v1.8.0 and Istio 1.15.3 as network layer. Exact same behavior as described by @dharsanb.

After some digging it seems the issue is related to the created Gateway during the service creation, which has tls.httpsRedirect: true set, but no 443 endpoint defined:

apiVersion: networking.istio.io/v1beta1
kind: Gateway
metadata:
  creationTimestamp: "2022-11-17T14:02:55Z"
  generation: 1
  labels:
    networking.internal.knative.dev/ingress: kn-test3
  name: kn-test3-3797421420
  namespace: default
  ownerReferences:
  - apiVersion: networking.internal.knative.dev/v1alpha1
    blockOwnerDeletion: true
    controller: true
    kind: Ingress
    name: kn-test3
    uid: 6c2f0e38-663b-473a-b9b0-0a14f237f0df
  resourceVersion: "20029848"
  uid: 3a28b2a5-9acc-49f0-bb95-6eea3defc8ac
spec:
  selector:
    app: istio-ingressgateway
    istio: ingressgateway
  servers:
  - hosts:
    - kn-test3.default.x.x.x.x.sslip.io
    port:
      name: http-server
      number: 80
      protocol: HTTP
    tls:
      httpsRedirect: true

This leads to a failing request to kn-test3.default.x.x.x.x.sslip.io/.well-known/acme-challenge/<token> during the HTTP-01 ACME challenge:

curl kn-test3.default.x.x.x.x.sslip.io/.well-known/acme-challenge/<token> -L 
...
< location: https://kn-test3.default.x.x.x.x.sslip.io/.well-known/acme-challenge/<token>
...
curl: (35) OpenSSL SSL_connect: Connection reset by peer in connection

My current workaround is to simply edit the Gateway directly and disable https redirection:

apiVersion: networking.istio.io/v1beta1
kind: Gateway
metadata:
  ...
  name: kn-test3-3797421420
spec:
  selector:
    app: istio-ingressgateway
    istio: ingressgateway
  servers:
    ...
    tls:
      httpsRedirect: false 

Re-apply it and after the certificate was successfully issued the Gateway updates and re-enables the https redirection again. So in case of Istio a way for fix this could be to omit the https redirection until the certificate was successfully issued, or?

felipereyel commented 12 months ago

Is there any development in this case?

I am facing this exact issue with domain mappings

nak3 commented 10 months ago

FYI - @dprotaso @ReToCode As per totday's Serving Meeting Notes, net-http01 might be deprecated in the future. But this issue is a net-http01 and I think it is still active.

ReToCode commented 10 months ago

Just for context, the plan is to use net-certmanager and use the http01 implementation of cert-manager to achieve the same outside of the context of Knative.

msaustral commented 7 months ago

Hi we have implemented knative-serving by operator with kourier

We get a service and route url with https but the traffic from the envoy to the the app container is still over http

400 Bad Request
The plain HTTP request was sent to HTTPS port
nginx

we need to work nginx over https

How can we enable https between the proxy and the app?

this is the setting:

apiVersion: operator.knative.dev/v1beta1
kind: KnativeServing
metadata:
  name: knative-serving
  namespace: knative-serving
spec:
  config:
    domain:
      xxx.com: "|
        selector:
          app: web-xxx"
    network:
      ingress-class: kourier.ingress.networking.knative.dev
      certificate-class: net-http01.certificate.networking.knative.dev
      external-domain-tls: Enabled
      domain-template: '{{.Domain}}'
      default-external-scheme: https
      autocreate-cluster-domain-claims: "true"
      cluster-local-domain-tls: Enabled
      system-internal-tls: Enabled
      #http-protocol: Redirected
      auto-tls: Enabled
    features:
      kubernetes.podspec-persistent-volume-claim: enabled
      kubernetes.podspec-persistent-volume-write: enabled
      secure-pod-defaults: enabled
      kubernetes.podspec-affinity: enabled
      kubernetes.podspec-tolerations: enabled
      kubernetes.podspec-nodeselector: enabled
      kubernetes.podspec-securitycontext: enabled
      kubernetes.podspec-topologyspreadconstraints: enabled
      kubernetes.podspec-shareprocessnamespace: enabled
      autodetect-http2: enabled
    high-availability:
      replicas: "2"
    kourier:
      cluster-cert-secret: kourier-cert
    certmanager:
      issuer: |
        kind: ClusterIssuer
        name: letsencrypt-http01-issuer
  workloads:
  - name: webhook
    tolerations:
    - key: CriticalAddonsOnly
      operator: Exists
    affinity:
      podAntiAffinity:
        preferredDuringSchedulingIgnoredDuringExecution:
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  app: webhook
              topologyKey: kubernetes.io/hostname
            weight: 100
      nodeAffinity:
        requiredDuringSchedulingIgnoredDuringExecution:
          nodeSelectorTerms:
          - matchExpressions:
            - key: use
              operator: In
              values:
              - system
  - name: net-kourier-controller
    tolerations:
    - key: CriticalAddonsOnly
      operator: Exists
    affinity:
      podAntiAffinity:
        preferredDuringSchedulingIgnoredDuringExecution:
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  app: net-kourier-controller
              topologyKey: kubernetes.io/hostname
            weight: 100
      nodeAffinity:
        requiredDuringSchedulingIgnoredDuringExecution:
          nodeSelectorTerms:
          - matchExpressions:
            - key: use
              operator: In
              values:
              - system
  - name: 3scale-kourier-gateway
    tolerations:
    - key: CriticalAddonsOnly
      operator: Exists
    affinity:
      podAntiAffinity:
        preferredDuringSchedulingIgnoredDuringExecution:
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  app: 3scale-kourier-gateway
              topologyKey: kubernetes.io/hostname
            weight: 100
      nodeAffinity:
        requiredDuringSchedulingIgnoredDuringExecution:
          nodeSelectorTerms:
          - matchExpressions:
            - key: use
              operator: In
              values:
              - system
  - name: operator-webhook
    tolerations:
    - key: CriticalAddonsOnly
      operator: Exists
    affinity:
      podAntiAffinity:
        preferredDuringSchedulingIgnoredDuringExecution:
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  app: operator-webhook
              topologyKey: kubernetes.io/hostname
            weight: 100
      nodeAffinity:
        requiredDuringSchedulingIgnoredDuringExecution:
          nodeSelectorTerms:
          - matchExpressions:
            - key: use
              operator: In
              values:
              - system
  - name: activator
    tolerations:
    - key: CriticalAddonsOnly
      operator: Exists
    affinity:
      podAntiAffinity:
        preferredDuringSchedulingIgnoredDuringExecution:
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  app: activator
              topologyKey: kubernetes.io/hostname
            weight: 100
      nodeAffinity:
        requiredDuringSchedulingIgnoredDuringExecution:
          nodeSelectorTerms:
          - matchExpressions:
            - key: use
              operator: In
              values:
              - system
  - name: autoscaler-hpa
    tolerations:
    - key: CriticalAddonsOnly
      operator: Exists
    affinity:
      podAntiAffinity:
        preferredDuringSchedulingIgnoredDuringExecution:
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  app: autoscaler-hpa
              topologyKey: kubernetes.io/hostname
            weight: 100
      nodeAffinity:
        requiredDuringSchedulingIgnoredDuringExecution:
          nodeSelectorTerms:
          - matchExpressions:
            - key: use
              operator: In
              values:
              - system
  - name: controller
    tolerations:
    - key: CriticalAddonsOnly
      operator: Exists
    affinity:
      podAntiAffinity:
        preferredDuringSchedulingIgnoredDuringExecution:
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  app: controller
              topologyKey: kubernetes.io/hostname
            weight: 100
      nodeAffinity:
        requiredDuringSchedulingIgnoredDuringExecution:
          nodeSelectorTerms:
          - matchExpressions:
            - key: use
              operator: In
              values:
              - system
  - name: autoscaler
    tolerations:
    - key: CriticalAddonsOnly
      operator: Exists
    affinity:
      podAntiAffinity:
        preferredDuringSchedulingIgnoredDuringExecution:
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  app: autoscaler
              topologyKey: kubernetes.io/hostname
            weight: 100
      nodeAffinity:
        requiredDuringSchedulingIgnoredDuringExecution:
          nodeSelectorTerms:
          - matchExpressions:
            - key: use
              operator: In
              values:
              - system
  ingress:
    kourier:
      enabled: true
  services:
  - name: kourier
    annotations:
      service.beta.kubernetes.io/do-loadbalancer-protocol: "http2"
      service.beta.kubernetes.io/do-loadbalancer-tls-passthrough: "true"
      service.beta.kubernetes.io/do-loadbalancer-size-unit: "2"
      service.beta.kubernetes.io/do-loadbalancer-http2-ports: "443"
      service.beta.kubernetes.io/do-loadbalancer-enable-backend-keepalive: "true"
      service.beta.kubernetes.io/do-loadbalancer-http-idle-timeout-seconds: "120"