Hardcoded transport `tcp` comms to the leader node

[michaelhyatt]

The template seem to hardcode tcp as the protocol. Can you please add support for tls?

helm-charts/helm-chart-sources/logstream-leader/templates/_pod.tpl

Line 89 in c041366

value: "tcp://{{ .Values.config.token }}@{{ .Values.config.bindHost }}:4200"

helm-charts/helm-chart-sources/logstream-leader/templates/_pod.tpl

Line 91 in c041366

value: "tcp://{{ .Values.config.token }}@{{ .Values.config.bindHost }}:4200"

[yenoromm]

A string value at config.overrideLeaderURL would be ideal. This would provide a very flexible value for also configuring tls.<settingName> in the url

[bdalpe]

@michaelhyatt @yenoromm You can do this today.

1. Create a secret (either on your own or use the extraObjects value)
2. Configure your Helm Values like the following:

config: 
  useExistingSecret: true

envValueFrom:
  - name: CRIBL_DIST_MASTER_URL
    valueFrom:
      secretKeyRef:
        name: <secretName>
        key: <secretKey>

Bonus setting for extraObjects:

extraObjects:
  - apiVersion: v1
    kind: Secret
    metadata:
      name: cribl-leader-secret
    type: Opaque
    data:
      leaderUrl: "tls://authToken@criblleader:4200?group=foo"  

Extra objects supports a string or object for each array item. You can see more examples of how to use this in https://github.com/criblio/helm-charts/blob/master/helm-chart-sources/logstream-workergroup/tests/extras_test.yaml

[yenoromm]

@bdalpe

If you look at the first comment or the PR, you will notice we are talking about the leader helm chart and not the workergroup chart.

The config.useExistingSecret is well documented and what we use to set the leader url for the workergroup.

[bdalpe]

Sorry, @yenoromm I missed that detail.

You can still do something similar for the leader helm chart by redefining the environment variable. Your new definition in the helm values would win.

https://stackoverflow.com/questions/73443818/precedence-rule-in-container-env-list-with-duplicate-name-key-in-kubernetes

I agree this is not perfect and should be addressed long-term. IMO, the best way to do this would be to remove the current way it's done and replace with user specifying all the details. That would be a major breaking change though.

[yenoromm]

In the PR it supplies the current value if an override value is not supplied. There should be no breaking change only added functionality if the user decides to use it.

We are using Pulumi to handle deployments. The tooling throws an error with duplicate keys even if it isn’t caused by helm directly. Therefore this isn’t a solution for us, other Pulumi shops and possibly even Terraform shops if the underlying provider is what throws the error.