Skip to content

✨ (feat): When using Boxcutter feature-gate, use ClusterExtension ServiceAccount for revision operations #2429

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.

Sign up for GitHub

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

Open
camilamacedo86 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

✨ (feat): When using Boxcutter feature-gate, use ClusterExtension ServiceAccount for revision operations #2429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1b86c8d
(feat): [Boxcutter] Use ClusterExtension ServiceAccount for revision …
camilamacedo86 Jan 6, 2026
049afed
(fix) e2e test RBAC: add bind/escalate verbs
camilamacedo86 Jan 7, 2026
16c8c14
(doc) Add godoc comments to label constants
camilamacedo86 Jan 7, 2026
2fee08f
(fix) Add ClusterExtensionRevision permissions to upgrade test RBAC
camilamacedo86 Jan 7, 2026
55e51dc
review changes
camilamacedo86 Jan 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 22 additions & 17 deletions cmd/operator-controller/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@ import (
_ "k8s.io/client-go/plugin/pkg/client/auth"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"
"pkg.package-operator.run/boxcutter/machinery"
"pkg.package-operator.run/boxcutter/managedcache"
"pkg.package-operator.run/boxcutter/ownerhandling"
"pkg.package-operator.run/boxcutter/validation"
ctrl "sigs.k8s.io/controller-runtime"
crcache "sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/certwatcher"
Expand Down Expand Up @@ -653,21 +650,29 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl
return fmt.Errorf("unable to add tracking cache to manager: %v", err)
}

cerCoreClient, err := corev1client.NewForConfig(c.mgr.GetConfig())
if err != nil {
return fmt.Errorf("unable to create client for ClusterExtensionRevision controller: %w", err)
}
cerTokenGetter := authentication.NewTokenGetter(cerCoreClient, authentication.WithExpirationDuration(1*time.Hour))

revisionEngineFactory, err := controllers.NewDefaultRevisionEngineFactory(
c.mgr.GetScheme(),
trackingCache,
discoveryClient,
c.mgr.GetRESTMapper(),
fieldOwnerPrefix,
c.mgr.GetConfig(),
cerTokenGetter,
)
if err != nil {
return fmt.Errorf("unable to create revision engine factory: %w", err)
}

if err = (&controllers.ClusterExtensionRevisionReconciler{
Client: c.mgr.GetClient(),
RevisionEngine: machinery.NewRevisionEngine(
machinery.NewPhaseEngine(
machinery.NewObjectEngine(
c.mgr.GetScheme(), trackingCache, c.mgr.GetClient(),
ownerhandling.NewNative(c.mgr.GetScheme()),
machinery.NewComparator(ownerhandling.NewNative(c.mgr.GetScheme()), discoveryClient, c.mgr.GetScheme(), fieldOwnerPrefix),
fieldOwnerPrefix, fieldOwnerPrefix,
),
validation.NewClusterPhaseValidator(c.mgr.GetRESTMapper(), c.mgr.GetClient()),
),
validation.NewRevisionValidator(), c.mgr.GetClient(),
),
TrackingCache: trackingCache,
Client: c.mgr.GetClient(),
RevisionEngineFactory: revisionEngineFactory,
TrackingCache: trackingCache,
}).SetupWithManager(c.mgr); err != nil {
return fmt.Errorf("unable to setup ClusterExtensionRevision controller: %w", err)
}
Expand Down
13 changes: 13 additions & 0 deletions hack/test/pre-upgrade-setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,19 @@ rules:
- "update"
resourceNames:
- "${TEST_CLUSTER_EXTENSION_NAME}"
- apiGroups:
- "olm.operatorframework.io"
resources:
- "clusterextensionrevisions"
- "clusterextensionrevisions/finalizers"
verbs:
- "create"
- "update"
- "patch"
- "delete"
- "get"
- "list"
- "watch"
EOF
Copy link
Contributor Author

@camilamacedo86 camilamacedo86 Jan 8, 2026

Choose a reason for hiding this comment

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

To fix: https://github.com/operator-framework/operator-controller/actions/runs/20792120869/job/59718591823


kubectl apply -f - <<EOF
Expand Down
6 changes: 6 additions & 0 deletions internal/operator-controller/applier/boxcutter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,12 @@ func (r *SimpleRevisionGenerator) buildClusterExtensionRevision(
ext *ocv1.ClusterExtension,
annotations map[string]string,
) *ocv1.ClusterExtensionRevision {
if annotations == nil {
annotations = make(map[string]string)
}
annotations[labels.ServiceAccountNameKey] = ext.Spec.ServiceAccount.Name
annotations[labels.ServiceAccountNamespaceKey] = ext.Spec.Namespace

return &ocv1.ClusterExtensionRevision{
ObjectMeta: metav1.ObjectMeta{
Annotations: annotations,
Expand Down
36 changes: 30 additions & 6 deletions internal/operator-controller/applier/boxcutter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
ObjectMeta: metav1.ObjectMeta{
Name: "test-123",
},
Spec: ocv1.ClusterExtensionSpec{
Namespace: "test-namespace",
ServiceAccount: ocv1.ServiceAccountReference{
Name: "test-sa",
},
},
}

objectLabels := map[string]string{
Expand All @@ -79,10 +85,12 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
ObjectMeta: metav1.ObjectMeta{
Name: "test-123-1",
Annotations: map[string]string{
"olm.operatorframework.io/bundle-name": "my-bundle",
"olm.operatorframework.io/bundle-reference": "bundle-ref",
"olm.operatorframework.io/bundle-version": "1.2.0",
"olm.operatorframework.io/package-name": "my-package",
"olm.operatorframework.io/bundle-name": "my-bundle",
"olm.operatorframework.io/bundle-reference": "bundle-ref",
"olm.operatorframework.io/bundle-version": "1.2.0",
"olm.operatorframework.io/package-name": "my-package",
"olm.operatorframework.io/service-account-name": "test-sa",
"olm.operatorframework.io/service-account-namespace": "test-namespace",
},
Labels: map[string]string{
labels.OwnerNameKey: "test-123",
Expand Down Expand Up @@ -172,6 +180,12 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "test-extension",
},
Spec: ocv1.ClusterExtensionSpec{
Namespace: "test-namespace",
ServiceAccount: ocv1.ServiceAccountReference{
Name: "test-sa",
},
},
}

rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, ext, map[string]string{}, map[string]string{})
Expand Down Expand Up @@ -291,7 +305,12 @@ func Test_SimpleRevisionGenerator_AppliesObjectLabelsAndRevisionAnnotations(t *t
"other": "value",
}

rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, &ocv1.ClusterExtension{}, map[string]string{
rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, &ocv1.ClusterExtension{
Spec: ocv1.ClusterExtensionSpec{
Namespace: "test-namespace",
ServiceAccount: ocv1.ServiceAccountReference{Name: "test-sa"},
},
}, map[string]string{
"some": "value",
}, revAnnotations)
require.NoError(t, err)
Expand Down Expand Up @@ -319,7 +338,12 @@ func Test_SimpleRevisionGenerator_Failure(t *testing.T) {
ManifestProvider: r,
}

rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, &ocv1.ClusterExtension{}, map[string]string{}, map[string]string{})
rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, &ocv1.ClusterExtension{
Spec: ocv1.ClusterExtensionSpec{
Namespace: "test-namespace",
ServiceAccount: ocv1.ServiceAccountReference{Name: "test-sa"},
},
}, map[string]string{}, map[string]string{})
require.Nil(t, rev)
t.Log("by checking rendering errors are propagated")
require.Error(t, err)
Expand Down
27 changes: 17 additions & 10 deletions internal/operator-controller/controllers/clusterextensionrevision_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ const (
// ClusterExtensionRevisionReconciler actions individual snapshots of ClusterExtensions,
// as part of the boxcutter integration.
type ClusterExtensionRevisionReconciler struct {
Client client.Client
RevisionEngine RevisionEngine
TrackingCache trackingCache
Client client.Client
RevisionEngineFactory RevisionEngineFactory
TrackingCache trackingCache
}

type trackingCache interface {
Expand All @@ -55,11 +55,6 @@ type trackingCache interface {
Free(ctx context.Context, user client.Object) error
}

type RevisionEngine interface {
Teardown(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionTeardownOption) (machinery.RevisionTeardownResult, error)
Reconcile(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error)
}

//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensionrevisions,verbs=get;list;watch;update;patch;create;delete
//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensionrevisions/status,verbs=update;patch
//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensionrevisions/finalizers,verbs=update
Expand Down Expand Up @@ -139,7 +134,13 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
return ctrl.Result{}, werr
}

rres, err := c.RevisionEngine.Reconcile(ctx, *revision, opts...)
revisionEngine, err := c.RevisionEngineFactory.CreateRevisionEngine(ctx, rev)
if err != nil {
setRetryingConditions(rev, err.Error())
return ctrl.Result{}, fmt.Errorf("failed to create revision engine: %v", err)
}

rres, err := revisionEngine.Reconcile(ctx, *revision, opts...)
if err != nil {
if rres != nil {
// Log detailed reconcile reports only in debug mode (V(1)) to reduce verbosity.
Expand Down Expand Up @@ -253,7 +254,13 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
func (c *ClusterExtensionRevisionReconciler) teardown(ctx context.Context, rev *ocv1.ClusterExtensionRevision, revision *boxcutter.Revision) (ctrl.Result, error) {
l := log.FromContext(ctx)

tres, err := c.RevisionEngine.Teardown(ctx, *revision)
revisionEngine, err := c.RevisionEngineFactory.CreateRevisionEngine(ctx, rev)
if err != nil {
markAsAvailableUnknown(rev, ocv1.ClusterExtensionRevisionReasonReconciling, err.Error())
return ctrl.Result{}, fmt.Errorf("failed to create revision engine for teardown: %v", err)
}

tres, err := revisionEngine.Teardown(ctx, *revision)
if err != nil {
if tres != nil {
l.V(1).Info("teardown failure report", "report", tres.String())
Expand Down
Loading
Loading