Repository: crossplane/crossplane
Pull request #5815 (2024-07-04 07:43:17)

Add configuration.meta/crossplane.yaml support for dependencies to beta validate
<!--
Thank you for helping to improve Crossplane! Please read the contribution docs
(linked below) if this is your first Crossplane pull request.
-->

### Description of your changes

<!--
Briefly describe what this pull request does, and how it is covered by tests.
Be proactive - direct your reviewers' attention to anything that needs special
consideration.

We love pull requests that fix an open issue. If yours does, use the below line
to indicate which issue it fixes, for example "Fixes #500".
-->
This PR enables `crossplane beta validate` command to use `Configuration.meta/crossplane.yaml` for pulling dependencies. With this PR `beta validate` command can validate the correctness of the Configurations using their `crossplane.yaml` file before the build, without an already published `Configuration.pkg`. 

Here is what this PR does:

**Current usage** (with https://github.com/enesonus/configuration-aws-eks/tree/config-meta-validate):

_We need an already built and published Configuration.pkg specified at examples/configuration.yaml_
```
crossplane beta render examples/eks-xr.yaml apis/composition.yaml examples/functions.yaml --include-full-xr | crossplane beta validate examples/configuration.yaml -
 
package schemas does not exist, downloading:  xpkg.upbound.io/crossplane-contrib/provider-kubernetes:v0.12.1
package schemas does not exist, downloading:  xpkg.upbound.io/upbound/provider-aws-ec2:v1.2.0
package schemas does not exist, downloading:  xpkg.upbound.io/upbound/provider-aws-eks:v1.2.0
package schemas does not exist, downloading:  xpkg.upbound.io/upbound/provider-aws-iam:v1.2.0
package schemas does not exist, downloading:  xpkg.upbound.io/crossplane-contrib/function-patch-and-transform:v0.4.0
package schemas does not exist, downloading:  xpkg.upbound.io/upbound/configuration-aws-eks:v0.11.0
package schemas does not exist, downloading:  xpkg.upbound.io/upbound/configuration-aws-network:v0.12.0
package schemas does not exist, downloading:  xpkg.upbound.io/crossplane-contrib/provider-helm:v0.17.0
[x] schema validation error aws.platform.upbound.io/v1alpha1, Kind=XEKS, configuration-aws-eks : spec.parameters.deletionPolicy: Required value
[x] schema validation error aws.platform.upbound.io/v1alpha1, Kind=XEKS, configuration-aws-eks : spec.parameters.providerConfigName: Required value
[✓] kubernetes.crossplane.io/v1alpha2, Kind=Object, configuration-aws-eks-aws-auth validated successfully
[✓] iam.aws.upbound.io/v1beta1, Kind=RolePolicyAttachment, clusterRolePolicyAttachment validated successfully
[✓] iam.aws.upbound.io/v1beta1, Kind=RolePolicyAttachment, cniRolePolicyAttachment validated successfully
[✓] iam.aws.upbound.io/v1beta1, Kind=RolePolicyAttachment, containerRegistryRolePolicyAttachment validated successfully
[✓] iam.aws.upbound.io/v1beta1, Kind=Role, controlplaneRole validated successfully
[✓] iam.aws.upbound.io/v1beta1, Kind=RolePolicyAttachment, ebsCsiRolePolicyAttachment validated successfully
[✓] kubernetes.crossplane.io/v1alpha2, Kind=Object, configuration-aws-eks-irsa-settings validated successfully
[✓] eks.aws.upbound.io/v1beta1, Kind=Cluster, kubernetesCluster validated successfully
[x] schema validation error eks.aws.upbound.io/v1beta1, Kind=ClusterAuth, kubernetesClusterAuth : spec.writeConnectionSecretToRef.name: Required value
[✓] iam.aws.upbound.io/v1beta1, Kind=Role, nodegroupRole validated successfully
[x] schema validation error helm.crossplane.io/v1beta1, Kind=ProviderConfig, configuration-aws-eks : spec.credentials.secretRef.name: Required value
[x] schema validation error kubernetes.crossplane.io/v1alpha1, Kind=ProviderConfig, configuration-aws-eks : spec.credentials.secretRef.name: Required value
[✓] iam.aws.upbound.io/v1beta1, Kind=RolePolicyAttachment, workerNodeRolePolicyAttachment validated successfully
Total 14 resources: 0 missing schemas, 10 success cases, 4 failure cases
crossplane: error: cannot validate resources: could not validate all resources

```

**New usage** (with https://github.com/enesonus/configuration-aws-eks/tree/config-meta-validate):

_No need for building and publishing the package. Pulls dependencies from `Configuration.meta/crossplane.yaml`_
```
crossplane beta render examples/eks-xr.yaml apis/composition.yaml examples/functions.yaml --include-full-xr | go run cmd/crank/main.go beta validate crossplane.yaml -

package schemas does not exist, downloading:  xpkg.upbound.io/crossplane-contrib/provider-kubernetes:v0.12.1
package schemas does not exist, downloading:  xpkg.upbound.io/upbound/provider-aws-ec2:v1.2.0
package schemas does not exist, downloading:  xpkg.upbound.io/crossplane-contrib/function-patch-and-transform:v0.4.0
package schemas does not exist, downloading:  xpkg.upbound.io/crossplane-contrib/function-kcl:v0.6.0
package schemas does not exist, downloading:  xpkg.upbound.io/crossplane-contrib/function-auto-ready:v0.2.1
package schemas does not exist, downloading:  xpkg.upbound.io/crossplane-contrib/function-sequencer:v0.1.2
package schemas does not exist, downloading:  xpkg.upbound.io/upbound/configuration-aws-network:v0.12.0
package schemas does not exist, downloading:  xpkg.upbound.io/crossplane-contrib/provider-helm:v0.17.0
package schemas does not exist, downloading:  xpkg.upbound.io/upbound/provider-aws-eks:v1.2.0
package schemas does not exist, downloading:  xpkg.upbound.io/upbound/provider-aws-iam:v1.2.0
[!] could not find CRD/XRD for: aws.platform.upbound.io/v1alpha1, Kind=XEKS
[✓] kubernetes.crossplane.io/v1alpha2, Kind=Object, configuration-aws-eks-aws-auth validated successfully
[✓] iam.aws.upbound.io/v1beta1, Kind=RolePolicyAttachment, clusterRolePolicyAttachment validated successfully
[✓] iam.aws.upbound.io/v1beta1, Kind=RolePolicyAttachment, cniRolePolicyAttachment validated successfully
[✓] iam.aws.upbound.io/v1beta1, Kind=RolePolicyAttachment, containerRegistryRolePolicyAttachment validated successfully
[✓] iam.aws.upbound.io/v1beta1, Kind=Role, controlplaneRole validated successfully
[✓] iam.aws.upbound.io/v1beta1, Kind=RolePolicyAttachment, ebsCsiRolePolicyAttachment validated successfully
[✓] kubernetes.crossplane.io/v1alpha2, Kind=Object, configuration-aws-eks-irsa-settings validated successfully
[✓] eks.aws.upbound.io/v1beta1, Kind=Cluster, kubernetesCluster validated successfully
[x] schema validation error eks.aws.upbound.io/v1beta1, Kind=ClusterAuth, kubernetesClusterAuth : spec.writeConnectionSecretToRef.name: Required value
[✓] iam.aws.upbound.io/v1beta1, Kind=Role, nodegroupRole validated successfully
[x] schema validation error helm.crossplane.io/v1beta1, Kind=ProviderConfig, configuration-aws-eks : spec.credentials.secretRef.name: Required value
[x] schema validation error kubernetes.crossplane.io/v1alpha1, Kind=ProviderConfig, configuration-aws-eks : spec.credentials.secretRef.name: Required value
[✓] iam.aws.upbound.io/v1beta1, Kind=RolePolicyAttachment, workerNodeRolePolicyAttachment validated successfully
Total 14 resources: 1 missing schemas, 10 success cases, 3 failure cases
crossplane: error: cannot validate resources: could not validate all resources
```

Fixes https://github.com/crossplane/crossplane/issues/5728

I have: <!--You MUST either [x] check or [ ] ~strike through~ every item.-->

- [x] Read and followed Crossplane's [contribution process].
- [x] Run `earthly +reviewable` to ensure this PR is ready for review.
- [x] Added or updated unit tests.
- [ ] ~Added or updated e2e tests.~
- [ ] ~Linked a PR or a [docs tracking issue] to [document this change].~
- [ ] ~Added `backport release-x.y` labels to auto-backport this PR.~

Need help with this checklist? See the [cheat sheet].

[contribution process]: https://github.com/crossplane/crossplane/tree/master/contributing
[docs tracking issue]: https://github.com/crossplane/docs/issues/new
[document this change]: https://docs.crossplane.io/contribute/contribute
[cheat sheet]: https://github.com/crossplane/crossplane/tree/master/contributing#checklist-cheat-sheet



Patch:
diff --git a/README.md b/README.md
index ee6292fa7..24f7abb44 100644
--- a/README.md
+++ b/README.md
@@ -82,7 +82,6 @@ find the meetings in the [Community Calendar][community calendar].
 - [#sig-composition-functions][sig-composition-functions-slack]
 - [#sig-deletion-ordering][sig-deletion-ordering-slack]
 - [#sig-devex][sig-devex-slack]
-- [#sig-docs][sig-docs-slack]
 - [#sig-e2e-testing][sig-e2e-testing-slack]
 - [#sig-observability][sig-observability-slack]
 - [#sig-observe-only][sig-observe-only-slack]
@@ -129,7 +128,6 @@ Crossplane is under the Apache 2.0 license.
 [sig-composition-functions-slack]: https://crossplane.slack.com/archives/C031Y29CSAE
 [sig-deletion-ordering-slack]: https://crossplane.slack.com/archives/C05BP8W5ALW
 [sig-devex-slack]: https://crossplane.slack.com/archives/C05U1LLM3B2
-[sig-docs-slack]: https://crossplane.slack.com/archives/C02CAQ52DPU
 [sig-e2e-testing-slack]: https://crossplane.slack.com/archives/C05C8CCTVNV
 [sig-observability-slack]: https://crossplane.slack.com/archives/C061GNH3LA0
 [sig-observe-only-slack]: https://crossplane.slack.com/archives/C04D5988QEA
diff --git a/apis/generate.go b/apis/generate.go
index c272ffe2b..7ca29c9b5 100644
--- a/apis/generate.go
+++ b/apis/generate.go
@@ -18,7 +18,7 @@ limitations under the License.
 */
 
 // NOTE(negz): See the below link for details on what is happening here.
-// https://go.dev/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module
+// https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module
 
 // Remove existing manifests
 //go:generate rm -rf ../cluster/crds
diff --git a/cmd/crank/beta/trace/internal/resource/xrm/client.go b/cmd/crank/beta/trace/internal/resource/xrm/client.go
index b143d2724..336c80860 100644
--- a/cmd/crank/beta/trace/internal/resource/xrm/client.go
+++ b/cmd/crank/beta/trace/internal/resource/xrm/client.go
@@ -33,15 +33,11 @@ import (
 	"github.com/crossplane/crossplane/cmd/crank/beta/trace/internal/resource"
 )
 
-// defaultConcurrency is the concurrency using which the resource tree if loaded when not explicitly specified.
-const defaultConcurrency = 5
-
 // Client to get a Resource with all its children.
 type Client struct {
 	getConnectionSecrets bool
 
-	client      client.Client
-	concurrency int
+	client client.Client
 }
 
 // ResourceClientOption is a functional option for a Client.
@@ -54,20 +50,12 @@ func WithConnectionSecrets(v bool) ResourceClientOption {
 	}
 }
 
-// WithConcurrency is a functional option that sets the concurrency for the resource load.
-func WithConcurrency(n int) ResourceClientOption {
-	return func(c *Client) {
-		c.concurrency = n
-	}
-}
-
 // NewClient returns a new Client.
 func NewClient(in client.Client, opts ...ResourceClientOption) (*Client, error) {
 	uClient := xpunstructured.NewClient(in)
 
 	c := &Client{
-		client:      uClient,
-		concurrency: defaultConcurrency,
+		client: uClient,
 	}
 
 	for _, o := range opts {
@@ -79,20 +67,25 @@ func NewClient(in client.Client, opts ...ResourceClientOption) (*Client, error)
 
 // GetResourceTree returns the requested Crossplane Resource and all its children.
 func (kc *Client) GetResourceTree(ctx context.Context, root *resource.Resource) (*resource.Resource, error) {
-	q := newLoader(root, kc, defaultChannelCapacity)
-	q.load(ctx, kc.concurrency)
-	return root, nil
-}
+	// Set up a FIFO queue to traverse the resource tree breadth first.
+	queue := []*resource.Resource{root}
 
-// loadResource returns the resource for the specified object reference.
-func (kc *Client) loadResource(ctx context.Context, ref *v1.ObjectReference) *resource.Resource {
-	return resource.GetResource(ctx, kc.client, ref)
-}
+	for len(queue) > 0 {
+		// Pop the first element from the queue.
+		res := queue[0]
+		queue = queue[1:]
 
-// getResourceChildrenRefs returns the references to the children for the given
-// Resource, assuming it's a Crossplane resource, XR or XRC.
-func (kc *Client) getResourceChildrenRefs(r *resource.Resource) []v1.ObjectReference {
-	return getResourceChildrenRefs(r, kc.getConnectionSecrets)
+		refs := getResourceChildrenRefs(res, kc.getConnectionSecrets)
+
+		for i := range refs {
+			child := resource.GetResource(ctx, kc.client, &refs[i])
+
+			res.Children = append(res.Children, child)
+			queue = append(queue, child)
+		}
+	}
+
+	return root, nil
 }
 
 // getResourceChildrenRefs returns the references to the children for the given
diff --git a/cmd/crank/beta/trace/internal/resource/xrm/loader.go b/cmd/crank/beta/trace/internal/resource/xrm/loader.go
deleted file mode 100644
index 5580faa5d..000000000
--- a/cmd/crank/beta/trace/internal/resource/xrm/loader.go
+++ /dev/null
@@ -1,150 +0,0 @@
-/*
-Copyright 2023 The Crossplane 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 xrm
-
-import (
-	"context"
-	"sort"
-	"sync"
-
-	v1 "k8s.io/api/core/v1"
-
-	"github.com/crossplane/crossplane/cmd/crank/beta/trace/internal/resource"
-)
-
-// defaultChannelCapacity is the buffer size of the processing channel, should be a high value
-// so that there is no blocking. Correctness of processing does not depend on the channel capacity.
-const defaultChannelCapacity = 1000
-
-// workItem maintains the relationship of a resource to be loaded with its parent
-// such that the resource that is loaded can be added as a child.
-type workItem struct {
-	parent *resource.Resource
-	child  v1.ObjectReference
-}
-
-// resourceLoader is a delegate that loads resources and returns child resource refs.
-type resourceLoader interface {
-	loadResource(ctx context.Context, ref *v1.ObjectReference) *resource.Resource
-	getResourceChildrenRefs(r *resource.Resource) []v1.ObjectReference
-}
-
-// loader loads resources concurrently.
-type loader struct {
-	root         *resource.Resource // the root resource for which the tree is loaded
-	rl           resourceLoader     // the resource loader
-	resourceLock sync.Mutex         // lock when updating the children of any resource
-	processing   sync.WaitGroup     // "counter" to track requests in flight
-	ch           chan workItem      // processing channel
-	done         chan struct{}      // done channel, signaled when all resources are loaded
-}
-
-// newLoader creates a loader for the root resource.
-func newLoader(root *resource.Resource, rl resourceLoader, channelCapacity int) *loader {
-	l := &loader{
-		rl:   rl,
-		ch:   make(chan workItem, channelCapacity),
-		done: make(chan struct{}),
-		root: root,
-	}
-	return l
-}
-
-// load loads the full resource tree in a concurrent manner.
-func (l *loader) load(ctx context.Context, concurrency int) {
-	// make sure counters are incremented for root child refs before starting concurrent processing
-	refs := l.rl.getResourceChildrenRefs(l.root)
-	l.addRefs(l.root, refs)
-
-	// signal the done channel after all items are processed
-	go func() {
-		l.processing.Wait()
-		close(l.done)
-	}()
-
-	if concurrency < 1 {
-		concurrency = defaultConcurrency
-	}
-	var wg sync.WaitGroup
-	for range concurrency {
-		wg.Add(1)
-		// spin up a worker that processes items from the channel until the done channel is signaled.
-		go func() {
-			defer wg.Done()
-			for {
-				select {
-				case <-l.done:
-					return
-				case item := <-l.ch:
-					l.processItem(ctx, item)
-				}
-			}
-		}()
-	}
-	wg.Wait()
-	// order of children loaded for resources is not deterministic because of concurrent processing.
-	// Sort children explicitly to make this so.
-	sortRefs(l.root)
-}
-
-func sortRefs(root *resource.Resource) {
-	for _, child := range root.Children {
-		sortRefs(child)
-	}
-	// this duplicates the sorting logic from internal/controller/apiextensions/composite/composition_functions.go
-	sort.Slice(root.Children, func(i, j int) bool {
-		l := root.Children[i].Unstructured
-		r := root.Children[j].Unstructured
-		return l.GetAPIVersion()+l.GetKind()+l.GetName() < r.GetAPIVersion()+r.GetKind()+r.GetName()
-	})
-}
-
-// addRefs adds work items to the queue.
-func (l *loader) addRefs(parent *resource.Resource, refs []v1.ObjectReference) {
-	// only perform work and spin up a goroutine if references are present.
-	if len(refs) == 0 {
-		return
-	}
-	// ensure counters are updated synchronously
-	l.processing.Add(len(refs))
-	// free up the current processing routine even if the channel would block.
-	go func() {
-		for _, ref := range refs {
-			l.ch <- workItem{
-				parent: parent,
-				child:  ref,
-			}
-		}
-	}()
-}
-
-// processItem processes a single work item in the queue and decrements the in-process counter
-// after adding child references.
-func (l *loader) processItem(ctx context.Context, item workItem) {
-	defer l.processing.Done()
-	res := l.rl.loadResource(ctx, &item.child)
-	refs := l.rl.getResourceChildrenRefs(res)
-	l.updateChild(item, res)
-	l.addRefs(res, refs)
-}
-
-// updateChild adds the supplied child resource to its parent.
-func (l *loader) updateChild(item workItem, res *resource.Resource) {
-	l.resourceLock.Lock()
-	item.parent.Children = append(item.parent.Children, res)
-	l.resourceLock.Unlock()
-}
diff --git a/cmd/crank/beta/trace/trace.go b/cmd/crank/beta/trace/trace.go
index 924c05934..98704c9c0 100644
--- a/cmd/crank/beta/trace/trace.go
+++ b/cmd/crank/beta/trace/trace.go
@@ -69,7 +69,6 @@ type Cmd struct {
 	ShowPackageDependencies   string `default:"unique"                              enum:"unique,all,none"                             help:"Show package dependencies in the output. One of: unique, all, none." name:"show-package-dependencies"`
 	ShowPackageRevisions      string `default:"active"                              enum:"active,all,none"                             help:"Show package revisions in the output. One of: active, all, none."    name:"show-package-revisions"`
 	ShowPackageRuntimeConfigs bool   `default:"false"                               help:"Show package runtime configs in the output." name:"show-package-runtime-configs"`
-	Concurrency               int    `default:"5"                                   help:"load concurrency"                            name:"concurrency"`
 }
 
 // Help returns help message for the trace command.
@@ -209,10 +208,7 @@ func (c *Cmd) Run(k *kong.Context, logger logging.Logger) error {
 		}
 	default:
 		logger.Debug("Requested resource is not a package, assumed to be an XR, XRC or MR")
-		treeClient, err = xrm.NewClient(client,
-			xrm.WithConnectionSecrets(c.ShowConnectionSecrets),
-			xrm.WithConcurrency(c.Concurrency),
-		)
+		treeClient, err = xrm.NewClient(client, xrm.WithConnectionSecrets(c.ShowConnectionSecrets))
 		if err != nil {
 			return errors.Wrap(err, errInitKubeClient)
 		}
diff --git a/cmd/crank/beta/validate/manager.go b/cmd/crank/beta/validate/manager.go
index 9d30dc397..0966cf09f 100644
--- a/cmd/crank/beta/validate/manager.go
+++ b/cmd/crank/beta/validate/manager.go
@@ -50,8 +50,8 @@ type Manager struct {
 	writer  io.Writer
 
 	crds  []*extv1.CustomResourceDefinition
-	deps  map[string]bool // One level dependency images
-	confs map[string]bool // Configuration images
+	deps  map[string]bool                  // Dependency images
+	confs map[string]*metav1.Configuration // Configuration images
 }
 
 // NewManager returns a new Manager.
@@ -67,7 +67,7 @@ func NewManager(cacheDir string, fs afero.Fs, w io.Writer) *Manager {
 	m.writer = w
 	m.crds = make([]*extv1.CustomResourceDefinition, 0)
 	m.deps = make(map[string]bool)
-	m.confs = make(map[string]bool)
+	m.confs = make(map[string]*metav1.Configuration)
 
 	return m
 }
@@ -131,7 +131,20 @@ func (m *Manager) PrepExtensions(extensions []*unstructured.Unstructured) error
 				return errors.Wrapf(err, "cannot get package image")
 			}
 
-			m.confs[image] = true
+			m.confs[image] = nil
+
+		case schema.GroupKind{Group: "meta.pkg.crossplane.io", Kind: "Configuration"}:
+			meta, err := e.MarshalJSON()
+			if err != nil {
+				return errors.Wrap(err, "cannot marshal configuration to JSON")
+			}
+
+			cfg := &metav1.Configuration{}
+			if err := yaml.Unmarshal(meta, cfg); err != nil {
+				return errors.Wrapf(err, "cannot unmarshal configuration YAML")
+			}
+
+			m.confs[cfg.Name] = cfg
 
 		default:
 			continue
@@ -171,21 +184,24 @@ func (m *Manager) CacheAndLoad(cleanCache bool) error {
 
 func (m *Manager) addDependencies() error {
 	for image := range m.confs {
-		m.deps[image] = true // we need to download the configuration package for the XRDs
+		cfg := m.confs[image]
 
-		layer, err := m.fetcher.FetchBaseLayer(image)
-		if err != nil {
-			return errors.Wrapf(err, "cannot download package %s", image)
-		}
+		if cfg == nil {
+			m.deps[image] = true // we need to download the configuration package for the XRDs
 
-		_, meta, err := extractPackageContent(*layer)
-		if err != nil {
-			return errors.Wrapf(err, "cannot extract package file and meta")
-		}
+			layer, err := m.fetcher.FetchBaseLayer(image)
+			if err != nil {
+				return errors.Wrapf(err, "cannot download package %s", image)
+			}
 
-		cfg := &metav1.Configuration{}
-		if err := yaml.Unmarshal(meta, cfg); err != nil {
-			return errors.Wrapf(err, "cannot unmarshal configuration YAML")
+			_, meta, err := extractPackageContent(*layer)
+			if err != nil {
+				return errors.Wrapf(err, "cannot extract package file and meta")
+			}
+			if err := yaml.Unmarshal(meta, &cfg); err != nil {
+				return errors.Wrapf(err, "cannot unmarshal configuration YAML")
+			}
+			m.confs[image] = cfg // update the configuration
 		}
 
 		deps := cfg.Spec.MetaSpec.DependsOn
diff --git a/contributing/README.md b/contributing/README.md
index ae6eb62b3..ed5fd72ab 100644
--- a/contributing/README.md
+++ b/contributing/README.md
@@ -854,10 +854,10 @@ func TestExample(t *testing.T) {
 [build submodule]: https://github.com/crossplane/build/
 [`kind`]: https://kind.sigs.k8s.io/
 [Crossplane release cycle]: https://docs.crossplane.io/knowledge-base/guides/release-cycle
-[good git commit hygiene]: https://www.futurelearn.com/info/blog/telling-stories-with-your-git-history?category=using-futurelearn
+[good git commit hygiene]: https://www.futurelearn.com/info/blog/telling-stories-with-your-git-history
 [Developer Certificate of Origin]: https://github.com/apps/dco
-[code review comments]: https://go.dev/wiki/CodeReviewComments
-[test review comments]: https://go.dev/wiki/TestComments
+[code review comments]: https://github.com/golang/go/wiki/CodeReviewComments
+[test review comments]: https://github.com/golang/go/wiki/TestComments
 [E2E readme]: ../test/e2e/README.md
 [docs]: https://github.com/crossplane/docs
 [Effective Go]: https://golang.org/doc/effective_go
@@ -871,4 +871,4 @@ func TestExample(t *testing.T) {
 [CODEOWNERS]: ../CODEOWNERS
 [Reviewers]: ../OWNERS.md#reviewers
 [Maintainers]: ../OWNERS.md#maintainers
-[#4514]: https://github.com/crossplane/crossplane/issues/4514
+[#4514]: https://github.com/crossplane/crossplane/issues/4514
\ No newline at end of file


Test patch:
diff --git a/cmd/crank/beta/trace/internal/resource/xrm/loader_test.go b/cmd/crank/beta/trace/internal/resource/xrm/loader_test.go
deleted file mode 100644
index da2ced64d..000000000
--- a/cmd/crank/beta/trace/internal/resource/xrm/loader_test.go
+++ /dev/null
@@ -1,181 +0,0 @@
-package xrm
-
-import (
-	"context"
-	"fmt"
-	"math/rand"
-	"sync"
-	"testing"
-
-	v1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-
-	"github.com/crossplane/crossplane/cmd/crank/beta/trace/internal/resource"
-)
-
-// simpleGenerator generates a tree of resources for a specific depth and the number of children to
-// create at any level.
-type simpleGenerator struct {
-	childDepth int
-	numItems   int
-	l          sync.Mutex     // lock for accessing the depth map
-	depthMap   map[string]int // tracks resource names and their depth so that we can stop when the desired depth is reached.
-}
-
-func newSimpleGenerator(childDepth, numItems int) *simpleGenerator {
-	return &simpleGenerator{
-		childDepth: childDepth,
-		numItems:   numItems,
-		depthMap:   map[string]int{},
-	}
-}
-
-func (d *simpleGenerator) createResource(apiVersion, kind, name string) *resource.Resource {
-	obj := map[string]any{
-		"apiVersion": apiVersion,
-		"kind":       kind,
-		"metadata": map[string]any{
-			"name": name,
-		},
-	}
-	return &resource.Resource{Unstructured: unstructured.Unstructured{Object: obj}}
-}
-
-func (d *simpleGenerator) trackResourceDepth(name string, depth int) {
-	d.l.Lock()
-	defer d.l.Unlock()
-	d.depthMap[name] = depth
-}
-
-func (d *simpleGenerator) createRefAtDepth(depth int) v1.ObjectReference {
-	prefix := "comp-res"
-	if depth == d.childDepth {
-		prefix = "managed-res"
-	}
-	name := fmt.Sprintf("%s-%d-%d", prefix, rand.Int(), depth)
-	d.trackResourceDepth(name, depth)
-	return v1.ObjectReference{
-		Kind:       fmt.Sprintf("Depth%d", depth),
-		Name:       name,
-		APIVersion: "example.com/v1",
-	}
-}
-
-func (d *simpleGenerator) createResourceFromRef(ref *v1.ObjectReference) *resource.Resource {
-	return d.createResource(ref.APIVersion, ref.Kind, ref.Name)
-}
-
-func (d *simpleGenerator) loadResource(_ context.Context, ref *v1.ObjectReference) *resource.Resource {
-	return d.createResourceFromRef(ref)
-}
-
-func (d *simpleGenerator) depthFromResource(res *resource.Resource) int {
-	d.l.Lock()
-	defer d.l.Unlock()
-	return d.depthMap[res.Unstructured.GetName()]
-}
-
-func (d *simpleGenerator) getResourceChildrenRefs(r *resource.Resource) []v1.ObjectReference {
-	depth := d.depthFromResource(r)
-	if depth == d.childDepth {
-		return nil
-	}
-	ret := make([]v1.ObjectReference, 0, d.numItems)
-	for range d.numItems {
-		ret = append(ret, d.createRefAtDepth(depth+1))
-	}
-	return ret
-}
-
-var _ resourceLoader = &simpleGenerator{}
-
-func countItems(root *resource.Resource) int {
-	ret := 1
-	for _, child := range root.Children {
-		ret += countItems(child)
-	}
-	return ret
-}
-
-func TestLoader(t *testing.T) {
-	type want struct {
-		expectedResources int
-	}
-	type args struct {
-		childDepth      int
-		numItems        int
-		channelCapacity int
-		concurrency     int
-	}
-	tests := map[string]struct {
-		reason string
-		args   args
-		want   want
-	}{
-		"Basic": {
-			reason: "simple test with default concurrency",
-			args: args{
-				childDepth: 3,
-				numItems:   3,
-			},
-			want: want{
-				expectedResources: 1 + 3 + 9 + 27,
-			},
-		},
-		"BlockingBuffer": {
-			reason: "in-process resources greater than channel buffer, causing blocking",
-			args: args{
-				channelCapacity: 1,
-				concurrency:     1,
-				childDepth:      3,
-				numItems:        10,
-			},
-			want: want{
-				expectedResources: 1 + 10 + 100 + 1000,
-			},
-		},
-		"NoRootChildren": {
-			reason: "top-level resource has no children",
-			args: args{
-				childDepth: 0,
-				numItems:   0,
-			},
-			want: want{
-				expectedResources: 1,
-			},
-		},
-		"BadConcurrency": {
-			reason: "invalid concurrency is adjusted to be valid",
-			args: args{
-				concurrency: -1,
-				childDepth:  3,
-				numItems:    3,
-			},
-			want: want{
-				expectedResources: 1 + 3 + 9 + 27,
-			},
-		},
-	}
-
-	for name, test := range tests {
-		t.Run(name, func(t *testing.T) {
-			channelCapacity := defaultChannelCapacity
-			if test.args.channelCapacity > 0 {
-				channelCapacity = test.args.channelCapacity
-			}
-			concurrency := defaultConcurrency
-			if test.args.concurrency != 0 {
-				concurrency = test.args.concurrency
-			}
-			sg := newSimpleGenerator(test.args.childDepth, test.args.numItems)
-			rootRef := sg.createRefAtDepth(0)
-			root := sg.createResourceFromRef(&rootRef)
-			l := newLoader(root, sg, channelCapacity)
-			l.load(context.Background(), concurrency)
-			n := countItems(root)
-			if test.want.expectedResources != n {
-				t.Errorf("resource count mismatch: want %d, got %d", test.want.expectedResources, n)
-			}
-		})
-	}
-}
diff --git a/cmd/crank/beta/validate/manager_test.go b/cmd/crank/beta/validate/manager_test.go
new file mode 100644
index 000000000..1168547c8
--- /dev/null
+++ b/cmd/crank/beta/validate/manager_test.go
@@ -0,0 +1,239 @@
+/*
+Copyright 2024 The Crossplane 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 validate
+
+import (
+	"bytes"
+	"fmt"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	conregv1 "github.com/google/go-containerregistry/pkg/v1"
+	"github.com/google/go-containerregistry/pkg/v1/static"
+	"github.com/google/go-containerregistry/pkg/v1/types"
+	"github.com/spf13/afero"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+
+	"github.com/crossplane/crossplane-runtime/pkg/test"
+)
+
+var (
+	// config-pkg:v1.3.0
+	configPkg = []byte(`apiVersion: meta.pkg.crossplane.io/v1alpha1
+kind: Configuration
+metadata:
+  name: config-pkg
+spec:
+  dependsOn:
+    - provider: provider-dep-1
+      version: "v1.3.0"
+---
+
+`)
+
+	// provider-dep-1:v1.3.0
+	providerYaml = []byte(`apiVersion: meta.pkg.crossplane.io/v1
+kind: Provider
+metadata:
+  name: provider-dep-1
+---
+
+`)
+
+	// function-dep-1:v1.3.0
+	funcYaml = []byte(`apiVersion: meta.pkg.crossplane.io/v1beta1
+kind: Function
+metadata:
+  name: function-dep-1
+---
+
+`)
+)
+
+func TestConfigurationTypeSupport(t *testing.T) {
+	confpkg := static.NewLayer(configPkg, types.OCILayer)
+	pd := static.NewLayer(providerYaml, types.OCILayer)
+	fd := static.NewLayer(funcYaml, types.OCILayer)
+
+	fetchMockFunc := func(image string) (*conregv1.Layer, error) {
+		switch image {
+		case "config-pkg:v1.3.0":
+			return &confpkg, nil
+		case "provider-dep-1:v1.3.0":
+			return &pd, nil
+		case "function-dep-1:v1.3.0":
+			return &fd, nil
+		default:
+			return nil, fmt.Errorf("unknown image: %s", image)
+		}
+	}
+
+	type args struct {
+		extensions []*unstructured.Unstructured
+		fetchMock  func(image string) (*conregv1.Layer, error)
+	}
+	type want struct {
+		err   error
+		confs int
+		deps  int
+	}
+	cases := map[string]struct {
+		reason string
+		args   args
+		want   want
+	}{
+		"SuccessfulConfigPkg": {
+			//config-pkg
+			//└─►provider-dep-1
+			reason: "All dependencies should be successfully added from Configuration.pkg",
+			args: args{
+				extensions: []*unstructured.Unstructured{
+					{
+						Object: map[string]interface{}{
+							"apiVersion": "pkg.crossplane.io/v1alpha1",
+							"kind":       "Configuration",
+							"metadata": map[string]interface{}{
+								"name": "config-pkg",
+							},
+							"spec": map[string]interface{}{
+								"package": "config-pkg:v1.3.0",
+							},
+						},
+					},
+				},
+				fetchMock: fetchMockFunc,
+			},
+			want: want{
+				err:   nil,
+				confs: 1, // Configuration.pkg from remote
+				deps:  2, // 1 provider, 1 Configuration.pkg dependency
+			},
+		},
+		"SuccessfulConfigMeta": {
+			//config-meta
+			//└─►function-dep-1
+			reason: "All dependencies should be successfully added from Configuration.meta",
+			args: args{
+				extensions: []*unstructured.Unstructured{
+					{
+						Object: map[string]interface{}{
+							"apiVersion": "meta.pkg.crossplane.io/v1alpha1",
+							"kind":       "Configuration",
+							"metadata": map[string]interface{}{
+								"name": "config-meta",
+							},
+							"spec": map[string]interface{}{
+								"dependsOn": []map[string]interface{}{
+									{
+										"function": "function-dep-1",
+										"version":  "v1.3.0",
+									},
+								},
+							},
+						},
+					},
+				},
+				fetchMock: fetchMockFunc,
+			},
+			want: want{
+				err:   nil,
+				confs: 1, // Configuration.meta
+				deps:  1, // Not adding Configuration.meta itself to not send it to cacheDependencies() for download
+			},
+		},
+		"SuccessfulConfigMetaAndPkg": {
+			//config-meta
+			//└─►function-dep-1
+			//config-pkg
+			//└─►provider-dep-1
+			reason: "All dependencies should be successfully added from both Configuration.meta and Configuration.pkg",
+			args: args{
+				extensions: []*unstructured.Unstructured{
+					{
+						Object: map[string]interface{}{
+							"apiVersion": "meta.pkg.crossplane.io/v1alpha1",
+							"kind":       "Configuration",
+							"metadata": map[string]interface{}{
+								"name": "config-meta",
+							},
+							"spec": map[string]interface{}{
+								"dependsOn": []map[string]interface{}{
+									{
+										"function": "function-dep-1",
+										"version":  "v1.3.0",
+									},
+								},
+							},
+						},
+					},
+					{
+						Object: map[string]interface{}{
+							"apiVersion": "pkg.crossplane.io/v1alpha1",
+							"kind":       "Configuration",
+							"metadata": map[string]interface{}{
+								"name": "config-pkg",
+							},
+							"spec": map[string]interface{}{
+								"package": "config-pkg:v1.3.0",
+							},
+						},
+					},
+				},
+				fetchMock: fetchMockFunc,
+			},
+			want: want{
+				err:   nil,
+				confs: 2, // Configuration.meta and Configuration.pkg
+				deps:  3, // 1 Configuration.pkg, 1 provider, 1 function
+			},
+		},
+	}
+	for name, tc := range cases {
+		fs := afero.NewMemMapFs()
+		w := &bytes.Buffer{}
+
+		m := NewManager(".crossplane/cache", fs, w)
+		t.Run(name, func(t *testing.T) {
+			m.fetcher = &MockFetcher{tc.args.fetchMock}
+			err := m.PrepExtensions(tc.args.extensions)
+
+			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
+				t.Errorf("\n%s\nPrepExtensions(...): -want error, +got error:\n%s", tc.reason, diff)
+			}
+
+			err = m.addDependencies()
+			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
+				t.Errorf("\n%s\naddDependencies(...): -want error, +got error:\n%s", tc.reason, diff)
+			}
+
+			if diff := cmp.Diff(tc.want.confs, len(m.confs)); diff != "" {
+				t.Errorf("\n%s\naddDependencies(...): -want confs, +got confs:\n%s", tc.reason, diff)
+			}
+			if diff := cmp.Diff(tc.want.deps, len(m.deps)); diff != "" {
+				t.Errorf("\n%s\naddDependencies(...): -want deps, +got deps:\n%s", tc.reason, diff)
+			}
+		})
+	}
+}
+
+type MockFetcher struct {
+	fetch func(image string) (*conregv1.Layer, error)
+}
+
+func (m *MockFetcher) FetchBaseLayer(image string) (*conregv1.Layer, error) {
+	return m.fetch(image)
+}