fix(Packaging): Fix support of the artifact S3 uri with region #9411
Conversation
| const patterns = [ | ||
| // S3 URI. Ex: s3://bucket/path/to/artifact.zip | ||
| new RegExp('^s3://([^/]+)/(.+)'), | ||
| new RegExp('^s3://(?<bucket>[^/]+)/(?<key>.+)'), |
There was a problem hiding this comment.
I didnt realize you could label your matched groups. That is really cool.
| // Old style S3 URL. Ex: https://s3.amazonaws.com/bucket/path/to/artifact.zip | ||
| new RegExp('s3\\.amazonaws\\.com/([^/]+)/(.+)'), | ||
| // Old style S3 URL. Ex: https://s3.REGION.amazonaws.com/bucket/path/to/artifact.zip | ||
| new RegExp('s3(\\.[\\w\\d-]+)?\\.amazonaws\\.com/(?<bucket>[^/]+)/(?<key>.+)'), |
There was a problem hiding this comment.
For this regex and the one above, you should probably support urls with dashes separating the s3 from the region as apparently s3-region is a valid url is some aws regions.
Ex: https://my-bucket.s3-us-west-2.amazonaws.com
So ([.-][\\w\\d-]+)?.
Codecov Report
@@ Coverage Diff @@
## master #9411 +/- ##
=======================================
Coverage 86.89% 86.90%
=======================================
Files 321 321
Lines 11961 11966 +5
=======================================
+ Hits 10394 10399 +5
Misses 1567 1567
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thank you @zachwhaley - it looks great 🙌 Before we merge, it looks like there are minor issues with formatting - could you please take a look at it? After that we should be good to go 🚀
pgrzesik
changed the title
#9380 Added support for most S3 bucket URIs, but not ones with region in their format, e.g. any bucket outside of us-east-1.
Closes: #9403