asprom icon indicating copy to clipboard operation
asprom copied to clipboard

Published 20 hours ago • verified publisher icon alicebob

tls_support

Open Liron24 opened this issue 6 years ago • 5 comments

Added tls support Changed arguments to support it and used NewSecureConnection

Liron24 avatar Nov 30 '18 01:11 Liron24

Hi, sorry for the delay. Thanks for the PR! I have a few questions, but I'll start with the most important one: would it be possible to keep all the current flags as-is, and have the TLS options be new flags only? I'm thinking something along these lines:

enableTLS = flag.Bool("tls", false, "enable TLS")
tlsName   = flag.String("tlsName", "myServer", "TLS name")
tlsKey    = flag.String("tlsKey", "./tls.key", "TLS certificate - key")
tlsCert   = flag.String("tlsCert", "./tls.cert", "TLS certificate - cert")

alicebob avatar Dec 04 '18 05:12 alicebob

Hi, Yes it's definitely possible. I wanted to seperate the port from the node address since usually secured connection to aerospike is done via port 3010 and not 3000. I will refactor and update the PR.

Liron24 avatar Dec 04 '18 10:12 Liron24

Done.

Liron24 avatar Dec 04 '18 16:12 Liron24

Thanks!

  • Please run all code through go fmt
  • The default for all flags should stay no TLS, I think (also the port should stay :3000 by default?)
  • It looks like it always makes a secure connection now (with NewSecureConnection()). Are you sure this still works with the default (non-secure) connection?

alicebob avatar Dec 04 '18 17:12 alicebob

You were correct I was missing an if to choose between secure connection and non secure connection. Fixed and performed other requests as well.

Liron24 avatar Dec 04 '18 18:12 Liron24