OCPNETUI-63: Support custom TLS settings in the Helm charts - #467
OCPNETUI-63: Support custom TLS settings in the Helm charts#467rszwajko wants to merge 1 commit into
Conversation
Provide default values based on tlsref.org intermediate config. Reference-Url: https://configurator.tlsref.org/#server=nginx&version=1.27.3&config=intermediate&openssl=4.0.1&hsts&guideline=6.0 Reference-Url: openshift/cluster-network-operator#3043 Signed-off-by: Radoslaw Szwajkowski <rszwajko@redhat.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
@rszwajko: This pull request references OCPNETUI-63 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "5.0.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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rszwajko 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
WalkthroughThe Helm chart adds plugin TLS protocol, cipher, and server-cipher-preference values. The NGINX ConfigMap renders the related directives when values are set. Several scalar values now use single quotes. ChangesPlugin TLS configuration
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/openshift-console-plugin/templates/configmap.yaml`:
- Around line 22-30: Update the TLS configuration block to read protocols,
ciphers, and preferServerCiphers from .Values.plugin.tls instead of .Values.tls,
preserving the existing conditional rendering and NGINX directives.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5b1e0a58-a7a7-4132-9cf5-cc6be0cfbef2
📒 Files selected for processing (2)
charts/openshift-console-plugin/templates/configmap.yamlcharts/openshift-console-plugin/values.yaml
| {{- if .Values.tls.protocols }} | ||
| ssl_protocols {{ .Values.tls.protocols }}; | ||
| {{- end }} | ||
| {{- if .Values.tls.ciphers }} | ||
| ssl_ciphers {{ .Values.tls.ciphers }}; | ||
| {{- end }} | ||
| {{- if .Values.tls.preferServerCiphers }} | ||
| ssl_prefer_server_ciphers {{ .Values.tls.preferServerCiphers }}; | ||
| {{- end }} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- chart files ---'
git ls-files charts/openshift-console-plugin | sed -n '1,120p'
printf '%s\n' '--- values.yaml ---'
cat -n charts/openshift-console-plugin/values.yaml
printf '%s\n' '--- configmap.yaml ---'
cat -n charts/openshift-console-plugin/templates/configmap.yaml
printf '%s\n' '--- TLS references in the chart ---'
rg -n --glob '*.yaml' --glob '*.yml' --glob '*.tpl' 'tls|ssl_protocols|ssl_ciphers|ssl_prefer_server_ciphers' charts/openshift-console-pluginRepository: openshift/networking-console-plugin
Length of output: 5157
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- all chart value and TLS references ---'
rg -n --hidden --glob '!charts/openshift-console-plugin/templates/configmap.yaml' \
--glob '!charts/openshift-console-plugin/.helmignore' \
'(^|[[:space:]])tls([.:]|:|$)|preferServerCiphers|ssl_protocols|ssl_ciphers|ssl_prefer_server_ciphers' .
printf '%s\n' '--- chart metadata and documentation files ---'
cat -n charts/openshift-console-plugin/Chart.yaml
git ls-files | rg '(^|/)(README|values)([^/]*)?$|charts/openshift-console-plugin'
printf '%s\n' '--- default-value path probe ---'
python3 - <<'PY'
from pathlib import Path
values = Path("charts/openshift-console-plugin/values.yaml").read_text()
paths = []
stack = []
for line in values.splitlines():
if not line.strip() or line.lstrip().startswith("#") or line.strip() == "---":
continue
indent = len(line) - len(line.lstrip())
key, sep, value = line.strip().partition(":")
if not sep:
continue
while stack and stack[-1][0] >= indent:
stack.pop()
path = ".".join([item[1] for item in stack] + [key])
if value.strip():
paths.append(path)
else:
stack.append((indent, key))
template = Path("charts/openshift-console-plugin/templates/configmap.yaml").read_text()
for path in (
"tls.protocols",
"tls.ciphers",
"tls.preferServerCiphers",
"plugin.tls.protocols",
"plugin.tls.ciphers",
"plugin.tls.preferServerCiphers",
):
print(f"{path}: {'defined' if path in paths else 'undefined'}")
print("template paths:")
for line in template.splitlines():
if ".Values." in line and ("tls" in line or "ssl_" in line):
print(line.strip())
PYRepository: openshift/networking-console-plugin
Length of output: 2133
Read TLS values from the plugin scope.
The default values and user overrides are defined under .Values.plugin.tls, but these conditions and directives use .Values.tls.*. The rendered NGINX configuration therefore omits all three TLS settings. Change each reference to .Values.plugin.tls.*.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@charts/openshift-console-plugin/templates/configmap.yaml` around lines 22 -
30, Update the TLS configuration block to read protocols, ciphers, and
preferServerCiphers from .Values.plugin.tls instead of .Values.tls, preserving
the existing conditional rendering and NGINX directives.
2 participants
Provide default values based on tlsref.org intermediate config.
Reference-Url: https://configurator.tlsref.org/#server=nginx&version=1.27.3&config=intermediate&openssl=4.0.1&hsts&guideline=6.0
Reference-Url: openshift/cluster-network-operator#3043
Summary by CodeRabbit