Skip to content

Commit da3d157

Browse files
jcogilviejcogilvie
authored
Add support for nested xrs (#77)
* Add failing tests for nested xrs Signed-off-by: Jonathan Ogilvie <[email protected]> # Conflicts: # cmd/diff/diff_integration_test.go * feat: add support for nested XRs Signed-off-by: Jonathan Ogilvie <[email protected]> * Add e2e for nested; add cmd flag Signed-off-by: Jonathan Ogilvie <[email protected]> * Try to fix e2e teardown Signed-off-by: Jonathan Ogilvie <[email protected]> * Fix new e2e Signed-off-by: Jonathan Ogilvie <[email protected]> * Fix lint Signed-off-by: Jonathan Ogilvie <[email protected]> * Cleanup Signed-off-by: Jonathan Ogilvie <[email protected]> --------- Signed-off-by: Jonathan Ogilvie <[email protected]>
1 parent 05d9f78 commit da3d157

35 files changed

+1702
-119
lines changed

‎CLAUDE.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@
22
- The diff command should strive for accuracy above all else, so in cases where errors or guesses may compromise accuracy, we should fail the diff completely.
33
- We should not emit partial results for a given XR; if given multiple XRs, it's okay to emit results only for those that pass so long as we call attention to the others that failed.
44
- We should always emit useful logging with appropriate contextual objects attached.
5-
- the earthly `reviewable` command should always be run with a long timeout. it runs all of our ITs and they can take several minutes.
5+
- the earthly `reviewable` command should always be run with a long timeout. it runs all of our ITs and they can take several minutes.
6+
- in the e2es, every test composition needs to end with function-auto-ready in order for the setup and teardown to work correctly. this is what causes status conditions to be bubbled up from child resources.

‎cmd/diff/client/kubernetes/schema_client_test.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,7 @@ func TestSchemaClient_LoadCRDsFromXRDs(t *testing.T) {
335335
xrd := tu.NewXRD(testXResourcePlural+".example.org", testExampleOrgGroup, testXResourceKind).
336336
WithPlural(testXResourcePlural).
337337
WithSingular("xresource").
338+
WithDefaultVersion().
338339
WithRawSchema([]byte(`{
339340
"type": "object",
340341
"properties": {
@@ -358,6 +359,7 @@ func TestSchemaClient_LoadCRDsFromXRDs(t *testing.T) {
358359
tu.NewCRD(testXResourcePlural+".example.org", testExampleOrgGroup, testXResourceKind).
359360
WithPlural(testXResourcePlural).
360361
WithSingular("xresource").
362+
WithDefaultVersion().
361363
Build())
362364
correspondingCRD := &un.Unstructured{Object: correspondingCRDObj}
363365

@@ -508,6 +510,7 @@ func TestSchemaClient_GetCRDByName(t *testing.T) {
508510
testCRD := tu.NewCRD(testCRDName, testExampleOrgGroup, testXResourceKind).
509511
WithPlural(testXResourcePlural).
510512
WithSingular("xresource").
513+
WithDefaultVersion().
511514
Build()
512515

513516
tests := map[string]struct {
@@ -534,7 +537,7 @@ func TestSchemaClient_GetCRDByName(t *testing.T) {
534537
"DifferentCRDInCache": {
535538
reason: "Should return error when searching for CRD that doesn't exist",
536539
setupCRDs: []*extv1.CustomResourceDefinition{
537-
tu.NewCRD("other."+testExampleOrgGroup, testExampleOrgGroup, "OtherKind").Build(),
540+
tu.NewCRD("other."+testExampleOrgGroup, testExampleOrgGroup, "OtherKind").WithDefaultVersion().Build(),
538541
},
539542
searchName: testCRDName,
540543
expectError: true,
@@ -596,8 +599,8 @@ func TestSchemaClient_GetCRDByName(t *testing.T) {
596599

597600
func TestSchemaClient_GetAllCRDs(t *testing.T) {
598601
// Create test CRDs
599-
crd1 := tu.NewCRD("crd1."+testExampleOrgGroup, testExampleOrgGroup, "TestKind1").Build()
600-
crd2 := tu.NewCRD("crd2."+testExampleOrgGroup, testExampleOrgGroup, "TestKind2").Build()
602+
crd1 := tu.NewCRD("crd1."+testExampleOrgGroup, testExampleOrgGroup, "TestKind1").WithDefaultVersion().Build()
603+
crd2 := tu.NewCRD("crd2."+testExampleOrgGroup, testExampleOrgGroup, "TestKind2").WithDefaultVersion().Build()
601604

602605
tests := map[string]struct {
603606
reason string

‎cmd/diff/cmd_utils.go‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,6 @@ import (
3636
//nolint:gochecknoglobals // Required for global serialization across processors
3737
var globalRenderMutex sync.Mutex
3838

39-
// CommonCmdFields contains common fields shared by both XR and Comp commands.
40-
type CommonCmdFields struct {
41-
// Configuration options
42-
NoColor bool `help:"Disable colorized output." name:"no-color"`
43-
Compact bool `help:"Show compact diffs with minimal context." name:"compact"`
44-
Timeout time.Duration `default:"1m" help:"How long to run before timing out."`
45-
QPS float32 `default:"0" help:"Maximum QPS to the API server."`
46-
Burst int `default:"0" help:"Maximum burst for throttle."`
47-
}
48-
4939
// initializeSharedDependencies handles the common initialization logic for both commands.
5040
func initializeSharedDependencies(ctx *kong.Context, log logging.Logger, config *rest.Config, fields CommonCmdFields) (*AppContext, error) {
5141
config = initRestConfig(config, log, fields)
@@ -102,10 +92,12 @@ func initializeAppContext(timeout time.Duration, appCtx *AppContext, log logging
10292
}
10393

10494
// defaultProcessorOptions returns the standard default options used by both XR and composition processors.
105-
// Call sites can append additional options or override these defaults as needed.
106-
func defaultProcessorOptions() []dp.ProcessorOption {
95+
// This is the single source of truth for behavior defaults in the CLI layer.
96+
func defaultProcessorOptions(fields CommonCmdFields, namespace string) []dp.ProcessorOption {
10797
return []dp.ProcessorOption{
108-
dp.WithColorize(true),
109-
dp.WithCompact(false),
98+
dp.WithNamespace(namespace),
99+
dp.WithColorize(!fields.NoColor),
100+
dp.WithCompact(fields.Compact),
101+
dp.WithMaxNestedDepth(fields.MaxNestedDepth),
110102
}
111103
}

‎cmd/diff/comp.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,16 @@ func (c *CompCmd) initializeDependencies(ctx *kong.Context, log logging.Logger,
8888
}
8989

9090
func makeDefaultCompProc(c *CompCmd, ctx *AppContext, log logging.Logger) dp.CompDiffProcessor {
91+
// Use provided namespace or default to "default"
92+
namespace := c.Namespace
93+
if namespace == "" {
94+
namespace = "default"
95+
}
96+
9197
// Both processors share the same options since they're part of the same command
92-
opts := defaultProcessorOptions()
98+
opts := defaultProcessorOptions(c.CommonCmdFields, namespace)
9399
opts = append(opts,
94-
dp.WithNamespace(c.Namespace),
95100
dp.WithLogger(log),
96-
dp.WithColorize(!c.NoColor), // Override default if NoColor is set
97-
dp.WithCompact(c.Compact), // Override default if Compact is set
98101
dp.WithRenderMutex(&globalRenderMutex),
99102
)
100103

‎cmd/diff/diff_integration_test.go‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,6 +1306,132 @@ Summary: 1 added`,
13061306
expectedError: false,
13071307
noColor: true,
13081308
},
1309+
"New nested XR creates child XR and downstream resources": {
1310+
setupFiles: []string{
1311+
// XRDs for parent and child
1312+
"testdata/diff/resources/nested/parent-xrd.yaml",
1313+
"testdata/diff/resources/nested/child-xrd.yaml",
1314+
// Compositions for parent and child
1315+
"testdata/diff/resources/nested/parent-composition.yaml",
1316+
"testdata/diff/resources/nested/child-composition.yaml",
1317+
// XRD for downstream managed resource
1318+
"testdata/diff/resources/xdownstreamenvresource-xrd.yaml",
1319+
"testdata/diff/resources/functions.yaml",
1320+
},
1321+
inputFiles: []string{"testdata/diff/new-nested-xr.yaml"},
1322+
expectedOutput: `
1323+
+++ XChildResource/test-parent-child
1324+
+ apiVersion: ns.nested.example.org/v1alpha1
1325+
+ kind: XChildResource
1326+
+ metadata:
1327+
+ annotations:
1328+
+ crossplane.io/composition-resource-name: child-xr
1329+
+ labels:
1330+
+ crossplane.io/composite: test-parent
1331+
+ name: test-parent-child
1332+
+ namespace: default
1333+
+ spec:
1334+
+ childField: parent-value
1335+
1336+
---
1337+
+++ XDownstreamResource/test-parent-child-managed
1338+
+ apiVersion: ns.nop.example.org/v1alpha1
1339+
+ kind: XDownstreamResource
1340+
+ metadata:
1341+
+ annotations:
1342+
+ crossplane.io/composition-resource-name: managed-resource
1343+
+ labels:
1344+
+ crossplane.io/composite: test-parent-child
1345+
+ name: test-parent-child-managed
1346+
+ namespace: default
1347+
+ spec:
1348+
+ forProvider:
1349+
+ configData: parent-value
1350+
1351+
---
1352+
+++ XParentResource/test-parent
1353+
+ apiVersion: ns.nested.example.org/v1alpha1
1354+
+ kind: XParentResource
1355+
+ metadata:
1356+
+ name: test-parent
1357+
+ namespace: default
1358+
+ spec:
1359+
+ parentField: parent-value
1360+
1361+
---
1362+
1363+
Summary: 3 added`,
1364+
expectedError: false,
1365+
noColor: true,
1366+
},
1367+
"Modified nested XR propagates changes through child XR to downstream resources": {
1368+
setupFiles: []string{
1369+
// XRDs for parent and child
1370+
"testdata/diff/resources/nested/parent-xrd.yaml",
1371+
"testdata/diff/resources/nested/child-xrd.yaml",
1372+
// Compositions for parent and child
1373+
"testdata/diff/resources/nested/parent-composition.yaml",
1374+
"testdata/diff/resources/nested/child-composition.yaml",
1375+
// XRD for downstream managed resource
1376+
"testdata/diff/resources/xdownstreamenvresource-xrd.yaml",
1377+
"testdata/diff/resources/functions.yaml",
1378+
// Existing resources
1379+
"testdata/diff/resources/nested/existing-parent-xr.yaml",
1380+
"testdata/diff/resources/nested/existing-child-xr.yaml",
1381+
"testdata/diff/resources/nested/existing-managed-resource.yaml",
1382+
},
1383+
inputFiles: []string{"testdata/diff/modified-nested-xr.yaml"},
1384+
expectedOutput: `
1385+
~~~ XChildResource/test-parent-child
1386+
apiVersion: ns.nested.example.org/v1alpha1
1387+
kind: XChildResource
1388+
metadata:
1389+
+ annotations:
1390+
+ crossplane.io/composition-resource-name: child-xr
1391+
+ labels:
1392+
+ crossplane.io/composite: test-parent
1393+
name: test-parent-child
1394+
namespace: default
1395+
spec:
1396+
- childField: existing-value
1397+
+ childField: modified-value
1398+
1399+
---
1400+
~~~ XDownstreamResource/test-parent-child-managed
1401+
apiVersion: ns.nop.example.org/v1alpha1
1402+
kind: XDownstreamResource
1403+
metadata:
1404+
annotations:
1405+
- gotemplating.fn.crossplane.io/composition-resource-name: managed-resource
1406+
+ crossplane.io/composition-resource-name: managed-resource
1407+
+ gotemplating.fn.crossplane.io/composition-resource-name: managed-resource
1408+
+ labels:
1409+
+ crossplane.io/composite: test-parent-child
1410+
name: test-parent-child-managed
1411+
namespace: default
1412+
spec:
1413+
forProvider:
1414+
- configData: existing-value
1415+
+ configData: modified-value
1416+
1417+
1418+
---
1419+
~~~ XParentResource/test-parent
1420+
apiVersion: ns.nested.example.org/v1alpha1
1421+
kind: XParentResource
1422+
metadata:
1423+
name: test-parent
1424+
namespace: default
1425+
spec:
1426+
- parentField: existing-value
1427+
+ parentField: modified-value
1428+
1429+
---
1430+
1431+
Summary: 3 modified`,
1432+
expectedError: false,
1433+
noColor: true,
1434+
},
13091435
}
13101436

13111437
runIntegrationTest(t, XRDiffTest, tests)

0 commit comments

Comments
 (0)