projectcapsule / capsule

Multi-tenancy and policy-based framework for Kubernetes.
https://capsule.clastix.io
Apache License 2.0
1.56k stars 153 forks source link

Prevent by default ingress hostname collision #218

Closed bsctl closed 3 years ago

bsctl commented 3 years ago

Describe the feature

Currently it is possible to create multiple ingresses with same ingress hostname across different tenants and within the same tenant as well.

What would the new user story look like?

A new CLI flag --allow-ingress-hostname-collision is added, defaulted to false since this seems the most common case. Capsule will check if the Ingress hostname is already assigned to another ingress across different tenants and within the same tenant as well.

The check happens only in the Namespace resources managed by Capsule.

Expected behavior

A configurable way to avoid ingress hostname collision

MaxFedotov commented 3 years ago

@bsctl, and can we limit this check to be only across different tenants\namespaces?

On ingress-nginx, it's a quite common situation, when you had to create 2 ingresses with the same host and different paths. For example, if we had an application deployed, and we want to apply whitelists only for a single path, for example to make /adminbe available only on internal network. Then we had to create 2 ingresses like:

---
kind: Ingress
metadata:
  name: main
  namespace: myns
spec:
  rules:
  - host: oil.corp.com
    http:
      paths:
      - backend:
          serviceName: net-oil
          servicePort: http
        path: /
---
kind: Ingress
metadata:
  annotations:
    nginx.ingress.kubernetes.io/whitelist-source-range: 192.168.0.0/16
  name: admin
  namespace: myns
spec:
  rules:
  - host: oil.corp.com
    http:
      paths:
      - backend:
          serviceName: net-oil
          servicePort: http
        path: /admin

And there are a lot of cases like this, as the only way to work with nginx.conf is via annotations. And sometimes you had to apply different annotations to different path for the same host

bsctl commented 3 years ago

@MaxFedotov that'a good point. It looks like there are different levels where we can prevent ingress hostname collision:

bsctl commented 3 years ago

@prometherion @MaxFedotov one more reason we should go to #51 All the cases above could be easily addressed with a programmable policy engine, case by case, without trying to guess all possible scenarios and put the logic in Capsule.

prometherion commented 3 years ago

We can close this, already fixed with #269.

However, this is interesting:

  • prevent collision across tenants -> very critical requirement in a public CaaS
  • prevent collision across namespaces of the same tenant
  • prevent collision in the same namespace

The first option is already put in place by the CRD key allowTenantIngressHostnamesCollision.

The latter ones can be easily implemented at the Tenant level: WDYT?

bsctl commented 3 years ago

We can close this, already fixed with #269.

Closed by #269