Fix duplicate dbms.connector.https.enabled key in default-config ConfigMap on neo4j-standalone

[andrewnicolalde]

This PR fixes a bug in the neo4j-standalone template which causes a duplicate dbms.connector.https.enabled key to be created in the {{ .Release.Name }}-default-config ConfigMap.

dbms.connector.https.enabled is set in neo4j-community.conf and neo4j-enterpise.conf and imported as a ConfigMap key into the {{ .Release.Name }}-default-config ConfigMap. However, a duplicate dbms.connector.https.enabled key is created if the user has set ssl.https.privateKey.secretName in values.yaml.

Note the two dbms.connector.https.enabled keys in the resulting ConfigMap:

# Source: neo4j-standalone/templates/neo4j-config.yaml
# Default Neo4j config values, these are overridden by user-provided values in neo4j-standalone-user-config
apiVersion: v1
kind: ConfigMap
metadata:
  name: "neo4j-standalone-default-config"
  namespace: "foo"
  labels:
    app: "bar"    
data:

  # Neo4j defaults
  dbms.tx_state.memory_allocation: ON_HEAP
  dbms.connector.bolt.enabled: 'true'
  dbms.connector.http.enabled: 'true'
  dbms.connector.https.enabled: 'false'
  dbms.tx_log.rotation.retention_policy: 1 days
  dbms.windows_service_name: neo4j

  # Helm defaults
  dbms.mode: "SINGLE"

  # Bolt keep alive
  # this helps to ensure that LoadBalancers do not close bolt connections that are in use but appear idle
  dbms.connector.bolt.connection_keep_alive: "30s"
  dbms.connector.bolt.connection_keep_alive_for_requests: "ALL"
  dbms.connector.bolt.connection_keep_alive_streaming_scheduling_interval: "30s"

  # If we set default advertised address it over-rides the bolt address used to populate the browser in a really annoying way
  # dbms.default_advertised_address: "$(bash -c 'echo ${SERVICE_DOMAIN}')"


  # Other
  dbms.config.strict_validation: "true"
  dbms.logs.user.stdout_enabled: "false"
  unsupported.dbms.ssl.system.ignore_dot_files: "true"
  # Logging
  dbms.directories.logs: "/logs"
  # Import
  dbms.directories.import: "/import"

  # Use more reliable defaults SSL / TLS settings for K8s
  dbms.ssl.policy.bolt.client_auth: "NONE"
  dbms.ssl.policy.https.client_auth: "NONE"
  dbms.connector.bolt.tls_level: "REQUIRED"
  dbms.connector.https.enabled: "true"
  # Automatically enable SSL policy for bolt because privateKey secret is present
  dbms.ssl.policy.bolt.enabled: "true"
  # Automatically enable SSL policy for https because privateKey secret is present
  dbms.ssl.policy.https.enabled: "true"

Duplicate keys violate the YAML 1.2 specification, which means documents with duplicate keys will fail validation by the likes of go-yaml, which is the underlying yaml parsing library used by popular Kubernetes tools like kustomize, resulting in errors like:

Error: map[string]interface {}(nil): yaml: unmarshal errors:
  line 41: mapping key "dbms.connector.https.enabled" already defined at line 16

Related kustomize PR: kubernetes-sigs/kustomize#4096

Same issue on GitLab's helm chart: https://gitlab.com/gitlab-org/charts/gitlab/-/issues/2582

[andrewnicolalde]

I have not checked to see if there are other keys which are similarly duplicated in this manifest or any others, but there may well be more.

[andrewnicolalde]

@harshitsinghvi22 What do you think about these changes? Is there anything you'd like to see done differently to make it merge-worthy?

[harshitsinghvi22]

i think we did fix this issue in past but it seems we missed it ..i will take a look and update you back in sometime

[andrewnicolalde]

i think we did fix this issue in past but it seems we missed it ..i will take a look and update you back in sometime

@harshitsinghvi22 Hi Harshit! Have you had a chance to have a look at the underlying cause which PR (hopefully) would fix? Please do let me know if there's anyway I could provide further insight 🙂

[harshitsinghvi22]

Hi @andrewnicolalde

Unfortunately, we are occupied with other priority tasks and this has been put to backlog.

Will update once this is looked at.

Thanks
Harshit

[andrewnicolalde]

No worries, looking forward to it :).

[harshitsinghvi22]

this is fixed..not able to reproduce with 4.4.19 standalone and cluster core charts

[awinter125-pdy]

@harshitsinghvi22 this is not fixed. The relevant code is not changed nor has the pr been merged.

Still getting
HelmRelease reconciliation failed: Helm install failed: error while running post render on files: map[string]interface {}(nil): yaml: unmarshal errors: line 41: mapping key "dbms.connector.https.enabled" already defined at line 17