Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
s3: handle 307 redirects
Browse files Browse the repository at this point in the history
alxndrsn authored and alxndrsn committed Sep 20, 2024

Unverified

This user has not yet uploaded their public signing key.
1 parent 0da0ae8 commit 1b5337d
Showing 6 changed files with 57 additions and 0 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/test-nginx.yml
Original file line number Diff line number Diff line change
@@ -22,3 +22,5 @@ jobs:
run: docker logs test-service-1
- if: always()
run: docker logs test-enketo-1
- if: always()
run: docker logs test-mock_s3-1
16 changes: 16 additions & 0 deletions files/nginx/odk.conf.template
Original file line number Diff line number Diff line change
@@ -58,6 +58,22 @@ server {
proxy_request_buffering on;
proxy_buffering off;
proxy_read_timeout 2m;

# transparently follow 307 redirects
# See: https://serverfault.com/a/792035
proxy_intercept_errors on;
error_page 307 = @follow_redirect_transparently;
}

location @follow_redirect_transparently {
# TODO 127.0.0.11 is docker DNS, and may not be required in production
# TODO 8.8.8.8 is google DNS, and may not be palatable to some users
# TODO resolver config will need testing IRL
# See: https://nginx.org/en/docs/http/ngx_http_core_module.html#resolver
# See: https://stackoverflow.com/a/40331256
resolver 127.0.0.11 8.8.8.8;
set $hdr_location '$upstream_http_location';
proxy_pass '$hdr_location';
}

location / {
9 changes: 9 additions & 0 deletions test/mock-http-server/index.js
Original file line number Diff line number Diff line change
@@ -14,6 +14,15 @@ app.get('/reset', withStdLogging((req, res) => {
res.json('OK');
}));

app.get('/blob-server/*', withStdLogging((req, res) => {
res.send(`blob:${req.path.replace('/blob-server/', '')}`);
}));

app.get('/*blob/*', withStdLogging((req, res) => {
// NOTE this may require tweaking when reality of using real nginx server is understood.
res.redirect(307, 'http://mock_s3:33333/blob-server/' + req.path.replace(/.*blob\//, ''));
}));

app.get('/*', ok('GET'));
app.post('/*', ok('POST'));
// TODO add more methods as required
8 changes: 8 additions & 0 deletions test/nginx.test.docker-compose.yml
Original file line number Diff line number Diff line change
@@ -13,13 +13,21 @@ services:
- "8383:8383"
environment:
- PORT=8383
mock_s3:
build:
dockerfile: mock-http-service.dockerfile
ports:
- "33333:33333"
environment:
- PORT=33333
nginx:
build:
context: ..
dockerfile: ./test/nginx-test.dockerfile
depends_on:
- service
- enketo
- mock_s3
environment:
- DOMAIN=odk-nginx.example.test
- SENTRY_ORG_SUBDOMAIN=example
2 changes: 2 additions & 0 deletions test/run-tests.sh
Original file line number Diff line number Diff line change
@@ -38,6 +38,8 @@ log "Waiting for mock backend..."
wait_for_http_response 5 localhost:8383/health 200
log "Waiting for mock enketo..."
wait_for_http_response 5 localhost:8005/health 200
log "Waiting for mock s3..."
wait_for_http_response 5 localhost:33333/health 200
log "Waiting for nginx..."
wait_for_http_response 90 localhost:9000 301

20 changes: 20 additions & 0 deletions test/test-nginx.js
Original file line number Diff line number Diff line change
@@ -104,6 +104,26 @@ describe('nginx config', () => {
{ method:'GET', path:'/v1/some/central-backend/path' },
);
});

it('should transparently follow backend 307 redirects', async () => {
// when
const res = await fetchHttps('/v1/blob/some-bucket/some-id');

// then
assert.isTrue(res.ok);
assert.equal(res.status, 200);
assert.equal(await res.text(), 'blob:some-bucket/some-id');
});

it('should not modify enketo 307 redirects', async () => {
// when
const res = await fetchHttps('/-/blob/some-bucket/some-id');

// then
assert.isFalse(res.ok);
assert.equal(res.status, 307);
assert.equal(await res.headers.get('location'), 'http://mock_s3:33333/blob-server/some-bucket/some-id');
});
});

function fetchHttp(path, options) {

0 comments on commit 1b5337d

Please sign in to comment.