OTA-1836: Honor the centralized TLS configuration#1338
Open
DavidHurta wants to merge 3 commits intoopenshift:mainfrom
Open
OTA-1836: Honor the centralized TLS configuration#1338DavidHurta wants to merge 3 commits intoopenshift:mainfrom
DavidHurta wants to merge 3 commits intoopenshift:mainfrom
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
Contributor
Author
|
/test ? |
|
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:
WalkthroughAdds APIServer informer propagation to the CVO, implements a TLS profile manager that applies APIServer TLS profiles to the metrics server (with CLI-configurable overrides), validates override inputs at startup, and expands tests to cover profile application, overrides, and informer-driven updates. Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI
participant Start
participant CVO
participant Informer
participant TLSMgr
participant Metrics
User->>CLI: launch with flags (--tls-min-version, --tls-cipher-suites)
CLI->>Start: populate MetricsOptions
Start->>CVO: New(..., APIServerInformer)
CVO->>Informer: register Add/Update/Delete handlers
Start->>Metrics: RunMetrics(ctx, restConfig, APIServerInformer, options)
Metrics->>Informer: WaitForCacheSync
Informer-->>Metrics: cache synced
Metrics->>TLSMgr: initialize (read central profile)
Informer->>TLSMgr: Add/Update/Delete events
TLSMgr->>Metrics: provide per-handshake tls.Config (central profile + overrides)
Metrics-->>User: serve metrics with resolved TLS config
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 10 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DavidHurta 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 |
openshift-ci
Bot
added
the
approved
Contributor
Author
|
/test e2e-agnostic-operator |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
pkg/start/start.go (1)
358-363:⚠️ Potential issue | 🟠 MajorWait for the APIServer informer before starting metrics.
RunMetricsnow reads the TLS profile throughcontrollerCtx.CVO.APIServerLister(), but this path still only blocks onClusterVersionInformerFactory.WaitForCacheSyncabove. The APIServer informer is created later inNewControllerContext, so on a fresh leader transition its cache can still be cold here and the first handshakes will race an empty/NotFound profile. Please gate metrics startup on the APIServer informer, or the fullConfigInformerFactory, being synced whenRespectCentralTLSProfileis enabled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/start/start.go` around lines 358 - 363, The metrics server is started without ensuring the APIServer informer (or the full ConfigInformerFactory) has synced, causing races when RunMetrics reads TLS profiles via controllerCtx.CVO.APIServerLister(); update the metrics startup to, when RespectCentralTLSProfile is enabled, wait for the APIServer informer (or controllerCtx.ConfigInformerFactory) to be synced before spawning the goroutine that calls cvo.RunMetrics (check o.MetricsOptions.ListenAddress and o.RespectCentralTLSProfile, then call the appropriate WaitForCacheSync on the APIServer informer or ConfigInformerFactory from controllerCtx and only start the metrics goroutine after that returns true).pkg/cvo/metrics.go (2)
353-360:⚠️ Potential issue | 🟠 MajorReject missing
apiServerListerat startup.If
RespectCentralTLSProfileis true andapiServerListeris nil, the process won't fail until Line 485, where the first handshake dereferences it. Please turn that into an early configuration error.Suggested fix
func RunMetrics(runContext context.Context, shutdownContext context.Context, restConfig *rest.Config, apiServerLister configlistersv1.APIServerLister, options MetricsOptions) error { if options.ListenAddress == "" { return errors.New("listen address is required to serve metrics") } if options.DisableAuthentication && !options.DisableAuthorization { return errors.New("invalid configuration: cannot enable authorization without authentication") } + if options.RespectCentralTLSProfile && apiServerLister == nil { + return errors.New("apiServerLister is required when RespectCentralTLSProfile is enabled") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cvo/metrics.go` around lines 353 - 360, Add an early validation in RunMetrics to reject a nil apiServerLister when options.RespectCentralTLSProfile is true: after the existing ListenAddress and auth checks, check if options.RespectCentralTLSProfile && apiServerLister == nil and return a clear configuration error (e.g. "apiServerLister is required when RespectCentralTLSProfile is true"). This prevents the later nil dereference in TLS handshake code that relies on apiServerLister.
467-490:⚠️ Potential issue | 🔴 CriticalSynchronize
lastValidProfilein the TLS callback.
lastValidProfileis captured byGetConfigForClientand then read and overwritten on each handshake without synchronization. That is a data race, and concurrent handshakes can also write back an older snapshot after a profile change.Suggested fix
import ( "context" "crypto/tls" "crypto/x509" "errors" "fmt" "net" "net/http" "slices" + "sync" "time" @@ - var lastValidProfile *cachedTLSProfile + var ( + lastValidProfile *cachedTLSProfile + lastValidProfileMu sync.Mutex + ) @@ if options.RespectCentralTLSProfile { + lastValidProfileMu.Lock() profile, err := getAPIServerTLSProfile(apiServerLister, lastValidProfile) + if err == nil { + lastValidProfile = profile + } + lastValidProfileMu.Unlock() if err != nil { return nil, fmt.Errorf("failed to get TLS profile for metrics server: %w", err) } - lastValidProfile = profile profile.apply(config) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cvo/metrics.go` around lines 467 - 490, The TLS callback captures and mutates lastValidProfile unsafely; add synchronization (e.g., a package-local sync.RWMutex like lastValidProfileMu) and use RLock when reading and Lock when updating to prevent data races and stale overwrites in GetConfigForClient; wrap the call to getAPIServerTLSProfile and the assignment lastValidProfile = profile (and the subsequent profile.apply(config) if it relies on the stored state) inside the mutex so readers/writers are serialized and the cachedTLSProfile is updated atomically.
🤖 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/cvo/metrics.go`:
- Around line 176-180: The log at the klog.Infof call prints profile.Ciphers
using %s which causes invalid formatting; update the klog.Infof in the TLS
profile change block (after tlsprofile.NewTLSConfigFromProfile and the
unsupportedCiphers check) to format the cipher list correctly by using a verbs
that match the type (e.g., %v) or join the slice into a string (e.g.,
strings.Join(profile.Ciphers, ",")). Ensure you import strings if you choose
Join and keep the rest of the message unchanged.
---
Outside diff comments:
In `@pkg/cvo/metrics.go`:
- Around line 353-360: Add an early validation in RunMetrics to reject a nil
apiServerLister when options.RespectCentralTLSProfile is true: after the
existing ListenAddress and auth checks, check if
options.RespectCentralTLSProfile && apiServerLister == nil and return a clear
configuration error (e.g. "apiServerLister is required when
RespectCentralTLSProfile is true"). This prevents the later nil dereference in
TLS handshake code that relies on apiServerLister.
- Around line 467-490: The TLS callback captures and mutates lastValidProfile
unsafely; add synchronization (e.g., a package-local sync.RWMutex like
lastValidProfileMu) and use RLock when reading and Lock when updating to prevent
data races and stale overwrites in GetConfigForClient; wrap the call to
getAPIServerTLSProfile and the assignment lastValidProfile = profile (and the
subsequent profile.apply(config) if it relies on the stored state) inside the
mutex so readers/writers are serialized and the cachedTLSProfile is updated
atomically.
In `@pkg/start/start.go`:
- Around line 358-363: The metrics server is started without ensuring the
APIServer informer (or the full ConfigInformerFactory) has synced, causing races
when RunMetrics reads TLS profiles via controllerCtx.CVO.APIServerLister();
update the metrics startup to, when RespectCentralTLSProfile is enabled, wait
for the APIServer informer (or controllerCtx.ConfigInformerFactory) to be synced
before spawning the goroutine that calls cvo.RunMetrics (check
o.MetricsOptions.ListenAddress and o.RespectCentralTLSProfile, then call the
appropriate WaitForCacheSync on the APIServer informer or ConfigInformerFactory
from controllerCtx and only start the metrics goroutine after that returns
true).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b2ebb59-479e-4c67-b71f-fec6dbcfc90e
⛔ Files ignored due to path filters (291)
go.sumis excluded by!**/*.sumvendor/github.com/evanphx/json-patch/v5/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/evanphx/json-patch/v5/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/fold.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/fuzz.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/indent.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/scanner.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/stream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/tables.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/tags.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/evanphx/json-patch/v5/merge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/evanphx/json-patch/v5/patch.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/google/btree/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/btree/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/btree/btree.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/btree/btree_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/pprof/profile/merge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/pprof/profile/profile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/pprof/profile/proto.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/pprof/profile/prune.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/format/format.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gomega_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/assertion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/async_assertion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/duration_bundle.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/gomega.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/polling_signal_error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/vetoptdesc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/and.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/assignable_to_type_of_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_a_directory.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_a_regular_file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_an_existing_file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_closed_matcher.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/be_element_of_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_empty_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_equivalent_to_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_false_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_identical_to.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_key_of_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_nil_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_numerically_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_sent_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_temporally_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_true_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_zero_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/consist_of.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/contain_element_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/contain_elements_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/contain_substring_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/equal_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_cap_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_each_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_exact_elements.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_existing_field_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_field.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_http_body_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_http_header_with_value_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_http_status_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_key_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_key_with_value_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_len_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_occurred_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_prefix_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_suffix_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_value.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/internal/miter/type_support_iter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/internal/miter/type_support_noiter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_error_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_error_strictly_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_json_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_regexp_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_xml_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_yaml_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/not.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/or.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/panic_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/receive_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/satisfy_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/semi_structured_data_support.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/succeed_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/bipartitegraph/bipartitegraph.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/edge/edge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/node/node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/type_support.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/with_transform.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/types/types.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_infrastructure.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_network.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.crd-manifests/0000_10_config-operator_01_apiservers-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-OKD.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_insightsdatagathers-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_insightsdatagathers-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_insightsdatagathers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**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_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_insights.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/types_insights.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha2/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*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_network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/types_clusterapi.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_30_cluster-api_01_clusterapis-CustomNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_30_cluster-api_01_clusterapis-DevPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.crd-manifests/0000_30_cluster-api_01_clusterapis-TechPreviewNoUpgrade.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/controller-runtime-common/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/controller.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/controller-runtime-common/pkg/tls/tls.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/crypto/crypto.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/manifest/manifest.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/api/client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/desc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/internal/difflib.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/internal/go_runtime_metrics.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/labels.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/metric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/process_collector_darwin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/process_collector_mem_nocgo_darwin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/process_collector_procfsenabled.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/vec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/wrap.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/config/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/config/headers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/config/http_config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/expfmt.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/fuzz.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/openmetrics_create.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/text_create.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/text_parse.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/alert.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/labels.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/labelset.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/metric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/time.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/value.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/value_histogram.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/value_type.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/Makefile.commonis excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/arp.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/fs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/fs_statfs_notype.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/fscache.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/internal/fs/fs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/internal/util/parse.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/internal/util/sysreadfile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/mountstats.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_dev_snmp6.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_ip_socket.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_protocols.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_tcp.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/proc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/proc_cgroup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/proc_io.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/proc_netstat.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/proc_smaps.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/proc_snmp.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/proc_snmp6.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/proc_status.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/proc_sys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/softirqs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/bool_func.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/count.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/flag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/func.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/golangflag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/ipnet_slice.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/string_to_string.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/text.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/time.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/transport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc9218.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/trace/events.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/clientcredentials/clientcredentials.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/internal/doc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/internal/oauth2.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/internal/token.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/internal/transport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/oauth2.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/pkce.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/token.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/transport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sync/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sync/PATENTSis excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sync/errgroup/errgroup.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/mkerrors.shis excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_386.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_arm.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_loong64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_mips.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_mips64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_mips64le.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_mipsle.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_ppc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_ppc64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_ppc64le.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_riscv64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_s390x.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_sparc64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_netbsd_arm.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/term/terminal.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/encoding/japanese/eucjp.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/encoding/japanese/iso2022jp.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/encoding/japanese/shiftjis.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/encoding/korean/euckr.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/encoding/simplifiedchinese/gbk.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/encoding/simplifiedchinese/hzgb2312.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/encoding/traditionalchinese/big5.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/encoding/unicode/unicode.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ast/inspector/cursor.gois excluded by!**/vendor/**,!vendor/**vendor/gomodules.xyz/jsonpatch/v2/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/gomodules.xyz/jsonpatch/v2/jsonpatch.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/encoding/protowire/wire.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/editiondefaults/editions_defaults.binpbis excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/editions.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/presence.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/genid/api_gen.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/genid/descriptor_gen.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/impl/codec_message_opaque.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/impl/message_opaque.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/impl/presence.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/strs/strings_unsafe.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/strs/strings_unsafe_go120.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/version/version.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/proto/merge.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/reflect/protoreflect/source_gen.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/reflect/protoreflect/value_unsafe.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/reflect/protoreflect/value_unsafe_go120.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/types/descriptorpb/descriptor.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/types/gofeaturespb/go_features.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/types/known/anypb/any.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/types/known/durationpb/duration.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/types/known/emptypb/empty.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/types/known/structpb/struct.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/types/known/timestamppb/timestamp.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/types/known/wrapperspb/wrappers.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/gopkg.in/evanphx/json-patch.v4/README.mdis excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/evanphx/json-patch.v4/patch.gois excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v2/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v2/LICENSE.libyamlis excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v2/README.mdis excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v2/apic.gois excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v2/decode.gois excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v2/emitterc.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (9)
go.modlib/manifest/manifest.gopkg/cvo/cvo.gopkg/cvo/featuregate_integration_test.gopkg/cvo/metrics.gopkg/cvo/sync_worker.gopkg/payload/payload.gopkg/payload/render.gopkg/start/start.go
openshift-merge-robot
added
the
needs-rebase
DavidHurta
force-pushed
the
central-tls-profile
branch
from
3f595e0 to
1362d3f
Compare
openshift-merge-robot
removed
the
needs-rebase
DavidHurta
changed the title
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@DavidHurta: This pull request references OTA-1764 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
DavidHurta
force-pushed
the
central-tls-profile
branch
2 times, most recently
from
9990e0a to
f9d748d
Compare
Contributor
Author
|
/test all |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pkg/cvo/metrics.go (1)
164-170: Remove or clarify the dead code block.The
if falseblock with the TODO comment is unreachable and will never execute. If this is a placeholder for future API changes, consider using a build tag or feature flag instead, or remove it entirely until the API changes are ready.♻️ Suggested approach
Either remove the dead code block entirely:
- if false { // TODO: Add TLS adherence logic when API changes merge - return &cachedTLSProfile{ - spec: nil, - apply: func(config *tls.Config) {}, // do nothing - generation: apiServer.Generation, - }, nil - } -Or if keeping it as a placeholder, add a clearer skip mechanism:
+ // TODO(OTA-XXXX): Add TLS adherence logic when API changes merge + // When implemented, check apiServer.Spec.TLSSecurityProfile adherence field here🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cvo/metrics.go` around lines 164 - 170, The unreachable "if false" block should be removed or replaced with a proper toggle; delete the dead branch that returns a cachedTLSProfile with spec nil/apply noop (references: cachedTLSProfile, apiServer.Generation, tls.Config) or, if you need a placeholder, replace it with a clear feature flag/build-tag guard (not "if false") and document the intent; ensure any retained placeholder still compiles and preserves the intended return shape for the surrounding function.
🤖 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`:
- Around line 29-36: The go.mod shows a mismatch: most k8s modules are v0.35.1
while k8s.io/kube-aggregator remains v0.34.1; verify compatibility and align
versions by either upgrading k8s.io/kube-aggregator to a v0.35.x release if
available, or downgrade the other k8s modules to the kube-aggregator compatible
version (e.g., v0.34.x), or add a go.mod replace to force a tested compatible
commit; update the k8s module entries (k8s.io/kube-aggregator and the other
k8s.io/* lines) so all Kubernetes component modules use a mutually compatible
version set and run go mod tidy and go test to ensure no API/type mismatches.
- Line 3: The go.mod contains an invalid Go version "go 1.25.0"; update the
module's go directive to a valid released version (e.g., "go 1.25.8" or "go
1.26.1") by replacing the current go directive value so the go directive in
go.mod reflects an actual Go release.
---
Nitpick comments:
In `@pkg/cvo/metrics.go`:
- Around line 164-170: The unreachable "if false" block should be removed or
replaced with a proper toggle; delete the dead branch that returns a
cachedTLSProfile with spec nil/apply noop (references: cachedTLSProfile,
apiServer.Generation, tls.Config) or, if you need a placeholder, replace it with
a clear feature flag/build-tag guard (not "if false") and document the intent;
ensure any retained placeholder still compiles and preserves the intended return
shape for the surrounding function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b19eb2ef-b766-4f59-b7e5-9d3710e2fd62
⛔ Files ignored due to path filters (113)
go.sumis excluded by!**/*.sumvendor/github.com/evanphx/json-patch/v5/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/decode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/encode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/fold.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/fuzz.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/indent.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/scanner.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/stream.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/tables.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/tags.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/merge.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/patch.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/btree/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/btree/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/btree/btree.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/btree/btree_generic.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/pprof/profile/merge.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/pprof/profile/profile.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/pprof/profile/proto.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/pprof/profile/prune.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/format/format.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/gomega_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/assertion.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/async_assertion.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/duration_bundle.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/gomega.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/polling_signal_error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/vetoptdesc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/and.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/assignable_to_type_of_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_a_directory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_a_regular_file.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_an_existing_file.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_closed_matcher.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/be_element_of_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_empty_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_equivalent_to_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_false_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_identical_to.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_key_of_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_nil_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_numerically_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_sent_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_temporally_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_true_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_zero_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/consist_of.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/contain_element_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/contain_elements_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/contain_substring_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/equal_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_cap_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_each_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_exact_elements.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_existing_field_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_field.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_http_body_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_http_header_with_value_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_http_status_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_key_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_key_with_value_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_len_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_occurred_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_prefix_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_suffix_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_value.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/match_error_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/match_error_strictly_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/match_json_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/match_regexp_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/match_xml_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/match_yaml_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/not.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/or.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/panic_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/receive_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/satisfy_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/semi_structured_data_support.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/succeed_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/bipartitegraph/bipartitegraph.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/edge/edge.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/node/node.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/type_support.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/with_transform.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/types/types.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_infrastructure.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_network.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.crd-manifests/0000_10_config-operator_01_apiservers-CustomNoUpgrade.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-Default.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-DevPreviewNoUpgrade.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-OKD.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-TechPreviewNoUpgrade.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-CustomNoUpgrade.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-Default.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-DevPreviewNoUpgrade.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-OKD.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_authentications-TechPreviewNoUpgrade.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-OKD.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_insightsdatagathers-CustomNoUpgrade.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_insightsdatagathers-DevPreviewNoUpgrade.crd.yamlis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (9)
go.modlib/manifest/manifest.gopkg/cvo/cvo.gopkg/cvo/featuregate_integration_test.gopkg/cvo/metrics.gopkg/cvo/sync_worker.gopkg/payload/payload.gopkg/payload/render.gopkg/start/start.go
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/cvo/featuregate_integration_test.go
- lib/manifest/manifest.go
- pkg/payload/render.go
| module github.com/openshift/cluster-version-operator | ||
|
|
||
| go 1.24.0 | ||
| go 1.25.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the latest stable Go releases from golang.org
# Fetch the list of available Go versions
curl -s 'https://go.dev/dl/?mode=json' | jq -r '.[].version' | head -10
# Check if go1.25.0 exists in the official releases
curl -s 'https://go.dev/dl/?mode=json' | jq -r '.[].version' | grep -q 'go1.25.0' && echo "go1.25.0 exists" || echo "go1.25.0 does NOT exist"Repository: openshift/cluster-version-operator
Length of output: 119
Update Go version to a valid release.
Go 1.25.0 does not exist as an official release. Valid alternatives include go1.24.x, go1.25.8, or go1.26.1. Update go 1.25.0 to a valid version:
-go 1.25.0
+go 1.25.8📝 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
| go 1.25.0 | |
| go 1.25.8 |
🤖 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 contains an invalid Go version "go 1.25.0";
update the module's go directive to a valid released version (e.g., "go 1.25.8"
or "go 1.26.1") by replacing the current go directive value so the go directive
in go.mod reflects an actual Go release.
f9d748d to
8553636
Compare
Contributor
Author
|
/test ? |
Contributor
Author
|
/test tls-scanner |
DavidHurta
force-pushed
the
central-tls-profile
branch
from
8553636 to
7b9c98f
Compare
Contributor
Author
|
/test tls-scanner |
Contributor
Author
|
/test tls-scanner |
DavidHurta
changed the title
DavidHurta
force-pushed
the
central-tls-profile
branch
from
70099c9 to
29e1401
Compare
Contributor
Author
|
/test tls-scanner |
Draft
4 tasks
DavidHurta
force-pushed
the
central-tls-profile
branch
2 times, most recently
from
e7776e4 to
203183d
Compare
Contributor
Author
|
/hold Waiting for other PRs in the repository to merge due to dependency bumps and linting warnings. However, the core of the PR, the last two commits, are ready for feedback. |
openshift-ci
Bot
added
do-not-merge/hold
Contributor
|
@DavidHurta: This pull request references OTA-1764 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
DavidHurta
changed the title
Contributor
|
@DavidHurta: This pull request references OTA-1836 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 story 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
|
@DavidHurta: This pull request references OTA-1836 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Contributor
|
@DavidHurta: This pull request references OTA-1836 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Contributor
|
@DavidHurta: This pull request references OTA-1836 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Contributor
Author
|
/hold |
openshift-ci
Bot
added
the
needs-rebase
DavidHurta
force-pushed
the
central-tls-profile
branch
from
203183d to
ee6f9e8
Compare
openshift-ci
Bot
removed
the
needs-rebase
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 `@pkg/cvo/metrics.go`:
- Line 450: RunMetrics currently dereferences apiServerInformer without checking
for nil, which can panic; add a nil guard at the start of RunMetrics (the
function) to validate apiServerInformer and return a descriptive error if it's
nil before any use (including code paths that access
apiServerInformer.Informer() or apiServerInformer.Lister() later in the
function). Ensure the check covers all execution paths that reference
apiServerInformer (the paths around the existing uses at the APIServer informer
handling) and return an error rather than proceeding to avoid a runtime panic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: aefadc1b-4bb6-480e-90c5-13053e84cb5f
⛔ Files ignored due to path filters (41)
go.sumis excluded by!**/*.sum,!go.sumvendor/github.com/evanphx/json-patch/v5/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/decode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/encode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/fold.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/fuzz.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/indent.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/scanner.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/stream.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/tables.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/internal/json/tags.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/merge.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/evanphx/json-patch/v5/patch.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/btree/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/btree/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/google/btree/btree.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/btree/btree_generic.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/pprof/profile/merge.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/pprof/profile/profile.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/pprof/profile/proto.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/google/pprof/profile/prune.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/format/format.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/gomega_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_key_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/have_key_with_value_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/match_error_strictly_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/edge/edge.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/.coderabbit.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/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.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/types_dns.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (7)
cmd/cluster-version-operator/start.gogo.modpkg/cvo/cvo.gopkg/cvo/metrics.gopkg/cvo/metrics_test.gopkg/start/start.gopkg/start/start_integration_test.go
✅ Files skipped from review due to trivial changes (1)
- go.mod
Contributor
Author
|
/test e2e-agnostic-ovn-upgrade-into-change |
Contributor
Author
|
/test ? |
Contributor
Author
|
/test tls-scanner |
Honor the central TLS profile [1] with event-driven dynamic updates. Implementation uses an APIServer informer with event handlers to proactively cache TLS settings, eliminating per-handshake lister calls while maintaining dynamic reconfiguration capability. The cached settings are applied during TLS handshakes via GetConfigForClient. The commit aims to focus on availability over strict consistency on errors, such as an error fetching the API server object. The CVO provides critical metrics and as such, I am inclined towards availability instead of strict TLS configuration consistency. The TLS adherence feature is currently in Tech Preview. Components do not need to check the feature gate explicitly though [2]: > Component Interaction with the Feature Gate: The feature gate controls > whether the tlsAdherence field is accepted by the API server — > components themselves do not need to check the feature gate. > Because the field is optional (+optional, omitempty), components only > need to handle the field's value when unmarshaling the APIServer config > ... > This means components do not need to set up feature gate watching or > add feature-gate-specific code paths. The ShouldHonorClusterTLSProfile > helper in library-go encapsulates all of this logic. The ShouldHonorClusterTLSProfile helper from library-go encapsulates this logic. Configuration precedence: crypto defaults → central profile → overrides (override support added in next commit for HyperShift compatibility). [1]: https://github.com/openshift/enhancements/blob/master/enhancements/security/centralized-tls-config.md [2]: https://github.com/openshift/enhancements/blob/master/enhancements/security/centralized-tls-config.md#feature-gate Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
…perShift Add --tls-min-version and --tls-cipher-suites flags based on recommendations from the centralized TLS config enhancement [1] to support HyperShift deployments: > When these flags are set by the CPO, they take precedence over any > value the component would read from > apiservers.config.openshift.io/cluster. When they are not set, the > component falls back to its normal behavior of watching the cluster config. This allows hosted control planes components, which are deployed in the management cluster, to have different TLS setting or for the components to not need to read the management cluster Kubernetes API server. [1]: https://github.com/openshift/enhancements/blob/master/enhancements/security/centralized-tls-config.md Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
DavidHurta
force-pushed
the
central-tls-profile
branch
from
5f9f7d9 to
d2f5f50
Compare
Contributor
|
@DavidHurta: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Honor the centralized TLS configuration.
A follow-up PR openshift/hypershift#8013 after this one merges.
Summary by CodeRabbit
New Features
--tls-min-versionand--tls-cipher-suitesCLI flags for metrics TLS.Tests
Chores