Skip to content

replace legacy kubebuilder-tools setup with sdk-go envtest script [main] - #87

Merged
xuezhaojun merged 2 commits into
open-cluster-management-io:mainfrom
xuezhaojun:update-envtest-setup
Mar 4, 2026
Merged

replace legacy kubebuilder-tools setup with sdk-go envtest script [main]#87
xuezhaojun merged 2 commits into
open-cluster-management-io:mainfrom
xuezhaojun:update-envtest-setup

Conversation

@xuezhaojun

@xuezhaojun xuezhaojun commented Mar 4, 2026

Copy link
Copy Markdown
Member

Summary

  • Replace the deprecated Google Cloud Storage kubebuilder-tools download (ensure-kubebuilder-tools target) with the new sdk-go ensure-envtest.sh script
  • The new envtest-setup target auto-detects the Kubernetes version from go.mod and installs setup-envtest automatically
  • Remove legacy variables (ENVTEST_K8S_VERSION, KUBEBUILDER_ASSETS, K8S_VERSION, GOHOSTOS, GOHOSTARCH, KB_TOOLS_ARCHIVE_NAME, KB_TOOLS_ARCHIVE_PATH, ENVTEST)
  • Add _output/ to .gitignore for envtest cached binaries
  • Fix test assertion: ClusterPermission with empty Subjects in ClusterRoleBinding is now rejected by CRD validation

Details

The old approach manually downloaded kubebuilder-tools from storage.googleapis.com/kubebuilder-tools/ with hardcoded K8s versions. This is now deprecated.

The new approach uses open-cluster-management-io/sdk-go/ci/envtest/ensure-envtest.sh which:

  • Extracts the Kubernetes version from go.mod (k8s.io/api dependency)
  • Determines the correct setup-envtest branch from the controller-runtime version
  • Downloads and caches envtest binaries in _output/tools/bin
  • Outputs the KUBEBUILDER_ASSETS path for use in test targets

Reference guide: https://github.com/open-cluster-management-io/sdk-go/blob/main/ci/envtest/README-envtest.md

Test fix: CRD XValidation

The new envtest uses K8s 1.31 (up from 1.24/1.26), which enforces CRD CEL validation rules more strictly. The CRD defines x-kubernetes-validations rules using CEL expressions that require either subject or subjects to be present in bindings:

  • api/v1alpha1/clusterpermission_types.go:46ClusterRoleBinding field:
    // +kubebuilder:validation:XValidation:rule="has(self.subject) || has(self.subjects)",message="Either subject or subjects has to exist in clusterRoleBinding"
  • api/v1alpha1/clusterpermission_types.go:51ClusterRoleBindings field:
    // +kubebuilder:validation:XValidation:rule="self.all(i, has(i.subject) || has(i.subjects))",message="Either subject or subjects has to exist in every clusterRoleBinding"
  • api/v1alpha1/clusterpermission_types.go:60RoleBindings field:
    // +kubebuilder:validation:XValidation:rule="self.all(i, has(i.subject) || has(i.subjects))",message="Either subject or subjects has to exist in every roleBinding"

These rules are propagated to the CRD YAML at config/crds/rbac.open-cluster-management.io_clusterpermissions.yaml as x-kubernetes-validations entries.

The test previously expected Create to succeed with an empty Subjects array, but newer K8s correctly rejects this. The fix changes the assertion from .Should(Succeed()) to .ShouldNot(Succeed()).

Test plan

  • Run make envtest-setup and verify KUBEBUILDER_ASSETS is printed correctly
  • Run make test and verify tests execute with the new envtest setup
  • Verify _output/ directory is created with cached binaries

🤖 Generated with Claude Code

Replace the deprecated Google Cloud Storage kubebuilder-tools download
with the sdk-go ensure-envtest.sh script that auto-detects K8s version
from go.mod and installs setup-envtest automatically.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: xuezhaojun <zxue@redhat.com>
@openshift-ci
openshift-ci Bot requested a review from elgnay March 4, 2026 05:07
@openshift-ci

openshift-ci Bot commented Mar 4, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: xuezhaojun
Once this PR has been reviewed and has the lgtm label, please assign elgnay for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…leBinding

With the upgraded envtest (K8s 1.31), CRD CEL validation rules are now
properly enforced. Creating a ClusterPermission with an empty Subjects
array in ClusterRoleBinding is correctly rejected by the XValidation
rule defined in clusterpermission_types.go. Update the test assertion
from Should(Succeed()) to ShouldNot(Succeed()).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: xuezhaojun <zxue@redhat.com>
@codecov

codecov Bot commented Mar 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.91%. Comparing base (740564a) to head (45a4a78).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #87      +/-   ##
==========================================
- Coverage   69.13%   68.91%   -0.23%     
==========================================
  Files           4        4              
  Lines         891      891              
==========================================
- Hits          616      614       -2     
- Misses        242      243       +1     
- Partials       33       34       +1     
Flag Coverage Δ
unit 68.91% <ø> (-0.23%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@xuezhaojun
xuezhaojun merged commit a79de25 into open-cluster-management-io:main Mar 4, 2026
7 of 8 checks passed
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.

1 participant