Bug 217924

Summary: [results.webkit.org] Refresh S3 context when token expires
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, dewei_zhu, ryanhaddad, slewis, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
dewei_zhu: review+
Patch none

Description Jonathan Bedard 2020-10-19 15:22:37 PDT 
S3 access tokens may expire, we need to refresh the S3 context when we detect that the authentication provider has changed the access token.
Comment 1 Jonathan Bedard 2020-10-19 15:23:11 PDT 
<rdar://problem/70387457>
Comment 2 Jonathan Bedard 2020-10-19 15:30:17 PDT 
Created attachment 411806 [details]
Patch
Comment 3 dewei_zhu 2020-10-19 15:41:34 PDT 
Comment on attachment 411806 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411806&action=review

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/model/s3_archiver.py:92
> +        if self._cached_token != self.credentials.aws_session_token:

Maybe adding a null check here or assert on constructor to make sure self.credentials is not None?
Comment 4 Jonathan Bedard 2020-10-19 15:52:28 PDT 
Comment on attachment 411806 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411806&action=review

>> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/model/s3_archiver.py:92
>> +        if self._cached_token != self.credentials.aws_session_token:
> 
> Maybe adding a null check here or assert on constructor to make sure self.credentials is not None?

I can put this check in the __init__, because (although the review hides it), line 63 will raise an exception first.
Comment 5 Jonathan Bedard 2020-10-19 16:33:32 PDT 
Created attachment 411811 [details]
Patch
Comment 6 dewei_zhu 2020-10-19 16:42:27 PDT 
Comment on attachment 411806 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411806&action=review

>>> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/model/s3_archiver.py:92
>>> +        if self._cached_token != self.credentials.aws_session_token:
>> 
>> Maybe adding a null check here or assert on constructor to make sure self.credentials is not None?
> 
> I can put this check in the __init__, because (although the review hides it), line 63 will raise an exception first.

Got it.
Comment 7 dewei_zhu 2020-10-19 16:42:56 PDT 
r=me
Comment 8 EWS 2020-10-19 17:29:53 PDT 
Committed r268702: <https://trac.webkit.org/changeset/268702>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 411811 [details].