Skip to content

Conversation

wangke19
Copy link
Contributor

No description provided.

Copy link

coderabbitai bot commented Aug 29, 2025

Walkthrough

Adds extensive OpenShift/Kubernetes extended test infrastructure: many new util packages and helpers, a comprehensive CLI test harness, a new kube-apiserver operator extended test, removal of a trivial test, and a large go.mod dependency and replace update.

Changes

Cohort / File(s) Summary of changes
Module dependencies
go.mod
Large dependency update: multiple version bumps, many new indirect requires, added google/uuid/zerolog/k8s placeholders, and an extensive set of k8s.io -> OpenShift staging replace directives.
New extended test
test/extended/ckao.go
Adds a Ginkgo/Gomega test covering kube-apiserver operator pause/resume and cert-rotation sidecar scenarios with helpers to scale deployments and patch secret annotations.
Removed trivial test
test/extended/main.go
Deleted the previous trivial always-passing Ginkgo test file.
CLI wrapper / core harness
test/extended/util/client.go
Adds a large CLI test harness (type CLI) with project/namespace/user management, command execution, many Kubernetes/OpenShift client accessors, resource deletion tracking, and token/user utilities.
Assertions & Gomega helpers
test/extended/util/assert.go, test/extended/util/gomega_helpers.go, test/extended/util/gwrapper.go
Adds AssertWaitPollNoErr, generic OrFail, SecureMatcher (hides values unless debug enabled), and a thin ginkgo.By wrapper.
Test framework & init
test/extended/util/framework.go, test/extended/util/test.go
Adds WaitForServiceAccount, KubeConfigPath, test initialization routines, environment defaults, namespace setup, SCC/role helpers, and TestContext wiring.
Logging extension
test/extended/util/logext/log.go
Adds a zerolog-backed logger bridged to GinkgoWriter with env-controlled debug level and Infof/Debugf/Warnf/Errorf helpers.
Pods & workload helpers
test/extended/util/pods.go
Adds Pod type and helpers: create/delete pods, readiness assertions, pod log retrieval, pod name lookup, and assertions for all pods readiness with polling.
Node & debugging helpers
test/extended/util/nodes.go
Adds node debug helpers (chroot support), node listing by label, detection of default node selector, and worker-node checks.
Resource operations
test/extended/util/resource_op.go
Adds label/annotation add/remove/get utilities, annotation existence checks, and string-slice helper functions.
Template utilities
test/extended/util/template.go
Adds create/apply-from-template helpers (cluster and namespaced), retrying process execution, and GetRandomString helper.
Provider config & init
test/extended/util/provider/config.go, test/extended/util/provider/provider.go
Adds ClusterConfiguration struct and InitializeProvider to parse TEST_PROVIDER (JSON or named provider) and populate e2e TestContext.
SAR & auth helpers
test/extended/util/sar.go
Adds WaitForSelfSAR to poll SelfSubjectAccessReview until allowed.
Clusters helper
test/extended/util/clusters.go
Adds GetInfraID to obtain infrastructure ID via oc/kubectl JSONPath output trimming quotes.
Kubeconfig / oc copy helpers
test/extended/util/oc_copy.go
Adds internal helpers to construct kubeconfig-like Config objects from rest.Config (nickname derivation, credentials, clusters, contexts).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description Check ❓ Inconclusive No description was provided, so there is insufficient information to understand the intent and scope of the changes from the author’s prose. Please add a brief description explaining the purpose and scope of the changes, including what ocpbugs-60628 is and how the new tests address it.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title “[WIP]Add test for ocpbugs-60628” clearly reflects the primary intent of the changeset by specifying that tests are being added for bug ocpbugs-60628, so it accurately summarizes the main change.
Docstring Coverage ✅ Passed Docstring coverage is 80.56% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 29, 2025
Copy link
Contributor

openshift-ci bot commented Aug 29, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link
Contributor

openshift-ci bot commented Aug 29, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

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

@wangke19 wangke19 force-pushed the ocpbugs-60628-test branch from 33b1d97 to 6b2a50f Compare October 5, 2025 19:35
@wangke19 wangke19 marked this pull request as ready for review October 5, 2025 19:36
@openshift-ci openshift-ci bot requested review from benluddy and deads2k October 5, 2025 19:37
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 51e09bc and 6b2a50f.

⛔ Files ignored due to path filters (281)
  • go.sum is excluded by !**/*.sum
  • vendor/github.com/JeffAshton/win_pdh/AUTHORS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/JeffAshton/win_pdh/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/JeffAshton/win_pdh/README.mdown is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/JeffAshton/win_pdh/pdh.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/.gitattributes is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/.gitignore is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/.golangci.yml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/CODEOWNERS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/SECURITY.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/backup.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/ea.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/file.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/fileinfo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/hvsock.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/fs/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/fs/fs.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/fs/security.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/fs/zsyscall_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/socket/rawaddr.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/socket/socket.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/socket/zsyscall_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/stringbuffer/wstring.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/pipe.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/pkg/guid/guid.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/pkg/guid/guid_nonwindows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/pkg/guid/guid_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/pkg/guid/variant_string.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/privilege.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/reparse.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/sd.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/syscall.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/zsyscall_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/.gitattributes is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/.gitignore is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/CODEOWNERS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/SECURITY.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/errors.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/hns_v1.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/hnsaccelnet.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/hnsglobals.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/hnssupport.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hns.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnsaccelnet.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnsendpoint.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnserrors.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnsfuncs.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnsglobals.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnsnetwork.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnspolicy.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnspolicylist.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnssupport.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/namespace.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/zsyscall_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/interop/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/interop/interop.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/interop/zsyscall_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/armon/circbuf/.gitignore is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/armon/circbuf/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/armon/circbuf/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/armon/circbuf/circbuf.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/container-storage-interface/spec/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/container-storage-interface/spec/lib/go/csi/csi.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/containers/v1/containers.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/containers/v1/containers.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/containers/v1/containers_grpc.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/containers/v1/containers_ttrpc.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/containers/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/tasks/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks_grpc.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks_ttrpc.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/version/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/version/v1/version.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/version/v1/version.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/version/v1/version_grpc.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/version/v1/version_ttrpc.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/descriptor.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/descriptor.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/event.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/event.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/fieldpath.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/fieldpath.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/introspection.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/introspection.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/metrics.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/metrics.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/mount.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/mount.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/platform.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/platform.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/platform_helpers.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/sandbox.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/sandbox.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/task/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/task/task.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/task/task.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/errors.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/pkg/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/pkg/errgrpc/grpc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/pkg/internal/cause/cause.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/pkg/internal/types/collapsible.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/resolve.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/log/.golangci.yml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/log/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/log/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/log/context.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/.gitattributes is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/.gitignore is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/.golangci.yml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/Makefile is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/PROTOCOL.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/Protobuild.toml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/channel.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/client.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/codec.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/config.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/errors.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/handshake.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/interceptor.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/metadata.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/request.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/request.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/server.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/services.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/stream.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/stream_server.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/test.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/unixcreds_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/typeurl/v2/.gitignore is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/typeurl/v2/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/typeurl/v2/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/typeurl/v2/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/typeurl/v2/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/typeurl/v2/types_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/coreos/go-systemd/v22/dbus/dbus.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/coreos/go-systemd/v22/dbus/methods.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/coreos/go-systemd/v22/dbus/properties.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/coreos/go-systemd/v22/dbus/set.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/coreos/go-systemd/v22/dbus/subscription.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/coreos/go-systemd/v22/dbus/subscription_set.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/CHANGELOG.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/VERSION is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/gocompat_errors_go120.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/gocompat_errors_unsupported.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/gocompat_generics_go121.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/gocompat_generics_unsupported.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/join.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/lookup_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/mkdir_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/open_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/openat2_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/openat_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/procfs_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/vfs.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/.gitattributes is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/.gitignore is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/.golangci.yml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/CODE-OF-CONDUCT.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/CONTRIBUTING.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/GOVERNANCE.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/MAINTAINERS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/Makefile is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/SECURITY.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/distribution-logo.svg is excluded by !**/*.svg, !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/helpers.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/normalize.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/reference.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/regexp.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/sort.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/CONTRIBUTING.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/MAINTAINERS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/circle.yml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/duration.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/size.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/ulimit.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/euank/go-kmsg-parser/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/euank/go-kmsg-parser/kmsgparser/kmsgparser.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/euank/go-kmsg-parser/kmsgparser/log.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/CONTRIBUTING.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/MAINTAINERS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/auth.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/auth_anonymous.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/auth_external.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/auth_sha1.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/call.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/conn.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/conn_darwin.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/conn_other.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/conn_unix.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/conn_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/dbus.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/decoder.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/default_handler.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/encoder.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/escape.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/export.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/homedir.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/match.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/message.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/object.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/sequence.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/sequential_handler.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/server_interfaces.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/sig.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_darwin.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_generic.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_nonce_tcp.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_tcp.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unix.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unixcred_dragonfly.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unixcred_freebsd.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unixcred_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unixcred_netbsd.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unixcred_openbsd.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_zos.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/variant.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/variant_lexer.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/variant_parser.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/golang/protobuf/protoc-gen-go/descriptor/descriptor.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/golang/protobuf/ptypes/timestamp/timestamp.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/golang/protobuf/ptypes/wrappers/wrappers.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/AUTHORS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/cache/memory/memory.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/collector/collector_manager.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/collector/config.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/collector/fakes.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/collector/generic_collector.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/collector/prometheus_collector.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/collector/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/collector/util.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/common/container_hints.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/common/fsHandler.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/common/helpers.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/common/inotify_watcher.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/container.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/client.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/containers/containers.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/factory.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/grpc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/handler.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/identifiers/validate.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/install/install.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/namespaces/context.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/namespaces/grpc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/namespaces/store.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/namespaces/ttrpc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/pkg/dialer/dialer.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/pkg/dialer/dialer_unix.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/pkg/dialer/dialer_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/plugin.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/crio/client.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/crio/factory.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/crio/handler.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/crio/install/install.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/crio/plugin.go is excluded by !vendor/**, !**/vendor/**
📒 Files selected for processing (19)
  • go.mod (7 hunks)
  • test/extended/ckao.go (1 hunks)
  • test/extended/main.go (0 hunks)
  • test/extended/util/assert.go (1 hunks)
  • test/extended/util/client.go (1 hunks)
  • test/extended/util/clusters.go (1 hunks)
  • test/extended/util/framework.go (1 hunks)
  • test/extended/util/gomega_helpers.go (1 hunks)
  • test/extended/util/gwrapper.go (1 hunks)
  • test/extended/util/logext/log.go (1 hunks)
  • test/extended/util/nodes.go (1 hunks)
  • test/extended/util/oc_copy.go (1 hunks)
  • test/extended/util/pods.go (1 hunks)
  • test/extended/util/provider/config.go (1 hunks)
  • test/extended/util/provider/provider.go (1 hunks)
  • test/extended/util/resource_op.go (1 hunks)
  • test/extended/util/sar.go (1 hunks)
  • test/extended/util/template.go (1 hunks)
  • test/extended/util/test.go (1 hunks)
💤 Files with no reviewable changes (1)
  • test/extended/main.go
🧰 Additional context used
🪛 OSV Scanner (2.2.3)
go.mod

[HIGH] 137-137: go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful 0.42.0: Memory exhaustion in go.opentelemetry.io/contrib/instrumentation

(GO-2023-2113)


[HIGH] 137-137: go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful 0.42.0: OpenTelemetry-Go Contrib vulnerable to denial of service in otelhttp due to unbound cardinality metrics

(GHSA-rcjv-mgp8-qvmr)

Comment on lines +101 to +141
o.Consistently(func() bool {
s, _ := kc.CoreV1().Secrets(ns).Get(ctx, secretName, metav1.GetOptions{})
ann := s.Annotations
_, comp := ann["auth.openshift.io/component"]
_, desc := ann["auth.openshift.io/description"]
dataNow := base64.StdEncoding.EncodeToString(s.Data["tls.crt"])
return !comp && !desc && dataNow == dataBefore
}, 2*time.Minute, 10*time.Second).Should(o.BeTrue())

// --- Scenario B
g.By("Scenario B: operator up, metadata restored without cert rotation")
scaleDeployment(oc, "openshift-kube-apiserver-operator", "openshift-kube-apiserver-operator", 1)

o.Eventually(func() bool {
s, _ := kc.CoreV1().Secrets(ns).Get(ctx, secretName, metav1.GetOptions{})
ann := s.Annotations
comp := ann["auth.openshift.io/component"] != ""
desc := ann["auth.openshift.io/description"] != ""
dataNow := base64.StdEncoding.EncodeToString(s.Data["tls.crt"])
return comp && desc && dataNow == dataBefore
}, 3*time.Minute, 10*time.Second).Should(o.BeTrue())

// --- Scenario C
g.By("Scenario C: operator down, expired not-after triggers sidecar rotation")
scaleDeployment(oc, "openshift-kube-apiserver-operator", "openshift-kube-apiserver-operator", 0)
past := "2000-01-01T00:00:00Z"
patchAnnotationsOnSecret(oc, ns, secretName, map[string]*string{
"auth.openshift.io/not-after": &past,
})

o.Eventually(func() bool {
s, _ := kc.CoreV1().Secrets(ns).Get(ctx, secretName, metav1.GetOptions{})
dataNow := base64.StdEncoding.EncodeToString(s.Data["tls.crt"])
ann := s.Annotations
na := ann["auth.openshift.io/not-after"]
if na == "" {
return false
}
// content rotated
return dataNow != dataBefore
}, 3*time.Minute, 10*time.Second).Should(o.BeTrue())
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Handle secret Get errors before dereferencing the result

All three polling blocks ignore the error returned by kc.CoreV1().Secrets(...).Get. If that call transiently fails (network hiccup, API temporarily returning NotFound), s is nil and s.Annotations will panic, aborting the test instead of retrying. Ensure we check the error before touching the secret (and apply the same fix to the Scenario B and Scenario C loops).

-		o.Consistently(func() bool {
-			s, _ := kc.CoreV1().Secrets(ns).Get(ctx, secretName, metav1.GetOptions{})
+		o.Consistently(func() bool {
+			s, err := kc.CoreV1().Secrets(ns).Get(ctx, secretName, metav1.GetOptions{})
+			if err != nil {
+				return false
+			}
 			ann := s.Annotations
-		o.Eventually(func() bool {
-			s, _ := kc.CoreV1().Secrets(ns).Get(ctx, secretName, metav1.GetOptions{})
+		o.Eventually(func() bool {
+			s, err := kc.CoreV1().Secrets(ns).Get(ctx, secretName, metav1.GetOptions{})
+			if err != nil {
+				return false
+			}
-		o.Eventually(func() bool {
-			s, _ := kc.CoreV1().Secrets(ns).Get(ctx, secretName, metav1.GetOptions{})
+		o.Eventually(func() bool {
+			s, err := kc.CoreV1().Secrets(ns).Get(ctx, secretName, metav1.GetOptions{})
+			if err != nil {
+				return false
+			}
🤖 Prompt for AI Agents
In test/extended/ckao.go around lines 101 to 141, each o.Consistently/Eventually
polling lambda ignores the error returned by kc.CoreV1().Secrets(...).Get and
dereferences s (s.Annotations/s.Data) which can panic if Get fails or returns
nil; update each lambda (Scenario A, B, and C) to check the returned error and
that s != nil before accessing s.Annotations or s.Data, and if err != nil or s
== nil return false (so the poll retries) rather than dereferencing; preserve
the existing return conditions when no error is present.

Comment on lines +153 to +161
f, err := os.CreateTemp("", "configfile")
if err != nil {
e2e.Failf("failure creating temporal kubeconfig file for user %s. %v", name, err)
}
c.configPath = f.Name()
err = clientcmd.WriteToFile(*kubeConfig, c.configPath)
if err != nil {
e2e.Failf("failure writing temporal kubeconfig file for user %s. %v", name, err)
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Close the temp kubeconfig before reuse

We create a temp kubeconfig but never close its file descriptor. Because SetupProject calls ChangeUser for every test, this leaks one FD per spec until Go’s finalizer eventually runs, and large suites will easily hit “too many open files”. Close the handle before writing.

 	f, err := os.CreateTemp("", "configfile")
 	if err != nil {
 		e2e.Failf("failure creating temporal kubeconfig file for user %s. %v", name, err)
 	}
 	c.configPath = f.Name()
-	err = clientcmd.WriteToFile(*kubeConfig, c.configPath)
-	if err != nil {
-		e2e.Failf("failure writing temporal kubeconfig file for user %s. %v", name, err)
-	}
+	if err := f.Close(); err != nil {
+		e2e.Failf("failure closing temporal kubeconfig file for user %s. %v", name, err)
+	}
+	if err := clientcmd.WriteToFile(*kubeConfig, c.configPath); err != nil {
+		e2e.Failf("failure writing temporal kubeconfig file for user %s. %v", name, err)
+	}
🤖 Prompt for AI Agents
In test/extended/util/client.go around lines 153 to 161, the code creates a temp
kubeconfig file but never closes its file descriptor before calling
clientcmd.WriteToFile, leaking FDs across tests; update the code to close the
temp file handle (call f.Close()) before calling WriteToFile and handle any
close error (fail the test or log the error) so the descriptor is released prior
to reuse.

Comment on lines +639 to +673
// This function also override the default 'stdout' to redirect all output
// to a buffer and prepare the global flags such as namespace and config path.
func (c *CLI) Run(commands ...string) *CLI {
in, out, errout := &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}
nc := &CLI{
execPath: c.execPath,
verb: commands[0],
kubeFramework: c.KubeFramework(),
adminConfigPath: c.adminConfigPath,
configPath: c.configPath,
showInfo: c.showInfo,
guestConfigPath: c.guestConfigPath,
user: c.user,
globalArgs: commands,
}
if !c.withoutKubeconf {
if c.asGuestKubeconf {
if c.guestConfigPath != "" {
nc.globalArgs = append([]string{fmt.Sprintf("--kubeconfig=%s", c.guestConfigPath)}, nc.globalArgs...)
} else {
e2e.Failf("want to use guest cluster kubeconfig, but it is not set, so please use oc.SetGuestKubeconf to set it firstly")
}
} else {
nc.globalArgs = append([]string{fmt.Sprintf("--kubeconfig=%s", c.configPath)}, nc.globalArgs...)
}
}
if c.asGuestKubeconf && !c.withoutNamespace {
e2e.Failf("you are doing something in ns of guest cluster, please use WithoutNamespace and set ns in Args, for example, oc.AsGuestKubeconf().WithoutNamespace().Run(\"get\").Args(\"pods\", \"-n\", \"guestclusterns\").Output()")
}
if !c.withoutNamespace {
nc.globalArgs = append([]string{fmt.Sprintf("--namespace=%s", c.Namespace())}, nc.globalArgs...)
}
nc.stdin, nc.stdout, nc.stderr = in, out, errout
return nc.setOutput(c.stdout)
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Run drops the verb when Args isn’t called

Run() never seeds finalArgs, so oc.Run("whoami").Output() (and any call that relies only on the verb or arguments passed via Run) executes plain oc with no subcommand. That’s a hard functional bug.

 	if !c.withoutNamespace {
 		nc.globalArgs = append([]string{fmt.Sprintf("--namespace=%s", c.Namespace())}, nc.globalArgs...)
 	}
-	nc.stdin, nc.stdout, nc.stderr = in, out, errout
-	return nc.setOutput(c.stdout)
+	nc.stdin, nc.stdout, nc.stderr = in, out, errout
+	nc.finalArgs = slices.Clone(nc.globalArgs)
+	return nc.setOutput(c.stdout)
📝 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
// This function also override the default 'stdout' to redirect all output
// to a buffer and prepare the global flags such as namespace and config path.
func (c *CLI) Run(commands ...string) *CLI {
in, out, errout := &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}
nc := &CLI{
execPath: c.execPath,
verb: commands[0],
kubeFramework: c.KubeFramework(),
adminConfigPath: c.adminConfigPath,
configPath: c.configPath,
showInfo: c.showInfo,
guestConfigPath: c.guestConfigPath,
user: c.user,
globalArgs: commands,
}
if !c.withoutKubeconf {
if c.asGuestKubeconf {
if c.guestConfigPath != "" {
nc.globalArgs = append([]string{fmt.Sprintf("--kubeconfig=%s", c.guestConfigPath)}, nc.globalArgs...)
} else {
e2e.Failf("want to use guest cluster kubeconfig, but it is not set, so please use oc.SetGuestKubeconf to set it firstly")
}
} else {
nc.globalArgs = append([]string{fmt.Sprintf("--kubeconfig=%s", c.configPath)}, nc.globalArgs...)
}
}
if c.asGuestKubeconf && !c.withoutNamespace {
e2e.Failf("you are doing something in ns of guest cluster, please use WithoutNamespace and set ns in Args, for example, oc.AsGuestKubeconf().WithoutNamespace().Run(\"get\").Args(\"pods\", \"-n\", \"guestclusterns\").Output()")
}
if !c.withoutNamespace {
nc.globalArgs = append([]string{fmt.Sprintf("--namespace=%s", c.Namespace())}, nc.globalArgs...)
}
nc.stdin, nc.stdout, nc.stderr = in, out, errout
return nc.setOutput(c.stdout)
}
// This function also override the default 'stdout' to redirect all output
// to a buffer and prepare the global flags such as namespace and config path.
func (c *CLI) Run(commands ...string) *CLI {
in, out, errout := &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}
nc := &CLI{
execPath: c.execPath,
verb: commands[0],
kubeFramework: c.KubeFramework(),
adminConfigPath: c.adminConfigPath,
configPath: c.configPath,
showInfo: c.showInfo,
guestConfigPath: c.guestConfigPath,
user: c.user,
globalArgs: commands,
}
if !c.withoutKubeconf {
if c.asGuestKubeconf {
if c.guestConfigPath != "" {
nc.globalArgs = append([]string{fmt.Sprintf("--kubeconfig=%s", c.guestConfigPath)}, nc.globalArgs...)
} else {
e2e.Failf("want to use guest cluster kubeconfig, but it is not set, so please use oc.SetGuestKubeconf to set it firstly")
}
} else {
nc.globalArgs = append([]string{fmt.Sprintf("--kubeconfig=%s", c.configPath)}, nc.globalArgs...)
}
}
if c.asGuestKubeconf && !c.withoutNamespace {
e2e.Failf("you are doing something in ns of guest cluster, please use WithoutNamespace and set ns in Args, for example, oc.AsGuestKubeconf().WithoutNamespace().Run(\"get\").Args(\"pods\", \"-n\", \"guestclusterns\").Output()")
}
if !c.withoutNamespace {
nc.globalArgs = append([]string{fmt.Sprintf("--namespace=%s", c.Namespace())}, nc.globalArgs...)
}
nc.stdin, nc.stdout, nc.stderr = in, out, errout
nc.finalArgs = slices.Clone(nc.globalArgs)
return nc.setOutput(c.stdout)
}
🤖 Prompt for AI Agents
In test/extended/util/client.go around lines 639 to 673, Run currently sets
nc.verb = commands[0] but does not ensure the verb is present in nc.globalArgs
so calls like oc.Run("whoami").Output() end up invoking just "oc" with no
subcommand; fix by ensuring the verb is seeded into nc.globalArgs when building
nc (i.e., initialize nc.globalArgs to include commands[0] as the first element,
followed by any additional command tokens, instead of only using commands as-is)
so that invocations that rely solely on the verb execute the subcommand
correctly.

Comment on lines +88 to +95
defaultNodeSelector, getNodeSelectorErr := oc.AsAdmin().WithoutNamespace().Run("get").Args("scheduler", "cluster", "-o=jsonpath={.spec.defaultNodeSelector}").Output()
if getNodeSelectorErr != nil && strings.Contains(defaultNodeSelector, `the server doesn't have a resource type`) {
e2e.Logf("WARNING: The scheduler API is not supported on the test cluster")
return false
}
o.Expect(getNodeSelectorErr).NotTo(o.HaveOccurred(), "Fail to get cluster scheduler defaultNodeSelector got error: %v\n", getNodeSelectorErr)
return !strings.EqualFold(defaultNodeSelector, "")
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix scheduler API fallback check

When oc get scheduler cluster hits a cluster that doesn’t expose the scheduler API, the error text lives in getNodeSelectorErr, not in the (empty) defaultNodeSelector output. The current guard never fires, so we fall through to the Expect and fail every time on clusters without that API. Please test the error string instead so we actually return false in this scenario.

-	defaultNodeSelector, getNodeSelectorErr := oc.AsAdmin().WithoutNamespace().Run("get").Args("scheduler", "cluster", "-o=jsonpath={.spec.defaultNodeSelector}").Output()
-	if getNodeSelectorErr != nil && strings.Contains(defaultNodeSelector, `the server doesn't have a resource type`) {
+	defaultNodeSelector, getNodeSelectorErr := oc.AsAdmin().WithoutNamespace().Run("get").Args("scheduler", "cluster", "-o=jsonpath={.spec.defaultNodeSelector}").Output()
+	if getNodeSelectorErr != nil && strings.Contains(getNodeSelectorErr.Error(), `the server doesn't have a resource type`) {
 		e2e.Logf("WARNING: The scheduler API is not supported on the test cluster")
 		return false
 	}
 	o.Expect(getNodeSelectorErr).NotTo(o.HaveOccurred(), "Fail to get cluster scheduler defaultNodeSelector got error: %v\n", getNodeSelectorErr)
📝 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
defaultNodeSelector, getNodeSelectorErr := oc.AsAdmin().WithoutNamespace().Run("get").Args("scheduler", "cluster", "-o=jsonpath={.spec.defaultNodeSelector}").Output()
if getNodeSelectorErr != nil && strings.Contains(defaultNodeSelector, `the server doesn't have a resource type`) {
e2e.Logf("WARNING: The scheduler API is not supported on the test cluster")
return false
}
o.Expect(getNodeSelectorErr).NotTo(o.HaveOccurred(), "Fail to get cluster scheduler defaultNodeSelector got error: %v\n", getNodeSelectorErr)
return !strings.EqualFold(defaultNodeSelector, "")
}
defaultNodeSelector, getNodeSelectorErr := oc.AsAdmin().WithoutNamespace().Run("get").Args("scheduler", "cluster", "-o=jsonpath={.spec.defaultNodeSelector}").Output()
if getNodeSelectorErr != nil && strings.Contains(getNodeSelectorErr.Error(), `the server doesn't have a resource type`) {
e2e.Logf("WARNING: The scheduler API is not supported on the test cluster")
return false
}
o.Expect(getNodeSelectorErr).NotTo(o.HaveOccurred(), "Fail to get cluster scheduler defaultNodeSelector got error: %v\n", getNodeSelectorErr)
return !strings.EqualFold(defaultNodeSelector, "")
}
🤖 Prompt for AI Agents
In test/extended/util/nodes.go around lines 88-95, the guard checking for a
scheduler-API-missing condition inspects the (empty) defaultNodeSelector string
instead of the error; update the logic to test the error object (e.g., if
getNodeSelectorErr != nil && strings.Contains(getNodeSelectorErr.Error(), "the
server doesn't have a resource type") { log and return false }) and ensure the
Expect(...) call is only executed when getNodeSelectorErr is nil (so we don't
call Expect on clusters that lack the scheduler API).

Comment on lines +75 to +86
var filterCmd = ""
if filter != "" {
filterCmd = " | grep -i " + filter
}
var filteredLogs []byte
var errCmd error
if combined {
filteredLogs, errCmd = exec.Command("bash", "-c", "cat "+podLogs+filterCmd).CombinedOutput()
} else {
filteredLogs, errCmd = exec.Command("bash", "-c", "cat "+podLogs+filterCmd).Output()
}
return string(filteredLogs), errCmd
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Stop shelling out with unescaped filter

bash -c "cat … | grep -i <filter>" lets the caller inject arbitrary shell syntax (spaces, quotes, ;, etc.) and even breaks for perfectly valid multi-word filters. Build the command without invoking a shell so every argument is kept literal.

-	var filterCmd = ""
-	if filter != "" {
-		filterCmd = " | grep -i " + filter
-	}
-	var filteredLogs []byte
-	var errCmd error
-	if combined {
-		filteredLogs, errCmd = exec.Command("bash", "-c", "cat "+podLogs+filterCmd).CombinedOutput()
-	} else {
-		filteredLogs, errCmd = exec.Command("bash", "-c", "cat "+podLogs+filterCmd).Output()
-	}
-	return string(filteredLogs), errCmd
+	var cmd *exec.Cmd
+	if filter == "" {
+		cmd = exec.Command("cat", podLogs)
+	} else {
+		cmd = exec.Command("grep", "-i", "--", filter, podLogs)
+	}
+	if combined {
+		filteredLogs, errCmd := cmd.CombinedOutput()
+		return string(filteredLogs), errCmd
+	}
+	filteredLogs, errCmd := cmd.Output()
+	return string(filteredLogs), errCmd
📝 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
var filterCmd = ""
if filter != "" {
filterCmd = " | grep -i " + filter
}
var filteredLogs []byte
var errCmd error
if combined {
filteredLogs, errCmd = exec.Command("bash", "-c", "cat "+podLogs+filterCmd).CombinedOutput()
} else {
filteredLogs, errCmd = exec.Command("bash", "-c", "cat "+podLogs+filterCmd).Output()
}
return string(filteredLogs), errCmd
var cmd *exec.Cmd
if filter == "" {
cmd = exec.Command("cat", podLogs)
} else {
cmd = exec.Command("grep", "-i", "--", filter, podLogs)
}
if combined {
filteredLogs, errCmd := cmd.CombinedOutput()
return string(filteredLogs), errCmd
}
filteredLogs, errCmd := cmd.Output()
return string(filteredLogs), errCmd
🤖 Prompt for AI Agents
In test/extended/util/pods.go around lines 75-86, stop invoking bash -c with an
unescaped filter (which allows shell injection); instead read the podLogs file
with os.ReadFile (return the read error if any) and if filter is non-empty
perform a case-insensitive line-by-line filter in Go (e.g., strings.ToLower on
line and filter, include matching lines, join with "\n"), then return the
resulting string and nil error; this removes the need to shell out and preserves
literal filter content safely.

Comment on lines +35 to +52
providerInfo := &ClusterConfiguration{}
if err := json.Unmarshal([]byte(envVar), &providerInfo); err != nil {
return fmt.Errorf("provider must be a JSON object with the 'type' key at a minimum: %v", err)
}

// update context with loaded config
context.Provider = providerInfo.ProviderName
context.CloudConfig = e2e.CloudConfig{
ProjectID: providerInfo.ProjectID,
Region: providerInfo.Region,
Zone: providerInfo.Zone,
Zones: providerInfo.Zones,
NumNodes: providerInfo.NumNodes,
MultiMaster: providerInfo.MultiMaster,
MultiZone: providerInfo.MultiZone,
ConfigFile: providerInfo.ConfigFile,
}
return nil
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Reject null or missing-type provider configs to avoid panics

If TEST_PROVIDER is the valid JSON literal null, json.Unmarshal leaves providerInfo as nil, and the subsequent providerInfo.ProviderName dereference panics before we can report a useful error. Likewise, a JSON object without "type" silently yields an empty provider name. Guard both cases so we fail fast with a clear message instead of crashing.

-	providerInfo := &ClusterConfiguration{}
-	if err := json.Unmarshal([]byte(envVar), &providerInfo); err != nil {
+	providerInfo := &ClusterConfiguration{}
+	if err := json.Unmarshal([]byte(envVar), providerInfo); err != nil {
 		return fmt.Errorf("provider must be a JSON object with the 'type' key at a minimum: %v", err)
 	}
+	if providerInfo == nil {
+		return fmt.Errorf("provider must be a non-null JSON object with the 'type' key at a minimum")
+	}
+	if providerInfo.ProviderName == "" {
+		return fmt.Errorf("provider configuration must include a non-empty \"type\" field")
+	}
 
 	// update context with loaded config
 	context.Provider = providerInfo.ProviderName
📝 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
providerInfo := &ClusterConfiguration{}
if err := json.Unmarshal([]byte(envVar), &providerInfo); err != nil {
return fmt.Errorf("provider must be a JSON object with the 'type' key at a minimum: %v", err)
}
// update context with loaded config
context.Provider = providerInfo.ProviderName
context.CloudConfig = e2e.CloudConfig{
ProjectID: providerInfo.ProjectID,
Region: providerInfo.Region,
Zone: providerInfo.Zone,
Zones: providerInfo.Zones,
NumNodes: providerInfo.NumNodes,
MultiMaster: providerInfo.MultiMaster,
MultiZone: providerInfo.MultiZone,
ConfigFile: providerInfo.ConfigFile,
}
return nil
providerInfo := &ClusterConfiguration{}
if err := json.Unmarshal([]byte(envVar), providerInfo); err != nil {
return fmt.Errorf("provider must be a JSON object with the 'type' key at a minimum: %v", err)
}
if providerInfo == nil {
return fmt.Errorf("provider must be a non-null JSON object with the 'type' key at a minimum")
}
if providerInfo.ProviderName == "" {
return fmt.Errorf("provider configuration must include a non-empty \"type\" field")
}
// update context with loaded config
context.Provider = providerInfo.ProviderName
context.CloudConfig = e2e.CloudConfig{
ProjectID: providerInfo.ProjectID,
Region: providerInfo.Region,
Zone: providerInfo.Zone,
Zones: providerInfo.Zones,
NumNodes: providerInfo.NumNodes,
MultiMaster: providerInfo.MultiMaster,
MultiZone: providerInfo.MultiZone,
ConfigFile: providerInfo.ConfigFile,
}
return nil
🤖 Prompt for AI Agents
In test/extended/util/provider/provider.go around lines 35 to 52, guard against
JSON null and missing provider type: ensure json.Unmarshal writes into the
ClusterConfiguration value (not a pointer-to-pointer), then after unmarshalling
check if providerInfo is nil or providerInfo.ProviderName is empty and return a
clear error (e.g. "TEST_PROVIDER must be a non-null JSON object containing a
non-empty 'type' field") instead of proceeding to dereference and panic; update
the error paths accordingly so the function fails fast with that message.

Comment on lines +39 to +40
cargs = append(cargs, resourceKindAndName, "-o=jsonpath={.metadata.labels."+labelName+"}")
return oc.AsAdmin().WithoutNamespace().Run("get").Args(cargs...).Output()
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Quote label keys in JSONPath

Label keys like app.kubernetes.io/name include dots, so .metadata.labels.app.kubernetes.io/name is parsed as nested fields and fails. Use bracket notation so the entire key is treated literally.

-	cargs = append(cargs, resourceKindAndName, "-o=jsonpath={.metadata.labels."+labelName+"}")
+	cargs = append(cargs, resourceKindAndName, "-o=jsonpath={.metadata.labels['"+labelName+"']}")
📝 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
cargs = append(cargs, resourceKindAndName, "-o=jsonpath={.metadata.labels."+labelName+"}")
return oc.AsAdmin().WithoutNamespace().Run("get").Args(cargs...).Output()
cargs = append(cargs, resourceKindAndName, "-o=jsonpath={.metadata.labels['"+labelName+"']}")
return oc.AsAdmin().WithoutNamespace().Run("get").Args(cargs...).Output()
🤖 Prompt for AI Agents
In test/extended/util/resource_op.go around lines 39-40, the JSONPath output
uses dot notation for label keys which fails for keys with dots; change the
JSONPath to use bracket notation so the entire label key is treated literally:
construct the -o=jsonpath argument as {.metadata.labels['<labelName>']} (i.e.
"-o=jsonpath={.metadata.labels['"+labelName+"']}") making sure the quotes are
placed/escaped correctly when building the string.

Copy link
Contributor

openshift-ci bot commented Oct 5, 2025

@wangke19: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-operator-disruptive-single-node 6b2a50f link false /test e2e-aws-operator-disruptive-single-node
ci/prow/e2e-aws-ovn-single-node 6b2a50f link false /test e2e-aws-ovn-single-node
ci/prow/okd-scos-e2e-aws-ovn 6b2a50f link false /test okd-scos-e2e-aws-ovn
ci/prow/e2e-azure-ovn 6b2a50f link false /test e2e-azure-ovn
ci/prow/e2e-aws-ovn 6b2a50f link true /test e2e-aws-ovn
ci/prow/e2e-gcp-operator-single-node 6b2a50f link false /test e2e-gcp-operator-single-node

Full PR test history. Your PR dashboard.

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (4)
go.mod (1)

135-135: Upgrade otelrestful to a patched release.

go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/[email protected] carries GO-2023-2113 / GHSA-rcjv-mgp8-qvmr (high-severity DoS). Please bump to a fixed build (e.g. v0.58.0, which ships alongside the other 0.58.x contrib packages already in this module) and refresh go.sum. The existing bot comment on this line is still unresolved.

-	go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful v0.42.0 // indirect
+	go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful v0.58.0 // indirect
test/extended/util/provider/provider.go (1)

35-52: Critical issue already flagged: nil pointer dereference on JSON null.

The past review correctly identifies that passing &providerInfo (pointer-to-pointer) to json.Unmarshal allows JSON null to set providerInfo to nil, causing a panic at line 41 when dereferencing providerInfo.ProviderName. Additionally, an empty ProviderName from a JSON object missing the "type" field should be validated. The suggested fix in the past review addresses both concerns.

test/extended/util/resource_op.go (1)

39-41: Quote label keys in the JSONPath selector

Keys like app.kubernetes.io/name currently break because .metadata.labels.<key> is parsed as nested fields. Wrap the key in brackets so it’s treated literally.

-	cargs = append(cargs, resourceKindAndName, "-o=jsonpath={.metadata.labels."+labelName+"}")
+	cargs = append(cargs, resourceKindAndName, "-o=jsonpath={.metadata.labels['"+labelName+"']}")
test/extended/util/pods.go (1)

75-86: Stop shelling out with unescaped filter – CRITICAL shell injection vulnerability.

Lines 77, 82, and 84 construct and execute a shell command with an unescaped filter parameter, allowing arbitrary command injection. This is the same critical security issue previously flagged. Any caller passing a filter like foo; rm -rf / would execute the malicious command.

Apply this diff to eliminate the shell invocation and perform filtering safely in Go:

-	var filterCmd = ""
-	if filter != "" {
-		filterCmd = " | grep -i " + filter
-	}
-	var filteredLogs []byte
-	var errCmd error
-	if combined {
-		filteredLogs, errCmd = exec.Command("bash", "-c", "cat "+podLogs+filterCmd).CombinedOutput()
-	} else {
-		filteredLogs, errCmd = exec.Command("bash", "-c", "cat "+podLogs+filterCmd).Output()
-	}
-	return string(filteredLogs), errCmd
+	// Read the log file
+	logContent, err := os.ReadFile(podLogs)
+	if err != nil {
+		return "", fmt.Errorf("failed to read log file: %w", err)
+	}
+	
+	// Apply filter if provided
+	if filter == "" {
+		return string(logContent), nil
+	}
+	
+	// Case-insensitive line-by-line filtering
+	filterLower := strings.ToLower(filter)
+	var filteredLines []string
+	for _, line := range strings.Split(string(logContent), "\n") {
+		if strings.Contains(strings.ToLower(line), filterLower) {
+			filteredLines = append(filteredLines, line)
+		}
+	}
+	return strings.Join(filteredLines, "\n"), nil

Note: This implementation also renders the combined parameter obsolete since we're reading from a file rather than capturing command output. If combined stderr/stdout is required, it should be handled when writing to podLogs.txt (line 70), not during filtering.

🧹 Nitpick comments (6)
test/extended/util/sar.go (1)

16-21: Propagate the poll context to the SelfSAR call

Right now we drop the poll loop’s context and always call the API with context.Background(), so cancellations/timeouts aren’t honored by the request. Please pass the provided context through to Create instead.

-	err := wait.PollUntilContextTimeout(context.Background(), interval, timeout, true, func(_ context.Context) (bool, error) {
-		res, err := c.AuthorizationV1().SelfSubjectAccessReviews().Create(context.Background(),
+	err := wait.PollUntilContextTimeout(context.Background(), interval, timeout, true, func(ctx context.Context) (bool, error) {
+		res, err := c.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx,
test/extended/util/resource_op.go (1)

124-132: Use the input length when preallocating the new slice

The helper always allocates capacity 10, forcing extra allocations for larger inputs. Size it to the source slice instead.

-	var newStringsSlice = make([]string, 0, 10)
+	newStringsSlice := make([]string, 0, len(stringsSlice))
test/extended/util/assert.go (1)

36-36: Fix typo in comment.

"ane error" should be "an error".

test/extended/util/template.go (2)

69-81: Eliminate code duplication in resource operations.

The namespace handling logic is duplicated across create and apply branches. Extract the common pattern to reduce duplication.

Apply this diff:

 	var resourceErr error
+	operation := "create"
 	if create {
-		if namespace != "" {
-			resourceErr = oc.AsAdmin().WithoutNamespace().Run("create").Args("-f", configFile, "-n", namespace).Execute()
-		} else {
-			resourceErr = oc.AsAdmin().WithoutNamespace().Run("create").Args("-f", configFile).Execute()
-		}
+		operation = "create"
 	} else {
-		if namespace != "" {
-			resourceErr = oc.AsAdmin().WithoutNamespace().Run("apply").Args("-f", configFile, "-n", namespace).Execute()
-		} else {
-			resourceErr = oc.AsAdmin().WithoutNamespace().Run("apply").Args("-f", configFile).Execute()
-		}
+		operation = "apply"
+	}
+	args := []string{"-f", configFile}
+	if namespace != "" {
+		args = append(args, "-n", namespace)
 	}
+	resourceErr = oc.AsAdmin().WithoutNamespace().Run(operation).Args(args...).Execute()

47-88: Consider cleanup of generated config files.

The function generates temporary config files (line 50) but never cleans them up. While this might be acceptable for test environments, consider adding cleanup logic or documenting that callers are responsible for cleanup.

test/extended/util/pods.go (1)

105-105: Consider extracting the complex jsonpath template to a constant.

The jsonpath template on line 105 is long and complex, making the code harder to read. Extracting it to a package-level constant with a descriptive name would improve maintainability.

For example:

const podReadyStatusTemplate = "'{{- range .items -}}{{- range .status.conditions -}}{{- if ne .reason \"PodCompleted\" -}}{{- if eq .type \"Ready\" -}}{{- .status}} {{\" \"}}{{- end -}}{{- end -}}{{- end -}}{{- end -}}'"

Then use template := podReadyStatusTemplate on line 105.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 6b2a50f and 8568377.

⛔ Files ignored due to path filters (278)
  • go.sum is excluded by !**/*.sum
  • vendor/github.com/JeffAshton/win_pdh/AUTHORS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/JeffAshton/win_pdh/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/JeffAshton/win_pdh/README.mdown is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/JeffAshton/win_pdh/pdh.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/.gitattributes is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/.gitignore is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/.golangci.yml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/CODEOWNERS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/SECURITY.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/backup.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/ea.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/file.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/fileinfo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/hvsock.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/fs/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/fs/fs.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/fs/security.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/fs/zsyscall_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/socket/rawaddr.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/socket/socket.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/socket/zsyscall_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/internal/stringbuffer/wstring.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/pipe.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/pkg/guid/guid.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/pkg/guid/guid_nonwindows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/pkg/guid/guid_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/pkg/guid/variant_string.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/privilege.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/reparse.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/sd.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/syscall.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/go-winio/zsyscall_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/.gitattributes is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/.gitignore is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/CODEOWNERS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/SECURITY.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/errors.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/hns_v1.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/hnsaccelnet.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/hnsglobals.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/hnssupport.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hns.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnsaccelnet.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnsendpoint.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnserrors.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnsfuncs.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnsglobals.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnsnetwork.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnspolicy.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnspolicylist.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/hnssupport.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/namespace.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/hns/zsyscall_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/interop/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/interop/interop.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/Microsoft/hnslib/internal/interop/zsyscall_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/armon/circbuf/.gitignore is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/armon/circbuf/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/armon/circbuf/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/armon/circbuf/circbuf.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/container-storage-interface/spec/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/container-storage-interface/spec/lib/go/csi/csi.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/containers/v1/containers.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/containers/v1/containers.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/containers/v1/containers_grpc.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/containers/v1/containers_ttrpc.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/containers/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/tasks/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks_grpc.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks_ttrpc.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/version/v1/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/version/v1/version.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/version/v1/version.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/version/v1/version_grpc.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/services/version/v1/version_ttrpc.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/descriptor.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/descriptor.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/event.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/event.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/fieldpath.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/fieldpath.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/introspection.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/introspection.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/metrics.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/metrics.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/mount.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/mount.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/platform.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/platform.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/platform_helpers.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/sandbox.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/sandbox.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/task/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/task/task.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/containerd/api/types/task/task.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/errors.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/pkg/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/pkg/errgrpc/grpc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/pkg/internal/cause/cause.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/pkg/internal/types/collapsible.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/errdefs/resolve.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/log/.golangci.yml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/log/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/log/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/log/context.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/.gitattributes is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/.gitignore is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/.golangci.yml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/Makefile is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/PROTOCOL.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/Protobuild.toml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/channel.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/client.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/codec.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/config.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/errors.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/handshake.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/interceptor.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/metadata.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/request.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/request.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/server.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/services.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/stream.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/stream_server.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/test.proto is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/ttrpc/unixcreds_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/typeurl/v2/.gitignore is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/typeurl/v2/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/typeurl/v2/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/typeurl/v2/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/typeurl/v2/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/containerd/typeurl/v2/types_gogo.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/coreos/go-systemd/v22/dbus/dbus.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/coreos/go-systemd/v22/dbus/methods.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/coreos/go-systemd/v22/dbus/properties.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/coreos/go-systemd/v22/dbus/set.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/coreos/go-systemd/v22/dbus/subscription.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/coreos/go-systemd/v22/dbus/subscription_set.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/CHANGELOG.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/VERSION is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/gocompat_errors_go120.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/gocompat_errors_unsupported.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/gocompat_generics_go121.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/gocompat_generics_unsupported.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/join.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/lookup_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/mkdir_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/open_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/openat2_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/openat_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/procfs_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/vfs.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/.gitattributes is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/.gitignore is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/.golangci.yml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/CODE-OF-CONDUCT.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/CONTRIBUTING.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/GOVERNANCE.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/MAINTAINERS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/Makefile is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/SECURITY.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/distribution-logo.svg is excluded by !**/*.svg, !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/helpers.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/normalize.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/reference.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/regexp.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/distribution/reference/sort.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/CONTRIBUTING.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/MAINTAINERS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/circle.yml is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/duration.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/size.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/docker/go-units/ulimit.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/euank/go-kmsg-parser/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/euank/go-kmsg-parser/kmsgparser/kmsgparser.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/euank/go-kmsg-parser/kmsgparser/log.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/CONTRIBUTING.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/MAINTAINERS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/README.md is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/auth.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/auth_anonymous.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/auth_external.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/auth_sha1.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/call.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/conn.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/conn_darwin.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/conn_other.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/conn_unix.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/conn_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/dbus.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/decoder.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/default_handler.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/doc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/encoder.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/escape.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/export.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/homedir.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/match.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/message.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/object.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/sequence.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/sequential_handler.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/server_interfaces.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/sig.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_darwin.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_generic.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_nonce_tcp.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_tcp.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unix.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unixcred_dragonfly.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unixcred_freebsd.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unixcred_linux.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unixcred_netbsd.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unixcred_openbsd.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_zos.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/variant.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/variant_lexer.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/godbus/dbus/v5/variant_parser.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/golang/protobuf/protoc-gen-go/descriptor/descriptor.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/golang/protobuf/ptypes/timestamp/timestamp.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/golang/protobuf/ptypes/wrappers/wrappers.pb.go is excluded by !**/*.pb.go, !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/AUTHORS is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/LICENSE is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/cache/memory/memory.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/collector/collector_manager.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/collector/config.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/collector/fakes.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/collector/generic_collector.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/collector/prometheus_collector.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/collector/types.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/collector/util.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/common/container_hints.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/common/fsHandler.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/common/helpers.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/common/inotify_watcher.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/container.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/client.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/containers/containers.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/factory.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/grpc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/handler.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/identifiers/validate.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/install/install.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/namespaces/context.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/namespaces/grpc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/namespaces/store.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/namespaces/ttrpc.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/pkg/dialer/dialer.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/pkg/dialer/dialer_unix.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/pkg/dialer/dialer_windows.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/containerd/plugin.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/crio/client.go is excluded by !vendor/**, !**/vendor/**
  • vendor/github.com/google/cadvisor/container/crio/factory.go is excluded by !vendor/**, !**/vendor/**
📒 Files selected for processing (19)
  • go.mod (7 hunks)
  • test/extended/ckao.go (1 hunks)
  • test/extended/main.go (0 hunks)
  • test/extended/util/assert.go (1 hunks)
  • test/extended/util/client.go (1 hunks)
  • test/extended/util/clusters.go (1 hunks)
  • test/extended/util/framework.go (1 hunks)
  • test/extended/util/gomega_helpers.go (1 hunks)
  • test/extended/util/gwrapper.go (1 hunks)
  • test/extended/util/logext/log.go (1 hunks)
  • test/extended/util/nodes.go (1 hunks)
  • test/extended/util/oc_copy.go (1 hunks)
  • test/extended/util/pods.go (1 hunks)
  • test/extended/util/provider/config.go (1 hunks)
  • test/extended/util/provider/provider.go (1 hunks)
  • test/extended/util/resource_op.go (1 hunks)
  • test/extended/util/sar.go (1 hunks)
  • test/extended/util/template.go (1 hunks)
  • test/extended/util/test.go (1 hunks)
💤 Files with no reviewable changes (1)
  • test/extended/main.go
🚧 Files skipped from review as they are similar to previous changes (7)
  • test/extended/util/clusters.go
  • test/extended/ckao.go
  • test/extended/util/nodes.go
  • test/extended/util/oc_copy.go
  • test/extended/util/test.go
  • test/extended/util/provider/config.go
  • test/extended/util/client.go
🧰 Additional context used
🪛 OSV Scanner (2.2.3)
go.mod

[HIGH] 135-135: go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful 0.42.0: Memory exhaustion in go.opentelemetry.io/contrib/instrumentation

(GO-2023-2113)


[HIGH] 135-135: go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful 0.42.0: OpenTelemetry-Go Contrib vulnerable to denial of service in otelhttp due to unbound cardinality metrics

(GHSA-rcjv-mgp8-qvmr)

🔇 Additional comments (6)
test/extended/util/provider/provider.go (3)

1-17: LGTM!

Package declaration, imports, and constant definition are appropriate. The blank import ensures cloud provider initialization at vendor time.


19-24: LGTM!

The empty environment variable handling is correct, allowing the e2e framework to default to the skeleton provider.


25-33: LGTM!

The validation logic correctly handles both JSON and plain provider names, with appropriate fallback to known providers from the e2e framework.

test/extended/util/framework.go (1)

58-61: LGTM!

Clean helper function that returns the KUBECONFIG environment variable. The comment explaining why Gomega isn't used is helpful.

test/extended/util/pods.go (1)

26-31: LGTM!

The Create method correctly orchestrates pod creation by building template parameters, delegating to the template helper, and waiting for readiness. The hardcoded NAME parameter requirement is clearly documented in the comment.

test/extended/util/template.go (1)

60-64: Fix inconsistent error handling logic.

When returnError is true, the function should return the error immediately without calling AssertWaitPollNoErr (which would fail the test). Currently, line 64 calls AssertWaitPollNoErr even after line 62 returns the error, making the return statement unreachable when an error occurs.

Apply this diff to fix the inconsistency at both error handling sites (lines 60-64 and 82-86):

-	if returnError && err != nil {
-		e2e.Logf("fail to process %v", parameters)
-		return err
-	}
-	AssertWaitPollNoErr(err, fmt.Sprintf("fail to process %v", parameters))
+	if err != nil {
+		if returnError {
+			e2e.Logf("fail to process %v", parameters)
+			return err
+		}
+		AssertWaitPollNoErr(err, fmt.Sprintf("fail to process %v", parameters))
+	}

Apply the same pattern at lines 82-86 for the resource creation error handling.

Likely an incorrect or invalid review comment.

Comment on lines +26 to +28
if errors.Is(e, context.DeadlineExceeded) ||
strings.Compare(e.Error(), "timed out waiting for the condition") == 0 ||
strings.Compare(e.Error(), "context deadline exceeded") == 0 {
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Replace fragile string comparisons with error type checks.

Line 27 uses string comparison to detect timeout errors, which is brittle if error message formatting changes. The errors.Is check on line 26 is correct, but the string comparisons on lines 27-28 should be replaced with proper error type checking or removed since line 26 already handles context.DeadlineExceeded.

Apply this diff:

 	var err error
-	if errors.Is(e, context.DeadlineExceeded) ||
-		strings.Compare(e.Error(), "timed out waiting for the condition") == 0 ||
-		strings.Compare(e.Error(), "context deadline exceeded") == 0 {
+	if errors.Is(e, context.DeadlineExceeded) ||
+		errors.Is(e, wait.ErrWaitTimeout) {
 		err = fmt.Errorf("case: %v\nerror: %s", g.CurrentSpecReport().FullText(), msg)
 	} else {

If wait.ErrWaitTimeout is not exported, check the k8s.io/apimachinery/pkg/util/wait package for the appropriate sentinel error.

📝 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
if errors.Is(e, context.DeadlineExceeded) ||
strings.Compare(e.Error(), "timed out waiting for the condition") == 0 ||
strings.Compare(e.Error(), "context deadline exceeded") == 0 {
var err error
- if errors.Is(e, context.DeadlineExceeded) ||
- strings.Compare(e.Error(), "timed out waiting for the condition") == 0 ||
if errors.Is(e, context.DeadlineExceeded) ||
errors.Is(e, wait.ErrWaitTimeout) {
err = fmt.Errorf("case: %v\nerror: %s", g.CurrentSpecReport().FullText(), msg)
} else {
🤖 Prompt for AI Agents
In test/extended/util/assert.go around lines 26 to 28, the code uses fragile
string comparisons to detect timeout errors; replace those comparisons with a
proper sentinel error check by adding errors.Is(e, wait.ErrWaitTimeout) (and
keep the existing errors.Is(e, context.DeadlineExceeded) check), and add the
import for "k8s.io/apimachinery/pkg/util/wait" if not present; if
wait.ErrWaitTimeout is unavailable, simply remove the string comparisons so only
errors.Is(e, context.DeadlineExceeded) is used.

Comment on lines +40 to +50
func OrFail[T any](vals ...any) T {

for _, val := range vals {
err, ok := val.(error)
if ok {
o.ExpectWithOffset(1, err).NotTo(o.HaveOccurred())
}
}

return vals[0].(T)
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Add safety checks to prevent panics.

The OrFail function has two critical issues:

  1. Line 49 performs an unsafe type assertion that will panic if vals[0] cannot be cast to type T
  2. No check for empty vals slice, which will panic on line 49

Apply this diff:

 func OrFail[T any](vals ...any) T {
+	o.ExpectWithOffset(1, vals).NotTo(o.BeEmpty(), "OrFail called with no values")
 
 	for _, val := range vals {
 		err, ok := val.(error)
 		if ok {
 			o.ExpectWithOffset(1, err).NotTo(o.HaveOccurred())
 		}
 	}
 
-	return vals[0].(T)
+	result, ok := vals[0].(T)
+	o.ExpectWithOffset(1, ok).To(o.BeTrue(), "OrFail: first value cannot be cast to expected type")
+	return result
 }
📝 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
func OrFail[T any](vals ...any) T {
for _, val := range vals {
err, ok := val.(error)
if ok {
o.ExpectWithOffset(1, err).NotTo(o.HaveOccurred())
}
}
return vals[0].(T)
}
func OrFail[T any](vals ...any) T {
o.ExpectWithOffset(1, vals).NotTo(o.BeEmpty(), "OrFail called with no values")
for _, val := range vals {
err, ok := val.(error)
if ok {
o.ExpectWithOffset(1, err).NotTo(o.HaveOccurred())
}
}
result, ok := vals[0].(T)
o.ExpectWithOffset(1, ok).To(o.BeTrue(), "OrFail: first value cannot be cast to expected type")
return result
}

Comment on lines +24 to +26
waitFn := func(_ context.Context) (bool, error) {
countOutput++
sc, err := c.Get(context.Background(), name, metav1.GetOptions{})
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use the context passed to the wait function.

Line 26 creates a new context.Background() inside the wait function, but the waitFn already receives a context parameter (line 24) that should be used instead. This ensures proper cancellation propagation and timeout handling.

Apply this diff:

-	waitFn := func(_ context.Context) (bool, error) {
+	waitFn := func(ctx context.Context) (bool, error) {
 		countOutput++
-		sc, err := c.Get(context.Background(), name, metav1.GetOptions{})
+		sc, err := c.Get(ctx, name, metav1.GetOptions{})
📝 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
waitFn := func(_ context.Context) (bool, error) {
countOutput++
sc, err := c.Get(context.Background(), name, metav1.GetOptions{})
waitFn := func(ctx context.Context) (bool, error) {
countOutput++
sc, err := c.Get(ctx, name, metav1.GetOptions{})
🤖 Prompt for AI Agents
In test/extended/util/framework.go around lines 24 to 26, the waitFn is creating
a new context with context.Background() instead of using the ctx parameter
passed into the function; replace the call to c.Get(context.Background(), ...)
with c.Get(ctx, ...) so cancellation and timeouts propagate correctly from the
caller to the Get call.

@wangke19
Copy link
Contributor Author

/test k8s-e2e-gcp

@wangke19
Copy link
Contributor Author

/test e2e-aws-ovn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant