Skip to content

bump crossplane-runtime to v2.3.1#664

Open
jschoombee wants to merge 6 commits into
crossplane:mainfrom
jschoombee:crossplane-runtime-v2.3.1
Open

bump crossplane-runtime to v2.3.1#664
jschoombee wants to merge 6 commits into
crossplane:mainfrom
jschoombee:crossplane-runtime-v2.3.1

Conversation

@jschoombee

@jschoombee jschoombee commented May 31, 2026

Copy link
Copy Markdown

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-runtime into the consolidated github.com/crossplane/crossplane/apis/v2/core/v2 package 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:

Old (crossplane-runtime/v2/apis/common/v1) New (crossplane/apis/v2/core/v2)
ResourceSpec (cluster-scoped MRs) ClusterManagedResourceSpec
ResourceStatus ManagedResourceStatus

All 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

  1. Imports across 17 hand-written files (pkg/config, pkg/controller, pkg/resource, pkg/terraform, pkg/types, pkg/pipeline, tests/conversion, plus the GoMock fake at pkg/resource/fake/mocks/mock.go): swap the three retired package paths (apis/common, apis/common/v1, apis/common/v2) for github.com/crossplane/crossplane/apis/v2/core/v2. gci re-ordered into the project's import sections.
  2. Codegen constants in pkg/types/reference.go: both PackagePathXPCommonAPIs and PackagePathXPV2CommonAPIs now resolve to the consolidated path. They're kept as two constants for source-level backward compatibility — typewriter's UsePackage dedupes by path so emitted imports collapse to a single alias.
  3. Codegen template in pkg/pipeline/templates/crd_types.go.tmpl:
    • ResourceSpecClusterManagedResourceSpec for cluster-scoped CRDs.
    • ResourceStatusManagedResourceStatus for the embedded status (both scopes).
  4. Embedded type-string fixtures in pkg/types/builder_test.go refreshed to match the new fully-qualified path.
  5. tests/conversion/test_resource.go field embeds renamed to ClusterManagedResourceSpec / ManagedResourceStatus.
  6. pkg/pipeline/hooks_test.go golden fixture aligned with the updated crd_types.go.tmpl output.
  7. .github/workflows/ci.yml GO_VERSION bumped from 1.25.8 to 1.25.10crossplane-runtime/v2 v2.3.1 requires Go 1.25.9 and go mod tidy settles go.mod on 1.25.10, so the workflow's pin would otherwise fail make vendor with GOTOOLCHAIN=local.
  8. go.mod: crossplane-runtime/v2 v2.2.0v2.3.1; new direct require on crossplane/apis/v2 v2.3.1; go mod tidy.
  9. 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:

  • Read and followed Upjet's contribution process.
  • Verified locally: go vet ./..., gci write with the repo's section config, go generate ./... (no diff), and go test ./... -count=1 (all packages green, including tests/conversion/TestConversionIntegration which exercises the codegen template end-to-end).
  • Verified on CI: a workflow_dispatch of ci.yml on the fork's branch passes lint, unit-tests, and check-diff.
  • Added backport release-x.y labels — intentionally not requesting a backport: release-2.2 is pinned to a pseudo-version of crossplane-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=1 clean 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/TestConversionIntegration is 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 updated crd_types.go.tmpl and the new package path constants.
  • Fork-side CI run is green: lint (incl. golangci-lint(gci)), unit tests, and make check-diff all pass on the workflow_dispatch run.

Note for reviewers: CodeRabbit's path filters exclude go.mod, go.sum, pkg/pipeline/templates/crd_types.go.tmpl, and pkg/resource/fake/mocks/mock.go from automated review. The template change in particular is the most consequential part of the PR — eyes welcome there.

….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>
@coderabbitai

coderabbitai Bot commented May 31, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 72c20cb8-5e21-409d-bcef-e06c774456c8

📥 Commits

Reviewing files that changed from the base of the PR and between eb242fc and 7aa83b7.

📒 Files selected for processing (15)
  • .github/workflows/ci.yml
  • pkg/config/resource.go
  • pkg/controller/api.go
  • pkg/controller/external.go
  • pkg/controller/external_async_tfpluginfw.go
  • pkg/controller/external_async_tfpluginsdk.go
  • pkg/controller/external_test.go
  • pkg/controller/external_tfpluginfw.go
  • pkg/controller/external_tfpluginsdk.go
  • pkg/resource/conditions.go
  • pkg/resource/sensitive.go
  • pkg/resource/sensitive_test.go
  • pkg/terraform/files.go
  • pkg/terraform/files_test.go
  • tests/conversion/test_resource.go
✅ Files skipped from review due to trivial changes (1)
  • pkg/controller/external_async_tfpluginfw.go
🚧 Files skipped from review as they are similar to previous changes (12)
  • pkg/terraform/files.go
  • pkg/config/resource.go
  • pkg/terraform/files_test.go
  • pkg/controller/external_async_tfpluginsdk.go
  • .github/workflows/ci.yml
  • tests/conversion/test_resource.go
  • pkg/controller/external_test.go
  • pkg/resource/sensitive.go
  • pkg/resource/conditions.go
  • pkg/resource/sensitive_test.go
  • pkg/controller/external_tfpluginsdk.go
  • pkg/controller/external.go

📝 Walkthrough

Walkthrough

This PR migrates Crossplane common API imports and symbols to github.com/crossplane/crossplane/apis/v2/core/v2 across controllers, resources, tests, codegen constants, CI workflow, and upgrade docs.

Changes

Common API Package Migration

Layer / File(s) Summary
Controller & config imports
pkg/config/resource.go, pkg/controller/api.go, pkg/controller/external.go, pkg/controller/external_async_tfpluginfw.go, pkg/controller/external_async_tfpluginsdk.go, pkg/controller/external_test.go, pkg/controller/external_tfpluginfw.go, pkg/controller/external_tfpluginsdk.go
Update imports to github.com/crossplane/crossplane/apis/v2/core/v2 and switch management-policy, readiness/availability, and reconcile-condition symbols to xpv2 usage.
Resource conditions, sensitive handling, terraform files & tests
pkg/resource/conditions.go, pkg/resource/sensitive.go, pkg/resource/sensitive_test.go, pkg/terraform/files.go, pkg/terraform/files_test.go, pkg/terraform/* tests
Migrate condition reason constants and helpers, SecretReference/SecretKeySelector types, SecretClient signatures, GetSensitiveObservation, connection secret reference helpers, and many gomock expectations to core v2 types.
Codegen constants and builder test expectations
pkg/types/reference.go, pkg/types/builder_test.go, pkg/pipeline/hooks_test.go
Update package-path constants to core v2 and update builder test expected generated forProvider types and embedded test snippet import paths to reference core v2 selector/reference types.
Test fixtures and conversion resource updates
tests/conversion/test_resource.go, pkg/pipeline/hooks_test.go
Update conversion test resource embeds to ClusterManagedResourceSpec/ManagedResourceStatus and switch test imports/mocks to xpv2 types.
CI workflow and docs
.github/workflows/ci.yml, docs/upjet-v2-upgrade.md
Bump CI env.GO_VERSION and have setup-go use go-version-file: go.mod; add upgrade guide describing import/embed migrations and regeneration steps.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • ulucinar
  • sergenyalcin
🚥 Pre-merge checks | ✅ 7
✅ Passed checks (7 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: bumping crossplane-runtime to v2.3.1, which aligns with the extensive migration of APIs and imports throughout the PR.
Description check ✅ Passed The description is thorough and directly related to the changeset, explaining the API relocation, renamed types, migration steps, and testing performed.
Linked Issues check ✅ Passed All coding requirements from issue #663 are met: imports migrated to consolidated path, codegen constants updated, template modified for renamed types, test fixtures refreshed, go.mod bumped, and Go version updated.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #663's requirements: API migration, codegen updates, dependency bumps, documentation, and CI toolchain alignment. No unrelated modifications detected.
Configuration Api Breaking Changes ✅ Passed pkg/config/resource.go contains only internal implementation changes (xpv1→xpv2 dependency migration); no exported types/functions/signatures changed, confirming the PR summary statement.
Generated Code Manual Edits ✅ Passed No files matching 'zz_*.go' pattern were modified in this PR. All 19 changed files are hand-written source files and documentation, not generated code.
Template Breaking Changes ✅ Passed Changes to pkg/controller/external*.go are type/import migrations (xpv1→xpv2 APIs) with no business logic or control flow alterations; no actual controller templates were modified.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 win

Migration 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 embed v1.ResourceSpec / v1.ResourceStatus, which were renamed to ClusterManagedResourceSpec / ManagedResourceStatus at the consolidated path. Since baseContent is only a string fixture for marker manipulation (not compiled), the tests still pass — so no functional breakage here. That said, given the crd_types.go.tmpl template 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4edc29a and ab80340.

⛔ Files ignored due to path filters (4)
  • go.mod is excluded by none and included by none
  • go.sum is excluded by !**/*.sum and included by none
  • pkg/pipeline/templates/crd_types.go.tmpl is excluded by none and included by none
  • pkg/resource/fake/mocks/mock.go is excluded by !**/fake/** and included by **/*.go
📒 Files selected for processing (17)
  • pkg/config/resource.go
  • pkg/controller/api.go
  • pkg/controller/external.go
  • pkg/controller/external_async_tfpluginfw.go
  • pkg/controller/external_async_tfpluginsdk.go
  • pkg/controller/external_test.go
  • pkg/controller/external_tfpluginfw.go
  • pkg/controller/external_tfpluginsdk.go
  • pkg/pipeline/hooks_test.go
  • pkg/resource/conditions.go
  • pkg/resource/sensitive.go
  • pkg/resource/sensitive_test.go
  • pkg/terraform/files.go
  • pkg/terraform/files_test.go
  • pkg/types/builder_test.go
  • pkg/types/reference.go
  • tests/conversion/test_resource.go

Comment thread pkg/types/reference.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>
@Upbound-CLA

Upbound-CLA commented Jun 2, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Comment thread .github/workflows/ci.yml Outdated
env:
# Common versions
GO_VERSION: "1.25.8"
GO_VERSION: "1.25.10"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Out of the scope of this PR, but we can use go-version-file: go.mod in the setup-go action to avoid this coupling.

Comment thread pkg/controller/external.go Outdated
"github.com/crossplane/crossplane-runtime/v2/pkg/logging"
"github.com/crossplane/crossplane-runtime/v2/pkg/reconciler/managed"
xpresource "github.com/crossplane/crossplane-runtime/v2/pkg/resource"
xpv1 "github.com/crossplane/crossplane/apis/v2/core/v2"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

xpv2? It sounds weird to have this renamed to xpv1

jschoombee added a commit to jschoombee/upjet that referenced this pull request Jun 8, 2026
The common APIs were relocated to a v2 package, so the xpv1 alias no
longer reflects the import path. Standardise on xpv2 across all
hand-written files; also unify the lone v1 alias in sensitive.go onto
the same name. mockgen-generated fake still uses its default basename
alias (v2) since it is regenerated.

Addresses review feedback on crossplane#664.
jschoombee added a commit to jschoombee/upjet that referenced this pull request Jun 8, 2026
Removes the env-var coupling so the workflow's Go version follows
whatever go.mod declares. Addresses review feedback on crossplane#664.
The common APIs were relocated to a v2 package, so the xpv1 alias no
longer reflects the import path. Standardise on xpv2 across all
hand-written files; also unify the lone v1 alias in sensitive.go onto
the same name. mockgen-generated fake still uses its default basename
alias (v2) since it is regenerated.

Addresses review feedback on crossplane#664.

Signed-off-by: JP Schoombee <jpschoombee@gmail.com>
Removes the env-var coupling so the workflow's Go version follows
whatever go.mod declares. Addresses review feedback on crossplane#664.

Signed-off-by: JP Schoombee <jpschoombee@gmail.com>
@jschoombee jschoombee force-pushed the crossplane-runtime-v2.3.1 branch from eb242fc to 7aa83b7 Compare June 8, 2026 14:30
@jschoombee

jschoombee commented Jun 8, 2026

Copy link
Copy Markdown
Author

Thanks @fernandezcuesta — both addressed:

  • xpv1xpv2 everywhere (also caught a stray v1 alias in sensitive.go). 8f76572
  • go-version-file: go.mod across all three jobs, dropped the env var. 7aa83b7

Fork CI green.

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.

Adopt crossplane-runtime v2.3.x: migrate common APIs to crossplane/apis/v2/core/v2

3 participants