Istio 1.17.0 regression with Knative DomainMappings

dprotaso commented 1 year ago

Bug Description

Downstream issue: https://github.com/knative/serving/issues/13712

Upgrading to Istio v1.17.0 has caused our DomainMapping feature to break - Istio now returns 503 no health upstream.

We have no issues with v1.16.x. The diff between our v1.17 and v1.16 deployment is here: https://github.com/knative-sandbox/net-istio/pull/1061/files#diff-d30c8c1936a00c0ef1c073c949441ea6cec0aa9d66d9a6da585ed998f642056b

Dump of our Virtual Services and K8s Services are in this gist - https://gist.github.com/dprotaso/64c53787a85b7e7cd1ec9f2b0c427bbe

I think the notable thing is one of our services uses ExternalName

Version

$ istioctl version
client version: 1.17.0
control plane version: 1.17.0
data plane version: 1.17.0 (3 proxies)

Additional Information

No response

dprotaso commented 1 year ago

It's possible to reproduce this in kind+metallb

Setup

$ kind create cluster
$ kubectl apply -f https://github.com/knative/serving/releases/download/knative-v1.9.0/serving-core.yaml
$ kubectl apply -f https://raw.githubusercontent.com/knative-sandbox/net-istio/ef720f6/third_party/istio-latest/istio-ci-no-mesh/istio.yaml
$ kubectl apply -f https://github.com/knative-sandbox/net-istio/releases/download/knative-v1.9.0/net-istio.yaml

$ kubectl patch configmap/config-domain --namespace knative-serving --type merge --patch '{"data":{"example.com":""}}'
$ kubectl patch configmap/config-network --namespace knative-serving --type merge --patch '{"data":{"autocreate-cluster-domain-claims":"true"}}'

Fixtures

$ kubectl apply -f - <<EOF
apiVersion: serving.knative.dev/v1
kind: Service
metadata:
  name: hello
spec:
  template:
    spec:
      containers:
        - image: gcr.io/knative-samples/helloworld-go
          ports:
            - containerPort: 8080
          env:
            - name: TARGET
              value: "World"
EOF
$ kubectl apply -f - <<EOF
apiVersion: serving.knative.dev/v1alpha1
kind: DomainMapping
metadata:
  name: blah.com
  namespace: default
spec:
  ref:
    name: hello
    kind: Service
    apiVersion: serving.knative.dev/v1
EOF

Run the Test

First one succeeds - second one fails

$ docker run --network kind -it --rm cgr.dev/chainguard/curl -v -H 'Host: hello.default.example.com' 172.18.255.20 # Istio Ingress LB
$ docker run --network kind -it --rm cgr.dev/chainguard/curl -v -H 'Host: blah.com' 172.18.255.20 # Istio Ingress LB

dprotaso commented 1 year ago

~Here's another bug where it returns 404. This setup is simpler as it doesn't require Knative~

(edit: the below was misconfigured as pointed out in a comment below)

Setup

Setup Kind with MetalLB and install istio with istioctl install

apiVersion: v1
kind: Namespace
metadata:
  name: knative-serving  
---
apiVersion: networking.istio.io/v1alpha3
kind: Gateway
metadata:
  name: knative-ingress-gateway
  namespace: knative-serving
  labels:
    app.kubernetes.io/component: net-istio
    app.kubernetes.io/name: knative-serving
    app.kubernetes.io/version: devel
    networking.knative.dev/ingress-provider: istio
spec:
  selector:
    istio: ingressgateway
  servers:
  - port:
      number: 80
      name: http
      protocol: HTTP
    hosts:
    - "*"
---
apiVersion: networking.istio.io/v1alpha3
kind: Gateway
metadata:
  name: knative-local-gateway
  namespace: knative-serving
  labels:
    app.kubernetes.io/component: net-istio
    app.kubernetes.io/name: knative-serving
    app.kubernetes.io/version: devel
    networking.knative.dev/ingress-provider: istio
spec:
  selector:
    istio: ingressgateway
  servers:
    - port:
        number: 8081
        name: http
        protocol: HTTP
      hosts:
        - "*"
---
apiVersion: v1
kind: Service
metadata:
  name: knative-local-gateway
  namespace: istio-system
  labels:
    app.kubernetes.io/component: net-istio
    app.kubernetes.io/name: knative-serving
    app.kubernetes.io/version: devel
    networking.knative.dev/ingress-provider: istio
    experimental.istio.io/disable-gateway-port-translation: "true"
spec:
  type: ClusterIP
  selector:
    istio: ingressgateway
  ports:
    - name: http2
      port: 80
      targetPort: 8081
---
apiVersion: v1
kind: Pod
metadata:
  name: nginx
  labels:
    app.kubernetes.io/name: proxy
spec:
  containers:
  - name: nginx
    image: nginx:stable
    ports:
      - containerPort: 80
        name: http-web-svc
---
apiVersion: v1
kind: Service
metadata:
  name: nginx
spec:
  selector:
    app.kubernetes.io/name: proxy
  ports:
  - name: http
    protocol: TCP
    port: 80
    targetPort: http-web-svc
---
apiVersion: v1
kind: Service
metadata:  
  name: nginx-internal
  namespace: default  
spec:
  externalName: knative-local-gateway.istio-system.svc.cluster.local
  ports:
  - name: http
    port: 80
    protocol: TCP
    targetPort: 80
  sessionAffinity: None
  type: ExternalName
status:
  loadBalancer: {}    
---
apiVersion: networking.istio.io/v1beta1
kind: VirtualService
metadata:
  name: nginx-ingress
  namespace: default  
spec:
  gateways:
  - knative-serving/knative-ingress-gateway
  - knative-serving/knative-local-gateway
  hosts:
  - nginx.default.example.com  
  http:
  - headers:
      request:
        set:
          K-Network-Hash: 0878975ddb3e11427b140f810d15eb3cc2993d744e2f689514280dd8e43b24c5
    match:
    - authority:
        prefix: nginx.default
      gateways:
      - knative-serving/knative-local-gateway
      headers:
        K-Network-Hash:
          exact: override
    retries: {}
    route:
    - destination:
        host: nginx.default.svc.cluster.local
        port:
          number: 80
      headers:
        request:
          set:
            Knative-Serving-Namespace: default
            Knative-Serving-Revision: hello-00001
      weight: 100
  - match:
    - authority:
        prefix: nginx.default
      gateways:
      - knative-serving/knative-local-gateway
    retries: {}
    route:
    - destination:
        host: nginx.default.svc.cluster.local
        port:
          number: 80
      weight: 100
  - headers:
      request:
        set:
          K-Network-Hash: 0878975ddb3e11427b140f810d15eb3cc2993d744e2f689514280dd8e43b24c5
    match:
    - authority:
        prefix: nginx.default.example.com
      gateways:
      - knative-serving/knative-ingress-gateway
      headers:
        K-Network-Hash:
          exact: override
    retries: {}
    route:
    - destination:
        host: nginx.default.svc.cluster.local
        port:
          number: 80
      headers:
        request:
          set:
            Knative-Serving-Namespace: default
            Knative-Serving-Revision: hello-00001
      weight: 100
  - match:
    - authority:
        prefix: nginx.default.example.com
      gateways:
      - knative-serving/knative-ingress-gateway
    retries: {}
    route:
    - destination:
        host: nginx.default.svc.cluster.local
        port:
          number: 80
      headers:
        request:
          set:
            Knative-Serving-Namespace: default
            Knative-Serving-Revision: hello-00001
      weight: 100
---
apiVersion: networking.istio.io/v1beta1
kind: VirtualService
metadata:  
  name: foo.com-ingress
  namespace: default  
spec:
  gateways:
  - knative-serving/knative-ingress-gateway
  hosts:
  - foo.com
  http:
  - match:
    - authority:
        prefix: foo.com
      gateways:
      - knative-serving/knative-ingress-gateway
    retries: {}
    rewrite:
      authority: nginx-internal.default.svc.cluster.local
    route:
    - destination:
        host: nginx-internal.default.svc.cluster.local
        port:
          number: 80
      headers:
        request:
          set:
            K-Original-Host: foo.com
      weight: 100

Run the test

Fix one succeeds - second one does not

$ docker run --network kind -it --rm cgr.dev/chainguard/curl -v -H 'Host: nginx.default.example.com' 172.18.255.200
$ docker run --network kind -it --rm cgr.dev/chainguard/curl -v -H 'Host: foo.com' 172.18.255.200

dddddai commented 1 year ago

Here's another bug where it returns 404. This setup is simpler as it doesn't require Knative

I think this one is expected, as in the foo.com-ingress VS the authority is rewrited to nginx-internal.default.svc.cluster.local while the nginx-ingress VS matches nginx.default.example.com

dprotaso commented 1 year ago

Here's another bug where it returns 404. This setup is simpler as it doesn't require Knative

I think this one is expected, as in the foo.com-ingress VS the authority is rewrited to nginx-internal.default.svc.cluster.local while the nginx-ingress VS matches nginx.default.example.com

Ah yeah I see I'm missing the entry in hosts

dprotaso commented 1 year ago

Hey curious when istio has patch releases?

On Mon, Feb 20, 2023 at 09:37 Istio Automation @.***> wrote:

Closed #43440 https://github.com/istio/istio/issues/43440 as completed via #43463 https://github.com/istio/istio/pull/43463.

— Reply to this email directly, view it on GitHub https://github.com/istio/istio/issues/43440#event-8562410011, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAERAQR36JREPHBBAHYVKTWYN6RPANCNFSM6AAAAAAU7URTXI . You are receiving this because you authored the thread.Message ID: @.***>

dddddai commented 1 year ago

FYI it's out https://istio.io/latest/news/releases/1.17.x/announcing-1.17.1/

istio / istio