clastix / cluster-api-control-plane-provider-kamaji

The Kamaji Control Plane provider implementation of the Cluster Management API
Apache License 2.0
74 stars 25 forks source link

Allow overriding secretKey for kubeconfig #78

Closed kvaps closed 4 months ago

kvaps commented 7 months ago

During reconciliation, the control plane provider copies the content from the secret provided by Kamaji, named -admin-kubeconfig, into a generic Cluster API secret, -kubeconfig, which can then be used by the bootstrap provider and other cluster components.

This change introduces a new annotation, kamaji.clastix.io/kubeconfig-secret-key, for the KamajiControlPlane resource. This annotation instructs the control plane provider to read the kubeconfig from a specific key (the default one is admin.conf).

Example:

kamaji.clastix.io/kubeconfig-secret-key: super-admin.svc

This will instruct the system to use super-admin.svc a kubeconfig with a local service FQDN (introduced by https://github.com/clastix/kamaji/pull/403).

And also copy this annotation for TenantControlPlane object (see: https://github.com/clastix/kamaji/pull/408)

requires https://github.com/clastix/cluster-api-control-plane-provider-kamaji/pull/93 to get merged first

kvaps commented 7 months ago

Ah that was just a draft, thanks for your corrections. PR updated and ready for review.

prometherion commented 7 months ago

I'm thinking of the use case you're trying to solve, here, also considering the discussion we had on Slack.

Don't you think it would be better to have the TenantControlPlane exposed using the ClusterIP and provisioning manually (or via a third-party integration) the kubeconfig as well as the Ingress object to access it externally?

I'm a bit worried about the too many knobs we could introduce: although you know what you're doing other users could be able to shoot themselves in the foot, and this is something that must be carefully prevented to ensure the onboarding process works as expected.

Happy to chat.

kvaps commented 7 months ago

Sure, it's open for discussion.

I suggested using an annotation because this setup might be uncommon.

Alternatively, I think we can introduce an option that will be completely understandable to everyone.

For example, spec.network.preferLocalEndpoints: <true|false>.

What do you think?

andreykont commented 6 months ago

Hello. I like this PR. Because I would prefer use super-admin.conf as generic kubeconfig (based on 'system:master' Group to prevent losing access capi to managed kubernetes clusters)

prometherion commented 6 months ago

@kvaps if we get clastix/kamaji#408 merged first, we could reuse the constant defined in the root repository.

Overall, LGTM.

kvaps commented 4 months ago

@prometherion PR rebased. But now it depends on imported lib, so please review and merge https://github.com/clastix/cluster-api-control-plane-provider-kamaji/pull/93 first

prometherion commented 4 months ago

@kvaps merged, please, rebase so we can use the last code base from Kamaji, thanks!

kvaps commented 4 months ago

job is done