Add mechanism to inject a JWT + account name #577
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vlovich
reviewed
Apr 27, 2023
vlovich
reviewed
Apr 28, 2023
jasnell
reviewed
Apr 28, 2023
jasnell
reviewed
Apr 28, 2023
OilyLime
force-pushed
the
oilylime/r2-jwt-crossaccount
branch
from
e35ba0b
to
37e01ed
Compare
OilyLime
commented
Apr 28, 2023
OilyLime
force-pushed
the
oilylime/r2-jwt-crossaccount
branch
from
37e01ed
to
7f461b2
Compare
jasnell
reviewed
May 2, 2023
jasnell
reviewed
May 2, 2023
| R2Admin(FeatureFlags featureFlags, | ||
| uint subrequestChannel, | ||
| kj::String account, | ||
| kj::String jwt, |
There was a problem hiding this comment.
If these are always meant to be used together, I would suggest potentially combining them into a single struct.
jasnell
reviewed
May 2, 2023
vlovich
reviewed
May 2, 2023
a-robinson
reviewed
May 2, 2023
OilyLime
force-pushed
the
oilylime/r2-jwt-crossaccount
branch
from
7f461b2
to
894e087
Compare
OilyLime
force-pushed
the
oilylime/r2-jwt-crossaccount
branch
from
894e087
to
8a5ca89
Compare
vlovich
changed the title
To allow greater flexibility in how R2 RPC operations can be performed, introducing the jwt as optional attribute. Also refactored the way R2 paths are built and passed to the doR2HTTP(verb)Request functions, since we expect to support greater variety of paths. Introduced concept of R2CrossAccount, which will leverage the new underlying flexibility to perform cross account operations with proper auth.
OilyLime
force-pushed
the
oilylime/r2-jwt-crossaccount
branch
from
8a5ca89
to
90e7b3b
Compare
OilyLime
added a commit
to OilyLime/workerd
that referenced
this pull request
May 24, 2023
The admin account path is not needed. This was an oversight from cloudflare#577.
OilyLime
added a commit
to OilyLime/workerd
that referenced
this pull request
May 25, 2023
The admin account path is not needed. This was an oversight from cloudflare#577.
OilyLime
added a commit
to OilyLime/workerd
that referenced
this pull request
May 25, 2023
The admin account path is not needed. This was an oversight from cloudflare#577.
OilyLime
added a commit
to OilyLime/workerd
that referenced
this pull request
May 25, 2023
The admin account path is not needed. This was an oversight from cloudflare#577.
OilyLime
added a commit
to OilyLime/workerd
that referenced
this pull request
May 25, 2023
The admin account path is not needed. This was an oversight from cloudflare#577.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To allow greater flexibility in how R2 RPC operations can be performed, introducing the jwt as optional attribute. Also refactored the way R2 paths are built and passed to the doR2HTTPRequest functions, since we expect to support greater variety of paths.
Introduced concept of R2CrossAccount, which will leverage the new underlying flexibility to perform cross account operations with proper auth.