Skip to content

Commit

Permalink
Added flag to pass in CAcerts (flyteorg#242)
Browse files Browse the repository at this point in the history
* Added flag to pass in CAcerts

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* Incorporated feedback

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* caCert instead of caCerts

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* caCert to caCertFilePath

Signed-off-by: Prafulla Mahindrakar <[email protected]>
  • Loading branch information
pmahindrakar-oss authored Dec 30, 2021
1 parent b24a555 commit 4c7bd49
Show file tree
Hide file tree
Showing 9 changed files with 131 additions and 6 deletions.
22 changes: 22 additions & 0 deletions clients/go/admin/cert_loader.go
Original file line number Diff line number Diff line change
@@ -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
}
28 changes: 28 additions & 0 deletions clients/go/admin/cert_loader_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
19 changes: 13 additions & 6 deletions clients/go/admin/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package admin
import (
"context"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"

Expand Down Expand Up @@ -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))
}
Expand Down
28 changes: 28 additions & 0 deletions clients/go/admin/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,41 @@ 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)
assert.NotNil(t, clientSet.AuthMetadataClient())
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) {
Expand Down
1 change: 1 addition & 0 deletions clients/go/admin/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
1 change: 1 addition & 0 deletions clients/go/admin/config_flags.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions clients/go/admin/config_flags_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Empty file.
24 changes: 24 additions & 0 deletions clients/go/admin/testdata/root.pem
Original file line number Diff line number Diff line change
@@ -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-----

0 comments on commit 4c7bd49

Please sign in to comment.