Merge pull request #2985 from buildkite/artifact-tidy

agent/ec2_meta_data.go

@@ -2,6 +2,7 @@ package agent
 
 import (
 	"github.com/aws/aws-sdk-go/aws/ec2metadata"
+	"github.com/buildkite/agent/v3/internal/awslib"
 )
 
 type EC2MetaData struct {
@@ -58,7 +59,7 @@ func (e EC2MetaData) Get() (map[string]string, error) {
 }
 
 func newAWSClient() (*ec2metadata.EC2Metadata, error) {
-	sess, err := awsSession()
+	sess, err := awslib.Session()
 	if err != nil {
 		return &ec2metadata.EC2Metadata{}, err
 	}

agent/ec2_tags.go

@@ -4,13 +4,14 @@ import (
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/ec2metadata"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/buildkite/agent/v3/internal/awslib"
 )
 
 type EC2Tags struct {
 }
 
 func (e EC2Tags) Get() (map[string]string, error) {
-	sess, err := awsSession()
+	sess, err := awslib.Session()
 	if err != nil {
 		return nil, err
 	}

clicommand/agent_start.go

@@ -25,6 +25,7 @@ import (
 	"github.com/buildkite/agent/v3/api"
 	"github.com/buildkite/agent/v3/core"
 	"github.com/buildkite/agent/v3/internal/agentapi"
+	"github.com/buildkite/agent/v3/internal/awslib"
 	awssigner "github.com/buildkite/agent/v3/internal/cryptosigner/aws"
 	"github.com/buildkite/agent/v3/internal/experiments"
 	"github.com/buildkite/agent/v3/internal/job/hook"
@@ -904,7 +905,7 @@ var AgentStartCommand = cli.Command{
 
 			// this is currently loaded here to ensure it is ONLY loaded if the agent is using KMS for signing
 			// this will limit the possible impact of this new SDK on the rest of the agent users
-			awscfg, err := agent.GetAWSConfigV2(
+			awscfg, err := awslib.GetConfigV2(
 				ctx,
 				config.WithClientLogMode(logMode),
 			)

clicommand/artifact_download.go

@@ -4,8 +4,8 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/buildkite/agent/v3/agent"
 	"github.com/buildkite/agent/v3/api"
+	"github.com/buildkite/agent/v3/internal/artifact"
 	"github.com/urfave/cli"
 )
 
@@ -111,7 +111,7 @@ var ArtifactDownloadCommand = cli.Command{
 		client := api.NewClient(l, loadAPIClientConfig(cfg, "AgentAccessToken"))
 
 		// Setup the downloader
-		downloader := agent.NewArtifactDownloader(l, client, agent.ArtifactDownloaderConfig{
+		downloader := artifact.NewDownloader(l, client, artifact.DownloaderConfig{
 			Query:              cfg.Query,
 			Destination:        cfg.Destination,
 			BuildID:            cfg.Build,

clicommand/artifact_search.go

@@ -7,8 +7,8 @@ import (
 	"strings"
 	"time"
 
-	"github.com/buildkite/agent/v3/agent"
 	"github.com/buildkite/agent/v3/api"
+	"github.com/buildkite/agent/v3/internal/artifact"
 	"github.com/urfave/cli"
 )
 
@@ -144,7 +144,7 @@ var ArtifactSearchCommand = cli.Command{
 		client := api.NewClient(l, loadAPIClientConfig(cfg, "AgentAccessToken"))
 
 		// Setup the searcher and try get the artifacts
-		searcher := agent.NewArtifactSearcher(l, client, cfg.Build)
+		searcher := artifact.NewSearcher(l, client, cfg.Build)
 		artifacts, err := searcher.Search(ctx, cfg.Query, cfg.Step, cfg.IncludeRetriedJobs, true)
 		if err != nil {
 			return err

clicommand/artifact_shasum.go

@@ -6,8 +6,8 @@ import (
 	"io"
 	"os"
 
-	"github.com/buildkite/agent/v3/agent"
 	"github.com/buildkite/agent/v3/api"
+	"github.com/buildkite/agent/v3/internal/artifact"
 	"github.com/buildkite/agent/v3/logger"
 	"github.com/urfave/cli"
 )
@@ -127,7 +127,7 @@ func searchAndPrintShaSum(
 	client := api.NewClient(l, loadAPIClientConfig(cfg, "AgentAccessToken"))
 
 	// Find the artifact we want to show the SHASUM for
-	searcher := agent.NewArtifactSearcher(l, client, cfg.Build)
+	searcher := artifact.NewSearcher(l, client, cfg.Build)
 	artifacts, err := searcher.Search(ctx, cfg.Query, cfg.Step, cfg.IncludeRetriedJobs, false)
 	if err != nil {
 		return fmt.Errorf("Error searching for artifacts: %s", err)

clicommand/artifact_upload.go

@@ -4,8 +4,8 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/buildkite/agent/v3/agent"
 	"github.com/buildkite/agent/v3/api"
+	"github.com/buildkite/agent/v3/internal/artifact"
 	"github.com/urfave/cli"
 )
 
@@ -148,7 +148,7 @@ var ArtifactUploadCommand = cli.Command{
 		client := api.NewClient(l, loadAPIClientConfig(cfg, "AgentAccessToken"))
 
 		// Setup the uploader
-		uploader := agent.NewArtifactUploader(l, client, agent.ArtifactUploaderConfig{
+		uploader := artifact.NewUploader(l, client, artifact.UploaderConfig{
 			JobID:       cfg.Job,
 			Paths:       cfg.UploadPaths,
 			Destination: cfg.Destination,

clicommand/pipeline_upload.go

@@ -18,6 +18,7 @@ import (
 	"github.com/buildkite/agent/v3/agent"
 	"github.com/buildkite/agent/v3/api"
 	"github.com/buildkite/agent/v3/env"
+	"github.com/buildkite/agent/v3/internal/awslib"
 	awssigner "github.com/buildkite/agent/v3/internal/cryptosigner/aws"
 	"github.com/buildkite/agent/v3/internal/experiments"
 	"github.com/buildkite/agent/v3/internal/redact"
@@ -289,7 +290,7 @@ var PipelineUploadCommand = cli.Command{
 
 		switch {
 		case cfg.SigningAWSKMSKey != "":
-			awscfg, err := agent.GetAWSConfigV2(ctx)
+			awscfg, err := awslib.GetConfigV2(ctx)
 			if err != nil {
 				return err
 			}

clicommand/tool_sign.go

@@ -10,7 +10,7 @@ import (
 	"strings"
 
 	"github.com/aws/aws-sdk-go-v2/service/kms"
-	"github.com/buildkite/agent/v3/agent"
+	"github.com/buildkite/agent/v3/internal/awslib"
 	"github.com/buildkite/agent/v3/internal/bkgql"
 	awssigner "github.com/buildkite/agent/v3/internal/cryptosigner/aws"
 	"github.com/buildkite/agent/v3/internal/stdin"
@@ -190,7 +190,7 @@ Signing a pipeline from a file:
 		switch {
 		case cfg.AWSKMSKeyID != "":
 			// load the AWS SDK V2 config
-			awscfg, err := agent.GetAWSConfigV2(ctx)
+			awscfg, err := awslib.GetConfigV2(ctx)
 			if err != nil {
 				return err
 			}

internal/artifact/api_client.go

@@ -0,0 +1,14 @@
+package artifact
+
+import (
+	"context"
+
+	"github.com/buildkite/agent/v3/api"
+)
+
+// APIClient describes the Agent REST API methods used by the artifact package.
+type APIClient interface {
+	CreateArtifacts(context.Context, string, *api.ArtifactBatch) (*api.ArtifactBatchCreateResponse, *api.Response, error)
+	SearchArtifacts(context.Context, string, *api.ArtifactSearchOptions) ([]*api.Artifact, *api.Response, error)
+	UpdateArtifacts(context.Context, string, map[string]string) (*api.Response, error)
+}

agent/artifactory_downloader.go → internal/artifact/artifactory_downloader.go

@@ -1,4 +1,4 @@
-package agent
+package artifact
 
 import (
 	"context"

agent/artifactory_downloader_test.go → internal/artifact/artifactory_downloader_test.go

@@ -1,4 +1,4 @@
-package agent
+package artifact
 
 import (
 	"testing"

agent/artifactory_uploader.go → internal/artifact/artifactory_uploader.go

@@ -1,4 +1,4 @@
-package agent
+package artifact
 
 import (
 	"context"

agent/artifactory_uploader_test.go → internal/artifact/artifactory_uploader_test.go

@@ -1,4 +1,4 @@
-package agent
+package artifact
 
 import (
 	"testing"

agent/artifact_batch_creator.go → internal/artifact/batch_creator.go

@@ -1,4 +1,4 @@
-package agent
+package artifact
 
 import (
 	"context"
@@ -9,7 +9,7 @@ import (
 	"github.com/buildkite/roko"
 )
 
-type ArtifactBatchCreatorConfig struct {
+type BatchCreatorConfig struct {
 	// The ID of the Job that these artifacts belong to
 	JobID string
 
@@ -24,9 +24,9 @@ type ArtifactBatchCreatorConfig struct {
 	CreateArtifactsTimeout time.Duration
 }
 
-type ArtifactBatchCreator struct {
+type BatchCreator struct {
 	// The creation config
-	conf ArtifactBatchCreatorConfig
+	conf BatchCreatorConfig
 
 	// The logger instance to use
 	logger logger.Logger
@@ -35,15 +35,15 @@ type ArtifactBatchCreator struct {
 	apiClient APIClient
 }
 
-func NewArtifactBatchCreator(l logger.Logger, ac APIClient, c ArtifactBatchCreatorConfig) *ArtifactBatchCreator {
-	return &ArtifactBatchCreator{
+func NewArtifactBatchCreator(l logger.Logger, ac APIClient, c BatchCreatorConfig) *BatchCreator {
+	return &BatchCreator{
 		logger:    l,
 		conf:      c,
 		apiClient: ac,
 	}
 }
 
-func (a *ArtifactBatchCreator) Create(ctx context.Context) ([]*api.Artifact, error) {
+func (a *BatchCreator) Create(ctx context.Context) ([]*api.Artifact, error) {
 	length := len(a.conf.Artifacts)
 	chunks := 30
 

agent/download.go → internal/artifact/download.go

@@ -1,4 +1,4 @@
-package agent
+package artifact
 
 import (
 	"context"

agent/download_test.go → internal/artifact/download_test.go

@@ -1,7 +1,6 @@
 //go:build !windows
-// +build !windows
 
-package agent
+package artifact
 
 import (
 	"context"

agent/download_unix.go → internal/artifact/download_unix.go

@@ -1,6 +1,6 @@
 //go:build unix
 
-package agent
+package artifact
 
 import (
 	"os"

agent/artifact_downloader.go → internal/artifact/downloader.go

@@ -1,4 +1,4 @@
-package agent
+package artifact
 
 import (
 	"context"
@@ -12,12 +12,11 @@ import (
 
 	"github.com/aws/aws-sdk-go/service/s3"
 	"github.com/buildkite/agent/v3/api"
-	iartifact "github.com/buildkite/agent/v3/internal/artifact"
 	"github.com/buildkite/agent/v3/logger"
 	"github.com/buildkite/agent/v3/pool"
 )
 
-type ArtifactDownloaderConfig struct {
+type DownloaderConfig struct {
 	// The ID of the Build
 	BuildID string
 
@@ -37,9 +36,9 @@ type ArtifactDownloaderConfig struct {
 	DebugHTTP bool
 }
 
-type ArtifactDownloader struct {
+type Downloader struct {
 	// The config for downloading
-	conf ArtifactDownloaderConfig
+	conf DownloaderConfig
 
 	// The logger instance to use
 	logger logger.Logger
@@ -48,15 +47,15 @@ type ArtifactDownloader struct {
 	apiClient APIClient
 }
 
-func NewArtifactDownloader(l logger.Logger, ac APIClient, c ArtifactDownloaderConfig) ArtifactDownloader {
-	return ArtifactDownloader{
+func NewDownloader(l logger.Logger, ac APIClient, c DownloaderConfig) Downloader {
+	return Downloader{
 		logger:    l,
 		apiClient: ac,
 		conf:      c,
 	}
 }
 
-func (a *ArtifactDownloader) Download(ctx context.Context) error {
+func (a *Downloader) Download(ctx context.Context) error {
 	// Turn the download destination into an absolute path and confirm it exists
 	destination, _ := filepath.Abs(a.conf.Destination)
 	fileInfo, err := os.Stat(destination)
@@ -68,7 +67,7 @@ func (a *ArtifactDownloader) Download(ctx context.Context) error {
 		return fmt.Errorf("%s is not a directory", destination)
 	}
 
-	artifacts, err := NewArtifactSearcher(a.logger, a.apiClient, a.conf.BuildID).
+	artifacts, err := NewSearcher(a.logger, a.apiClient, a.conf.BuildID).
 		Search(ctx, a.conf.Query, a.conf.Step, a.conf.IncludeRetriedJobs, false)
 	if err != nil {
 		return err
@@ -125,7 +124,7 @@ func (a *ArtifactDownloader) Download(ctx context.Context) error {
 // We want to have as few S3 clients as possible, as creating them is kind of an expensive operation
 // But it's also theoretically possible that we'll have multiple artifacts with different S3 buckets, and each
 // S3Client only applies to one bucket, so we need to store the S3 clients in a map, one for each bucket
-func (a *ArtifactDownloader) generateS3Clients(artifacts []*api.Artifact) (map[string]*s3.S3, error) {
+func (a *Downloader) generateS3Clients(artifacts []*api.Artifact) (map[string]*s3.S3, error) {
 	s3Clients := map[string]*s3.S3{}
 
 	for _, artifact := range artifacts {
@@ -151,7 +150,7 @@ type downloader interface {
 	Start(context.Context) error
 }
 
-func (a *ArtifactDownloader) createDownloader(artifact *api.Artifact, path, destination string, s3Clients map[string]*s3.S3) downloader {
+func (a *Downloader) createDownloader(artifact *api.Artifact, path, destination string, s3Clients map[string]*s3.S3) downloader {
 	// Handle downloading from S3, GS, RT, or Azure
 	switch {
 	case strings.HasPrefix(artifact.UploadDestination, "s3://"):
@@ -183,8 +182,8 @@ func (a *ArtifactDownloader) createDownloader(artifact *api.Artifact, path, dest
 			DebugHTTP:   a.conf.DebugHTTP,
 		})
 
-	case iartifact.IsAzureBlobPath(artifact.UploadDestination):
-		return iartifact.NewAzureBlobDownloader(a.logger, iartifact.AzureBlobDownloaderConfig{
+	case IsAzureBlobPath(artifact.UploadDestination):
+		return NewAzureBlobDownloader(a.logger, AzureBlobDownloaderConfig{
 			Path:        path,
 			Repository:  artifact.UploadDestination,
 			Destination: destination,

agent/artifact_downloader_test.go → internal/artifact/downloader_test.go

@@ -1,4 +1,4 @@
-package agent
+package artifact
 
 import (
 	"context"
@@ -40,7 +40,7 @@ func TestArtifactDownloaderConnectsToEndpoint(t *testing.T) {
 		Token:    "llamasforever",
 	})
 
-	d := NewArtifactDownloader(logger.Discard, ac, ArtifactDownloaderConfig{
+	d := NewDownloader(logger.Discard, ac, DownloaderConfig{
 		BuildID: "my-build",
 	})
 

agent/form_uploader.go → internal/artifact/form_uploader.go

@@ -1,4 +1,4 @@
-package agent
+package artifact
 
 import (
 	"bytes"