main: add upload support directly to build

This commit adds support to upload the build image directly to
the target cloud. Currently only ami/AWS is supported.

If the cloud specific configuration is given at the commandline
and the image type is a cloud image the cloud upload will happen
automatically (just like with bib). Incomplete upload config is
an error.

.spellcheck-en-custom.txt

@@ -1,3 +1,4 @@
+ami
 backend
 bootc
 bootmode

README.md

@@ -114,6 +114,22 @@ It is possible to generate spdx based SBOM (software bill of materials)
 documents as part of the build. Just pass `--with-sbom` and
 it will put them into the output directory.
 
+### Cloud integration
+
+When building an image type that can be uploaded to the cloud
+(e.g. an "ami") image-builder will automatically upload if
+all cloud parameters are provided, e.g.
+```
+$ image-builder build ami --distro centos-9 \
+    --aws-region us-east-1 \
+	--aws-bucket example-bucket \
+	--aws-ami-name my-image-1
+```
+Images can also be uploaded with the `image-builder upload` command
+after they are built.
+
+
+
 ### Filtering
 
 When listing images, it is possible to filter:

cmd/image-builder/build.go

@@ -21,11 +21,6 @@ func buildImage(pbar progress.ProgressBar, res *imagefilter.Result, osbuildManif
 		opts = &buildOptions{}
 	}
 
-	// XXX: support output filename via commandline (c.f.
-	//   https://github.com/osbuild/images/pull/1039)
-	if opts.OutputDir == "" {
-		opts.OutputDir = outputNameFor(res)
-	}
 	if opts.WriteManifest {
 		p := filepath.Join(opts.OutputDir, fmt.Sprintf("%s.osbuild-manifest.json", outputNameFor(res)))
 		if err := os.MkdirAll(filepath.Dir(p), 0755); err != nil {

cmd/image-builder/main.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -9,13 +10,11 @@ import (
 	"path/filepath"
 	"syscall"
 
-	"github.com/cheggaaa/pb/v3"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 
 	"github.com/osbuild/bootc-image-builder/bib/pkg/progress"
 	"github.com/osbuild/images/pkg/arch"
-	"github.com/osbuild/images/pkg/cloud/awscloud"
 	"github.com/osbuild/images/pkg/imagefilter"
 	"github.com/osbuild/images/pkg/osbuild"
 	"github.com/osbuild/images/pkg/ostree"
@@ -176,66 +175,6 @@ func progressFromCmd(cmd *cobra.Command) (progress.ProgressBar, error) {
 	return progress.New(progressType)
 }
 
-var awscloudNewUploader = awscloud.NewUploader
-
-func cmdUploadAWS(cmd *cobra.Command, args []string) error {
-	amiName, err := cmd.Flags().GetString("aws-ami-name")
-	if err != nil {
-		return err
-	}
-	bucketName, err := cmd.Flags().GetString("aws-bucket")
-	if err != nil {
-		return err
-	}
-	region, err := cmd.Flags().GetString("aws-region")
-	if err != nil {
-		return err
-	}
-
-	rawDiskPath := args[0]
-	// XXX: can we actually inspect the image or leave some artifacts?
-	if filepath.Ext(rawDiskPath) != ".raw" {
-		return fmt.Errorf("expecting a raw disk ending with '.raw', got %q", filepath.Base(rawDiskPath))
-	}
-
-	uploader, err := awscloudNewUploader(region, bucketName, amiName, nil)
-	if err != nil {
-		return err
-	}
-	f, err := os.Open(rawDiskPath)
-	if err != nil {
-		return err
-	}
-	defer f.Close()
-
-	// setup basic progress
-	st, err := f.Stat()
-	if err != nil {
-		return fmt.Errorf("cannot stat upload: %v", err)
-	}
-	pbar := pb.New64(st.Size())
-	pbar.Set(pb.Bytes, true)
-	pbar.SetWriter(osStdout)
-	r := pbar.NewProxyReader(f)
-	pbar.Start()
-	defer pbar.Finish()
-
-	return uploader.UploadAndRegister(r, osStderr)
-}
-
-func cmdUpload(cmd *cobra.Command, args []string) error {
-	uploadTo, err := cmd.Flags().GetString("to")
-	if err != nil {
-		return err
-	}
-	switch uploadTo {
-	case "aws":
-		return cmdUploadAWS(cmd, args)
-	default:
-		return fmt.Errorf("unsupported cloud %q", uploadTo)
-	}
-}
-
 func cmdBuild(cmd *cobra.Command, args []string) error {
 	cacheDir, err := cmd.Flags().GetString("cache")
 	if err != nil {
@@ -274,13 +213,49 @@ func cmdBuild(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
+	var uploadUnsupported *UploadTypeUnsupportedError
+	var missingUploadConfig *MissingUploadConfigError
+	uploader, err := uploaderFor(cmd, res.ImgType.Name())
+	if err != nil && !errors.As(err, &missingUploadConfig) && !errors.As(err, &uploadUnsupported) {
+		return err
+	}
+	if missingUploadConfig != nil && !missingUploadConfig.allMissing {
+		return fmt.Errorf("partial upload config provided: %w", err)
+	}
+
+	if uploader != nil {
+		pbar.SetPulseMsgf("Checking cloud access")
+		if err := uploaderCheckWithProgress(pbar, uploader); err != nil {
+			return err
+		}
+	}
+
+	// XXX: support output filename via commandline (c.f.
+	//   https://github.com/osbuild/images/pull/1039)
+	if outputDir == "" {
+		outputDir = outputNameFor(res)
+	}
 	buildOpts := &buildOptions{
 		OutputDir:     outputDir,
 		StoreDir:      cacheDir,
 		WriteManifest: withManifest,
 	}
 	pbar.SetPulseMsgf("Image building step")
-	return buildImage(pbar, res, mf.Bytes(), buildOpts)
+	if err := buildImage(pbar, res, mf.Bytes(), buildOpts); err != nil {
+		return err
+	}
+
+	if uploader != nil {
+		// XXX: integrate better into the progress, see bib
+		pbar.Stop()
+		imagePath := filepath.Join(outputDir, res.ImgType.Name(), res.ImgType.Filename())
+
+		if err := uploadImageWithProgress(uploader, imagePath); err != nil {
+			return err
+		}
+	}
+
+	return nil
 }
 
 func cmdDescribeImg(cmd *cobra.Command, args []string) error {
@@ -374,7 +349,6 @@ operating systems like Fedora, CentOS and RHEL with easy customizations support.
 	uploadCmd.Flags().String("aws-bucket", "", "target S3 bucket name for intermediate storage when creating AMI (only for type=ami)")
 	uploadCmd.Flags().String("aws-region", "", "target region for AWS uploads (only for type=ami)")
 	rootCmd.AddCommand(uploadCmd)
-	uploadCmd.Flags().String("to", "", "upload to the given cloud")
 
 	buildCmd := &cobra.Command{
 		Use:          "build <image-type>",
@@ -391,6 +365,10 @@ operating systems like Fedora, CentOS and RHEL with easy customizations support.
 	// (see https://github.com/osbuild/bootc-image-builder/pull/790/commits/5cec7ffd8a526e2ca1e8ada0ea18f927695dfe43)
 	buildCmd.Flags().String("progress", "auto", "type of progress bar to use (e.g. verbose,term)")
 	rootCmd.AddCommand(buildCmd)
+	buildCmd.Flags().AddFlagSet(uploadCmd.Flags())
+	// add after the rest of the uploadCmd flag set is added to avoid
+	// that build gets a "--to" parameter
+	uploadCmd.Flags().String("to", "", "upload to the given cloud")
 
 	// XXX: add --format=json too?
 	describeImgCmd := &cobra.Command{

cmd/image-builder/main_test.go

@@ -15,10 +15,7 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 
-	"github.com/osbuild/images/pkg/cloud"
-	"github.com/osbuild/images/pkg/cloud/awscloud"
 	testrepos "github.com/osbuild/images/test/data/repositories"
 
 	main "github.com/osbuild/image-builder-cli/cmd/image-builder"
@@ -602,64 +599,3 @@ func TestProgressFromCmd(t *testing.T) {
 		assert.Equal(t, tc.expectedProgress, fmt.Sprintf("%T", pbar))
 	}
 }
-
-type fakeAwsUploader struct {
-	checkCalls int
-
-	uploadAndRegisterRead  bytes.Buffer
-	uploadAndRegisterCalls int
-}
-
-var _ = cloud.Uploader(&fakeAwsUploader{})
-
-func (fa *fakeAwsUploader) Check(status io.Writer) error {
-	fa.checkCalls++
-	return nil
-}
-
-func (fa *fakeAwsUploader) UploadAndRegister(r io.Reader, status io.Writer) error {
-	fa.uploadAndRegisterCalls++
-	_, err := io.Copy(&fa.uploadAndRegisterRead, r)
-	return err
-}
-
-func TestUploadWithAWSMock(t *testing.T) {
-	fakeDiskContent := "fake-raw-img"
-
-	fakeImageFilePath := filepath.Join(t.TempDir(), "disk.raw")
-	err := os.WriteFile(fakeImageFilePath, []byte(fakeDiskContent), 0644)
-	assert.NoError(t, err)
-
-	var regionName, bucketName, amiName string
-	var fa fakeAwsUploader
-	restore := main.MockAwscloudNewUploader(func(region string, bucket string, ami string, opts *awscloud.UploaderOptions) (cloud.Uploader, error) {
-		regionName = region
-		bucketName = bucket
-		amiName = ami
-		return &fa, nil
-	})
-	defer restore()
-
-	var fakeStdout bytes.Buffer
-	restore = main.MockOsStdout(&fakeStdout)
-	defer restore()
-
-	restore = main.MockOsArgs([]string{
-		"upload",
-		"--to=aws",
-		"--aws-region=aws-region-1",
-		"--aws-bucket=aws-bucket-2",
-		"--aws-ami-name=aws-ami-3",
-		fakeImageFilePath,
-	})
-	err = main.Run()
-	require.NoError(t, err)
-
-	assert.Equal(t, regionName, "aws-region-1")
-	assert.Equal(t, bucketName, "aws-bucket-2")
-	assert.Equal(t, amiName, "aws-ami-3")
-
-	assert.Equal(t, fakeDiskContent, fa.uploadAndRegisterRead.String())
-	// progress was rendered
-	assert.Contains(t, fakeStdout.String(), "--] 100.00%")
-}

cmd/image-builder/upload.go

@@ -0,0 +1,132 @@
+package main
+
+import (
+	"bufio"
+	"fmt"
+	"io"
+	"os"
+
+	"github.com/cheggaaa/pb/v3"
+	"github.com/spf13/cobra"
+
+	"github.com/osbuild/bootc-image-builder/bib/pkg/progress"
+	"github.com/osbuild/images/pkg/cloud"
+	"github.com/osbuild/images/pkg/cloud/awscloud"
+)
+
+type MissingUploadConfigError struct {
+	missing    []string
+	allMissing bool
+}
+
+func (e *MissingUploadConfigError) Error() string {
+	return fmt.Sprintf("missing upload configuration: %q", e.missing)
+}
+
+type UploadTypeUnsupportedError struct {
+	typ string
+}
+
+func (e *UploadTypeUnsupportedError) Error() string {
+	return fmt.Sprintf("unsupported upload type %q", e.typ)
+}
+
+var awscloudNewUploader = awscloud.NewUploader
+
+func uploadImageWithProgress(uploader cloud.Uploader, imagePath string) error {
+	f, err := os.Open(imagePath)
+	if err != nil {
+		return err
+	}
+	defer f.Close()
+
+	// setup basic progress
+	st, err := f.Stat()
+	if err != nil {
+		return fmt.Errorf("cannot stat upload: %v", err)
+	}
+	pbar := pb.New64(st.Size())
+	pbar.Set(pb.Bytes, true)
+	pbar.SetWriter(osStdout)
+	r := pbar.NewProxyReader(f)
+	pbar.Start()
+	defer pbar.Finish()
+
+	return uploader.UploadAndRegister(r, osStderr)
+}
+
+func uploaderCheckWithProgress(pbar progress.ProgressBar, uploader cloud.Uploader) error {
+	pr, pw := io.Pipe()
+	defer pw.Close()
+
+	go func() {
+		scanner := bufio.NewScanner(pr)
+		for scanner.Scan() {
+			pbar.SetMessagef("%s", scanner.Text())
+		}
+	}()
+	return uploader.Check(pw)
+}
+
+func uploaderFor(cmd *cobra.Command, typeOrCloud string) (cloud.Uploader, error) {
+	switch typeOrCloud {
+	case "ami", "aws":
+		return uploaderForCmdAWS(cmd)
+	default:
+		return nil, &UploadTypeUnsupportedError{typeOrCloud}
+	}
+
+}
+
+func uploaderForCmdAWS(cmd *cobra.Command) (cloud.Uploader, error) {
+	amiName, err := cmd.Flags().GetString("aws-ami-name")
+	if err != nil {
+		return nil, err
+	}
+	bucketName, err := cmd.Flags().GetString("aws-bucket")
+	if err != nil {
+		return nil, err
+	}
+	region, err := cmd.Flags().GetString("aws-region")
+	if err != nil {
+		return nil, err
+	}
+
+	var missing []string
+	requiredArgs := []string{"aws-ami-name", "aws-bucket", "aws-region"}
+	for _, argName := range requiredArgs {
+		arg, err := cmd.Flags().GetString(argName)
+		if err != nil {
+			return nil, err
+		}
+		if arg == "" {
+			missing = append(missing, fmt.Sprintf("--%s", argName))
+		}
+	}
+	if len(missing) > 0 {
+		return nil, &MissingUploadConfigError{
+			missing:    missing,
+			allMissing: len(missing) == len(requiredArgs),
+		}
+	}
+
+	return awscloudNewUploader(region, bucketName, amiName, nil)
+}
+
+func cmdUpload(cmd *cobra.Command, args []string) error {
+	uploadTo, err := cmd.Flags().GetString("to")
+	if err != nil {
+		return err
+	}
+	if uploadTo == "" {
+		return fmt.Errorf("missing --to parameter, try --to=aws")
+	}
+
+	imagePath := args[0]
+	uploader, err := uploaderFor(cmd, uploadTo)
+	if err != nil {
+		return err
+	}
+
+	return uploadImageWithProgress(uploader, imagePath)
+}

cmd/image-builder/upload_test.go

@@ -0,0 +1,246 @@
+package main_test
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/osbuild/images/pkg/cloud"
+	"github.com/osbuild/images/pkg/cloud/awscloud"
+
+	main "github.com/osbuild/image-builder-cli/cmd/image-builder"
+	"github.com/osbuild/image-builder-cli/internal/testutil"
+)
+
+type fakeAwsUploader struct {
+	checkCalls int
+
+	uploadAndRegisterRead  bytes.Buffer
+	uploadAndRegisterCalls int
+	uploadAndRegisterErr   error
+}
+
+var _ = cloud.Uploader(&fakeAwsUploader{})
+
+func (fa *fakeAwsUploader) Check(status io.Writer) error {
+	fa.checkCalls++
+	return nil
+}
+
+func (fa *fakeAwsUploader) UploadAndRegister(r io.Reader, status io.Writer) error {
+	fa.uploadAndRegisterCalls++
+	_, err := io.Copy(&fa.uploadAndRegisterRead, r)
+	if err != nil {
+		panic(err)
+	}
+	return fa.uploadAndRegisterErr
+}
+
+func TestUploadWithAWSMock(t *testing.T) {
+	fakeDiskContent := "fake-raw-img"
+
+	fakeImageFilePath := filepath.Join(t.TempDir(), "disk.raw")
+	err := os.WriteFile(fakeImageFilePath, []byte(fakeDiskContent), 0644)
+	assert.NoError(t, err)
+
+	var regionName, bucketName, amiName string
+	var fa fakeAwsUploader
+	restore := main.MockAwscloudNewUploader(func(region string, bucket string, ami string, opts *awscloud.UploaderOptions) (cloud.Uploader, error) {
+		regionName = region
+		bucketName = bucket
+		amiName = ami
+		return &fa, nil
+	})
+	defer restore()
+
+	var fakeStdout bytes.Buffer
+	restore = main.MockOsStdout(&fakeStdout)
+	defer restore()
+
+	restore = main.MockOsArgs([]string{
+		"upload",
+		"--to=aws",
+		"--aws-region=aws-region-1",
+		"--aws-bucket=aws-bucket-2",
+		"--aws-ami-name=aws-ami-3",
+		fakeImageFilePath,
+	})
+	defer restore()
+
+	err = main.Run()
+	require.NoError(t, err)
+
+	assert.Equal(t, regionName, "aws-region-1")
+	assert.Equal(t, bucketName, "aws-bucket-2")
+	assert.Equal(t, amiName, "aws-ami-3")
+
+	assert.Equal(t, 0, fa.checkCalls)
+	assert.Equal(t, 1, fa.uploadAndRegisterCalls)
+	assert.Equal(t, fakeDiskContent, fa.uploadAndRegisterRead.String())
+	// progress was rendered
+	assert.Contains(t, fakeStdout.String(), "--] 100.00%")
+}
+
+func TestUploadCmdlineErrors(t *testing.T) {
+	for _, tc := range []struct {
+		cmdline     []string
+		expectedErr string
+	}{
+		{
+			nil,
+			`missing --to parameter, try --to=aws`,
+		}, {
+			[]string{"--to=aws"},
+			`missing upload configuration: ["--aws-ami-name" "--aws-bucket" "--aws-region"]`,
+		},
+		{
+			[]string{"--to=aws", "--aws-ami-name=1"},
+			`missing upload configuration: ["--aws-bucket" "--aws-region"]`,
+		},
+		{
+			[]string{"--to=aws", "--aws-ami-name=1", "--aws-bucket=2"},
+			`missing upload configuration: ["--aws-region"]`,
+		},
+	} {
+		t.Run(strings.Join(tc.cmdline, ","), func(t *testing.T) {
+			cmd := append([]string{"upload"}, tc.cmdline...)
+			cmd = append(cmd, "/path/to/some/image")
+			restore := main.MockOsArgs(cmd)
+			defer restore()
+
+			err := main.Run()
+			require.EqualError(t, err, tc.expectedErr)
+		})
+	}
+}
+
+var fakeOsbuildScriptAmiFmt = `#!/bin/sh -e
+cat - > "$0".stdin
+mkdir -p %[1]s/ami
+echo -n %[2]s > %[1]s/ami/image.raw
+`
+
+func TestBuildAndUploadWithAWSMock(t *testing.T) {
+	if testing.Short() {
+		t.Skip("manifest generation takes a while")
+	}
+	if !hasDepsolveDnf() {
+		t.Skip("no osbuild-depsolve-dnf binary found")
+	}
+
+	var regionName, bucketName, amiName string
+	var fa fakeAwsUploader
+	restore := main.MockAwscloudNewUploader(func(region string, bucket string, ami string, opts *awscloud.UploaderOptions) (cloud.Uploader, error) {
+		regionName = region
+		bucketName = bucket
+		amiName = ami
+		return &fa, nil
+	})
+	defer restore()
+
+	fakeDiskContent := "fake-raw-img"
+	outputDir := t.TempDir()
+	fakeOsbuildScript := fmt.Sprintf(fakeOsbuildScriptAmiFmt, outputDir, fakeDiskContent)
+	fakeOsbuildCmd := testutil.MockCommand(t, "osbuild", fakeOsbuildScript)
+	defer fakeOsbuildCmd.Restore()
+
+	var fakeStdout bytes.Buffer
+	restore = main.MockOsStdout(&fakeStdout)
+	defer restore()
+
+	restore = main.MockOsArgs([]string{
+		"build",
+		"--output-dir", outputDir,
+		"--aws-region=aws-region-1",
+		"--aws-bucket=aws-bucket-2",
+		"--aws-ami-name=aws-ami-3",
+		"ami",
+		"--distro=centos-9",
+	})
+	defer restore()
+
+	err := main.Run()
+	require.NoError(t, err)
+
+	assert.Equal(t, regionName, "aws-region-1")
+	assert.Equal(t, bucketName, "aws-bucket-2")
+	assert.Equal(t, amiName, "aws-ami-3")
+	assert.Equal(t, 1, fa.checkCalls)
+	assert.Equal(t, 1, fa.uploadAndRegisterCalls)
+	assert.Equal(t, fakeDiskContent, fa.uploadAndRegisterRead.String())
+}
+
+func TestBuildAmiButNotUpload(t *testing.T) {
+	if testing.Short() {
+		t.Skip("manifest generation takes a while")
+	}
+	if !hasDepsolveDnf() {
+		t.Skip("no osbuild-depsolve-dnf binary found")
+	}
+
+	fa := fakeAwsUploader{
+		uploadAndRegisterErr: fmt.Errorf("upload should not be called"),
+	}
+	restore := main.MockAwscloudNewUploader(func(region string, bucket string, ami string, opts *awscloud.UploaderOptions) (cloud.Uploader, error) {
+		return &fa, nil
+	})
+	defer restore()
+
+	fakeDiskContent := "fake-raw-img"
+	outputDir := t.TempDir()
+	fakeOsbuildScript := fmt.Sprintf(fakeOsbuildScriptAmiFmt, outputDir, fakeDiskContent)
+	fakeOsbuildCmd := testutil.MockCommand(t, "osbuild", fakeOsbuildScript)
+	defer fakeOsbuildCmd.Restore()
+
+	var fakeStdout bytes.Buffer
+	restore = main.MockOsStdout(&fakeStdout)
+	defer restore()
+
+	restore = main.MockOsArgs([]string{
+		"build",
+		"--output-dir", outputDir,
+		"ami",
+		"--distro=centos-9",
+	})
+	defer restore()
+
+	err := main.Run()
+	require.NoError(t, err)
+
+	assert.Equal(t, 0, fa.uploadAndRegisterCalls)
+}
+
+func TestBuildAndUploadWithAWSPartialCmdlineErrors(t *testing.T) {
+	if testing.Short() {
+		t.Skip("manifest generation takes a while")
+	}
+	if !hasDepsolveDnf() {
+		t.Skip("no osbuild-depsolve-dnf binary found")
+	}
+
+	fakeDiskContent := "fake-raw-img"
+	outputDir := t.TempDir()
+	fakeOsbuildScript := fmt.Sprintf(fakeOsbuildScriptAmiFmt, outputDir, fakeDiskContent)
+	fakeOsbuildCmd := testutil.MockCommand(t, "osbuild", fakeOsbuildScript)
+	defer fakeOsbuildCmd.Restore()
+
+	restore := main.MockOsArgs([]string{
+		"build",
+		"--output-dir", outputDir,
+		// note that --aws-{ami-name,bucket} is missing
+		"--aws-region=aws-region-1",
+		"ami",
+		"--distro=centos-9",
+	})
+	defer restore()
+
+	err := main.Run()
+	assert.EqualError(t, err, `partial upload config provided: missing upload configuration: ["--aws-ami-name" "--aws-bucket"]`)
+}