diff --git a/openstack/client.go b/openstack/client.go index df0e4071f..7ab2b5a36 100644 --- a/openstack/client.go +++ b/openstack/client.go @@ -281,12 +281,16 @@ func v3AKSKAuth(client *golangsdk.ProviderClient, endpoint string, options golan // update AKSKAuthOptions of ProviderClient // ProviderClient(client) is a reference to the ServiceClient(v3Client) defer func() { - client.AKSKAuthOptions.ProjectId = options.ProjectId - client.AKSKAuthOptions.DomainID = options.DomainID + client.UpdateAKSKOptions(func(o *golangsdk.AKSKAuthOptions) { + o.ProjectId = options.ProjectId + o.DomainID = options.DomainID + }) }() - client.AKSKAuthOptions = options - client.AKSKAuthOptions.DomainID = "" + client.UpdateAKSKOptions(func(o *golangsdk.AKSKAuthOptions) { + *o = options + o.DomainID = "" + }) if options.ProjectId == "" && options.ProjectName != "" { id, err := getProjectID(v3Client, options.ProjectName) @@ -294,7 +298,9 @@ func v3AKSKAuth(client *golangsdk.ProviderClient, endpoint string, options golan return err } options.ProjectId = id - client.AKSKAuthOptions.ProjectId = options.ProjectId + client.UpdateAKSKOptions(func(o *golangsdk.AKSKAuthOptions) { + o.ProjectId = options.ProjectId + }) } if options.DomainID == "" && options.Domain != "" { @@ -368,11 +374,13 @@ func authWithAgencyByAKSK(client *golangsdk.ProviderClient, endpoint string, opt return fmt.Errorf("error obtaining temporary AK/SK for agency: %w", err) } - client.AKSKAuthOptions.AccessKey = credential.AccessKey - client.AKSKAuthOptions.SecretKey = credential.SecretKey - client.AKSKAuthOptions.SecurityToken = credential.SecurityToken - client.AKSKAuthOptions.ProjectId = "" - client.AKSKAuthOptions.DomainID = "" + client.UpdateAKSKOptions(func(o *golangsdk.AKSKAuthOptions) { + o.AccessKey = credential.AccessKey + o.SecretKey = credential.SecretKey + o.SecurityToken = credential.SecurityToken + o.ProjectId = "" + o.DomainID = "" + }) if opts.DelegatedProject != "" { projectID, err := getProjectID(v3Client, opts.DelegatedProject) @@ -380,7 +388,9 @@ func authWithAgencyByAKSK(client *golangsdk.ProviderClient, endpoint string, opt return fmt.Errorf("error resolving delegated project %q: %w", opts.DelegatedProject, err) } client.ProjectID = projectID - client.AKSKAuthOptions.ProjectId = projectID + client.UpdateAKSKOptions(func(o *golangsdk.AKSKAuthOptions) { + o.ProjectId = projectID + }) } else { domainID, err := getDomainID(opts.AgencyDomainName, v3Client) if err != nil { @@ -388,7 +398,9 @@ func authWithAgencyByAKSK(client *golangsdk.ProviderClient, endpoint string, opt } client.ProjectID = "" client.DomainID = domainID - client.AKSKAuthOptions.DomainID = domainID + client.UpdateAKSKOptions(func(o *golangsdk.AKSKAuthOptions) { + o.DomainID = domainID + }) } client.ReauthFunc = func() error { diff --git a/openstack/testing/client_test.go b/openstack/testing/client_test.go index 8eb974640..4c00e3de5 100644 --- a/openstack/testing/client_test.go +++ b/openstack/testing/client_test.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "strings" + "sync" "testing" "github.com/opentelekomcloud/gophertelekomcloud" @@ -490,3 +491,133 @@ func TestAuthenticatedClientV3WithAgencyAKSKWithoutDelegatedProjectKeepsDomainSc th.CheckEquals(t, "temporary-ak", client.AKSKAuthOptions.AccessKey) th.CheckEquals(t, "temporary-security-token", client.AKSKAuthOptions.SecurityToken) } + +// TestAuthenticatedClientV3AgencyAKSKConcurrentReauthIsRaceFree drives many concurrent signed +// requests while the agency ReauthFunc keeps refreshing the temporary AK/SK. Run with -race +func TestAuthenticatedClientV3AgencyAKSKConcurrentReauthIsRaceFree(t *testing.T) { + th.SetupHTTP() + defer th.TeardownHTTP() + + th.Mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + _, _ = fmt.Fprintf(w, ` + { + "versions": { + "values": [ + { + "status": "stable", + "id": "v3.0", + "links": [ + { "href": "%s", "rel": "self" } + ] + } + ] + } + } + `, th.Endpoint()+"v3/") + }) + + th.Mux.HandleFunc("/v3/auth/catalog", func(w http.ResponseWriter, r *http.Request) { + _, _ = fmt.Fprintf(w, ` + { + "catalog": [ + { + "type": "identity", + "name": "iam", + "endpoints": [ + { + "interface": "public", + "region": "eu-de", + "url": "%s" + } + ] + } + ], + "links": { "next": null, "previous": null } + } + `, th.Endpoint()+"v3/") + }) + + th.Mux.HandleFunc("/v3.0/OS-CREDENTIAL/securitytokens", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusCreated) + _, _ = fmt.Fprint(w, ` + { + "credential": { + "access": "temporary-ak", + "secret": "temporary-sk", + "securitytoken": "temporary-security-token", + "expires_at": "2030-01-01T00:00:00.000000Z" + } + } + `) + }) + + th.Mux.HandleFunc("/v3/projects", func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") == "" { + t.Errorf("expected request to be signed") + } + _, _ = fmt.Fprint(w, ` + { + "projects": [ + { + "id": "target-project-id", + "name": "target-project" + } + ], + "links": { "next": null, "previous": null } + } + `) + }) + + options := golangsdk.AKSKAuthOptions{ + IdentityEndpoint: th.Endpoint(), + DomainID: "source-domain-id", + AccessKey: "source-ak", + SecretKey: "source-sk", + AgencyName: "target-agency", + AgencyDomainName: "target-domain", + DelegatedProject: "target-project", + } + + client, err := openstack.AuthenticatedClient(options) + th.AssertNoErr(t, err) + + const ( + readers = 16 + iterations = 50 + ) + + var wg sync.WaitGroup + + // Single writer goroutine, mirroring the serialized reauth in reauthenticateAndRetry: it keeps + // publishing fresh temporary credentials while the readers below sign concurrently. + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < iterations; i++ { + if err := client.ReauthFunc(); err != nil { + t.Errorf("reauth failed: %s", err) + return + } + } + }() + + endpoint := th.Endpoint() + "v3/projects" + for r := 0; r < readers; r++ { + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < iterations; i++ { + resp, err := client.Request("GET", endpoint, &golangsdk.RequestOpts{ + OkCodes: []int{200}, + }) + if err != nil { + t.Errorf("signed request failed: %s", err) + return + } + _ = resp.Body.Close() + } + }() + } + + wg.Wait() +} diff --git a/provider_client.go b/provider_client.go index 2a56d3726..88782844c 100644 --- a/provider_client.go +++ b/provider_client.go @@ -96,6 +96,10 @@ type ProviderClient struct { mut *sync.RWMutex reauthmut *reauthlock + + // akskMut guards concurrent access to AKSKAuthOptions. It allows request signing to read + // a consistent snapshot of the AK/SK credentials while a ReauthFunc refreshes them. + akskMut *sync.RWMutex } type reauthlock struct { @@ -126,6 +130,30 @@ func (client *ProviderClient) AuthenticatedHeaders() (m map[string]string) { func (client *ProviderClient) UseTokenLock() { client.mut = new(sync.RWMutex) client.reauthmut = new(reauthlock) + client.akskMut = new(sync.RWMutex) +} + +// AKSKOptions safely reads a consistent snapshot of the AK/SK auth options. Applications and custom +// ReauthFuncs should use this together with UpdateAKSKOptions instead of accessing the +// AKSKAuthOptions field directly when the client is used concurrently. +func (client *ProviderClient) AKSKOptions() AKSKAuthOptions { + if client.akskMut != nil { + client.akskMut.RLock() + defer client.akskMut.RUnlock() + } + return client.AKSKAuthOptions +} + +// UpdateAKSKOptions safely mutates the AK/SK auth options under the write lock. It is intended for +// use from a custom ReauthFunc to publish refreshed temporary credentials without racing concurrent +// request signing. The callback must not perform any I/O, as that would hold the lock across the +// request path and deadlock concurrent readers. +func (client *ProviderClient) UpdateAKSKOptions(update func(opts *AKSKAuthOptions)) { + if client.akskMut != nil { + client.akskMut.Lock() + defer client.akskMut.Unlock() + } + update(&client.AKSKAuthOptions) } // Token safely reads the value of the auth token from the ProviderClient. Applications should @@ -288,19 +316,20 @@ func (client *ProviderClient) Request(method, url string, options *RequestOpts) prereqtok := req.Header.Get("X-Auth-Token") - if client.AKSKAuthOptions.AccessKey != "" { + akskOpts := client.AKSKOptions() + if akskOpts.AccessKey != "" { Sign(req, SignOptions{ - AccessKey: client.AKSKAuthOptions.AccessKey, - SecretKey: client.AKSKAuthOptions.SecretKey, + AccessKey: akskOpts.AccessKey, + SecretKey: akskOpts.SecretKey, }) - if client.AKSKAuthOptions.ProjectId != "" && client.AKSKAuthOptions.DomainID == "" { - req.Header.Set("X-Project-Id", client.AKSKAuthOptions.ProjectId) + if akskOpts.ProjectId != "" && akskOpts.DomainID == "" { + req.Header.Set("X-Project-Id", akskOpts.ProjectId) } - if client.AKSKAuthOptions.DomainID != "" { - req.Header.Set("X-Domain-Id", client.AKSKAuthOptions.DomainID) + if akskOpts.DomainID != "" { + req.Header.Set("X-Domain-Id", akskOpts.DomainID) } - if client.AKSKAuthOptions.SecurityToken != "" { - req.Header.Set("X-Security-Token", client.AKSKAuthOptions.SecurityToken) + if akskOpts.SecurityToken != "" { + req.Header.Set("X-Security-Token", akskOpts.SecurityToken) } }