credentials: remove the context timeout to fix token request failure with non-GCE ADC

[rockspore]

The defer cancel() is called when NewDefaultCredentialsWithOptions returns and this is definitely a problem. It means the context is done, but all non-GCE token sources will rely on it to make HTTP requests to the token endpoint.

Alternatively (or maybe preferably), NewDefaultCredentialsWithOptions should take a top-level context. Despite this being an experimental API, I don't think such a breaking change is easy.

RELEASE NOTES: NONE

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.74%. Comparing base (66385b2) to head (0381aae).
Report is 8 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7845      +/-   ##
==========================================
- Coverage   81.84%   81.74%   -0.11%     
==========================================
  Files         374      374              
  Lines       37993    37994       +1     
==========================================
- Hits        31096    31057      -39     
- Misses       5598     5622      +24     
- Partials     1299     1315      +16     

Files with missing lines	Coverage Δ
credentials/google/google.go	77.33% <100.00%> (+0.30%)	⬆️

... and 25 files with indirect coverage changes

---- 🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[dfawley]

Why change this part? Maybe we should leave this alone and use context.TODO() at the call site instead of something that adds a deadline to a context.Background.

This seems like an incorrect use of contexts to me, though, by the oauth package.

[rockspore]

Why change this part? Maybe we should leave this alone and use context.TODO() at the call site instead of something that adds a deadline to a context.Background.

It's internal method so I didn't think too much. But it's perfectly fine to leave it unchanged.

This seems like an incorrect use of contexts to me, though, by the oauth package.

I fully agree. As I pointed out in internal ticket, there is definitely some code smell since it keeps this given context in a struct. So basically, it can only take a top-level context from a main func and doesn't allow any deadline or timeout to be set.

[rockspore]

The OAuth library team also maintains the DetectDefault API that's meant to support both gRPC and HTTP for client SDK. That API doesn't need a context. Some adapter is needed to create the gRPC's RPCCreds from it so I am not sure whether it's worth moving to that API.

I went for this simplistic fix as I just wanted to unblock Cloud Spanner's issue with Direct Path. But still wanted to just mention the above option.

[dfawley]

I'm fine with this as a workaround, and if there is some better way to be using oauth, then I'm happy to review changes that move us to that as well. It doesn't seem like using this API is a problem, so I think we can treat anything like that as a cleanup.

[dfawley]

Thank you for the fix! I searched and found that this should fix an old issue that was closed without a fix (#6285).