[bitnami/zookeeper] Make Zookeeper DefaultMode YAML 1.2 Compliant

[schrieveslaach]

Description of the change

With the YAML 1.2 spec octals must be prefixed with 0o and not 0. Parsers implementing the latest spec run into issues.

Benefits

This commit makes the Zookeeper script mount compliant with YAML 1.2.

Possible drawbacks

Applicable issues

None at the moment

Additional information

See background discussion here: dtolnay/serde-yaml#225

Checklist

• Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
• Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
• Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
• All commits signed off and in agreement of Developer Certificate of Origin (DCO)

[andresbono]

Hi, thanks for opening this PR! I have some concerns about merging this PR because the Kubernetes API Reference for v1.28 is still using the YAML v1.1 format for octals:

Must be an octal value between 0000 and 0777 [...] YAML accepts both octal and decimal values [...]. Defaults to 0644.

Could this cause some backwards incompatibility in some old (or current) versions of the k8s tooling? helm, kubectl... Would this change generate confudion for users as the official k8s API spec is documented using the old format? If we apply this update, shouldn't we apply it globally to all the charts for homogeneity?

What is the specific issue you are finding by sticking to YAML v1.1 format?

[schrieveslaach]

What is the specific issue you are finding by sticking to YAML v1.1 format?

I'm trying to parse the output of this Helm chart with Rust and the crate kube. This is needed for aixigo/PREvant#146 (code will be public in a few days). I came across dtolnay/serde-yaml#225 which showed Rust's serde library uses YAML 1.2 and thus my code failed because defaultMode was then a string and not a numeric value anymore.

However, I noticed that the chart also makes use of decimals (see TLS secrets) and thus I applied the same trick and this addresses following questions:

Could this cause some backwards incompatibility in some old (or current) versions of the k8s tooling? helm, kubectl... Would this change generate confudion for users as the official k8s API spec is documented using the old format?

If we apply this update, shouldn't we apply it globally to all the charts for homogeneity?

I was thinking about doing so but then I read in the contribution guide to only raise PRs for single charts. I'm up to do it for all charts. Just tell me what is your preferred approach.

[schrieveslaach]

@andresbono, after fighting the CI a bit I got it working and CI is green. Do you mind to have another look?

[andresbono]

However, I noticed that the chart also makes use of decimals (see TLS secrets) and thus I applied the same trick

That makes total sense 👍

If we apply this update, shouldn't we apply it globally to all the charts for homogeneity?

I was thinking about doing so but then I read in the contribution guide to only raise PRs for single charts. I'm up to do it for all charts. Just tell me what is your preferred approach.

Giving it a second thought, let's keep the rest of the charts as they are right now. We can reconsider moving to YAML 1.2 based on future user requests. But let's merge your changes so your use case is also covered.

[andresbono]

LGTM

[schrieveslaach]

Thanks a lot.

[schrieveslaach]

@andresbono, I did the same trick for Kafka (see #21086). Do you mind to check this out as well?