Addressing review comments part 4:

* Add docs.go and interface docs
* Remove pkg/metadata/finalizer.go changes and create a separate PR kubernetes-sigs#625
* nits and adding comments
* move envtest makefile changes to kubernetes-sigs#636
* removing labeller changes and moving them to kubernetes-sigs#631
* Fix regression when removing labeller changes:
  `dynamic-controller      Error syncing item, requeuing with rate limit   {"item": {"NamespacedKey":"chainsaw-special-quail/test-instance","GVR":{"Group":"kro.run","Version":"v1alpha1","Resource":"checkinstancecreationsimpledeployments"}}, "error": "failed to setup instance: failed to set finalizer: error getting finalizers: .metadata.finalizers accessor error: [] is of the type []string, expected []interface{}"}`

Author: barney-s

Makefile

@@ -94,9 +94,9 @@ vet: ## Run go vet against code.
 	go vet ./...
 
 .PHONY: test
-test: manifests generate fmt vet envtest ## Run tests. Use WHAT=unit or WHAT=integration
+test: manifests generate fmt vet ## Run tests. Use WHAT=unit or WHAT=integration
 ifeq ($(WHAT),integration)
-	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_VERSION) --bin-dir $(LOCALBIN) -p path)" go test -v ./test/integration/suites/... -coverprofile integration-cover.out
+	go test -v ./test/integration/suites/... -coverprofile integration-cover.out
 else ifeq ($(WHAT),unit)
 	go test -v ./pkg/... -coverprofile unit-cover.out
 else
@@ -190,7 +190,6 @@ $(CHAINSAW): $(LOCALBIN)
 	fi
 	test -s $(LOCALBIN)/chainsaw || GOBIN=$(LOCALBIN) GO111MODULE=on go install github.com/kyverno/chainsaw@$(CHAINSAW_VERSION)
 
-ENVTEST_VERSION ?= 1.31.x
 
 .PHONY: ko
 ko: $(KO) ## Download ko locally if necessary. If wrong version is installed, it will be removed before downloading.
@@ -216,12 +215,6 @@ $(CONTROLLER_GEN): $(LOCALBIN)
 	test -s $(LOCALBIN)/controller-gen && $(LOCALBIN)/controller-gen --version | grep -q $(CONTROLLER_TOOLS_VERSION) || \
 	GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_TOOLS_VERSION)
 
-.PHONY: envtest
-envtest: $(ENVTEST) ## Download envtest-setup locally if necessary.
-$(ENVTEST): $(LOCALBIN)
-	test -s $(LOCALBIN)/setup-envtest || GOBIN=$(LOCALBIN) $(GOPREFIX) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
-
-
 .PHONY: image
 build-image: ko ## Build the kro controller images using ko build
 	echo "Building kro image $(RELEASE_VERSION).."

pkg/applyset/applyset.go

@@ -49,26 +49,120 @@ func (t ToolingID) String() string {
 	return fmt.Sprintf("%s/%s", t.Name, t.Version)
 }
 
+/*
+The Set interface provides methods for:
+  - Add    - Add an object to the set
+  - Apply  - Apply objects in the set to the cluster along with pruning
+  - DryRun - Dry run calls the kubernetes API with dryrun flag set to true.
+    No actual resources are created or pruned.
+
+Add() is used to add object to the apply Set.
+It takes unstructured object.
+It does a get from the cluster to note the resource-version before apply.
+
+Apply() method applies the objects in the set to a Kubernetes cluster. If
+the prune parameter is true, any objects that were previously applied but are
+no longer in the set will be deleted from the cluster.
+
+DryRun() method can be used to see what changes would be made without actually making them.
+
+Example Usage:
+
+	// Create an ApplySet
+	// aset, err := applyset.New(parent, restMapper, dynamicClient, applySetConfig)
+
+	// Add a ConfigMap to the ApplySet
+	// configMap := &unstructured.Unstructured{ ... }
+	err = aset.Add(context.TODO(), applyset.ApplyableObject{
+		Unstructured: configMap,
+		ID:  "my-config-map", // optional
+	})
+	if err != nil {
+		log.Fatalf("Failed to add object to ApplySet: %v", err)
+	}
+
+	// Apply the changes to the cluster (or dry-run)
+	// To apply:
+	result, err := aset.Apply(context.TODO(), true) // true to enable pruning
+	// or dry-run:
+	// result, err := aset.DryRun(context.TODO(), true)
+	if err != nil {
+		log.Fatalf("Failed to apply/dry-run ApplySet: %v", err)
+	}
+
+	if result.Errors() != nil {
+		fmt.Printf("ApplySet completed with errors: %v\n", result.Errors())
+	} else {
+		fmt.Println("ApplySet completed successfully (or dry-run successful).")
+	}
+*/
 type Set interface {
 	Add(ctx context.Context, obj ApplyableObject) error
 	Apply(ctx context.Context, prune bool) (*ApplyResult, error)
 	DryRun(ctx context.Context, prune bool) (*ApplyResult, error)
 }
 
 type Config struct {
-	ToolLabels   map[string]string
+	// ToolLabels can be used to inject labels into all resources managed by applyset
+	ToolLabels map[string]string
+
+	// Provide an identifier which is used as field manager for server side apply
+	// https://kubernetes.io/docs/reference/using-api/server-side-apply/#managers
 	FieldManager string
-	ToolingID    ToolingID
-	Log          logr.Logger
+
+	// concats the name and version and adds it as an annotatiuon
+	// https://kubernetes.io/docs/reference/using-api/server-side-apply/#managers
+	ToolingID ToolingID
+
+	// Log is used to inject the calling reconciler's logger
+	Log logr.Logger
 
 	// Callbacks
-	LoadCallback        func(obj ApplyableObject, clusterValue *unstructured.Unstructured) error
-	AfterApplyCallback  func(obj AppliedObject) error
+
+	// LoadCallback is called after an object is read from the cluster.
+	// This is done as part of Add() interface call.
+	LoadCallback func(obj ApplyableObject, clusterValue *unstructured.Unstructured) error
+
+	// AfterApplyCallback is called after an object has been applied to the cluster.
+	// This callback is passed the return value of the apply call.
+	AfterApplyCallback func(obj AppliedObject) error
+
+	// BeforePruneCallback is called before an object is pruned from the cluster.
+	// This callback is passed the object being pruned.
+	// This would not be part of the current applyset by definition.
 	BeforePruneCallback func(obj PrunedObject) error
 }
 
-// New creates a new ApplySet
-// parent object is expected to be the current one existing in the cluster
+/*
+New creates a new ApplySet
+parent object is expected to be the current one existing in the cluster
+Use New() to create an apply Set. This function takes the parent object,
+a RESTMapper, a dynamic client, and a configuration object. The parent
+object is the object that "owns" the set of resources being applied.
+
+Example usage:
+
+		// Set up Kubernetes client and RESTMapper
+		// dynamicClient = ...
+		// restMapper = ...
+
+		// Define a parent. The parent should exist in the cluster
+		// Can be any Kubernetes resource even a custom resource instance
+		parent := &unstructured.Unstructured{ Object: map[string]interface{}{} } }
+		parent.SetGroupVersionKind(schema.GroupVersionKind{Group: "example.com", Version: "v1", Kind: "MyCustomResource"})
+		parent.SetName("somename")
+
+	    // ApplySet configuration
+		applySetConfig := applyset.Config{
+			ToolingID:    applyset.ToolingID{Name: "my-controller", Version: "v1.0.0"},
+			FieldManager: "my-controller",
+			Log:          logr.Discard(), // Use a real logger in production
+		}
+
+		// Create an ApplySet
+		aset, err := applyset.New(parent, restMapper, dynamicClient, applySetConfig)
+		// if err != nil { ...  }
+*/
 func New(
 	parent *unstructured.Unstructured,
 	restMapper meta.RESTMapper,

pkg/applyset/const.go

@@ -1,3 +1,4 @@
+// Copyright 2023 The Kubernetes Authors.
 // Copyright 2025 The Kube Resource Orchestrator Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");

pkg/applyset/docs.go

@@ -0,0 +1,51 @@
+// Copyright 2023 The Kubernetes Authors.
+// Copyright 2025 The Kube Resource Orchestrator 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 applyset provides a library for managing sets of Kubernetes objects.
+The core of this package is the Set interface, which defines the operations
+for an apply set. An apply set is a collection of Kubernetes objects that are
+managed together as a group. This is useful for managing the resources of a
+composite object, like a CRD that creates other resources.
+
+This is implemented as per the KEP:
+https://github.com/kubernetes/enhancements/blob/master/keps/sig-cli/3659-kubectl-apply-prune/README.md
+
+Existing implementations of the KEP:
+
+ 1. kubectl implements the KEP in its codebase:
+    https://github.com/kubernetes/kubectl/blob/master/pkg/cmd/apply/applyset.go
+    https://github.com/kubernetes/kubectl/blob/master/pkg/cmd/apply/applyset_pruner.go
+    cli focussed, pulls in a lot of dependencies not required for a controller.
+
+ 2. kubernetes-declarative-pattern (kdp)
+    https://pkg.go.dev/sigs.k8s.io/kubebuilder-declarative-pattern/applylib/applyset
+    controller focussed, missing callbacks, lifecycle support etc.
+
+Why Fork:
+* The kubectl implementation is built for cli and has many dependencies that we dont need in a controller.
+* The kdp implementation is good for controller but lacks some enhancements that are required for it to work with KRO.
+* We need the ability to do partial applies of an applyset (without pruning) that is not supported by KDP library.
+* Also we need the ability to read-in the result of an apply for use by KRO engine. This requires some callback support.
+* We ended up adding call-backs (to populate KRO engine)
+* Also we enhanced applylib to support ExternalRef (read from cluster, no changes) - KRO specific for now.
+* Planning to add LifeCycle modifiers to support additional behaviours like abandon on delete, no update to a resource after creation etc.
+
+Upstreaming:
+The goal is to upstream the changes back to either kubernetes-declarative-pattern or controller-runtime.
+We may need to do the enhancements and decide if all of it or some of it can be upstreamed.
+We will need to do a few iterations in the KRO repo first before we get an idea of where and how to upstream.
+*/
+package applyset

pkg/applyset/prune.go

@@ -1,3 +1,4 @@
+// Copyright 2023 The Kubernetes Authors.
 // Copyright 2025 The Kube Resource Orchestrator Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -34,6 +35,14 @@ import (
 	"k8s.io/client-go/dynamic"
 )
 
+const (
+	// This is set to an arbitrary number here for now.
+	// This ensures we are no unbounded when pruning many GVKs.
+	// Could be parameterized later on
+	// TODO (barney-s): Possible parameterization target
+	PruneGVKParallelizationLimit = 5
+)
+
 // PruneObject is an apiserver object that should be deleted as part of prune.
 type PruneObject struct {
 	*unstructured.Unstructured
@@ -64,7 +73,8 @@ func (a *applySet) FindAllObjectsToPrune(
 		restMapping *meta.RESTMapping
 		results     []PruneObject
 	}
-	var tasks []*task
+
+	tasks := make([]*task, 0, len(a.desiredRESTMappings))
 
 	// We run discovery in parallel, in as many goroutines as priority and fairness will allow
 	// (We don't expect many requests in real-world scenarios - maybe tens, unlikely to be hundreds)
@@ -95,15 +105,15 @@ func (a *applySet) FindAllObjectsToPrune(
 	}
 
 	group, ctx := errgroup.WithContext(ctx)
+	group.SetLimit(PruneGVKParallelizationLimit)
 	for i := range tasks {
 		task := tasks[i]
 		group.Go(func() error {
 			results, err := a.findObjectsToPrune(ctx, dynamicClient, visitedUIDs, task.namespace, task.restMapping)
 			if err != nil {
 				return fmt.Errorf("listing %v objects for pruning: %w", task.restMapping.GroupVersionKind.String(), err)
-			} else {
-				task.results = results
 			}
+			task.results = results
 			return nil
 		})
 	}

pkg/applyset/result.go

@@ -1,4 +1,5 @@
 // Copyright 2025 The Kube Resource Orchestrator Authors
+// Copyright 2022 The Kubernetes Authors.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -12,6 +13,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// Derived from: https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern/blob/master/applylib/applyset/results.go
+
 package applyset
 
 import (
@@ -52,6 +55,11 @@ type PrunedObject struct {
 	Error error
 }
 
+// ApplyResult summarizes the results of an apply operation
+// It returns a list of applied and pruned objects
+// AppliedObject has both the input object and the result of the apply operation
+// Any errors returned by the apply call is also recorded in it.
+// PrunedObject records any error returned by the delete call.
 type ApplyResult struct {
 	DesiredCount   int
 	AppliedObjects []AppliedObject

pkg/applyset/tracker.go

@@ -44,7 +44,10 @@ type Applyable interface {
 type ApplyableObject struct {
 	*unstructured.Unstructured
 
+	// Optional
 	// User provided unique identifier for the object.
+	// If present a uniqeness check is done when adding
+	// It is opaque and is passed in the callbacks as is
 	ID string
 
 	// Lifecycle hints
@@ -104,12 +107,12 @@ func (t *tracker) Add(obj ApplyableObject) error {
 	// TODO(barney-s): Do we need to care about client side uniqueness?
 	// We could just not take the ID (opaque string) and let user deal with mapping
 	// GVKNN to their ID. Adding a todo here to revisit this.
-
-	// client side uniqueness check
-	if _, found := t.clientIDs[obj.ID]; found {
-		return fmt.Errorf("duplicate object ID %v", obj.ID)
+	if obj.ID != "" {
+		if _, found := t.clientIDs[obj.ID]; found {
+			return fmt.Errorf("duplicate object ID %v", obj.ID)
+		}
+		t.clientIDs[obj.ID] = true
 	}
-	t.clientIDs[obj.ID] = true
 
 	// Ensure the object is marshallable
 	if _, err := json.Marshal(obj); err != nil {

pkg/client/fake/fake.go

@@ -20,6 +20,7 @@ import (
 	"github.com/kro-run/kro/pkg/client"
 	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
@@ -31,6 +32,7 @@ type FakeSet struct {
 	KubernetesClient    kubernetes.Interface
 	ApiExtensionsClient apiextensionsv1.ApiextensionsV1Interface
 	Config              *rest.Config
+	restMapper          meta.RESTMapper
 }
 
 var _ client.SetInterface = (*FakeSet)(nil)
@@ -75,6 +77,14 @@ func (f *FakeSet) WithImpersonation(user string) (client.SetInterface, error) {
 	return f, nil
 }
 
+func (f *FakeSet) RESTMapper() meta.RESTMapper {
+	return f.restMapper
+}
+
+func (f *FakeSet) SetRESTMapper(restMapper meta.RESTMapper) {
+	f.restMapper = restMapper
+}
+
 // FakeCRD is a fake implementation of CRDInterface for testing
 type FakeCRD struct{}
 

pkg/client/set.go

@@ -46,6 +46,9 @@ type SetInterface interface {
 
 	// WithImpersonation returns a new client that impersonates the given user
 	WithImpersonation(user string) (SetInterface, error)
+
+	RESTMapper() meta.RESTMapper
+	SetRESTMapper(restMapper meta.RESTMapper)
 }
 
 // Set provides a unified interface for different Kubernetes clients
@@ -156,11 +159,6 @@ func (c *Set) RESTMapper() meta.RESTMapper {
 	return c.restMapper
 }
 
-// RESTMapper returns the REST mapper
-func (c *Set) RESTMapper() meta.RESTMapper {
-	return c.restMapper
-}
-
 // CRD returns a new CRDInterface instance
 func (c *Set) CRD(cfg CRDWrapperConfig) CRDInterface {
 	if cfg.Client == nil {