feat: add pregenerated certificates support with comprehensive testing#928
Draft
rossigee wants to merge 9 commits into
Draft
feat: add pregenerated certificates support with comprehensive testing#928rossigee wants to merge 9 commits into
rossigee wants to merge 9 commits into
Conversation
✅ Deploy Preview for kamaji-documentation ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Member
|
This is absolutely massive and awesome, thanks Ross! I'll need some time to review it property but this feature unlocks a stale feature request we had for so long time. |
prometherion
requested changes
Sep 1, 2025
prometherion
left a comment
prometherion
left a comment
Member
There was a problem hiding this comment.
It would be great to have the e2e being green before planning this to get merged: that would be helpful for avoiding introducing breaking changes.
Comment on lines
+163
to
+168
| // If bootstrap RBAC is configured, use that instead of default kubeadm behavior | ||
| if tcp.Spec.Bootstrap != nil && tcp.Spec.Bootstrap.RBAC != nil && tcp.Spec.Bootstrap.RBAC.Enabled { | ||
| return r.createBootstrapRBAC(ctx, c, tcp) | ||
| } | ||
|
|
||
| // Fallback to original kubeadm behavior for backward compatibility |
Member
There was a problem hiding this comment.
I like the idea of making toggleable the RBAC setup, but I would avoid having a separate function such as createBootstrapRBAC.
We should make Bootstrap and RBAC enabled by default, with the same defaults as Kubernetes, and try to resuse the underlying kubeadm library for DRY purposes/duplication of code.
Author
|
@prometherion - thanks for the feedback! I've also found some further issues since posting this, so it seems I was a bit premature. I'll put this in draft mode and deal with the remaining bugs, and your comments above first. |
Member
|
@rossigee this is great! thank you for contributing to Kamaji! |
rossigee
force-pushed
the
feature/pregenerated-certificates
branch
3 times, most recently
from
a277c8e to
5754162
Compare
Author
|
FTR, I'm trying to break it up into more manageable chunks. Here's a related PR... |
This commit adds support for pre-generated certificates and bootstrap configuration to TenantControlPlane resources. Key features: - PreGeneratedCertificates: Allow specifying existing certificates instead of generating new ones - Bootstrap: Configure initial RBAC setup and init manifests for clusters - CertificateReference and KeyReference types for certificate management - RBACBootstrapSpec for RBAC configuration during cluster creation - BootstrapSpec for initial cluster setup including CNI, GitOps operators, etc. Includes: - API types and validation - Certificate management logic - Webhook defaults and validation - Comprehensive test coverage - Updated CRDs and documentation
Add import for cmp package and fix cmpt.Or to cmp.Or in tenantcontrolplane_public_address.go to resolve compilation failure.
Update the test to expect 7 patches instead of 6 due to DataStoreUsername defaulting being added. Also set DataStoreUsername in the 'fields already set' test to prevent unwanted patches.
…AC creation - Change RBAC bootstrap to be enabled by default when bootstrap.RBAC is nil - Filter out 'kubernetes-admin' from additional admin users to avoid duplication - Ensure RBAC is created for all tenants unless explicitly disabled
- Add default value ['system:masters'] for AdminGroups field - Update createBootstrapRBAC to default both users and groups - Follows Kubernetes conventions for standard admin group - Ensures RBAC bootstrap provides complete default admin access
- Remove unused errors import from bootstraptoken.go - Fix k8serrors.Wrap to fmt.Errorf in kubeadm_utils.go - Remove unused endpoint variable in kubeadm_config.go - Update generated files after rebase
- Remove TenantControlPlaneServiceCIDR and TenantControlPlaneLoadBalancerSourceRanges handlers that were not defined after rebase
…ignature - Add nil parameter for DataStoreOverrides slice to match upstream function signature
rossigee
force-pushed
the
feature/pregenerated-certificates
branch
from
00a449f to
cdaca2b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This feature allows users to specify existing certificates and keys instead of generating new ones, addressing enterprise security requirements and certificate lifecycle management needs.
Key changes:
This implementation maintains backward compatibility while providing flexibility for certificate management in enterprise environments.