capsule icon indicating copy to clipboard operation
capsule copied to clipboard

feat: support additional resources across tenant namespaces

Open gernest opened this issue 3 years ago • 2 comments

This attempts to address #525 which is related to #416.

problem this commit is trying to solve

Alice would like to create several resources on each of their Namespaces.
Upon the creation of each Namespace, they have to create the desired resources on each Namespace

On the above linked tickets there are two proposed approaches

approach 01

Create a new resource TenantResource something like this

apiVersion: capsule.clastix.io/v1beta2
kind: TenantResource
metadata:
  name: database
  namespace: solar-management
spec:
  resyncPeriod: 60s
  additionalResources:
    namespaceSelector:
      matchLabels:
        tier: one
    items:
    - apiVersion: presslabs.io/v1beta1
      kind: MySQL
      spec:
        foo: bar
  clusterRoles:
    namespaceSelector:
      matchLabels:
        tier: one
    items:
      - name: database-admin
        subjects:
        - kind: Group
          name: developers

approach 02

Extend Tenant to support addtional resources

apiVersion: capsule.clastix.io/v1beta1
kind: Tenant
metadata:
  name: gas
spec:
  additionalResources:
    -  apiVersion: "cilium.io/v2"
       kind: CiliumNetworkPolicy
       metadata:
         name: "l3-rule"
      spec:
        endpointSelector:
          matchLabels:
             role: backend
       ingress:
       - fromEndpoints:
         - matchLabels:
             role: frontend

This commit implements approach 02 due to the following reasons

  • The namespaces belong to the tenant already, extra TenantResource seems redundant given that the lifecycle of the additional resources are tied to the Tenant.

How does the crd look like now ?

apiVersion: capsule.clastix.io/v1beta1
kind: Tenant
metadata:
  name: oil
spec:
  resyncPeriod: 60s
  additionalResources:
    namespaceSelector:
      matchLabels:
        tier: one
    items:
      - |
        apiVersion: v1
        kind: Pod
        metadata:
          name: nginx
          labels:
            app.kubernetes.io/name: proxy
        spec:
          containers:
            - name: nginx
              image: nginx:11.14.2
              ports:
                - containerPort: 80
                  name: http-web-svc
      - |
        apiVersion: v1
        kind: Service
        metadata:
          name: nginx-service
          labels:
            app.kubernetes.io/name: proxy
        spec:
          selector:
            app.kubernetes.io/name: proxy
          ports:
            - name: name-of-service-port
              protocol: TCP
              port: 80
              targetPort: http-web-svc
  owners:
    - name: alice
      kind: User

The difference with the proposed crd is, items are strings of k8s objects in yaml format. I ran through decoding issues when I tried to use Unstructured so I decided to settle on strings instead.

How it works ?

We search for namespaces specified by namespaceSelector that are owned by the tenant. For each matched namespace, apply all resources specified in additionalResources.items on any error, reschedule next reconciliation by resyncPeriod.

What is missing ?

  • [ ] Tests
  • [ ] What happens when a tenant is deleted ?
  • [ ] Does additionalRoleBindings cover for clusterRoles defined in approach 01?

I will wait for feedback/discussion on how to proceed from here.

gernest avatar Apr 18 '22 17:04 gernest

Deploy Preview for capsule-documentation canceled.

Name Link
Latest commit 142bc8ba9240d1da7972aec5a500c00a166094a5
Latest deploy log https://app.netlify.com/sites/capsule-documentation/deploys/625d9f0d1e1747000952a741

netlify[bot] avatar Apr 18 '22 17:04 netlify[bot]

@gernest @prometherion I'm proposing to close this since no progresses.

bsctl avatar Jul 13 '22 21:07 bsctl

@bsctl @gernest this feature is very useful. Why no progresses?

bobsongplus avatar Aug 19 '22 07:08 bobsongplus

My bad for not providing useful insights on this feature which is highly requested, and I'll try to elaborate properly.

Any new change in the API definition of a CRD, in our case the Tenant one, must follow a specific path for many reasons, although all it could be summed up by the fact that our preferred way to manage the lifecycle of Capsule is using Helm.

A new specification field in the Tenant definition would require a change in the charts/capsule/crds folder which is containing the Custom Resource manifests, and due to the opinionated way Helm is managing CRDs, these manifests are just installed when the release is missing in the cluster, requiring the Cluster Administrator to patch the CustomResourceDefinitions manually. No objection at all to this workflow, it's well known, and it has been deliberately put in place for many reasons I cannot share here since it would be off-topic.

Said so, we tend to implement features using the annotations of the custom resource, and it's really annoying due to the lack of type validation, issues with the max length of the key (RFC DNS-1123), and many other problems.

As maintainers, our idea is to propose this new feature for the upcoming milestone and we're still arguing on how to implement it properly, by asking for feedback from the community, and talking directly with the adopters, and as here on GitHub, as you could have seen.

I absolutely value the @gernest proactivity in trying to address this feature on his own, just a bit awkward fact that most of the hard work must be revisited, and since this is going to be a heavy feature, it should be managed by the maintainers itself to ensure a reliable implementation, and avoid eventually SPOF with the adopters that are contributing during their free time.

The main issue here has been my inactivity in pointing out all of these critic points before getting this PR opened, and a bit awkward that all the effort showed in this contribution will be lost, although we, as a community, are always welcoming contributions, and we're actively looking forward to it: syncing people, in a distributed and asynchronous world, is really challenging, I'll try to take note and behave more wisely in the future.

prometherion avatar Sep 12 '22 08:09 prometherion