feat: Support AWS S3 Express One Zone buckets #229

4141done · 2024-04-18T22:27:42Z

What

This change adds the S3_SERVICE configuration variable which will default to s3 and may be one of s3express or s3.

It also introduces the virtual-v2 S3_STYLE argument option in support of the connectivity requirement of the S3 Express One Zone (directory) buckets. We are using this as a successor to virtual and believe it should work well in all AWS usages but want to be cautious as we make this change.

Many thanks for @hveiga for driving the implementation of this feature in their original pull request.

Setting this variable to s3express will change the "service" used to sign the requests with the V4 header to s3express. Currently the gateway works without this step, but it's advised in the documentation here.

Other Changes

We are moving the determination of the hostname used to query S3 into the docker entrypoint (or bootstrap script for non-docker installs). If S3_STYLE is set to virtual (this is the default and aws recommended scheme) then the hostname will be:

${S3_BUCKET_NAME}.${S3_SERVER}:${S3_SERVER_PORT}

which will be used in these locations:

The proxy_path directive
The HTTP Host header sent to AWS
The host element of the canonical headers used in signing AWS signature V4 requests.

Based on my reading here: https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html
It looks like AWS recommends that the bucket be always prepended and other schemes exist only for backwards compatibility reasons. However, please comment on this discussion if you have concerns #231

hveiga · 2024-04-19T15:21:58Z

deployments/s3_express/settings.s3express.example

@@ -0,0 +1,21 @@
+S3_BUCKET_NAME=bucket_name=my-bucket-name--usw2-az1--x-s3


I think there's a typo here: bucket_name= should not be present in the value.

Thanks for catching that, I'll fix in a moment!

4141done · 2024-04-19T16:25:11Z

One argument for making a code change is that the security guidance advises us to use the service name s3express in the v4 signing header.

You use your S3 Express One Zone credentials to sign Zonal endpoint (object level) API requests with AWS Signature Version 4, with s3express as the service name.

It's hard to tell from the doc, but that might just be required if you're attempting to create a persistent session. It seems to work without the service name change but we might want to consider sending the correct name in case it's a bug on their end.

I'm on the fence about whether it's enough to warrant another config var, but maybe if enabling the variable also enabled the session keeping it would be worth it.

4141done · 2024-04-22T16:07:59Z

oss/etc/nginx/templates/upstreams.conf.template

@@ -7,5 +7,5 @@ upstream storage_urls {

    # Be sure to specify the port in the S3_SERVER and be sure that port
    # corresponds to the https/http in the proxy_pass directive.
-    server ${S3_SERVER}:${S3_SERVER_PORT};
+    server ${S3_BUCKET_NAME}.${S3_SERVER}:${S3_SERVER_PORT};


Annotation:
Make sure we are forwarding to the full zonal endpoint. If this winds up working, we will find a way to switch on the S3_SERVICE variable to template it this way

hveiga · 2024-04-22T17:58:22Z

common/etc/nginx/include/s3gateway.js

@@ -86,7 +86,7 @@ const INDEX_PAGE = "index.html";
 * Constant defining the service requests are being signed for.
 * @type {string}
 */
-const SERVICE = 's3';
+const SERVICE = 's3express';


I believe we can undo this change.

4141done · 2024-04-22T21:41:01Z

Now that we've verified that this change works, I'm going to follow @hveiga 's original direction and add the S3_SERVICE configuration variable which will default to s3 and may be one of s3express or s3.

Setting this variable to s3express will have the following effects:

Change the "service" used to sign the requests with the V4 header to s3express. Currently the gateway works without this step, but it's advised in the documentation here.
Set the upstreams for both open source and plus to prepend the bucket name to the server address as seen in this PR

I may do a bit of a refactor to accomplish this since we currently have a good number of places where the host is being assembled:

nginx-s3-gateway/common/etc/nginx/templates/default.conf.template

Line 22 in 7f3064b

map $S3_STYLE $s3_host_hdr {

for the Host header
nginx-s3-gateway/common/etc/nginx/include/s3gateway.js

Line 230 in 7f3064b

if (S3_STYLE === 'virtual' || S3_STYLE === 'default' || S3_STYLE === undefined) {

for the host used to sign the v4 headers
nginx-s3-gateway/oss/etc/nginx/templates/upstreams.conf.template

Line 10 in 7f3064b

server ${S3_SERVER}:${S3_SERVER_PORT};

for the actual proxying of the server.

Based on my reading, for a virtual style request signed with the V4 signature we want:

Upstream pointing to: https://bucket-name.s3.region-code.amazonaws.com (S3_BUCKET_NAME and ${S3_SERVER}` in our configuration model)
Host header set to same
The host portion of the AWS v4 signature should match the Host header (ref)

This is what is happening at the moment but for forward maintainability I'd like to see if we can colocate the logic as much as possible. It may not be possible to avoid duplication due to the fact that the upstream blocks need to have the server address templated in before the nginx server is started.

hveiga

LGTM

4141done · 2024-04-23T18:25:26Z

Thank you for the approval @hveiga . I am going to have some other NGINX folks look over this before I merge. I've also opened a discussion here #231 on the chance that this change could cause issues for others. I will wait for a while to give people a chance to speak up before merging. I will make sure that you are credited for this contribution when the PR is merged

4141done · 2024-04-23T18:28:55Z

common/etc/nginx/templates/default.conf.template

+# FINAL_S3_SERVER is set in the startup script
+# (either ./common/docker-entrypoint.sh or ./standalone_ubuntu_oss_install.sh)
+# based on the S3_STYLE configuration option.
+js_var $s3_host ${FINAL_S3_SERVER};


Annotation:
This is the primary change. We determine this value in the startup script then template it in before NGINX starts. While before this value was calculated in three different places, using js_var allows access to its value both in the njs script and in the configuration.

4141done · 2024-04-24T19:07:52Z

.github/workflows/main.yml

@@ -54,6 +54,9 @@ jobs:
  test-oss:
    runs-on: ubuntu-22.04
    needs: build-oss-for-test
+    strategy:
+      matrix:
+        path_style: [virtual, virtual-v2]


Annotation:
We are only testing both path styles at the main test as a compromise on test time. This is the primary place where something might be different and the rest of tests are variations on deployment options.

4141done · 2024-04-25T14:23:51Z

Thanks again for all your help on this change @hveiga - please note that in the end I wound up adding the S3_STYLE config option virtual-v2 to cut down on risk so you'll need to use that config. Please let me know if you experience any issues and we can fix forward.

hveiga · 2024-04-25T14:38:51Z

Thanks again for all your help on this change @hveiga - please note that in the end I wound up adding the S3_STYLE config option virtual-v2 to cut down on risk so you'll need to use that config. Please let me know if you experience any issues and we can fix forward.

Thanks to you for the final push to get this in. I'll be testing today and I can report back if I find any issues. 🙌

hveiga · 2024-04-25T15:52:15Z

I just tested both S3 General and S3 Express buckets with ghcr.io/nginxinc/nginx-s3-gateway/nginx-oss-s3-gateway:latest-20240425 and it works as expected 🎉 🙌

4141done · 2024-04-25T15:53:55Z

I just tested both S3 General and S3 Express buckets with ghcr.io/nginxinc/nginx-s3-gateway/nginx-oss-s3-gateway:latest-20240425 and it works as expected 🎉 🙌

Woohoo! I'm going to close out the issue and the discussion point then.

4141done added 2 commits April 18, 2024 15:26

docs: include instructions for usage with AWS S3 One Zone

3855e86

fix typo

41fe06a

4141done mentioned this pull request Apr 18, 2024

feature: add support for S3 Express #225 #228

Closed

hveiga reviewed Apr 19, 2024

View reviewed changes

4141done added 2 commits April 19, 2024 11:37

fix typo

0424425

move back to not including bucket in s3 address for express zone

16a2fea

4141done commented Apr 22, 2024

View reviewed changes

hveiga reviewed Apr 22, 2024

View reviewed changes

4141done added 3 commits April 23, 2024 08:47

experimental refactor to normalize bucket naming needs

cdd150d

docs, add s3_style config

05ba58b

fix tests for bucket prepended hostname

fee3ef0

hveiga approved these changes Apr 23, 2024

View reviewed changes

4141done changed the title ~~docs: include instructions for usage with AWS S3 One Zone~~ feat: Support AWS S3 Express One Zone buckets Apr 23, 2024

4141done commented Apr 23, 2024

View reviewed changes

4141done added 2 commits April 24, 2024 11:18

move to using as the path style

ce773b6

add matrix to maintain coverage for both path styles

1d10989

4141done commented Apr 24, 2024

View reviewed changes

4141done added 3 commits April 24, 2024 12:15

fix some omissions

de7cb16

default to s3 as a service type if variable not provided

2d8b6a0

fix readme

437cc07

4141done merged commit 7f5df74 into master Apr 25, 2024
8 checks passed

4141done mentioned this pull request Apr 25, 2024

Support AWS S3 Express One Zone #225

Closed

4141done mentioned this pull request May 15, 2024

The request signature we calculated does not match the signature #251

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support AWS S3 Express One Zone buckets #229

feat: Support AWS S3 Express One Zone buckets #229

4141done commented Apr 18, 2024 •

edited

Loading

hveiga Apr 19, 2024

4141done Apr 19, 2024

4141done commented Apr 19, 2024

4141done Apr 22, 2024

hveiga Apr 22, 2024

4141done commented Apr 22, 2024

hveiga left a comment

4141done commented Apr 23, 2024

4141done Apr 23, 2024

4141done Apr 24, 2024

4141done commented Apr 25, 2024

hveiga commented Apr 25, 2024

hveiga commented Apr 25, 2024

4141done commented Apr 25, 2024

		@@ -0,0 +1,21 @@
		S3_BUCKET_NAME=bucket_name=my-bucket-name--usw2-az1--x-s3

feat: Support AWS S3 Express One Zone buckets #229

feat: Support AWS S3 Express One Zone buckets #229

Conversation

4141done commented Apr 18, 2024 • edited Loading

What

Other Changes

hveiga Apr 19, 2024

Choose a reason for hiding this comment

4141done Apr 19, 2024

Choose a reason for hiding this comment

4141done commented Apr 19, 2024

4141done Apr 22, 2024

Choose a reason for hiding this comment

hveiga Apr 22, 2024

Choose a reason for hiding this comment

4141done commented Apr 22, 2024

hveiga left a comment

Choose a reason for hiding this comment

4141done commented Apr 23, 2024

4141done Apr 23, 2024

Choose a reason for hiding this comment

4141done Apr 24, 2024

Choose a reason for hiding this comment

4141done commented Apr 25, 2024

hveiga commented Apr 25, 2024

hveiga commented Apr 25, 2024

4141done commented Apr 25, 2024

4141done commented Apr 18, 2024 •

edited

Loading