PS-1470 Fix issue where artifacts uploaded to custom s3 buckets would have a double slash prefix

[moskyb]

Description

Fix issue where artifacts uploaded to custom s3 buckets would have a double slash prefix

There's an edge case in URL construction for artifacts uploaded to custom S3 buckets where the key for the object would be generated as /object-key (note preceding slash), and then directly appended to a url like https://my-definitely-realy-s3-bucket.s3.amazonaws.com/, leading to a final object url of https://my-definitely-real-s3-bucket.s3.amazonaws.com//object-key (note the double slash!). See the implementation for more details — u.BucketPath is empty by default in custom artifact bucket cases.

When we were on the AWS Go SDK v1, this was fine, as the SDK cleaned urls by default, leading to the object's key being normalized from /object-key to object-key. However, in an undocumented breaking change (🫠), the SDK v2 doesn't do this cleaning, so the object would be uploaded with a leading slash on its key.

The artifact downloader relied on the URL cleaning functionality of the go SDK, so when attempting to download the artifact, it would generate a key of object-key, without the leading slash, leading to consistent 404s when trying to download artifacts from custom buckets.

This PR updates the artifact upload URL generation logic to do a path-join of the key rather than a crude append, essentially emulating the logic that was present in v1 of the AWS Go SDK.

Context

PS-1470

Testing

• Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
• Code is formatted (with go tool gofumpt -extra -w .)\

Disclosures / Credits

I used Amp to investigate the issue initially, then wrote the fourteen-character fix all by myself.

[zhming0]

Is this something we can write an new e2e test for?? with our new e2e test gear

[moskyb]

oooh, good call!

[DrJosh9000]

Suggested change

	var leadingTabsRe = regexp.MustCompile(`(?m)^(\t+)`)
	var leadingTabsRE = regexp.MustCompile(`(?m)^(\t+)`)

[DrJosh9000]

Should this have a depends_on or wait to make the ordering clear?

[DrJosh9000]

Relying on the job to succeed or fail might be enough. Having to pull the job logs might make the test run longer for not much benefit.

[DrJosh9000]

I assume you're working on a version that uses a custom S3 bucket?

[DrJosh9000]

What about:

Suggested change

	set -eo pipefail
	if [[ -z "${BUILDKITE_TRIGGERED_FROM_BUILD_ID}" ]] ; then
	set -euo pipefail
	if [[ -z "${BUILDKITE_TRIGGERED_FROM_BUILD_ID:-}" ]] ; then

[moskyb]

i'm going to merge this as is, and add an e2e test for artifact upload/download with a custom bucket in a followup — i've tested this and made sure it works, but there's quite a bit of infra stuff i'll need to do to set up the e2e test, and i don't want that to block shipping a fix to the customer.