Skip to content

Conversation

@susesamu
Copy link
Contributor

@susesamu susesamu commented Sep 2, 2025

issue: rancher/rancher#51705

dan edit: moved version to 4.10.0-rancher.x - we will reassign a version when 2.13.2 is closer.

@susesamu susesamu changed the title [rancher-logging 4.10.0-rancher.14] Add a loggingOverlay for each type of Logging [rancher-logging 4.10.0-rancher.13] Add a loggingOverlay for each type of Logging Sep 3, 2025
@susesamu susesamu changed the title [rancher-logging 4.10.0-rancher.13] Add a loggingOverlay for each type of Logging [rancher-logging 4.10.0-rancher.X] Add a loggingOverlay for each type of Logging Sep 5, 2025
@skanakal
Copy link
Contributor

@susesamu Can you rebase this pr to resolve the conflicts...

@susesamu susesamu force-pushed the feat/51705 branch 2 times, most recently from 3d47424 to 42784f0 Compare October 15, 2025 21:52
@susesamu susesamu changed the title [rancher-logging 4.10.0-rancher.X] Add a loggingOverlay for each type of Logging [rancher-logging 4.10.0-rancher.16] Add a loggingOverlay for each type of Logging Oct 15, 2025
@skanakal skanakal self-requested a review October 17, 2025 07:21
@mallardduck mallardduck changed the title [rancher-logging 4.10.0-rancher.16] Add a loggingOverlay for each type of Logging [rancher-logging 4.10.0-rancher.x] Add a loggingOverlay for each type of Logging Nov 25, 2025
@adamkpickering
Copy link
Contributor

So I think there's a deeper problem here. Try doing the following on the chart from the PR:

helm template . --set 'additionalLoggingSources.rke2.enabled=true' --set 'additionalLoggingSources.kubeAudit.enabled=true' --set 'additionalLoggingSources.kubeAudit.loggingRef=setInKubeAuditTemplate' --set 'additionalLoggingSources.kubeAudit.loggingOverlay.spec.loggingRef=setInKubeAuditLoggingOverlay' | less

Because the individual logging overlay (i.e. additionalLoggingSources.kubeAudit.loggingOverlay) takes precedence over the individual template (i.e. the place that additionalLoggingSources.kubeAudit.loggingRef gets inserted into), we would expect that the value of loggingRef in the template is setInKubeAuditLoggingOverlay. But when we run the command, it has the value setInkubeAuditTemplate.

This seems to be happening because parsing the output of an included template (in this case the "logging-operator.logging.kube-audit.logging-overlay" template) using fromYaml produces the following error. You can see this above one of the Logging resources in the result of the helm template command I pasted above:

Error: 'error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string
  into Go value of type map[string]interface {}'

This error corresponds to the line {{- $individualLoggingOverlay = fromYaml (include $overlayName $top | default "") | default (dict) -}}. But I don't know why it would be choking on it. I think that we are using helm templates in a way that the helm developers never intended, and it's breaking as a result. I would play with this more in order to understand what is going on, but I don't think it's worth it.

I think our best bet is to do something like this in each individual template file e.g. templates/loggings/kube-audit/logging.yaml:

{{- if .Values.additionalLoggingSources.kubeAudit.enabled }}
{{- $genericTemplate := fromYaml (include "logging-operator.logging.tpl" .) -}}
{{- $individualTemplate := fromYaml (include "logging-operator.logging.kube-audit" .) -}}
{{- $genericOverlay := .Values.loggingOverlay -}}
{{- $individualOverlay := .Values.additionalLoggingSources.kubeAudit.loggingOverlay -}}
{{/* further left is higher precedence */}}
{{- $merged := merge $individualOverlay $genericOverlay $individualTemplate $genericTemplate -}}
{{- toYaml $merged }}
{{- end }}

This does duplicate the merging logic, but it is much much clearer. I'm tired of double checking my understanding of how the merging works and this would largely solve it. Note that this would require adding loggingOverlay: {} (i.e. the generic logging overlay) to values.yaml, which IMO should have been added when we started using .Values.loggingOverlay in templates.

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.

4 participants