Skip to content

refactor(xpkg): fold Resolver into a resolving xpkg Client - #483

Merged
jcogilvie merged 2 commits into
crossplane:mainfrom
jcogilvie:jco-xpkg-client-resolve
Oct 31, 2026
Merged

jcogilvie merged 2 commits into
crossplane:mainfrom
jcogilvie:jco-xpkg-client-resolve

Conversation

@jcogilvie

@jcogilvie jcogilvie commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Description of your changes

Follow-up to #478, implementing @adamwg's review suggestion there: rather than passing both an xpkg client and a Resolver everywhere, internal/xpkg now has a Client interface that extends the upstream xpkg.Client with Resolve.

  • internal/xpkg.Client = upstream xpkg.Client + Resolve(ctx, ref) (name.Reference, string, error).
  • WrapClient(xpkg.Client) Client adds resolution to any upstream client; NewClient and project.NewXpkgClient return the wrapped client.
  • Resolver/NewResolver are removed; dependency.WithResolver is removed and the manager resolves through the client passed via WithXpkgClient.
  • ResolveConfigurationFunctions takes the Client.
  • Resolution rules, caching, ImageConfig handling and error messages are unchanged; the resolve tests are unchanged apart from the constructor.

Testing: go build ./..., go vet ./internal/... ./cmd/..., go test ./internal/... ./cmd/..., and golangci-lint run on the touched packages (0 issues); ./nix.sh run .#test and ./nix.sh run .#lint (0 issues).

Related: #436, #477, #480, #482.

I have:

  • Read and followed Crossplane's contribution process.
  • Run ./nix.sh flake check to ensure this PR is ready for review. (./nix.sh run .#test and ./nix.sh run .#lint pass.)
  • Added or updated unit tests.
  • Linked a PR or a docs tracking issue to document this change. Internal refactor, no user-facing change.
  • Added backport release-x.y labels to auto-backport this PR. Refactor only, no backport needed.

Need help with this checklist? See the cheat sheet.

🤖 Generated with Claude Code

Add an internal/xpkg Client interface that extends the upstream
xpkg.Client with Resolve. NewClient and project.NewXpkgClient now
return it, so callers carry one value instead of a client plus a
Resolver. The dependency manager drops WithResolver and resolves
through its client; ResolveConfigurationFunctions takes the Client.
Resolution rules and error messages are unchanged.

Suggested by adamwg in review of crossplane#478.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
@jcogilvie
jcogilvie marked this pull request as ready for review October 30, 2026 19:48
@jcogilvie
jcogilvie requested review from a team and tampakrap as code owners October 30, 2026 19:48
@jcogilvie
jcogilvie requested review from phisco and removed request for a team October 30, 2026 19:48
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between ba029f2 and 8a3897c.

Files selected for processing: 16
  • cmd/crossplane/composition/generate.go
  • cmd/crossplane/dependency/add.go
  • cmd/crossplane/dependency/cache.go
  • cmd/crossplane/project/build.go
  • cmd/crossplane/project/run.go
  • cmd/crossplane/render/op/cmd.go
  • cmd/crossplane/render/xr/cmd.go
  • cmd/crossplane/simulate/simulate.go
  • internal/dependency/manager.go
  • internal/dependency/manager_test.go
  • internal/project/xpkg.go
  • internal/xpkg/client.go
  • internal/xpkg/configuration.go
  • internal/xpkg/configuration_test.go
  • internal/xpkg/resolve.go
  • internal/xpkg/resolve_test.go
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@adamwg adamwg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One further simplification suggestion inline, but I'll leave it up to you. Thanks for the refactor 🙏

Comment thread internal/project/xpkg.go Resolved
…Client

NewClient always uses the remote fetcher, so callers no longer pass one.
Callers of project.NewXpkgClient now call xpkg.NewClient directly with the
same cache dir default and image configs.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
@jcogilvie
jcogilvie merged commit beea6dc into crossplane:main Oct 31, 2026
10 checks passed
@jcogilvie
jcogilvie deleted the jco-xpkg-client-resolve branch October 31, 2026 20:59
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