CNTRLPLANE-3037: Introduce envtest for integration tests alternative - #8089
Merged
openshift-merge-bot[bot] merged 9 commits intoMar 30, 2026
Merged
Conversation
- Add an envtest setup goal on the Makefile - Add a new unit test using envtest to cover the same as the create cluster e2e test. Signed-off-by: Borja Clemente <bclement@redhat.com>
Introducing envtest requires adding new dependencies, which are being vendored in a separate commit to ease review. Signed-off-by: Borja Clemente <bclement@redhat.com>
Replace the Go-based envtest test with a YAML-driven test framework following the openshift/api tests pattern. The framework: - Loads test suites from tests/<crdname>/ directories matching o/api layout - Resolves CRDs via ../../zz_generated.crd-manifests/ relative paths - Filters by featureGates using payload-manifests/featuregates/ - Per suite, installs/uninstalls the CRD under test - Supports expectedError, expectedStatusError for validation tests - Supports initialCRDPatches for ratcheting validation via yaml-patch - Uses //go:build envtest tag so tests are excluded from go test ./... Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@enxebre: This pull request references CNTRLPLANE-3037 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Contributor
📝 WalkthroughWalkthroughThis pull request introduces a new YAML-driven envtest-based integration test framework for validating HyperShift CRD schemas and CEL rules. The changes replace the existing API UX validation test approach with a comprehensive testing infrastructure that includes test suite discovery and generation, feature-gate-aware CRD filtering, and automated CRD lifecycle management during testing. Build system updates enable running these tests across multiple Kubernetes and OpenShift versions, and the test suite is integrated into the standard Sequence DiagramsequenceDiagram
participant Test as Test Runner<br/>(suite_test.go)
participant Setup as envtest.Environment
participant Discovery as Suite Discovery<br/>(generator.go)
participant CRDMgmt as CRD Management<br/>(test/envtest/)
participant APIServer as Kubernetes<br/>API Server
participant Validation as Resource<br/>Validation
Test->>Setup: BeforeSuite: Create Environment
Setup->>APIServer: Start control plane
APIServer-->>Setup: Config & client ready
Setup-->>Test: testEnv, cfg, k8sClient initialized
Test->>Discovery: LoadTestSuiteSpecs(paths)
Discovery-->>Test: Return suite specifications
Test->>CRDMgmt: GenerateCRDInstallTest("Default")
CRDMgmt->>APIServer: Install all CRDs for feature set
APIServer-->>CRDMgmt: CRDs ready
loop For each loaded test suite
Test->>CRDMgmt: GenerateTestSuite(suiteSpec)
CRDMgmt->>APIServer: Install suite's CRD
APIServer-->>CRDMgmt: CRD installed & ready
loop For each test case (OnCreate/OnUpdate)
CRDMgmt->>Validation: Create/Update resource from YAML
Validation->>APIServer: Validate against CRD schema
APIServer-->>Validation: Validation result
Validation-->>CRDMgmt: Assert expected output/error
end
CRDMgmt->>APIServer: Uninstall CRD
APIServer-->>CRDMgmt: CRD removed
end
Test->>Setup: AfterSuite: Teardown
Setup->>APIServer: Stop control plane
APIServer-->>Setup: Shutdown complete
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
area/ci-tooling
4 tasks
|
@enxebre: This pull request references CNTRLPLANE-3037 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 335-337: The default Makefile target "test" currently invokes the
full envtest matrix via "generate test-envtest-api-all" which runs many
networked integration passes; change the Makefile so "test" runs only the fast
unit/path (keep the existing $(GO) test ... line) and move the envtest matrix
invocation into a separate dedicated target (e.g., "test-envtest" or
"test-integration" / "test-all") that depends on "generate test-envtest-api-all"
and then runs the full matrix; update CI workflows to call the new dedicated
target for integration runs instead of plain "make test".
- Around line 340-353: ENVTEST_OCP_INDEX is currently set to the moving master
URL which makes test behavior non-deterministic; update the ENVTEST_OCP_INDEX
variable to point to a specific immutable revision (commit hash or release tag)
of the openshift/api repository (instead of
https://raw.githubusercontent.com/openshift/api/master/envtest-releases.yaml) so
targets like test-envtest-ocp and the use of $(SETUP_ENVTEST) will consistently
fetch the same envtest-releases.yaml; replace the master URL with the chosen
commit hash or tag URL and ensure README or a comment records the chosen
revision.
In `@test/envtest/crd_filter.go`:
- Around line 217-243: The code assumes uncastFeatureGateSlice[0] and unchecked
type assertions which can panic; update the logic around
unstructured.NestedSlice usage (the call that assigns uncastFeatureGateSlice and
subsequent enabledFeatureGates/disabledFeatureGates) to validate that
uncastFeatureGateSlice is non-nil and has length > 0, cast its first element
using a safe type assertion (e.g., value, ok :=
uncastFeatureGateSlice[0].(map[string]interface{})) and return a clear error if
the shape is unexpected, and likewise check the results of the
NestedSlice/NestedString calls before indexing or casting so featureGateMapping
population (the loops referencing currGate and featureGateName) never triggers a
panic but returns a descriptive error instead.
In `@test/envtest/generator.go`:
- Around line 389-442: The saved CRD spec is being assigned as a value
(originalCRDSpec = *originalCRD.Spec.DeepCopy()) but later reused by direct
assignment (originalCRD.Spec = originalCRDSpec), causing the Properties map to
be shared and sentinel mutations to leak across tests; fix by always using a
DeepCopy of the spec when restoring or reassigning (e.g., store originalCRDSpec
as a pointer via originalCRD.Spec.DeepCopy() or call DeepCopy() on
originalCRDSpec before assigning it back to originalCRD.Spec) so modifications
to Properties["sentinel"] do not alias into the saved copy (ensure this change
is applied at the places where originalCRDSpec is created and where
originalCRD.Spec is reassigned).
In `@test/envtest/README.md`:
- Line 11: Fix the grammatical error in the README sentence "Each test suite get
its CRD installed and uninstalled before and after running." by changing "get"
to "gets" so the sentence reads "Each test suite gets its CRD installed and
uninstalled before and after running." Update the README.md content accordingly.
In `@test/envtest/suite_test.go`:
- Around line 90-93: The Describe block is executed at package init time so the
for loop over suites (the variable named suites) runs before TestAPIs populates
that slice, producing zero tests; fix by generating the suites after they are
populated — either move the for loop that calls GenerateTestSuite(suite) into an
init() that populates suites or (preferably) into TestAPIs before RunSpecs is
invoked so GenerateTestSuite is called only after TestAPIs has built the suites
slice; update the code to ensure Describe/GenerateTestSuite calls happen after
suites is filled.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 19a0ddf7-c920-435b-bcde-ee12bfb9490a
⛔ Files ignored due to path filters (75)
cmd/install/assets/hypershift-operator/payload-manifests/featuregates/featureGate-Hypershift-Default.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.azure.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.capabilities.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.dns.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.networking.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.ratcheting.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.services.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.validation.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/techpreview.hostedclusters.gcp.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/tests/nodepools.hypershift.openshift.io/stable.nodepools.autoscaling.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/tests/nodepools.hypershift.openshift.io/stable.nodepools.aws.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/tests/nodepools.hypershift.openshift.io/stable.nodepools.azure.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/hypershift-operator/tests/nodepools.hypershift.openshift.io/stable.nodepools.validation.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlgo.sumis excluded by!**/*.sumhack/tools/go.sumis excluded by!**/*.sumhack/tools/vendor/github.com/go-logr/logr/slogr/slogr.gois excluded by!**/vendor/**hack/tools/vendor/github.com/go-logr/zapr/.gitignoreis excluded by!**/vendor/**hack/tools/vendor/github.com/go-logr/zapr/.golangci.yamlis excluded by!**/vendor/**hack/tools/vendor/github.com/go-logr/zapr/LICENSEis excluded by!**/vendor/**hack/tools/vendor/github.com/go-logr/zapr/README.mdis excluded by!**/vendor/**hack/tools/vendor/github.com/go-logr/zapr/slogzapr.gois excluded by!**/vendor/**hack/tools/vendor/github.com/go-logr/zapr/zapr.gois excluded by!**/vendor/**hack/tools/vendor/github.com/go-logr/zapr/zapr_noslog.gois excluded by!**/vendor/**hack/tools/vendor/github.com/go-logr/zapr/zapr_slog.gois excluded by!**/vendor/**hack/tools/vendor/modules.txtis excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/LICENSEis excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/README.mdis excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/env/env.gois excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/env/exit.gois excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/env/helpers.gois excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/main.gois excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/remote/client.gois excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/remote/http_client.gois excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/remote/read_body.gois excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/store/helpers.gois excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/store/store.gois excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/version/version.gois excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/versions/parse.gois excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/versions/platform.gois excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/versions/version.gois excluded by!**/vendor/**hack/tools/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest/workflows/workflows.gois excluded by!**/vendor/**vendor/github.com/vmware-archive/yaml-patch/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/vmware-archive/yaml-patch/Makefileis excluded by!vendor/**,!**/vendor/**vendor/github.com/vmware-archive/yaml-patch/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/vmware-archive/yaml-patch/container.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/vmware-archive/yaml-patch/node.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/vmware-archive/yaml-patch/operation.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/vmware-archive/yaml-patch/patch.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/vmware-archive/yaml-patch/pathfinder.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/vmware-archive/yaml-patch/placeholder_wrapper.gois excluded by!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/envtest/crd.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/envtest/doc.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/envtest/helper.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/envtest/server.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/envtest/webhook.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/doc.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/errors.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/flock_other.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/flock_unix.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/addr/manager.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/certs/tinyca.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/apiserver.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/auth.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/etcd.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/kubectl.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/plane.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/arguments.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/bin_path_finder.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/procattr_other.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/procattr_unix.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/process.gois excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (15)
.golangci.ymlMakefilecmd/install/install.gogo.modhack/tools/go.modhack/tools/tools.gotest/e2e/assets/hostedcluster-base.yamltest/e2e/assets/nodepool-base.yamltest/e2e/create_cluster_test.gotest/e2e/v2/tests/api_ux_validation_test.gotest/envtest/README.mdtest/envtest/crd_filter.gotest/envtest/generator.gotest/envtest/suite_test.gotest/envtest/types.go
💤 Files with no reviewable changes (4)
- test/e2e/assets/nodepool-base.yaml
- test/e2e/assets/hostedcluster-base.yaml
- test/e2e/create_cluster_test.go
- test/e2e/v2/tests/api_ux_validation_test.go
Comment on lines
+335
to
337
| test: generate test-envtest-api-all | ||
| @echo "Running tests with $(NUM_CORES) parallel jobs..." | ||
| $(GO) test -race -parallel=$(NUM_CORES) -count=1 -timeout=30m ./... -coverprofile cover.out |
Contributor
There was a problem hiding this comment.
Keep the full envtest matrix out of the default make test path.
Lines 335-367 make every make test run 11 envtest passes before regular unit tests. .github/workflows/test.yaml:1-14 still runs plain make test in a single 60-minute job, so the default test path is now a long, networked integration matrix. I'd keep this behind a dedicated target/job and leave make test as the fast path.
Also applies to: 348-367
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 335 - 337, The default Makefile target "test"
currently invokes the full envtest matrix via "generate test-envtest-api-all"
which runs many networked integration passes; change the Makefile so "test" runs
only the fast unit/path (keep the existing $(GO) test ... line) and move the
envtest matrix invocation into a separate dedicated target (e.g., "test-envtest"
or "test-integration" / "test-all") that depends on "generate
test-envtest-api-all" and then runs the full matrix; update CI workflows to call
the new dedicated target for integration runs instead of plain "make test".
Comment on lines
+340
to
+353
| ENVTEST_OCP_INDEX := https://raw.githubusercontent.com/openshift/api/master/envtest-releases.yaml | ||
| # OCP version to Kubernetes version mapping (OCP 4.x -> K8s 1.(x+13)) | ||
| # OCP 4.17=1.30, 4.18=1.31, 4.19=1.32, 4.20=1.33, 4.21=1.34, 4.22=1.35 | ||
| ENVTEST_OCP_K8S_VERSIONS ?= 1.30.3 1.31.2 1.32.1 1.33.2 1.34.1 1.35.1 | ||
|
|
||
| # Vanilla Kubernetes versions for envtest (upstream kubebuilder assets) | ||
| ENVTEST_KUBE_VERSIONS ?= 1.31.0 1.32.0 1.33.0 1.34.0 1.35.0 | ||
|
|
||
| .PHONY: test-envtest-ocp | ||
| test-envtest-ocp: generate $(SETUP_ENVTEST) ## Run envtest tests for all supported OCP versions (4.17-4.22) | ||
| @for k8s_ver in $(ENVTEST_OCP_K8S_VERSIONS); do \ | ||
| echo "=== Running envtest for OCP (K8s $$k8s_ver) ==="; \ | ||
| KUBEBUILDER_ASSETS="$$($(SETUP_ENVTEST) use --use-env --bin-dir $(ENVTEST_OCP_ASSETS_DIR) -p path --index $(ENVTEST_OCP_INDEX) $$k8s_ver)" \ | ||
| $(GO) test -tags envtest -race -count=1 -timeout=30m ./test/envtest/... || exit 1; \ |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the Makefile to understand the context around lines 340-353
wc -l MakefileRepository: openshift/hypershift
Length of output: 76
🏁 Script executed:
# Get the actual content of the Makefile around the specified lines
sed -n '335,360p' MakefileRepository: openshift/hypershift
Length of output: 1537
🏁 Script executed:
# Search for all references to ENVTEST_OCP_INDEX in the codebase
rg "ENVTEST_OCP_INDEX" --type makefile -A 2 -B 2Repository: openshift/hypershift
Length of output: 96
🏁 Script executed:
# Check if there are other similar URLs in the Makefile that may be pinned or unpinned
rg "github\.com.*/(master|main|head)" MakefileRepository: openshift/hypershift
Length of output: 46
🏁 Script executed:
# Look for any vendoring or caching strategies in the repo for remote files
rg -i "vendor|cache" Makefile | head -20Repository: openshift/hypershift
Length of output: 2776
🏁 Script executed:
# Search for ENVTEST_OCP_INDEX without file type restriction
rg "ENVTEST_OCP_INDEX" -A 2 -B 2Repository: openshift/hypershift
Length of output: 797
🏁 Script executed:
# Check if there are other index URLs or similar variables in Makefile
rg "INDEX|index" Makefile | head -20Repository: openshift/hypershift
Length of output: 480
🏁 Script executed:
# Look for any documentation or comments about this configuration
rg -i "envtest.*ocp|ocp.*envtest" Makefile -B 3 -A 3Repository: openshift/hypershift
Length of output: 1990
🏁 Script executed:
# Check if there's a go.mod or vendor info about openshift/api
fd "go.mod\|go.sum" -type f | head -5Repository: openshift/hypershift
Length of output: 234
🏁 Script executed:
# Check if there's a way to pin or if similar pattern exists for ENVTEST_KUBE_VERSIONS
rg "ENVTEST_KUBE" Makefile -B 2 -A 2Repository: openshift/hypershift
Length of output: 1660
🏁 Script executed:
# Search for any comments or documentation about why master branch is used
rg -i "master.*branch|branch.*master|envtest.*pin|pin.*envtest" Makefile Makefile.* 2>/dev/null || rg "TODO\|FIXME\|NOTE" Makefile | grep -i envtestRepository: openshift/hypershift
Length of output: 46
🏁 Script executed:
# Check if there's any caching mechanism in setup-envtest or other tooling
rg "setup-envtest\|SETUP_ENVTEST" Makefile -B 1 -A 5 | head -30Repository: openshift/hypershift
Length of output: 46
🏁 Script executed:
# Check git history or comments for context on this decision
git log --oneline -n 20 -- Makefile | head -10Repository: openshift/hypershift
Length of output: 132
Pin ENVTEST_OCP_INDEX to an immutable revision.
Line 340 fetches envtest-releases.yaml from openshift/api/master, so this repo's test behavior can change underneath the branch with no change here. Pin the URL to a commit hash or release tag instead of using the moving master branch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 340 - 353, ENVTEST_OCP_INDEX is currently set to the
moving master URL which makes test behavior non-deterministic; update the
ENVTEST_OCP_INDEX variable to point to a specific immutable revision (commit
hash or release tag) of the openshift/api repository (instead of
https://raw.githubusercontent.com/openshift/api/master/envtest-releases.yaml) so
targets like test-envtest-ocp and the use of $(SETUP_ENVTEST) will consistently
fetch the same envtest-releases.yaml; replace the master URL with the chosen
commit hash or tag URL and ensure README or a comment records the chosen
revision.
Comment on lines
+217
to
+243
| uncastFeatureGateSlice, _, err := unstructured.NestedSlice(uncastFeatureGate, "status", "featureGates") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("no slice found %w", err) | ||
| } | ||
| enabledFeatureGates, _, err := unstructured.NestedSlice(uncastFeatureGateSlice[0].(map[string]interface{}), "enabled") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("no enabled found %w", err) | ||
| } | ||
| disabledFeatureGates, _, err := unstructured.NestedSlice(uncastFeatureGateSlice[0].(map[string]interface{}), "disabled") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("no disabled found %w", err) | ||
| } | ||
|
|
||
| featureGateMapping := map[string]bool{} | ||
| for _, currGate := range enabledFeatureGates { | ||
| featureGateName, _, err := unstructured.NestedString(currGate.(map[string]interface{}), "name") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("no gate name found %w", err) | ||
| } | ||
| featureGateMapping[featureGateName] = true | ||
| } | ||
| for _, currGate := range disabledFeatureGates { | ||
| featureGateName, _, err := unstructured.NestedString(currGate.(map[string]interface{}), "name") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("no gate name found %w", err) | ||
| } | ||
| featureGateMapping[featureGateName] = false |
Contributor
There was a problem hiding this comment.
Avoid panics while decoding status.featureGates.
If the matched featuregate manifest is missing status.featureGates or the first entry has an unexpected shape, Lines 217-243 panic on [0] and unchecked type assertions instead of returning a normal error. One bad asset will abort suite discovery before any tests run.
Suggested hardening
- uncastFeatureGateSlice, _, err := unstructured.NestedSlice(uncastFeatureGate, "status", "featureGates")
+ uncastFeatureGateSlice, found, err := unstructured.NestedSlice(uncastFeatureGate, "status", "featureGates")
if err != nil {
return nil, fmt.Errorf("no slice found %w", err)
}
- enabledFeatureGates, _, err := unstructured.NestedSlice(uncastFeatureGateSlice[0].(map[string]interface{}), "enabled")
+ if !found || len(uncastFeatureGateSlice) == 0 {
+ return nil, fmt.Errorf("feature gate manifest is missing status.featureGates")
+ }
+ featureGateStatus, ok := uncastFeatureGateSlice[0].(map[string]interface{})
+ if !ok {
+ return nil, fmt.Errorf("feature gate manifest has an invalid status.featureGates entry")
+ }
+ enabledFeatureGates, _, err := unstructured.NestedSlice(featureGateStatus, "enabled")
if err != nil {
return nil, fmt.Errorf("no enabled found %w", err)
}
- disabledFeatureGates, _, err := unstructured.NestedSlice(uncastFeatureGateSlice[0].(map[string]interface{}), "disabled")
+ disabledFeatureGates, _, err := unstructured.NestedSlice(featureGateStatus, "disabled")
if err != nil {
return nil, fmt.Errorf("no disabled found %w", err)
}
featureGateMapping := map[string]bool{}
for _, currGate := range enabledFeatureGates {
- featureGateName, _, err := unstructured.NestedString(currGate.(map[string]interface{}), "name")
+ gateObj, ok := currGate.(map[string]interface{})
+ if !ok {
+ return nil, fmt.Errorf("enabled feature gate entry has unexpected type %T", currGate)
+ }
+ featureGateName, _, err := unstructured.NestedString(gateObj, "name")
if err != nil {
return nil, fmt.Errorf("no gate name found %w", err)
}
featureGateMapping[featureGateName] = true
}
for _, currGate := range disabledFeatureGates {
- featureGateName, _, err := unstructured.NestedString(currGate.(map[string]interface{}), "name")
+ gateObj, ok := currGate.(map[string]interface{})
+ if !ok {
+ return nil, fmt.Errorf("disabled feature gate entry has unexpected type %T", currGate)
+ }
+ featureGateName, _, err := unstructured.NestedString(gateObj, "name")
if err != nil {
return nil, fmt.Errorf("no gate name found %w", err)
}
featureGateMapping[featureGateName] = false
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| uncastFeatureGateSlice, _, err := unstructured.NestedSlice(uncastFeatureGate, "status", "featureGates") | |
| if err != nil { | |
| return nil, fmt.Errorf("no slice found %w", err) | |
| } | |
| enabledFeatureGates, _, err := unstructured.NestedSlice(uncastFeatureGateSlice[0].(map[string]interface{}), "enabled") | |
| if err != nil { | |
| return nil, fmt.Errorf("no enabled found %w", err) | |
| } | |
| disabledFeatureGates, _, err := unstructured.NestedSlice(uncastFeatureGateSlice[0].(map[string]interface{}), "disabled") | |
| if err != nil { | |
| return nil, fmt.Errorf("no disabled found %w", err) | |
| } | |
| featureGateMapping := map[string]bool{} | |
| for _, currGate := range enabledFeatureGates { | |
| featureGateName, _, err := unstructured.NestedString(currGate.(map[string]interface{}), "name") | |
| if err != nil { | |
| return nil, fmt.Errorf("no gate name found %w", err) | |
| } | |
| featureGateMapping[featureGateName] = true | |
| } | |
| for _, currGate := range disabledFeatureGates { | |
| featureGateName, _, err := unstructured.NestedString(currGate.(map[string]interface{}), "name") | |
| if err != nil { | |
| return nil, fmt.Errorf("no gate name found %w", err) | |
| } | |
| featureGateMapping[featureGateName] = false | |
| uncastFeatureGateSlice, found, err := unstructured.NestedSlice(uncastFeatureGate, "status", "featureGates") | |
| if err != nil { | |
| return nil, fmt.Errorf("no slice found %w", err) | |
| } | |
| if !found || len(uncastFeatureGateSlice) == 0 { | |
| return nil, fmt.Errorf("feature gate manifest is missing status.featureGates") | |
| } | |
| featureGateStatus, ok := uncastFeatureGateSlice[0].(map[string]interface{}) | |
| if !ok { | |
| return nil, fmt.Errorf("feature gate manifest has an invalid status.featureGates entry") | |
| } | |
| enabledFeatureGates, _, err := unstructured.NestedSlice(featureGateStatus, "enabled") | |
| if err != nil { | |
| return nil, fmt.Errorf("no enabled found %w", err) | |
| } | |
| disabledFeatureGates, _, err := unstructured.NestedSlice(featureGateStatus, "disabled") | |
| if err != nil { | |
| return nil, fmt.Errorf("no disabled found %w", err) | |
| } | |
| featureGateMapping := map[string]bool{} | |
| for _, currGate := range enabledFeatureGates { | |
| gateObj, ok := currGate.(map[string]interface{}) | |
| if !ok { | |
| return nil, fmt.Errorf("enabled feature gate entry has unexpected type %T", currGate) | |
| } | |
| featureGateName, _, err := unstructured.NestedString(gateObj, "name") | |
| if err != nil { | |
| return nil, fmt.Errorf("no gate name found %w", err) | |
| } | |
| featureGateMapping[featureGateName] = true | |
| } | |
| for _, currGate := range disabledFeatureGates { | |
| gateObj, ok := currGate.(map[string]interface{}) | |
| if !ok { | |
| return nil, fmt.Errorf("disabled feature gate entry has unexpected type %T", currGate) | |
| } | |
| featureGateName, _, err := unstructured.NestedString(gateObj, "name") | |
| if err != nil { | |
| return nil, fmt.Errorf("no gate name found %w", err) | |
| } | |
| featureGateMapping[featureGateName] = false | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/envtest/crd_filter.go` around lines 217 - 243, The code assumes
uncastFeatureGateSlice[0] and unchecked type assertions which can panic; update
the logic around unstructured.NestedSlice usage (the call that assigns
uncastFeatureGateSlice and subsequent enabledFeatureGates/disabledFeatureGates)
to validate that uncastFeatureGateSlice is non-nil and has length > 0, cast its
first element using a safe type assertion (e.g., value, ok :=
uncastFeatureGateSlice[0].(map[string]interface{})) and return a clear error if
the shape is unexpected, and likewise check the results of the
NestedSlice/NestedString calls before indexing or casting so featureGateMapping
population (the loops referencing currGate and featureGateName) never triggers a
panic but returns a descriptive error instead.
Comment on lines
+389
to
+442
| originalCRDSpec = *originalCRD.Spec.DeepCopy() | ||
| originalCRD.Spec = patchedCRD.Spec | ||
|
|
||
| // Add a sentinel field so that we can check that the schema update has persisted. | ||
| originalCRD.Spec.Versions[0].Schema.OpenAPIV3Schema.Properties["sentinel"] = apiextensionsv1.JSONSchemaProps{ | ||
| Type: "string", | ||
| Enum: []apiextensionsv1.JSON{ | ||
| {Raw: []byte(fmt.Sprintf(`"%s+patched"`, initialObj.GetUID()))}, | ||
| }, | ||
| } | ||
| initialObj.Object["sentinel"] = initialObj.GetUID() + "+patched" | ||
|
|
||
| Expect(k8sClient.Update(ctx, originalCRD)).To(Succeed(), "failed updating patched CRD schema") | ||
| } | ||
|
|
||
| initialStatus, hasStatus, err := unstructured.NestedFieldNoCopy(initialObj.Object, "status") | ||
| Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| // Use an eventually here, so that we retry until the sentinel correctly applies. | ||
| Eventually(func() error { | ||
| return k8sClient.Create(ctx, initialObj) | ||
| }, "5s").Should(Succeed(), "initial object should create successfully") | ||
|
|
||
| if hasStatus && initialStatus != nil { | ||
| Expect(unstructured.SetNestedField(initialObj.Object, initialStatus, "status")).To(Succeed()) | ||
| Expect(k8sClient.Status().Update(ctx, initialObj)).ToNot(HaveOccurred(), "initial object status should update successfully") | ||
| } | ||
|
|
||
| if len(in.crdPatches) > 0 { | ||
| originalCRD := &apiextensionsv1.CustomResourceDefinition{} | ||
| Expect(k8sClient.Get(ctx, originalCRDObjectKey, originalCRD)).To(Succeed()) | ||
|
|
||
| originalCRD.Spec = originalCRDSpec | ||
|
|
||
| // Add a sentinel field so that we can check that the schema update has persisted. | ||
| originalCRD.Spec.Versions[0].Schema.OpenAPIV3Schema.Properties["sentinel"] = apiextensionsv1.JSONSchemaProps{ | ||
| Type: "string", | ||
| Enum: []apiextensionsv1.JSON{ | ||
| {Raw: []byte(fmt.Sprintf(`"%s+restored"`, initialObj.GetUID()))}, | ||
| }, | ||
| } | ||
|
|
||
| Expect(k8sClient.Update(ctx, originalCRD)).To(Succeed()) | ||
|
|
||
| Eventually(func() error { | ||
| updatedObj := initialObj.DeepCopy() | ||
| updatedObj.Object["sentinel"] = initialObj.GetUID() + "+restored" | ||
|
|
||
| return k8sClient.Update(ctx, updatedObj) | ||
| }, "5s").Should(Succeed(), "Sentinel should be persisted") | ||
|
|
||
| // Drop the sentinel field now we know the rest of the CRD schema is up to date. | ||
| originalCRD.Spec = originalCRDSpec | ||
| Expect(k8sClient.Update(ctx, originalCRD)).To(Succeed()) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the actual file to confirm the code matches the review
head -n 450 test/envtest/generator.go | tail -n 65Repository: openshift/hypershift
Length of output: 2752
🏁 Script executed:
# Look for the DeepCopy method signature to understand its return type
rg "func.*DeepCopy" test/envtest/generator.go -A 3Repository: openshift/hypershift
Length of output: 46
🏁 Script executed:
# Find the apiextensionsv1 import and check if DeepCopy is a method we can inspect
rg "apiextensionsv1|CustomResourceDefinitionSpec" test/envtest/generator.go | head -20Repository: openshift/hypershift
Length of output: 1512
🏁 Script executed:
# Let's see what module this is using for apiextensionsv1
grep -r "apiextensionsv1" test/envtest/generator.go | head -5Repository: openshift/hypershift
Length of output: 447
🏁 Script executed:
# Check the go.mod to understand the dependencies
cat go.mod | grep -i "apiextensions\|k8s"Repository: openshift/hypershift
Length of output: 2223
🏁 Script executed:
# Let's check the Kubernetes source for how DeepCopy handles maps
# First, search if there are any DeepCopy method signatures visible in the codebase
rg "DeepCopy.*CustomResourceDefinitionSpec" --type goRepository: openshift/hypershift
Length of output: 4653
🏁 Script executed:
# Let's create a simple test to understand the Go semantics of map aliasing with struct assignment
cat > /tmp/test_map_aliasing.go << 'EOF'
package main
import "fmt"
type Inner struct {
Props map[string]string
}
type Outer struct {
Inner *Inner
}
func main() {
// Simulate what DeepCopy might return
original := &Outer{
Inner: &Inner{
Props: map[string]string{"key": "original"},
},
}
// Simulate dereferencing a DeepCopy result
// In this case, we'll manually copy the pointer fields
copied := *original // Value copy of Outer, but Inner pointer is shared
// Now both original and copied have Inner pointing to same map
copied.Inner.Props["key"] = "modified"
fmt.Printf("original.Inner.Props: %v\n", original.Inner.Props)
fmt.Printf("copied.Inner.Props: %v\n", copied.Inner.Props)
fmt.Printf("Same map? %v\n", original.Inner == copied.Inner)
}
EOF
python3 << 'PYTEST'
import subprocess
result = subprocess.run(['go', 'run', '/tmp/test_map_aliasing.go'], capture_output=True, text=True)
print(result.stdout)
if result.stderr:
print("STDERR:", result.stderr)
PYTESTRepository: openshift/hypershift
Length of output: 157
🏁 Script executed:
# Now let's check what the actual Kubernetes v0.34.3 DeepCopy implementation looks like
# by examining if we can find the generated code
find . -name "*.go" -type f | xargs grep -l "CustomResourceDefinitionSpec.*DeepCopy" | head -5Repository: openshift/hypershift
Length of output: 612
🏁 Script executed:
# Look for generated DeepCopy code in vendor or similar
rg "func \(.*CustomResourceDefinitionSpec\) DeepCopy" --type go -A 20 2>/dev/null | head -50Repository: openshift/hypershift
Length of output: 5850
🏁 Script executed:
# Find JSONSchemaProps DeepCopy implementation
rg "func.*JSONSchemaProps.*DeepCopy" vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/ -A 15 | head -80Repository: openshift/hypershift
Length of output: 9156
🏁 Script executed:
# Let's also check the actual Properties field type and how it's copied
rg "type JSONSchemaProps struct" vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/ -A 30 | head -50Repository: openshift/hypershift
Length of output: 8071
🏁 Script executed:
# Get the Properties field definition from JSONSchemaProps
rg "type JSONSchemaProps struct" vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types_jsonschema.go -A 100 | grep -E "(Properties|^--)" | head -20Repository: openshift/hypershift
Length of output: 718
🏁 Script executed:
# Now let's look at the actual DeepCopy implementation for JSONSchemaProps
rg "func \(in \*JSONSchemaProps\) DeepCopy" vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/ -A 20 | head -100Repository: openshift/hypershift
Length of output: 10686
🏁 Script executed:
# Get the full JSONSchemaProps.DeepCopy() implementation to see how it handles the Properties map
rg "func \(in \*JSONSchemaProps\) DeepCopy" vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/deepcopy.go -A 100 | head -120Repository: openshift/hypershift
Length of output: 1716
🏁 Script executed:
# Continue searching for Properties map handling in DeepCopy
rg "func \(in \*JSONSchemaProps\) DeepCopy" vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/deepcopy.go -A 200 | grep -A 20 "Properties"Repository: openshift/hypershift
Length of output: 1657
🏁 Script executed:
# Let me trace through the code logic one more time to confirm
# The issue is: when you assign a value containing map fields, both copies reference the same underlying map
# Let's verify the exact scenario in the code
cat test/envtest/generator.go | sed -n '389,442p'Repository: openshift/hypershift
Length of output: 2332
Deep-copy the saved CRD spec before each sentinel mutation to prevent map aliasing across test table entries.
When originalCRDSpec = *originalCRD.Spec.DeepCopy() assigns the dereferenced pointer to a value variable, and then originalCRD.Spec = originalCRDSpec assigns it back, both refer to the same underlying Properties map. Mutating Properties["sentinel"] leaks into the saved copy, preventing proper schema restoration and breaking test isolation for subsequent table entries.
Suggested fix
- var originalCRDSpec apiextensionsv1.CustomResourceDefinitionSpec
+ var originalCRDSpec *apiextensionsv1.CustomResourceDefinitionSpec
@@
- originalCRDSpec = *originalCRD.Spec.DeepCopy()
+ originalCRDSpec = originalCRD.Spec.DeepCopy()
originalCRD.Spec = patchedCRD.Spec
@@
- originalCRD.Spec = originalCRDSpec
+ originalCRD.Spec = *originalCRDSpec.DeepCopy()
@@
- originalCRD.Spec = originalCRDSpec
+ originalCRD.Spec = *originalCRDSpec.DeepCopy()
Expect(k8sClient.Update(ctx, originalCRD)).To(Succeed())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/envtest/generator.go` around lines 389 - 442, The saved CRD spec is
being assigned as a value (originalCRDSpec = *originalCRD.Spec.DeepCopy()) but
later reused by direct assignment (originalCRD.Spec = originalCRDSpec), causing
the Properties map to be shared and sentinel mutations to leak across tests; fix
by always using a DeepCopy of the spec when restoring or reassigning (e.g.,
store originalCRDSpec as a pointer via originalCRD.Spec.DeepCopy() or call
DeepCopy() on originalCRDSpec before assigning it back to originalCRD.Spec) so
modifications to Properties["sentinel"] do not alias into the saved copy (ensure
this change is applied at the places where originalCRDSpec is created and where
originalCRD.Spec is reassigned).
|
|
||
| ## How it works | ||
|
|
||
| 1. Each test suite get its CRD installed and uninstalled before and after running. |
Contributor
There was a problem hiding this comment.
Minor grammatical fix needed.
"Each test suite get" should be "Each test suite gets".
📝 Suggested fix
-1. Each test suite get its CRD installed and uninstalled before and after running.
+1. Each test suite gets its CRD installed and uninstalled before and after running.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 1. Each test suite get its CRD installed and uninstalled before and after running. | |
| 1. Each test suite gets its CRD installed and uninstalled before and after running. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/envtest/README.md` at line 11, Fix the grammatical error in the README
sentence "Each test suite get its CRD installed and uninstalled before and after
running." by changing "get" to "gets" so the sentence reads "Each test suite
gets its CRD installed and uninstalled before and after running." Update the
README.md content accordingly.
Comment on lines
+90
to
+93
| var _ = Describe("", func() { | ||
| for _, suite := range suites { | ||
| GenerateTestSuite(suite) | ||
| } |
Contributor
There was a problem hiding this comment.
Critical: Test suites won't be generated due to Ginkgo execution order.
The suites slice is populated in TestAPIs (line 43), but the Describe block on lines 90-93 is evaluated at package initialization time (when var _ = Describe(...) runs), which occurs before TestAPIs executes. At that point, suites is an empty slice, so the for loop generates zero test cases.
This means the dynamically-loaded test suites will never actually run.
🐛 Suggested fix: Load suites at init time
var cfg *rest.Config
var k8sClient client.Client
var testEnv *envtest.Environment
var ctx = context.Background()
var suites []SuiteSpec
+var assetsDir string
+
+func init() {
+ _, thisFile, _, _ := runtime.Caller(0)
+ testDir := filepath.Dir(thisFile)
+ assetsDir = filepath.Join(testDir, "..", "..", "cmd", "install", "assets", "hypershift-operator")
+
+ var err error
+ suites, err = LoadTestSuiteSpecs(assetsDir)
+ if err != nil {
+ panic(fmt.Sprintf("failed to load test suite specs: %v", err))
+ }
+}
func TestAPIs(t *testing.T) {
RegisterFailHandler(Fail)
- g := NewGomegaWithT(t)
-
- _, thisFile, _, _ := runtime.Caller(0)
- testDir := filepath.Dir(thisFile)
- assetsDir := filepath.Join(testDir, "..", "..", "cmd", "install", "assets", "hypershift-operator")
-
- var err error
- suites, err = LoadTestSuiteSpecs(assetsDir)
- g.Expect(err).ToNot(HaveOccurred())
-
RunSpecs(t, "HyperShift API Integration Suite")
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/envtest/suite_test.go` around lines 90 - 93, The Describe block is
executed at package init time so the for loop over suites (the variable named
suites) runs before TestAPIs populates that slice, producing zero tests; fix by
generating the suites after they are populated — either move the for loop that
calls GenerateTestSuite(suite) into an init() that populates suites or
(preferably) into TestAPIs before RunSpecs is invoked so GenerateTestSuite is
called only after TestAPIs has built the suites slice; update the code to ensure
Describe/GenerateTestSuite calls happen after suites is filled.
JoelSpeed
reviewed
Mar 27, 2026
| echo "Running tests with $(NUM_CORES) parallel jobs..." | ||
| test: generate test-envtest-api-all | ||
| @echo "Running tests with $(NUM_CORES) parallel jobs..." | ||
| $(GO) test -race -parallel=$(NUM_CORES) -count=1 -timeout=30m ./... -coverprofile cover.out |
Contributor
There was a problem hiding this comment.
Out of interest, why not use the -p auto detection of number of cores? Is this equivalent?
| # OCP envtest index for downstream kubebuilder assets | ||
| ENVTEST_OCP_INDEX := https://raw.githubusercontent.com/openshift/api/master/envtest-releases.yaml | ||
| # OCP version to Kubernetes version mapping (OCP 4.x -> K8s 1.(x+13)) | ||
| # OCP 4.17=1.30, 4.18=1.31, 4.19=1.32, 4.20=1.33, 4.21=1.34, 4.22=1.35 |
Contributor
There was a problem hiding this comment.
We currently also need 4.15 and 4.16, I'm working on getting those in as 4.15 is a little more manual to build out the tars
Not blocking, we can follow up to add those later
Comment on lines
+5
to
+6
| // annotations["release.openshift.io/feature-set"], but HyperShift's featuregate | ||
| // manifests use spec.featureSet instead. We check both for compatibility. |
Contributor
There was a problem hiding this comment.
Why are they different? It surprises me that these would need to be different approaches
Member
Author
There was a problem hiding this comment.
#8034 (comment) my reasoning is there, I might be wrong though
| clusterProfileToShortName = map[string]string{ | ||
| "include.release.openshift.io/ibm-cloud-managed": "Hypershift", | ||
| "include.release.openshift.io/self-managed-high-availability": "SelfManagedHA", | ||
| "include.release.openshift.io/single-node-developer": "SingleNode", |
Contributor
There was a problem hiding this comment.
Technically this isn't a cluster profile, not one CVO supports anyway. Was this copied from o/api?
Member
Author
|
/test e2e-aws |
1 similar comment
Member
Author
|
/test e2e-aws |
Contributor
|
/lgtm |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre, JoelSpeed The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Test Resultse2e-aws
|
Member
Author
|
/pipeline required |
|
Scheduling tests matching the |
openshift-ci-robot
added
the
verified
|
@enxebre: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8089 +/- ##
=======================================
Coverage ? 26.34%
=======================================
Files ? 1087
Lines ? 104856
Branches ? 0
=======================================
Hits ? 27621
Misses ? 74838
Partials ? 2397
🚀 New features to boost your workflow:
|
Contributor
|
@enxebre: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 task
bryan-cox
added a commit
to bryan-cox/hypershift
that referenced
this pull request
Mar 30, 2026
PR openshift#8089 copied feature gate YAML files from api/ to install assets, but the copies were made before PR openshift#7774 added ExternalOIDCWithUpstreamParity to the source files. Regenerate the copies via `make api`. Also make `git update-index --refresh` non-fatal in the Verify workflow, since it can return non-zero on timestamp-only changes even when content is identical. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4 tasks
enxebre
added a commit
that referenced
this pull request
Apr 13, 2026
NO-JIRA The envtest suites introduced in PR #8089 cover the same CEL and schema validations exercised by this E2E test. The envtest approach runs against a lightweight API server during `make test` without requiring a live management cluster, making it faster and more reliable. Remove the E2E test file and its dedicated base assets since they are now fully superseded by the declarative envtest test suites in cmd/install/assets/crds/hypershift-operator/tests/. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 13, 2026
Closed
4 tasks
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
5 participants
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.
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes
Special notes for your reviewer:
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Tests
Documentation
Chores