Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Nextstrain.org/fetch broke sometime between yesterday and today #461

Closed
sidneymbell opened this issue Feb 3, 2022 · 3 comments · Fixed by #462
Closed

Nextstrain.org/fetch broke sometime between yesterday and today #461

sidneymbell opened this issue Feb 3, 2022 · 3 comments · Fixed by #462
Assignees
@tsibley
Labels
bug Something isn't working

Comments

@sidneymbell
Copy link
Contributor

Current Behavior
We use long, AWS pre-signed URLs with nextstrain.org/fetch.

These were working as of Tuesday 1/31 at ~2pm PST, but then we noticed this morning that they no longer work (we get a blank screen). I've posted an example URL / dataset in the blab slack.

Expected behavior
A clear and concise description of what you expected to happen instead.

How to reproduce
Steps to reproduce the current behavior:

  1. Navigate to URL described in this message: https://bedfordlab.slack.com/archives/C7SDVPBLZ/p1643846043841639?thread_ts=1643844470.928079&cid=C7SDVPBLZ

Possible solution
(optional)

Your environment: if browsing Nextstrain online
Chrome and safari

Your environment: if running Nextstrain locally

  • Operating system:
  • Browser:
  • Version (e.g. auspice 2.7.0):

Additional context
Add any other context about the problem here.
@sidneymbell sidneymbell added the bug Something isn't working label Feb 3, 2022
@tsibley tsibley self-assigned this Feb 3, 2022
@tsibley
Copy link
Member

tsibley commented Feb 3, 2022

Server logs show a HEAD request to the given S3 signed URL failing with a 403. This is expected for signed URLs because they dictate the permitted S3 action and the same URL can't support both a HEAD (headObject) and GET (getObject). Prior to the merge and deploy of #443 this morning, our /fetch endpoints by design didn't guard Auspice's entrypoint with a HEAD request for the dataset to check existence. This inadvertently changed in 86190b7.

I think I have a fix (overriding exists() in the relevant place) and will test and make a PR soon/tomorrow.

@vincent-czi
Copy link

In case it's useful, here's a long-lived (7 days) pre-signed URL from an S3 bucket of ours, object is the same data as https://data.nextstrain.org/zika.json

https://genepi-auspice-external-data-gestaging.s3.us-west-2.amazonaws.com/data_nextstrain_zika.json?response-content-disposition=inline&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEDEaCXVzLXdlc3QtMSJGMEQCICPJwxhLzK86nji0VgahJf0rV1Q4RfFRChq2YR1EnsdQAiA%2Fq7MD7K2FqdsGneqqA3%2FxHqpBaNPT1XKPO%2BJhUuV7Jiq6AwhqEAEaDDQ3MzAwNDQ5OTA5MSIMN9x6knrnBNHFq%2BttKpcDyu2pBnzTntFrbklRL%2FQqLvnISylEJvHatuhlHpbMWk2sCqPrweB9ke%2FtShACBS%2FQGbYpMkt4CrhNgwimuGxdvaLOmpsqAboDPP0x32k%2FuzbuT48Tod5m%2B8HUQWvAsWe71aL%2BaS3P5fOf3IjLLtOLvKi8MaOTC2mFjXjAPJpTO8CWvj4W2gFeG1ppMCLLPybKRouaqfuhFWZrfR0XU%2BuPFezj%2FYcVyifWCMwhcZ8to82lONyJrAOAYVW4H%2B804%2BB1lRi7ufckECZpVWw2cuNANS6NcekAybBQDiwI57jsSGtLuqBHdolpT9wN0%2BRnxcLHo4nvhBlEvo%2B6MddOSBwbkVh4eiTP83rCgRuRsF6tClOJO0cmZkB3rAi45MHOGmRojDmbknuShVgT07uDFZNyEKnodm1rX75nCR8QojwXcK9wP6yuMQX3Fhf71015OcigPjUWju0VQHh%2BC8fWsBDk2WR7NcVkHX0dcY%2FmLDLs4kgEDmv7zZA6qGmYRKPmPLHtuncjB5x9as8SDlvJeJ4L%2Ba3D0au4CcwwlsTsjwY6hQKFNIuBzTp7732XCLNjIYe6C1yNEqSY4qN8vIYtxN9UH%2Foe7kLKMkgcJfNSyuU7ubvez29uyNeRPGs7QUlLNVxOSCWPnauskdkibUqgEPPH5E%2FTJUih16dVdfz%2F0eibOgIPWwSQCHqzhZ3nd5D8EqGMOcH%2Fqvdxt64tMaxdfpsmOhSqbUSfvTW247jTo3wgmJnF7IZoMhTEDYuEZboXgWiAQS%2FmxU72aIoo7DdZAR01uIZXx%2BsGLykyPLMTYFHQ98QdArXOWaXPVzlKd8R1vfG1piXh56aFJ1WPMfTZvg5JcsKtieWx8Pe3p1zPGV77sNcAIo2Z4CDytfUYO9uVTUbASkmJaqE%3D&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Date=20220203T003157Z&X-Amz-SignedHeaders=host&X-Amz-Expires=604800&X-Amz-Credential=ASIAW4IKD4CJT7VS6PZQ%2F20220203%2Fus-west-2%2Fs3%2Faws4_request&X-Amz-Signature=113cef246a95a3dbf90c2de0d1c7fad6037e79b2c98ea4a4b326c0b08343b9c3

tsibley added a commit that referenced this issue Feb 3, 2022
@tsibley
Don't check existence of datasets and narratives for /fetch/…
8a0db13 
This fixes a key use case of /fetch where AWS S3 signed URLs are given.

These signed URLs dictate the permitted S3 action, and the same URL
can't support both a HEAD (headObject) and GET (getObject) request.
Prior to the merge and deploy of #443 this morning, the /fetch endpoints
by design didn't guard Auspice's entrypoint with a HEAD request for the
dataset to check existence.  This inadvertently changed in "Refactor
endpoints for GETing a dataset/narrative to make room for additional
media types" (86190b7).

Now once again for /fetch we'll always serve up Auspice, which will make
a request to Charon, which may return an error if the upstream request
fails, and an error from Charon will trigger Auspice's error page.
That's unlike everywhere else where we try to avoid Auspice's error
page, but it's ok.

Resolves #461.
@tsibley tsibley mentioned this issue Feb 3, 2022
Merged
@tsibley
Copy link
Member

tsibley commented Feb 3, 2022

@vincent-czi Thanks! I'd already generated a signed URL for testing, using that same zika dataset. :-)

PR with fix is #462.

@tsibley tsibley moved this from New to In Review in Nextstrain planning (archived) Feb 3, 2022
Repository owner moved this from In Review to Done in Nextstrain planning (archived) Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tsibley tsibley

Labels
bug Something isn't working
Projects
No open projects
Nextstrain planning (archived)
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't check existence of datasets and narratives for /fetch/… nextstrain/nextstrain.org
3 participants
@tsibley @sidneymbell @vincent-czi