NO-ISSUE: Add DNS' core-rbac-proxy args and template TLS Cipher Suites & Min Version#6368
NO-ISSUE: Add DNS' core-rbac-proxy args and template TLS Cipher Suites & Min Version#6368pmtk wants to merge 2 commits intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@pmtk: This pull request explicitly references no 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. |
WalkthroughThis PR updates nightly OpenShift release versions and many image SHA256 digests to 2026-03-17 timestamps across aarch64 and x86_64, adds TLSCipherSuites and TLSMinVersion to template render parameters, and centralizes DNS kube-rbac-proxy TLS args via templating. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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.3)level=error msg="Running error: context loading failed: failed to load packages: failed to load packages: failed to load with go/packages: err: exit status 1: stderr: go: inconsistent vendoring in :\n\tgithub.com/apparentlymart/go-cidr@v1.1.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/coreos/go-systemd@v0.0.0-20190321100706-95778dfbb74e: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/google/go-cmp@v0.7.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/miekg/dns@v1.1.63: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/openshift/api@v0.0.0-20260309155933-45fd88d185dd: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/openshift/build-machinery-go@v0.0.0-20251023084048-5d77c1a5e5af: is explicitly required in go.mod, but not marked as explicit ... [truncated 29518 characters] ... belet: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/metrics: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/mount-utils: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/pod-security-admission: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/sample-apiserver: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/sample-cli-plugin: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/sample-controller: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\n\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor\n" Comment |
openshift-ci
bot
added
the
approved
![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 the current code and only fix it if needed.
Inline comments:
In `@scripts/auto-rebase/rebase.sh`:
- Line 766: The arg entry in the yq invocation currently embeds quotes around
the TLS cipher placeholder; update the args array in the yq command that sets
containers[1].args (the line containing '--tls-cipher-suites=\"{{
.TLSCipherSuites }}\"') to use an unquoted placeholder '--tls-cipher-suites={{
.TLSCipherSuites }}' instead, and make the same change in the checked-in
manifest assets/components/openshift-dns/dns/daemonset.yaml (the daemonset args
line around the TLS cipher setting) so kube-rbac-proxy does not receive literal
quote characters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 1434d150-12c0-499e-99e5-81f7e6fb4fac
⛔ Files ignored due to path filters (5)
etcd/go.sumis excluded by!**/*.sumetcd/vendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**etcd/vendor/modules.txtis excluded by!**/vendor/**
📒 Files selected for processing (21)
Makefile.version.aarch64.varMakefile.version.x86_64.varassets/components/multus/kustomization.aarch64.yamlassets/components/multus/kustomization.x86_64.yamlassets/components/multus/release-multus-aarch64.jsonassets/components/multus/release-multus-x86_64.jsonassets/components/openshift-dns/dns/daemonset.yamlassets/optional/operator-lifecycle-manager/kustomization.aarch64.yamlassets/optional/operator-lifecycle-manager/kustomization.x86_64.yamlassets/optional/operator-lifecycle-manager/release-olm-aarch64.jsonassets/optional/operator-lifecycle-manager/release-olm-x86_64.jsonassets/release/release-aarch64.jsonassets/release/release-x86_64.jsonetcd/go.modpackaging/crio.conf.d/10-microshift_amd64.confpackaging/crio.conf.d/10-microshift_arm64.confpkg/components/render.goscripts/auto-rebase/changelog.txtscripts/auto-rebase/commits.txtscripts/auto-rebase/last_rebase.shscripts/auto-rebase/rebase.sh
scripts/auto-rebase/rebase.sh
Outdated
| yq -i '.spec.template.spec.volumes[1] += {"secret": {"defaultMode": 420, "secretName": "dns-default-metrics-tls"}}' "${REPOROOT}"/assets/components/openshift-dns/dns/daemonset.yaml | ||
| yq -i '.spec.template.spec.tolerations = [{"key": "node-role.kubernetes.io/master", "operator": "Exists"}]' "${REPOROOT}"/assets/components/openshift-dns/dns/daemonset.yaml | ||
| sed -i '/#.*set at runtime/d' "${REPOROOT}"/assets/components/openshift-dns/dns/daemonset.yaml | ||
| yq -i '.spec.template.spec.containers[1].args = ["--logtostderr", "--secure-listen-address=:9154", "--tls-cipher-suites=\"{{ .TLSCipherSuites }}\"", "--upstream=http://127.0.0.1:9153/", "--tls-cert-file=/etc/tls/private/tls.crt", "--tls-private-key-file=/etc/tls/private/tls.key"]' "${REPOROOT}"/assets/components/openshift-dns/dns/daemonset.yaml |
There was a problem hiding this comment.
Drop the embedded quotes from --tls-cipher-suites.
Container args are passed verbatim, so --tls-cipher-suites=\"{{ .TLSCipherSuites }}\" makes kube-rbac-proxy receive " as part of the first and last cipher names. Use --tls-cipher-suites={{ .TLSCipherSuites }} here, and mirror the same edit in assets/components/openshift-dns/dns/daemonset.yaml Line 87.
Suggested fix
-yq -i '.spec.template.spec.containers[1].args = ["--logtostderr", "--secure-listen-address=:9154", "--tls-cipher-suites=\"{{ .TLSCipherSuites }}\"", "--upstream=http://127.0.0.1:9153/", "--tls-cert-file=/etc/tls/private/tls.crt", "--tls-private-key-file=/etc/tls/private/tls.key"]' "${REPOROOT}"/assets/components/openshift-dns/dns/daemonset.yaml
+yq -i '.spec.template.spec.containers[1].args = ["--logtostderr", "--secure-listen-address=:9154", "--tls-cipher-suites={{ .TLSCipherSuites }}", "--upstream=http://127.0.0.1:9153/", "--tls-cert-file=/etc/tls/private/tls.crt", "--tls-private-key-file=/etc/tls/private/tls.key"]' "${REPOROOT}"/assets/components/openshift-dns/dns/daemonset.yaml- - --tls-cipher-suites="{{ .TLSCipherSuites }}"
+ - --tls-cipher-suites={{ .TLSCipherSuites }}Verify that neither the generator nor the checked-in daemonset still contains a quoted placeholder for this arg. This script should fail on the current branch and pass after the fix:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
checks = {
"scripts/auto-rebase/rebase.sh": '--tls-cipher-suites=\\"{{ .TLSCipherSuites }}\\"',
"assets/components/openshift-dns/dns/daemonset.yaml": '--tls-cipher-suites="{{ .TLSCipherSuites }}"',
}
bad = []
for path, needle in checks.items():
text = Path(path).read_text()
if needle in text:
bad.append(path)
print(f"BAD {path}: found quoted tls-cipher-suites placeholder")
else:
print(f"OK {path}: quoted tls-cipher-suites placeholder not found")
if bad:
raise SystemExit(1)
PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/auto-rebase/rebase.sh` at line 766, The arg entry in the yq
invocation currently embeds quotes around the TLS cipher placeholder; update the
args array in the yq command that sets containers[1].args (the line containing
'--tls-cipher-suites=\"{{ .TLSCipherSuites }}\"') to use an unquoted placeholder
'--tls-cipher-suites={{ .TLSCipherSuites }}' instead, and make the same change
in the checked-in manifest assets/components/openshift-dns/dns/daemonset.yaml
(the daemonset args line around the TLS cipher setting) so kube-rbac-proxy does
not receive literal quote characters.
pmtk
force-pushed
the
dns-pod-kube-rbac-proxy-fix-args
branch
from
fbcf67c to
5f80861
Compare
pmtk
changed the title
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/auto-rebase/rebase.sh (1)
766-766:⚠️ Potential issue | 🔴 CriticalRemove embedded quotes from templated TLS flag values.
At Line 766,
--tls-cipher-suites=\"{{ .TLSCipherSuites }}\"and--tls-min-version=\"{{ .TLSMinVersion }}\"pass literal"characters in argv; kube-rbac-proxy may treat these as invalid flag values.Proposed fix
-yq -i '.spec.template.spec.containers[1].args = ["--logtostderr", "--secure-listen-address=:9154", "--tls-cipher-suites=\"{{ .TLSCipherSuites }}\"", "--tls-min-version=\"{{ .TLSMinVersion }}\"", "--upstream=http://127.0.0.1:9153/", "--tls-cert-file=/etc/tls/private/tls.crt", "--tls-private-key-file=/etc/tls/private/tls.key"]' "${REPOROOT}"/assets/components/openshift-dns/dns/daemonset.yaml +yq -i '.spec.template.spec.containers[1].args = ["--logtostderr", "--secure-listen-address=:9154", "--tls-cipher-suites={{ .TLSCipherSuites }}", "--tls-min-version={{ .TLSMinVersion }}", "--upstream=http://127.0.0.1:9153/", "--tls-cert-file=/etc/tls/private/tls.crt", "--tls-private-key-file=/etc/tls/private/tls.key"]' "${REPOROOT}"/assets/components/openshift-dns/dns/daemonset.yaml- - --tls-cipher-suites="{{ .TLSCipherSuites }}" - - --tls-min-version="{{ .TLSMinVersion }}" + - --tls-cipher-suites={{ .TLSCipherSuites }} + - --tls-min-version={{ .TLSMinVersion }}As per coding guidelines, "-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path checks = { "scripts/auto-rebase/rebase.sh": [ '--tls-cipher-suites=\\"{{ .TLSCipherSuites }}\\"', '--tls-min-version=\\"{{ .TLSMinVersion }}\\"', ], "assets/components/openshift-dns/dns/daemonset.yaml": [ '--tls-cipher-suites="{{ .TLSCipherSuites }}"', '--tls-min-version="{{ .TLSMinVersion }}"', ], } bad = False for path, needles in checks.items(): text = Path(path).read_text() for needle in needles: if needle in text: print(f"BAD {path}: found quoted TLS placeholder: {needle}") bad = True else: print(f"OK {path}: not found {needle}") if bad: raise SystemExit(1) PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/auto-rebase/rebase.sh` at line 766, The yq command is inserting literal quote characters into the kube-rbac-proxy argv for the container; update the args array assignment in the yq invocation so the TLS placeholders are not wrapped in embedded quotes—replace "--tls-cipher-suites=\"{{ .TLSCipherSuites }}\"" and "--tls-min-version=\"{{ .TLSMinVersion }}\"" with unquoted flag values like "--tls-cipher-suites={{ .TLSCipherSuites }}" and "--tls-min-version={{ .TLSMinVersion }}" (leave the rest of the args and the containers[1].args assignment intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/auto-rebase/rebase.sh`:
- Line 766: The yq command is inserting literal quote characters into the
kube-rbac-proxy argv for the container; update the args array assignment in the
yq invocation so the TLS placeholders are not wrapped in embedded quotes—replace
"--tls-cipher-suites=\"{{ .TLSCipherSuites }}\"" and "--tls-min-version=\"{{
.TLSMinVersion }}\"" with unquoted flag values like "--tls-cipher-suites={{
.TLSCipherSuites }}" and "--tls-min-version={{ .TLSMinVersion }}" (leave the
rest of the args and the containers[1].args assignment intact).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: c3cb03e1-0cb9-4cbf-8b7a-970769dd21d5
⛔ Files ignored due to path filters (5)
etcd/go.sumis excluded by!**/*.sumetcd/vendor/github.com/openshift/api/config/v1/types_apiserver.gois excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**etcd/vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**etcd/vendor/modules.txtis excluded by!**/vendor/**
📒 Files selected for processing (21)
Makefile.version.aarch64.varMakefile.version.x86_64.varassets/components/multus/kustomization.aarch64.yamlassets/components/multus/kustomization.x86_64.yamlassets/components/multus/release-multus-aarch64.jsonassets/components/multus/release-multus-x86_64.jsonassets/components/openshift-dns/dns/daemonset.yamlassets/optional/operator-lifecycle-manager/kustomization.aarch64.yamlassets/optional/operator-lifecycle-manager/kustomization.x86_64.yamlassets/optional/operator-lifecycle-manager/release-olm-aarch64.jsonassets/optional/operator-lifecycle-manager/release-olm-x86_64.jsonassets/release/release-aarch64.jsonassets/release/release-x86_64.jsonetcd/go.modpackaging/crio.conf.d/10-microshift_amd64.confpackaging/crio.conf.d/10-microshift_arm64.confpkg/components/render.goscripts/auto-rebase/changelog.txtscripts/auto-rebase/commits.txtscripts/auto-rebase/last_rebase.shscripts/auto-rebase/rebase.sh
🚧 Files skipped from review as they are similar to previous changes (11)
- assets/components/multus/release-multus-x86_64.json
- Makefile.version.aarch64.var
- packaging/crio.conf.d/10-microshift_amd64.conf
- assets/release/release-aarch64.json
- assets/components/multus/kustomization.x86_64.yaml
- assets/optional/operator-lifecycle-manager/kustomization.x86_64.yaml
- packaging/crio.conf.d/10-microshift_arm64.conf
- assets/optional/operator-lifecycle-manager/release-olm-aarch64.json
- assets/components/multus/kustomization.aarch64.yaml
- pkg/components/render.go
- assets/optional/operator-lifecycle-manager/kustomization.aarch64.yaml
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pacevedom, pmtk 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 |
|
@pmtk: 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. |
3 participants
No description provided.