diff --git a/internal/artifact/bk_uploader.go b/internal/artifact/bk_uploader.go index 814eed5fb1..0142f61156 100644 --- a/internal/artifact/bk_uploader.go +++ b/internal/artifact/bk_uploader.go @@ -26,9 +26,8 @@ const artifactPathVariable = "${artifact:path}" const ( // BKUploader uploads to S3 either as: - // - a single signed POST, which has a hard limit of 5GB, or - // - as a signed multipart, which has a limit of 5GB per _part_, but we - // aren't supporting larger artifacts yet. + // - a single signed POST, which has a hard limit of 5 GiB, or + // - as a signed multipart, which has a limit of 5 GiB per part. // Note that multipart parts have a minimum size of 5MB. maxFormUploadedArtifactSize = int64(5 * 1024 * 1024 * 1024) ) @@ -61,9 +60,10 @@ func NewBKUploader(l logger.Logger, c BKUploaderConfig) *BKUploader { // it is provided by Buildkite after uploading. func (u *BKUploader) URL(*api.Artifact) string { return "" } -// CreateWork checks the artifact size, then creates one worker. +// CreateWork creates workers for the artifact's upload instructions. func (u *BKUploader) CreateWork(artifact *api.Artifact) ([]workUnit, error) { - if artifact.FileSize > maxFormUploadedArtifactSize { + if artifact.FileSize > maxFormUploadedArtifactSize && + (artifact.UploadInstructions == nil || len(artifact.UploadInstructions.Actions) == 0) { return nil, errArtifactTooLarge{Size: artifact.FileSize} } actions := artifact.UploadInstructions.Actions @@ -383,5 +383,5 @@ type errArtifactTooLarge struct { func (e errArtifactTooLarge) Error() string { // TODO: Clean up error strings // https://github.com/golang/go/wiki/CodeReviewComments#error-strings - return fmt.Sprintf("File size (%d bytes) exceeds the maximum supported by Buildkite's default artifact storage (5Gb). Alternative artifact storage options may support larger files.", e.Size) + return fmt.Sprintf("File size (%d bytes) exceeds the 5 GiB limit for a single upload to Buildkite artifact storage. Multipart upload instructions are required for larger files.", e.Size) } diff --git a/internal/artifact/bk_uploader_test.go b/internal/artifact/bk_uploader_test.go index 0a4fd6bd1f..15888b45ff 100644 --- a/internal/artifact/bk_uploader_test.go +++ b/internal/artifact/bk_uploader_test.go @@ -293,19 +293,62 @@ func TestFormUploadFileMissing(t *testing.T) { func TestFormUploadTooBig(t *testing.T) { uploader := NewBKUploader(logger.Discard, BKUploaderConfig{}) - const size = int64(6442450944) // 6Gb + const size = int64(6 * 1024 * 1024 * 1024) // 6 GiB + for _, test := range []struct { + name string + instructions *api.ArtifactUploadInstructions + }{ + {name: "no actions", instructions: &api.ArtifactUploadInstructions{}}, + {name: "nil instructions"}, + } { + t.Run(test.name, func(t *testing.T) { + artifact := &api.Artifact{ + ID: "xxxxx-xxxx-xxxx-xxxx-xxxxxxxxxx", + Path: "llamas.txt", + AbsolutePath: "/llamas.txt", + GlobPath: "llamas.txt", + ContentType: "text/plain", + FileSize: size, + UploadInstructions: test.instructions, + } + + wantErr := errArtifactTooLarge{Size: size} + if _, err := uploader.CreateWork(artifact); !errors.Is(err, wantErr) { + t.Fatalf("uploader.CreateWork(artifact) error = %v, want %v", err, wantErr) + } + }) + } +} + +func TestMultipartUploadAboveFormLimit(t *testing.T) { + uploader := NewBKUploader(logger.Discard, BKUploaderConfig{}) + const size = int64(5*1024*1024*1024 + 1) + actions := []api.ArtifactUploadAction{ + {URL: "https://example.com/part-2", Method: "PUT", PartNumber: 2}, + {URL: "https://example.com/part-1", Method: "PUT", PartNumber: 1}, + } artifact := &api.Artifact{ - ID: "xxxxx-xxxx-xxxx-xxxx-xxxxxxxxxx", - Path: "llamas.txt", - AbsolutePath: "/llamas.txt", - GlobPath: "llamas.txt", - ContentType: "text/plain", - FileSize: size, - UploadInstructions: &api.ArtifactUploadInstructions{}, + ID: "xxxxx-xxxx-xxxx-xxxx-xxxxxxxxxx", + Path: "llamas.txt", + FileSize: size, + UploadInstructions: &api.ArtifactUploadInstructions{ + Actions: actions, + }, } - wantErr := errArtifactTooLarge{Size: size} - if _, err := uploader.CreateWork(artifact); !errors.Is(err, wantErr) { - t.Fatalf("uploader.CreateWork(artifact) error = %v, want %v", err, wantErr) + work, err := uploader.CreateWork(artifact) + if err != nil { + t.Fatalf("uploader.CreateWork(artifact) error = %v", err) + } + + want := []workUnit{ + &bkMultipartUpload{BKUploader: uploader, artifact: artifact, partCount: 2, action: &actions[0], offset: 0, size: size/2 + 1}, + &bkMultipartUpload{BKUploader: uploader, artifact: artifact, partCount: 2, action: &actions[1], offset: size/2 + 1, size: size / 2}, + } + if diff := cmp.Diff(work, want, + cmp.AllowUnexported(bkMultipartUpload{}), + cmpopts.EquateComparable(uploader), + ); diff != "" { + t.Fatalf("CreateWork diff (-got +want):\n%s", diff) } }