Platform Chart changes to Temporal Deployment causing TLS issues.

[PurseChicken]

Helm Chart Version

0.399.0

What step the error happened?

On deploy

Relevant information

The Temporal Deployment manifest was changed to assume that TLS\SSL is needed if using an external Database:

        {{- if eq .Values.global.database.type "external" }}
        # Assume an external database requires SSL.
          - name: POSTGRES_TLS_ENABLED
            value: "true"
          - name: POSTGRES_TLS_DISABLE_HOST_VERIFICATION
            value: "true"
          - name: SQL_TLS_ENABLED
            value: "true"
          - name: SQL_TLS_DISABLE_HOST_VERIFICATION
            value: "true"
        {{- end }}

This causes significant issues when using, for example, a sidecar Proxy which does not require TLS.

We should not be making an assumption here. In my opinion, there should be a key value pair in values which enables \ disables TLS\SSL if required in the deployment.

E.G.

temporal:
  enabled: true
  externalSSL: false

Relevant log output

[Fx] Error returned: received non-nil error from function "go.temporal.io/server/temporal".ServerOptionsProvider
	/home/builder/temporal/temporal/fx.go:180:
sql schema version compatibility check failed: pq: SSL is not enabled on the server

[chulkilee]

Hitting this issue as well. Please do not make assumptions without making it configurable.

[film42]

@davinchia Can we get a fix for this? This broke the helm chart.

Edit: Why is it a bad assumption?

1. Using a service mesh like isito where you intend for app-to-app traffic to "appear" unencrypted so the service mesh can handle all of the encryption for you.
2. If you're running a local sidecar like google cloud sql auth proxy which handles ssl on your behalf.

Hope that helps.

[davinchia]

Hi everyone, thanks for the feedback, and sorry for the inconvenience.

Curious, why are folks setting this field? This doesn't do anything except enable Temporal SSL on the backend, and is more of an experimental flag we are testing internally. Unsetting this should fix this for everyone.

[film42]

I didn’t set any flag, I think. It failed when I upgraded the helm chart version.

[PurseChicken]

Hi everyone, thanks for the feedback, and sorry for the inconvenience.

Curious, why are folks setting this field? This doesn't do anything except enable Temporal SSL on the backend, and is more of an experimental flag we are testing internally. Unsetting this should fix this for everyone.

What field are you referring to? .Values.global.database.type?

The issue here is that if .Values.global.database.type is set to external, then automatically the Temporal deployment gets specific TLS environment variables applied which is causing issues. I believe most of us who are running into issues are using external databases and setting .Values.global.database.type to external.

Setting that flag to internal when actually using an external database does not seem like the right way to configure the charts values file. If thats the instruction, thats not an acceptable fix in my opinion. More like a workaround with other possible downstream affects. If not now, then in the future as that field may be referenced in other areas of the chart(s).

As mentioned in the OP, these environment variables for Temporal TLS should be controlled in the temporal section of the charts values file rather than setting them based on an assumption.

[djpirra]

How was this solved? I am trying to use an external PostgreSQL database and having this notification of SSL.

sql schema version compatibility check failed: pq: SSL is not enabled on the server
Unable to create server. Error: could not build arguments for function "go.uber.org/fx".(*module).constructCustomLogger.func2 (/go/pkg/mod/go.uber.org/[email protected]/module.go:251): failed to build fxevent.Logger: could not build arguments for function "go.temporal.io/server/temporal".init.func8 (/home/builder/temporal/temporal/fx.go:1029): failed to build log.Logger: received non-nil error from function "go.temporal.io/server/temporal".ServerOptionsProvider (/home/builder/temporal/temporal/fx.go:180): sql schema version compatibility check failed: pq: SSL is not enabled on the server.

[film42]

For the moment I deploy the helm chart and manually edit the airbyte-temporal so the following envs are "false":

        - name: POSTGRES_TLS_ENABLED
          value: "false"
        - name: SQL_TLS_ENABLED
          value: "false"

If you're using something like kustomize you might be able to apply a patch to do this but the stack I have using Airbyte isn't set up that way. Hoping to see this issue fixed soon.

[marcosmarxm]

Hello all 👋 sorry the lack of updates. I asked the platform team to take a look into airbytehq/airbyte-platform#361 any return from them I'm going to update here.

[PurseChicken]

Just checking in on this issue. This is still a problem in v1.1.1 of the helm chart. This is preventing us from updating.

Thank you

[PurseChicken]

As a workaround to this issue, you can set ".Values.global.database.type" to internal, and then the TLS environment variables will not be added to the temporal deployment.

I looked pretty heavily through the charts manifests, and currently it seems only the temporal deployment references the ".Values.global.database.type" key in values. If that key is ever used elsewhere in the future this could be problematic though.. but for now as a workaround you can set it to the default of internal. I do feel strongly that this should still be tied to something more specific and not by an assumption.

[kev-datams]

@PurseChicken the problem is that we want to use an external database without SSL, as the CloudSQL Auth Proxy (GCP) is already managing the TLS/SSL natively