projectsyn / lieutenant-api

The Project Syn Kubernetes Cluster and Tenants Inventory API
https://docs.syn.tools/lieutenant-api/
BSD 3-Clause "New" or "Revised" License
9 stars 1 forks source link

Set valid clusterTemplate when creating Tenant Objects #89

Closed tobru closed 2 years ago

tobru commented 3 years ago

Context

https://github.com/projectsyn/lieutenant-operator/pull/110 added a cluster template to the Tenant object which is not accessible throught the API. This is an issue as `spec.clusterTemplate.gitRepoTemplate.apiSecretRef.name needs to be set. The name of that secret differs between instances of Lieutenant.

When creating a tenant the following structure must be set:

{
  "spec": {
    "clusterTemplate": {
      "gitRepoTemplate": {
        "apiSecretRef": {
          "name": "<name of secret>"
        },
        "path": "",
        "repoName": "{{ .Name }}"
      },
      "tenantRef": {}
    }
  }
}

This could be done by exposing the field clusterTemplate as a whole or the variable field of the secret name. It also would be acceptable to have the secret name configured as an environment variable and have that data structure hard coded.

tobru commented 3 years ago

As a workaround we might hard code the cluster template creation using the env var DEFAULT_API_SECRET_REF_NAME:

{
  "spec": {
    "clusterTemplate": {
      "gitRepoTemplate": {
        "apiSecretRef": {
          "name": "$DEFAULT_API_SECRET_REF_NAME"
        },
        "path": "",
        "repoName": "{{ .Name }}"
      },
      "tenantRef": {}
    }
  }
}
corvus-ch commented 3 years ago

The field clusterTemplate holds quite a complex data structure. Adding this to the API will be a lot of simple but time consuming work. As the API is eventually going away, this is not worth the effort.

Just exposing the secret name does not feel right. It would be an unprecedented exception.

So going with the hard coded option an the environment varialbe is what is left.

When implementing https://github.com/projectsyn/lieutenant-operator/pull/110, we talked about having also a TenantTemplate that will act similar to the clusterTemplate within the Tenant CR. This would proably be the most robust solution. I suggest to have this issue closed. Instead we will create one at the Operator to have that TenanTemplate implemented.

tobru commented 3 years ago

I agree that exposing the whole clusterTemplate field via the API doesn't make any sense (anymore). I also agree that we need a Tenant template. Would you mind creating an issue for that? After this follow-up issue is created I'd like to reevaluate if we still need to have this hard coded environment variable DEFAULT_API_SECRET_REF_NAME as a quick fix until the Tenant template is in place.

simu commented 2 years ago

Do we still need this, since we have support for configuring TenantTemplate objects with https://github.com/projectsyn/lieutenant-operator/issues/136?

corvus-ch commented 2 years ago

At least it not a blocker anymore. If one wants to have different cluster templates per tenant, changes to the tenant must be made through the Kubernetes API instead of the Lieutenant API. I consider this to be an unlikely use case.

Let us see, if this comes up again. It might even become obsolete should we manage to drop the Lieutenant API in favor of Kubernetes.