zalando / skipper

An HTTP router and reverse proxy for service composition, including use cases like Kubernetes Ingress
https://opensource.zalando.com/skipper/
Other
3.09k stars 350 forks source link

kubernetes: ingress class appears to be ignored in route printing #1028

Open antoniobeyah opened 5 years ago

antoniobeyah commented 5 years ago

Describe the bug When specifying an ingress class skipper appears to be creating routes for ingress classes that don't match the specified ingress class

To Reproduce

  1. Deploy skipper with -kubernetes-ingress-class=skipper-ingress-public
  2. Deploy the following manifests:
{
  "apiVersion": "v1",
  "items": [
    {
      "apiVersion": "extensions/v1beta1",
      "kind": "Ingress",
      "metadata": {
        "annotations": {
          "kubernetes.io/ingress.class": "skipper-ingress-internal",
          "namespace": "default"
        },
        "name": "examplebackend-skipper-ingress-internal",
        "namespace": "default"
      },
      "spec": {
        "rules": [
          {
            "host": "example.example.com",
            "http": {
              "paths": [
                {
                  "backend": {
                    "serviceName": "examplebackend",
                    "servicePort": 80
                  }
                }
              ]
            }
          }
        ]
      }
    },
    {
      "apiVersion": "extensions/v1beta1",
      "kind": "Ingress",
      "metadata": {
        "annotations": {
          "kubernetes.io/ingress.class": "skipper-ingress-private",
          "namespace": "default"
        },
        "name": "examplebackend-skipper-ingress-private",
        "namespace": "default"
      },
      "spec": {
        "rules": [
          {
            "host": "example.example.com",
            "http": {
              "paths": [
                {
                  "backend": {
                    "serviceName": "examplebackend",
                    "servicePort": 80
                  }
                }
              ]
            }
          }
        ]
      }
    },
    {
      "apiVersion": "v1",
      "kind": "Service",
      "metadata": {
        "labels": {
          "app": "examplebackend"
        },
        "name": "examplebackend",
        "namespace": "default"
      },
      "spec": {
        "ports": [
          {
            "name": "external",
            "port": 80,
            "protocol": "TCP",
            "targetPort": 8080
          }
        ],
        "selector": {
          "app": "examplebackend"
        },
        "type": "ClusterIP"
      }
    }
  ],
  "kind": "List",
  "metadata": {
    "resourceVersion": "",
    "selfLink": ""
  }
}
  1. Review the routes using the support listener http://localhost:9911/routes

Expected behavior no routes created

Observed behavior

kube_default__examplebackend_skipper_ingress_internal__example_example_com____examplebackend: Host(/^example[.]example[.]com$/)
  -> "http://10.12.175.220:8080";

Other Notes Even though skipper is reporting a route from an incorrect ingress class you are unable to reach it when manually setting the host header

szuecs commented 5 years ago

Thanks for reporting the issue with all the details!

As far as I can see in the code the log message correlates with the "Other Notes". Skipper will not create routes for this ingress during the LoadAll nor the LoadUpdate operation in the kubernetes dataclient. As far as I understand the question is why you see the incorrect behavior in the route printing endpoint, which sounds like a bug to me. Do you had the ingress without annotation first and later added?

antoniobeyah commented 5 years ago

Do you had the ingress without annotation first and later added?

No the ingress always had the annotation. It was discovered when we were trying to track down an inconsistent 404 that we were seeing.

szuecs commented 5 years ago

@antoniobeyah what is the priority in your opinion for fixing this bug? Is this blocking you or more "nice to have"?

antoniobeyah commented 5 years ago

Its probably more of a "nice to have" than a need- here is what I was trying to do:

We are intermittently seeing 404s to different endpoints behind skipper and we are unsure of why that is happening. What I was going to attempt to do is to write something that loaded the skipper routes and compared them across all of our skipper instances to ensure they are consistent.

I am unsure how to create a monitor to ensure that each skipper is identical if we are unable to hit the /routes endpoint and get consistent results.

A bigger concern I have is that the /routes endpoint doesn't match the actual routing table- it is unclear to me how skipper can get into that state.

To provide some context, we run 3 skippers:

1) internet facing - publicly available 2) internal facing - still public, but restricted to internal networks 3) private facing - rfc1918 space

For each of those skippers the expectation is that there are no routes to different ingress classes. Hitting the /routes endpoint and seeing internal routes within the public skipper is very concerning.

From an audit perspective it would likely be a High priority given the intent of different skippers for different app classifications.

szuecs commented 5 years ago

@antoniobeyah thanks for giving some context. Right now I am working on another reliability issue that we found, which has higher priority, but I also share your concerns. A simple trick to find out who is responding 404 is to check Server header and if your backend sets it, it is reliable to use if skipper returns 404 or the backend.

ruudk commented 5 years ago

I have the same problem. I'm using a public and private one and both skipper pods return the same routes.

szuecs commented 5 years ago

@ruudk would you mind sharing skipper version and flags you run with?

ruudk commented 5 years ago

@szuecs

name: private-skipper-ingress
image: registry.opensource.zalan.do/pathfinder/skipper:v0.10.273
ports:
    -   name: ingress-port
        containerPort: 9997
        hostPort: 9997
    -   name: metrics-port
        containerPort: 9909
args:
    - "skipper"
    - "-kubernetes"
    - "-kubernetes-in-cluster"
    - "-address=:9997"
    - "-application-log-level=INFO"
    - "-wait-first-route-load"
    - "-proxy-preserve-host"
    - "-serve-host-metrics"
    - "-enable-ratelimits"
    - "-experimental-upgrade"
    - "-metrics-exp-decay-sample"
    - "-reverse-source-predicate"
    - "-lb-healthcheck-interval=3s"
    - "-metrics-flavour=codahale,prometheus"
    - "-enable-connection-metrics"
    - "-max-audit-body=0"
    - "-kubernetes-ingress-class=private"
szuecs commented 5 years ago

@ruudk and you annotate all your ingress with kubernetes.io/ingress.class: private and kubernetes.io/ingress.class: public ?

All ingress without ingress.class will be in all different skipper routes.

ruudk commented 5 years ago

@szuecs Yes

szuecs commented 5 years ago

@ruudk I just tested with registry.opensource.zalan.do/pathfinder/skipper:v0.10.282 with - "-kubernetes-ingress-class=private" to skipper and added to one ingress kubernetes.io/ingress.class: public and to another one kubernetes.io/ingress.class: private. I can not reproduce your issue.

I get no route from public annotated ingress in the route table print curl localhost:9911/routes and get as a result a 404. The kubernetes.io/ingress.class: private annotated ingress shows up in the route table print and returns the result as expected.

I also know, that shopgun runs a setup with 3 different "environment" separated controllers and that it works for them.

Something seems to be not correctly done or the expectation is different from different people. What I can tell is that all ingress that are not annotated are served by all skipper ingress controllers. Maybe this is what you observe, if not please provide steps to reproduce the observed behavior.

maelvls commented 2 years ago

I made a discovery today. It is most probably not what cause @antoniobeyah's and @ruudk's issue, but I think it is worth mentioning it here since the title of this issue relates 100% to the symptom I was seeing.

I was running two versions of Skipper:

Instance Command-line argument
Skipper 1 -kubernetes-ingress-class=skipper
Skipper 2 -kubernetes-ingress-class=skipper-internal

But when creating an Ingress with skipper set, both instances would pick up the Ingress:

kind: Ingress
metadata:
  annotations:
    kubernetes.io/ingress.class: skipper

I realized that the command-line argument -kubernetes-ingress-class is actually matched using a regex, not "just" the string equality.

In order to fix this, I had to change the configuration of my Skippers to:

Instance Command-line argument
Skipper 1 -kubernetes-ingress-class="^skipper$"
Skipper 2 -kubernetes-ingress-class="^skipper-internal$"
szuecs commented 2 years ago

@maelvls interesting, I guess this needs a documentation fix. The CLI help message also shows it, but user docs seem to be not good enough, because they only say <string>.

  -kubernetes-ingress-class string
        ingress class regular expression used to filter ingress resources for kubernetes