Ingress hostname collision check at Tenant level

[bsctl]

Problem statement

Going to think about how to refactor the ingress hostname collision checks in Capsule since it looks there is still a grey area there

• Prevent by default ingress hostname collision #218 (comment)
• Prevent by default ingress hostname collision #218 (comment)
• Prevent by default ingress hostname collision #218 (comment)

Capsule in v1beta1 implements two different kinds of check for hostname collision

  allowTenantIngressHostnamesCollision: false # default
  allowIngressHostnameCollision: false # default

Option allowTenantIngressHostnamesCollision

This implement is a check on the Tenant custom resource creation about the allowed list of tenant.spec.ingressHostnames.allowed preventing the cluster admin to create two tenants using the same allowed hostname list:

kubectl apply -f - <<EOF
apiVersion: capsule.clastix.io/v1beta1
kind: Tenant
metadata:
  name: gold
spec:
  owners:
  - kind: User
    name: alice
  ingressHostnames:
    allowed:
    - app.clastix.io
---
apiVersion: capsule.clastix.io/v1beta1
kind: Tenant
metadata:
  name: silver
spec:
  owners:
  - kind: User
    name: alice
  ingressHostnames:
    allowed:
    - app.clastix.io
EOF

tenant.capsule.clastix.io/gold created

Error from server (the allowed hostname app.clastix.io is already used by the Tenant gold, cannot proceed): error when creating "STDIN": admission webhook "tenants.capsule.clastix.io" denied the request: the allowed hostname app.clastix.io is already used by the Tenant gold, cannot proceed

Only the first tenant is created. However, the allowTenantIngressHostnamesCollision check does not apply to regex

kubectl apply -f - <<EOF
apiVersion: capsule.clastix.io/v1beta1
kind: Tenant
metadata:
  name: gold
spec:
  owners:
  - kind: User
    name: alice
  ingressHostnames:
    allowedRegex: ".clastix.io$"
---
apiVersion: capsule.clastix.io/v1beta1
kind: Tenant
metadata:
  name: silver
spec:
  owners:
  - kind: User
    name: alice
  ingressHostnames:
    allowedRegex: ".clastix.io$"
EOF

tenant.capsule.clastix.io/gold created
tenant.capsule.clastix.io/silver created

Both the gold and silver tenants above can now use the same domain clastix.io.

Option allowIngressHostnameCollision

As tenant admin, let's to create a couple of namespaces in each tenant:

kubectl --as alice --as-group capsule.clastix.io create ns gold-production
kubectl --as alice --as-group capsule.clastix.io create ns gold-development
kubectl --as alice --as-group capsule.clastix.io create ns silver-production
kubectl --as alice --as-group capsule.clastix.io create ns silver-development

Working on both the tenants

kubectl --as alice --as-group capsule.clastix.io apply -f - <<EOF
apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: example
  namespace: gold-production
spec:
  rules:
    - host: webapp.clastix.io
      http:
        paths:
          - path: /
            backend:
              serviceName: webapp
              servicePort: 80
---
apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: example
  namespace: silver-production
spec:
  rules:
    - host: webapp.clastix.io
      http:
        paths:
          - path: /
            backend:
              serviceName: webapp
              servicePort: 80
EOF
ingress.networking.k8s.io/example created
Error from server (hostname webapp.clastix.io is already used across the cluster: please, reach out to the system administrators): error when creating "STDIN": admission webhook "ingress.capsule.clastix.io" denied the request: hostname webapp.clastix.io is already used across the cluster: please, reach out to the system administrators

Good job! Capsule prevents the same webapp.clastix.io hostname across namespaces because of the allowIngressHostnameCollision=false.

Please note the other option allowTenantIngressHostnamesCollision=false does not play here.

Working on the gold-production tenant only

kubectl --as alice --as-group capsule.clastix.io delete ing --all -n gold-production
kubectl --as alice --as-group capsule.clastix.io apply -f - <<EOF
apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: example
  namespace: gold-production
spec:
  rules:
    - host: goldapp.clastix.io
      http:
        paths:
          - path: /
            backend:
              serviceName: goldapp
              servicePort: 80
    - host: goldapp.clastix.io
      http:
        paths:
          - path: /testpath
            backend:
              serviceName: test
              servicePort: 80
EOF
ingress.networking.k8s.io/example created

However, if we have to cordon the https://goldapp.clastix.io/testpath from public access

kubectl --as alice --as-group capsule.clastix.io delete ing example -n gold-production
kubectl --as alice --as-group capsule.clastix.io apply -f - <<EOF
apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: example
  namespace: gold-production
spec:
  rules:
    - host: goldapp.clastix.io
      http:
        paths:
          - path: /
            backend:
              serviceName: goldapp
              servicePort: 80
---
apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: testpath
  annotations:
    nginx.ingress.kubernetes.io/whitelist-source-range: 192.168.0.0/16
  namespace: gold-production
spec:
  rules:
    - host: goldapp.clastix.io
      http:
        paths:
          - path: /testpath
            backend:
              serviceName: test
              servicePort: 80
EOF
ingress.networking.k8s.io/example created
Error from server (hostname goldapp.clastix.io is already used across the cluster: please, reach out to the system administrators): error when creating "STDIN": admission webhook "ingress.capsule.clastix.io" denied the request: hostname goldapp.clastix.io is already used across the cluster: please, reach out to the system administrators

The second ingress is denied because of the same goldapp.clastix.io across namespaces and allowIngressHostnameCollision=false. This is very common as pointed out by @MaxFedotov.

As cluster admin, let's to allow hostname collision by putting allowIngressHostnameCollision=true in Capsule configuration.

As tenant admin, now it's possible to create two ingresses in the same namespace with the same hostname goldapp.clastix.io but different paths

kubectl --as alice --as-group capsule.clastix.io apply -f - <<EOF
apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: example
  namespace: gold-production
spec:
  rules:
    - host: goldapp.clastix.io
      http:
        paths:
          - path: /
            backend:
              serviceName: goldapp
              servicePort: 80
---
apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: testpath
  annotations:
    nginx.ingress.kubernetes.io/whitelist-source-range: 192.168.0.0/16
  namespace: gold-production
spec:
  rules:
    - host: goldapp.clastix.io
      http:
        paths:
          - path: /testpath
            backend:
              serviceName: test
              servicePort: 80
EOF
ingress.networking.k8s.io/example created
ingress.networking.k8s.io/testpath created

Now there are no checks to prevent tenant owner to:

• create two ingresses with same hostname in two different namespaces of the same tenant
• create two ingresses with same hostname in two different namespaces of two different tenants

kubectl --as alice --as-group capsule.clastix.io apply -f - <<EOF
apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: example
  namespace: gold-production
spec:
  rules:
    - host: webapp.clastix.io
      http:
        paths:
          - path: /
            backend:
              serviceName: webapp
              servicePort: 80

---
apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: testpath
  namespace: gold-development
spec:
  rules:
    - host: webapp.clastix.io
      http:
        paths:
          - path: /
            backend:
              serviceName: webapp
              servicePort: 80

---
apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  name: testpath
  namespace: silver-production
spec:
  rules:
    - host: webapp.clastix.io
      http:
        paths:
          - path: /
            backend:
              serviceName: webapp
              servicePort: 80
EOF

ingress.networking.k8s.io/example created
ingress.networking.k8s.io/testpath created
ingress.networking.k8s.io/testpath created

Proposal

Long story here to say we have to address three common scenarios, from high to low priority

1. prevent hostname collision across tenants - high
2. prevent hostname collision across namespaces of the same tenant - medium
3. prevent hostname collision across same namespace - low

Unfortunately, the current implementation of allowTenantIngressHostnamesCollision does not help here and, in general, it has a limited utility (imho).

• Change the behaviour of allowTenantIngressHostnamesCollision to prevent hostname collision across tenants at runtime (first scenario)
• Change the behaviour of allowIngressHostnameCollision to prevent hostname collision across namespaces of the same tenant (second scenario)
• Do not address the third scenario since it's more under control and responsibility of the developer than the cluster admin

@prometherion @MaxFedotov please, comment.

[prometherion]

I think we can try to address this with a fine grained control, at multiple levels.

We could do a change to the CapsuleConfiguration API that would enforce all the rules you described, @bsctl.

apiVersion: capsule.clastix.io/v1alpha1
kind: CapsuleConfiguration
spec:
  hostnameCollision:
    enabled: # boolean
    scope: # enum=Cluster;Tenant;Namespace
    strict: # bool

The spec.hostnameCollision.strict option is the toggle to enforce the strict check (no multiple Ingress resources using the same hostname) or the loose one (considering hostname and the subsequent path).

The change could be considered breaking but we haven't yet released the CRD, so we can plan this for the upcoming v0.1.0 release.

We should also consider if this must be a global configuration or if this could be implemented at Tenant level, keeping the actual CapsuleConfiguration's spec.allowTenantIngressHostnamesCollision option for the Cluster level enforcement.

[bsctl]

We should also consider if this must be a global configuration or if this could be implemented at Tenant level, keeping the actual CapsuleConfiguration's spec.allowTenantIngressHostnamesCollision option for the Cluster level enforcement.

A Tenant level check is more powerful. +1

In addition, we should consider to review the meaning of spec.allowTenantIngressHostnamesCollision option as global configuration. As seen above, it has a limited usefulness because works only on matching strings, and not on regex.

Proposal is: prevent hostname collision across tenants during Ingress provisioning not during tenant provisioning. Tenant provisioning is matter of cluster admin and it should not be managed by Capsule.

[prometherion]

prevent hostname collision across tenants during Ingress provisioning not during tenant provisioning. Tenant provisioning is matter of cluster admin and it should not be managed by Capsule.

Couldn't agree more.

This means we're going to deprecate this option in the CapsuleConfiguration CRD and moving this to the Tenant specification.

I'd suggest also a refactoring on how structuring the API, something as a new parameter, eg. ingressOptions.