OCPCLOUD-3346: tls: use centralized TLS#1456
Open
RadekManak wants to merge 15 commits intoopenshift:mainfrom
Open
OCPCLOUD-3346: tls: use centralized TLS#1456RadekManak wants to merge 15 commits intoopenshift:mainfrom
RadekManak wants to merge 15 commits intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@RadekManak: This pull request references OCPCLOUD-3346 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
Author
|
/assign @damdo |
damdo
reviewed
Jan 26, 2026
Member
damdo
left a comment
damdo
left a comment
There was a problem hiding this comment.
Thanks! Looks good, a couple of minor points.
pkg/operator/operator.go
Outdated
| var tlsProfile *osconfigv1.TLSProfileSpec | ||
| apiServer, err := optr.osClient.ConfigV1().APIServers().Get(context.Background(), "cluster", metav1.GetOptions{}) | ||
| if err != nil { | ||
| klog.Warningf("Failed to fetch APIServer, using default TLS profile: %v", err) |
Member
There was a problem hiding this comment.
Shouldn't we return here?
What would happen otherwise?
RadekManak
force-pushed
the
feature-centralized-tsl-endpoints
branch
from
4e665c0 to
24eed11
Compare
Contributor
|
@RadekManak: This pull request references OCPCLOUD-3346 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. |
RadekManak
force-pushed
the
feature-centralized-tsl-endpoints
branch
from
24eed11 to
6b35a05
Compare
damdo
reviewed
Feb 23, 2026
Member
damdo
left a comment
damdo
left a comment
There was a problem hiding this comment.
Thanks for this Radek!
I left a bunch of comments but it mostly looks good! TY
Member
|
/retest |
RadekManak
force-pushed
the
feature-centralized-tsl-endpoints
branch
from
9b911a2 to
fe1ce87
Compare
WalkthroughReplaces HTTP /metrics with a controller-runtime TLS metrics server, adds cluster TLS-profile resolution and CLI overrides, propagates TLS args into controllers/webhooks, removes kube-rbac-proxy sidecar, adds APIServer TLS-profile watcher triggering graceful shutdown on changes, and migrates event-recording APIs and tests to use events.EventRecorder. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
Contributor
|
@RadekManak: This pull request references OCPCLOUD-3346 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
Author
|
/hold This will crash baremetal until the PR merges. |
openshift-ci
bot
added
the
do-not-merge/hold
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/machine-api-operator/start.go`:
- Around line 247-251: The current goroutine fatals on any metricsServer.Start
error; change it to only treat context.Canceled as a non-fatal shutdown and exit
gracefully, while still fataling for other unexpected errors. Locate the
anonymous goroutine that calls metricsServer.Start(metricsServerCtx) and update
its error handling to check for errors.Is(err, context.Canceled) (allowing a
clean shutdown/log and not calling klog.Fatalf) and call klog.Fatalf only for
other error values; do not special-case http.ErrServerClosed because
metricsServer.Start already filters that.
In `@cmd/machineset/main.go`:
- Line 177: The call to utiltls.NewTLSConfigFromProfile is ignoring its error
(tlsOpts, _ := utiltls.NewTLSConfigFromProfile(tlsProfile)); change this to
capture and handle the error (e.g., tlsOpts, err :=
utiltls.NewTLSConfigFromProfile(tlsProfile)) and if err != nil log the error via
the existing logger or fmt and exit/return a non-zero status so the webhook
server does not start with invalid TLS settings; update the initialization path
in main (where tlsProfile and tlsOpts are used) to propagate or handle the error
appropriately.
In `@pkg/operator/sync.go`:
- Around line 865-875: In getTLSArgs, capture the second return value from
utiltls.NewTLSConfigFromProfile (the unsupportedCiphers slice) instead of
discarding it, and emit a log containing those unsupported cipher names so
profile-to-flag translation mismatches are visible; specifically, change the
call to utiltls.NewTLSConfigFromProfile to receive (tlsConfigFn,
unsupportedCiphers), and then log unsupportedCiphers (mirroring the pattern used
in cmd/machine-api-operator/start.go) before continuing to build the tlsArgs.
ℹ️ Review info
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (287)
go.sumis excluded by!**/*.sumvendor/github.com/BurntSushi/toml/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/BurntSushi/toml/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/BurntSushi/toml/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/BurntSushi/toml/error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/BurntSushi/toml/lex.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/BurntSushi/toml/meta.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/BurntSushi/toml/parse.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/dbus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/methods.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/subscription.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/subscription_set.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/journal/journal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/journal/journal_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/COPYING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/LICENSE.BSDis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/LICENSE.MPL-2.0is excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/VERSIONis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/codecov.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/deprecated_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/gocompat_generics_go121.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/gocompat_generics_unsupported.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/internal/consts/consts.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/join.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/openat2_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/openat_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/assert/assert.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/errors_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/at_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/fd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/fd_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/mount_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/openat2_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/gocompat_atomic_go119.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/gocompat_atomic_unsupported.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/gocompat_errors_go120.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/gocompat_errors_unsupported.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/gocompat_generics_go121.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/gocompat_generics_unsupported.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/kernelversion/kernel_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/linux/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/linux/mount_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/linux/openat2_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/procfs/procfs_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/procfs/procfs_lookup_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/lookup_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/mkdir_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/open_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/openat2_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/procfs/procfs_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/procfs_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cyphar/filepath-securejoin/vfs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/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/gmeasure/experiment.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/opencontainers/runtime-spec/specs-go/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/opencontainers/runtime-spec/specs-go/version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/opencontainers/selinux/go-selinux/label/label.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/opencontainers/selinux/go-selinux/label/label_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/opencontainers/selinux/go-selinux/label/label_stub.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/opencontainers/selinux/go-selinux/selinux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/opencontainers/selinux/go-selinux/selinux_stub.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/clusterimagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/custom.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gatherconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gathererconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gatherers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicyfulciocawithrekorrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicypkirootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicypublickeyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagepolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/imagesigstoreverificationpolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/insightsdatagather.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/insightsdatagatherspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/persistentvolumeclaimreference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/persistentvolumeconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/policyrootoftrust.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/storage.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/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/imagesigstoreverificationpolicy.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/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1/config_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1/fake/fake_config_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1/fake/fake_insightsdatagather.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1/generated_expansion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1/insightsdatagather.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1/insightsdatagather.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1/interface.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/v1/expansion_generated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1/insightsdatagather.gois excluded by!**/vendor/**,!vendor/**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/operator/certrotation/signer.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/chacha20/chacha_arm64.sis excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/ssh/keys.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/ssh/messages.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/ssh/ssh_gss.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/ssh/streamlocal.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/ssh/tcpip.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/mod/modfile/print.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/mod/modfile/read.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/mod/modfile/rule.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/mod/module/module.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/mod/semver/semver.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/context/context.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/frame.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/transport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/writesched.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc7540.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/net/websocket/hybi.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sync/errgroup/errgroup.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/cpu/cpu_arm64.sis excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/cpu/cpu_x86.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/mkerrors.shis excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux.gois 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/zsyscall_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_netbsd_arm.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/syscall_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/types_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/zsyscall_windows.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/analysis/diagnostic.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/appends/appends.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/asmdecl/asmdecl.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/assign/assign.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/atomic/atomic.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/atomicalign/atomicalign.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/bools/bools.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/buildssa/buildssa.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/buildtag/buildtag.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/cgocall/cgocall.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/copylock/copylock.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/ctrlflow/ctrlflow.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/deepequalerrors/deepequalerrors.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/defers/defers.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/directive/directive.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/errorsas/errorsas.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/fieldalignment/fieldalignment.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/framepointer/framepointer.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/httpresponse/httpresponse.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/ifaceassert/ifaceassert.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/inspect/inspect.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/internal/analysisutil/util.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/loopclosure/loopclosure.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/lostcancel/lostcancel.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/nilfunc/nilfunc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/nilness/nilness.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/pkgfact/pkgfact.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/printf/doc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/printf/printf.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/printf/types.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/reflectvaluecompare/reflectvaluecompare.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/shadow/shadow.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/shift/shift.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/sigchanyzer/sigchanyzer.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/slog/slog.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/sortslice/analyzer.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/stdmethods/stdmethods.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/stdversion/stdversion.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/stringintconv/string.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/structtag/structtag.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine/testinggoroutine.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine/util.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/tests/tests.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/timeformat/timeformat.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/unmarshal/unmarshal.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/unreachable/unreachable.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/unsafeptr/unsafeptr.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/unusedresult/unusedresult.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/unusedwrite/unusedwrite.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/waitgroup/waitgroup.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ast/astutil/imports.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ast/inspector/cursor.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/buildutil/allpackages.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/buildutil/tags.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/cfg/builder.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/cfg/cfg.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/packages/golist.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/packages/packages.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/packages/visit.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ssa/builder.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ssa/create.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ssa/emit.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ssa/func.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ssa/instantiate.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ssa/ssa.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ssa/ssautil/visit.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ssa/subst.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ssa/util.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/types/objectpath/objectpath.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/types/typeutil/callee.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/types/typeutil/map.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/imports/forward.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/analysis/analyzerutil/doc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/analysis/analyzerutil/extractdoc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/analysis/analyzerutil/readfile.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/analysis/analyzerutil/version.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/analysis/typeindex/typeindex.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/analysisinternal/analysis.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/astutil/comment.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/astutil/equal.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/astutil/stringlit.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/astutil/util.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/event/core/event.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/event/core/export.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/event/label/label.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/gcimporter/bimport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/gcimporter/iexport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/gcimporter/iimport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/imports/fix.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/imports/sortimports.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/modindex/index.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/modindex/lookup.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/modindex/symbols.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/moreiters/iters.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/packagepath/packagepath.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/refactor/delete.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/refactor/edit.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/refactor/imports.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/refactor/refactor.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/deps.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/import.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/manifest.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/stdlib.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typeparams/normalize.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/classify_call.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/element.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/fx.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/isnamed.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/qualifier.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/typeindex/typeindex.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/types.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (13)
cmd/machine-api-operator/start.gocmd/machineset/main.gogo.modhack/go-build.shinstall/0000_30_machine-api-operator_09_rbac.yamlinstall/0000_30_machine-api-operator_11_deployment.yamlpkg/controller/machine/machine_controller_test.gopkg/operator/config.gopkg/operator/operator.gopkg/operator/operator_test.gopkg/operator/sync.gopkg/operator/sync_test.gopkg/webhooks/machine_webhook.go
Contributor
|
@RadekManak: This pull request references OCPCLOUD-3346 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. |
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 `@cmd/machine-api-operator/start.go`:
- Around line 299-316: The code must avoid converting a shutdown-triggered cache
sync failure into a hard crash: in the initMachineAPIInformers() path where
cache.WaitForCacheSync(...) is currently followed by klog.Fatal on false, change
the logic to detect a shutdown caused by
setupTLSProfileWatcher/handleTLSProfileEvent calling shutdown() (i.e., the stop
channel being closed or context cancelled) and return an error or nil to allow
graceful restart instead of calling klog.Fatal; update initMachineAPIInformers()
to check the stop channel or context before deciding to fatal, and ensure
setupTLSProfileWatcher/handleTLSProfileEvent continue to call shutdown() as
before.
ℹ️ Review info
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
cmd/machine-api-operator/start.gocmd/machineset/main.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/machineset/main.go
cmd/machine-api-operator/start.go
Outdated
Comment on lines
+299
to
+316
| func setupTLSProfileWatcher(ctx *ControllerContext, shutdown func()) error { | ||
| configClient := ctx.ClientBuilder.OpenshiftClientOrDie("tls-profile-watcher") | ||
| initialProfile, err := fetchAPIServerTLSProfileSpec(context.Background(), configClient) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| klog.Fatal(server.ListenAndServe()) | ||
|
|
||
| apiServerInformer := ctx.ConfigInformerFactory.Config().V1().APIServers().Informer() | ||
| _, err = apiServerInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ | ||
| AddFunc: func(obj interface{}) { | ||
| handleTLSProfileEvent(obj, &initialProfile, shutdown) | ||
| }, | ||
| UpdateFunc: func(_, newObj interface{}) { | ||
| handleTLSProfileEvent(newObj, &initialProfile, shutdown) | ||
| }, | ||
| DeleteFunc: func(obj interface{}) { | ||
| handleTLSProfileEvent(obj, &initialProfile, shutdown) | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify control flow: read the key sections mentioned in the review
echo "=== Section 1: Lines 96-140 (startup context) ==="
sed -n '96,140p' cmd/machine-api-operator/start.go
echo -e "\n=== Section 2: Lines 143-153 (cache sync with fatal) ==="
sed -n '143,153p' cmd/machine-api-operator/start.go
echo -e "\n=== Section 3: Lines 299-370 (setupTLSProfileWatcher) ==="
sed -n '299,370p' cmd/machine-api-operator/start.goRepository: openshift/machine-api-operator
Length of output: 4655
Prevent fatal crash when TLS watcher triggers shutdown before cache sync completes.
shutdown() can be called from APIServer events after the TLS watcher is registered but before initMachineAPIInformers() cache sync finishes. This closes stopCh, causing cache.WaitForCacheSync() to return false, which unconditionally triggers klog.Fatal() at line 150—converting an intended graceful restart into a hard crash.
Suggested hardening
func initMachineAPIInformers(ctx *ControllerContext) {
mInformer := ctx.MachineInformerFactory.Machine().V1beta1().Machines().Informer()
msInformer := ctx.MachineInformerFactory.Machine().V1beta1().MachineSets().Informer()
ctx.MachineInformerFactory.Start(ctx.Stop)
if !cache.WaitForCacheSync(ctx.Stop,
mInformer.HasSynced,
msInformer.HasSynced) {
+ select {
+ case <-ctx.Stop:
+ klog.V(2).Info("Skipping Machine API informer sync due to shutdown")
+ return
+ default:
+ klog.Fatal("Failed to sync caches for Machine api informers")
+ }
}
klog.Info("Synced up machine api informer caches")
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/machine-api-operator/start.go` around lines 299 - 316, The code must
avoid converting a shutdown-triggered cache sync failure into a hard crash: in
the initMachineAPIInformers() path where cache.WaitForCacheSync(...) is
currently followed by klog.Fatal on false, change the logic to detect a shutdown
caused by setupTLSProfileWatcher/handleTLSProfileEvent calling shutdown() (i.e.,
the stop channel being closed or context cancelled) and return an error or nil
to allow graceful restart instead of calling klog.Fatal; update
initMachineAPIInformers() to check the stop channel or context before deciding
to fatal, and ensure setupTLSProfileWatcher/handleTLSProfileEvent continue to
call shutdown() as before.
1659685 to
8048f70
Compare
Contributor
|
@RadekManak: This pull request references OCPCLOUD-3346 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. |
Remove the kube-rbac-proxy sidecar, mount the serving cert, and restart the operator on APIServer TLS profile changes.
Capture the APIServer TLS profile in operator config and use it to configure kube-rbac-proxy TLS args, with unit coverage.
Add unit tests to verify TLS configuration handling in newKubeProxyContainer, including tests for TLS 1.2 with cipher suites and TLS 1.3 without cipher suites.
Move TLS configuration computation from per-container to once per batch in newKubeProxyContainers. This avoids redundant processing when creating multiple kube-rbac-proxy containers with the same TLS profile.
Introduce command-line flags for TLS cipher suites and minimum version in the machineset controller. Update the container creation logic to utilize these TLS settings
This ensures Metal3 remediation webhooks use the configured TLS profile while keeping other controllers unchanged.
Mirror machine-api-operator logging so ignored ciphers are visible during startup.
Only apply cluster TLS profile arguments when adherence requires it, and restart on either TLS profile or adherence-policy changes.
RadekManak
force-pushed
the
feature-centralized-tsl-endpoints
branch
from
8048f70 to
28078b4
Compare
Contributor
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/machine-api-operator/start.go (1)
149-153:⚠️ Potential issue | 🟡 MinorConsider graceful handling when shutdown occurs during cache sync.
If the TLS profile watcher triggers shutdown while
WaitForCacheSyncis in progress, it will returnfalseand cause aFatalexit rather than a graceful restart. This is a narrow timing window but could cause unexpected crashes during startup.♻️ Suggested adjustment
if !cache.WaitForCacheSync(ctx.Stop, mInformer.HasSynced, msInformer.HasSynced) { + select { + case <-ctx.Stop: + klog.Info("Cache sync interrupted by shutdown, exiting gracefully") + return + default: + } klog.Fatal("Failed to sync caches for Machine api informers") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/machine-api-operator/start.go` around lines 149 - 153, WaitForCacheSync can return false because ctx.Stop was closed during shutdown; instead of unconditionally calling klog.Fatal, check whether the context was cancelled and handle that gracefully. Modify the block around cache.WaitForCacheSync to detect cancellation (inspect ctx.Done()/ctx.Err()) when it returns false: if the context is cancelled, log an informational message and return/exit cleanly (no klog.Fatal), otherwise keep the klog.Fatal for real sync failures; reference cache.WaitForCacheSync, ctx.Stop (or ctx.Done/ctx.Err), mInformer.HasSynced, msInformer.HasSynced, and klog.Fatal when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@cmd/machine-api-operator/start.go`:
- Around line 149-153: WaitForCacheSync can return false because ctx.Stop was
closed during shutdown; instead of unconditionally calling klog.Fatal, check
whether the context was cancelled and handle that gracefully. Modify the block
around cache.WaitForCacheSync to detect cancellation (inspect
ctx.Done()/ctx.Err()) when it returns false: if the context is cancelled, log an
informational message and return/exit cleanly (no klog.Fatal), otherwise keep
the klog.Fatal for real sync failures; reference cache.WaitForCacheSync,
ctx.Stop (or ctx.Done/ctx.Err), mInformer.HasSynced, msInformer.HasSynced, and
klog.Fatal when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 90175a7d-7894-4755-80a7-620d53d89e07
⛔ Files ignored due to path filters (274)
go.sumis excluded by!**/*.sumvendor/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/gmeasure/experiment.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/Makefileis 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_cluster_version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusterversions-CustomNoUpgrade.crd.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusterversions-DevPreviewNoUpgrade.crd.yamlis 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-DevPreviewNoUpgrade.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.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/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.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/applyconfigurations/utils.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/fake/fake_clusterimagepolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/fake/fake_config_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/fake/fake_imagepolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/fake/fake_pki.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/machine/applyconfigurations/internal/internal.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/internal/internal.gois excluded by!vendor/**,!**/vendor/**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/tls_adherence.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/library-go/pkg/operator/v1helpers/helpers.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/mod/modfile/print.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/mod/modfile/read.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/mod/modfile/rule.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/mod/module/module.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/mod/semver/semver.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/net/websocket/hybi.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sync/errgroup/errgroup.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/cpu/cpu.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/cpu/cpu_arm64.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/cpu/cpu_arm64.sis excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/cpu/cpu_gc_arm64.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/cpu/cpu_gccgo_arm64.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/cpu/cpu_netbsd_arm64.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/cpu/cpu_openbsd_arm64.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/sys/cpu/cpu_x86.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/analysis/diagnostic.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/appends/appends.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/asmdecl/asmdecl.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/assign/assign.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/atomic/atomic.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/bools/bools.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/buildssa/buildssa.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/buildtag/buildtag.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/cgocall/cgocall.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/copylock/copylock.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/ctrlflow/ctrlflow.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/deepequalerrors/deepequalerrors.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/defers/defers.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/directive/directive.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/errorsas/errorsas.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/fieldalignment/fieldalignment.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/framepointer/framepointer.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/ifaceassert/ifaceassert.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/inspect/inspect.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/internal/analysisutil/util.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/loopclosure/loopclosure.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/lostcancel/lostcancel.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/nilfunc/nilfunc.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/nilness/nilness.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/printf/doc.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/printf/printf.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/printf/types.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/reflectvaluecompare/reflectvaluecompare.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/shadow/shadow.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/sigchanyzer/sigchanyzer.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/slog/slog.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/stdmethods/stdmethods.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/stdversion/stdversion.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/stringintconv/string.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine/testinggoroutine.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine/util.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/tests/tests.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/timeformat/timeformat.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/unmarshal/unmarshal.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/unreachable/unreachable.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/unsafeptr/unsafeptr.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/unusedresult/unusedresult.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/unusedwrite/unusedwrite.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/analysis/passes/waitgroup/waitgroup.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/astutil/imports.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/inspector/cursor.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/cfg/builder.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/cfg/cfg.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/packages/packages.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/packages/visit.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ssa/builder.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ssa/create.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ssa/emit.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ssa/func.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ssa/instantiate.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ssa/ssa.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ssa/ssautil/visit.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ssa/subst.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ssa/util.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/types/objectpath/objectpath.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/types/typeutil/callee.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/types/typeutil/map.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/imports/forward.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/analysis/analyzerutil/doc.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/analysis/analyzerutil/extractdoc.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/analysis/analyzerutil/readfile.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/analysis/analyzerutil/version.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/analysis/typeindex/typeindex.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/analysisinternal/analysis.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/astutil/stringlit.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/astutil/util.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/event/core/export.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/event/label/label.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gcimporter/bimport.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gcimporter/iexport.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gcimporter/iimport.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/imports/sortimports.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/modindex/index.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/modindex/lookup.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/moreiters/iters.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/packagepath/packagepath.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/refactor/delete.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/refactor/edit.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/refactor/imports.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/refactor/refactor.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/stdlib/deps.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/stdlib/import.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/stdlib/manifest.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/stdlib/stdlib.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typeparams/normalize.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/classify_call.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/element.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/fx.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/isnamed.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/qualifier.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/types.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/varkind.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/varkind_go124.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/zerovalue.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/versions/features.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/refactor/satisfy/find.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/utils/buffer/ring_fixed.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/utils/exec/exec.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/utils/exec/fixup_go118.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/utils/exec/fixup_go119.gois excluded by!vendor/**,!**/vendor/**vendor/k8s.io/utils/strings/slices/slices.gois excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (26)
AGENTS.mdcmd/machine-api-operator/start.gocmd/machineset/main.gocmd/vsphere/main.gogo.modhack/go-build.shinstall/0000_30_machine-api-operator_09_rbac.yamlinstall/0000_30_machine-api-operator_11_deployment.yamlpkg/controller/machine/controller.gopkg/controller/machine/drain_controller.gopkg/controller/machine/drain_controller_test.gopkg/controller/machinehealthcheck/machinehealthcheck_controller.gopkg/controller/machinehealthcheck/machinehealthcheck_controller_test.gopkg/controller/machineset/controller.gopkg/controller/machineset/controller_test.gopkg/controller/vsphere/actuator.gopkg/controller/vsphere/actuator_test.gopkg/controller/vsphere/machineset/controller.gopkg/controller/vsphere/machineset/controller_test.gopkg/operator/config.gopkg/operator/operator.gopkg/operator/operator_test.gopkg/operator/sync.gopkg/operator/sync_test.gopkg/tls/tls.gopkg/webhooks/machine_webhook.go
damdo
reviewed
Mar 27, 2026
Member
damdo
left a comment
damdo
left a comment
There was a problem hiding this comment.
Thanks Radek!
A couple of Q. but
/approve
Comment on lines
+96
to
+97
| pflag.StringVar(&tlsMinVersionFlag, "tls-min-version", "", "Minimum TLS version supported. When set with --tls-cipher-suites, overrides the cluster-wide TLS profile. Possible values: "+strings.Join(cliflag.TLSPossibleVersions(), ", ")) | ||
| pflag.StringSliceVar(&tlsCipherSuitesFlag, "tls-cipher-suites", nil, "Comma-separated list of cipher suites for the server. When set with --tls-min-version, overrides the cluster-wide TLS profile. Possible values: "+strings.Join(cliflag.TLSCipherPossibleValues(), ", ")) |
Member
There was a problem hiding this comment.
Do we need pflag? I'd prefer if used flag where possible
| const ( | ||
| // defaultMetricsPort is the default port to expose metrics. | ||
| defaultMetricsPort = 8080 | ||
| defaultMetricsPort = 8443 |
Member
There was a problem hiding this comment.
Is the external port changing? Or is this the original external port that the proxy had?
| klog.Fatalf("Unable to resolve TLS configuration: %v", err) | ||
| } | ||
| if err := setupTLSProfileWatcher(ctrlCtx, tlsResult, shutdown); err != nil { | ||
| klog.Fatalf("Unable to set up TLS profile watcher: %v", err) |
Member
There was a problem hiding this comment.
Is the fatal error consistent to what we do in c-r ?
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: damdo 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
Member
|
/retest |
Contributor
|
@RadekManak: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
:8443) using controller-runtime’s metrics server with delegated authn/authz (WithAuthenticationAndAuthorization), and remove the MAOkube-rbac-proxysidecar.APIServer/clusterTLS profile on startup,kube-rbac-proxysidecars (machine,machineset,mhc) by generating--tls-min-versionand profile-derived--tls-cipher-suitesargs.METRICS_PORT=8443) for direct secure serving,config.openshift.io/apiservers.controller-runtime,openshift/api,openshift/client-go, etc.),go-build.shroot-dir handling fix,pkg/webhooks/machine_webhook.go, context import updates).Notes
kube-rbac-proxyand continue using the existingnamespace/metricsauthorization model./metricsauth uses delegated token/SAR checks; scraper access relies on existing cluster-monitoringprometheus-k8scluster RBAC.