Add tls to gin apiserver #363
Conversation
Gregory-Pereira
force-pushed
the
add-tls-to-gin-apiserver
branch
from
854fbc4 to
c065a22
Compare
ui/apiserver/apiserver.go
Outdated
| api.logger.Warnf("failed to load client certificate/key: %w", err) | ||
| return defaultHTTPClient, fmt.Errorf("Error load client certificate/key, defaulting to TLS Insecure session (http)") | ||
| } | ||
| // // NOT SURE WE NEED SERVER CA CERT FOR THIS, PLEASE ADVISE |
There was a problem hiding this comment.
I think you don't need to load it for the client, but the root ca need to be locally deployed (mkcert -i in the apiserver container to make the http request. For prod I think we need to update ansible to make sure that it generates the cert/keys/rootca and deploy those in the gobot http server as well as the apiserver client.
On the side note, I think things will be much better and cleaner once we move to kuberentes/ocp/kind for cert management.
| tlsServerCaCertPath := pflag.String("tls-server-ca-cert", "", "Path to the TLS server CA certificate. Evantually defaults to '$HOME/server-ca-crt.pem2'") | ||
| pflag.Parse() | ||
|
|
||
| /* ENV support, most variabls take 3 options, with the following priority: |
There was a problem hiding this comment.
I think let's take these as an input parameter only. We should probably move to urfave/cli? It does pretty clean job with these 3 options. Irrespective of that, I think it would be good if we take care of this (checking env variable) across all the go binaries on the repo in separate PR? wdyt?
ui/apiserver/apiserver.go
Outdated
| preCheckEndpointURL := pflag.String("precheck-endpoint", "", "Precheck endpoint URL") | ||
| InstructLabBotUrl := pflag.String("bot-url", InstructLabBotUrl, "InstructLab Bot URL") | ||
| // TLS variables | ||
| tlsInsecure := pflag.Bool("tls-insecure", false, "Whether to skip TLS verification") |
There was a problem hiding this comment.
I think we should introduce a "dev" flag, that basically means --tls-insecure or http. Switching to insecure connection because the tls handshake failed is not a good idea in my opinion. in non-dev mode, if tls fails, just fail, log error and exit. In dev mode, just use --tls-insecure or even http. That will give user the behavior they are expecting from these cli flags.
| network_mode: "host" | ||
| depends_on: | ||
| - redis | ||
| env_file: |
There was a problem hiding this comment.
I think you also need to fix the deploy/compose/deplo
Signed-off-by: greg pereira <[email protected]>
Signed-off-by: greg pereira <[email protected]>
Signed-off-by: greg pereira <[email protected]>
Gregory-Pereira
force-pushed
the
add-tls-to-gin-apiserver
branch
from
06b33e8 to
30b7da4
Compare
Signed-off-by: greg pereira <[email protected]>
Gregory-Pereira
force-pushed
the
add-tls-to-gin-apiserver
branch
from
30b7da4 to
c483619
Compare
2 participants
Addresses: #361
Looking for feedback:
sendPostRequestmethod for the API server, I load in the server CA certificate, which I am not sure I need.Changes:
PTAL @vishnoianil @nerdalert