CNTRLPLANE-2947: Enable configurable PKI for signing CA and serving certificates#327
Merged
Merged
Conversation
Contributor
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughImplements configurable PKI support via feature gates by introducing a Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Line 89: Update the OpenTelemetry SDK dependency from
go.opentelemetry.io/otel/sdk v1.36.0 to v1.40.0 (or later) in the module
declaration so the vulnerable version is replaced; then run go mod tidy (and
optionally go mod vendor) to update the go.sum and lock the new version. Locate
the dependency string "go.opentelemetry.io/otel/sdk v1.36.0" in go.mod and
change the version to "v1.40.0" or newer, and verify builds/tests pass to
confirm the update succeeded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8f0f072b-039b-4cc9-89f5-4eebef921fae
⛔ Files ignored due to path filters (299)
go.sumis excluded by!**/*.sumvendor/cel.dev/expr/.bazelversionis excluded by!**/vendor/**,!vendor/**vendor/cel.dev/expr/MODULE.bazelis excluded by!**/vendor/**,!vendor/**vendor/cel.dev/expr/cloudbuild.yamlis excluded by!**/vendor/**,!vendor/**vendor/cel.dev/expr/eval.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/arrays.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/converter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/numerics.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/patterns.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/validator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/wercker.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/.cirrus.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_fen.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_inotify.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_kqueue.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_other.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/fsnotify.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/darwin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/freebsd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/shared.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/staticcheck.confis excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/bytestring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/cache.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/common.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/encode_map.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/encode_map_go117.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/omitzero_go124.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/omitzero_pre_go124.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/simplevalue.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/stream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/structfields.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/tag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/.golangci.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/funcr/funcr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/sortkeys/sortkeys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/context.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/extensions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/helpers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/reader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/jsonschema/models.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/jsonschema/reader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/jsonschema/writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv2/OpenAPIv2.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv2/document.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv3/OpenAPIv3.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv3/document.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/bytesource/bytesource.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/fuzz.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/BUILD.bazelis excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/annotations.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/annotations.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/annotations_protoopaque.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/buf.gen.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2_protoopaque.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/.gitattributesis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/.goreleaser.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/compressible.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/bitreader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/bitwriter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/bytereader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/compress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/decompress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/fse.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/gen.shis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/bitreader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/bitwriter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/compress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/decompress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/decompress_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/decompress_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/decompress_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/huff0.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/cpuinfo/cpuinfo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/cpuinfo/cpuinfo_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/cpuinfo/cpuinfo_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/le/le.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/le/unsafe_disabled.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/le/unsafe_enabled.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/decode_other.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/encode_other.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/snappy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/s2sx.modis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/s2sx.sumis excluded by!**/*.sum,!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/bitreader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/bitwriter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/blockdec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/blockenc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/blocktype_string.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/bytebuf.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/bytereader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/decodeheader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/decoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/decoder_options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/dict.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_base.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_best.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_better.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_dfast.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_fast.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/encoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/encoder_options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/framedec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/frameenc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_encoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_predefined.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/hash.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/history.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_arm64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_asm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_other.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_safe.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/matchlen_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/matchlen_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/matchlen_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqenc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/snappy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/zip.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/zstd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/modern-go/reflect2/safe_type.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gomega_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/async_assertion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_comparable_to_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_yaml_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.ci-operator.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.coderabbit.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.golangci.go-validated.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.golangci.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/AGENTS.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/Dockerfile.ocpis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/install.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/types_compatibilityrequirement.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apiextensions/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apiextensions/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apiserver/v1/types_apirequestcount.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiserver/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apps/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apps/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apps/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apps/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apps/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apps/v1/zz_prerelease_lifecycle_generated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/build/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/build/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/build/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/build/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/build/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/cloudnetwork/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/cloudnetwork/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/cloudnetwork/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/cloudnetwork/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/cloudnetwork/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/install.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_operator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_console.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_feature.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_kmsencryption.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_operatorhub.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_scheduling.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_tlssecurityprofile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_backup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_crio_credential_provider_config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_pki.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha2/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha2/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha2/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/console/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/types_console_cli_download.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/types_console_link.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/types_console_plugin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/types_console_sample.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/console/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/console/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/envtest-releases.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/install.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/types_pacemakercluster.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/etcd/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/etcd/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/features.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/legacyfeaturegates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/helm/v1beta1/types_helm_chart_repository.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/helm/v1beta1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/image/docker10/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/image/dockerpre012/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/image/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/image/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/imageregistry/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/imageregistry/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/imageregistry/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/install.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/kubecontrolplane/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/legacyconfig/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/legacyconfig/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/legacyconfig/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/machine/v1/types_alibabaprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/machine/v1/types_controlplanemachineset.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/machine/v1/types_nutanixprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/machine/v1/types_powervsprovider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/machine/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*
📒 Files selected for processing (1)
go.mod
| go.opentelemetry.io/otel/sdk v1.35.0 // indirect | ||
| go.opentelemetry.io/otel/trace v1.35.0 // indirect | ||
| go.opentelemetry.io/otel/metric v1.36.0 // indirect | ||
| go.opentelemetry.io/otel/sdk v1.36.0 // indirect |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for available patched versions and vulnerability details
echo "=== Checking for security advisories ==="
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: GO, package: "go.opentelemetry.io/otel/sdk") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
echo -e "\n=== Checking available versions ==="
go list -m -versions go.opentelemetry.io/otel/sdk 2>/dev/null || echo "Module info not available in current context"Repository: openshift/service-ca-operator
Length of output: 355
🌐 Web query:
GO-2026-4394 OpenTelemetry SDK vulnerability
💡 Result:
GO-2026-4394 is a local arbitrary code execution vulnerability in the OpenTelemetry Go SDK caused by PATH hijacking (CWE-426) on macOS/Darwin. The SDK’s resource detection code (sdk/resource/host_id.go) executes the ioreg command via a search path; if an attacker can influence the process PATH, they may get a malicious ioreg executed in the application’s context. It’s tracked as CVE-2026-24051 / GHSA-9h8m-3fm2-qjrq. (github.com)
- Affected:
go.opentelemetry.io/otel/sdk(and.../sdk/resource) >= 1.21.0 and < 1.40.0 (GitHub advisory lists <= 1.39.0). (osv.dev) - Fixed: 1.40.0 (patch commit
d45961b...released in v1.40.0). (github.com)
What to do: upgrade OpenTelemetry Go SDK to v1.40.0+; if you can’t immediately, reduce exposure by ensuring the running environment’s PATH cannot be modified by untrusted users/processes (the issue is fundamentally a local environment/path control problem). (github.com)
Citations:
- 1: GHSA-9h8m-3fm2-qjrq
- 2: https://osv.dev/vulnerability/GO-2026-4394
- 3: GHSA-9h8m-3fm2-qjrq
- 4: GHSA-9h8m-3fm2-qjrq
🌐 Web query:
GHSA-9h8m-3fm2-qjrq security advisory
💡 Result:
GHSA-9h8m-3fm2-qjrq (aka CVE-2026-24051, Go vuln GO-2026-4394) is a PATH hijacking issue in the OpenTelemetry Go SDK (go.opentelemetry.io/otel/sdk) on macOS/Darwin: resource detection runs the ioreg command via the system PATH, so a local attacker who can influence PATH can potentially achieve arbitrary code execution in the application’s context. [1][2]
- Affected versions:
>= 1.21.0, < 1.40.0[1][3] - Fixed version:
1.40.0(patch commitd45961b…) [1][2] - Severity (GitHub CNA): CVSS 3.1 7.0 (High) [1][2]
What to do: upgrade go.opentelemetry.io/otel/sdk to v1.40.0 or later. [1][3]
Sources
[1] GitHub Advisory Database (GHSA-9h8m-3fm2-qjrq) (github.com)
[2] NVD entry for CVE-2026-24051 (nvd.nist.gov)
[3] Go vulnerability report GO-2026-4394 (pkg.go.dev) (pkg.go.dev)
Citations:
- 1: GHSA-9h8m-3fm2-qjrq
- 2: https://nvd.nist.gov/vuln/detail/CVE-2026-24051
- 3: https://pkg.go.dev/vuln/GO-2026-4394
🌐 Web query:
go.opentelemetry.io/otel/sdk v1.36.0 security vulnerability
💡 Result:
Yes—go.opentelemetry.io/otel/sdk v1.36.0 falls in the affected range for GO-2026-4394 / CVE-2026-24051 / GHSA-9h8m-3fm2-qjrq (introduced 1.21.0, fixed 1.40.0). This is a local arbitrary code execution risk on macOS/Darwin due to PATH hijacking when the SDK’s resource host-id detection runs ioreg via an untrusted search path. The fix is to use the full path to ioreg (/usr/sbin/ioreg). [1][2][3]
What to do: upgrade to go.opentelemetry.io/otel/sdk v1.40.0 or later. [1][2]
Sources: [1] [2] [3]
Upgrade OpenTelemetry SDK to v1.40.0 or later to address HIGH severity security vulnerability.
go.opentelemetry.io/otel/sdk v1.36.0 is vulnerable to GO-2026-4394 / GHSA-9h8m-3fm2-qjrq (CVE-2026-24051), a PATH hijacking vulnerability that enables local arbitrary code execution on macOS/Darwin. The SDK's resource detection code runs the ioreg command via the system PATH instead of its absolute path, allowing code execution if PATH can be influenced. Upgrade to v1.40.0 or later to resolve this.
🧰 Tools
🪛 OSV Scanner (2.3.3)
[HIGH] 89-89: go.opentelemetry.io/otel/sdk 1.36.0: OpenTelemetry Go SDK Vulnerable to Arbitrary Code Execution via PATH Hijacking in go.opentelemetry.io/otel/sdk
(GO-2026-4394)
[HIGH] 89-89: go.opentelemetry.io/otel/sdk 1.36.0: OpenTelemetry Go SDK Vulnerable to Arbitrary Code Execution via PATH Hijacking
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` at line 89, Update the OpenTelemetry SDK dependency from
go.opentelemetry.io/otel/sdk v1.36.0 to v1.40.0 (or later) in the module
declaration so the vulnerable version is replaced; then run go mod tidy (and
optionally go mod vendor) to update the go.sum and lock the new version. Locate
the dependency string "go.opentelemetry.io/otel/sdk v1.36.0" in go.mod and
change the version to "v1.40.0" or newer, and verify builds/tests pass to
confirm the update succeeded.
sanchezl
changed the title
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@sanchezl: This pull request references CNTRLPLANE-2947 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Contributor
|
@sanchezl: This pull request references CNTRLPLANE-2947 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
go.mod (1)
3-3: Consider keepinggoat the real minimum and usingtoolchainfor 1.25.Since Go 1.21, the
goline is the module’s enforced minimum version, whiletoolchainis the knob for preferring a newer compiler when working in the repo. If this PR doesn’t actually require Go 1.25-only features, raisinggohere will unnecessarily force every consumer/CI image onto 1.25. (tip.golang.org)Suggested change
-go 1.25.0 +go 1.24.0 +toolchain go1.25.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 3, The go.mod currently sets "go 1.25.0" which raises the enforced minimum; change it back to the real minimum required (e.g., "go 1.20" or the actual lowest supported version) and instead add a // toolchain directive to prefer Go 1.25 for development/CI. Update the go.mod by lowering the "go" version to the true minimum and adding a toolchain comment like "// toolchain: go1.25" (or equivalent) so consumers are not forced to use 1.25 while your repo prefers that compiler.pkg/controller/servingcert/starter/starter.go (1)
125-126: RedundantconfigInformers.Start()call.
configInformers.Start(stopChan)is called on line 58 and again on line 126. The second call is redundant since informers are already started.kubeInformers.Start(stopChan) - configInformers.Start(stopChan)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controller/servingcert/starter/starter.go` around lines 125 - 126, The configInformers.Start(stopChan) call is duplicated (already invoked earlier) causing a redundant second start; remove the second invocation on the line alongside kubeInformers.Start(stopChan) so only kubeInformers.Start(stopChan) and the prior configInformers.Start(stopChan) remain, ensuring stopChan is still passed to all informer Start calls and no additional Start is invoked for configInformers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Line 130: The go.mod contains a permanent replace directive replacing
github.com/openshift/library-go with your personal fork (the line starting
"replace github.com/openshift/library-go => github.com/sanchezl/library-go
..."); remove this personal replace before merging and either point the replace
to an org-owned fork or upstream the needed changes and switch back to the
official module path, or convert the change to a temporary, clearly-commented
local-only workaround (e.g., remove the replace or replace with the org fork
name) so CI/release builds resolve the upstream github.com/openshift/library-go
module instead of a personal namespace.
In `@pkg/controller/servingcert/starter/starter.go`:
- Around line 70-88: CurrentFeatureGates()'s error return is being ignored after
InitialFeatureGatesObserved(); change the select branch to capture the error
from featureGateAccessor.CurrentFeatureGates() (e.g. fg, err :=
featureGateAccessor.CurrentFeatureGates()), check for err != nil and return a
wrapped fmt.Errorf including the err, before using fg.Enabled(...); keep using
InitialFeatureGatesObserved() and the existing PKI resolution logic
(pki.NewListerPKIProfileProvider, pki.ResolveCertificateConfig, cfg.Key)
unchanged otherwise.
- Around line 74-84: The PKIs informer lister is being used before its cache is
guaranteed to be synced; before calling pki.ResolveCertificateConfig and using
PKIs().Lister() (in the block that constructs pkiProvider and calls
pki.ResolveCertificateConfig), call cache.WaitForCacheSync for the PKIs informer
(after configInformers.Start() and alongside the existing wait for
featureGateAccessor.InitialFeatureGatesObserved()) and ensure it returns true;
if it fails, return an error so you don't access an unsynced lister.
In `@pkg/operator/starter.go`:
- Around line 164-179: The PKIs informer must be started and its cache synced
before using its lister; before calling pki.NewListerPKIProfileProvider /
pki.ResolveCertificateConfig (the block that sets signerKeyConfig), call the
informer start/sync helper to WaitForCacheSync for the PKIs informer (e.g. use
configInformers.Start(stopChan) if not already started and then wait on the PKIs
informer’s HasSynced via
configInformers.Config().V1alpha1().PKIs().Informer().HasSynced); if the sync
fails return an error so ResolveCertificateConfig isn’t called against an
unsynced lister.
---
Nitpick comments:
In `@go.mod`:
- Line 3: The go.mod currently sets "go 1.25.0" which raises the enforced
minimum; change it back to the real minimum required (e.g., "go 1.20" or the
actual lowest supported version) and instead add a // toolchain directive to
prefer Go 1.25 for development/CI. Update the go.mod by lowering the "go"
version to the true minimum and adding a toolchain comment like "// toolchain:
go1.25" (or equivalent) so consumers are not forced to use 1.25 while your repo
prefers that compiler.
In `@pkg/controller/servingcert/starter/starter.go`:
- Around line 125-126: The configInformers.Start(stopChan) call is duplicated
(already invoked earlier) causing a redundant second start; remove the second
invocation on the line alongside kubeInformers.Start(stopChan) so only
kubeInformers.Start(stopChan) and the prior configInformers.Start(stopChan)
remain, ensuring stopChan is still passed to all informer Start calls and no
additional Start is invoked for configInformers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ac5134cf-15b2-4320-965b-dc0d7c17bd7e
⛔ Files ignored due to path filters (290)
go.sumis excluded by!**/*.sumvendor/cel.dev/expr/.bazelversionis excluded by!**/vendor/**,!vendor/**vendor/cel.dev/expr/MODULE.bazelis excluded by!**/vendor/**,!vendor/**vendor/cel.dev/expr/cloudbuild.yamlis excluded by!**/vendor/**,!vendor/**vendor/cel.dev/expr/eval.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/arrays.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/converter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/numerics.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/patterns.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/validator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/wercker.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/.cirrus.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_fen.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_inotify.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_kqueue.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_other.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/fsnotify.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/darwin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/freebsd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/shared.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/staticcheck.confis excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/bytestring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/cache.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/common.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/encode_map.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/encode_map_go117.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/omitzero_go124.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/omitzero_pre_go124.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/simplevalue.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/stream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/structfields.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/tag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/.golangci.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/funcr/funcr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/sortkeys/sortkeys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/context.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/extensions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/helpers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/reader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/jsonschema/models.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/jsonschema/reader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/jsonschema/writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv2/OpenAPIv2.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv2/document.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv3/OpenAPIv3.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv3/document.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/bytesource/bytesource.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/fuzz.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/BUILD.bazelis excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/annotations.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/annotations.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/annotations_protoopaque.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/buf.gen.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2_protoopaque.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/.gitattributesis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/.goreleaser.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/compressible.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/bitreader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/bitwriter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/bytereader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/compress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/decompress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/fse.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/gen.shis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/bitreader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/bitwriter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/compress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/decompress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/decompress_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/decompress_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/decompress_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/huff0.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/cpuinfo/cpuinfo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/cpuinfo/cpuinfo_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/cpuinfo/cpuinfo_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/le/le.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/le/unsafe_disabled.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/le/unsafe_enabled.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/decode_other.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/encode_other.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/snappy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/s2sx.modis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/s2sx.sumis excluded by!**/*.sum,!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/bitreader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/bitwriter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/blockdec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/blockenc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/blocktype_string.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/bytebuf.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/bytereader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/decodeheader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/decoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/decoder_options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/dict.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_base.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_best.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_better.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_dfast.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_fast.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/encoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/encoder_options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/framedec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/frameenc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_encoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_predefined.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/hash.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/history.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_arm64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_asm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_other.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_safe.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/matchlen_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/matchlen_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/matchlen_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqenc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/snappy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/zip.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/zstd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/modern-go/reflect2/safe_type.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gomega_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/async_assertion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_comparable_to_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_yaml_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.ci-operator.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.coderabbit.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.golangci.go-validated.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.golangci.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/AGENTS.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/Dockerfile.ocpis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/install.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/types_compatibilityrequirement.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apiextensions/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apiextensions/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apiserver/v1/types_apirequestcount.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiserver/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apps/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apps/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apps/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apps/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apps/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apps/v1/zz_prerelease_lifecycle_generated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/build/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/build/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/build/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/build/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/build/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/cloudnetwork/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/cloudnetwork/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/cloudnetwork/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/cloudnetwork/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/cloudnetwork/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/install.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_operator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_console.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_feature.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_kmsencryption.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_operatorhub.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_scheduling.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_tlssecurityprofile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_backup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_crio_credential_provider_config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_pki.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha2/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha2/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha2/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/console/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/types_console_cli_download.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/types_console_link.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/types_console_plugin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/types_console_sample.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/console/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/console/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/envtest-releases.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/install.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/types_pacemakercluster.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/etcd/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/etcd/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/features.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/legacyfeaturegates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/helm/v1beta1/types_helm_chart_repository.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/helm/v1beta1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/image/docker10/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/image/dockerpre012/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/image/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/image/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/imageregistry/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/imageregistry/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/imageregistry/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/install.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (10)
go.modpkg/controller/servingcert/controller/secret_creating_controller.gopkg/controller/servingcert/controller/secret_updating_controller.gopkg/controller/servingcert/starter/starter.gopkg/operator/operator.gopkg/operator/rotate.gopkg/operator/rotate_test.gopkg/operator/starter.gopkg/operator/sync_common.gopkg/operator/sync_common_test.go
Comment on lines
+70
to
+88
| select { | ||
| case <-featureGateAccessor.InitialFeatureGatesObserved(): | ||
| fg, _ := featureGateAccessor.CurrentFeatureGates() | ||
| if fg.Enabled(features.FeatureGateConfigurablePKI) { | ||
| pkiProvider := pki.NewListerPKIProfileProvider( | ||
| configInformers.Config().V1alpha1().PKIs().Lister(), | ||
| "cluster", | ||
| ) | ||
| cfg, err := pki.ResolveCertificateConfig(pkiProvider, pki.CertificateTypeServing, "service-ca.service-serving") | ||
| if err != nil { | ||
| return fmt.Errorf("failed to resolve PKI serving config: %w", err) | ||
| } | ||
| if cfg != nil { | ||
| servingKeyConfig = &cfg.Key | ||
| } | ||
| } | ||
| case <-time.After(1 * time.Minute): | ||
| return fmt.Errorf("timed out waiting for FeatureGate detection") | ||
| } |
There was a problem hiding this comment.
Error from CurrentFeatureGates() is discarded.
Line 72 ignores the error returned by CurrentFeatureGates(). While this may be intentional after InitialFeatureGatesObserved() signals, silently ignoring errors could mask issues.
select {
case <-featureGateAccessor.InitialFeatureGatesObserved():
- fg, _ := featureGateAccessor.CurrentFeatureGates()
+ fg, err := featureGateAccessor.CurrentFeatureGates()
+ if err != nil {
+ return fmt.Errorf("failed to get current feature gates: %w", err)
+ }
if fg.Enabled(features.FeatureGateConfigurablePKI) {📝 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
| select { | |
| case <-featureGateAccessor.InitialFeatureGatesObserved(): | |
| fg, _ := featureGateAccessor.CurrentFeatureGates() | |
| if fg.Enabled(features.FeatureGateConfigurablePKI) { | |
| pkiProvider := pki.NewListerPKIProfileProvider( | |
| configInformers.Config().V1alpha1().PKIs().Lister(), | |
| "cluster", | |
| ) | |
| cfg, err := pki.ResolveCertificateConfig(pkiProvider, pki.CertificateTypeServing, "service-ca.service-serving") | |
| if err != nil { | |
| return fmt.Errorf("failed to resolve PKI serving config: %w", err) | |
| } | |
| if cfg != nil { | |
| servingKeyConfig = &cfg.Key | |
| } | |
| } | |
| case <-time.After(1 * time.Minute): | |
| return fmt.Errorf("timed out waiting for FeatureGate detection") | |
| } | |
| select { | |
| case <-featureGateAccessor.InitialFeatureGatesObserved(): | |
| fg, err := featureGateAccessor.CurrentFeatureGates() | |
| if err != nil { | |
| return fmt.Errorf("failed to get current feature gates: %w", err) | |
| } | |
| if fg.Enabled(features.FeatureGateConfigurablePKI) { | |
| pkiProvider := pki.NewListerPKIProfileProvider( | |
| configInformers.Config().V1alpha1().PKIs().Lister(), | |
| "cluster", | |
| ) | |
| cfg, err := pki.ResolveCertificateConfig(pkiProvider, pki.CertificateTypeServing, "service-ca.service-serving") | |
| if err != nil { | |
| return fmt.Errorf("failed to resolve PKI serving config: %w", err) | |
| } | |
| if cfg != nil { | |
| servingKeyConfig = &cfg.Key | |
| } | |
| } | |
| case <-time.After(1 * time.Minute): | |
| return fmt.Errorf("timed out waiting for FeatureGate detection") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/controller/servingcert/starter/starter.go` around lines 70 - 88,
CurrentFeatureGates()'s error return is being ignored after
InitialFeatureGatesObserved(); change the select branch to capture the error
from featureGateAccessor.CurrentFeatureGates() (e.g. fg, err :=
featureGateAccessor.CurrentFeatureGates()), check for err != nil and return a
wrapped fmt.Errorf including the err, before using fg.Enabled(...); keep using
InitialFeatureGatesObserved() and the existing PKI resolution logic
(pki.NewListerPKIProfileProvider, pki.ResolveCertificateConfig, cfg.Key)
unchanged otherwise.
Contributor
Author
There was a problem hiding this comment.
The only way to get an error when calling featureGateAccessor.CurrentFeatureGates() is when featureGateAccessor.AreInitialFeatureGatesObserved() returns false. Since we wait for the featureGateAccessor.InitialFeatureGatesObserved() channel to be closed, there is no way for the error to occur.
There was a problem hiding this comment.
@sanchezl, that's a valid point. Since InitialFeatureGatesObserved() channel closure guarantees AreInitialFeatureGatesObserved() returns true before CurrentFeatureGates() is ever called, the error path is indeed unreachable. The blank identifier is correct here.
📝 Learnings were identified and not saved (knowledge base disabled). Enable
Comment on lines
+74
to
+84
| pkiProvider := pki.NewListerPKIProfileProvider( | ||
| configInformers.Config().V1alpha1().PKIs().Lister(), | ||
| "cluster", | ||
| ) | ||
| cfg, err := pki.ResolveCertificateConfig(pkiProvider, pki.CertificateTypeServing, "service-ca.service-serving") | ||
| if err != nil { | ||
| return fmt.Errorf("failed to resolve PKI serving config: %w", err) | ||
| } | ||
| if cfg != nil { | ||
| servingKeyConfig = &cfg.Key | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does OpenShift library-go NewListerPKIProfileProvider require cache sync before use?
💡 Result:
NewListerPKIProfileProvider itself shouldn’t need an extra “sync” call, but it is only safe/meaningful to use once the informer(s) backing the lister have synced—otherwise the lister can return empty/stale results simply because its local cache hasn’t been populated yet.
In practice that means: start the informer factory and wait for cache.WaitForCacheSync(...) (or the informer’s HasSynced) before calling into the provider. Kubernetes client-go explicitly provides WaitForCacheSync for “waits for caches to populate.” [1]
Note: I couldn’t find NewListerPKIProfileProvider by name in the upstream openshift/library-go API docs as of the currently published module on pkg.go.dev (published Mar 11, 2026), so if you’re looking at a vendored/forked copy, the above guidance still applies whenever it reads via a lister-backed informer cache.
Sources
[1] k8s.io/client-go/tools/cache — WaitForCacheSync docs (pkg.go.dev) (pkg.go.dev)
Citations:
🏁 Script executed:
find . -name "starter.go" -path "*/servingcert/starter/*" | head -5Repository: openshift/service-ca-operator
Length of output: 120
🏁 Script executed:
cat -n ./pkg/controller/servingcert/starter/starter.go | head -120Repository: openshift/service-ca-operator
Length of output: 5007
🏁 Script executed:
cat -n ./pkg/controller/servingcert/starter/starter.go | tail -40Repository: openshift/service-ca-operator
Length of output: 1570
🏁 Script executed:
grep -n "WaitForCacheSync" ./pkg/controller/servingcert/starter/starter.goRepository: openshift/service-ca-operator
Length of output: 55
PKI informer cache must be synced before accessing the lister.
The PKIs().Lister() is accessed at line 74 without waiting for the informer cache to sync. Although configInformers.Start() is called at line 58, the subsequent select waits only for featureGateAccessor.InitialFeatureGatesObserved() (which syncs the FeatureGates informer), not the PKI informer. This can result in empty or stale PKI data if the cache hasn't been populated yet.
Add cache.WaitForCacheSync() before accessing the PKI lister:
Suggested fix
+ if !cache.WaitForCacheSync(ctx.Done(), configInformers.Config().V1alpha1().PKIs().Informer().HasSynced) {
+ return fmt.Errorf("timed out waiting for PKI informer cache to sync")
+ }
pkiProvider := pki.NewListerPKIProfileProvider(
configInformers.Config().V1alpha1().PKIs().Lister(),
"cluster",
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/controller/servingcert/starter/starter.go` around lines 74 - 84, The PKIs
informer lister is being used before its cache is guaranteed to be synced;
before calling pki.ResolveCertificateConfig and using PKIs().Lister() (in the
block that constructs pkiProvider and calls pki.ResolveCertificateConfig), call
cache.WaitForCacheSync for the PKIs informer (after configInformers.Start() and
alongside the existing wait for
featureGateAccessor.InitialFeatureGatesObserved()) and ensure it returns true;
if it fails, return an error so you don't access an unsynced lister.
Contributor
Author
|
/hold for openshift/library-go#2127 |
openshift-ci
Bot
added
do-not-merge/hold
Contributor
|
@sanchezl: This pull request references CNTRLPLANE-2947 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
pkg/controller/servingcert/starter/starter.go (1)
64-71:⚠️ Potential issue | 🟡 MinorError from
CurrentFeatureGates()should be handled.While the error after
InitialFeatureGatesObserved()may be unlikely, silently ignoring it could mask issues. The logging statement uses the feature gates, so the error should be checked.🛡️ Proposed fix
select { case <-featureGateAccessor.InitialFeatureGatesObserved(): - featureGates, _ := featureGateAccessor.CurrentFeatureGates() + featureGates, err := featureGateAccessor.CurrentFeatureGates() + if err != nil { + return fmt.Errorf("failed to get current feature gates: %w", err) + } klog.Infof("FeatureGates initialized: knownFeatureGates=%v", featureGates.KnownFeatures()) case <-time.After(1 * time.Minute):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controller/servingcert/starter/starter.go` around lines 64 - 71, Handle the error returned by featureGateAccessor.CurrentFeatureGates() instead of ignoring it: inside the select branch that waits on featureGateAccessor.InitialFeatureGatesObserved(), call CurrentFeatureGates(), check its error, and if non-nil log it with klog.Errorf (including the error) and return that error (or wrap it with context) instead of proceeding to use featureGates; otherwise continue to log featureGates.KnownFeatures() as before. Ensure you reference featureGateAccessor.InitialFeatureGatesObserved(), featureGateAccessor.CurrentFeatureGates(), and the existing klog.Infof/klog.Errorf calls when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/controller/servingcert/starter/starter.go`:
- Around line 64-71: Handle the error returned by
featureGateAccessor.CurrentFeatureGates() instead of ignoring it: inside the
select branch that waits on featureGateAccessor.InitialFeatureGatesObserved(),
call CurrentFeatureGates(), check its error, and if non-nil log it with
klog.Errorf (including the error) and return that error (or wrap it with
context) instead of proceeding to use featureGates; otherwise continue to log
featureGates.KnownFeatures() as before. Ensure you reference
featureGateAccessor.InitialFeatureGatesObserved(),
featureGateAccessor.CurrentFeatureGates(), and the existing
klog.Infof/klog.Errorf calls when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec165874-9a2b-4086-8f64-525feeb196fe
📒 Files selected for processing (11)
pkg/controller/servingcert/controller/pki.gopkg/controller/servingcert/controller/pki_test.gopkg/controller/servingcert/controller/secret_creating_controller.gopkg/controller/servingcert/controller/secret_updating_controller.gopkg/controller/servingcert/starter/starter.gopkg/operator/operator.gopkg/operator/rotate.gopkg/operator/rotate_test.gopkg/operator/starter.gopkg/operator/sync_common.gopkg/operator/sync_common_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/operator/operator.go
Contributor
|
@sanchezl: This pull request references CNTRLPLANE-2947 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
pkg/controller/servingcert/starter/starter.go (1)
64-67:⚠️ Potential issue | 🟡 MinorHandle
CurrentFeatureGates()error before proceeding.Line 66 discards the error return from
CurrentFeatureGates(). Please fail fast if it returns an error instead of continuing with potentially invalid gate state.Suggested fix
select { case <-featureGateAccessor.InitialFeatureGatesObserved(): - featureGates, _ := featureGateAccessor.CurrentFeatureGates() + featureGates, err := featureGateAccessor.CurrentFeatureGates() + if err != nil { + return fmt.Errorf("failed to get current feature gates: %w", err) + } klog.Infof("FeatureGates initialized: knownFeatureGates=%v", featureGates.KnownFeatures()) case <-time.After(1 * time.Minute): klog.Errorf("timed out waiting for FeatureGate detection") return fmt.Errorf("timed out waiting for FeatureGate detection") }#!/bin/bash # Verify the ignored error pattern exists in the current file. rg -nP --type=go 'featureGates,\s*_\s*:=\s*featureGateAccessor\.CurrentFeatureGates\(\)' pkg/controller/servingcert/starter/starter.go -C2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controller/servingcert/starter/starter.go` around lines 64 - 67, The code currently discards the error from featureGateAccessor.CurrentFeatureGates() inside the select branch; update the select branch that calls featureGateAccessor.CurrentFeatureGates() to check the returned error and fail fast (e.g., return the error or klog.Fatalf/klog.Errorf + return) instead of continuing with a possibly invalid featureGates value—modify the block around featureGateAccessor.InitialFeatureGatesObserved() to capture (featureGates, err), handle err != nil (using the existing function/method context where this select runs), and only log “FeatureGates initialized” and use featureGates.KnownFeatures() when err == nil.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/controller/servingcert/controller/secret_creating_controller.go`:
- Around line 164-167: When resolveServingKeyConfig(sc.featureGateAccessor,
sc.pkiLister) returns an error, don’t return immediately; instead call the
controller’s updateServiceFailure(...) helper to record the key-config
resolution failure on the Service (so it is annotated and counted toward
maxRetries), then return the original error. Update the error branch around
resolveServingKeyConfig to invoke updateServiceFailure with the Service object
in scope and the error (using the same failure reason/message scheme as other
failure paths) before returning.
In `@pkg/controller/servingcert/controller/secret_updating_controller.go`:
- Around line 104-111: The code currently calls
resolveServingKeyConfig(sc.featureGateAccessor, sc.pkiLister) at the top of the
sync which causes PKI resolution errors to fail every sync; instead, remove that
eager call and defer calling resolveServingKeyConfig until you know the secret
requires regeneration (the branch that checks expiry/needsRegeneration). Move
the resolveServingKeyConfig invocation into the regeneration code path (where
you build/generate a new secret/certificate), handle and return errors only from
that regeneration step, and ensure non-regenerating paths do not call
resolveServingKeyConfig or surface its errors (so healthy secrets continue
syncing even if PKI resolution is transiently failing).
---
Duplicate comments:
In `@pkg/controller/servingcert/starter/starter.go`:
- Around line 64-67: The code currently discards the error from
featureGateAccessor.CurrentFeatureGates() inside the select branch; update the
select branch that calls featureGateAccessor.CurrentFeatureGates() to check the
returned error and fail fast (e.g., return the error or klog.Fatalf/klog.Errorf
+ return) instead of continuing with a possibly invalid featureGates
value—modify the block around featureGateAccessor.InitialFeatureGatesObserved()
to capture (featureGates, err), handle err != nil (using the existing
function/method context where this select runs), and only log “FeatureGates
initialized” and use featureGates.KnownFeatures() when err == nil.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 13c732dc-90eb-4198-bbcf-77f134ac4e3a
⛔ Files ignored due to path filters (286)
go.sumis excluded by!**/*.sumvendor/cel.dev/expr/.bazelversionis excluded by!**/vendor/**,!vendor/**vendor/cel.dev/expr/MODULE.bazelis excluded by!**/vendor/**,!vendor/**vendor/cel.dev/expr/cloudbuild.yamlis excluded by!**/vendor/**,!vendor/**vendor/cel.dev/expr/eval.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/arrays.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/converter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/numerics.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/patterns.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/validator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/wercker.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/.cirrus.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_fen.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_inotify.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_kqueue.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_other.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/fsnotify.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/darwin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/freebsd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/shared.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/staticcheck.confis excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/bytestring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/cache.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/common.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/encode_map.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/encode_map_go117.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/omitzero_go124.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/omitzero_pre_go124.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/simplevalue.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/stream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/structfields.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/tag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/.golangci.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/funcr/funcr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/sortkeys/sortkeys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/context.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/extensions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/helpers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/reader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/jsonschema/models.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/jsonschema/reader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/jsonschema/writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv2/OpenAPIv2.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv2/document.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv3/OpenAPIv3.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv3/document.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/bytesource/bytesource.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/fuzz.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/BUILD.bazelis excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/annotations.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/annotations.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/annotations_protoopaque.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/buf.gen.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2_protoopaque.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/.gitattributesis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/.goreleaser.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/compressible.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/bitreader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/bitwriter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/bytereader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/compress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/decompress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/fse/fse.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/gen.shis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/bitreader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/bitwriter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/compress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/decompress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/decompress_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/decompress_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/decompress_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/huff0/huff0.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/cpuinfo/cpuinfo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/cpuinfo/cpuinfo_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/cpuinfo/cpuinfo_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/le/le.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/le/unsafe_disabled.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/le/unsafe_enabled.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/decode_other.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/encode_other.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/internal/snapref/snappy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/s2sx.modis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/s2sx.sumis excluded by!**/*.sum,!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/bitreader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/bitwriter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/blockdec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/blockenc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/blocktype_string.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/bytebuf.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/bytereader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/decodeheader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/decoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/decoder_options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/dict.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_base.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_best.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_better.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_dfast.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/enc_fast.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/encoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/encoder_options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/framedec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/frameenc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_decoder_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_encoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/fse_predefined.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/hash.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/history.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_arm64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_asm.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_other.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/internal/xxhash/xxhash_safe.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/matchlen_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/matchlen_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/matchlen_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_amd64.sis excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqdec_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/seqenc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/snappy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/zip.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/klauspost/compress/zstd/zstd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/modern-go/reflect2/safe_type.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gomega_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/async_assertion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_comparable_to_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_yaml_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.ci-operator.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.coderabbit.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.golangci.go-validated.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.golangci.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/AGENTS.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/Dockerfile.ocpis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/install.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/types_compatibilityrequirement.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apiextensions/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apiextensions/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apiserver/v1/types_apirequestcount.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiserver/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apps/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apps/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apps/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apps/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apps/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/apps/v1/zz_prerelease_lifecycle_generated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/build/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/build/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/build/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/build/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/build/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/cloudnetwork/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/cloudnetwork/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/cloudnetwork/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/cloudnetwork/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/cloudnetwork/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/install.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_operator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_console.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_feature.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_ingress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_kmsencryption.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_operatorhub.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_scheduling.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/types_tlssecurityprofile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_backup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_crio_credential_provider_config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_image_policy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_pki.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha2/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha2/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha2/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/console/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/types_console_cli_download.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/types_console_link.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/types_console_plugin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/types_console_sample.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/console/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/console/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/console/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/envtest-releases.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/install.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/types_pacemakercluster.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/etcd/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/etcd/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/etcd/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/features.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/legacyfeaturegates.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/helm/v1beta1/types_helm_chart_repository.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/helm/v1beta1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/image/docker10/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/image/dockerpre012/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/image/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/image/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*
📒 Files selected for processing (14)
.ci-operator.yamlDockerfile.rhel7go.modpkg/controller/servingcert/controller/pki.gopkg/controller/servingcert/controller/pki_test.gopkg/controller/servingcert/controller/secret_creating_controller.gopkg/controller/servingcert/controller/secret_updating_controller.gopkg/controller/servingcert/starter/starter.gopkg/operator/operator.gopkg/operator/rotate.gopkg/operator/rotate_test.gopkg/operator/starter.gopkg/operator/sync_common.gopkg/operator/sync_common_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/operator/sync_common_test.go
Comment on lines
+164
to
+167
| keyConfig, err := resolveServingKeyConfig(sc.featureGateAccessor, sc.pkiLister) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Record key-config resolution failures via service failure annotations.
Returning directly here bypasses updateServiceFailure, so this new failure mode is not counted against maxRetries and does not annotate the Service with actionable error state.
💡 Suggested fix
keyConfig, err := resolveServingKeyConfig(sc.featureGateAccessor, sc.pkiLister)
if err != nil {
- return err
+ return sc.updateServiceFailure(ctx, serviceCopy, fmt.Errorf("unable to resolve serving key config: %w", err))
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/controller/servingcert/controller/secret_creating_controller.go` around
lines 164 - 167, When resolveServingKeyConfig(sc.featureGateAccessor,
sc.pkiLister) returns an error, don’t return immediately; instead call the
controller’s updateServiceFailure(...) helper to record the key-config
resolution failure on the Service (so it is annotated and counted toward
maxRetries), then return the original error. Update the error branch around
resolveServingKeyConfig to invoke updateServiceFailure with the Service object
in scope and the error (using the same failure reason/message scheme as other
failure paths) before returning.
Contributor
|
@sanchezl: An error was encountered searching for bug CNTRLPLANE-2947 on the Jira server at https://issues.redhat.com. No known errors were detected, please see the full error message for details. Full error message.
request failed. Please analyze the request body for more details. Status code: 403:
Please contact an administrator to resolve this issue, then request a bug refresh with DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
removed
the
jira/valid-reference
There was a problem hiding this comment.
♻️ Duplicate comments (3)
pkg/controller/servingcert/controller/secret_updating_controller.go (1)
104-111:⚠️ Potential issue | 🟠 MajorDefer key-config resolution to regeneration-only paths.
Resolving at Line 108 makes healthy secrets fail sync on transient PKI lookup errors, even when no cert rewrite is needed. Resolve only in branches that actually call
toRequiredSecret.Suggested direction
- keyConfig, err := resolveServingKeyConfig(sc.featureGateAccessor, sc.pkiLister) - if err != nil { - return err - } - if sc.requiresRegeneration(service, sharedSecret, sc.minTimeLeftForCert) { + keyConfig, err := resolveServingKeyConfig(sc.featureGateAccessor, sc.pkiLister) + if err != nil { + return err + } if err := toRequiredSecret(sc.dnsSuffix, sc.ca, sc.intermediateCACert, service, secretCopy, sc.certificateLifetime, keyConfig); err != nil { return err } _, err := sc.secretClient.Secrets(secretCopy.Namespace).Update(ctx, secretCopy, metav1.UpdateOptions{}) return err } - update, err := sc.ensureSecretData(service, secretCopy, keyConfig) + update, err := sc.ensureSecretData(service, secretCopy)Also applies to: 125-125
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controller/servingcert/controller/secret_updating_controller.go` around lines 104 - 111, The code eagerly calls resolveServingKeyConfig and assigns keyConfig before determining whether regeneration is needed, causing transient PKI lookup failures to break healthy secret syncs; move the call to resolveServingKeyConfig so it is only invoked in the branches that actually call toRequiredSecret (i.e., defer resolution into the regeneration/code-paths that build required secrets), remove the early resolution and any use of keyConfig outside those branches, and ensure both occurrences (the current early call and the duplicate at the later location) are consolidated so resolution happens only when toRequiredSecret will be executed.pkg/controller/servingcert/controller/secret_creating_controller.go (1)
164-167:⚠️ Potential issue | 🟠 MajorRoute PKI key-config resolution failures through
updateServiceFailure.At Line 166, returning directly skips service failure annotations/retry accounting, so this new failure mode is not tracked like other generation failures.
Suggested fix
keyConfig, err := resolveServingKeyConfig(sc.featureGateAccessor, sc.pkiLister) if err != nil { - return err + return sc.updateServiceFailure(ctx, serviceCopy, fmt.Errorf("unable to resolve serving key config: %w", err)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controller/servingcert/controller/secret_creating_controller.go` around lines 164 - 167, The direct return of err from resolveServingKeyConfig bypasses service failure handling; replace the bare "return err" with a call to the controller's failure annotator (updateServiceFailure) so the service's failure annotation and retry accounting are updated. Specifically, where resolveServingKeyConfig(sc.featureGateAccessor, sc.pkiLister) currently returns an error, invoke sc.updateServiceFailure(...) (using the same service identifier/context used elsewhere in this file), pass a concise failure message indicating key-config resolution and the err, and follow the established return behavior used by other failure paths in this controller (mirror the pattern around other calls to updateServiceFailure in this file to determine whether to return the error or nil after annotating).go.mod (1)
130-130:⚠️ Potential issue | 🟠 MajorRemove the personal-fork
replacebefore merge.Line 130 routes all
github.com/openshift/library-goresolution through a personal fork, which is a release/supply-chain risk if this lands inmain. Keep this only as a short-lived development override and switch back to upstream/org-owned source before merge.Suggested minimal change
-replace github.com/openshift/library-go => github.com/sanchezl/library-go v0.0.0-20260316031911-77a86494b72bUse this read-only check before merge (expected: second command returns no matches):
#!/bin/bash set -euo pipefail # 1) Find any replace for openshift/library-go rg -nP '^\s*replace\s+github\.com/openshift/library-go\s*=>' # 2) Ensure it does not point outside the openshift org rg -nP '^\s*replace\s+github\.com/openshift/library-go\s*=>\s+github\.com/(?!openshift/)' || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 130, The go.mod contains a development-only replace directive "replace github.com/openshift/library-go => github.com/sanchezl/library-go ..." that must be removed before merging; delete that replace line in go.mod (or revert it to the original upstream module) so imports resolve to github.com/openshift/library-go, then run the provided two-step grep/rg checks to verify no remaining replace entries point outside the openshift org; keep this change local to the feature branch only if you still need the fork during testing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@go.mod`:
- Line 130: The go.mod contains a development-only replace directive "replace
github.com/openshift/library-go => github.com/sanchezl/library-go ..." that must
be removed before merging; delete that replace line in go.mod (or revert it to
the original upstream module) so imports resolve to
github.com/openshift/library-go, then run the provided two-step grep/rg checks
to verify no remaining replace entries point outside the openshift org; keep
this change local to the feature branch only if you still need the fork during
testing.
In `@pkg/controller/servingcert/controller/secret_creating_controller.go`:
- Around line 164-167: The direct return of err from resolveServingKeyConfig
bypasses service failure handling; replace the bare "return err" with a call to
the controller's failure annotator (updateServiceFailure) so the service's
failure annotation and retry accounting are updated. Specifically, where
resolveServingKeyConfig(sc.featureGateAccessor, sc.pkiLister) currently returns
an error, invoke sc.updateServiceFailure(...) (using the same service
identifier/context used elsewhere in this file), pass a concise failure message
indicating key-config resolution and the err, and follow the established return
behavior used by other failure paths in this controller (mirror the pattern
around other calls to updateServiceFailure in this file to determine whether to
return the error or nil after annotating).
In `@pkg/controller/servingcert/controller/secret_updating_controller.go`:
- Around line 104-111: The code eagerly calls resolveServingKeyConfig and
assigns keyConfig before determining whether regeneration is needed, causing
transient PKI lookup failures to break healthy secret syncs; move the call to
resolveServingKeyConfig so it is only invoked in the branches that actually call
toRequiredSecret (i.e., defer resolution into the regeneration/code-paths that
build required secrets), remove the early resolution and any use of keyConfig
outside those branches, and ensure both occurrences (the current early call and
the duplicate at the later location) are consolidated so resolution happens only
when toRequiredSecret will be executed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99c3e15d-1ac4-4606-9a55-1539b39a825d
⛔ Files ignored due to path filters (47)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/AGENTS.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/types_compatibilityrequirement.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1alpha1/register.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_pki.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/features.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/prefixedclaimmapping.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/tokenclaimmapping.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/usernameclaimmapping.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/certificateconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/custompkipolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/defaultcertificateconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/ecdsakeyconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/keyconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/pki.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/pkicertificatemanagement.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/pkiprofile.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/pkispec.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/rsakeyconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/internal/internal.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/config_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/generated_expansion.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/pki.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1alpha1/interface.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1alpha1/pki.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/generic.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1alpha1/expansion_generated.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1alpha1/pki.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/internal/internal.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/crypto/cert_config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/crypto/key_config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/crypto/keygen.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/crypto/options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/operator/certrotation/client_cert_rotation_controller.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/operator/certrotation/signer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/operator/certrotation/target.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/pki/profile.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/pki/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/pki/resolve.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/pki/types.gois excluded by!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (12)
go.modpkg/controller/servingcert/controller/pki.gopkg/controller/servingcert/controller/pki_test.gopkg/controller/servingcert/controller/secret_creating_controller.gopkg/controller/servingcert/controller/secret_updating_controller.gopkg/controller/servingcert/starter/starter.gopkg/operator/operator.gopkg/operator/rotate.gopkg/operator/rotate_test.gopkg/operator/starter.gopkg/operator/sync_common.gopkg/operator/sync_common_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- pkg/operator/operator.go
- pkg/controller/servingcert/starter/starter.go
- pkg/operator/rotate_test.go
- pkg/operator/sync_common_test.go
- pkg/controller/servingcert/controller/pki.go
Contributor
Author
|
/retest |
sanchezl
force-pushed
the
pki-config
branch
2 times, most recently
from
0e8591b to
a214746
Compare
openshift-ci
Bot
added
the
needs-rebase
openshift-ci
Bot
removed
the
needs-rebase
sanchezl
force-pushed
the
pki-config
branch
2 times, most recently
from
baa5826 to
718d1da
Compare
Contributor
|
@sanchezl: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Contributor
Author
|
/payload-job-with-prs periodic-ci-openshift-release-main-ci-4.22-e2e-aws-ovn-techpreview #327 |
Contributor
|
@sanchezl: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/4adf4dc0-2df2-11f1-8241-50cd0463c517-0 |
Contributor
|
@sanchezl: This PR was included in a payload test run from #327
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/4adf4dc0-2df2-11f1-8241-50cd0463c517-0 |
The ValidatingWebhookConfigurations informer was being accessed after SharedInformerFactory.Start(), so the factory never started it, causing WaitForCacheSync to block indefinitely and the test to time out.
Contributor
Author
|
/payload-job periodic-ci-openshift-release-main-ci-4.22-e2e-aws-ovn-techpreview |
Contributor
|
@sanchezl: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/716fb4e0-2e9f-11f1-9f05-183014ecb950-0 |
Contributor
Author
|
/verified by "e2e-aws-ovn-techpreview" Verification SummaryThe Apr 2 TechPreview (ConfigurablePKI Enabled): ECDSA certs generated
Non-TechPreview (ConfigurablePKI Disabled): RSA certs preserved
Side-by-Side
|
Contributor
|
@sanchezl: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
verified
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pkg/controller/servingcert/starter/starter.go (1)
111-112: DuplicateconfigInformers.Start()call.
configInformers.Start(ctx.Done())is already called at line 61. WhileStart()is idempotent onSharedInformerFactory, calling it twice is unnecessary and may cause confusion.♻️ Suggested fix: remove duplicate call
kubeInformers.Start(ctx.Done()) - configInformers.Start(ctx.Done()) go servingCertController.Run(ctx, 5)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controller/servingcert/starter/starter.go` around lines 111 - 112, The file contains a duplicate call to configInformers.Start(ctx.Done()) (called earlier and again at lines shown); remove the second invocation so only kubeInformers.Start(ctx.Done()) and the single configInformers.Start(ctx.Done()) remain; update the starter.go block that calls kubeInformers.Start and configInformers.Start to call each SharedInformerFactory.Start exactly once.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/controller/servingcert/controller/secret_creating_controller_test.go`:
- Around line 99-103: The test initializes servingCertIssuer without setting
certificateLifetime, so makeServingCert ends up passing a zero duration to
ca.MakeServerCert; update the test setup where servingCertIssuer is constructed
(servingCertIssuer{...}) to set certificateLifetime to a non-zero duration (e.g.
a sensible time.Duration like 1*time.Hour or the test-suite constant) so that
makeServingCert calls ca.MakeServerCert with a valid lifetime; ensure the field
name certificateLifetime in the servingCertIssuer struct is included in that
literal initialization.
In `@pkg/controller/servingcert/controller/secret_creating_controller.go`:
- Around line 164-166: The call to sc.toRequiredSecret(serviceCopy, secret) can
return PKI resolution errors from makeServingCert and currently returns raw
errors, bypassing updateServiceFailure; change the error handling to mirror the
other branches in generateCert by invoking sc.updateServiceFailure(serviceCopy,
err) (and any relevant message/annotations used elsewhere) before returning the
error so the failure is counted, annotated on the Service, and subject to
maxRetries logic (targeting sc.toRequiredSecret and makeServingCert as the error
sources to locate the fix).
---
Nitpick comments:
In `@pkg/controller/servingcert/starter/starter.go`:
- Around line 111-112: The file contains a duplicate call to
configInformers.Start(ctx.Done()) (called earlier and again at lines shown);
remove the second invocation so only kubeInformers.Start(ctx.Done()) and the
single configInformers.Start(ctx.Done()) remain; update the starter.go block
that calls kubeInformers.Start and configInformers.Start to call each
SharedInformerFactory.Start exactly once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eba556a7-0c18-4516-8815-afad61657ba8
⛔ Files ignored due to path filters (111)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/.coderabbit.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/AGENTS.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/Makefileis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/types_compatibilityrequirement.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/types_authentication.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/types_cluster_version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1alpha1/register.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_image_policy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_image_policy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_pki.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/envtest-releases.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/features.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/features/legacyfeaturegates.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/operator/v1/types_machineconfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-Default.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-OKD.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/apiserverspec.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/prefixedclaimmapping.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/tokenclaimmapping.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/update.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/usernameclaimmapping.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/additionalalertmanagerconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/alertmanagercustomconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/authorizationconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/basicauth.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/certificateconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clusterimagepolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clusterimagepolicyspec.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clusterimagepolicystatus.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clustermonitoringspec.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/custompkipolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/defaultcertificateconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/dropequalactionconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/ecdsakeyconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/hashmodactionconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicyfulciocawithrekorrootoftrust.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicypkirootoftrust.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicypublickeyrootoftrust.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicyspec.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagepolicystatus.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/imagesigstoreverificationpolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/keepequalactionconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/keyconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/label.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/labelmapactionconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/lowercaseactionconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/metadataconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/metadataconfigcustom.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/oauth2.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/oauth2endpointparam.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/openshiftstatemetricsconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/pki.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/pkicertificatemanagement.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/pkicertificatesubject.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/pkiprofile.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/pkispec.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policyfulciosubject.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policyidentity.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policymatchexactrepository.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policymatchremapidentity.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/policyrootoftrust.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusremotewriteheader.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/queueconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/relabelactionconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/relabelconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/remotewriteauthorization.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/remotewritespec.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/replaceactionconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/retention.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/rsakeyconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/secretkeyselector.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/sigv4.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/tlsconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/uppercaseactionconfig.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/internal/internal.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/clusterimagepolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/config_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/generated_expansion.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/imagepolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/pki.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1alpha1/clusterimagepolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1alpha1/interface.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1alpha1/pki.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/generic.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1alpha1/clusterimagepolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1alpha1/expansion_generated.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1alpha1/imagepolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1alpha1/pki.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/internal/internal.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/route/applyconfigurations/internal/internal.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/crypto/cert_config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/crypto/keygen.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/crypto/options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/crypto/tls_adherence.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/operator/v1helpers/helpers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/pki/profile.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/pki/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/pki/resolve.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/pki/types.gois excluded by!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (15)
bindata/assets/clusterrole.yamlbindata/assets/deployment.yamlgo.modpkg/controller/cabundleinjector/admissionwebhook_test.gopkg/controller/servingcert/controller/secret_creating_controller.gopkg/controller/servingcert/controller/secret_creating_controller_test.gopkg/controller/servingcert/controller/secret_updating_controller.gopkg/controller/servingcert/controller/serving_cert_issuer.gopkg/controller/servingcert/starter/starter.gopkg/operator/operator.gopkg/operator/rotate.gopkg/operator/rotate_test.gopkg/operator/starter.gopkg/operator/sync_common.gopkg/operator/sync_common_test.go
✅ Files skipped from review due to trivial changes (1)
- go.mod
🚧 Files skipped from review as they are similar to previous changes (6)
- pkg/operator/rotate_test.go
- pkg/operator/operator.go
- pkg/operator/sync_common_test.go
- pkg/operator/sync_common.go
- pkg/operator/rotate.go
- pkg/operator/starter.go
Comment on lines
+99
to
+103
| servingCertIssuer: servingCertIssuer{ | ||
| ca: ca, | ||
| intermediateCACert: nil, | ||
| dnsSuffix: "cluster.local", | ||
| }, |
There was a problem hiding this comment.
Missing certificateLifetime initialization in test setup.
The embedded servingCertIssuer does not set certificateLifetime, leaving it at the zero value. When makeServingCert calls ca.MakeServerCert(subjects, s.certificateLifetime, ...), a zero duration may produce certificates with unexpected validity periods or cause test failures.
🔧 Proposed fix
controller := &serviceServingCertController{
servingCertIssuer: servingCertIssuer{
ca: ca,
intermediateCACert: nil,
dnsSuffix: "cluster.local",
+ certificateLifetime: 2 * 365 * 24 * time.Hour,
},📝 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
| servingCertIssuer: servingCertIssuer{ | |
| ca: ca, | |
| intermediateCACert: nil, | |
| dnsSuffix: "cluster.local", | |
| }, | |
| servingCertIssuer: servingCertIssuer{ | |
| ca: ca, | |
| intermediateCACert: nil, | |
| dnsSuffix: "cluster.local", | |
| certificateLifetime: 2 * 365 * 24 * time.Hour, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/controller/servingcert/controller/secret_creating_controller_test.go`
around lines 99 - 103, The test initializes servingCertIssuer without setting
certificateLifetime, so makeServingCert ends up passing a zero duration to
ca.MakeServerCert; update the test setup where servingCertIssuer is constructed
(servingCertIssuer{...}) to set certificateLifetime to a non-zero duration (e.g.
a sensible time.Duration like 1*time.Hour or the test-suite constant) so that
makeServingCert calls ca.MakeServerCert with a valid lifetime; ensure the field
name certificateLifetime in the servingCertIssuer struct is included in that
literal initialization.
Comment on lines
+164
to
166
| if err := sc.toRequiredSecret(serviceCopy, secret); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
PKI resolution errors bypass failure tracking.
When sc.toRequiredSecret fails due to PKI resolution errors (from the embedded makeServingCert), the error returns directly without calling updateServiceFailure. This means:
- Failures aren't counted toward
maxRetries - The service isn't annotated with error information
- On persistent PKI misconfiguration, services will retry indefinitely without actionable error state
Other error paths in generateCert (lines 171, 176, 181, 187) consistently use updateServiceFailure.
🔧 Proposed fix
- if err := sc.toRequiredSecret(serviceCopy, secret); err != nil {
- return err
- }
+ if err := sc.toRequiredSecret(serviceCopy, secret); err != nil {
+ return sc.updateServiceFailure(ctx, serviceCopy, err)
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/controller/servingcert/controller/secret_creating_controller.go` around
lines 164 - 166, The call to sc.toRequiredSecret(serviceCopy, secret) can return
PKI resolution errors from makeServingCert and currently returns raw errors,
bypassing updateServiceFailure; change the error handling to mirror the other
branches in generateCert by invoking sc.updateServiceFailure(serviceCopy, err)
(and any relevant message/annotations used elsewhere) before returning the error
so the failure is counted, annotated on the Service, and subject to maxRetries
logic (targeting sc.toRequiredSecret and makeServingCert as the error sources to
locate the fix).
7 tasks
Contributor
|
/verified by @kaleemsiddiqu |
Contributor
|
@kaleemsiddiqu: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Contributor
|
/lgtm |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kaleemsiddiqu, sanchezl The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Overview
Wires configurable PKI support into the service-ca-operator, enabling the signing CA and per-service serving certificates to use key algorithms configured via the
PKIcluster resource when theConfigurablePKIfeature gate is enabled. When the gate is off, behavior is unchanged.Depends on library-go#2127.
Changes
Operator (
pkg/operator/)operator.go: AddedconfigurablePKIEnabledbool andpkiProviderfields. Constructor checks the feature gate once at startup and conditionally wires the PKI informer and lister-based provider.sync_common.go:initializeSigningSecretbranches onconfigurablePKIEnabled— resolves signer key config viapki.ResolveCertificateConfigand callscrypto.NewSigningCertificate, or falls back tocrypto.MakeSelfSignedCAConfig.rotate.go: Same branching inrotateSigningCA. Widened*rsa.PrivateKey→crypto.Signerthroughout (maybeRotateSigningSecret,rotateSigningCA,createIntermediateCACert) for algorithm agnosticism.starter.go: Passes resolvedfeatureGatestoNewServiceCAOperator.Controller (
pkg/controller/servingcert/)serving_cert_issuer.go(new): ExtractedservingCertIssuerstruct holdingca,intermediateCACert,dnsSuffix,certificateLifetime,configurablePKIEnabled, andpkiProvider. ProvidesmakeServingCertandtoRequiredSecretmethods shared by both controllers, plus an exportedMakeServingCertstandalone function for operator rotation tests.secret_creating_controller.go: EmbedsservingCertIssuer. Constructor acceptsconfigInformersandfeatureGates; conditionally wires PKI informer/provider.secret_updating_controller.go: Same embedding and constructor pattern.starter.go: InitializesFeatureGateAccess, waits for initial observation, passesfeatureGatessnapshot to both controller constructors.Design notes
FeatureGateAccessdefault handler callsos.Exit(0)on change, so the process restarts and picks up new gate values. No need for runtime re-checking.service-ca.service-serving-signer(signing CA),service-ca.service-serving(all per-service serving certs).nilprovider preserves legacy behavior — when the gate is off,configurablePKIEnabledisfalseand the legacycrypto.MakeSelfSignedCAConfig/ca.MakeServerCertpaths are used. Zero impact on existing clusters.