[bitnami/external-dns] Fix bug with trailing comma in azure.json

[MaxAnderson95]

This PR resolves #33882.

A breaking change was introduced in external-dns 0.17.0 which now (correctly) fails to parse a previously innocuous bug in how the azure.json was generated in this chart. Since the introduction of workload identity support in this chart (and likely before), the Azure credentials were generated by constructing a raw JSON string. When using only workload identity, this resulted in an invalid azure.json containing a trailing comma on the last key-value pair:

{
  "tenantId": "11111111-1111-1111-1111-111111111111",
  "subscriptionId": "0000000-0000-0000-0000-000000000000",
  "resourceGroup": "foo-group",
  "useWorkloadIdentityExtension":  true,
}

In external-dns 0.16.1 this was apparently accepted as valid JSON and did not cause a parsing error. Since the upgrade to chart version 8.8.3 which included external-dns 0.17.0, this is now causing a parsing error:

failed to read Azure config file '/etc/kubernetes/azure.json': failed to parse Azure config file '/etc/kubernetes/azure.json': invalid character '}' looking for beginning of object key string

Description of the change

This PR uses the toJson helm function to dynamically generate the JSON object, eliminating the need to worry about key order or trailing commas.

Benefits

The method is much cleaner rather than constructing a raw string.

Possible drawbacks

One slight drawback is that falsey values now appear in the JSON object passed to azure.json.

Before:

{
  "tenantId": "11111111-1111-1111-1111-111111111111",
  "subscriptionId": "0000000-0000-0000-0000-000000000000",
  "resourceGroup": "foo-group",
  "useWorkloadIdentityExtension":  true,
}

After:

{
  "aadClientId": "",
  "aadClientSecret": "",
  "cloud": "",
  "resourceGroup": "foo-group",
  "subscriptionId": "0000000-0000-0000-0000-000000000000",
  "tenantId": "11111111-1111-1111-1111-111111111111",
  "useManagedIdentityExtension": false,
  "useWorkloadIdentityExtension": true,
  "userAssignedIdentityID": ""
}

I tested in one of my test clusters and had no issues, and I also reviewed the relevant parsing code and found no indication it would handle falsey values differently than non-existent keys.

Applicable issues

• Fixes azure.json cannot be parsed when working with workload identity #33882

Checklist

• Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
• 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)

[migruiz4]

Hi there!

Thank you for your contribution @MaxAnderson95! I agree that using toJson here is the way to go, but I'm worried about transforming the entire .Values.azure into azure.json.

In the future, extra values may be added under .Values.azure that we may not want to add into the azure.json and cause issues for existing deployments by accident.

I would rather keep logic similar to how it currently is:

• Build a dict from scratch.
• Add key-values individually.
• Then convert the dict using toJson (instead of printing it line by line as it currently is).

It is not as pretty, but it would give us better control over what is included in the final Json.
If external-dns adds a new supported field for the azure.json, then we just need to add a new value to the values.yaml and update the helper.

[MaxAnderson95]

Hi there!

Thank you for your contribution @MaxAnderson95! I agree that using toJson here is the way to go, but I'm worried about transforming the entire .Values.azure into azure.json.

In the future, extra values may be added under .Values.azure that we may not want to add into the azure.json and cause issues for existing deployments by accident.

I would rather keep logic similar to how it currently is:

• Build a dict from scratch.
• Add key-values individually.
• Then convert the dict using toJson (instead of printing it line by line as it currently is).

It is not as pretty, but it would give us better control over what is included in the final Json. If external-dns adds a new supported field for the azure.json, then we just need to add a new value to the values.yaml and update the helper.

No worries, better safe than sorry! I'll get those changes pushed soon.

EDIT: Changes pushed, ready for re-review

[MaxAnderson95]

@migruiz4 can you re-review this?

[MaxAnderson95]

Sorry for the notification spam. I screwed up trying to fix a merge conflict. Anyway I think we're good now.

[MaxAnderson95]

@migruiz4 sorry to bother, but do you mind re-reviewing the changes you requested? I keep having to re-base this PR because changes are being made in other branches.

[migruiz4]

I'm very sorry for the late response @MaxAnderson95, thank you for applying my suggestion!