Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow overriding secretKey for kubeconfig #78

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

kvaps
Copy link
Contributor

@kvaps kvaps commented Feb 4, 2024

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 clastix/kamaji#403).

And also copy this annotation for TenantControlPlane object (see: clastix/kamaji#408)

requires #93 to get merged first

Copy link
Member

@prometherion prometherion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we could take advantage of constants, it would be much better, wondering why I didn't do from.the beginning.

controllers/kamajicontrolplane_controller_resources.go Outdated Show resolved Hide resolved
@kvaps kvaps force-pushed the override-secret-key branch 3 times, most recently from a1c7eac to d57dc5f Compare February 5, 2024 08:45
@kvaps kvaps changed the title allow overriding secretKey for kubeconfig Allow overriding secretKey for kubeconfig Feb 5, 2024
@kvaps kvaps marked this pull request as ready for review February 5, 2024 08:46
@kvaps
Copy link
Contributor Author

kvaps commented Feb 5, 2024

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

@prometherion
Copy link
Member

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
Copy link
Contributor Author

kvaps commented Feb 10, 2024

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
Copy link

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
Copy link
Member

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

Overall, LGTM.

@kvaps kvaps mentioned this pull request Apr 22, 2024
@kvaps
Copy link
Contributor Author

kvaps commented Apr 22, 2024

@prometherion PR rebased. But now it depends on imported lib, so please review and merge #93 first

@prometherion
Copy link
Member

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

@kvaps kvaps force-pushed the override-secret-key branch 2 times, most recently from 86ab061 to 19b83d6 Compare April 22, 2024 17:17
During reconciliation, the control plane provider copies the content from the secret provided by
Kamaji, named <cluster>-admin-kubeconfig, into a generic Cluster API secret, <cluster>-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: admin.svc
```

This will instruct the system to use `admin.svc` a kubeconfig with a local service FQDN
(introduced by clastix/kamaji#403).

Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
@kvaps
Copy link
Contributor Author

kvaps commented Apr 22, 2024

job is done

@prometherion prometherion added this to the v0.8.0 milestone Apr 23, 2024
@prometherion prometherion merged commit 9ff3a33 into clastix:master Apr 23, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants