Change remote gRPC configuration parameters

Use the tlscfg package for the TLS configuration and change the
cli option prefix from `grpc-storage-plugin` to `grpc-storage`.

plugin/storage/grpc/config/config.go

@@ -25,22 +25,22 @@ import (
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-plugin"
 	"github.com/opentracing/opentracing-go"
+	"go.uber.org/zap"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
 
+	"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
 	"github.com/jaegertracing/jaeger/plugin/storage/grpc/shared"
 )
 
 // Configuration describes the options to customize the storage behavior.
 type Configuration struct {
-	PluginBinary             string        `yaml:"binary" mapstructure:"binary"`
-	PluginConfigurationFile  string        `yaml:"configuration-file" mapstructure:"configuration_file"`
-	PluginLogLevel           string        `yaml:"log-level" mapstructure:"log_level"`
-	RemoteServerAddr         string        `yaml:"server" mapstructure:"server"`
-	RemoteTLS                bool          `yaml:"tls" mapstructure:"tls"`
-	RemoteCAFile             string        `yaml:"cafile" mapstructure:"cafile"`
-	RemoteServerHostOverride string        `yaml:"server-host-override" mapstructure:"server-host-override"`
-	RemoteConnectTimeout     time.Duration `yaml:"connection-timeout" mapstructure:"connection-timeout"`
+	PluginBinary            string `yaml:"binary" mapstructure:"binary"`
+	PluginConfigurationFile string `yaml:"configuration-file" mapstructure:"configuration_file"`
+	PluginLogLevel          string `yaml:"log-level" mapstructure:"log_level"`
+	RemoteServerAddr        string `yaml:"server" mapstructure:"server"`
+	RemoteTLS               tlscfg.Options
+	RemoteConnectTimeout    time.Duration `yaml:"connection-timeout" mapstructure:"connection-timeout"`
 }
 
 // ClientPluginServices defines services plugin can expose and its capabilities
@@ -51,33 +51,39 @@ type ClientPluginServices struct {
 
 // PluginBuilder is used to create storage plugins. Implemented by Configuration.
 type PluginBuilder interface {
-	Build() (*ClientPluginServices, error)
+	Build(logger *zap.Logger) (*ClientPluginServices, error)
+	Close() error
 }
 
 // Build instantiates a PluginServices
-func (c *Configuration) Build() (*ClientPluginServices, error) {
+func (c *Configuration) Build(logger *zap.Logger) (*ClientPluginServices, error) {
 	if c.PluginBinary != "" {
 		return c.buildPlugin()
 	} else {
-		return c.BuildRemote()
+		return c.buildRemote(logger)
 	}
 }
 
-func (c *Configuration) BuildRemote() (*ClientPluginServices, error) {
+func (c *Configuration) Close() error {
+	if c.PluginBinary == "" {
+		return c.RemoteTLS.Close()
+	}
+	return nil
+}
+
+func (c *Configuration) buildRemote(logger *zap.Logger) (*ClientPluginServices, error) {
 	opts := []grpc.DialOption{
 		grpc.WithUnaryInterceptor(otgrpc.OpenTracingClientInterceptor(opentracing.GlobalTracer())),
 		grpc.WithStreamInterceptor(otgrpc.OpenTracingStreamClientInterceptor(opentracing.GlobalTracer())),
 		grpc.WithBlock(),
 	}
 	var err error
-	if c.RemoteTLS {
-		if c.RemoteCAFile == "" {
-			return nil, fmt.Errorf("ca file is required with TLS")
-		}
-		creds, err := credentials.NewClientTLSFromFile(c.RemoteCAFile, c.RemoteServerHostOverride)
+	if c.RemoteTLS.Enabled {
+		tlsCfg, err := c.RemoteTLS.Config(logger)
 		if err != nil {
-			return nil, fmt.Errorf("failed to create TLS credentials %w", err)
+			return nil, err
 		}
+		creds := credentials.NewTLS(tlsCfg)
 		opts = append(opts, grpc.WithTransportCredentials(creds))
 	} else {
 		opts = append(opts, grpc.WithInsecure())

plugin/storage/grpc/factory.go

@@ -68,7 +68,7 @@ func (f *Factory) InitFromOptions(opts Options) {
 func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error {
 	f.metricsFactory, f.logger = metricsFactory, logger
 
-	services, err := f.builder.Build()
+	services, err := f.builder.Build(logger)
 	if err != nil {
 		return fmt.Errorf("grpc-plugin builder failed to create a store: %w", err)
 	}
@@ -124,3 +124,8 @@ func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) {
 	}
 	return f.archiveStore.ArchiveSpanWriter(), nil
 }
+
+// Close closes the resources held by the factory
+func (f *Factory) Close() error {
+	return f.builder.Close()
+}

plugin/storage/grpc/factory_test.go

@@ -42,7 +42,7 @@ type mockPluginBuilder struct {
 	err    error
 }
 
-func (b *mockPluginBuilder) Build() (*grpcConfig.ClientPluginServices, error) {
+func (b *mockPluginBuilder) Build(logger *zap.Logger) (*grpcConfig.ClientPluginServices, error) {
 	if b.err != nil {
 		return nil, b.err
 	}
@@ -60,6 +60,10 @@ func (b *mockPluginBuilder) Build() (*grpcConfig.ClientPluginServices, error) {
 	return services, nil
 }
 
+func (b *mockPluginBuilder) Close() error {
+	return nil
+}
+
 type mockPlugin struct {
 	spanReader       spanstore.Reader
 	spanWriter       spanstore.Writer

plugin/storage/grpc/options.go

@@ -20,18 +20,17 @@ import (
 
 	"github.com/spf13/viper"
 
+	"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
 	"github.com/jaegertracing/jaeger/plugin/storage/grpc/config"
 )
 
 const (
 	pluginBinary             = "grpc-storage-plugin.binary"
 	pluginConfigurationFile  = "grpc-storage-plugin.configuration-file"
 	pluginLogLevel           = "grpc-storage-plugin.log-level"
-	pluginServer             = "grpc-storage-plugin.server"
-	pluginTLS                = "grpc-storage-plugin.tls"
-	pluginCAFile             = "grpc-storage-plugin.ca-file"
-	pluginServerHostOverride = "grpc-storage-plugin.server-host-override"
-	pluginConnectionTimeout  = "grpc-storage-plugin.connection-timeout"
+	remotePrefix             = "grpc-storage"
+	pluginServer             = remotePrefix + ".server"
+	pluginConnectionTimeout  = remotePrefix + ".connection-timeout"
 	defaultPluginLogLevel    = "warn"
 	defaultConnectionTimeout = time.Duration(5 * time.Second)
 )
@@ -42,28 +41,32 @@ type Options struct {
 	Configuration config.Configuration `mapstructure:",squash"`
 }
 
+func tlsFlagsConfig() tlscfg.ClientFlagsConfig {
+	return tlscfg.ClientFlagsConfig{
+		Prefix: remotePrefix,
+	}
+}
+
 // AddFlags adds flags for Options
 func (opt *Options) AddFlags(flagSet *flag.FlagSet) {
+	var tlsFlagsConfig = tlsFlagsConfig()
+	tlsFlagsConfig.AddFlags(flagSet)
+
 	flagSet.String(pluginBinary, "", "The location of the plugin binary")
 	flagSet.String(pluginConfigurationFile, "", "A path pointing to the plugin's configuration file, made available to the plugin with the --config arg")
 	flagSet.String(pluginLogLevel, defaultPluginLogLevel, "Set the log level of the plugin's logger")
 	flagSet.String(pluginServer, "", "The server address for the remote gRPC server")
-	flagSet.Bool(pluginTLS, false, "Whether to use TLS for the remote connection")
-	flagSet.String(pluginCAFile, "", "The CA file for the remote connection")
-	flagSet.String(pluginServerHostOverride, "", "The server host override for the remote connection")
 	flagSet.Duration(pluginConnectionTimeout, defaultConnectionTimeout, "The connection timeout for connecting to the remote server")
 
 }
 
 // InitFromViper initializes Options with properties from viper
 func (opt *Options) InitFromViper(v *viper.Viper) {
+	var tlsFlagsConfig = tlsFlagsConfig()
 	opt.Configuration.PluginBinary = v.GetString(pluginBinary)
 	opt.Configuration.PluginConfigurationFile = v.GetString(pluginConfigurationFile)
 	opt.Configuration.PluginLogLevel = v.GetString(pluginLogLevel)
 	opt.Configuration.RemoteServerAddr = v.GetString(pluginServer)
-	opt.Configuration.RemoteTLS = v.GetBool(pluginTLS)
-	opt.Configuration.RemoteCAFile = v.GetString(pluginCAFile)
-	opt.Configuration.RemoteServerHostOverride = v.GetString(pluginServerHostOverride)
+	opt.Configuration.RemoteTLS = tlsFlagsConfig.InitFromViper(v)
 	opt.Configuration.RemoteConnectTimeout = v.GetDuration(pluginConnectionTimeout)
-
 }

plugin/storage/grpc/options_test.go

@@ -43,19 +43,15 @@ func TestRemoteOptionsWithFlags(t *testing.T) {
 	opts := &Options{}
 	v, command := config.Viperize(opts.AddFlags)
 	err := command.ParseFlags([]string{
-		"--grpc-storage-plugin.server=localhost:2001",
-		"--grpc-storage-plugin.tls=true",
-		"--grpc-storage-plugin.ca-file=cafile",
-		"--grpc-storage-plugin.server-host-override=example.com",
-		"--grpc-storage-plugin.connection-timeout=60s",
+		"--grpc-storage.server=localhost:2001",
+		"--grpc-storage.tls.enabled=true",
+		"--grpc-storage.connection-timeout=60s",
 	})
 	assert.NoError(t, err)
 	opts.InitFromViper(v)
 
 	assert.Equal(t, opts.Configuration.PluginBinary, "")
 	assert.Equal(t, opts.Configuration.RemoteServerAddr, "localhost:2001")
-	assert.Equal(t, opts.Configuration.RemoteTLS, true)
-	assert.Equal(t, opts.Configuration.RemoteCAFile, "cafile")
-	assert.Equal(t, opts.Configuration.RemoteServerHostOverride, "example.com")
+	assert.Equal(t, opts.Configuration.RemoteTLS.Enabled, true)
 	assert.Equal(t, opts.Configuration.RemoteConnectTimeout, 60*time.Second)
 }