Skip to content

Commit 88562dd

Browse files
committed
OCPBUGS-123707: prevent ClusterCatalog catalog rollbacks
Protect OCI image-sourced catalogs against a different digest whose usable OCI image config creation timestamp is not newer than the accepted catalog for the same source reference. Persist creation timestamps and accepted publication state alongside cached catalog content. Existing cache entries without timestamp metadata are repulled once. Images without a usable creation timestamp remain compatible and log that rollback protection is not enforced. Configure rollback protection on catalogd's containers-image puller, preserving the existing generic Puller interface and avoiding a public API policy before its broader scope is designed. When a date regression is rejected, retain the served catalog and status, record a CatalogRollbackPrevented Event, and resume polling on the normal interval. Add cache, puller, and controller tests. Validation: make test-unit Signed-off-by: Todd Short <tshort@redhat.com>
1 parent b980fea commit 88562dd

8 files changed

Lines changed: 441 additions & 24 deletions

File tree

‎cmd/catalogd/main.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@ func run(ctx context.Context) error {
332332

333333
imageCache := imageutil.CatalogCache(unpackCacheBasePath)
334334
imagePuller := &imageutil.ContainersImagePuller{
335+
CatalogRollbackProtectionEnabled: true,
335336
SourceCtxFunc: func(ctx context.Context) (*types.SystemContext, error) {
336337
logger := log.FromContext(ctx)
337338
srcContext := &types.SystemContext{
@@ -403,10 +404,11 @@ func run(ctx context.Context) error {
403404
}
404405

405406
if err = (&corecontrollers.ClusterCatalogReconciler{
406-
Client: mgr.GetClient(),
407-
ImageCache: imageCache,
408-
ImagePuller: imagePuller,
409-
Storage: localStorage,
407+
Client: mgr.GetClient(),
408+
ImageCache: imageCache,
409+
ImagePuller: imagePuller,
410+
EventRecorder: mgr.GetEventRecorder("clustercatalog-controller"),
411+
Storage: localStorage,
410412
}).SetupWithManager(mgr); err != nil {
411413
setupLog.Error(err, "unable to create controller", "controller", "ClusterCatalog")
412414
return err

‎internal/catalogd/controllers/core/clustercatalog_controller.go‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,13 @@ import (
2525
"time"
2626

2727
"go.podman.io/image/v5/docker/reference"
28+
corev1 "k8s.io/api/core/v1"
2829
"k8s.io/apimachinery/pkg/api/equality"
2930
"k8s.io/apimachinery/pkg/api/meta"
3031
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3132
"k8s.io/apimachinery/pkg/util/sets"
3233
"k8s.io/apimachinery/pkg/util/wait"
34+
"k8s.io/client-go/tools/events"
3335
"k8s.io/utils/ptr"
3436
ctrl "sigs.k8s.io/controller-runtime"
3537
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -56,8 +58,9 @@ const (
5658
type ClusterCatalogReconciler struct {
5759
client.Client
5860

59-
ImageCache imageutil.Cache
60-
ImagePuller imageutil.Puller
61+
ImageCache imageutil.Cache
62+
ImagePuller imageutil.Puller
63+
EventRecorder events.EventRecorder
6164

6265
Storage storage.Instance
6366

@@ -255,6 +258,18 @@ func (r *ClusterCatalogReconciler) reconcile(ctx context.Context, catalog *ocv1.
255258

256259
fsys, canonicalRef, unpackTime, err := r.ImagePuller.Pull(ctx, catalog.Name, catalog.Spec.Source.Image.Ref, r.ImageCache)
257260
if err != nil {
261+
var rollbackErr *imageutil.CatalogRollbackError
262+
if errors.As(err, &rollbackErr) {
263+
if r.EventRecorder != nil {
264+
r.EventRecorder.Eventf(catalog, nil, corev1.EventTypeWarning, "CatalogRollbackPrevented", "CatalogRollbackPrevented", "%s", rollbackErr)
265+
}
266+
lastSuccessfulPoll := time.Now()
267+
r.storedCatalogsMu.Lock()
268+
storedCatalog.lastSuccessfulPoll = lastSuccessfulPoll
269+
r.storedCatalogs[catalog.Name] = storedCatalog
270+
r.storedCatalogsMu.Unlock()
271+
return nextPollResult(lastSuccessfulPoll, catalog), nil
272+
}
258273
unpackErr := fmt.Errorf("source catalog content: %w", err)
259274
updateStatusProgressing(&catalog.Status, catalog.GetGeneration(), unpackErr)
260275
return ctrl.Result{}, unpackErr
@@ -350,10 +365,8 @@ func updateStatusProgressing(status *ocv1.ClusterCatalogStatus, generation int64
350365

351366
func updateStatusServing(status *ocv1.ClusterCatalogStatus, ref reference.Canonical, modTime time.Time, baseURL string, generation int64) {
352367
status.ResolvedSource = &ocv1.ResolvedCatalogSource{
353-
Type: ocv1.SourceTypeImage,
354-
Image: &ocv1.ResolvedImageSource{
355-
Ref: ref.String(),
356-
},
368+
Type: ocv1.SourceTypeImage,
369+
Image: &ocv1.ResolvedImageSource{Ref: ref.String()},
357370
}
358371
status.URLs = &ocv1.ClusterCatalogURLs{
359372
Base: baseURL,
@@ -398,6 +411,13 @@ func updateStatusProgressingUserSpecifiedUnavailable(status *ocv1.ClusterCatalog
398411

399412
func updateStatusNotServing(status *ocv1.ClusterCatalogStatus, generation int64) {
400413
status.ResolvedSource = nil
414+
updateStatusNotServingWithResolvedSource(status, generation)
415+
}
416+
417+
// updateStatusNotServingWithResolvedSource marks content unavailable while retaining the last accepted
418+
// source. In particular, this preserves a catalog publication version so a rejected rollback cannot
419+
// become acceptable on the next reconcile merely because served content is missing.
420+
func updateStatusNotServingWithResolvedSource(status *ocv1.ClusterCatalogStatus, generation int64) {
401421
status.URLs = nil
402422
status.LastUnpacked = nil
403423
meta.SetStatusCondition(&status.Conditions, metav1.Condition{

‎internal/catalogd/controllers/core/clustercatalog_controller_test.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"go.uber.org/mock/gomock"
1717
"k8s.io/apimachinery/pkg/api/meta"
1818
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
19+
"k8s.io/apimachinery/pkg/runtime"
1920
"k8s.io/utils/ptr"
2021
ctrl "sigs.k8s.io/controller-runtime"
2122
"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -40,6 +41,14 @@ func newMockStore(ctrl *gomock.Controller, shouldError bool) *mockstorage.MockIn
4041
return m
4142
}
4243

44+
type fakeEventRecorder struct {
45+
events chan string
46+
}
47+
48+
func (f *fakeEventRecorder) Eventf(_ runtime.Object, _ runtime.Object, _ string, reason, action, note string, args ...interface{}) {
49+
f.events <- fmt.Sprintf("%s:%s:%s", reason, action, fmt.Sprintf(note, args...))
50+
}
51+
4352
func TestCatalogdControllerReconcile(t *testing.T) {
4453
mockCtrl := gomock.NewController(t)
4554
for _, tt := range []struct {
@@ -1140,3 +1149,42 @@ func mustRef(t *testing.T, ref string) reference.Canonical {
11401149
}
11411150
return p.(reference.Canonical)
11421151
}
1152+
1153+
func TestReconcileCatalogRollbackKeepsServingCatalog(t *testing.T) {
1154+
mockCtrl := gomock.NewController(t)
1155+
acceptedRef := mustRef(t, "my.org/catalog@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855")
1156+
catalog := &ocv1.ClusterCatalog{
1157+
ObjectMeta: metav1.ObjectMeta{Name: "catalog", Finalizers: []string{fbcDeletionFinalizer}},
1158+
Spec: ocv1.ClusterCatalogSpec{
1159+
Source: ocv1.CatalogSource{Type: ocv1.SourceTypeImage, Image: &ocv1.ImageSource{Ref: "my.org/catalog:latest", PollIntervalMinutes: ptr.To(5)}},
1160+
AvailabilityMode: ocv1.AvailabilityModeAvailable,
1161+
},
1162+
Status: ocv1.ClusterCatalogStatus{
1163+
Conditions: []metav1.Condition{
1164+
{Type: ocv1.TypeServing, Status: metav1.ConditionTrue, Reason: ocv1.ReasonAvailable},
1165+
{Type: ocv1.TypeProgressing, Status: metav1.ConditionTrue, Reason: ocv1.ReasonSucceeded},
1166+
},
1167+
ResolvedSource: &ocv1.ResolvedCatalogSource{Type: ocv1.SourceTypeImage, Image: &ocv1.ResolvedImageSource{Ref: acceptedRef.String()}},
1168+
URLs: &ocv1.ClusterCatalogURLs{Base: "URL"},
1169+
LastUnpacked: ptr.To(metav1.Now()),
1170+
},
1171+
}
1172+
originalStatus := catalog.Status.DeepCopy()
1173+
events := &fakeEventRecorder{events: make(chan string, 1)}
1174+
reconciler := &ClusterCatalogReconciler{
1175+
ImagePuller: &imageutil.FakePuller{Error: &imageutil.CatalogRollbackError{
1176+
CandidateDigest: "sha256:candidate", CandidateCreated: time.Date(2026, time.January, 1, 0, 0, 0, 0, time.UTC),
1177+
AcceptedDigest: "sha256:accepted", AcceptedCreated: time.Date(2026, time.January, 2, 0, 0, 0, 0, time.UTC),
1178+
}},
1179+
ImageCache: &imageutil.FakeCache{},
1180+
Storage: newMockStore(mockCtrl, false),
1181+
EventRecorder: events,
1182+
storedCatalogs: map[string]storedCatalogData{catalog.Name: {ref: acceptedRef, lastUnpack: catalog.Status.LastUnpacked.Time, lastSuccessfulPoll: time.Now().Add(-6 * time.Minute)}},
1183+
}
1184+
require.NoError(t, reconciler.setupFinalizers())
1185+
result, err := reconciler.reconcile(context.Background(), catalog)
1186+
require.NoError(t, err)
1187+
assert.Positive(t, result.RequeueAfter)
1188+
assert.Equal(t, *originalStatus, catalog.Status)
1189+
assert.Contains(t, <-events.events, "CatalogRollbackPrevented")
1190+
}

0 commit comments

Comments
 (0)