certprovider-api: update api to include certificate name

credentials/tls/certprovider/distributor.go

@@ -42,7 +42,11 @@ type Distributor struct {
 	km   *KeyMaterial
 	pErr error
 
-	ready  *grpcsync.Event
+	// ready channel to unblock KeyMaterial() invocations blocked on
+	// availability of key material.
+	ready *grpcsync.Event
+	// done channel to notify provider implementations and unblock any
+	// KeyMaterial() calls, once the Distributor is closed.
 	closed *grpcsync.Event
 }
 
@@ -75,7 +79,7 @@ func (d *Distributor) Set(km *KeyMaterial, err error) {
 	d.mu.Unlock()
 }
 
-// KeyMaterial returns the most recent key material provided to the distributor.
+// KeyMaterial returns the most recent key material provided to the Distributor.
 // If no key material was provided at the time of this call, it will block until
 // the deadline on the context expires or fresh key material arrives.
 func (d *Distributor) KeyMaterial(ctx context.Context) (*KeyMaterial, error) {

credentials/tls/certprovider/distributor_test.go

@@ -1,3 +1,5 @@
+// +build go1.13
+
 /*
  *
  * Copyright 2020 gRPC authors.
@@ -22,7 +24,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"reflect"
 	"sync"
 	"testing"
 	"time"
@@ -36,124 +37,121 @@ func (s) TestDistributor(t *testing.T) {
 	dist := NewDistributor()
 
 	// Read cert/key files from testdata.
-	km, err := loadKeyMaterials()
-	if err != nil {
-		t.Fatal(err)
-	}
+	km := loadKeyMaterials(t, "x509/server1_cert.pem", "x509/server1_key.pem", "x509/client_ca_cert.pem")
+
 	// wantKM1 has both local and root certs.
 	wantKM1 := *km
 	// wantKM2 has only local certs. Roots are nil-ed out.
 	wantKM2 := *km
 	wantKM2.Roots = nil
 
-	// Create a goroutines which work in lockstep with the rest of the test.
-	// This goroutine reads the key material from the distributor while the rest
-	// of the test sets it.
-	var wg sync.WaitGroup
-	wg.Add(1)
-	errCh := make(chan error)
-	proceedCh := make(chan struct{})
-	go func() {
-		defer wg.Done()
-
-		// The first call to KeyMaterial() should timeout because no key
-		// material has been set on the distributor as yet.
-		ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout/2)
-		defer cancel()
-		if _, err := dist.KeyMaterial(ctx); err != context.DeadlineExceeded {
-			errCh <- err
-			return
-		}
-		proceedCh <- struct{}{}
-
-		// This call to KeyMaterial() should return the key material with both
-		// the local certs and the root certs.
-		ctx, cancel = context.WithTimeout(context.Background(), defaultTestTimeout)
-		defer cancel()
-		gotKM, err := dist.KeyMaterial(ctx)
-		if err != nil {
-			errCh <- err
-			return
-		}
-		if !reflect.DeepEqual(gotKM, &wantKM1) {
-			errCh <- fmt.Errorf("provider.KeyMaterial() = %+v, want %+v", gotKM, wantKM1)
-		}
-		proceedCh <- struct{}{}
-
-		// This call to KeyMaterial() should eventually return key material with
-		// only the local certs.
-		ctx, cancel = context.WithTimeout(context.Background(), defaultTestTimeout)
-		defer cancel()
-		for {
-			gotKM, err := dist.KeyMaterial(ctx)
-			if err != nil {
-				errCh <- err
-				return
-			}
-			if reflect.DeepEqual(gotKM, &wantKM2) {
-				break
-			}
-		}
-		proceedCh <- struct{}{}
-
-		// This call to KeyMaterial() should return nil key material and a
-		// non-nil error.
-		ctx, cancel = context.WithTimeout(context.Background(), defaultTestTimeout)
-		defer cancel()
-		for {
-			gotKM, err := dist.KeyMaterial(ctx)
-			if gotKM == nil && err == errProviderTestInternal {
-				break
-			}
-			if err != nil {
-				// If we have gotten any error other than
-				// errProviderTestInternal, we should bail out.
-				errCh <- err
-				return
-			}
-		}
-		proceedCh <- struct{}{}
-
-		// This call to KeyMaterial() should eventually return errProviderClosed
-		// error.
-		ctx, cancel = context.WithTimeout(context.Background(), defaultTestTimeout)
-		defer cancel()
-		for {
-			if _, err := dist.KeyMaterial(ctx); err == errProviderClosed {
-				break
-			}
-			time.Sleep(100 * time.Millisecond)
-		}
-	}()
-
-	waitAndDo(t, proceedCh, errCh, func() {
-		dist.Set(&wantKM1, nil)
-	})
+	// The first call to KeyMaterial() should timeout because no key
+	// material has been set on the distributor as yet.
+	ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
+	defer cancel()
+	if err := readAndVerifyKeyMaterial(ctx, dist, nil); !errors.Is(err, context.DeadlineExceeded) {
+		t.Fatal(err)
+	}
 
-	waitAndDo(t, proceedCh, errCh, func() {
-		dist.Set(&wantKM2, nil)
-	})
+	// Push key material into the distributor and make sure that a call to
+	// KeyMaterial() returns the expected key material, with both the local
+	// certs and root certs.
+	dist.Set(&wantKM1, nil)
+	ctx, cancel = context.WithTimeout(context.Background(), defaultTestTimeout)
+	defer cancel()
+	if err := readAndVerifyKeyMaterial(ctx, dist, &wantKM1); err != nil {
+		t.Fatal(err)
+	}
 
-	waitAndDo(t, proceedCh, errCh, func() {
-		dist.Set(&wantKM2, errProviderTestInternal)
-	})
+	// Push new key material into the distributor and make sure that a call to
+	// KeyMaterial() returns the expected key material, with only root certs.
+	dist.Set(&wantKM2, nil)
+	ctx, cancel = context.WithTimeout(context.Background(), defaultTestTimeout)
+	defer cancel()
+	if err := readAndVerifyKeyMaterial(ctx, dist, &wantKM2); err != nil {
+		t.Fatal(err)
+	}
 
-	waitAndDo(t, proceedCh, errCh, func() {
-		dist.Stop()
-	})
+	// Push an error into the distributor and make sure that a call to
+	// KeyMaterial() returns that error and nil keyMaterial.
+	dist.Set(&wantKM2, errProviderTestInternal)
+	if gotKM, err := dist.KeyMaterial(ctx); gotKM != nil || !errors.Is(err, errProviderTestInternal) {
+		t.Fatalf("KeyMaterial() = {%v, %v}, want {nil, %v}", gotKM, err, errProviderTestInternal)
+	}
 
+	// Stop the distributor and  KeyMaterial() should return errProviderClosed.
+	dist.Stop()
+	ctx, cancel = context.WithTimeout(context.Background(), defaultTestTimeout)
+	defer cancel()
+	if km, err := dist.KeyMaterial(ctx); !errors.Is(err, errProviderClosed) {
+		t.Fatalf("KeyMaterial() = {%v, %v}, want {nil, %v}", km, err, errProviderClosed)
+	}
 }
 
-func waitAndDo(t *testing.T, proceedCh chan struct{}, errCh chan error, do func()) {
-	t.Helper()
+// TestDistributorConcurrency invokes methods on the distributor in parallel.
+func (s) TestDistributorConcurrency(t *testing.T) {
+	dist := NewDistributor()
+
+	// Read cert/key files from testdata.
+	km1 := loadKeyMaterials(t, "x509/server1_cert.pem", "x509/server1_key.pem", "x509/client_ca_cert.pem")
+	km2 := loadKeyMaterials(t, "x509/server2_cert.pem", "x509/server2_key.pem", "x509/client_ca_cert.pem")
+
+	var wg sync.WaitGroup
+	errCh := make(chan error, 1)
 
-	timer := time.NewTimer(defaultTestTimeout)
-	select {
-	case <-timer.C:
-		t.Fatalf("test timed out when waiting for event from distributor")
-	case <-proceedCh:
-		do()
-	case err := <-errCh:
+	// Push key material into the distributor from here and spawn a goroutine to
+	// verify that the distributor returns the expected keyMaterial.
+	ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
+	defer cancel()
+	wg.Add(1)
+	go func() {
+		waitForKeyMaterial(ctx, dist, km1, nil, errCh)
+		wg.Done()
+	}()
+	dist.Set(km1, nil)
+	wg.Wait()
+	if err := <-errCh; err != nil {
 		t.Fatal(err)
 	}
+
+	// Push new key material into the distributor from here and spawn a
+	// goroutine to verify that the distributor returns the expected keyMaterial
+	// eventually.
+	ctx, cancel = context.WithTimeout(context.Background(), defaultTestTimeout)
+	defer cancel()
+	wg.Add(1)
+	go func() {
+		waitForKeyMaterial(ctx, dist, km2, nil, errCh)
+		wg.Done()
+	}()
+	dist.Set(km2, nil)
+	wg.Wait()
+	if err := <-errCh; err != nil {
+		t.Fatal(err)
+	}
+}
+
+// waitForKeyMaterial reads key material from the given distributor until one of
+// the following conditions are met:
+// 1. Returned keyMaterial and error matches wantKM and wantErr.
+// 2. Provider ctx deadline expires.
+func waitForKeyMaterial(ctx context.Context, dist *Distributor, wantKM *KeyMaterial, wantErr error, errCh chan error) {
+	var (
+		gotKM *KeyMaterial
+		err   error
+	)
+
+	for {
+		if gotErr := readAndVerifyKeyMaterial(ctx, dist, wantKM); gotErr == wantErr {
+			errCh <- nil
+			return
+
+		}
+		if errors.Is(err, context.DeadlineExceeded) {
+			errCh <- fmt.Errorf("KeyMaterial() = {%v, %v}, want {%v, %v}", gotKM, err, wantKM, wantErr)
+			return
+		}
+		// Don't busy loop.
+		time.Sleep(100 * time.Millisecond)
+	}
 }

credentials/tls/certprovider/provider.go

@@ -16,7 +16,7 @@
  *
  */
 
-// Package certprovider defines APIs for certificate providers in gRPC.
+// Package certprovider defines APIs for Certificate Providers in gRPC.
 //
 // Experimental
 //
@@ -36,18 +36,18 @@ var (
 	// closed.
 	errProviderClosed = errors.New("provider instance is closed")
 
-	// m is a map from name to provider builder.
+	// m is a map from name to Provider builder.
 	m = make(map[string]Builder)
 )
 
-// Register registers the provider builder, whose name as returned by its Name()
+// Register registers the Provider builder, whose name as returned by its Name()
 // method will be used as the name registered with this builder. Registered
 // Builders are used by the Store to create Providers.
 func Register(b Builder) {
 	m[b.Name()] = b
 }
 
-// getBuilder returns the provider builder registered with the given name.
+// getBuilder returns the Provider builder registered with the given name.
 // If no builder is registered with the provided name, nil will be returned.
 func getBuilder(name string) Builder {
 	if b, ok := m[name]; ok {
@@ -58,8 +58,9 @@ func getBuilder(name string) Builder {
 
 // Builder creates a Provider.
 type Builder interface {
-	// Build creates a new provider with the provided config.
-	Build(StableConfig) Provider
+	// 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
@@ -72,9 +73,9 @@ type Builder interface {
 	Name() string
 }
 
-// StableConfig wraps the method to return a stable provider configuration.
+// StableConfig wraps the method to return a stable Provider configuration.
 type StableConfig interface {
-	// Canonical returns provider config as an arbitrary byte slice.
+	// Canonical returns Provider config as an arbitrary byte slice.
 	// Equivalent configurations must return the same output.
 	Canonical() []byte
 }
@@ -87,18 +88,30 @@ type StableConfig interface {
 // the latest secrets, and free to share any state between different
 // instantiations as they deem fit.
 type Provider interface {
-	// KeyMaterial returns the key material sourced by the provider.
+	// KeyMaterial returns the key material sourced by the Provider.
 	// Callers are expected to use the returned value as read-only.
 	KeyMaterial(ctx context.Context) (*KeyMaterial, error)
 
-	// Close cleans up resources allocated by the provider.
+	// Close cleans up resources allocated by the Provider.
 	Close()
 }
 
-// KeyMaterial wraps the certificates and keys returned by a provider instance.
+// KeyMaterial wraps the certificates and keys returned by a Provider instance.
 type KeyMaterial struct {
 	// Certs contains a slice of cert/key pairs used to prove local identity.
 	Certs []tls.Certificate
 	// Roots contains the set of trusted roots to validate the peer's identity.
 	Roots *x509.CertPool
 }
+
+// Options contains configuration knobs passed to a Provider at creation time.
+type Options struct {
+	// CertName holds the certificate name, whose key material is of interest to
+	// the caller.
+	CertName string
+	// WantRoot indicates if the caller is interested in the root certificate.
+	WantRoot bool
+	// WantIdentity indicates if the caller is interested in the identity
+	// certificate.
+	WantIdentity bool
+}