Support migrator.databaseAuthOverrideEnvVars in sourcegraph-migrator chart job

[loujar]

Allowing override of migrator auth ENVs (.Values.migrator.databaseAuthOverrideEnvVars) in the sourcegraph-migrator chart's sourcegraph-migrator job template, consistent with the sourcegraph chart's sourcegraph-frontend Deployment migrator initContainer
https://github.com/sourcegraph/deploy-sourcegraph-helm/blob/v5.8.1579/charts/sourcegraph/templates/frontend/sourcegraph-frontend.Deployment.yaml#L57

Checklist

• Follow the manual testing process
• Update changelog
• Update Kubernetes update doc

Test plan

CI

[loujar]

@DaedalusG I could use your opinion on this. The user requesting this change also asked if we could include a similar conditional for the frontend container's auth ENV. This seems like a reasonable request, but I'm struggling with the best approach to implement this. Considering three options:

1. Use the same .Values.migrator.databaseAuthOverrideEnvVars config key for the frontend container, which is not ideal considering it's nested under the migrator section but at the same time the auth overrides shouldn't differ
2. Add a duplicate .Values.frontend.databaseAuthOverrideEnvVars config key, which is not ideal because it would be duplicating the same functional config in two different place in our helm config values
3. Changing the .Values.migrator.databaseAuthOverrideEnvVars config key to a more generic name like .Values.sourcegraph.databaseAuthOverrideEnvVars, which would potentially break the chart for users that are already leveraging the .Values.migrator.databaseAuthOverrideEnvVars config value

What's the least worst option in your opinion? Maybe we could implement #3 and keep .Values.migrator.databaseAuthOverrideEnvVars for some amount of time in a double (OR) conditional for backwards compatibility.

[DaedalusG]

@loujar I agree with he approach of just broadening the options to hit the migrator job and frontend. Perhaps scoped to override anything using sourcegraph.databaseAuth

Its surprising to me that other customers are using this config option since it was mostly introduced for scale testing it looks like: #181 In what scenario can't the connection vars just be set in the values file? Do we expect many users outside of SG team are using this?

The repo seems to imply that 2 may actually be most inline with the style conventions of the repo: sourcegraph/sourcegraph-public-snapshot#29063 (comment)

[DaedalusG]

I see why you're torn though, discussed with team a little and now I'm sympathetic to 1 since the frontend is already using the config defined in migrator, also yea, we should never really run migrator differently in different services (or at least I can't think of a case where we'd want that)