Clean up the platform config (istio#3628)

* Clean up the platform config

* Fix lint

security/cmd/node_agent/main.go

@@ -28,11 +28,6 @@ import (
 	"istio.io/istio/security/pkg/cmd"
 )
 
-const (
-	// The default path/file of root cert.
-	defaultRoot = "/etc/certs/root-cert.pem"
-)
-
 var (
 	naConfig = na.NewConfig()
 
@@ -65,20 +60,12 @@ func init() {
 	flags.StringVar(&naConfig.Env, "env", "unspecified",
 		"Node Environment : unspecified | onprem | gcp | aws")
 
-	flags.StringVar(&naConfig.PlatformConfig.OnPremConfig.CertChainFile, "onprem-cert-chain",
-		"/etc/certs/cert-chain.pem", "Node Agent identity cert file in on premise environment")
-	flags.StringVar(&naConfig.PlatformConfig.OnPremConfig.KeyFile,
-		"onprem-key", "/etc/certs/key.pem", "Node identity private key file in on premise environment")
-	flags.StringVar(&naConfig.PlatformConfig.OnPremConfig.RootCACertFile, "onprem-root-cert",
-		defaultRoot, "Root Certificate file in on premise environment")
-
-	flags.StringVar(&naConfig.PlatformConfig.GcpConfig.RootCACertFile, "gcp-root-cert",
-		defaultRoot, "Root Certificate file in GCP environment")
-	flags.StringVar(&naConfig.PlatformConfig.GcpConfig.CAAddr, "gcp-ca-address",
-		"istio-ca:8060", "Istio CA address in GCP environment")
-
-	flags.StringVar(&naConfig.PlatformConfig.AwsConfig.RootCACertFile, "aws-root-cert",
-		defaultRoot, "Root Certificate file in AWS environment")
+	flags.StringVar(&naConfig.CertChainFile, "cert-chain",
+		"/etc/certs/cert-chain.pem", "Node Agent identity cert file")
+	flags.StringVar(&naConfig.KeyFile,
+		"key", "/etc/certs/key.pem", "Node Agent private key file")
+	flags.StringVar(&naConfig.RootCertFile, "root-cert",
+		"/etc/certs/root-cert.pem", "Root Certificate file")
 
 	naConfig.LoggingOptions.AttachCobraFlags(rootCmd)
 	cmd.InitializeFlags(rootCmd)

security/cmd/node_agent/na/config.go

@@ -18,7 +18,6 @@ import (
 	"time"
 
 	"istio.io/istio/pkg/log"
-	"istio.io/istio/security/pkg/platform"
 )
 
 const (
@@ -56,11 +55,17 @@ type Config struct {
 	// percentage of the entire certificate TTL.
 	CSRGracePeriodPercentage int
 
-	// The Configuration for talking to the platform metadata server.
-	PlatformConfig platform.ClientConfig
-
 	// LoggingOptions is the options for Istio logging.
 	LoggingOptions *log.Options
+
+	// CertChainFile defines the cert chain file of node agent.
+	CertChainFile string
+
+	// KeyFile defines the private key of node agent.
+	KeyFile string
+
+	// RootCertFile defines the root cert of node agent.
+	RootCertFile string
 }
 
 // NewConfig creates a new Config instance with default values.
@@ -69,7 +74,6 @@ func NewConfig() *Config {
 		CSRInitialRetrialInterval: defaultCSRInitialRetrialInterval,
 		CSRMaxRetries:             defaultCSRMaxRetries,
 		CSRGracePeriodPercentage:  defaultCSRGracePeriodPercentage,
-		PlatformConfig:            platform.ClientConfig{},
 		LoggingOptions:            log.NewOptions(),
 	}
 }

security/cmd/node_agent/na/nafactory.go

@@ -44,19 +44,19 @@ func NewNodeAgent(cfg *Config) (NodeAgent, error) {
 	}
 
 	env := determinePlatform(cfg)
-	if pc, err := platform.NewClient(env, cfg.PlatformConfig, cfg.IstioCAAddress); err == nil {
-		na.pc = pc
-	} else {
+	pc, err := platform.NewClient(env, cfg.RootCertFile, cfg.KeyFile,
+		cfg.CertChainFile, cfg.IstioCAAddress)
+	if err != nil {
 		return nil, err
 	}
+	na.pc = pc
 
 	cAClient := &grpc.CAGrpcClientImpl{}
 	na.cAClient = cAClient
 
 	// TODO: Specify files for service identity cert/key instead of node agent files.
 	secretServer, err := workload.NewSecretServer(
-		workload.NewSecretFileServerConfig(cfg.PlatformConfig.OnPremConfig.CertChainFile,
-			cfg.PlatformConfig.OnPremConfig.KeyFile))
+		workload.NewSecretFileServerConfig(cfg.CertChainFile, cfg.KeyFile))
 	if err != nil {
 		log.Errorf("Workload IO creation error: %v", err)
 		os.Exit(-1)

security/cmd/node_agent/na/nodeagent_test.go

@@ -44,13 +44,6 @@ func (f FakeCertUtil) GetWaitTime(certBytes []byte, now time.Time, gracePeriodPe
 }
 
 func TestStartWithArgs(t *testing.T) {
-	generalPcConfig := platform.ClientConfig{
-		OnPremConfig: platform.OnPremConfig{
-			RootCACertFile: "ca_file",
-			KeyFile:        "pkey",
-			CertChainFile:  "cert_file",
-		},
-	}
 	generalConfig := Config{
 		IstioCAAddress:     "ca_addr",
 		ServiceIdentityOrg: "Google Inc.",
@@ -59,8 +52,10 @@ func TestStartWithArgs(t *testing.T) {
 		CSRInitialRetrialInterval: time.Millisecond,
 		CSRMaxRetries:             3,
 		CSRGracePeriodPercentage:  50,
-		PlatformConfig:            generalPcConfig,
 		LoggingOptions:            log.NewOptions(),
+		RootCertFile:              "ca_file",
+		KeyFile:                   "pkey",
+		CertChainFile:             "cert_file",
 	}
 	testCases := map[string]struct {
 		config      *Config
@@ -104,7 +99,9 @@ func TestStartWithArgs(t *testing.T) {
 				CSRInitialRetrialInterval: time.Millisecond,
 				CSRMaxRetries:             3,
 				CSRGracePeriodPercentage:  50,
-				PlatformConfig:            generalPcConfig,
+				RootCertFile:              "ca_file",
+				KeyFile:                   "pkey",
+				CertChainFile:             "cert_file",
 				LoggingOptions:            log.NewOptions(),
 			},
 			pc:          mockpc.FakeClient{nil, "", "service1", "", []byte{}, "", true},

security/pkg/platform/aws.go

@@ -51,29 +51,25 @@ C1haGgSI/A1uZUKs/Zfnph0oEI0/hu1IIJ/SKBDtN5lvmZ/IzbOPIJWirlsllQIQ
 -----END CERTIFICATE-----`
 )
 
-// AwsConfig ...
-type AwsConfig struct {
-	// Root CA cert file to validate the gRPC service in CA.
-	RootCACertFile string
-}
-
 // AwsClientImpl is the implementation of AWS metadata client.
 type AwsClientImpl struct {
-	config AwsConfig
+	// Root CA cert file to validate the gRPC service in CA.
+	rootCertFile string
+
 	client *ec2metadata.EC2Metadata
 }
 
 // NewAwsClientImpl creates a new AwsClientImpl.
-func NewAwsClientImpl(config AwsConfig) *AwsClientImpl {
+func NewAwsClientImpl(rootCert string) *AwsClientImpl {
 	return &AwsClientImpl{
-		config: config,
-		client: ec2metadata.New(session.Must(session.NewSession())),
+		rootCertFile: rootCert,
+		client:       ec2metadata.New(session.Must(session.NewSession())),
 	}
 }
 
 // GetDialOptions returns the GRPC dial options to connect to the CA.
 func (ci *AwsClientImpl) GetDialOptions() ([]grpc.DialOption, error) {
-	creds, err := credentials.NewClientTLSFromFile(ci.config.RootCACertFile, "")
+	creds, err := credentials.NewClientTLSFromFile(ci.rootCertFile, "")
 	if err != nil {
 		return nil, err
 	}

security/pkg/platform/aws_test.go

@@ -82,7 +82,7 @@ func TestIsProperPlatform(t *testing.T) {
 }
 
 func TestNewAwsClientImpl(t *testing.T) {
-	client := NewAwsClientImpl(AwsConfig{})
+	client := NewAwsClientImpl("")
 	if client == nil {
 		t.Errorf("NewAwsClientImpl should not return nil")
 	}
@@ -263,27 +263,19 @@ func TestAwsGetDialOptions(t *testing.T) {
 
 	testCases := map[string]struct {
 		expectedErr     string
-		cfg             *ClientConfig
+		rootCertFile    string
 		expectedOptions []grpc.DialOption
 	}{
 		"Good DialOptions": {
-			expectedErr: "",
-			cfg: &ClientConfig{
-				AwsConfig: AwsConfig{
-					RootCACertFile: "testdata/cert-chain-good.pem",
-				},
-			},
+			expectedErr:  "",
+			rootCertFile: "testdata/cert-chain-good.pem",
 			expectedOptions: []grpc.DialOption{
 				grpc.WithTransportCredentials(creds),
 			},
 		},
 		"Bad DialOptions": {
-			expectedErr: "open testdata/cert-chain-good_not_exist.pem: no such file or directory",
-			cfg: &ClientConfig{
-				AwsConfig: AwsConfig{
-					RootCACertFile: "testdata/cert-chain-good_not_exist.pem",
-				},
-			},
+			expectedErr:  "open testdata/cert-chain-good_not_exist.pem: no such file or directory",
+			rootCertFile: "testdata/cert-chain-good_not_exist.pem",
 			expectedOptions: []grpc.DialOption{
 				grpc.WithTransportCredentials(creds),
 			},
@@ -292,8 +284,8 @@ func TestAwsGetDialOptions(t *testing.T) {
 
 	for id, c := range testCases {
 		awsc := &AwsClientImpl{
-			config: c.cfg.AwsConfig,
-			client: ec2metadata.New(unit.Session, &aws.Config{}),
+			rootCertFile: c.rootCertFile,
+			client:       ec2metadata.New(unit.Session, &aws.Config{}),
 		}
 
 		options, err := awsc.GetDialOptions()

security/pkg/platform/client.go

@@ -20,15 +20,6 @@ import (
 	"google.golang.org/grpc"
 )
 
-// ClientConfig consists of the platform client configuration.
-type ClientConfig struct {
-	OnPremConfig OnPremConfig
-
-	GcpConfig GcpConfig
-
-	AwsConfig AwsConfig
-}
-
 // Client is the interface for implementing the client to access platform metadata.
 type Client interface {
 	GetDialOptions() ([]grpc.DialOption, error)
@@ -44,14 +35,14 @@ type Client interface {
 }
 
 // NewClient is the function to create implementations of the platform metadata client.
-func NewClient(platform string, config ClientConfig, caAddr string) (Client, error) {
+func NewClient(platform, rootCertFile, keyFile, certChainFile, caAddr string) (Client, error) {
 	switch platform {
 	case "onprem":
-		return NewOnPremClientImpl(config.OnPremConfig), nil
+		return NewOnPremClientImpl(rootCertFile, keyFile, certChainFile), nil
 	case "gcp":
-		return NewGcpClientImpl(config.GcpConfig), nil
+		return NewGcpClientImpl(rootCertFile, caAddr), nil
 	case "aws":
-		return NewAwsClientImpl(config.AwsConfig), nil
+		return NewAwsClientImpl(rootCertFile), nil
 	default:
 		return nil, fmt.Errorf("invalid env %s specified", platform)
 	}

security/pkg/platform/client_test.go

@@ -20,44 +20,40 @@ import (
 
 func TestNewClient(t *testing.T) {
 	testCases := map[string]struct {
-		platform     string
-		cfg          ClientConfig
-		caAddr       string
-		expectedErr  string
-		expectedType string
+		platform      string
+		rootCertFile  string
+		keyFile       string
+		certChainFile string
+		caAddr        string
+		expectedErr   string
+		expectedType  string
 	}{
 		"onprem test": {
-			platform: "onprem",
-			cfg: ClientConfig{
-				OnPremConfig: OnPremConfig{
-					RootCACertFile: "testdata/cert-chain-good.pem",
-				},
-			},
-			caAddr:       "localhost",
-			expectedErr:  "",
-			expectedType: "onprem",
+			platform:      "onprem",
+			rootCertFile:  "testdata/root-cert-good.pem",
+			keyFile:       "testdata/key-good.pem",
+			certChainFile: "testdata/cert-chain-good.pem",
+			caAddr:        "localhost",
+			expectedErr:   "",
+			expectedType:  "onprem",
 		},
 		"gcp test": {
-			platform: "gcp",
-			cfg: ClientConfig{
-				GcpConfig: GcpConfig{
-					RootCACertFile: "testdata/cert-chain-good.pem",
-				},
-			},
-			caAddr:       "localhost",
-			expectedErr:  "",
-			expectedType: "gcp",
+			platform:      "gcp",
+			rootCertFile:  "testdata/root-cert-good.pem",
+			keyFile:       "testdata/key-good.pem",
+			certChainFile: "testdata/cert-chain-good.pem",
+			caAddr:        "localhost",
+			expectedErr:   "",
+			expectedType:  "gcp",
 		},
 		"aws test": {
-			platform: "aws",
-			cfg: ClientConfig{
-				AwsConfig: AwsConfig{
-					RootCACertFile: "testdata/cert-chain-good.pem",
-				},
-			},
-			caAddr:       "localhost",
-			expectedErr:  "",
-			expectedType: "aws",
+			platform:      "aws",
+			rootCertFile:  "testdata/root-cert-good.pem",
+			keyFile:       "testdata/key-good.pem",
+			certChainFile: "testdata/cert-chain-good.pem",
+			caAddr:        "localhost",
+			expectedErr:   "",
+			expectedType:  "aws",
 		},
 		"invalid test": {
 			platform:    "invalid",
@@ -66,7 +62,8 @@ func TestNewClient(t *testing.T) {
 	}
 
 	for id, tc := range testCases {
-		client, err := NewClient(tc.platform, tc.cfg, tc.caAddr)
+		client, err := NewClient(
+			tc.platform, tc.rootCertFile, tc.keyFile, tc.certChainFile, tc.caAddr)
 		if len(tc.expectedErr) > 0 {
 			if err == nil {
 				t.Errorf("%s: Succeeded. Error expected: %v", id, err)