certprovider: API tweaks. (grpc#3987)

credentials/tls/certprovider/meshca/builder.go

@@ -65,7 +65,7 @@ type refCountedCC struct {
 }
 
 // pluginBuilder is an implementation of the certprovider.Builder interface,
-// which build certificate provider instances which get certificates signed from
+// which builds certificate provider instances to get certificates signed from
 // the MeshCA.
 type pluginBuilder struct {
 	// A collection of ClientConns to the MeshCA server along with a reference
@@ -75,22 +75,30 @@ type pluginBuilder struct {
 	clients map[ccMapKey]*refCountedCC
 }
 
-// Build returns a MeshCA certificate provider for the passed in configuration
-// and options.
+// ParseConfig parses the configuration to be passed to the MeshCA plugin
+// implementation. Expects the config to be a json.RawMessage which contains a
+// serialized JSON representation of the meshca_experimental.GoogleMeshCaConfig
+// proto message.
 //
-// This builder takes care of sharing the ClientConn to the MeshCA server among
+// Takes care of sharing the ClientConn to the MeshCA server among
 // different plugin instantiations.
-func (b *pluginBuilder) Build(c certprovider.StableConfig, opts certprovider.Options) certprovider.Provider {
-	cfg, ok := c.(*pluginConfig)
+func (b *pluginBuilder) ParseConfig(c interface{}) (*certprovider.BuildableConfig, error) {
+	data, ok := c.(json.RawMessage)
 	if !ok {
-		// This is not expected when passing config returned by ParseConfig().
-		// This could indicate a bug in the certprovider.Store implementation or
-		// in cases where the user is directly using these APIs, could be a user
-		// error.
-		logger.Errorf("unsupported config type: %T", c)
-		return nil
+		return nil, fmt.Errorf("meshca: unsupported config type: %T", c)
+	}
+	cfg, err := pluginConfigFromJSON(data)
+	if err != nil {
+		return nil, err
 	}
+	return certprovider.NewBuildableConfig(pluginName, cfg.canonical(), func(opts certprovider.BuildOptions) certprovider.Provider {
+		return b.buildFromConfig(cfg, opts)
+	}), nil
+}
 
+// buildFromConfig builds a certificate provider instance for the given config
+// and options. Provider instances are shared wherever possible.
+func (b *pluginBuilder) buildFromConfig(cfg *pluginConfig, opts certprovider.BuildOptions) certprovider.Provider {
 	b.mu.Lock()
 	defer b.mu.Unlock()
 
@@ -151,18 +159,6 @@ func (b *pluginBuilder) Build(c certprovider.StableConfig, opts certprovider.Opt
 	return p
 }
 
-// ParseConfig parses the configuration to be passed to the MeshCA plugin
-// implementation. Expects the config to be a json.RawMessage which contains a
-// serialized JSON representation of the meshca_experimental.GoogleMeshCaConfig
-// proto message.
-func (b *pluginBuilder) ParseConfig(c interface{}) (certprovider.StableConfig, error) {
-	data, ok := c.(json.RawMessage)
-	if !ok {
-		return nil, fmt.Errorf("meshca: unsupported config type: %T", c)
-	}
-	return pluginConfigFromJSON(data)
-}
-
 // Name returns the MeshCA plugin name.
 func (b *pluginBuilder) Name() string {
 	return pluginName

credentials/tls/certprovider/meshca/builder_test.go

@@ -71,7 +71,7 @@ func (s) TestBuildSameConfig(t *testing.T) {
 	// invocations of Build().
 	inputConfig := makeJSONConfig(t, goodConfigFullySpecified)
 	builder := newPluginBuilder()
-	stableConfig, err := builder.ParseConfig(inputConfig)
+	buildableConfig, err := builder.ParseConfig(inputConfig)
 	if err != nil {
 		t.Fatalf("builder.ParseConfig(%q) failed: %v", inputConfig, err)
 	}
@@ -80,9 +80,9 @@ func (s) TestBuildSameConfig(t *testing.T) {
 	// end up sharing the same ClientConn.
 	providers := []certprovider.Provider{}
 	for i := 0; i < cnt; i++ {
-		p := builder.Build(stableConfig, certprovider.Options{})
-		if p == nil {
-			t.Fatalf("builder.Build(%s) failed: %v", string(stableConfig.Canonical()), err)
+		p, err := buildableConfig.Build(certprovider.BuildOptions{})
+		if err != nil {
+			t.Fatalf("Build(%+v) failed: %v", buildableConfig, err)
 		}
 		providers = append(providers, p)
 	}
@@ -146,14 +146,14 @@ func (s) TestBuildDifferentConfig(t *testing.T) {
 		cfg := proto.Clone(goodConfigFullySpecified).(*configpb.GoogleMeshCaConfig)
 		cfg.Server.GrpcServices[0].GetGoogleGrpc().TargetUri = fmt.Sprintf("test-mesh-ca:%d", i)
 		inputConfig := makeJSONConfig(t, cfg)
-		stableConfig, err := builder.ParseConfig(inputConfig)
+		buildableConfig, err := builder.ParseConfig(inputConfig)
 		if err != nil {
 			t.Fatalf("builder.ParseConfig(%q) failed: %v", inputConfig, err)
 		}
 
-		p := builder.Build(stableConfig, certprovider.Options{})
-		if p == nil {
-			t.Fatalf("builder.Build(%s) failed: %v", string(stableConfig.Canonical()), err)
+		p, err := buildableConfig.Build(certprovider.BuildOptions{})
+		if err != nil {
+			t.Fatalf("Build(%+v) failed: %v", buildableConfig, err)
 		}
 		providers = append(providers, p)
 	}

credentials/tls/certprovider/meshca/config.go

@@ -161,7 +161,7 @@ func pluginConfigFromJSON(data json.RawMessage) (*pluginConfig, error) {
 	return pc, nil
 }
 
-func (pc *pluginConfig) Canonical() []byte {
+func (pc *pluginConfig) canonical() []byte {
 	return []byte(fmt.Sprintf("%s:%s:%s:%s:%s:%s:%d:%s", pc.serverURI, pc.stsOpts, pc.callTimeout, pc.certLifetime, pc.certGraceTime, pc.keyType, pc.keySize, pc.location))
 }
 

credentials/tls/certprovider/meshca/config_test.go

@@ -160,13 +160,13 @@ func (s) TestParseConfigSuccessFullySpecified(t *testing.T) {
 	inputConfig := makeJSONConfig(t, goodConfigFullySpecified)
 	wantConfig := "test-meshca:http://test-sts:test-resource:test-audience:test-scope:test-requested-token-type:test-subject-token-path:test-subject-token-type:test-actor-token-path:test-actor-token-type:10s:24h0m0s:12h0m0s:RSA:2048:us-west1-b"
 
-	builder := newPluginBuilder()
-	gotConfig, err := builder.ParseConfig(inputConfig)
+	cfg, err := pluginConfigFromJSON(inputConfig)
 	if err != nil {
-		t.Fatalf("builder.ParseConfig(%q) failed: %v", inputConfig, err)
+		t.Fatalf("pluginConfigFromJSON(%q) failed: %v", inputConfig, err)
 	}
-	if diff := cmp.Diff(wantConfig, string(gotConfig.Canonical())); diff != "" {
-		t.Errorf("builder.ParseConfig(%q) returned config does not match expected (-want +got):\n%s", inputConfig, diff)
+	gotConfig := cfg.canonical()
+	if diff := cmp.Diff(wantConfig, string(gotConfig)); diff != "" {
+		t.Errorf("pluginConfigFromJSON(%q) returned config does not match expected (-want +got):\n%s", inputConfig, diff)
 	}
 }
 
@@ -248,13 +248,12 @@ func (s) TestParseConfigSuccessWithDefaults(t *testing.T) {
 		errCh <- nil
 	}()
 
-	builder := newPluginBuilder()
-	gotConfig, err := builder.ParseConfig(inputConfig)
+	cfg, err := pluginConfigFromJSON(inputConfig)
 	if err != nil {
-		t.Fatalf("builder.ParseConfig(%q) failed: %v", inputConfig, err)
-
+		t.Fatalf("pluginConfigFromJSON(%q) failed: %v", inputConfig, err)
 	}
-	if diff := cmp.Diff(wantConfig, string(gotConfig.Canonical())); diff != "" {
+	gotConfig := cfg.canonical()
+	if diff := cmp.Diff(wantConfig, string(gotConfig)); diff != "" {
 		t.Errorf("builder.ParseConfig(%q) returned config does not match expected (-want +got):\n%s", inputConfig, diff)
 	}
 
@@ -268,14 +267,9 @@ func (s) TestParseConfigSuccessWithDefaults(t *testing.T) {
 func (s) TestParseConfigFailureCases(t *testing.T) {
 	tests := []struct {
 		desc        string
-		inputConfig interface{}
+		inputConfig json.RawMessage
 		wantErr     string
 	}{
-		{
-			desc:        "bad config type",
-			inputConfig: struct{ foo string }{foo: "bar"},
-			wantErr:     "unsupported config type",
-		},
 		{
 			desc:        "invalid JSON",
 			inputConfig: json.RawMessage(`bad bad json`),
@@ -396,10 +390,9 @@ func (s) TestParseConfigFailureCases(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
-			builder := newPluginBuilder()
-			sc, err := builder.ParseConfig(test.inputConfig)
+			cfg, err := pluginConfigFromJSON(test.inputConfig)
 			if err == nil {
-				t.Fatalf("builder.ParseConfig(%q) = %v, expected to return error (%v)", test.inputConfig, string(sc.Canonical()), test.wantErr)
+				t.Fatalf("pluginConfigFromJSON(%q) = %v, expected to return error (%v)", test.inputConfig, string(cfg.canonical()), test.wantErr)
 
 			}
 			if !strings.Contains(err.Error(), test.wantErr) {

credentials/tls/certprovider/meshca/plugin.go

@@ -65,20 +65,20 @@ type distributor interface {
 type providerPlugin struct {
 	distributor // Holds the key material.
 	cancel      context.CancelFunc
-	cc          *grpc.ClientConn        // Connection to MeshCA server.
-	cfg         *pluginConfig           // Plugin configuration.
-	opts        certprovider.Options    // Key material options.
-	logger      *grpclog.PrefixLogger   // Plugin instance specific prefix.
-	backoff     func(int) time.Duration // Exponential backoff.
-	doneFunc    func()                  // Notify the builder when done.
+	cc          *grpc.ClientConn          // Connection to MeshCA server.
+	cfg         *pluginConfig             // Plugin configuration.
+	opts        certprovider.BuildOptions // Key material options.
+	logger      *grpclog.PrefixLogger     // Plugin instance specific prefix.
+	backoff     func(int) time.Duration   // Exponential backoff.
+	doneFunc    func()                    // Notify the builder when done.
 }
 
 // providerParams wraps params passed to the provider plugin at creation time.
 type providerParams struct {
 	// This ClientConn to the MeshCA server is owned by the builder.
 	cc       *grpc.ClientConn
 	cfg      *pluginConfig
-	opts     certprovider.Options
+	opts     certprovider.BuildOptions
 	backoff  func(int) time.Duration
 	doneFunc func()
 }

credentials/tls/certprovider/meshca/plugin_test.go

@@ -297,14 +297,15 @@ func (s) TestCreateCertificate(t *testing.T) {
 	e, addr, cancel := setup(t, opts{})
 	defer cancel()
 
-	// Set the MeshCA targetURI in the plugin configuration to point to our fake
-	// MeshCA.
+	// Set the MeshCA targetURI to point to our fake MeshCA.
 	cfg := proto.Clone(goodConfigFullySpecified).(*configpb.GoogleMeshCaConfig)
 	cfg.Server.GrpcServices[0].GetGoogleGrpc().TargetUri = addr
 	inputConfig := makeJSONConfig(t, cfg)
-	prov, err := certprovider.GetProvider(pluginName, inputConfig, certprovider.Options{})
+
+	// Lookup MeshCA plugin builder, parse config and start the plugin.
+	prov, err := certprovider.GetProvider(pluginName, inputConfig, certprovider.BuildOptions{})
 	if err != nil {
-		t.Fatalf("certprovider.GetProvider(%s, %s) failed: %v", pluginName, cfg, err)
+		t.Fatalf("GetProvider(%s, %s) failed: %v", pluginName, string(inputConfig), err)
 	}
 	defer prov.Close()
 
@@ -339,14 +340,15 @@ func (s) TestCreateCertificateWithBackoff(t *testing.T) {
 	e, addr, cancel := setup(t, opts{withbackoff: true})
 	defer cancel()
 
-	// Set the MeshCA targetURI in the plugin configuration to point to our fake
-	// MeshCA.
+	// Set the MeshCA targetURI to point to our fake MeshCA.
 	cfg := proto.Clone(goodConfigFullySpecified).(*configpb.GoogleMeshCaConfig)
 	cfg.Server.GrpcServices[0].GetGoogleGrpc().TargetUri = addr
 	inputConfig := makeJSONConfig(t, cfg)
-	prov, err := certprovider.GetProvider(pluginName, inputConfig, certprovider.Options{})
+
+	// Lookup MeshCA plugin builder, parse config and start the plugin.
+	prov, err := certprovider.GetProvider(pluginName, inputConfig, certprovider.BuildOptions{})
 	if err != nil {
-		t.Fatalf("certprovider.GetProvider(%s, %s) failed: %v", pluginName, cfg, err)
+		t.Fatalf("GetProvider(%s, %s) failed: %v", pluginName, string(inputConfig), err)
 	}
 	defer prov.Close()
 
@@ -394,14 +396,15 @@ func (s) TestCreateCertificateWithRefresh(t *testing.T) {
 	e, addr, cancel := setup(t, opts{withShortLife: true})
 	defer cancel()
 
-	// Set the MeshCA targetURI in the plugin configuration to point to our fake
-	// MeshCA.
+	// Set the MeshCA targetURI to point to our fake MeshCA.
 	cfg := proto.Clone(goodConfigFullySpecified).(*configpb.GoogleMeshCaConfig)
 	cfg.Server.GrpcServices[0].GetGoogleGrpc().TargetUri = addr
 	inputConfig := makeJSONConfig(t, cfg)
-	prov, err := certprovider.GetProvider(pluginName, inputConfig, certprovider.Options{})
+
+	// Lookup MeshCA plugin builder, parse config and start the plugin.
+	prov, err := certprovider.GetProvider(pluginName, inputConfig, certprovider.BuildOptions{})
 	if err != nil {
-		t.Fatalf("certprovider.GetProvider(%s, %s) failed: %v", pluginName, cfg, err)
+		t.Fatalf("GetProvider(%s, %s) failed: %v", pluginName, string(inputConfig), err)
 	}
 	defer prov.Close()
 

credentials/tls/certprovider/provider.go

@@ -64,28 +64,14 @@ func getBuilder(name string) Builder {
 
 // Builder creates a Provider.
 type Builder interface {
-	// Build creates a new Provider and initializes it with the given config and
-	// options combination.
-	Build(StableConfig, Options) Provider
-
-	// ParseConfig converts config input in a format specific to individual
-	// implementations and returns an implementation of the StableConfig
-	// interface.
-	// Equivalent configurations must return StableConfig types whose
-	// Canonical() method returns the same output.
-	ParseConfig(interface{}) (StableConfig, error)
+	// ParseConfig parses the given config, which is in a format specific to individual
+	// implementations, and returns a BuildableConfig on success.
+	ParseConfig(interface{}) (*BuildableConfig, error)
 
 	// Name returns the name of providers built by this builder.
 	Name() string
 }
 
-// StableConfig wraps the method to return a stable Provider configuration.
-type StableConfig interface {
-	// Canonical returns Provider config as an arbitrary byte slice.
-	// Equivalent configurations must return the same output.
-	Canonical() []byte
-}
-
 // Provider makes it possible to keep channel credential implementations up to
 // date with secrets that they rely on to secure communications on the
 // underlying channel.
@@ -110,8 +96,8 @@ type KeyMaterial struct {
 	Roots *x509.CertPool
 }
 
-// Options contains configuration knobs passed to a Provider at creation time.
-type Options struct {
+// BuildOptions contains parameters passed to a Provider at build time.
+type BuildOptions struct {
 	// CertName holds the certificate name, whose key material is of interest to
 	// the caller.
 	CertName string