diff --git a/commands/alpha/live/plan/command.go b/commands/alpha/live/plan/command.go index 886face464..843958831c 100644 --- a/commands/alpha/live/plan/command.go +++ b/commands/alpha/live/plan/command.go @@ -133,7 +133,7 @@ func (r *Runner) RunE(c *cobra.Command, args []string) error { } // Create and execute the planner. - planner, err := kptplanner.NewClusterPlanner(r.factory) + planner, err := kptplanner.NewClusterPlannerWithContext(r.ctx, r.factory) if err != nil { return err } diff --git a/commands/live/apply/cmdapply.go b/commands/live/apply/cmdapply.go index 2c5ea3e0c1..70ed253c77 100644 --- a/commands/live/apply/cmdapply.go +++ b/commands/live/apply/cmdapply.go @@ -228,7 +228,7 @@ func runApply(r *Runner, invInfo inventory.Info, objs []*unstructured.Unstructur if err = cmdutil.InstallResourceGroupCRD(r.ctx, f); err != nil { return err } - } else if !live.ResourceGroupCRDMatched(f) { + } else if !live.ResourceGroupCRDMatchedWithContext(r.ctx, f) { if err = cmdutil.InstallResourceGroupCRD(r.ctx, f); err != nil { return &cmdutil.ResourceGroupCRDNotLatestError{ Err: err, @@ -239,7 +239,7 @@ func runApply(r *Runner, invInfo inventory.Info, objs []*unstructured.Unstructur // Run the applier. It will return a channel where we can receive updates // to keep track of progress and any issues. - invClient, err := inventory.NewClient(r.factory, live.WrapInventoryObj, live.InvToUnstructuredFunc, r.statusPolicy, live.ResourceGroupGVK) + invClient, err := inventory.NewClient(r.factory, live.WrapInventoryObjWithContext(r.ctx), live.InvToUnstructuredFunc, r.statusPolicy, live.ResourceGroupGVK) if err != nil { return err } diff --git a/commands/live/destroy/cmddestroy.go b/commands/live/destroy/cmddestroy.go index bc274f2578..3d37503f5b 100644 --- a/commands/live/destroy/cmddestroy.go +++ b/commands/live/destroy/cmddestroy.go @@ -168,7 +168,7 @@ func (r *Runner) runE(c *cobra.Command, args []string) error { func runDestroy(r *Runner, inv inventory.Info, dryRunStrategy common.DryRunStrategy) error { // Run the destroyer. It will return a channel where we can receive updates // to keep track of progress and any issues. - invClient, err := inventory.NewClient(r.factory, live.WrapInventoryObj, live.InvToUnstructuredFunc, r.statusPolicy, live.ResourceGroupGVK) + invClient, err := inventory.NewClient(r.factory, live.WrapInventoryObjWithContext(r.ctx), live.InvToUnstructuredFunc, r.statusPolicy, live.ResourceGroupGVK) if err != nil { return err } @@ -192,7 +192,7 @@ func runDestroy(r *Runner, inv inventory.Info, dryRunStrategy common.DryRunStrat DryRunStrategy: dryRunStrategy, EmitStatusEvents: true, } - ch := destroyer.Run(context.Background(), inv, options) + ch := destroyer.Run(r.ctx, inv, options) // Print the preview strategy unless the output format is json. if dryRunStrategy.ClientOrServerDryRun() && r.output != printers.JSONPrinter { diff --git a/commands/live/livecmd.go b/commands/live/livecmd.go index fea734d7cb..e177668bad 100644 --- a/commands/live/livecmd.go +++ b/commands/live/livecmd.go @@ -47,7 +47,7 @@ func GetCommand(ctx context.Context, _, version string) *cobra.Command { } f := util.NewFactory(liveCmd, version) - invFactory := live.NewClusterClientFactory() + invFactory := live.NewClusterClientFactoryWithContext(ctx) loader := status.NewRGInventoryLoader(ctx, f) // Init command which updates a Kptfile for the ResourceGroup inventory object. diff --git a/commands/live/migrate/migratecmd.go b/commands/live/migrate/migratecmd.go index 616e228898..874d2c66fe 100644 --- a/commands/live/migrate/migratecmd.go +++ b/commands/live/migrate/migratecmd.go @@ -58,8 +58,8 @@ type Runner struct { name string rgFile string force bool - rgInvClientFunc func(util.Factory) (inventory.Client, error) - cmInvClientFunc func(util.Factory) (inventory.Client, error) + rgInvClientFunc func(context.Context, util.Factory) (inventory.Client, error) + cmInvClientFunc func(context.Context, util.Factory) (inventory.Client, error) cmLoader manifestreader.ManifestLoader cmNotMigrated bool // flag to determine if migration from ConfigMap has occurred } @@ -348,11 +348,11 @@ func validateParams(reader io.Reader, args []string) error { return nil } -func rgInvClient(factory util.Factory) (inventory.Client, error) { - return inventory.NewClient(factory, live.WrapInventoryObj, live.InvToUnstructuredFunc, inventory.StatusPolicyAll, live.ResourceGroupGVK) +func rgInvClient(ctx context.Context, factory util.Factory) (inventory.Client, error) { + return inventory.NewClient(factory, live.WrapInventoryObjWithContext(ctx), live.InvToUnstructuredFunc, inventory.StatusPolicyAll, live.ResourceGroupGVK) } -func cmInvClient(factory util.Factory) (inventory.Client, error) { +func cmInvClient(_ context.Context, factory util.Factory) (inventory.Client, error) { return inventory.NewClient(factory, inventory.WrapInventoryObj, inventory.InvInfoToConfigMap, inventory.StatusPolicyAll, live.ResourceGroupGVK) } @@ -412,11 +412,11 @@ func (mr *Runner) migrateKptfileToRG(args []string) error { func (mr *Runner) migrateCMToRG(stdinBytes []byte, args []string) error { // Create the inventory clients for reading inventories based on RG and // ConfigMap. - rgInvClient, err := mr.rgInvClientFunc(mr.factory) + rgInvClient, err := mr.rgInvClientFunc(mr.ctx, mr.factory) if err != nil { return err } - cmInvClient, err := mr.cmInvClientFunc(mr.factory) + cmInvClient, err := mr.cmInvClientFunc(mr.ctx, mr.factory) if err != nil { return err } diff --git a/commands/live/migrate/migratecmd_test.go b/commands/live/migrate/migratecmd_test.go index b6dbadda80..51273b01aa 100644 --- a/commands/live/migrate/migratecmd_test.go +++ b/commands/live/migrate/migratecmd_test.go @@ -15,6 +15,7 @@ package migrate import ( + "context" "os" "path/filepath" "strings" @@ -169,7 +170,7 @@ func TestKptMigrate_migrateKptfileToRG(t *testing.T) { migrateRunner := NewRunner(ctx, tf, cmLoader, ioStreams) migrateRunner.dryRun = tc.dryRun migrateRunner.rgFile = tc.rgFilename - migrateRunner.cmInvClientFunc = func(_ util.Factory) (inventory.Client, error) { + migrateRunner.cmInvClientFunc = func(_ context.Context, _ util.Factory) (inventory.Client, error) { return inventory.NewFakeClient([]object.ObjMetadata{}), nil } err = migrateRunner.migrateKptfileToRG([]string{dir}) @@ -247,7 +248,7 @@ func TestKptMigrate_retrieveConfigMapInv(t *testing.T) { // Create MigrateRunner and call "retrieveConfigMapInv" cmLoader := manifestreader.NewManifestLoader(tf) migrateRunner := NewRunner(ctx, tf, cmLoader, ioStreams) - migrateRunner.cmInvClientFunc = func(_ util.Factory) (inventory.Client, error) { + migrateRunner.cmInvClientFunc = func(_ context.Context, _ util.Factory) (inventory.Client, error) { return inventory.NewFakeClient([]object.ObjMetadata{}), nil } actual, err := migrateRunner.retrieveConfigMapInv(strings.NewReader(tc.configMap), []string{"-"}) diff --git a/pkg/live/inventory-client-factory.go b/pkg/live/inventory-client-factory.go index 74ded93db7..eaf01f9cc0 100644 --- a/pkg/live/inventory-client-factory.go +++ b/pkg/live/inventory-client-factory.go @@ -1,4 +1,4 @@ -// Copyright 2022 The kpt Authors +// Copyright 2022,2026 The kpt Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -15,18 +15,53 @@ package live import ( + "context" + cmdutil "k8s.io/kubectl/pkg/cmd/util" "sigs.k8s.io/cli-utils/pkg/inventory" ) -// ClusterClientFactory is a factory that creates instances of ClusterClient inventory client. +// ClusterClientFactory is a factory that creates instances of ClusterClient +// inventory client. +// +// Ctx, if set, is plumbed into the InventoryResourceGroup wrapper so +// Apply / ApplyWithPrune honor caller cancellation (Ctrl-C, timeouts). +// The upstream inventory.ClientFactory interface's NewClient signature +// does not accept a context, so we carry one on the factory instead; +// construct via NewClusterClientFactoryWithContext when you have one. type ClusterClientFactory struct { StatusPolicy inventory.StatusPolicy + Ctx context.Context } +// NewClusterClientFactory returns a ClusterClientFactory that will build +// inventory clients with no context propagation (cluster API calls use +// context.Background()). Prefer NewClusterClientFactoryWithContext. func NewClusterClientFactory() *ClusterClientFactory { return &ClusterClientFactory{StatusPolicy: inventory.StatusPolicyNone} } + +// NewClusterClientFactoryWithContext returns a ClusterClientFactory that +// threads ctx into every inventory client it produces. +// +// A nil ctx is normalized to context.Background() so the docstring's +// promise ("threads ctx into every inventory client") holds for every +// input: there is no hidden code path that silently drops propagation. +func NewClusterClientFactoryWithContext(ctx context.Context) *ClusterClientFactory { + if ctx == nil { + ctx = context.Background() + } + return &ClusterClientFactory{StatusPolicy: inventory.StatusPolicyNone, Ctx: ctx} +} + func (ccf *ClusterClientFactory) NewClient(factory cmdutil.Factory) (inventory.Client, error) { - return inventory.NewClient(factory, WrapInventoryObj, InvToUnstructuredFunc, ccf.StatusPolicy, ResourceGroupGVK) + // Defense in depth: normalize a nil Ctx here too. This covers the + // case where a caller constructed ClusterClientFactory as a struct + // literal (e.g. &ClusterClientFactory{StatusPolicy: ...}) and left + // Ctx unset — see NewClusterClientFactory, which does exactly that. + ctx := ccf.Ctx + if ctx == nil { + ctx = context.Background() + } + return inventory.NewClient(factory, WrapInventoryObjWithContext(ctx), InvToUnstructuredFunc, ccf.StatusPolicy, ResourceGroupGVK) } diff --git a/pkg/live/inventory-client-factory_test.go b/pkg/live/inventory-client-factory_test.go new file mode 100644 index 0000000000..c4c4346cfc --- /dev/null +++ b/pkg/live/inventory-client-factory_test.go @@ -0,0 +1,60 @@ +// Copyright 2026 The kpt Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package live + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestNewClusterClientFactoryWithContext_NilIsNormalized verifies nil ctx is +// normalized to Background(). +func TestNewClusterClientFactoryWithContext_NilIsNormalized(t *testing.T) { + //nolint:staticcheck // SA1012: deliberately passing nil to exercise the nil-safety guard. + ccf := NewClusterClientFactoryWithContext(nil) + require.NotNil(t, ccf, "NewClusterClientFactoryWithContext returned nil") + require.NotNil(t, ccf.Ctx, "expected nil ctx to be normalized to Background(); got nil") + // Background() never cancels; Done() returns a nil channel. + require.Nil(t, ccf.Ctx.Done(), "expected Background()-equivalent ctx; Done() returned non-nil") +} + +// TestNewClusterClientFactoryWithContext_PreservesRealCtx stores the caller +// ctx and preserves cancellation. +func TestNewClusterClientFactoryWithContext_PreservesRealCtx(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + ccf := NewClusterClientFactoryWithContext(ctx) + require.Same(t, ctx, ccf.Ctx, "factory stored a different ctx than the one passed in") + assert.NotNil(t, ccf.Ctx.Done(), "ctx should expose Done channel") + cancel() + select { + case <-ccf.Ctx.Done(): + default: + require.FailNow(t, "factory ctx did not observe cancellation of the source ctx") + } +} + +// TestNewClusterClientFactory_StructLiteralPathTolerated keeps legacy nil ctx. +func TestNewClusterClientFactory_StructLiteralPathTolerated(t *testing.T) { + // Exercises the legacy constructor which leaves Ctx unset. + ccf := NewClusterClientFactory() + require.Nil(t, ccf.Ctx, "legacy constructor must not synthesize a ctx; callers rely on nil to signal opt-out") + // We don't call ccf.NewClient here because it needs a real + // cmdutil.Factory; the observable contract is that inside NewClient + // the nil Ctx is normalized to Background(). That path is exercised + // in the existing apply/destroy tests via the CLI integration tests. +} diff --git a/pkg/live/inventoryrg.go b/pkg/live/inventoryrg.go index 5434d9abad..a2ac526ed9 100644 --- a/pkg/live/inventoryrg.go +++ b/pkg/live/inventoryrg.go @@ -1,4 +1,4 @@ -// Copyright 2020 The kpt Authors +// Copyright 2020,2026 The kpt Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -56,15 +56,23 @@ var ResourceGroupGVK = schema.GroupVersionKind{ Kind: "ResourceGroup", } -// InventoryResourceGroup wraps a ResourceGroup resource and implements -// the Inventory and InventoryInfo interface. This wrapper loads and stores the -// object metadata (inventory) to and from the wrapped ResourceGroup. +// InventoryResourceGroup wraps a ResourceGroup inventory and carries a caller +// context for API calls. type InventoryResourceGroup struct { + ctx context.Context inv *unstructured.Unstructured objMetas []object.ObjMetadata objStatus []actuation.ObjectStatus } +// contextOrBackground returns ctx when set, otherwise Background(). +func (icm *InventoryResourceGroup) contextOrBackground() context.Context { + if icm.ctx != nil { + return icm.ctx + } + return context.Background() +} + func (icm *InventoryResourceGroup) Strategy() inventory.Strategy { return inventory.NameStrategy } @@ -72,9 +80,7 @@ func (icm *InventoryResourceGroup) Strategy() inventory.Strategy { var _ inventory.Storage = &InventoryResourceGroup{} var _ inventory.Info = &InventoryResourceGroup{} -// WrapInventoryObj takes a passed ResourceGroup (as a resource.Info), -// wraps it with the InventoryResourceGroup and upcasts the wrapper as -// an the Inventory interface. +// WrapInventoryObj wraps inventory and uses Background() for API calls. func WrapInventoryObj(obj *unstructured.Unstructured) inventory.Storage { if obj != nil { klog.V(4).Infof("wrapping Inventory obj: %s/%s\n", obj.GetNamespace(), obj.GetName()) @@ -82,6 +88,20 @@ func WrapInventoryObj(obj *unstructured.Unstructured) inventory.Storage { return &InventoryResourceGroup{inv: obj} } +// WrapInventoryObjWithContext returns a WrapObjFunc that stores ctx. +// Nil ctx is normalized to Background(). +func WrapInventoryObjWithContext(ctx context.Context) func(*unstructured.Unstructured) inventory.Storage { + if ctx == nil { + ctx = context.Background() + } + return func(obj *unstructured.Unstructured) inventory.Storage { + if obj != nil { + klog.V(4).Infof("wrapping Inventory obj with ctx: %s/%s\n", obj.GetNamespace(), obj.GetName()) + } + return &InventoryResourceGroup{ctx: ctx, inv: obj} + } +} + func WrapInventoryInfoObj(obj *unstructured.Unstructured) inventory.Info { if obj != nil { klog.V(4).Infof("wrapping InventoryInfo obj: %s/%s\n", obj.GetNamespace(), obj.GetName()) @@ -256,9 +276,10 @@ func (icm *InventoryResourceGroup) Apply(dc dynamic.Interface, mapper meta.RESTM if err != nil { return err } + ctx := icm.contextOrBackground() - // Get cluster object, if exsists. - clusterObj, err := namespacedClient.Get(context.TODO(), invInfo.GetName(), metav1.GetOptions{}) + // Get cluster object, if exists. + clusterObj, err := namespacedClient.Get(ctx, invInfo.GetName(), metav1.GetOptions{}) if err != nil && !apierrors.IsNotFound(err) { return err } @@ -267,10 +288,10 @@ func (icm *InventoryResourceGroup) Apply(dc dynamic.Interface, mapper meta.RESTM if clusterObj == nil { // Create cluster inventory object, if it does not exist on cluster. - appliedObj, err = namespacedClient.Create(context.TODO(), invInfo, metav1.CreateOptions{}) + appliedObj, err = namespacedClient.Create(ctx, invInfo, metav1.CreateOptions{}) } else { // Update the cluster inventory object instead. - appliedObj, err = namespacedClient.Update(context.TODO(), invInfo, metav1.UpdateOptions{}) + appliedObj, err = namespacedClient.Update(ctx, invInfo, metav1.UpdateOptions{}) } if err != nil { return err @@ -279,7 +300,7 @@ func (icm *InventoryResourceGroup) Apply(dc dynamic.Interface, mapper meta.RESTM // Update status. if statusPolicy == inventory.StatusPolicyAll { invInfo.SetResourceVersion(appliedObj.GetResourceVersion()) - _, err = namespacedClient.UpdateStatus(context.TODO(), invInfo, metav1.UpdateOptions{}) + _, err = namespacedClient.UpdateStatus(ctx, invInfo, metav1.UpdateOptions{}) } return err @@ -290,11 +311,12 @@ func (icm *InventoryResourceGroup) ApplyWithPrune(dc dynamic.Interface, mapper m if err != nil { return err } + ctx := icm.contextOrBackground() // Update the cluster inventory object. // Since the ResourceGroup CRD specifies the status as a sub-resource, this // will not update the status. - appliedObj, err := namespacedClient.Update(context.TODO(), invInfo, metav1.UpdateOptions{}) + appliedObj, err := namespacedClient.Update(ctx, invInfo, metav1.UpdateOptions{}) if err != nil { return err } @@ -314,7 +336,7 @@ func (icm *InventoryResourceGroup) ApplyWithPrune(dc dynamic.Interface, mapper m if err != nil { return err } - _, err = namespacedClient.UpdateStatus(context.TODO(), appliedObj, metav1.UpdateOptions{}) + _, err = namespacedClient.UpdateStatus(ctx, appliedObj, metav1.UpdateOptions{}) if err != nil { return err } @@ -384,9 +406,17 @@ func ResourceGroupCRDApplied(factory cmdutil.Factory) bool { return true } -// ResourceGroupCRDMatched checks if the ResourceGroup CRD -// in the cluster matches the CRD in the kpt binary. +// ResourceGroupCRDMatched checks the cluster CRD using Background(). func ResourceGroupCRDMatched(factory cmdutil.Factory) bool { + return ResourceGroupCRDMatchedWithContext(context.Background(), factory) +} + +// ResourceGroupCRDMatchedWithContext checks the cluster CRD using ctx. +// Nil ctx is normalized to Background(). +func ResourceGroupCRDMatchedWithContext(ctx context.Context, factory cmdutil.Factory) bool { + if ctx == nil { + ctx = context.Background() + } mapper, err := factory.ToRESTMapper() if err != nil { klog.V(4).Infof("error retrieving RESTMapper when checking ResourceGroup CRD: %s\n", err) @@ -410,7 +440,7 @@ func ResourceGroupCRDMatched(factory cmdutil.Factory) bool { return false } - liveCRD, err := dc.Resource(mapping.Resource).Get(context.TODO(), "resourcegroups.kpt.dev", metav1.GetOptions{ + liveCRD, err := dc.Resource(mapping.Resource).Get(ctx, "resourcegroups.kpt.dev", metav1.GetOptions{ TypeMeta: metav1.TypeMeta{ APIVersion: crd.GetAPIVersion(), Kind: "CustomResourceDefinition", diff --git a/pkg/live/inventoryrg_test.go b/pkg/live/inventoryrg_test.go index 9e72668732..36439df142 100644 --- a/pkg/live/inventoryrg_test.go +++ b/pkg/live/inventoryrg_test.go @@ -1,4 +1,4 @@ -// Copyright 2020 The kpt Authors +// Copyright 2020,2026 The kpt Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -15,10 +15,14 @@ package live import ( + "context" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + cmdutil "k8s.io/kubectl/pkg/cmd/util" "sigs.k8s.io/cli-utils/pkg/apis/actuation" "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/cli-utils/pkg/inventory" @@ -155,31 +159,17 @@ func TestLoadStore(t *testing.T) { _ = wrapped.Store(tc.objs, tc.objStatus) invStored, err := wrapped.GetObject() if tc.isError { - if err == nil { - t.Fatalf("expected error but received none") - } - return - } - if !tc.isError && err != nil { - t.Fatalf("unexpected error %v received", err) + require.Error(t, err) return } + require.NoError(t, err) wrapped = WrapInventoryObj(invStored) objs, err := wrapped.Load() - if !tc.isError && err != nil { - t.Fatalf("unexpected error %v received", err) - return - } - if !objs.Equal(tc.objs) { - t.Fatalf("expected inventory objs (%v), got (%v)", tc.objs, objs) - } + require.NoError(t, err) + require.True(t, objs.Equal(tc.objs), "expected inventory objs (%v), got (%v)", tc.objs, objs) resourceStatus, _, err := unstructured.NestedSlice(invStored.Object, "status", "resourceStatuses") - if err != nil { - t.Fatalf("unexpected error %v received", err) - } - if len(resourceStatus) != len(tc.objStatus) { - t.Fatalf("expected %d resource status but got %d", len(tc.objStatus), len(resourceStatus)) - } + require.NoError(t, err) + require.Len(t, resourceStatus, len(tc.objStatus), "expected %d resource status but got %d", len(tc.objStatus), len(resourceStatus)) }) } } @@ -225,18 +215,78 @@ func TestIsResourceGroupInventory(t *testing.T) { t.Run(name, func(t *testing.T) { actual, err := IsResourceGroupInventory(tc.invObj) if tc.isError { - if err == nil { - t.Fatalf("expected error but received none") - } - return - } - if !tc.isError && err != nil { - t.Fatalf("unexpected error %v received", err) + require.Error(t, err) return } - if tc.expected != actual { - t.Errorf("expected inventory as (%t), got (%t)", tc.expected, actual) - } + require.NoError(t, err) + assert.Equal(t, tc.expected, actual, "expected inventory as (%t), got (%t)", tc.expected, actual) }) } } + +// TestWrapInventoryObjWithContext_StoresContext stores ctx on the wrapper. +func TestWrapInventoryObjWithContext_StoresContext(t *testing.T) { + type ctxKey struct{} + ctx := context.WithValue(context.Background(), ctxKey{}, "propagated") + + storage := WrapInventoryObjWithContext(ctx)(inventoryObj) + icm, ok := storage.(*InventoryResourceGroup) + require.True(t, ok, "WrapInventoryObjWithContext produced unexpected type %T", storage) + require.NotNil(t, icm.ctx, "expected ctx on InventoryResourceGroup; got nil") + require.Equal(t, "propagated", icm.ctx.Value(ctxKey{}), "expected stored ctx to carry propagated value") +} + +// TestWrapInventoryObj_LeavesContextNil keeps legacy nil ctx. +func TestWrapInventoryObj_LeavesContextNil(t *testing.T) { + storage := WrapInventoryObj(inventoryObj) + icm, ok := storage.(*InventoryResourceGroup) + require.True(t, ok, "WrapInventoryObj produced unexpected type %T", storage) + require.Nil(t, icm.ctx, "expected legacy wrapper to leave ctx nil; got %v", icm.ctx) +} + +// TestWrapInventoryObjWithContext_NilCtxDefaultsToBackground normalizes nil ctx. +func TestWrapInventoryObjWithContext_NilCtxDefaultsToBackground(t *testing.T) { + //nolint:staticcheck // SA1012: deliberately passing a nil context to exercise the nil-safety guard. + storage := WrapInventoryObjWithContext(nil)(inventoryObj) + icm, ok := storage.(*InventoryResourceGroup) + require.True(t, ok, "WrapInventoryObjWithContext(nil) produced unexpected type %T", storage) + require.NotNil(t, icm.ctx, "expected nil ctx to be normalized to Background(); got nil") + // Background() never cancels; Done() returns a nil channel. + require.Nil(t, icm.ctx.Done(), "expected Background()-equivalent ctx; Done() returned non-nil") +} + +// TestResourceGroupCRDMatched_BackCompatSignaturePreserved pins exported signatures. +func TestResourceGroupCRDMatched_BackCompatSignaturePreserved(t *testing.T) { + pinSignatures := func( + _ func(cmdutil.Factory) bool, + _ func(context.Context, cmdutil.Factory) bool, + ) { + } + pinSignatures(ResourceGroupCRDMatched, ResourceGroupCRDMatchedWithContext) +} + +// TestContextOrBackground covers stored ctx and fallback behavior. +func TestContextOrBackground(t *testing.T) { + t.Run("returns stored ctx when set", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + icm := &InventoryResourceGroup{ctx: ctx} + + got := icm.contextOrBackground() + require.Same(t, ctx, got, "expected contextOrBackground to return the stored ctx") + // Cancellation on the original ctx must be visible through the returned ctx. + cancel() + select { + case <-got.Done(): + default: + require.FailNow(t, "returned ctx did not observe cancellation of the stored ctx") + } + }) + + t.Run("falls back to Background when nil", func(t *testing.T) { + icm := &InventoryResourceGroup{} + got := icm.contextOrBackground() + require.NotNil(t, got, "contextOrBackground returned nil; expected context.Background()") + // Background() never cancels; Done() returns a nil channel. + require.Nil(t, got.Done(), "expected Background-equivalent ctx; Done channel was not nil") + }) +} diff --git a/pkg/live/planner/cluster.go b/pkg/live/planner/cluster.go index 8a1760be2a..e537c2b6aa 100644 --- a/pkg/live/planner/cluster.go +++ b/pkg/live/planner/cluster.go @@ -47,13 +47,29 @@ type ClusterPlanner struct { resourceFetcher ResourceFetcher } +// NewClusterPlanner builds a ClusterPlanner using context.Background() +// for the underlying inventory-client cluster calls. +// +// This signature is preserved for backward compatibility with external +// callers; it delegates to NewClusterPlannerWithContext. Prefer the +// context-aware constructor when you have a caller context so Ctrl-C +// and command-level timeouts can cancel inventory I/O. func NewClusterPlanner(f util.Factory) (*ClusterPlanner, error) { + return NewClusterPlannerWithContext(context.Background(), f) +} + +// NewClusterPlannerWithContext is the context-aware variant of +// NewClusterPlanner. ctx is plumbed into the inventory wrapper so +// Apply / ApplyWithPrune on the underlying ResourceGroup honor caller +// cancellation (Ctrl-C, deadlines). A nil ctx is normalized to +// context.Background() by WrapInventoryObjWithContext. +func NewClusterPlannerWithContext(ctx context.Context, f util.Factory) (*ClusterPlanner, error) { fetcher, err := NewResourceFetcher(f) if err != nil { return nil, err } - invClient, err := inventory.NewClient(f, live.WrapInventoryObj, live.InvToUnstructuredFunc, inventory.StatusPolicyNone, live.ResourceGroupGVK) + invClient, err := inventory.NewClient(f, live.WrapInventoryObjWithContext(ctx), live.InvToUnstructuredFunc, inventory.StatusPolicyNone, live.ResourceGroupGVK) if err != nil { return nil, err } diff --git a/pkg/live/planner/cluster_test.go b/pkg/live/planner/cluster_test.go index 71699c38b5..f72a43f035 100644 --- a/pkg/live/planner/cluster_test.go +++ b/pkg/live/planner/cluster_test.go @@ -19,8 +19,10 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/kubectl/pkg/cmd/util" "sigs.k8s.io/cli-utils/pkg/apply" "sigs.k8s.io/cli-utils/pkg/apply/event" "sigs.k8s.io/cli-utils/pkg/inventory" @@ -112,9 +114,8 @@ func TestClusterPlanner(t *testing.T) { }).BuildPlan(ctx, &FakeInventoryInfo{}, []*unstructured.Unstructured{}, Options{}) require.NoError(t, err) - if diff := cmp.Diff(tc.expectedPlan, plan); diff != "" { - t.Errorf("plan mismatch (-want +got):\n%s", diff) - } + diff := cmp.Diff(tc.expectedPlan, plan) + assert.Empty(t, diff, "plan mismatch (-want +got):\n%s", diff) }) } } @@ -166,3 +167,13 @@ func (fii *FakeInventoryInfo) ID() string { func (fii *FakeInventoryInfo) Strategy() inventory.Strategy { return inventory.NameStrategy } + +// TestNewClusterPlanner_BackCompatSignaturePreserved pins exported signatures. +func TestNewClusterPlanner_BackCompatSignaturePreserved(t *testing.T) { + pinSignatures := func( + _ func(util.Factory) (*ClusterPlanner, error), + _ func(context.Context, util.Factory) (*ClusterPlanner, error), + ) { + } + pinSignatures(NewClusterPlanner, NewClusterPlannerWithContext) +}