Allow dynamic configuration of `kube-apiserver`

[anton-johansson]

This replaces enable_admission_plugins. I don't think this complicates things more than enable_admission_plugins used to do, and I can very easily add whatever options I want, like this:

[all:vars]
custom_options_apiserver=--enable-admission-plugins=PodSecurityPolicy --oidc-issuer-url=https://dex.svc.example.local --oidc-client-id=loginapp --oidc-ca-file=/etc/kubernetes/pki/custom-ca.crt --oidc-username-claim=email --oidc-groups-claim=groups

Closes #37

[anton-johansson]

Decided to fork the repository after all, and try to keep up with your changes in the future. :)

Closing this!

[amimof]

I actually think this is a good idea though. Just haven't had time to look it through. But what happens if the flags already exist? Anyway, feel free to fork :)

[anton-johansson]

Yeah, I came to the same conclusion. If the flags already exist and you want to override (for example, I wanted to override audit log paths to /var/log/k8s-audit/audit.log, to avoid collisions with "regular" audit logs).

I think the idea is good too, but we should support overriding too, if anything, IMO. Ideas? :)

Maybe we could have two lists of switches. One with default switches (hard-coded by KTRW) and one that can be set using Ansible variables. The lists could be merged, where the custom list is preferred. Then the items could be looped and added to the service file.

However, how do we handle switches with some kind of logic in them? For example --etcd-servers.

[amimof]

If two flags are set on the cmdline and the latter is used then we can simply put the custom ones in the beginning as you have done. I'll see if i can do some tests tomorrow.

[anton-johansson]

Allright, cool! It's probably easier to configure (and ends up better looking in the unit file) if we use an array instead of a space-separated list.

[amimof]

Just ran some tests and can confirm that the latter set flags are used even if there are duplicates. An array would be nice. How would that look like in the inventory?

Also, I'd like to rename the var to flags_apiserver just to keep it short and simple. Hope you don't mind.

[anton-johansson]

I don't mind at all, I agree with you. I've updated the branch, please check it out. My configuration looks like this:

flags_apiserver=['enable-admission-plugins=PodSecurityPolicy', 'oidc-issuer-url=https://dex.svc.example.local', 'oidc-client-id=loginapp', 'oidc-ca-file=/etc/kubernetes/pki/example-ca.pem', 'oidc-username-claim=email', 'oidc-groups-claim=groups', 'audit-log-path=/var/log/k8s-audit/audit.log', 'audit-policy-file=/etc/kubernetes/config/audit-policy.yaml']

Not the prettiest, as it has to be on one line, but it ends up perfect in the service unit file.

Some thoughts though:

• This way, I cannot override settings. So my audit log path isn't really overridden. Could we have these flags last instead? See below.
• I add -- in the template, so there is no way to get rid of these. I did this to save space and make it look better in the Ansible inventory file.
• Should we do this for the other components too, or wait until we find any requirements for it?

[anton-johansson]

Actually, I moved the additional flags to the end, to allow overrides. OK?

[amimof]

Looks good, the build failed though :( The resulting service file without the flags_apiserver set will result in:

...
 --tls-cert-file=/etc/kubernetes/pki/apiserver.pem \
  --tls-private-key-file=/etc/kubernetes/pki/apiserver-key.pemRestart=on-failure
RestartSec=5

Looks like the last line needs a line break. Also since the last flag should not have a \, a check should be in place. Something like this perhaps:?

...
  --tls-private-key-file=/etc/kubernetes/pki/apiserver-key.pem \
{% for flag in flags_apiserver %}
  --{{ flag }}{% if !loop.last %}\{% endif %}
{% endfor %}

Restart=on-failure
...
--

[anton-johansson]

Above suggestion would add a trailing slash to the last line of the command line, wouldn't it?

I added a new commit that should fix it, however, I can't really explain the Ansible-logic. :D

[amimof]

Looks good. I'll merge this.

To answer your question about the rest of the components and their flags. Yes i think its a good idea. If you like you can open a PR for them as well, or leave it as is until someone does :p

[amimof]

@anton-johansson I just opened pr #39 where i want to remove -- from the template for flags_apiserver. It makes more sense to me to let the user set whichever flagformat he/she wants. Hope you don't mind.

[anton-johansson]

Of course not! It makes sense. :)