Ensure https tar urls are stored for future requests

[jonsharratt]

What did you implement:

Related to #30

Ensure tar urls are stored as https

How did you implement it:

• Split the url and ensured every url that is stored for tars gets a https url.

How can we verify it:

• Deploy and publish a package

Todos:

• Write tests
• Write documentation
• Fix linting errors
• Tag ready for review or wip

Is this a breaking change?: NO

[brimworks]

Unfortunately, when I yarn publish, the tarball URL becomes something like this:

https//registry.yarnpkg.com/PKGNAME/-/PKGNAME-1.0.1.tgz/registry/PKGNAME/-/PKGNAME-1.0.1.tgz

...so this change won't fix yarn publish.

...also, the domain name of your API Gateway may change over time, therefore I don't think it is a good idea to even store the full URL of the tarball in the index.json.

IMHO the URL of the tarball should be stored in S3 as a relative path, then when GET requests are issued, do the transform of the relative path into the "self-url" + relative path.

[jonsharratt]

As far as this change causing an issue with yarn, npm seems to work ok in suffixing https correctly. This was a fix (at least for us) was needed and unrelated to yarn support.

Your suggestion does make a lot of sense looking through it, you are right this is a way better approach to ensure this is not brittle going forward and supports whichever url the registry is sitting on.

Please do feel free to put a PR in for url resolving if you have changes almost ready to go 👍 if not I can sort this tomorrow.

Thanks for coming back and prodding this in the right direction, really appreciated. Excited to get yarn working.

[jonsharratt]

Sorry to go back and forth on this one, but think even though it appears to be brittle output using full url speaking with others the migration plugin fixes the actual root cause and ensures data is correct.

You will only ever then need to run it once if you change domains (also for previous users it will allow them to easily migrate to https only urls). Not sure why but manipulating the get data for every request makes me feel a little wrong inside especially when we are talking at times spikes in the 1000s of requests a second, it is unfortunate npm decided to store absolute urls.

🙈

[brimworks]

I'm not keen on manipulating the data in GET request, since it means the entire response body has to be buffered into memory and parsed out from JSON only to re-serialize it back to JSON. However, the existing code was already doing that, so it seemed okay to me.

Perhaps the migration plugin should also fix the issue that is forcing you to currently parse the JSON from S3?

[jonsharratt]

Just need to double check that attachments are not needed in the JSON (pretty sure we learnt they are not) then there is the option when publishing a package to not store attachments as the tars get stored.

Then we can clean that GET up as you quite rightly suggest.

Great stuff! 👍