Environment Vars Schema

[Nalum]

I think this is looking good, I don't have experience with KeyCloak so can't speak to it's configuration but here are some things around the environment variables that came to mind. I'll add some more issues around other things as I get to look at it more tomorrow.

Is there a reason you've locked the envvars down to this?

https://github.com/yyvess/keycloak-timoni/blob/c8639ec87d0e3ec29b789823aa5e0b9429a62eef/templates/config.cue#L147-L185

I would look to have this implemented like #Config: envs?: [...corev1.#EnvVar]. This lets the user define how their envs are brought in and simplifies the cue schema, though does make the user do more.

The deployment envs would then look like this:

#Deployment: spec: template: spec: containers: [
  {
    env: [
      if #config.envs != _|_ {
        for e in #config.envs {e}
      },
    ]
  }
]

Instead of this:

https://github.com/yyvess/keycloak-timoni/blob/c8639ec87d0e3ec29b789823aa5e0b9429a62eef/templates/deployment.cue#L40-L50

Anything that's a static value like this, should just be put directly into the target resource:

https://github.com/yyvess/keycloak-timoni/blob/c8639ec87d0e3ec29b789823aa5e0b9429a62eef/templates/config.cue#L149

[Nalum]

If part of the idea behind the setup of the envs is hiding the setup then maybe you should break it down into structures e.g.:

#Config: {
  database: {
    enabled: *false | true
    type: *"dev-file" | "dev-mem" | "postgres" | "mariadb" | "mssql" | "mysql" | "oracle"
    url?: string | #secretReference
    username?: string | #secretReference
    password?: #secretReference
  }

  hostName: {
    enabled: *false | true
    name?: string
    port?: int & >0 <=65535
    admin?: string
    url?: string
    adminUrl?: string
    path?: string
    strict: {
      enabled: *false | true
      https: *true | false
      backchannel: *true | false
    }
  }

  cache: {
    enabled: *true | false
    type: *"local" | "ispn"
    configFile: string
    stack: *"kubernetes" | "tcp" | "udp" | "ec2" | "azure" | "google"
  }
}

I would find something like this much easier to parse through, even more so when comments are added to each field.

[yyvess]

@Nalum Thanks for your feedback. I have start to re-write config as you proposed.

[Nalum]

Had a look through again. There are some mixed styles (I guess) for the metadata field, e.g. here:

https://github.com/yyvess/keycloak-timoni/blob/a78e086e284d7822d37e49c69b7feaf5c9a14f12/templates/certificates.cue#L23-L28

and here:

https://github.com/yyvess/keycloak-timoni/blob/a78e086e284d7822d37e49c69b7feaf5c9a14f12/templates/certificates.cue#L41-L44

But easily sorted over time. The only thing that jumps out at me at the moment is where the logic lives, and that is probably just down to a personal preference or perhaps even just the size of the #Instance structure. I put as much logic as I could into the object definitions and only have the logic for creation in #Instance.

Anyway, I think this is looking good and will be great to have it out here! Looking forward to seeing more!

[yyvess]

@Nalum Thanks again for your input. I have try to use #MetaComponent here but seem not work as expected with timoniv1.#ImmutableConfig https://github.com/yyvess/keycloak-timoni/blob/a78e086e284d7822d37e49c69b7feaf5c9a14f12/templates/certificates.cue#L23-L28

I have move some of the envs logic from Instance to Deployment but for others I have an issue on the generation.