bump crossplane-runtime to v2.3.1#664
Open
jschoombee wants to merge 4 commits into
Open
Conversation
….3.x
crossplane-runtime v2.3.0 moved the common APIs (formerly
crossplane-runtime/v2/apis/common/{v1,v2} and the unversioned apis/common
mock surface) into a single consolidated package at
github.com/crossplane/crossplane/apis/v2/core/v2, and renamed the
inline-embedded spec/status types:
- v1.ResourceSpec → v2.ClusterManagedResourceSpec (cluster-scoped)
→ v2.ManagedResourceSpec (namespaced; existing)
- v1.ResourceStatus → v2.ManagedResourceStatus
- v1/v2 SecretReference, SecretKeySelector, Reference, Selector, Condition,
ManagementAction*, ManagementPolicies, DeletionPolicy, etc. — same names,
new package path.
Migration in this commit:
- Bulk-swap every Go import of the three removed paths to the consolidated
path across pkg/, tests/, and the GoMock fake (apis/common at root).
- Refresh the embedded type-string fixtures in pkg/types/builder_test.go
to match the new fully-qualified path.
- Update pkg/types/reference.go PackagePath* constants so generated CRD
types reference the new package (both XPCommonAPIs and XPV2CommonAPIs
now point at the same consolidated package, kept as separate constants
for source-level backward compatibility).
- Update pkg/pipeline/templates/crd_types.go.tmpl to emit
ClusterManagedResourceSpec for cluster-scoped MRs and
ManagedResourceStatus for the embedded status — both required by the
consolidated v2 API.
- Hand-edit tests/conversion/test_resource.go ResourceSpec/Status
references to use the new cluster-scoped type names.
- go.mod: bump crossplane-runtime/v2 v2.2.0 → v2.3.1, add direct require
on crossplane/apis/v2 v2.3.1; go mod tidy.
Unblocks providers (e.g. provider-wiz) that consume upjet and want to
adopt crossplane-runtime v2.3.x.
Signed-off-by: JP Schoombee <jpschoombee@gmail.com>
jschoombee
requested review from
erhancagirici,
sergenyalcin and
ulucinar
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (17)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (14)
📝 WalkthroughWalkthroughThis PR migrates Crossplane common API imports from crossplane-runtime v2 common packages to github.com/crossplane/crossplane/apis/v2/core/v2, updates codegen package-path constants and generated-type test expectations, adjusts embedded resource spec/status types in tests, adds an upgrade docs section, and bumps CI Go version. ChangesCommon API Package Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Thank you — happy to expand any checklist or run a quick search to confirm no remaining old import paths. 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/pipeline/hooks_test.go (1)
167-178:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMigration looks half-applied in this fixture — import moved, but the embedded type names didn't. 🧐
The import on Line 167 now points to
github.com/crossplane/crossplane/apis/v2/core/v2, but Lines 171 and 176 still embedv1.ResourceSpec/v1.ResourceStatus, which were renamed toClusterManagedResourceSpec/ManagedResourceStatusat the consolidated path. SincebaseContentis only a string fixture for marker manipulation (not compiled), the tests still pass — so no functional breakage here. That said, given thecrd_types.go.tmpltemplate now emits the renamed types, updating the fixture keeps it faithful to real generated output and avoids confusing the next maintainer. Was leaving the old names here intentional?📝 Suggested fixture alignment
type MemberSpec struct { - v1.ResourceSpec ` + "`json:\",inline\"`" + ` + v1.ClusterManagedResourceSpec ` + "`json:\",inline\"`" + ` ForProvider MemberParameters ` + "`json:\"forProvider\"`" + ` } type MemberStatus struct { - v1.ResourceStatus ` + "`json:\",inline\"`" + ` + v1.ManagedResourceStatus ` + "`json:\",inline\"`" + ` AtProvider MemberObservation ` + "`json:\"atProvider,omitempty\"`" + ` }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/pipeline/hooks_test.go` around lines 167 - 178, The fixture import was updated to github.com/crossplane/crossplane/apis/v2/core/v2 but the embedded types in MemberSpec and MemberStatus still reference v1.ResourceSpec and v1.ResourceStatus; update those embeddings to the consolidated names ClusterManagedResourceSpec and ManagedResourceStatus respectively (e.g., replace v1.ResourceSpec with ClusterManagedResourceSpec and v1.ResourceStatus with ManagedResourceStatus in the MemberSpec and MemberStatus structs) so the test fixture matches the output produced by crd_types.go.tmpl.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/types/reference.go`:
- Around line 22-29: The doc comments above the constants
PackagePathXPCommonAPIs and PackagePathXPV2CommonAPIs are stale/misleading (they
say “Crossplane Runtime package” vs actual target) — update both comments to
describe that they point to the Crossplane core APIs package (e.g., “Crossplane
core APIs”) and optionally note that types like Reference, Selector,
SecretKeySelector, SecretReference, LocalSecretReference,
LocalSecretKeySelector, NamespacedReference, and NamespacedSelector are exported
from github.com/crossplane/crossplane/apis/v2/core/v2 so the consolidation is
safe; change only the comment text near PackagePathXPCommonAPIs and
PackagePathXPV2CommonAPIs without modifying the constant values or other code.
---
Outside diff comments:
In `@pkg/pipeline/hooks_test.go`:
- Around line 167-178: The fixture import was updated to
github.com/crossplane/crossplane/apis/v2/core/v2 but the embedded types in
MemberSpec and MemberStatus still reference v1.ResourceSpec and
v1.ResourceStatus; update those embeddings to the consolidated names
ClusterManagedResourceSpec and ManagedResourceStatus respectively (e.g., replace
v1.ResourceSpec with ClusterManagedResourceSpec and v1.ResourceStatus with
ManagedResourceStatus in the MemberSpec and MemberStatus structs) so the test
fixture matches the output produced by crd_types.go.tmpl.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a1dd9105-d8ff-4e29-a6ae-70adbb07c1d0
⛔ Files ignored due to path filters (4)
go.modis excluded by none and included by nonego.sumis excluded by!**/*.sumand included by nonepkg/pipeline/templates/crd_types.go.tmplis excluded by none and included by nonepkg/resource/fake/mocks/mock.gois excluded by!**/fake/**and included by**/*.go
📒 Files selected for processing (17)
pkg/config/resource.gopkg/controller/api.gopkg/controller/external.gopkg/controller/external_async_tfpluginfw.gopkg/controller/external_async_tfpluginsdk.gopkg/controller/external_test.gopkg/controller/external_tfpluginfw.gopkg/controller/external_tfpluginsdk.gopkg/pipeline/hooks_test.gopkg/resource/conditions.gopkg/resource/sensitive.gopkg/resource/sensitive_test.gopkg/terraform/files.gopkg/terraform/files_test.gopkg/types/builder_test.gopkg/types/reference.gotests/conversion/test_resource.go
go.mod's go directive is now 1.25.10 (auto-bumped by `go mod tidy` after the crossplane-runtime v2.3.1 upgrade — runtime v2.3.1 itself requires go 1.25.9, and downstream Kubernetes deps push it to 1.25.10). The CI workflow's GO_VERSION env was still pinned to 1.25.8, so `make vendor` fails with `go.mod requires go >= 1.25.10 (running go 1.25.8; GOTOOLCHAIN=local)`. Match the workflow pin to the module's floor. Signed-off-by: JP Schoombee <jpschoombee@gmail.com>
- Run gci with the project's section config (standard / default / prefix(github.com/crossplane/upjet) / blank / dot, custom-order) over pkg/ and tests/ — bulk import rewrites had landed in the wrong gci section. All 14 golangci-lint(gci) findings are now resolved. - pkg/resource/fake/mocks/mock.go regenerated via `go generate ./...` so it matches what `make check-diff` produces in CI. Import alias is now `v2` (mockgen-derived) instead of the hand-edited `common`. - pkg/types/reference.go: refresh stale doc comments on the two PackagePath constants — they used to point at crossplane-runtime, so the surrounding "Crossplane Runtime package" wording was misleading after the v2.3.0 consolidation. Per CodeRabbit feedback on the PR. - pkg/pipeline/hooks_test.go: the embedded golden fixture (lines 167-178) was half-migrated — import moved but `v1.ResourceSpec` and `v1.ResourceStatus` were left over. Rename to ClusterManagedResourceSpec and ManagedResourceStatus to match the updated crd_types.go.tmpl. Signed-off-by: JP Schoombee <jpschoombee@gmail.com>
crossplane-runtime v2.3.0 consolidates apis/common/{v1,v2} into
github.com/crossplane/crossplane/apis/v2/core/v2 and renames the
inline-embedded spec/status types. The existing v1→v2 migration guide
referenced the now-removed paths and would lead anyone following it
straight into the same `missing go.sum entry` wall.
Add a callout immediately after the v1→v2 import-rewrite step that:
- lists the three retired packages and the consolidated replacement;
- calls out the two type embed renames (ClusterManagedResourceSpec and
ManagedResourceStatus) — and notes that all other common symbols kept
their names;
- gives a two-step go.mod-bump + make-generate recipe for providers
already on Upjet v2;
- mentions sed as a fallback when committed zz_*.go files can't be
regenerated immediately.
Signed-off-by: JP Schoombee <jpschoombee@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of your changes
This PR adopts crossplane-runtime v2.3.x in upjet/v2.
The v2.3.0 release relocated the common APIs out of
crossplane-runtimeinto the consolidatedgithub.com/crossplane/crossplane/apis/v2/core/v2package and renamed the inline-embedded spec/status types. Today upjet still imports the removed paths, so any downstream provider that wants to pick up v2.3.x is blocked.API compatibility
The two renamed embeds are structurally identical to the originals — same fields, same JSON tags, same kubebuilder defaults:
ResourceSpec(cluster-scoped MRs)ClusterManagedResourceSpecResourceStatusManagedResourceStatusAll other common symbols (
Reference,Selector,SecretReference,SecretKeySelector,LocalSecretReference,LocalSecretKeySelector,NamespacedReference,NamespacedSelector,Condition*,ManagementAction*,ManagementPolicies,DeletionPolicy, …) keep their names — only the import path changes. So providers that regenerate after this lands see no CRD schema drift.Migration applied
pkg/config,pkg/controller,pkg/resource,pkg/terraform,pkg/types,pkg/pipeline,tests/conversion, plus the GoMock fake atpkg/resource/fake/mocks/mock.go): swap the three retired package paths (apis/common,apis/common/v1,apis/common/v2) forgithub.com/crossplane/crossplane/apis/v2/core/v2.gcire-ordered into the project's import sections.pkg/types/reference.go: bothPackagePathXPCommonAPIsandPackagePathXPV2CommonAPIsnow resolve to the consolidated path. They're kept as two constants for source-level backward compatibility — typewriter'sUsePackagededupes by path so emitted imports collapse to a single alias.pkg/pipeline/templates/crd_types.go.tmpl:ResourceSpec→ClusterManagedResourceSpecfor cluster-scoped CRDs.ResourceStatus→ManagedResourceStatusfor the embedded status (both scopes).pkg/types/builder_test.gorefreshed to match the new fully-qualified path.tests/conversion/test_resource.gofield embeds renamed toClusterManagedResourceSpec/ManagedResourceStatus.pkg/pipeline/hooks_test.gogolden fixture aligned with the updatedcrd_types.go.tmploutput..github/workflows/ci.ymlGO_VERSIONbumped from1.25.8to1.25.10—crossplane-runtime/v2 v2.3.1requires Go 1.25.9 andgo mod tidysettlesgo.modon 1.25.10, so the workflow's pin would otherwise failmake vendorwithGOTOOLCHAIN=local.go.mod:crossplane-runtime/v2 v2.2.0→v2.3.1; new direct require oncrossplane/apis/v2 v2.3.1;go mod tidy.docs/upjet-v2-upgrade.md: new "v2.3 common-API migration" subsection so anyone following the v1→v2 guide gets steered to the new package + renamed embeds, not into the wall the old guide leads to.Fixes #663
I have:
go vet ./...,gci writewith the repo's section config,go generate ./...(no diff), andgo test ./... -count=1(all packages green, includingtests/conversion/TestConversionIntegrationwhich exercises the codegen template end-to-end).workflow_dispatchofci.ymlon the fork's branch passeslint,unit-tests, andcheck-diff.backport release-x.ylabels — intentionally not requesting a backport:release-2.2is pinned to a pseudo-version ofcrossplane-runtime/v2(pre-v2.2.0) and Go 1.24.13, so pulling v2.3 onto that maintenance line would defeat the point of the line. Forward-only.How has this code been tested
go build ./...clean.go test ./... -count=1clean across every package:pkg/apitesting/roundtrip,pkg/config,pkg/config/conversion,pkg/controller,pkg/controller/conversion,pkg/pipeline,pkg/registry,pkg/resource,pkg/resource/json,pkg/schema/traverser,pkg/terraform,pkg/terraform/errors,pkg/transformers,pkg/types,pkg/types/comments,pkg/types/markers,pkg/types/markers/kubebuilder,pkg/types/name,pkg/types/structtag,tests/conversion.tests/conversion/TestConversionIntegrationis the most useful signal here — it renders the codegen pipeline end-to-end against a test provider and asserts roundtrip conversions across versions, exercising the updatedcrd_types.go.tmpland the new package path constants.golangci-lint(gci)), unit tests, andmake check-diffall pass on theworkflow_dispatchrun.