OCPCLOUD-3346: tls: use centralized TLS#1456
OCPCLOUD-3346: tls: use centralized TLS#1456RadekManak wants to merge 7 commits intoopenshift:mainfrom
Conversation
|
@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. |
|
/assign @damdo |
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) |
There was a problem hiding this comment.
Shouldn't we return here?
What would happen otherwise?
pkg/operator/operator.go
Outdated
| } else { | ||
| profile, err := utiltls.GetTLSProfileSpec(apiServer.Spec.TLSSecurityProfile) | ||
| if err != nil { | ||
| klog.Warningf("Failed to get TLS profile spec, using defaults: %v", err) |
There was a problem hiding this comment.
Shouldn't we return here?
What would happen otherwise?
pkg/operator/sync.go
Outdated
| } | ||
|
|
||
| func newKubeProxyContainers(image string, withMHCProxy bool) []corev1.Container { | ||
| func newKubeProxyContainers(image string, withMHCProxy bool, tlsProfile *configv1.TLSProfileSpec) []corev1.Container { |
There was a problem hiding this comment.
Let's not pass the pointer but the whole tlsProfile
pkg/operator/sync.go
Outdated
| // Use defaults if no profile provided | ||
| ciphers := utiltls.DefaultTLSCiphers | ||
| minVersion := utiltls.DefaultMinTLSVersion |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
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.
4e665c0 to
24eed11
Compare
|
@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. |
24eed11 to
6b35a05
Compare
|
@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. |
: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.