Adding TLS options to Redis & mailer connections#2322
Adding TLS options to Redis & mailer connections#2322rhclayto wants to merge 4 commits intogo-vikunja:mainfrom
Conversation
rhclayto
changed the title
kolaente
left a comment
kolaente
left a comment
There was a problem hiding this comment.
Thanks for the PR! Please see my suggestions below.
pkg/config/config.go
Outdated
| MailerQueuelength Key = `mailer.queuelength` | ||
| MailerQueueTimeout Key = `mailer.queuetimeout` | ||
| MailerForceSSL Key = `mailer.forcessl` | ||
| MailerTLSClientCert Key = `mailer.tlsclientcert` |
There was a problem hiding this comment.
This (and the other new variables) should end on path to make it clear that these expect file paths to certs, not actual certificate strings.
Preview DeploymentPreview deployments for this PR are available at:
The preview environment will start automatically on first visit. Subsequent pushes to this PR will update the Run locally with Dockerdocker pull ghcr.io/go-vikunja/vikunja:pr-2322
docker run -p 3456:3456 ghcr.io/go-vikunja/vikunja:pr-2322Last updated for commit 5f75ea6 |
| //#nosec G402 | ||
| InsecureSkipVerify: config.MailerSkipTLSVerify.GetBool(), | ||
| ServerName: config.MailerHost.GetString(), | ||
| MinVersion: tls.VersionTLS12, |
There was a problem hiding this comment.
Why this change?
This is a breaking change, we can't just add it like this.
| } | ||
|
|
||
| if config.RedisTLS.GetBool() { | ||
| tlsConfig := &tls.Config{ |
There was a problem hiding this comment.
It seems like the two ways the TLS config is set up is pretty much the same for redis and mail, just the inputs are different. Can you make that a helper function?
2 participants
Resolves #2321
Resolves #2323