dongjoon-hyun opened a new pull request, #648:
URL: https://github.com/apache/spark-kubernetes-operator/pull/648
### What changes were proposed in this pull request?
This PR fixes the Helm chart so that
`spark-kubernetes-operator-configuration` ConfigMap renders valid YAML under
the default `values.yaml`.
In `templates/spark-operator.yaml`, the three data keys
(`log4j2.properties`, `spark-operator.properties`, `metrics.properties`) used
the `|+` block scalar header followed by `{{- ... | nindent 4 -}}` for each
content source. Because `nindent` always emits a leading newline plus indent
(even for an empty input), an empty source — for example `$.Files.Get
"conf/log4j2.properties"` which currently always returns an empty string since
the chart contains no `conf/` directory — produced a leading line containing
only 4 trailing spaces.
The fix:
- Replace `|+` with `|` (clip chomping is sufficient for these properties
files).
- Replace `{{- if <source-non-empty-check>... | nindent 4 -}}` with `{{-
with <source> }}{{ . | nindent 4 }}{{ end }}`. `with` skips the block when the
source is an empty string, so no phantom indent line is emitted.
- The user-facing `append` toggle is still expressed with `if`; the `with`
guards only the content insertion underneath it.
### Why are the changes needed?
The previously rendered output looked like the following:
```yaml
data:
log4j2.properties: |+
# Logging Overrides
rootLogger.level=INFO
```
The first line of the literal block contains only 4 spaces. Strict YAML
parsers (e.g., YamlDotNet used by Octopus Deploy) reject this with:
```
YAML syntax error: While scanning a literal block scalar, found extra spaces
in first line.
```
This made the chart unusable for users on those platforms even though `helm
install` itself accepted it.
Reported in apache/spark-kubernetes-operator#647.
### Does this PR introduce _any_ user-facing change?
Yes. The rendered ConfigMap no longer contains the leading blank line with
stray spaces inside each block scalar. Functionally the ConfigMap content is
equivalent for non-empty values, and it now parses under strict YAML parsers.
Before:
```yaml
data:
log4j2.properties: |+
# Logging Overrides
rootLogger.level=INFO
```
After:
```yaml
data:
log4j2.properties: |
# Logging Overrides
rootLogger.level=INFO
```
### How was this patch tested?
Manually rendered with `helm template` and validated under multiple
scenarios. All pass strict YAML parsing via `python3 -c "import sys, yaml;
list(yaml.safe_load_all(sys.stdin))"`:
1. Default `values.yaml`.
2. `--set operatorConfiguration.append=false` with all three property
overrides set to empty strings.
3. `tests/e2e/helm/dynamic-config-values.yaml`.
4. `tests/e2e/helm/dynamic-config-values-2.yaml`.
5. `tests/e2e/helm/selector-config-values.yaml`.
6. `tests/e2e/helm/helm-test-values/configmap-metadata/values.yaml`.
Also verified `helm lint build-tools/helm/spark-kubernetes-operator` passes.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.7)
Closes #647
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
