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

Prevent by default ingress hostname collision #218

Closed
bsctl opened this issue Mar 5, 2021 · 5 comments
Closed

Prevent by default ingress hostname collision #218

bsctl opened this issue Mar 5, 2021 · 5 comments
Assignees
Labels
breaking-change enhancement New feature or request
Milestone

Comments

@bsctl
Copy link
Member

bsctl commented Mar 5, 2021

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

@bsctl bsctl added enhancement New feature or request v0.1.0 To be implemented/hotfixed for 0.1.0 labels Mar 5, 2021
@MaxFedotov
Copy link
Collaborator

MaxFedotov commented Mar 6, 2021

@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
Copy link
Member Author

bsctl commented Mar 6, 2021

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

  • 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

@bsctl
Copy link
Member Author

bsctl commented Mar 6, 2021

@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 prometherion added this to the v0.1.0 milestone May 4, 2021
@prometherion prometherion removed the v0.1.0 To be implemented/hotfixed for 0.1.0 label May 4, 2021
@prometherion
Copy link
Member

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
Copy link
Member Author

bsctl commented Jul 23, 2021

We can close this, already fixed with #269.

Closed by #269

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants