diff --git a/docs/component.md b/docs/component.md index b23ac37b..3ed1741c 100644 --- a/docs/component.md +++ b/docs/component.md @@ -65,14 +65,15 @@ if err != nil { Each resource is registered with a `ResourceOptions` struct that controls how the component interacts with it: -| Option | Behavior | -| ---------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `ResourceOptions{}` (default) | **Managed**: created or updated; health contributes to condition | -| `ResourceOptions{ReadOnly: true}` | **Read-only**: fetched but never modified; health still contributes | -| `ResourceOptions{Delete: true}` | **Delete-only**: removed from the cluster if present; does not contribute to health | -| `ResourceOptions{ParticipationMode: ParticipationModeAuxiliary}` | The resource's health does not contribute to the component condition. The component can become Ready regardless of this resource's state. **Exception:** a blocked [guard](#guards) always contributes to the condition regardless of participation mode, because it halts the entire reconciliation pipeline | -| `ResourceOptions{SuppressGraceInconsistencyWarning: true}` | Suppresses the warning log emitted when the resource's grace handler returns Healthy while its convergence handler returns non-healthy. Use this when the inconsistency is intentional (e.g., a custom grace handler that deliberately reports Healthy for a resource that has not fully converged) | -| `ResourceOptions{ReadOnly: true, BlockOnAbsence: true}` | **Read-only with watch-driven retry**: a NotFound from the cluster is recorded as a blocked status (`waiting for `) and short-circuits the remaining resources, instead of erroring back through controller-runtime's exponential backoff. Use only when the consumer has a watch on the resource's type so the reconcile is re-enqueued when it appears | +| Option | Behavior | +| ---------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `ResourceOptions{}` (default) | **Managed**: created or updated; health contributes to condition | +| `ResourceOptions{ReadOnly: true}` | **Read-only**: fetched but never modified; health still contributes | +| `ResourceOptions{Delete: true}` | **Delete-only**: removed from the cluster if present; does not contribute to health | +| `ResourceOptions{ParticipationMode: ParticipationModeAuxiliary}` | The resource's health does not contribute to the component condition. The component can become Ready regardless of this resource's state. **Exception:** a blocked [guard](#guards) always contributes to the condition regardless of participation mode, because it halts the entire reconciliation pipeline | +| `ResourceOptions{SuppressGraceInconsistencyWarning: true}` | Suppresses the warning log emitted when the resource's grace handler returns Healthy while its convergence handler returns non-healthy. Use this when the inconsistency is intentional (e.g., a custom grace handler that deliberately reports Healthy for a resource that has not fully converged) | +| `ResourceOptions{ReadOnly: true, BlockOnAbsence: true}` | **Read-only with watch-driven retry**: a NotFound from the cluster is recorded as a blocked status (`waiting for `) and short-circuits the remaining resources, instead of erroring back through controller-runtime's exponential backoff. Use only when the consumer has a watch on the resource's type so the reconcile is re-enqueued when it appears | +| `ResourceOptions{ReadOnly: true, IgnoreIfAbsent: true}` | **Optional read-only**: a NotFound from the cluster is silently ignored. The entry contributes nothing to the component's conditions, no observation is recorded, and the data extractor is not invoked. Subsequent resources reconcile unchanged. State recorded from earlier reconciles (last observation, extracted data) is preserved across an absence rather than reset. Use for resources that may legitimately be absent (e.g. a referenced Secret owned by another operator) | ### Building Resource Options with Feature Gating @@ -97,13 +98,14 @@ signature is unchanged. **Methods:** -| Method | Effect | -| --------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `WithFeatureGate(f feature.Gate)` | Gates the resource on a feature. When disabled, the resource is deleted. | -| `When(truth bool)` | Adds a boolean condition (AND logic). If any condition is false, the resource is deleted. Calls are additive. | -| `Auxiliary()` | Sets participation mode to `Auxiliary` (resource does not affect component health). | -| `ReadOnly()` | Marks the resource as read-only. If the resource is also gated by a disabled feature, deletion takes precedence over read-only. | -| `BlockOnAbsence()` | Opts a read-only resource into guard-blocked semantics on NotFound. Only meaningful alongside `ReadOnly()`; requires a watch on the resource's type to avoid stalling until the periodic resync. | +| Method | Effect | +| --------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `WithFeatureGate(f feature.Gate)` | Gates the resource on a feature. When disabled, the resource is deleted. | +| `When(truth bool)` | Adds a boolean condition (AND logic). If any condition is false, the resource is deleted. Calls are additive. | +| `Auxiliary()` | Sets participation mode to `Auxiliary` (resource does not affect component health). | +| `ReadOnly()` | Marks the resource as read-only. If the resource is also gated by a disabled feature, deletion takes precedence over read-only. | +| `BlockOnAbsence()` | Opts a read-only resource into guard-blocked semantics on NotFound. Requires `ReadOnly()` and is mutually exclusive with `IgnoreIfAbsent()`; `Build()` errors otherwise. Requires a watch on the resource's type to avoid stalling until the periodic resync. | +| `IgnoreIfAbsent()` | Opts a read-only resource into "optional" semantics: a NotFound is silently ignored, the entry is skipped, no condition or observation is reported, and the data extractor is not invoked. State recorded from earlier reconciles is preserved across an absence. Requires `ReadOnly()` and is mutually exclusive with `BlockOnAbsence()`; `Build()` errors otherwise. | For the common case of gating a resource on a single feature, use the convenience function: diff --git a/pkg/component/builder.go b/pkg/component/builder.go index 43d5b90f..5af1362b 100644 --- a/pkg/component/builder.go +++ b/pkg/component/builder.go @@ -33,11 +33,32 @@ type ResourceOptions struct { // resource that has not fully converged). SuppressGraceInconsistencyWarning bool // BlockOnAbsence applies to read-only resources. When true, a NotFound response - // from the cluster is treated as a guard-blocked condition rather than an error, - // preventing controller-runtime's exponential backoff and producing a meaningful - // status reason. Only use this when the consumer has a watch on the resource's - // type so that the reconcile is re-enqueued when the resource appears. + // from the cluster is treated as a guard-blocked condition rather than an + // error, preventing controller-runtime's exponential backoff and producing a + // meaningful status reason. Only use this when the consumer has a watch on + // the resource's type so that the reconcile is re-enqueued when the + // resource appears. Mutually exclusive with IgnoreIfAbsent; the builder + // rejects both at Build() time. BlockOnAbsence bool + // IgnoreIfAbsent applies to read-only resources. When true, a NotFound + // response from the cluster when reading the resource is silently ignored: + // the entry is skipped, no condition or observation is recorded, the data + // extractor is not invoked, and reconciliation of subsequent resources + // continues unchanged. + // + // State recorded in earlier reconciles (the last observation stored on the + // resource and any data extracted from it) is preserved across an absence + // rather than reset. Resources are reused across reconciles, so any + // downstream consumer that reads such state will see the last-known value + // until a future reconcile finds the resource present again. Use the + // returned ReadOnly+IgnoreIfAbsent flag combination only when last-known + // behavior is acceptable on absence; otherwise gate the resource with a + // When() condition that performs your own existence check. + // + // Use this when the resource is genuinely optional (e.g., a reference to a + // Secret owned by another operator that may or may not exist). It is + // mutually exclusive with BlockOnAbsence and requires ReadOnly. + IgnoreIfAbsent bool } // Builder implements the fluent API for constructing and validating a Component. diff --git a/pkg/component/create.go b/pkg/component/create.go index 115c7bbc..6395ab03 100644 --- a/pkg/component/create.go +++ b/pkg/component/create.go @@ -208,15 +208,20 @@ func reconcileResources( result, err = applyResource(ctx, rec, resource, fieldOwner, mapper) } if err != nil { - if entry.Options.ReadOnly && entry.Options.BlockOnAbsence && apierrors.IsNotFound(err) { - results = append(results, reconcileResult{ - Entry: entry, - Status: convergingStatusWithReason{ - Status: convergingStatusGuardBlocked, - Reason: fmt.Sprintf("waiting for %s", resource.Identity()), - }, - }) - return results, nil + if entry.Options.ReadOnly && apierrors.IsNotFound(err) { + switch { + case entry.Options.IgnoreIfAbsent: + continue + case entry.Options.BlockOnAbsence: + results = append(results, reconcileResult{ + Entry: entry, + Status: convergingStatusWithReason{ + Status: convergingStatusGuardBlocked, + Reason: fmt.Sprintf("waiting for %s", resource.Identity()), + }, + }) + return results, nil + } } return nil, err } diff --git a/pkg/component/create_test.go b/pkg/component/create_test.go index de83d831..487c7fb4 100644 --- a/pkg/component/create_test.go +++ b/pkg/component/create_test.go @@ -558,3 +558,96 @@ func TestReconcileResources_BlockOnAbsence(t *testing.T) { follower.AssertNotCalled(t, "Object") }) } + +func TestReconcileResources_IgnoreIfAbsent(t *testing.T) { + var ( + scheme = setupScheme() + namespace = "test-namespace" + owner = &MockOperatorCRD{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-owner", + Namespace: namespace, + }, + } + fakeClient = fake.NewClientBuilder().WithScheme(scheme).WithObjects(owner).Build() + reconcileContext = setupReconcileContext(scheme, owner, fakeClient) + mapper = createTestRESTMapper() + ctx = t.Context() + ) + + t.Run("a missing read-only resource with IgnoreIfAbsent is silently skipped", func(t *testing.T) { + missing := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "absent-optional-secret", + Namespace: namespace, + }, + } + resource := &MockResource{} + resource.On("Object").Return(missing, nil) + resource.On("Identity").Return("v1/Secret/absent-optional-secret") + + entry := reconcileEntry{ + Resource: resource, + Options: ResourceOptions{ReadOnly: true, IgnoreIfAbsent: true}, + } + + results, err := reconcileResources(ctx, reconcileContext, []reconcileEntry{entry}, "comp", mapper) + + require.NoError(t, err) + assert.Empty(t, results, "absent IgnoreIfAbsent resource must contribute no condition") + }) + + t.Run("subsequent resources still reconcile after an ignored absence", func(t *testing.T) { + missingLeader := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "absent-optional-leader", + Namespace: namespace, + }, + } + leader := &MockResource{} + leader.On("Object").Return(missingLeader, nil) + leader.On("Identity").Return("v1/Secret/absent-optional-leader") + + presentFollower := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "present-follower", + Namespace: namespace, + }, + } + require.NoError(t, fakeClient.Create(ctx, presentFollower)) + + follower := &MockResource{} + follower.On("Object").Return(presentFollower, nil) + follower.On("Identity").Return("v1/Secret/present-follower") + + entries := []reconcileEntry{ + {Resource: leader, Options: ResourceOptions{ReadOnly: true, IgnoreIfAbsent: true}}, + {Resource: follower, Options: ResourceOptions{ReadOnly: true}}, + } + + _, err := reconcileResources(ctx, reconcileContext, entries, "comp", mapper) + + require.NoError(t, err) + follower.AssertCalled(t, "Object") + }) + + t.Run("a missing read-only resource without any absence flag still errors", func(t *testing.T) { + missing := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "absent-strict-default", + Namespace: namespace, + }, + } + resource := &MockResource{} + resource.On("Object").Return(missing, nil) + resource.On("Identity").Return("v1/Secret/absent-strict-default") + + entry := reconcileEntry{ + Resource: resource, + Options: ResourceOptions{ReadOnly: true}, + } + + _, err := reconcileResources(ctx, reconcileContext, []reconcileEntry{entry}, "comp", mapper) + require.Error(t, err) + }) +} diff --git a/pkg/component/resource_options_builder.go b/pkg/component/resource_options_builder.go index 4cba7c52..b2dc9155 100644 --- a/pkg/component/resource_options_builder.go +++ b/pkg/component/resource_options_builder.go @@ -1,6 +1,10 @@ package component -import "github.com/sourcehawk/operator-component-framework/pkg/feature" +import ( + "fmt" + + "github.com/sourcehawk/operator-component-framework/pkg/feature" +) // ResourceOptionsBuilder constructs a ResourceOptions value, optionally // integrating with the feature gating system to control whether a resource @@ -16,6 +20,7 @@ type ResourceOptionsBuilder struct { readOnly bool blockOnAbsence bool + ignoreIfAbsent bool participationMode ParticipationMode suppressGraceInconsistencyWarning bool } @@ -89,12 +94,36 @@ func (b *ResourceOptionsBuilder) ReadOnly() *ResourceOptionsBuilder { // not verify that a watch exists; without one the component will only retry on // its periodic resync. // -// The flag has no effect on managed (non-read-only) resources. +// Only valid alongside ReadOnly(); Build() returns an error otherwise. +// Mutually exclusive with IgnoreIfAbsent(); Build() returns an error if both +// are set. func (b *ResourceOptionsBuilder) BlockOnAbsence() *ResourceOptionsBuilder { b.blockOnAbsence = true return b } +// IgnoreIfAbsent opts a read-only resource into "optional" semantics: if the +// cluster reports NotFound when reading the resource, the framework silently +// skips this entry and continues reconciling subsequent resources. No +// condition is reported, no observation is recorded, and the data extractor +// is not invoked. +// +// State recorded in earlier reconciles (the last observation stored on the +// resource and any data extracted from it) is preserved across an absence +// rather than reset. Downstream consumers that read that state will see the +// last-known value until a future reconcile finds the resource present +// again. Pick this flag only when last-known-good behavior on absence is +// acceptable; otherwise gate the resource with a When() condition that +// performs your own existence check. +// +// Only valid alongside ReadOnly(); Build() returns an error otherwise. +// Mutually exclusive with BlockOnAbsence(); Build() returns an error if both +// are set. +func (b *ResourceOptionsBuilder) IgnoreIfAbsent() *ResourceOptionsBuilder { + b.ignoreIfAbsent = true + return b +} + // Build evaluates the configured feature and truth conditions and returns // the resulting ResourceOptions. // @@ -108,6 +137,22 @@ func (b *ResourceOptionsBuilder) BlockOnAbsence() *ResourceOptionsBuilder { // - If Delete is true, ReadOnly is forced to false (deletion takes precedence). // - ParticipationMode is preserved regardless of deletion state. func (b *ResourceOptionsBuilder) Build() (ResourceOptions, error) { + if b.blockOnAbsence && b.ignoreIfAbsent { + return ResourceOptions{}, fmt.Errorf( + "resource options BlockOnAbsence and IgnoreIfAbsent are mutually exclusive", + ) + } + if b.blockOnAbsence && !b.readOnly { + return ResourceOptions{}, fmt.Errorf( + "resource option BlockOnAbsence requires ReadOnly", + ) + } + if b.ignoreIfAbsent && !b.readOnly { + return ResourceOptions{}, fmt.Errorf( + "resource option IgnoreIfAbsent requires ReadOnly", + ) + } + shouldDelete := false if b.feature != nil { @@ -133,6 +178,7 @@ func (b *ResourceOptionsBuilder) Build() (ResourceOptions, error) { Delete: shouldDelete, ReadOnly: b.readOnly && !shouldDelete, BlockOnAbsence: b.blockOnAbsence, + IgnoreIfAbsent: b.ignoreIfAbsent, ParticipationMode: b.participationMode, SuppressGraceInconsistencyWarning: b.suppressGraceInconsistencyWarning, }, nil diff --git a/pkg/component/resource_options_builder_test.go b/pkg/component/resource_options_builder_test.go index f97260c7..d39a36b0 100644 --- a/pkg/component/resource_options_builder_test.go +++ b/pkg/component/resource_options_builder_test.go @@ -177,6 +177,24 @@ func TestResourceOptionsBuilder_Build(t *testing.T) { }, want: ResourceOptions{Delete: true, ReadOnly: false, BlockOnAbsence: true}, }, + { + name: "ignore if absent sets flag alongside read-only", + build: func() (ResourceOptions, error) { + return NewResourceOptionsBuilder().ReadOnly().IgnoreIfAbsent().Build() + }, + want: ResourceOptions{ReadOnly: true, IgnoreIfAbsent: true}, + }, + { + name: "ignore if absent preserved when deletion forced", + build: func() (ResourceOptions, error) { + return NewResourceOptionsBuilder(). + WithFeatureGate(&disabledFeature{}). + ReadOnly(). + IgnoreIfAbsent(). + Build() + }, + want: ResourceOptions{Delete: true, ReadOnly: false, IgnoreIfAbsent: true}, + }, { name: "last WithFeatureGate wins", build: func() (ResourceOptions, error) { @@ -226,3 +244,60 @@ func TestResourceOptionsFor(t *testing.T) { require.Error(t, err) }) } + +func TestResourceOptionsBuilder_ValidationErrors(t *testing.T) { + tests := []struct { + name string + build func() (ResourceOptions, error) + wantErrIs string + }{ + { + name: "IgnoreIfAbsent without ReadOnly errors", + build: func() (ResourceOptions, error) { + return NewResourceOptionsBuilder().IgnoreIfAbsent().Build() + }, + wantErrIs: "IgnoreIfAbsent requires ReadOnly", + }, + { + name: "BlockOnAbsence without ReadOnly errors", + build: func() (ResourceOptions, error) { + return NewResourceOptionsBuilder().BlockOnAbsence().Build() + }, + wantErrIs: "BlockOnAbsence requires ReadOnly", + }, + { + name: "BlockOnAbsence and IgnoreIfAbsent are mutually exclusive", + build: func() (ResourceOptions, error) { + return NewResourceOptionsBuilder(). + ReadOnly(). + BlockOnAbsence(). + IgnoreIfAbsent(). + Build() + }, + wantErrIs: "BlockOnAbsence and IgnoreIfAbsent are mutually exclusive", + }, + { + name: "BlockOnAbsence + ReadOnly + disabled feature does not error", + build: func() (ResourceOptions, error) { + return NewResourceOptionsBuilder(). + WithFeatureGate(&disabledFeature{}). + ReadOnly(). + BlockOnAbsence(). + Build() + }, + wantErrIs: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := tt.build() + if tt.wantErrIs == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErrIs) + }) + } +}