Skip to content

console: configure client console nginx TLS from TLSProfile CR - #705

Open
vbnrh wants to merge 2 commits into
red-hat-storage:mainfrom
vbnrh:RHSTOR-8685-console-tls-profile
Open

console: configure client console nginx TLS from TLSProfile CR#705
vbnrh wants to merge 2 commits into
red-hat-storage:mainfrom
vbnrh:RHSTOR-8685-console-tls-profile

Conversation

@vbnrh

@vbnrh vbnrh commented Aug 4, 2026

Copy link
Copy Markdown
Member

Summary

  • Wire the ocs-client-operator-console nginx server to read TLS settings from the TLSProfile CRD and inject ssl_protocols, ssl_ciphers, and ssl_conf_command Groups directives into the nginx ConfigMap
  • Add a watch on TLSProfile in OperatorConfigMapReconciler.SetupWithManager() so nginx config is regenerated when the TLS profile changes
  • Add features.operators.openshift.io/tls-profiles: "true" CSV annotation

Changes

  • pkg/console/nginx_root.conf: add %s placeholder for TLS directive injection
  • pkg/console/console.go: replace GetNginxRootConf() with GenerateNginxConf(ossl) + buildTLSDirectives()
  • internal/controller/operatorconfigmap_controller.go: fetch TLSProfile in ensureConsolePlugin(), resolve OpenSSL config for domain ocs.openshift.io / server client-console, pass to nginx config generation; add TLSProfile watch with name/namespace/generation predicates
  • pkg/console/console_test.go: unit tests for GenerateNginxConf (nil, TLS 1.3 full, protocol-only)
  • CSV template + bundle: add tls-profiles feature annotation

Test plan

  • go build ./... passes
  • go test ./pkg/console/... passes (new + existing tests)
  • go test -run 'TestBuildDesiredNginxDataWithProxies' ./internal/controller/... passes
  • make bundle regenerates cleanly

Ref: RHSTOR-8685

@openshift-ci

openshift-ci Bot commented Aug 4, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vbnrh
Once this PR has been reviewed and has the lgtm label, please assign madhu-1 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@vbnrh
vbnrh force-pushed the RHSTOR-8685-console-tls-profile branch from 40f0461 to 911f118 Compare August 4, 2026 06:47

@leelavg leelavg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not reviewing, pls look at existing code, configmap controller already gets startuptlsprofile and just use it, see it's being reused for csi addons as well. should be straightforward.

@vbnrh
vbnrh force-pushed the RHSTOR-8685-console-tls-profile branch from 911f118 to b3ef2fc Compare August 4, 2026 07:40
@vbnrh
vbnrh requested a review from leelavg August 4, 2026 07:51
Comment thread pkg/console/console.go Outdated
}

func GetNginxProxyConf(uniqueIdentifier, exposeAs, endpointURL, endpointHost, certsPath string) (string, error) {
func buildTLSDirectives(ossl *ocstlsv1.OpenSSLConfig) string {

@SanjalKatiyar SanjalKatiyar Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits:

Suggested change
func buildTLSDirectives(ossl *ocstlsv1.OpenSSLConfig) string {
func buildRootTLSDirectives(ossl *ocstlsv1.OpenSSLConfig) string {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Real detection gap -- thanks. Command substitution and backtick interpolation inside a single-quoted SQL literal are a genuine injection shape distinct from $VAR. Fixed in 76231795ad: added a command-sub/backtick detection regex plus a unified _span_has_real_interpolation helper that checks every $-expansion, $( command-sub, and backtick per span with escape awareness (\$( and \` are bash-literal). Regression test added covering $(printf %s "$UNTRUSTED"), backtick `id`, and the escaped \$( negative. Also added the omitted REINDEX/VACUUM statement keywords in the same commit.

Comment thread pkg/console/console.go Outdated

func GetNginxRootConf() string {
return nginxRootConf
func GenerateNginxConf(ossl *ocstlsv1.OpenSSLConfig) string {

@SanjalKatiyar SanjalKatiyar Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits:

Suggested change
func GenerateNginxConf(ossl *ocstlsv1.OpenSSLConfig) string {
func GetNginxRootConf(ossl *ocstlsv1.OpenSSLConfig) string {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or,

Suggested change
func GenerateNginxConf(ossl *ocstlsv1.OpenSSLConfig) string {
func GenerateNginxRootConf(ossl *ocstlsv1.OpenSSLConfig) string {

}

err := c.computeDesiredProxyConfigByKey(out)
err := c.computeDesiredProxyConfigByKey(out, console.BuildProxyTLSDirectives(ossl))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits (optional): ideally console.BuildProxyTLSDirectives(ossl) should happen inside computeDesiredProxyConfigByKey func. That's the "desired" responsibility of the function, ossl should be the argument.

Comment thread pkg/console/nginx_root.conf Outdated
Comment on lines +104 to +105
ssl_certificate_key /var/serving-cert/tls.key;

%s

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we plz keep formatting aligned with ssl_certificate_key and location strings ??

Comment thread pkg/console/nginx_proxy.tmpl Outdated
Comment on lines +24 to +26
{{- if .ProxyTLSDirectives}}
{{.ProxyTLSDirectives}}
{{- end}}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we plz keep formatting aligned with rest of the file ??

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image Replaced with individual template fields

@vbnrh
vbnrh force-pushed the RHSTOR-8685-console-tls-profile branch 2 times, most recently from a4f0867 to bd7a0c6 Compare August 4, 2026 09:12
vbnrh added 2 commits August 6, 2026 18:15
Ref: RHSTOR-8685
Signed-off-by: vbadrina <vbadrina@redhat.com>
Ref: RHSTOR-8685
Signed-off-by: vbadrina <vbadrina@redhat.com>
@vbnrh
vbnrh force-pushed the RHSTOR-8685-console-tls-profile branch from bd7a0c6 to b4cfabe Compare August 6, 2026 12:46
@vbnrh
vbnrh requested a review from SanjalKatiyar August 7, 2026 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants