Add support for lazily authenticating to Vault (hashicorp#2049)

Author: benashz

internal/provider/meta.go

@@ -51,21 +51,39 @@ type ProviderMeta struct {
 	client       *api.Client
 	resourceData *schema.ResourceData
 	clientCache  map[string]*api.Client
-	m            sync.RWMutex
 	vaultVersion *version.Version
+	mu           sync.RWMutex
 }
 
 // GetClient returns the providers default Vault client.
-func (p *ProviderMeta) GetClient() *api.Client {
-	return p.client
+func (p *ProviderMeta) GetClient() (*api.Client, error) {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+
+	return p.getClient()
+}
+
+// MustGetClient returns the providers default Vault client. Panics on any error.
+func (p *ProviderMeta) MustGetClient() *api.Client {
+	client, err := p.GetClient()
+	if err != nil {
+		panic(err)
+	}
+
+	return client
 }
 
 // GetNSClient returns a namespaced Vault client.
 // The provided namespace will always be set relative to the default client's
 // namespace.
 func (p *ProviderMeta) GetNSClient(ns string) (*api.Client, error) {
-	p.m.Lock()
-	defer p.m.Unlock()
+	p.mu.Lock()
+	defer p.mu.Unlock()
+
+	client, err := p.getClient()
+	if err != nil {
+		return nil, err
+	}
 
 	if err := p.validate(); err != nil {
 		return nil, err
@@ -88,7 +106,7 @@ func (p *ProviderMeta) GetNSClient(ns string) (*api.Client, error) {
 		return v, nil
 	}
 
-	c, err := p.client.Clone()
+	c, err := client.Clone()
 	if err != nil {
 		return nil, err
 	}
@@ -122,12 +140,21 @@ func (p *ProviderMeta) IsEnterpriseSupported() bool {
 	if ver == nil {
 		return false
 	}
+
 	return strings.Contains(ver.Metadata(), enterpriseMetadata)
 }
 
 // GetVaultVersion returns the providerMeta
 // vaultVersion attribute.
 func (p *ProviderMeta) GetVaultVersion() *version.Version {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+
+	err := p.setVaultVersion()
+	if err != nil {
+		return nil
+	}
+
 	return p.vaultVersion
 }
 
@@ -143,12 +170,19 @@ func (p *ProviderMeta) validate() error {
 	return nil
 }
 
-// NewProviderMeta sets up the Provider to service Vault requests.
-// It is meant to be used as a schema.ConfigureFunc.
-func NewProviderMeta(d *schema.ResourceData) (interface{}, error) {
-	if d == nil {
-		return nil, fmt.Errorf("nil ResourceData provided")
+// setClient sets up an authenticated Vault client based on the
+// ProviderMeta.resourceData configuration. It should typically only need to be
+// called once per ProviderMeta instance. Must be called with a lock.
+func (p *ProviderMeta) setClient() error {
+	if p.client != nil {
+		return nil
+	}
+
+	if p.resourceData == nil {
+		return fmt.Errorf("nil ResourceData provided")
 	}
+
+	d := p.resourceData
 	clientConfig := api.DefaultConfig()
 	addr := d.Get(consts.FieldAddress).(string)
 	if addr != "" {
@@ -175,7 +209,7 @@ func NewProviderMeta(d *schema.ResourceData) (interface{}, error) {
 
 	err := clientConfig.ConfigureTLS(tlsConfig)
 	if err != nil {
-		return nil, fmt.Errorf("failed to configure TLS for Vault API: %s", err)
+		return fmt.Errorf("failed to configure TLS for Vault API: %s", err)
 	}
 
 	clientConfig.HttpClient.Transport = helper.NewTransport(
@@ -192,7 +226,7 @@ func NewProviderMeta(d *schema.ResourceData) (interface{}, error) {
 
 	client, err := api.NewClient(clientConfig)
 	if err != nil {
-		return nil, fmt.Errorf("failed to configure Vault API: %s", err)
+		return fmt.Errorf("failed to configure Vault API: %s", err)
 	}
 
 	// setting this is critical for proper namespace handling
@@ -226,15 +260,15 @@ func NewProviderMeta(d *schema.ResourceData) (interface{}, error) {
 
 	authLogin, err := GetAuthLogin(d)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	var token string
 	if authLogin != nil {
 		// the clone is only used to auth to Vault
 		clone, err := client.Clone()
 		if err != nil {
-			return nil, err
+			return err
 		}
 
 		if clone.Token() != "" {
@@ -256,15 +290,15 @@ func NewProviderMeta(d *schema.ResourceData) (interface{}, error) {
 
 		secret, err := authLogin.Login(clone)
 		if err != nil {
-			return nil, err
+			return err
 		}
 
 		token = secret.Auth.ClientToken
 	} else {
 		// try and get the token from the config or token helper
 		token, err = GetToken(d)
 		if err != nil {
-			return nil, err
+			return err
 		}
 	}
 
@@ -273,15 +307,15 @@ func NewProviderMeta(d *schema.ResourceData) (interface{}, error) {
 	}
 
 	if client.Token() == "" {
-		return nil, errors.New("no vault token set on Client")
+		return errors.New("no vault token set on Client")
 	}
 
 	tokenInfo, err := client.Auth().Token().LookupSelf()
 	if err != nil {
-		return nil, fmt.Errorf("failed to lookup token, err=%w", err)
+		return fmt.Errorf("failed to lookup token, err=%w", err)
 	}
 	if tokenInfo == nil {
-		return nil, fmt.Errorf("no token information returned from self lookup")
+		return fmt.Errorf("no token information returned from self lookup")
 	}
 
 	warnMinTokenTTL(tokenInfo)
@@ -295,7 +329,7 @@ func NewProviderMeta(d *schema.ResourceData) (interface{}, error) {
 		// a child token is always created in the namespace of the parent token.
 		token, err = createChildToken(d, client, tokenNamespace)
 		if err != nil {
-			return nil, err
+			return err
 		}
 
 		client.SetToken(token)
@@ -316,7 +350,7 @@ func NewProviderMeta(d *schema.ResourceData) (interface{}, error) {
 		// namespace paths are properly honoured.
 		if v, ok := d.Get(consts.FieldSetNamespaceFromToken).(bool); ok && v {
 			if err := d.Set(consts.FieldNamespace, namespace); err != nil {
-				return nil, err
+				return err
 			}
 		}
 	}
@@ -326,27 +360,61 @@ func NewProviderMeta(d *schema.ResourceData) (interface{}, error) {
 		client.SetNamespace(namespace)
 	}
 
+	p.client = client
+	return nil
+}
+
+func (p *ProviderMeta) setVaultVersion() error {
+	if p.vaultVersion != nil {
+		return nil
+	}
+
+	d := p.resourceData
 	var vaultVersion *version.Version
 	if v, ok := d.GetOk(consts.FieldVaultVersionOverride); ok {
 		ver, err := version.NewVersion(v.(string))
 		if err != nil {
-			return nil, fmt.Errorf("invalid value for %q, err=%w",
+			return fmt.Errorf("invalid value for %q, err=%w",
 				consts.FieldVaultVersionOverride, err)
 		}
 		vaultVersion = ver
 	} else if !d.Get(consts.FieldSkipGetVaultVersion).(bool) {
 		// Set the Vault version to *ProviderMeta object
+		client, err := p.getClient()
+		if err != nil {
+			return err
+		}
+
 		ver, err := getVaultVersion(client)
 		if err != nil {
-			return nil, err
+			return err
 		}
 		vaultVersion = ver
 	}
 
+	p.vaultVersion = vaultVersion
+
+	return nil
+}
+
+// getClient returns the provider's default Vault client. Must be called with ProviderMeta.mu
+func (p *ProviderMeta) getClient() (*api.Client, error) {
+	if err := p.setClient(); err != nil {
+		return nil, err
+	}
+
+	return p.client, nil
+}
+
+// NewProviderMeta sets up the Provider to service Vault requests.
+// It is meant to be used as a schema.ConfigureFunc.
+func NewProviderMeta(d *schema.ResourceData) (interface{}, error) {
+	if d == nil {
+		return nil, fmt.Errorf("nil ResourceData provided")
+	}
+
 	return &ProviderMeta{
 		resourceData: d,
-		client:       client,
-		vaultVersion: vaultVersion,
 	}, nil
 }
 
@@ -416,7 +484,7 @@ func GetClient(i interface{}, meta interface{}) (*api.Client, error) {
 		return p.GetNSClient(ns)
 	}
 
-	return p.GetClient(), nil
+	return p.GetClient()
 }
 
 func GetClientDiag(i interface{}, meta interface{}) (*api.Client, diag.Diagnostics) {

internal/provider/meta_test.go

@@ -40,13 +40,6 @@ func TestProviderMeta_GetNSClient(t *testing.T) {
 		expectErr    error
 		calls        int
 	}{
-		{
-			name:         "no-client",
-			client:       nil,
-			resourceData: &schema.ResourceData{},
-			wantErr:      true,
-			expectErr:    errors.New("root api.Client not set, init with NewProviderMeta()"),
-		},
 		{
 			name:         "no-resource-data",
 			client:       &api.Client{},
@@ -368,6 +361,9 @@ func TestGetClient(t *testing.T) {
 }
 
 func TestIsAPISupported(t *testing.T) {
+	testutil.SkipTestAcc(t)
+	testutil.TestAccPreCheck(t)
+
 	rootClient, err := api.NewClient(api.DefaultConfig())
 	if err != nil {
 		t.Fatalf("error initializing root client, err=%s", err)
@@ -416,15 +412,6 @@ func TestIsAPISupported(t *testing.T) {
 				vaultVersion: VaultVersion10,
 			},
 		},
-		{
-			name:       "unsupported-unset",
-			minVersion: version.Must(version.NewSemver("1.12.0")),
-			expected:   false,
-			meta: &ProviderMeta{
-				client:       rootClient,
-				vaultVersion: nil,
-			},
-		},
 	}
 
 	for _, tt := range testCases {
@@ -437,6 +424,12 @@ func TestIsAPISupported(t *testing.T) {
 							Type:     schema.TypeString,
 							Required: true,
 						},
+						consts.FieldVaultVersionOverride: {
+							Type: schema.TypeString,
+						},
+						consts.FieldSkipGetVaultVersion: {
+							Type: schema.TypeBool,
+						},
 					},
 					map[string]interface{}{},
 				)
@@ -453,6 +446,9 @@ func TestIsAPISupported(t *testing.T) {
 }
 
 func TestIsEnterpriseSupported(t *testing.T) {
+	testutil.SkipTestAcc(t)
+	testutil.TestAccPreCheck(t)
+
 	rootClient, err := api.NewClient(api.DefaultConfig())
 	if err != nil {
 		t.Fatalf("error initializing root client, err=%s", err)
@@ -502,14 +498,6 @@ func TestIsEnterpriseSupported(t *testing.T) {
 				vaultVersion: VaultVersion12,
 			},
 		},
-		{
-			name:     "unsupported unset",
-			expected: false,
-			meta: &ProviderMeta{
-				client:       rootClient,
-				vaultVersion: nil,
-			},
-		},
 	}
 
 	for _, tt := range testCases {
@@ -522,6 +510,12 @@ func TestIsEnterpriseSupported(t *testing.T) {
 							Type:     schema.TypeString,
 							Required: true,
 						},
+						consts.FieldVaultVersionOverride: {
+							Type: schema.TypeString,
+						},
+						consts.FieldSkipGetVaultVersion: {
+							Type: schema.TypeBool,
+						},
 					},
 					map[string]interface{}{},
 				)
@@ -787,8 +781,13 @@ func TestNewProviderMeta(t *testing.T) {
 				t.Fatalf("invalid type got %T, expected %T", got, &ProviderMeta{})
 			}
 
-			if !reflect.DeepEqual(p.client.Namespace(), tt.wantNamespace) {
-				t.Errorf("NewProviderMeta() got ns = %q, want ns %q", p.client.Namespace(), tt.wantNamespace)
+			client, err := p.GetClient()
+			if err != nil {
+				t.Fatalf("got unexpected error %s", err)
+			}
+
+			if !reflect.DeepEqual(client.Namespace(), tt.wantNamespace) {
+				t.Errorf("NewProviderMeta() got ns = %v, want ns %v", p.client.Namespace(), tt.wantNamespace)
 			}
 
 			if tt.checkSetSetTokenNamespace && tt.wantNamespaceFromToken != tt.d.Get(consts.FieldNamespace).(string) {
@@ -1010,7 +1009,12 @@ func TestNewProviderMeta_Cert(t *testing.T) {
 				t.Fatalf("invalid type got %T, expected %T", got, &ProviderMeta{})
 			}
 
-			if !reflect.DeepEqual(p.client.Namespace(), tt.wantNamespace) {
+			pClient, err := p.GetClient()
+			if err != nil {
+				t.Fatalf("got unexpected error %s", err)
+			}
+
+			if !reflect.DeepEqual(pClient.Namespace(), tt.wantNamespace) {
 				t.Errorf("NewProviderMeta() got ns = %v, want ns %v", p.client.Namespace(), tt.wantNamespace)
 			}
 

vault/data_source_kv_secret_v2_test.go

@@ -72,7 +72,7 @@ func TestDataSourceKVV2Secret_deletedSecret(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				PreConfig: func() {
-					client := testProvider.Meta().(*provider.ProviderMeta).GetClient()
+					client := testProvider.Meta().(*provider.ProviderMeta).MustGetClient()
 
 					err := client.Sys().Mount(mount, &api.MountInput{
 						Type:        "kv-v2",