[WIP]Add automated e2e test for cert rotation sidecar behavior (OCPBUGS-60628)#1914
Open
wangke19 wants to merge 1 commit intoopenshift:mainfrom
Open
[WIP]Add automated e2e test for cert rotation sidecar behavior (OCPBUGS-60628)#1914wangke19 wants to merge 1 commit intoopenshift:mainfrom
wangke19 wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughA new end-to-end test file introduces a comprehensive test for certificate rotation behavior via a sidecar in Kubernetes environments. The test validates three scenarios: operator downtime without metadata restoration, operator uptime with metadata restoration, and operator downtime with expired certificate triggering rotation. Helper functions manage CVO lifecycle, deployment scaling, and secret annotation patching. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
wangke19
force-pushed
the
ocpbugs-60628-test
branch
from
e0f5dcd to
33b1d97
Compare
wangke19
force-pushed
the
ocpbugs-60628-test
branch
from
33b1d97 to
6b2a50f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
⛔ Files ignored due to path filters (281)
go.sumis excluded by!**/*.sumvendor/github.com/JeffAshton/win_pdh/AUTHORSis excluded by!vendor/**,!**/vendor/**vendor/github.com/JeffAshton/win_pdh/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/JeffAshton/win_pdh/README.mdownis excluded by!vendor/**,!**/vendor/**vendor/github.com/JeffAshton/win_pdh/pdh.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/.gitattributesis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/.golangci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/CODEOWNERSis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/SECURITY.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/backup.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/ea.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/file.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/fileinfo.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/hvsock.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/fs/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/fs/fs.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/fs/security.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/fs/zsyscall_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/socket/rawaddr.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/socket/socket.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/socket/zsyscall_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/stringbuffer/wstring.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/pipe.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/pkg/guid/guid.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/pkg/guid/guid_nonwindows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/pkg/guid/guid_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/pkg/guid/variant_string.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/privilege.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/reparse.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/sd.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/syscall.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/zsyscall_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/.gitattributesis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/CODEOWNERSis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/SECURITY.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/hns_v1.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/hnsaccelnet.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/hnsglobals.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/hnssupport.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hns.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsaccelnet.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsendpoint.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnserrors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsfuncs.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsglobals.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsnetwork.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnspolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnspolicylist.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnssupport.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/namespace.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/zsyscall_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/interop/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/interop/interop.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/interop/zsyscall_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/armon/circbuf/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/armon/circbuf/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/armon/circbuf/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/armon/circbuf/circbuf.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/container-storage-interface/spec/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/container-storage-interface/spec/lib/go/csi/csi.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/containers.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/containers.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/containers_grpc.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/containers_ttrpc.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks_grpc.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks_ttrpc.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/version.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/version.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/version_grpc.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/version_ttrpc.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/descriptor.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/descriptor.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/event.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/event.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/fieldpath.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/fieldpath.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/introspection.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/introspection.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/metrics.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/metrics.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/mount.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/mount.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/platform.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/platform.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/platform_helpers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/sandbox.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/sandbox.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/task/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/task/task.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/task/task.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/pkg/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/pkg/errgrpc/grpc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/pkg/internal/cause/cause.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/pkg/internal/types/collapsible.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/resolve.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/log/.golangci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/log/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/log/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/log/context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/.gitattributesis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/.golangci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/Makefileis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/PROTOCOL.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/Protobuild.tomlis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/channel.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/codec.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/handshake.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/interceptor.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/request.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/request.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/server.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/services.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/stream.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/stream_server.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/test.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/unixcreds_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/typeurl/v2/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/typeurl/v2/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/typeurl/v2/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/typeurl/v2/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/typeurl/v2/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/typeurl/v2/types_gogo.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/dbus.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/methods.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/properties.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/set.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/subscription.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/subscription_set.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/VERSIONis excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/gocompat_errors_go120.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/gocompat_errors_unsupported.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/gocompat_generics_go121.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/gocompat_generics_unsupported.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/join.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/lookup_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/mkdir_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/open_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/openat2_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/openat_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/procfs_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/vfs.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/.gitattributesis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/.golangci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/CODE-OF-CONDUCT.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/CONTRIBUTING.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/GOVERNANCE.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/MAINTAINERSis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/Makefileis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/SECURITY.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/distribution-logo.svgis excluded by!**/*.svg,!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/helpers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/normalize.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/reference.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/regexp.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/sort.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/CONTRIBUTING.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/MAINTAINERSis excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/circle.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/duration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/size.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/ulimit.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/euank/go-kmsg-parser/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/euank/go-kmsg-parser/kmsgparser/kmsgparser.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/euank/go-kmsg-parser/kmsgparser/log.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/CONTRIBUTING.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/MAINTAINERSis excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/auth.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/auth_anonymous.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/auth_external.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/auth_sha1.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/call.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/conn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/conn_darwin.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/conn_other.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/conn_unix.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/conn_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/dbus.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/decoder.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/default_handler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/encoder.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/escape.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/export.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/homedir.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/match.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/message.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/object.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/sequence.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/sequential_handler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/server_interfaces.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/sig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_darwin.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_generic.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_nonce_tcp.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_tcp.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_unix.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_unixcred_dragonfly.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_unixcred_freebsd.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_unixcred_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_unixcred_netbsd.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_unixcred_openbsd.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_zos.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/variant.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/variant_lexer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/variant_parser.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/golang/protobuf/protoc-gen-go/descriptor/descriptor.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/golang/protobuf/ptypes/timestamp/timestamp.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/golang/protobuf/ptypes/wrappers/wrappers.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/AUTHORSis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/cache/memory/memory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/collector/collector_manager.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/collector/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/collector/fakes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/collector/generic_collector.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/collector/prometheus_collector.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/collector/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/collector/util.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/common/container_hints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/common/fsHandler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/common/helpers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/common/inotify_watcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/container.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/containers/containers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/factory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/grpc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/handler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/identifiers/validate.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/install/install.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/namespaces/context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/namespaces/grpc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/namespaces/store.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/namespaces/ttrpc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/pkg/dialer/dialer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/pkg/dialer/dialer_unix.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/pkg/dialer/dialer_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/plugin.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/crio/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/crio/factory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/crio/handler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/crio/install/install.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/crio/plugin.gois 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
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()) |
There was a problem hiding this comment.
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) | ||
| } |
There was a problem hiding this comment.
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) | ||
| } |
There was a problem hiding this comment.
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, "") | ||
| } |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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.
6b2a50f to
8568377
Compare
There was a problem hiding this comment.
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/otelrestful@v0.42.0carries 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 // indirecttest/extended/util/provider/provider.go (1)
35-52: Critical issue already flagged: nil pointer dereference on JSONnull.The past review correctly identifies that passing
&providerInfo(pointer-to-pointer) tojson.Unmarshalallows JSONnullto setproviderInfotonil, causing a panic at line 41 when dereferencingproviderInfo.ProviderName. Additionally, an emptyProviderNamefrom 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 selectorKeys like
app.kubernetes.io/namecurrently 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 unescapedfilter– CRITICAL shell injection vulnerability.Lines 77, 82, and 84 construct and execute a shell command with an unescaped
filterparameter, allowing arbitrary command injection. This is the same critical security issue previously flagged. Any caller passing a filter likefoo; 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"), nilNote: This implementation also renders the
combinedparameter obsolete since we're reading from a file rather than capturing command output. Ifcombinedstderr/stdout is required, it should be handled when writing topodLogs.txt(line 70), not during filtering.
🧹 Nitpick comments (6)
test/extended/util/sar.go (1)
16-21: Propagate the poll context to the SelfSAR callRight 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 toCreateinstead.- 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 sliceThe 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 := podReadyStatusTemplateon 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
⛔ Files ignored due to path filters (278)
go.sumis excluded by!**/*.sumvendor/github.com/JeffAshton/win_pdh/AUTHORSis excluded by!vendor/**,!**/vendor/**vendor/github.com/JeffAshton/win_pdh/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/JeffAshton/win_pdh/README.mdownis excluded by!vendor/**,!**/vendor/**vendor/github.com/JeffAshton/win_pdh/pdh.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/.gitattributesis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/.golangci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/CODEOWNERSis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/SECURITY.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/backup.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/ea.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/file.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/fileinfo.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/hvsock.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/fs/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/fs/fs.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/fs/security.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/fs/zsyscall_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/socket/rawaddr.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/socket/socket.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/socket/zsyscall_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/internal/stringbuffer/wstring.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/pipe.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/pkg/guid/guid.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/pkg/guid/guid_nonwindows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/pkg/guid/guid_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/pkg/guid/variant_string.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/privilege.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/reparse.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/sd.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/syscall.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/go-winio/zsyscall_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/.gitattributesis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/CODEOWNERSis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/SECURITY.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/hns_v1.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/hnsaccelnet.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/hnsglobals.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/hnssupport.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hns.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsaccelnet.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsendpoint.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnserrors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsfuncs.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsglobals.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnsnetwork.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnspolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnspolicylist.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/hnssupport.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/namespace.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/hns/zsyscall_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/interop/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/interop/interop.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Microsoft/hnslib/internal/interop/zsyscall_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/armon/circbuf/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/armon/circbuf/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/armon/circbuf/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/armon/circbuf/circbuf.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/container-storage-interface/spec/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/container-storage-interface/spec/lib/go/csi/csi.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/containers.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/containers.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/containers_grpc.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/containers_ttrpc.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/containers/v1/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks_grpc.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/tasks/v1/tasks_ttrpc.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/version.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/version.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/version_grpc.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/services/version/v1/version_ttrpc.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/descriptor.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/descriptor.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/event.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/event.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/fieldpath.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/fieldpath.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/introspection.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/introspection.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/metrics.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/metrics.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/mount.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/mount.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/platform.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/platform.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/platform_helpers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/sandbox.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/sandbox.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/task/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/task/task.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/containerd/api/types/task/task.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/pkg/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/pkg/errgrpc/grpc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/pkg/internal/cause/cause.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/pkg/internal/types/collapsible.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/errdefs/resolve.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/log/.golangci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/log/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/log/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/log/context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/.gitattributesis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/.golangci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/Makefileis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/PROTOCOL.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/Protobuild.tomlis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/channel.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/codec.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/handshake.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/interceptor.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/request.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/request.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/server.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/services.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/stream.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/stream_server.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/test.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/ttrpc/unixcreds_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/typeurl/v2/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/typeurl/v2/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/typeurl/v2/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/typeurl/v2/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/typeurl/v2/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/containerd/typeurl/v2/types_gogo.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/dbus.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/methods.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/properties.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/set.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/subscription.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/subscription_set.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/VERSIONis excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/gocompat_errors_go120.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/gocompat_errors_unsupported.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/gocompat_generics_go121.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/gocompat_generics_unsupported.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/join.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/lookup_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/mkdir_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/open_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/openat2_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/openat_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/procfs_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cyphar/filepath-securejoin/vfs.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/.gitattributesis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/.golangci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/CODE-OF-CONDUCT.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/CONTRIBUTING.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/GOVERNANCE.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/MAINTAINERSis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/Makefileis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/SECURITY.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/distribution-logo.svgis excluded by!**/*.svg,!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/helpers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/normalize.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/reference.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/regexp.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/distribution/reference/sort.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/CONTRIBUTING.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/MAINTAINERSis excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/circle.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/duration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/size.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/docker/go-units/ulimit.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/euank/go-kmsg-parser/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/euank/go-kmsg-parser/kmsgparser/kmsgparser.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/euank/go-kmsg-parser/kmsgparser/log.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/CONTRIBUTING.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/MAINTAINERSis excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/auth.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/auth_anonymous.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/auth_external.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/auth_sha1.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/call.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/conn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/conn_darwin.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/conn_other.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/conn_unix.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/conn_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/dbus.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/decoder.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/default_handler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/encoder.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/escape.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/export.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/homedir.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/match.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/message.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/object.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/sequence.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/sequential_handler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/server_interfaces.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/sig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_darwin.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_generic.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_nonce_tcp.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_tcp.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_unix.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_unixcred_dragonfly.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_unixcred_freebsd.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_unixcred_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_unixcred_netbsd.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_unixcred_openbsd.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/transport_zos.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/variant.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/variant_lexer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/godbus/dbus/v5/variant_parser.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/golang/protobuf/protoc-gen-go/descriptor/descriptor.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/golang/protobuf/ptypes/timestamp/timestamp.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/golang/protobuf/ptypes/wrappers/wrappers.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/AUTHORSis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/cache/memory/memory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/collector/collector_manager.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/collector/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/collector/fakes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/collector/generic_collector.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/collector/prometheus_collector.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/collector/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/collector/util.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/common/container_hints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/common/fsHandler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/common/helpers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/common/inotify_watcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/container.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/containers/containers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/factory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/grpc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/handler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/identifiers/validate.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/install/install.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/namespaces/context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/namespaces/grpc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/namespaces/store.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/namespaces/ttrpc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/pkg/dialer/dialer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/pkg/dialer/dialer_unix.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/pkg/dialer/dialer_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/containerd/plugin.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/crio/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/cadvisor/container/crio/factory.gois 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
🔇 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
Createmethod correctly orchestrates pod creation by building template parameters, delegating to the template helper, and waiting for readiness. The hardcodedNAMEparameter requirement is clearly documented in the comment.test/extended/util/template.go (1)
60-64: Fix inconsistent error handling logic.When
returnErroris true, the function should return the error immediately without callingAssertWaitPollNoErr(which would fail the test). Currently, line 64 callsAssertWaitPollNoErreven 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 { |
There was a problem hiding this comment.
🛠️ 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) | ||
| } |
There was a problem hiding this comment.
Add safety checks to prevent panics.
The OrFail function has two critical issues:
- Line 49 performs an unsafe type assertion that will panic if
vals[0]cannot be cast to typeT - No check for empty
valsslice, 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{}) |
There was a problem hiding this comment.
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.
Contributor
Author
|
/test k8s-e2e-gcp |
Contributor
Author
|
/test e2e-aws-ovn |
wangke19
changed the title
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
openshift-merge-robot
added
the
needs-rebase
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/e2e/cert_rotation_sidecar_test.go (2)
139-140: Annotation existence check should use comma-ok idiom.Checking
ann["key"] != ""differs from checking if the key exists. If the annotation exists with an empty value, this would incorrectly reportfalse.- hasComponent := ann["auth.openshift.io/component"] != "" - hasDescription := ann["auth.openshift.io/description"] != "" + _, hasComponent := ann["auth.openshift.io/component"] + _, hasDescription := ann["auth.openshift.io/description"]
253-268: Consider usingjson.Marshalfor safer JSON construction.Building JSON via string concatenation is fragile and could break with special characters in annotation keys/values. While it works for the current hardcoded annotations, using
encoding/jsonwould be more robust.import "encoding/json" func patchAnnotationsOnSecret(t *testing.T, ctx context.Context, kubeClient *kubernetes.Clientset, namespace, name string, annotations map[string]*string) { patchData := map[string]interface{}{ "metadata": map[string]interface{}{ "annotations": annotations, }, } patch, err := json.Marshal(patchData) require.NoError(t, err) _, err = kubeClient.CoreV1().Secrets(namespace).Patch(ctx, name, types.MergePatchType, patch, metav1.PatchOptions{}) require.NoError(t, err) }
📜 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
📒 Files selected for processing (1)
test/e2e/cert_rotation_sidecar_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
test/e2e/cert_rotation_sidecar_test.go
🧬 Code graph analysis (1)
test/e2e/cert_rotation_sidecar_test.go (1)
test/library/client.go (1)
NewClientConfigForTest(12-20)
🔇 Additional comments (2)
test/e2e/cert_rotation_sidecar_test.go (2)
154-197: LGTM!The logic correctly waits for the sidecar to rotate the certificate when the
not-afterannotation indicates expiry.
199-217: LGTM!The CVO pause/resume pattern is appropriate. Using
require.NoErrorfor pause and a warning log for resume cleanup is correct behavior.
Comment on lines
+96
to
+115
| err = wait.PollImmediate(10*time.Second, 2*time.Minute, func() (bool, error) { | ||
| s, err := kubeClient.CoreV1().Secrets(targetNamespace).Get(ctx, secretName, metav1.GetOptions{}) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| ann := s.Annotations | ||
| _, hasComponent := ann["auth.openshift.io/component"] | ||
| _, hasDescription := ann["auth.openshift.io/description"] | ||
| dataNow := base64.StdEncoding.EncodeToString(s.Data["tls.crt"]) | ||
|
|
||
| // If annotations are present or cert changed, fail | ||
| if hasComponent || hasDescription || dataNow != dataBefore { | ||
| return false, fmt.Errorf("unexpected change: hasComponent=%v, hasDescription=%v, certChanged=%v", | ||
| hasComponent, hasDescription, dataNow != dataBefore) | ||
| } | ||
|
|
||
| return true, nil | ||
| }) | ||
| require.NoError(t, err, "annotations should not be re-added and cert should not rotate when operator is down") |
There was a problem hiding this comment.
Logic bug: Poll succeeds immediately without verifying persistence over time.
The poll returns true, nil on the first check when annotations are absent and cert is unchanged. Since annotations were just removed, this condition is immediately true, and the poll succeeds without actually verifying that the sidecar doesn't restore them over time.
To test that annotations remain removed, consider one of these approaches:
- Use a
time.Sleepto wait a reasonable period, then check once - Invert the poll logic to wait for the undesired state and treat timeout as success
Example fix using approach 2 (timeout-as-success):
- // Verify that annotations are not re-added and cert is not rotated
- err = wait.PollImmediate(10*time.Second, 2*time.Minute, func() (bool, error) {
+ // Wait and verify annotations are not re-added and cert is not rotated
+ // We expect this poll to timeout (which indicates success - no changes occurred)
+ err = wait.PollImmediate(10*time.Second, 2*time.Minute, func() (bool, error) {
s, err := kubeClient.CoreV1().Secrets(targetNamespace).Get(ctx, secretName, metav1.GetOptions{})
if err != nil {
return false, err
}
ann := s.Annotations
_, hasComponent := ann["auth.openshift.io/component"]
_, hasDescription := ann["auth.openshift.io/description"]
dataNow := base64.StdEncoding.EncodeToString(s.Data["tls.crt"])
- // If annotations are present or cert changed, fail
+ // If annotations are present or cert changed, fail immediately
if hasComponent || hasDescription || dataNow != dataBefore {
return false, fmt.Errorf("unexpected change: hasComponent=%v, hasDescription=%v, certChanged=%v",
hasComponent, hasDescription, dataNow != dataBefore)
}
- return true, nil
+ // Keep polling - we want to verify state remains unchanged
+ return false, nil
})
- require.NoError(t, err, "annotations should not be re-added and cert should not rotate when operator is down")
+ // Timeout is expected (means no changes occurred during the wait period)
+ if err != nil && !wait.Interrupted(err) {
+ require.NoError(t, err, "annotations should not be re-added and cert should not rotate when operator is down")
+ }📝 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
| err = wait.PollImmediate(10*time.Second, 2*time.Minute, func() (bool, error) { | |
| s, err := kubeClient.CoreV1().Secrets(targetNamespace).Get(ctx, secretName, metav1.GetOptions{}) | |
| if err != nil { | |
| return false, err | |
| } | |
| ann := s.Annotations | |
| _, hasComponent := ann["auth.openshift.io/component"] | |
| _, hasDescription := ann["auth.openshift.io/description"] | |
| dataNow := base64.StdEncoding.EncodeToString(s.Data["tls.crt"]) | |
| // If annotations are present or cert changed, fail | |
| if hasComponent || hasDescription || dataNow != dataBefore { | |
| return false, fmt.Errorf("unexpected change: hasComponent=%v, hasDescription=%v, certChanged=%v", | |
| hasComponent, hasDescription, dataNow != dataBefore) | |
| } | |
| return true, nil | |
| }) | |
| require.NoError(t, err, "annotations should not be re-added and cert should not rotate when operator is down") | |
| err = wait.PollImmediate(10*time.Second, 2*time.Minute, func() (bool, error) { | |
| s, err := kubeClient.CoreV1().Secrets(targetNamespace).Get(ctx, secretName, metav1.GetOptions{}) | |
| if err != nil { | |
| return false, err | |
| } | |
| ann := s.Annotations | |
| _, hasComponent := ann["auth.openshift.io/component"] | |
| _, hasDescription := ann["auth.openshift.io/description"] | |
| dataNow := base64.StdEncoding.EncodeToString(s.Data["tls.crt"]) | |
| // If annotations are present or cert changed, fail immediately | |
| if hasComponent || hasDescription || dataNow != dataBefore { | |
| return false, fmt.Errorf("unexpected change: hasComponent=%v, hasDescription=%v, certChanged=%v", | |
| hasComponent, hasDescription, dataNow != dataBefore) | |
| } | |
| // Keep polling - we want to verify state remains unchanged over time | |
| return false, nil | |
| }) | |
| // Timeout is expected (means no changes occurred during the wait period) | |
| if err != nil && err != wait.ErrWaitTimeout { | |
| require.NoError(t, err, "annotations should not be re-added and cert should not rotate when operator is down") | |
| } |
🤖 Prompt for AI Agents
In test/e2e/cert_rotation_sidecar_test.go around lines 96 to 115, the poll
currently returns success immediately when annotations are absent and cert
unchanged, so it doesn't verify persistence; change the polling to wait for the
undesired state instead and treat a timeout as success: implement the poll
callback to return true (stop) if hasComponent || hasDescription || cert changed
(i.e., the sidecar restored annotations or rotated the cert) and return
false,nil otherwise; after wait.PollImmediate completes, assert that the
returned error is a timeout error (meaning the undesired state never appeared)
and treat that as success, failing the test only if the poll found the undesired
state within the timeout.
a1a578b to
6427d20
Compare
openshift-merge-robot
removed
the
needs-rebase
openshift-ci
Bot
added
the
approved
wangke19
changed the title
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
test/e2e/cert_rotation_sidecar_test.go (1)
96-115: Logic bug: Poll succeeds immediately without verifying persistence.The poll returns
true, nilon the first successful check when annotations are absent and cert is unchanged. Since annotations were just removed, this passes immediately without verifying that the sidecar doesn't restore them over time.The fix from the previous review should be applied: invert the poll to wait for the undesired state and treat timeout as success.
🧹 Nitpick comments (2)
test/e2e/cert_rotation_sidecar_test.go (2)
184-192: Consider simplifying the rotation check.The
notAfter == ""check on line 184 is potentially confusing since you just setnot-afterto a past date. The actual indicator of successful rotation is the cert data change. Consider whether this check is necessary or if it could be clarified with a comment explaining what state you're expecting (e.g., waiting for the sidecar to update the annotation with the new cert's expiry).
250-272: Consider usingjson.Marshalfor safer JSON construction.The manual JSON string building works for this use case but is fragile if annotation keys or values contain special characters. Using
json.Marshalwould be safer.+import "encoding/json" + // patchAnnotationsOnSecret patches annotations on a Secret. // Pass nil values in the map to remove annotations. func patchAnnotationsOnSecret(t *testing.T, ctx context.Context, kubeClient *kubernetes.Clientset, namespace, name string, annotations map[string]*string) { - patch := `{"metadata":{"annotations":{` - first := true - - for k, v := range annotations { - if !first { - patch += "," - } - if v == nil { - patch += fmt.Sprintf("%q:null", k) - } else { - patch += fmt.Sprintf("%q:%q", k, *v) - } - first = false - } - - patch += "}}}" - - _, err := kubeClient.CoreV1().Secrets(namespace).Patch(ctx, name, types.MergePatchType, []byte(patch), metav1.PatchOptions{}) + patchData := map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": annotations, + }, + } + patch, err := json.Marshal(patchData) + require.NoError(t, err) + + _, err = kubeClient.CoreV1().Secrets(namespace).Patch(ctx, name, types.MergePatchType, patch, metav1.PatchOptions{}) require.NoError(t, err) }
📜 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
📒 Files selected for processing (1)
test/e2e/cert_rotation_sidecar_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
test/e2e/cert_rotation_sidecar_test.go
🧬 Code graph analysis (1)
test/e2e/cert_rotation_sidecar_test.go (1)
test/library/client.go (1)
NewClientConfigForTest(12-20)
🔇 Additional comments (5)
test/e2e/cert_rotation_sidecar_test.go (5)
1-30: LGTM!The imports are appropriate for the e2e test, and the constants are well-organized and clearly named.
32-74: LGTM!The test structure is well-organized with proper CVO lifecycle management. The defer pattern ensures cleanup happens even if tests fail.
118-152: LGTM!The polling logic correctly waits for the operator to restore metadata while verifying the certificate remains unchanged.
199-217: LGTM!The CVO pause/resume helpers correctly manage the operator lifecycle with appropriate override patches. The asymmetric error handling (fail on pause, warn on resume) is reasonable for cleanup scenarios.
219-248: LGTM!The deployment scaling helper correctly patches replicas and waits for both the spec and ready replica count to match the expected value.
openshift-ci
Bot
removed
the
approved
wangke19
force-pushed
the
ocpbugs-60628-test
branch
from
8eff427 to
6e4dc97
Compare
openshift-merge-robot
added
the
needs-rebase
wangke19
force-pushed
the
ocpbugs-60628-test
branch
from
6e4dc97 to
1310630
Compare
openshift-merge-robot
removed
the
needs-rebase
openshift-ci
Bot
added
the
approved
wangke19
force-pushed
the
ocpbugs-60628-test
branch
from
1310630 to
f581621
Compare
Contributor
Author
|
/retest |
…628) This commit adds a new automated e2e test that validates the cert rotation sidecar behavior when certificates expire and the operator is scaled down. Test Scenarios: - Scenario A: When operator is down, sidecar should not restore metadata annotations - Scenario B: When operator is up, metadata is restored without cert rotation - Scenario C: When operator is down and cert expires, sidecar rotates the certificate The test follows the OTE (OpenShift Tests Extension) framework pattern used in test/e2e/, using standard Go testing with testify/require for assertions. Changes: - Add test/e2e/cert_rotation_sidecar_test.go: New automated test for OCPBUGS-60628 - Remove test/extended/ckao.go: Prototype/draft test code Related: OCPBUGS-60628 Migrate cert rotation sidecar test to OTE framework Convert test/e2e/cert_rotation_sidecar_test.go to Ginkgo format for OpenShift Tests Extension (OTE) compatibility. Changes: - Rename cert_rotation_sidecar_test.go -> cert_rotation_sidecar.go (non-test file required for OTE test discovery) - Convert from standard Go testing to Ginkgo/Gomega - Add [Operator][Serial] labels for suite filtering - Create dependencymagnet.go to register test package - Fix deployment name (kube-apiserver-operator) - Update test logic: - Test 1: Verify metadata not restored when operator down - Test 2: Verify metadata restored when operator up - Test 3: Verify cert rotation on expiry by sidecar - Use auth.openshift.io/certificate-not-after for cert expiry Tests can be executed with: ./cluster-kube-apiserver-operator-tests-ext run-suite \ openshift/cluster-kube-apiserver-operator/operator/serial -c 1 Related: OCPBUGS-60628 Fix cert rotation sidecar test annotation keys Use correct annotation keys that match the cert rotation controller: - openshift.io/owning-component (not auth.openshift.io/component) - openshift.io/description (not auth.openshift.io/description) This fixes test timeout issues where annotations were never restored because the test was checking for non-existent annotation keys. Test results: - Before: Test 2 timed out after 3 minutes - After: Test 2 passes in 21 seconds Related: OCPBUGS-60628
wangke19
force-pushed
the
ocpbugs-60628-test
branch
from
f581621 to
6fdf035
Compare
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wangke19 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
openshift-ci
Bot
removed
the
approved
openshift-ci
Bot
added
the
needs-rebase
Contributor
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Contributor
|
@wangke19: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR migrates the cert rotation sidecar e2e test to the OpenShift Tests Extension (OTE) framework for better integration with OpenShift CI/CD pipelines.
Changes
Test Migration to OTE/Ginkgo Format
Converted to Ginkgo/Gomega syntax:
testing.Tto GinkgoDescribe()/It()blocksrequire.NoError()withgomega.Expect().NotTo(gomega.HaveOccurred())ginkgo.BeforeEach()and cleanup toginkgo.AfterEach()ginkgo.By()for test step descriptionsAdded OTE labels:
[Operator][Serial]cmd/cluster-kube-apiserver-operator-tests-ext/main.goNew Files
cmd/cluster-kube-apiserver-operator-tests-ext/dependencymagnet.goTest Coverage
The migrated tests verify cert rotation sidecar behavior:
Testing
Build the test binary:
List tests:
Run tests (serial execution):
export KUBECONFIG=/path/to/kubeconfig ./cluster-kube-apiserver-operator-tests-ext run-suite openshift/cluster-kube-apiserver-operator/operator/serial -c 1Expected Results:
Related
Notes
-c 1) to avoid conflicts when pausing/resuming CVO1inmain.go, but-c 1flag is still recommended for reliability