Skip to content

Add TLS to redis config where required

What does this MR do?

Currently if a Redis host that uses SSL is used for GitLab KAS, KAS will refuse to connect, citing an i/o timeout. A confusing error, but one that eventually resulted in testing whether disabling in-transit encryption on our Redis instance (actually an Elasticache instance), would allow KAS to connect and succeed, similarly to the rest of GitLab.

This test proved successful, and the lack of redis.tls.enabled = true in the config was found to be the culprit. With in-transit encryption enabled, setting that field to true allowed the connection to succeed, and setting it to false replicated the i/o timeout error in the logs.

This MR resolves this by looking up the redis_ssl field in the gitlab.rb config, and enabling or disabling redis.tls.enabled based on the config value.

I have updated the tests which pass locally, but I ran out of shared minutes after I resolved the failure I saw on my fork.

Happy to hear of any adjustments or changes required to merge this, as this will allow us to get rid of a workaround that we need to run after every gitlab-ctl reconfigure, or running a separate Redis instance without TLS enabled.

I've not contributed to GitLab before, so please feel free to adjust this MR as you see fit. I would also accept change requests, I wasn't sure whether to modify an existing test to see whether by default it would set redis.tls.enabled = false, but I am definitely happy to add that to save regressing on this behaviour in the future.

Related issues

Checklist

See Definition of done.

For anything in this list which will not be completed, please provide a reason in the MR discussion

Required

  • Merge Request Title, and Description are up to date, accurate, and descriptive
  • MR targeting the appropriate branch
  • MR has a green pipeline on GitLab.com
  • Pipeline is green on dev.gitlab.org if the change is touching anything besides documentation or internal cookbooks
  • trigger-package has a green pipeline running against latest commit

Expected (please provide an explanation if not completing)

  • Test plan indicating conditions for success has been posted and passes
  • Documentation created/updated
  • Tests added
  • Integration tests added to GitLab QA
  • Equivalent MR/issue for the GitLab Chart opened

Merge request reports

Loading