feat: add ImagePullSecrets config private registries

[rafalgalaw]

One can now supply a list of ImagePullSecrets for runner pods in the provider-config.yaml which are attached to a runner pod if configured, so images can be pulled from private registries as well:

kubeConfigPath: "/path/to/kubeconfig"
containerRegistry: "sample.registry.com"
runnerNamespace: "test-namespace"
imagePullSecrets:
  - my-imagepullsecret1
  - my-imagepullsecret2

[bavarianbidi]

Shouldn't we document (with an example) the usage of the pod.template feature where it's also possible to inject the image-pull secret instead of adding a new field and appending to the spec?

Appending the pullSecret via a new configuration-value only make sense to me, if we also take care of the lifecycle of the secret itself (IMHO)

WDYT?

[bavarianbidi]

Shouldn't we document (with an example) the usage of the pod.template feature where it's also possible to inject the image-pull secret instead of adding a new field and appending to the spec?

Appending the pullSecret via a new configuration-value only make sense to me, if we also take care of the lifecycle of the secret itself (IMHO)

WDYT?

@gabriel-samfira i'm also interested in your opinion about that 🙏

In general the support for adding an imagePullSecret is already there - by using the pod.template.spec via they podTemplate configuration.
But if the desired feature is also about garm-provider-k8s should take care about the lifecycle of the imagepullsecret, then we have to implement it in here.

Both seems valid to me ATM

[rafalgalaw]

@bavarianbidi yes you are right, with the podTemplate we have this defacto already implemented, and i like the config via podTemplateSpec better, tbh didn´t give much thought about this. IMH we should rather provide some better documentation on how podTemplateSpec behaves, and how flexible this solution is for configuring your runner pod. So i can close this draft, and update the docs

[gabriel-samfira]

TL;DR;

Documenting that the pull secrets for the supplied registry can be set as pod spec values is okay. The key thing is to make it easy for users to understand how to deal with private registries. Even a quick example in the README is ok.

TS;NM;

In general, my personal preference is to keep all relevant dependencies grouped together. If an option is created to define a resource, from a user experience perspective, it's a good idea to have all relevant dependencies for that new option, clumped together in a single place.

The registry and the credentials can be specified at the pod spec level, but we also have a config value at the provider level via the containerRegistry config key. IMO, the desirable thing to do is to either define the registry and the credentials for it at the provider level, or define all the needed bits at the pod spec level (which can already be done if I am not mistaken), and simply document it. Both options are okay.

I would be okay with having the creds supplied as part of extraSpecs as well. Something like:

garm-cli pool add \
  --image registry.example.com:5000/runner-image:latest \
  --extra-specs='{"registry-secrets":{ "username": "admin", "password": "admin"}}'

So there are 3 options here, either one being okay as far as I am concerned.

[gabriel-samfira]

I would be okay with having the creds supplied as part of extraSpecs as well

I just realized, extra specs are not encrypted at rest, so this one is a bad idea (for now), and someting that I should fix in garm first.