Fix claim derived-resource lookups by label; fixes #26

cmd/diff/client/crossplane/definition_client.go

@@ -28,6 +28,9 @@ type DefinitionClient interface {
 
 	// GetXRDForXR finds the XRD that defines the given XR type
 	GetXRDForXR(ctx context.Context, gvk schema.GroupVersionKind) (*un.Unstructured, error)
+
+	// IsClaimResource checks if the given resource is a claim type
+	IsClaimResource(ctx context.Context, resource *un.Unstructured) bool
 }
 
 // DefaultDefinitionClient implements DefinitionClient.
@@ -210,3 +213,19 @@ func (c *DefaultDefinitionClient) GetXRDForXR(ctx context.Context, gvk schema.Gr
 
 	return nil, errors.Errorf("no XRD found that defines XR type %s", gvk.String())
 }
+
+// IsClaimResource checks if the given resource is a claim type by attempting
+// to find an XRD that defines it as a claim.
+func (c *DefaultDefinitionClient) IsClaimResource(ctx context.Context, resource *un.Unstructured) bool {
+	gvk := resource.GroupVersionKind()
+
+	// Try to find an XRD that defines this resource type as a claim
+	_, err := c.GetXRDForClaim(ctx, gvk)
+	if err != nil {
+		c.logger.Debug("Resource is not a claim type", "gvk", gvk.String(), "error", err)
+		return false
+	}
+
+	c.logger.Debug("Resource is a claim type", "gvk", gvk.String())
+	return true
+}

cmd/diff/client/crossplane/definition_client_test.go

@@ -626,3 +626,119 @@ func TestDefaultDefinitionClient_Initialize(t *testing.T) {
 		})
 	}
 }
+
+func TestDefaultDefinitionClient_IsClaimResource(t *testing.T) {
+	ctx := t.Context()
+
+	tests := map[string]struct {
+		reason       string
+		mockResource tu.MockResourceClient
+		cachedXRDs   []*un.Unstructured
+		resource     *un.Unstructured
+		expected     bool
+	}{
+		"ResourceIsClaim": {
+			reason: "Should return true when resource is a claim type",
+			mockResource: *tu.NewMockResourceClient().
+				WithSuccessfulInitialize().
+				Build(),
+			cachedXRDs: []*un.Unstructured{
+				// Create a mock XRD that defines this as a claim
+				tu.NewResource("apiextensions.crossplane.io/v1", "CompositeResourceDefinition", "testclaims.example.org").
+					WithSpecField("group", "example.org").
+					WithSpecField("names", map[string]interface{}{
+						"kind":     "XTestResource",
+						"plural":   "xtestresources",
+						"singular": "xtestresource",
+					}).
+					WithSpecField("claimNames", map[string]interface{}{
+						"kind":     "TestClaim",
+						"plural":   "testclaims",
+						"singular": "testclaim",
+					}).
+					Build(),
+			},
+			resource: tu.NewResource("example.org/v1", "TestClaim", "test-claim").
+				InNamespace("default").
+				Build(),
+			expected: true,
+		},
+		"ResourceIsNotClaim": {
+			reason: "Should return false when resource is not a claim type",
+			mockResource: *tu.NewMockResourceClient().
+				WithSuccessfulInitialize().
+				Build(),
+			cachedXRDs: []*un.Unstructured{
+				// Create a mock XRD that defines this as an XR (no claimNames)
+				tu.NewResource("apiextensions.crossplane.io/v1", "CompositeResourceDefinition", "testxrs.example.org").
+					WithSpecField("group", "example.org").
+					WithSpecField("names", map[string]interface{}{
+						"kind":     "TestXR",
+						"plural":   "testxrs",
+						"singular": "testxr",
+					}).
+					// No claimNames field
+					Build(),
+			},
+			resource: tu.NewResource("example.org/v1", "TestXR", "test-xr").
+				InNamespace("default").
+				Build(),
+			expected: false,
+		},
+		"GetXRDForClaimError": {
+			reason: "Should return false when GetXRDForClaim fails",
+			mockResource: *tu.NewMockResourceClient().
+				WithSuccessfulInitialize().
+				WithListResourcesFailure("list error").
+				Build(),
+			cachedXRDs: nil, // Force GetXRDs to fail
+			resource: tu.NewResource("example.org/v1", "TestResource", "test-resource").
+				Build(),
+			expected: false, // Should return false on error
+		},
+		"NoMatchingXRD": {
+			reason: "Should return false when no matching XRD exists",
+			mockResource: *tu.NewMockResourceClient().
+				WithSuccessfulInitialize().
+				Build(),
+			cachedXRDs: []*un.Unstructured{
+				// Create an XRD that doesn't match our resource
+				tu.NewResource("apiextensions.crossplane.io/v1", "CompositeResourceDefinition", "otherclaims.example.org").
+					WithSpecField("group", "example.org").
+					WithSpecField("names", map[string]interface{}{
+						"kind":     "XOtherResource",
+						"plural":   "xotherresources",
+						"singular": "xotherresource",
+					}).
+					WithSpecField("claimNames", map[string]interface{}{
+						"kind":     "OtherClaim",
+						"plural":   "otherclaims",
+						"singular": "otherclaim",
+					}).
+					Build(),
+			},
+			resource: tu.NewResource("example.org/v1", "TestClaim", "test-claim").
+				Build(),
+			expected: false,
+		},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			c := &DefaultDefinitionClient{
+				resourceClient: &tt.mockResource,
+				logger:         tu.TestLogger(t, false),
+				xrds:           tt.cachedXRDs,
+				xrdsLoaded:     tt.cachedXRDs != nil, // Only mark as loaded if we have cached XRDs
+			}
+
+			// Call the function under test
+			result := c.IsClaimResource(ctx, tt.resource)
+
+			// Check the result
+			if result != tt.expected {
+				t.Errorf("\n%s\nIsClaimResource() = %v, want %v", tt.reason, result, tt.expected)
+			}
+		})
+	}
+}

cmd/diff/diff_integration_test.go

@@ -61,31 +61,6 @@ func TestDiffIntegration(t *testing.T) {
 	_ = pkgv1.AddToScheme(scheme)
 	_ = extv1.AddToScheme(scheme)
 
-	// TODO:  is there a reason to even run this against v1 if everything is backwards compatible?
-	// claims are still here (for now).  we obviously need to keep tests for those.
-	// we've already removed the deprecated environmentconfig version.
-	// I can see running the ITs against v2 since we can test against an old image.  important to IT against v1 image
-	// given changes to move xp specific stuff into spec.crossplane, which will not be reflected in running these tests
-
-	// TODO:  add a test to cover v2 CompositeResourceDefinition (XRD) if running against Crossplane v2
-	// TODO:  add a test to cover namespaced xrds against v2
-	// update:  these ITs don't run against a version of xp besides what they are compiled against.  that'll matter
-	// in the e2es.
-	// we'll want to rig up some way to specify xrd-v1 or xrd-v2 or both in the test cases
-	// but the rub is that the cluster directory containing the crds is pulled from either v1 or v2, so we can't just
-	// run both.
-	// thinking we should just grab the cluster directory for v1 and check it in, since it won't advance anymore once
-	// v2 is out.  v2 we can update at build time.  every test spins up its own envtest with the crd path, so we can
-	// definitely toggle there.
-
-	// the CRDs that support the XRDs will vary based on the Crossplane version, though (namespaced vs cluster scoped),
-	// so we need to bifurcate /testdata/diff/crds accordingly.  although each XRD that we define will have a version
-	// specified inside it which will lead to the generation of that crd.  so maybe it's test specific actually.
-
-	// TODO:  namespaced XRDs cannot compose cluster-scoped resources, so we need to ensure XDownstreamResource definitions
-	// account for that.  maybe we just need to add parallel CRDs for namespace scoped and cluster scoped XRDs that can
-	// coexist.
-
 	// Test cases
 	tests := map[string]struct {
 		setupFiles              []string
@@ -1042,16 +1017,20 @@ Summary: 2 added`,
 +   coolField: modified-value
 
 ---
-~~~ XDownstreamResource/test-claim
+~~~ XDownstreamResource/test-claim-82crv
   apiVersion: nop.example.org/v1alpha1
   kind: XDownstreamResource
   metadata:
     annotations:
       crossplane.io/composition-resource-name: nop-resource
-    generateName: test-claim-
+-   generateName: test-claim-82crv-
     labels:
-      crossplane.io/composite: test-claim
-    name: test-claim
+      crossplane.io/claim-name: test-claim
+      crossplane.io/claim-namespace: existing-namespace
+-     crossplane.io/composite: test-claim-82crv
+-   name: test-claim-82crv
++     crossplane.io/composite: test-claim
++   name: test-claim
   spec:
     forProvider:
 -     configData: existing-value

cmd/diff/diffprocessor/diff_calculator_test.go

@@ -83,7 +83,7 @@ func TestDefaultDiffCalculator_CalculateDiff(t *testing.T) {
 					Build()
 
 				// Create resource manager
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},
@@ -113,7 +113,7 @@ func TestDefaultDiffCalculator_CalculateDiff(t *testing.T) {
 					Build()
 
 				// Create resource manager
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},
@@ -144,7 +144,7 @@ func TestDefaultDiffCalculator_CalculateDiff(t *testing.T) {
 					Build()
 
 				// Create resource manager
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},
@@ -182,7 +182,7 @@ func TestDefaultDiffCalculator_CalculateDiff(t *testing.T) {
 					Build()
 
 				// Create resource manager
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},
@@ -212,7 +212,7 @@ func TestDefaultDiffCalculator_CalculateDiff(t *testing.T) {
 					Build()
 
 				// Create resource manager
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},
@@ -238,7 +238,7 @@ func TestDefaultDiffCalculator_CalculateDiff(t *testing.T) {
 					Build()
 
 				// Create resource manager
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},
@@ -309,7 +309,7 @@ func TestDefaultDiffCalculator_CalculateDiff(t *testing.T) {
 					Build()
 
 				// Create resource manager
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},
@@ -455,7 +455,7 @@ func TestDefaultDiffCalculator_CalculateDiffs(t *testing.T) {
 					Build()
 
 				// Create resource manager
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},
@@ -491,7 +491,7 @@ func TestDefaultDiffCalculator_CalculateDiffs(t *testing.T) {
 					Build()
 
 				// Create resource manager
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},
@@ -529,7 +529,7 @@ func TestDefaultDiffCalculator_CalculateDiffs(t *testing.T) {
 					Build()
 
 				// Create resource manager
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},
@@ -572,7 +572,7 @@ func TestDefaultDiffCalculator_CalculateDiffs(t *testing.T) {
 					Build()
 
 				// Create resource manager
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},
@@ -629,7 +629,7 @@ func TestDefaultDiffCalculator_CalculateDiffs(t *testing.T) {
 					Build()
 
 				// Create resource manager
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},
@@ -762,7 +762,7 @@ func TestDefaultDiffCalculator_CalculateRemovedResourceDiffs(t *testing.T) {
 
 				// Create a resource manager (not directly used in this test)
 				resourceClient := tu.NewMockResourceClient().Build()
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},
@@ -791,7 +791,7 @@ func TestDefaultDiffCalculator_CalculateRemovedResourceDiffs(t *testing.T) {
 
 				// Create a resource manager (not directly used in this test)
 				resourceClient := tu.NewMockResourceClient().Build()
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},
@@ -817,7 +817,7 @@ func TestDefaultDiffCalculator_CalculateRemovedResourceDiffs(t *testing.T) {
 
 				// Create a resource manager (not directly used in this test)
 				resourceClient := tu.NewMockResourceClient().Build()
-				resourceManager := NewResourceManager(resourceClient, tu.TestLogger(t, false))
+				resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false))
 
 				return applyClient, resourceTreeClient, resourceManager
 			},

cmd/diff/diffprocessor/diff_processor.go

@@ -41,6 +41,7 @@ type DiffProcessor interface {
 type DefaultDiffProcessor struct {
 	fnClient             xp.FunctionClient
 	compClient           xp.CompositionClient
+	defClient            xp.DefinitionClient
 	config               ProcessorConfig
 	schemaValidator      SchemaValidator
 	diffCalculator       DiffCalculator
@@ -71,7 +72,7 @@ func NewDiffProcessor(k8cs k8.Clients, xpcs xp.Clients, opts ...ProcessorOption)
 	diffOpts := config.GetDiffOptions()
 
 	// Create components using factories
-	resourceManager := config.Factories.ResourceManager(k8cs.Resource, config.Logger)
+	resourceManager := config.Factories.ResourceManager(k8cs.Resource, xpcs.Definition, config.Logger)
 	schemaValidator := config.Factories.SchemaValidator(k8cs.Schema, xpcs.Definition, config.Logger)
 	requirementsProvider := config.Factories.RequirementsProvider(k8cs.Resource, xpcs.Environment, config.RenderFunc, config.Logger)
 	diffCalculator := config.Factories.DiffCalculator(k8cs.Apply, xpcs.ResourceTree, resourceManager, config.Logger, diffOpts)
@@ -80,6 +81,7 @@ func NewDiffProcessor(k8cs k8.Clients, xpcs xp.Clients, opts ...ProcessorOption)
 	processor := &DefaultDiffProcessor{
 		fnClient:             xpcs.Function,
 		compClient:           xpcs.Composition,
+		defClient:            xpcs.Definition,
 		config:               config,
 		schemaValidator:      schemaValidator,
 		diffCalculator:       diffCalculator,
@@ -222,7 +224,7 @@ func (p *DefaultDiffProcessor) DiffSingleResource(ctx context.Context, res *un.U
 	// WORKAROUND for https://github.com/crossplane/crossplane/issues/6782
 	// Propagate namespace from XR to namespaced managed resources
 	// For claims (v1 compatibility), skip namespace propagation entirely as all managed resources are cluster-scoped
-	isClaimRoot := p.schemaValidator.IsClaimResource(ctx, xrUnstructured)
+	isClaimRoot := p.defClient.IsClaimResource(ctx, xrUnstructured)
 	if !isClaimRoot {
 		p.propagateNamespacesToManagedResources(ctx, xrUnstructured, desired.ComposedResources)
 	} else {

cmd/diff/diffprocessor/processor_config.go

@@ -32,7 +32,7 @@ type ProcessorConfig struct {
 // ComponentFactories contains factory functions for creating processor components.
 type ComponentFactories struct {
 	// ResourceManager creates a ResourceManager
-	ResourceManager func(client k8.ResourceClient, logger logging.Logger) ResourceManager
+	ResourceManager func(client k8.ResourceClient, defClient xp.DefinitionClient, logger logging.Logger) ResourceManager
 
 	// SchemaValidator creates a SchemaValidator
 	SchemaValidator func(schema k8.SchemaClient, def xp.DefinitionClient, logger logging.Logger) SchemaValidator
@@ -86,7 +86,7 @@ func WithRenderFunc(renderFn RenderFunc) ProcessorOption {
 }
 
 // WithResourceManagerFactory sets the ResourceManager factory function.
-func WithResourceManagerFactory(factory func(k8.ResourceClient, logging.Logger) ResourceManager) ProcessorOption {
+func WithResourceManagerFactory(factory func(k8.ResourceClient, xp.DefinitionClient, logging.Logger) ResourceManager) ProcessorOption {
 	return func(config *ProcessorConfig) {
 		config.Factories.ResourceManager = factory
 	}