feat: Add TLS support. Closes #2764 #2766
Conversation
| golang.org/x/crypto v0.0.0-20200311171314-f7b00557c8c4 | ||
| golang.org/x/net v0.0.0-20200301022130-244492dfa37a | ||
| golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d | ||
| golang.org/x/tools v0.0.0-20200414205525-6a72e3782ce5 // indirect | ||
| google.golang.org/api v0.20.0 | ||
| google.golang.org/genproto v0.0.0-20200317114155-1f3552e48f24 | ||
| google.golang.org/grpc v1.28.0 | ||
| gopkg.in/gavv/httpexpect.v2 v2.0.0 |
There was a problem hiding this comment.
replaced this with go.mod format
| baseHRef string | ||
| baseHRef string | ||
| // https://itnext.io/practical-guide-to-securing-grpc-connections-with-go-and-tls-part-1-f63058e9d6d1 | ||
| tlsConfig *tls.Config |
There was a problem hiding this comment.
this one variable is everything
| @@ -55,8 +57,12 @@ func (a *argoServerClient) NewInfoServiceClient() (infopkg.InfoServiceClient, er | |||
| return infopkg.NewInfoServiceClient(a.ClientConn), nil | |||
| } | |||
|
|
|||
| func NewClientConn(argoServer string) (*grpc.ClientConn, error) { | |||
| conn, err := grpc.Dial(argoServer, grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(MaxClientGRPCMessageSize)), grpc.WithInsecure()) | |||
| func newClientConn(opts ArgoServerOpts) (*grpc.ClientConn, error) { | |||
There was a problem hiding this comment.
Instead of using argo --secure xxx, can we check the URL to see if it starts with https:// to determine? This will make it easy to use.
There was a problem hiding this comment.
I did consider usability, which is why you can set an env var, or you can use -e. The ARGO_SERVER is host:port not proto:https://host:port, so this would be more complex.
There was a problem hiding this comment.
I would prefer to make ARGO_SERVER smarter to support both host:port and protocol:https://host:port. Think about kubectl, curl and wget, they never ask the user to give an argument to indicate that. But it's up to you, I'll approve it.
| @@ -384,7 +384,8 @@ metadata: | |||
| name: argo-server | |||
| spec: | |||
| ports: | |||
| - port: 2746 | |||
| - name: web | |||
There was a problem hiding this comment.
A suggestion, can we use a different port when it's running secure mode?
There was a problem hiding this comment.
I did consider this. However, I wanted to keep things as simple as possible. If you want to run it on another port, you could use the --port flag to do so.
|
Kudos, SonarCloud Quality Gate passed!
|
Checklist:
"fix(controller): Updates such and such. Fixes #1234".How Is This Different to Argo CD?
The usage is documented in docs/tls.md, but essentially:
--insecureflag, we have a more specific--insecure-skip-verifyflag.