Skip to content

Conversation

@fabioaraujopt
Copy link

@fabioaraujopt fabioaraujopt commented Jan 10, 2026

Summary

This PR fixes two related bugs that prevented the tool from working correctly with non-default Kubernetes contexts and Crossplane v2 XRDs.

Bug 1: --context flag not being respected

Problem

The --context CLI flag was being ignored. The tool always used the default kubeconfig context regardless of what was specified on the command line.

Root Cause

Kong's dependency injection was calling getRestConfig() during provider initialization, which happened before the CLI flags were parsed. This meant CommonCmdFields.Context was always empty when getRestConfig() was called.

Fix

  • Capture the --context flag early in the BeforeApply() hook using a package-level variable
  • Update getRestConfig() to reference this captured value
  • Ensure xr.go and comp.go use GetRestConfig() method to get the correctly-configured client

Bug 2: Crossplane v2 composition selection failing

Problem

For Crossplane v2 XRDs, the tool failed to find compositionSelector and compositionRef fields, resulting in:

  • "ambiguous composition selection" errors when multiple compositions exist
  • Incorrect composition matching

Root Cause

The tool only looked for these fields at spec.crossplane.compositionSelector (v2 path), but some XRs have these fields directly at spec.compositionSelector (v1-style path, which is also valid for v2).

Fix

Modified makeCrossplaneRefPath() to return multiple possible paths, and updated all functions that use these paths to try both locations:

  • findByDirectReference()
  • findByLabelSelector()
  • getCompositionRevisionRef()
  • getCompositionUpdatePolicy()
  • resourceUsesComposition()

Files Changed

  • cmd/diff/main.go: Capture --context flag in BeforeApply()
  • cmd/diff/xr.go: Use GetRestConfig() method
  • cmd/diff/comp.go: Use GetRestConfig() method
  • cmd/diff/client/crossplane/composition_client.go: Support both v1 and v2 paths

Testing

Tested locally with:

  • Crossplane v2 XRDs in a vcluster
  • --context flag pointing to a specific vcluster context
  • XRs using compositionSelector with label matching

Before fix: Tool found only 10 XRDs (from wrong context) and failed composition selection
After fix: Tool found 44 XRDs (22 v1 + 22 v2) and correctly matched compositions

I have:

  • Read and followed Crossplane's contribution process.
  • Run earthly +reviewable to ensure this PR is ready for review.
  • Added or updated unit tests.
  • Added or updated e2e tests.
  • Documented this change as needed.
    - [ ] Followed the API promotion workflow if this PR introduces, removes, or promotes an API.

Need help with this checklist? See the cheat sheet.

This commit fixes two related bugs:

1. --context flag not being respected
   The --context CLI flag was being ignored because Kong's dependency
   injection called getRestConfig() before the CLI flags were parsed.
   Fixed by capturing the context in BeforeApply() hook and using a
   package-level variable that getRestConfig() can reference.

2. Crossplane v2 compositionRef/compositionSelector path resolution
   For Crossplane v2 XRDs, the tool only looked for compositionSelector
   and compositionRef at spec.crossplane.X paths. However, some XRs have
   these fields directly at spec.X (without the crossplane sub-field).
   Fixed by trying both path variants when looking up composition
   references, selectors, revision refs, and update policies.

Files changed:
- cmd/diff/main.go: Capture --context flag early in BeforeApply()
- cmd/diff/xr.go: Use GetRestConfig() method for correct context
- cmd/diff/comp.go: Use GetRestConfig() method for correct context
- cmd/diff/client/crossplane/composition_client.go: Try both v1 and v2
  paths for compositionRef, compositionSelector, compositionRevisionRef,
  and compositionUpdatePolicy
@jcogilvie
Copy link
Collaborator

jcogilvie commented Jan 14, 2026

@fabioaraujopt I appreciate the contribution! Please complete the contribution checklist from the PR template (which I have added to your PR body). Once tests pass and coverage is adequate, this looks good and I'd be happy to merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants