Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions internal/artifact/bk_uploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
65 changes: 54 additions & 11 deletions internal/artifact/bk_uploader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}