Removed dependency on TokenRequestOptions.TenantID (Azure#17709)

It wasn't working anyways and has been removed in the upcoming release.
Refactored the expiring resource machinery to use generics.

Author: jhendrixMSFT

sdk/keyvault/internal/CHANGELOG.md

@@ -1,14 +1,16 @@
 # Release History
 
-## 0.3.1 (Unreleased)
+## 0.4.0 (Unreleased)
 
 ### Features Added
 
 ### Breaking Changes
+* Updated `ExpiringResource` and its dependent types to use generics.
 
 ### Bugs Fixed
 
 ### Other Changes
+* Remove reference to `TokenRequestOptions.TenantID` as it's been removed and wasn't working anyways.
 
 ## 0.3.0 (2022-04-04)
 

sdk/keyvault/internal/challenge_policy.go

@@ -27,7 +27,7 @@ const bearerHeader = "Bearer "
 
 type KeyVaultChallengePolicy struct {
 	// mainResource is the resource to be retrieved using the tenant specified in the credential
-	mainResource *ExpiringResource
+	mainResource *ExpiringResource[*azcore.AccessToken, acquiringResourceState]
 	cred         azcore.TokenCredential
 	scope        *string
 	tenantID     *string
@@ -73,12 +73,10 @@ func (k *KeyVaultChallengePolicy) Do(req *policy.Request) (*http.Response, error
 		return nil, err
 	}
 
-	if token, ok := tk.(*azcore.AccessToken); ok {
-		req.Raw().Header.Set(
-			headerAuthorization,
-			fmt.Sprintf("%s%s", bearerHeader, token.Token),
-		)
-	}
+	req.Raw().Header.Set(
+		headerAuthorization,
+		fmt.Sprintf("%s%s", bearerHeader, tk.Token),
+	)
 
 	// send a copy of the request
 	cloneReq := req.Clone(req.Raw().Context())
@@ -104,15 +102,10 @@ func (k *KeyVaultChallengePolicy) Do(req *policy.Request) (*http.Response, error
 			return resp, err
 		}
 
-		if token, ok := tk.(*azcore.AccessToken); ok {
-			req.Raw().Header.Set(
-				headerAuthorization,
-				bearerHeader+token.Token,
-			)
-		} else {
-			// tk is not an azcore.AccessToken type, something went wrong and we should return the 401 and accompanying error
-			return resp, cloneReqErr
-		}
+		req.Raw().Header.Set(
+			headerAuthorization,
+			bearerHeader+tk.Token,
+		)
 
 		// send the original request now
 		return req.Next()
@@ -220,13 +213,11 @@ type acquiringResourceState struct {
 
 // acquire acquires or updates the resource; only one
 // thread/goroutine at a time ever calls this function
-func acquire(state interface{}) (newResource interface{}, newExpiration time.Time, err error) {
-	s := state.(acquiringResourceState)
-	tk, err := s.p.cred.GetToken(
-		s.req.Raw().Context(),
+func acquire(state acquiringResourceState) (newResource *azcore.AccessToken, newExpiration time.Time, err error) {
+	tk, err := state.p.cred.GetToken(
+		state.req.Raw().Context(),
 		policy.TokenRequestOptions{
-			Scopes:   []string{*s.p.scope},
-			TenantID: *s.p.scope,
+			Scopes: []string{*state.p.scope},
 		},
 	)
 	if err != nil {

sdk/keyvault/internal/constants.go

@@ -7,5 +7,5 @@
 package internal
 
 const (
-	version = "v0.3.1" //nolint
+	version = "v0.4.0" //nolint
 )

sdk/keyvault/internal/expiring_resource.go

@@ -12,34 +12,34 @@ import (
 )
 
 // AcquireResource abstracts a method for refreshing an expiring resource.
-type AcquireResource func(state interface{}) (newResource interface{}, newExpiration time.Time, err error)
+type AcquireResource[TResource any, TState any] func(state TState) (newResource TResource, newExpiration time.Time, err error)
 
 // ExpiringResource is a temporal resource (usually a credential), that requires periodic refreshing.
-type ExpiringResource struct {
+type ExpiringResource[TResource any, TState any] struct {
 	// cond is used to synchronize access to the shared resource embodied by the remaining fields
 	cond *sync.Cond
 
 	// acquiring indicates that some thread/goroutine is in the process of acquiring/updating the resource
 	acquiring bool
 
 	// resource contains the value of the shared resource
-	resource interface{}
+	resource TResource
 
 	// expiration indicates when the shared resource expires; it is 0 if the resource was never acquired
 	expiration time.Time
 
 	// acquireResource is the callback function that actually acquires the resource
-	acquireResource AcquireResource
+	acquireResource AcquireResource[TResource, TState]
 }
 
 // NewExpiringResource creates a new ExpiringResource that uses the specified AcquireResource for refreshing.
-func NewExpiringResource(ar AcquireResource) *ExpiringResource {
-	return &ExpiringResource{cond: sync.NewCond(&sync.Mutex{}), acquireResource: ar}
+func NewExpiringResource[TResource any, TState any](ar AcquireResource[TResource, TState]) *ExpiringResource[TResource, TState] {
+	return &ExpiringResource[TResource, TState]{cond: sync.NewCond(&sync.Mutex{}), acquireResource: ar}
 }
 
 // GetResource returns the underlying resource.
 // If the resource is fresh, no refresh is performed.
-func (er *ExpiringResource) GetResource(state interface{}) (interface{}, error) {
+func (er *ExpiringResource[TResource, TState]) GetResource(state TState) (TResource, error) {
 	// If the resource is expiring within this time window, update it eagerly.
 	// This allows other threads/goroutines to keep running by using the not-yet-expired
 	// resource value while one thread/goroutine updates the resource.
@@ -98,7 +98,7 @@ func (er *ExpiringResource) GetResource(state interface{}) (interface{}, error)
 	return resource, err // Return the resource this thread/goroutine can use
 }
 
-func (er *ExpiringResource) Reset() {
+func (er *ExpiringResource[TResource, TState]) Reset() {
 	// acquire exclusive lock
 	er.cond.L.Lock()
 

sdk/keyvault/internal/expiring_resource_test.go

@@ -15,15 +15,14 @@ import (
 )
 
 func TestNewExpiringResource(t *testing.T) {
-	er := NewExpiringResource(func(state interface{}) (newResource interface{}, newExpiration time.Time, err error) {
-		s := state.(string)
-		switch s {
+	er := NewExpiringResource(func(state string) (newResource string, newExpiration time.Time, err error) {
+		switch state {
 		case "initial":
 			return "updated", time.Now(), nil
 		case "updated":
 			return "refreshed", time.Now().Add(1 * time.Hour), nil
 		default:
-			t.Fatalf("unexpected state %s", s)
+			t.Fatalf("unexpected state %s", state)
 			return "", time.Time{}, errors.New("unexpected")
 		}
 	})