diff --git a/flyteidl/clients/go/admin/cert_loader.go b/flyteidl/clients/go/admin/cert_loader.go new file mode 100644 index 00000000000..2c17c60ee2a --- /dev/null +++ b/flyteidl/clients/go/admin/cert_loader.go @@ -0,0 +1,22 @@ +package admin + +import ( + "fmt" + "io/ioutil" + + "crypto/x509" +) + +// readCACerts from the passed in file at certLoc and return certpool. +func readCACerts(certLoc string) (*x509.CertPool, error) { + rootPEM, err := ioutil.ReadFile(certLoc) + if err != nil { + return nil, fmt.Errorf("unable to read from %v file due to %v", certLoc, err) + } + rootCertPool := x509.NewCertPool() + ok := rootCertPool.AppendCertsFromPEM(rootPEM) + if !ok { + return nil, fmt.Errorf("failed to parse root certificate file %v due to %v", certLoc, err) + } + return rootCertPool, err +} diff --git a/flyteidl/clients/go/admin/cert_loader_test.go b/flyteidl/clients/go/admin/cert_loader_test.go new file mode 100644 index 00000000000..ea0ff19c2ec --- /dev/null +++ b/flyteidl/clients/go/admin/cert_loader_test.go @@ -0,0 +1,28 @@ +package admin + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestReadCACerts(t *testing.T) { + + t.Run("legal", func(t *testing.T) { + x509Pool, err := readCACerts("testdata/root.pem") + assert.NoError(t, err) + assert.NotNil(t, x509Pool) + }) + + t.Run("illegal", func(t *testing.T) { + x509Pool, err := readCACerts("testdata/invalid-root.pem") + assert.NotNil(t, err) + assert.Nil(t, x509Pool) + }) + + t.Run("non-existent", func(t *testing.T) { + x509Pool, err := readCACerts("testdata/non-existent.pem") + assert.NotNil(t, err) + assert.Nil(t, x509Pool) + }) +} diff --git a/flyteidl/clients/go/admin/client.go b/flyteidl/clients/go/admin/client.go index 67afe5a2c58..76e44946980 100644 --- a/flyteidl/clients/go/admin/client.go +++ b/flyteidl/clients/go/admin/client.go @@ -3,6 +3,7 @@ package admin import ( "context" "crypto/tls" + "crypto/x509" "errors" "fmt" @@ -135,17 +136,23 @@ func NewAdminConnection(ctx context.Context, cfg *Config, opts ...grpc.DialOptio if cfg.UseInsecureConnection { opts = append(opts, grpc.WithInsecure()) } else { - // TODO: as of Go 1.11.4, this is not supported on Windows. https://github.com/golang/go/issues/16736 var creds credentials.TransportCredentials + var caCerts *x509.CertPool + var err error + tlsConfig := &tls.Config{} //nolint + // Use the cacerts passed in from the config parameter + if len(cfg.CACertFilePath) > 0 { + caCerts, err = readCACerts(cfg.CACertFilePath) + if err != nil { + return nil, err + } + } if cfg.InsecureSkipVerify { logger.Warnf(ctx, "using insecureSkipVerify. Server's certificate chain and host name wont be verified. Caution : shouldn't be used for production usecases") - tlsConfig := &tls.Config{ - InsecureSkipVerify: true, //nolint - - } + tlsConfig.InsecureSkipVerify = true creds = credentials.NewTLS(tlsConfig) } else { - creds = credentials.NewClientTLSFromCert(nil, "") + creds = credentials.NewClientTLSFromCert(caCerts, "") } opts = append(opts, grpc.WithTransportCredentials(creds)) } diff --git a/flyteidl/clients/go/admin/client_test.go b/flyteidl/clients/go/admin/client_test.go index 55f45e2a124..0d7405aafe6 100644 --- a/flyteidl/clients/go/admin/client_test.go +++ b/flyteidl/clients/go/admin/client_test.go @@ -76,6 +76,7 @@ func TestGetAdditionalAdminClientConfigOptions(t *testing.T) { }) t.Run("legal-from-config", func(t *testing.T) { + once = sync.Once{} clientSet, err := initializeClients(ctx, &Config{InsecureSkipVerify: true}, nil) assert.NoError(t, err) assert.NotNil(t, clientSet) @@ -83,6 +84,33 @@ func TestGetAdditionalAdminClientConfigOptions(t *testing.T) { assert.NotNil(t, clientSet.AdminClient()) assert.NotNil(t, clientSet.HealthServiceClient()) }) + t.Run("legal-from-config-with-cacerts", func(t *testing.T) { + once = sync.Once{} + clientSet, err := initializeClients(ctx, &Config{CACertFilePath: "testdata/root.pem"}, nil) + assert.NoError(t, err) + assert.NotNil(t, clientSet) + assert.NotNil(t, clientSet.AuthMetadataClient()) + assert.NotNil(t, clientSet.AdminClient()) + }) + t.Run("legal-from-config-with-invalid-cacerts", func(t *testing.T) { + once = sync.Once{} + defer func() { + if r := recover(); r == nil { + t.Errorf("The code did not panic") + } + }() + newAdminServiceConfig := &Config{ + Endpoint: config.URL{URL: *u}, + UseInsecureConnection: false, + CACertFilePath: "testdata/non-existent.pem", + PerRetryTimeout: config.Duration{Duration: 1 * time.Second}, + } + + assert.NoError(t, SetConfig(newAdminServiceConfig)) + clientSet, err := initializeClients(ctx, newAdminServiceConfig, nil) + assert.NotNil(t, err) + assert.Nil(t, clientSet) + }) } func TestGetAuthenticationDialOptionClientSecret(t *testing.T) { diff --git a/flyteidl/clients/go/admin/config.go b/flyteidl/clients/go/admin/config.go index 59d9b88a678..c57a5c0078c 100644 --- a/flyteidl/clients/go/admin/config.go +++ b/flyteidl/clients/go/admin/config.go @@ -40,6 +40,7 @@ type Config struct { Endpoint config.URL `json:"endpoint" pflag:",For admin types, specify where the uri of the service is located."` UseInsecureConnection bool `json:"insecure" pflag:",Use insecure connection."` InsecureSkipVerify bool `json:"insecureSkipVerify" pflag:",InsecureSkipVerify controls whether a client verifies the server's certificate chain and host name. Caution : shouldn't be use for production usecases'"` + CACertFilePath string `json:"caCertFilePath" pflag:",Use specified certificate file to verify the admin server peer."` MaxBackoffDelay config.Duration `json:"maxBackoffDelay" pflag:",Max delay for grpc backoff"` PerRetryTimeout config.Duration `json:"perRetryTimeout" pflag:",gRPC per retry timeout"` MaxRetries int `json:"maxRetries" pflag:",Max number of gRPC retries"` diff --git a/flyteidl/clients/go/admin/config_flags.go b/flyteidl/clients/go/admin/config_flags.go index 1e0eb1a9a40..d10d090eb75 100755 --- a/flyteidl/clients/go/admin/config_flags.go +++ b/flyteidl/clients/go/admin/config_flags.go @@ -53,6 +53,7 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet { cmdFlags.String(fmt.Sprintf("%v%v", prefix, "endpoint"), defaultConfig.Endpoint.String(), "For admin types, specify where the uri of the service is located.") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "insecure"), defaultConfig.UseInsecureConnection, "Use insecure connection.") cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "insecureSkipVerify"), defaultConfig.InsecureSkipVerify, "InsecureSkipVerify controls whether a client verifies the server's certificate chain and host name. Caution : shouldn't be use for production usecases'") + cmdFlags.String(fmt.Sprintf("%v%v", prefix, "caCertFilePath"), defaultConfig.CACertFilePath, "Use specified certificate file to verify the admin server peer.") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "maxBackoffDelay"), defaultConfig.MaxBackoffDelay.String(), "Max delay for grpc backoff") cmdFlags.String(fmt.Sprintf("%v%v", prefix, "perRetryTimeout"), defaultConfig.PerRetryTimeout.String(), "gRPC per retry timeout") cmdFlags.Int(fmt.Sprintf("%v%v", prefix, "maxRetries"), defaultConfig.MaxRetries, "Max number of gRPC retries") diff --git a/flyteidl/clients/go/admin/config_flags_test.go b/flyteidl/clients/go/admin/config_flags_test.go index 86b64a8ef91..9bced39d150 100755 --- a/flyteidl/clients/go/admin/config_flags_test.go +++ b/flyteidl/clients/go/admin/config_flags_test.go @@ -141,6 +141,20 @@ func TestConfig_SetFlags(t *testing.T) { } }) }) + t.Run("Test_caCertFilePath", func(t *testing.T) { + + t.Run("Override", func(t *testing.T) { + testValue := "1" + + cmdFlags.Set("caCertFilePath", testValue) + if vString, err := cmdFlags.GetString("caCertFilePath"); err == nil { + testDecodeJson_Config(t, fmt.Sprintf("%v", vString), &actual.CACertFilePath) + + } else { + assert.FailNow(t, err.Error()) + } + }) + }) t.Run("Test_maxBackoffDelay", func(t *testing.T) { t.Run("Override", func(t *testing.T) { diff --git a/flyteidl/clients/go/admin/testdata/invalid-root.pem b/flyteidl/clients/go/admin/testdata/invalid-root.pem new file mode 100644 index 00000000000..e69de29bb2d diff --git a/flyteidl/clients/go/admin/testdata/root.pem b/flyteidl/clients/go/admin/testdata/root.pem new file mode 100644 index 00000000000..856c15ad901 --- /dev/null +++ b/flyteidl/clients/go/admin/testdata/root.pem @@ -0,0 +1,24 @@ +-----BEGIN CERTIFICATE----- +MIIEBDCCAuygAwIBAgIDAjppMA0GCSqGSIb3DQEBBQUAMEIxCzAJBgNVBAYTAlVT +MRYwFAYDVQQKEw1HZW9UcnVzdCBJbmMuMRswGQYDVQQDExJHZW9UcnVzdCBHbG9i +YWwgQ0EwHhcNMTMwNDA1MTUxNTU1WhcNMTUwNDA0MTUxNTU1WjBJMQswCQYDVQQG +EwJVUzETMBEGA1UEChMKR29vZ2xlIEluYzElMCMGA1UEAxMcR29vZ2xlIEludGVy +bmV0IEF1dGhvcml0eSBHMjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB +AJwqBHdc2FCROgajguDYUEi8iT/xGXAaiEZ+4I/F8YnOIe5a/mENtzJEiaB0C1NP +VaTOgmKV7utZX8bhBYASxF6UP7xbSDj0U/ck5vuR6RXEz/RTDfRK/J9U3n2+oGtv +h8DQUB8oMANA2ghzUWx//zo8pzcGjr1LEQTrfSTe5vn8MXH7lNVg8y5Kr0LSy+rE +ahqyzFPdFUuLH8gZYR/Nnag+YyuENWllhMgZxUYi+FOVvuOAShDGKuy6lyARxzmZ +EASg8GF6lSWMTlJ14rbtCMoU/M4iarNOz0YDl5cDfsCx3nuvRTPPuj5xt970JSXC +DTWJnZ37DhF5iR43xa+OcmkCAwEAAaOB+zCB+DAfBgNVHSMEGDAWgBTAephojYn7 +qwVkDBF9qn1luMrMTjAdBgNVHQ4EFgQUSt0GFhu89mi1dvWBtrtiGrpagS8wEgYD +VR0TAQH/BAgwBgEB/wIBADAOBgNVHQ8BAf8EBAMCAQYwOgYDVR0fBDMwMTAvoC2g +K4YpaHR0cDovL2NybC5nZW90cnVzdC5jb20vY3Jscy9ndGdsb2JhbC5jcmwwPQYI +KwYBBQUHAQEEMTAvMC0GCCsGAQUFBzABhiFodHRwOi8vZ3RnbG9iYWwtb2NzcC5n +ZW90cnVzdC5jb20wFwYDVR0gBBAwDjAMBgorBgEEAdZ5AgUBMA0GCSqGSIb3DQEB +BQUAA4IBAQA21waAESetKhSbOHezI6B1WLuxfoNCunLaHtiONgaX4PCVOzf9G0JY +/iLIa704XtE7JW4S615ndkZAkNoUyHgN7ZVm2o6Gb4ChulYylYbc3GrKBIxbf/a/ +zG+FA1jDaFETzf3I93k9mTXwVqO94FntT0QJo544evZG0R0SnU++0ED8Vf4GXjza +HFa9llF7b1cq26KqltyMdMKVvvBulRP/F/A8rLIQjcxz++iPAsbw+zOzlTvjwsto +WHPbqCRiOwY1nQ2pM714A5AuTHhdUDqB1O6gyHA43LL5Z/qHQF1hwFGPa4NrzQU6 +yuGnBXj8ytqU0CwIPX4WecigUCAkVDNx +-----END CERTIFICATE----- \ No newline at end of file