Skip to content

Comments

[kube-prometheus-stack] fix admission webhook DNS name rendering#6646

Merged
jkroepke merged 1 commit intoprometheus-community:mainfrom
mjnagel:fix-chart-cert
Feb 13, 2026
Merged

[kube-prometheus-stack] fix admission webhook DNS name rendering#6646
jkroepke merged 1 commit intoprometheus-community:mainfrom
mjnagel:fix-chart-cert

Conversation

@mjnagel
Copy link
Contributor

@mjnagel mjnagel commented Feb 13, 2026

What this PR does / why we need it

This PR reverts #6629 which incorrectly introduced a chomp which results in errors at deploy time:

x509: certificate is valid for kube-prometheus-stack-operatorkube-prometheus-stack-operator.monitoring.svc, not kube-prometheus-stack-operator

Note the way that kube-prometheus-stack-operatorkube-prometheus-stack-operator.monitoring.svc is smushed together rather than two separate names (kube-prometheus-stack-operator and kube-prometheus-stack-operator.monitoring.svc).

Which issue this PR fixes

Haven't opened an issue, but I can.

Special notes for your reviewer

The prior PR introduced this issue to resolve a bug, but the problem in that case was related to Windows ^M (see https://unix.stackexchange.com/questions/32001/what-is-m-and-how-do-i-get-rid-of-it) - cc @vitrix1

Checklist

  • DCO signed
  • Chart Version bumped
  • Title of the PR starts with chart name (e.g. [prometheus-couchdb-exporter])

Signed-off-by: Micah Nagel <micah.nagel@defenseunicorns.com>
@mjnagel
Copy link
Contributor Author

mjnagel commented Feb 13, 2026

By way of additional explanation, the template here REQUIRES these cert names to be separated by a newline in order to properly be split/comma-separated. The previous PR was "resolved" by this change only because the - (chomp) character trimmed out the Windows carriage return, but would result in other issues due to the DNS names not properly being separated.

I believe the prior issue came down to this:

It is worth adding that I build the archive with the chart on Windows 11

@vitrix1 you likely need to switch your editor? from using carriage returns and make sure that it only has newline characters (reference this link as one helper for that). It might look different based on your editor/process though.

@jkroepke
Copy link
Member

I guess a bitter fix would be a comma separation, instead a newline spearation?

@mjnagel
Copy link
Contributor Author

mjnagel commented Feb 13, 2026

I guess a bitter fix would be a comma separation, instead a newline spearation?

I could make that change if it's the desired path - I think , would be a safe character to use here and split on. I'd still probably argue that the previous issue in #6629 is down to newlines being handled incorrectly when the chart archive was built, and could affect other templates beyond just this. Splitting on newlines isn't inherently wrong, switching to commas would just remove this one spot from being a potential problem with newlines.

@jkroepke
Copy link
Member

It just seems serios cross os issues with \r\n on windows and \n on linux

Copy link
Member

@jkroepke jkroepke left a comment

Choose a reason for hiding this comment

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

Approve for now to fix this in general

@jkroepke jkroepke merged commit 6fffed9 into prometheus-community:main Feb 13, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants