User-facing migration to ClusterBundle#702
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ec6c107 to
551e847
Compare
551e847 to
d9dd750
Compare
0b8b584 to
9dac46d
Compare
2da597b to
f9e3118
Compare
2062dd2 to
f114776
Compare
f114776 to
b87a514
Compare
b87a514 to
688c870
Compare
9947942 to
b456e1d
Compare
b456e1d to
b87b744
Compare
There was a problem hiding this comment.
Pull request overview
This pull request implements a migration from the deprecated trust.cert-manager.io/v1alpha1 Bundle API to the new trust-manager.io/v1alpha2 ClusterBundle API. The PR includes automatic migration via a controller that converts existing Bundle resources to ClusterBundle resources, maintains backward compatibility for JKS format through annotations, and adds appropriate deprecation warnings.
Changes:
- Introduces the new ClusterBundle API (trust-manager.io/v1alpha2) with updated resource structure
- Adds migration controller to automatically convert Bundle -> ClusterBundle
- Updates all internal code to work with ClusterBundle API
- Adds deprecation warnings and markers to the old Bundle API
- Restructures bundle sources and targets (inLineCAs, includeDefaultCAs, KeyValueTarget)
- Removes JKS support in favor of PKCS12 (with backward compatibility via annotations)
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/apis/trust/v1alpha1/types_bundle.go | Added deprecation warnings to Bundle and BundleList types |
| pkg/bundle/controller/bundle_controller.go | Added migration controller to convert Bundle to ClusterBundle |
| pkg/webhook/cluster_bundle.go | New webhook validator for ClusterBundle resources |
| test/gen/bundle.go | Updated test generator to create ClusterBundle objects |
| pkg/bundle/internal/source/source.go | Refactored to accept BundleSpec instead of sources array |
| pkg/bundle/internal/target/target.go | Updated hash calculation and binary data generation for new API structure |
| deploy/crds/*.yaml | Added ClusterBundle CRD and deprecated Bundle CRD |
| make/00_mod.mk | Removed CRD exclusion to make ClusterBundle user-facing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
deploy/charts/trust-manager/templates/crd-trust.cert-manager.io_bundles.yaml
Outdated
Show resolved
Hide resolved
44b4cb1 to
61c357b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/cc @SgtCoDFish |
SgtCoDFish
left a comment
There was a problem hiding this comment.
Some fairly superficial comments, but I think worth raising. I want to try and sit down and test this properly but I haven't got bandwidth for that today I don't think!
Signed-off-by: Erik Godding Boye <egboye@gmail.com>
982c1a7 to
135facf
Compare
|
Thanks for the initial review, @SgtCoDFish. All your remarks should be addressed now. I probably want to cut a new release before this is merged. So... /hold |
This PR introduces the new ClusterBundle CRD and enables the migration controller according to the accepted design.
The main controller is changed to reconcile ClusterBundle instead of Bundle.
I've tried to keep this PR as small as possible to make it easier to review. We will likely need to add additional tests for the new features in the ClusterBundle API (e.g., multiple keys) and consolidate the existing tests due to the more generic API. However, I suggest handling this in follow-up PRs. Let me know what you think!