internal/awscloud: use AWS.Register() from osbuild/images

Signed-off-by: Tomáš Hozza <thozza@redhat.com>

cmd/osbuild-upload-aws/main.go

@@ -4,7 +4,11 @@ import (
 	"flag"
 	"fmt"
 
+	ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
+	"github.com/osbuild/images/pkg/arch"
+	"github.com/osbuild/images/pkg/platform"
 	"github.com/osbuild/osbuild-composer/internal/cloud/awscloud"
+	"github.com/osbuild/osbuild-composer/internal/common"
 )
 
 func main() {
@@ -17,8 +21,8 @@ func main() {
 	var filename string
 	var imageName string
 	var shareWith string
-	var arch string
+	var archOpt string
-	var bootMode string
+	var bootModeOpt string
 	flag.StringVar(&accessKeyID, "access-key-id", "", "access key ID")
 	flag.StringVar(&secretAccessKey, "secret-access-key", "", "secret access key")
 	flag.StringVar(&sessionToken, "session-token", "", "session token")
@@ -28,8 +32,8 @@ func main() {
 	flag.StringVar(&filename, "image", "", "image file to upload")
 	flag.StringVar(&imageName, "name", "", "AMI name")
 	flag.StringVar(&shareWith, "account-id", "", "account id to share image with")
-	flag.StringVar(&arch, "arch", "", "arch (x86_64 or aarch64)")
+	flag.StringVar(&archOpt, "arch", "", "arch (x86_64 or aarch64)")
-	flag.StringVar(&bootMode, "boot-mode", "", "boot mode (legacy-bios, uefi, uefi-preferred)")
+	flag.StringVar(&bootModeOpt, "boot-mode", "", "boot mode (legacy-bios, uefi, uefi-preferred)")
 	flag.Parse()
 
 	a, err := awscloud.New(region, accessKeyID, secretAccessKey, sessionToken)
@@ -51,16 +55,32 @@ func main() {
 		share = append(share, shareWith)
 	}
 
-	var bootModePtr *string
+	imgArch, err := arch.FromString(archOpt)
-	if bootMode != "" {
-		bootModePtr = &bootMode
-	}
-
-	ami, err := a.Register(imageName, bucketName, keyName, share, arch, bootModePtr)
 	if err != nil {
 		println(err.Error())
 		return
 	}
 
-	fmt.Printf("AMI registered: %s\n", *ami)
+	var bootMode *platform.BootMode
+	switch bootModeOpt {
+	case string(ec2types.BootModeValuesLegacyBios):
+		bootMode = common.ToPtr(platform.BOOT_LEGACY)
+	case string(ec2types.BootModeValuesUefi):
+		bootMode = common.ToPtr(platform.BOOT_UEFI)
+	case string(ec2types.BootModeValuesUefiPreferred):
+		bootMode = common.ToPtr(platform.BOOT_HYBRID)
+	case "":
+		// do nothing
+	default:
+		println("Unknown boot mode %q, must be one of: legacy-bios, uefi, uefi-preferred", bootModeOpt)
+		return
+	}
+
+	ami, _, err := a.Register(imageName, bucketName, keyName, share, imgArch, bootMode, nil)
+	if err != nil {
+		println(err.Error())
+		return
+	}
+
+	fmt.Printf("AMI registered: %s\n", ami)
 }

cmd/osbuild-worker/jobimpl-osbuild.go

@@ -23,13 +23,16 @@ import (
 	"github.com/osbuild/images/pkg/osbuild"
 	"github.com/osbuild/images/pkg/sbom"
 
+	"github.com/osbuild/osbuild-composer/internal/common"
 	"github.com/osbuild/osbuild-composer/internal/upload/oci"
 	"github.com/osbuild/osbuild-composer/internal/upload/pulp"
 
 	"github.com/google/uuid"
 	"github.com/sirupsen/logrus"
 
+	ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
 	"github.com/osbuild/images/pkg/cloud/azure"
+	"github.com/osbuild/images/pkg/platform"
 	"github.com/osbuild/images/pkg/upload/koji"
 	"github.com/osbuild/osbuild-composer/internal/cloud/awscloud"
 	"github.com/osbuild/osbuild-composer/internal/cloud/gcp"
@@ -725,18 +728,30 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error {
 				break
 			}
 
-			ami, err := a.Register(jobTarget.ImageName, bucket, targetOptions.Key, targetOptions.ShareWithAccounts, arch.Current().String(), targetOptions.BootMode)
+			// XXX: We should adjust the target options type for the boot mode to replace the following workaround
+			// but that will require a backwards compatibility layer for old worker / new composer.
+			var bootMode *platform.BootMode
+			if targetOptions.BootMode != nil {
+				switch *targetOptions.BootMode {
+				case string(ec2types.BootModeValuesLegacyBios):
+					bootMode = common.ToPtr(platform.BOOT_LEGACY)
+				case string(ec2types.BootModeValuesUefi):
+					bootMode = common.ToPtr(platform.BOOT_UEFI)
+				case string(ec2types.BootModeValuesUefiPreferred):
+					bootMode = common.ToPtr(platform.BOOT_HYBRID)
+				default:
+					logrus.Warnf("Unknown boot mode %q, using default", *targetOptions.BootMode)
+				}
+			}
+
+			ami, _, err := a.Register(jobTarget.ImageName, bucket, targetOptions.Key, targetOptions.ShareWithAccounts, arch.Current(), bootMode, nil)
 			if err != nil {
 				targetResult.TargetError = clienterrors.New(clienterrors.ErrorImportingImage, err.Error(), nil)
 				break
 			}
 
-			if ami == nil {
-				targetResult.TargetError = clienterrors.New(clienterrors.ErrorImportingImage, "No ami returned", nil)
-				break
-			}
 			targetResult.Options = &target.AWSTargetResultOptions{
-				Ami:    *ami,
+				Ami:    ami,
 				Region: targetOptions.Region,
 			}
 

internal/cloud/awscloud/awscloud.go

@@ -217,166 +217,6 @@ func NewForEndpointFromFile(filename, endpoint, region, caBundle string, skipSSL
 	return newAwsFromCredsWithEndpoint(config.WithSharedCredentialsFiles([]string{filename, "default"}), region, endpoint, caBundle, skipSSLVerification, imagesAWS)
 }
 
-// Register is a function that imports a snapshot, waits for the snapshot to
-// fully import, tags the snapshot, cleans up the image in S3, and registers
-// an AMI in AWS.
-// The caller can optionally specify the boot mode of the AMI. If the boot
-// mode is not specified, then the instances launched from this AMI use the
-// default boot mode value of the instance type.
-func (a *AWS) Register(name, bucket, key string, shareWith []string, rpmArch string, bootMode *string) (*string, error) {
-	rpmArchToEC2Arch := map[string]ec2types.ArchitectureValues{
-		"x86_64":  ec2types.ArchitectureValuesX8664,
-		"aarch64": ec2types.ArchitectureValuesArm64,
-	}
-
-	ec2Arch, validArch := rpmArchToEC2Arch[rpmArch]
-	if !validArch {
-		return nil, fmt.Errorf("ec2 doesn't support the following arch: %s", rpmArch)
-	}
-
-	bootModeToEC2BootMode := map[string]ec2types.BootModeValues{
-		string(ec2types.BootModeValuesLegacyBios):    ec2types.BootModeValuesLegacyBios,
-		string(ec2types.BootModeValuesUefi):          ec2types.BootModeValuesUefi,
-		string(ec2types.BootModeValuesUefiPreferred): ec2types.BootModeValuesUefiPreferred,
-	}
-	ec2BootMode := ec2types.BootModeValuesUefiPreferred
-	if bootMode != nil {
-		bm, validBootMode := bootModeToEC2BootMode[*bootMode]
-		if !validBootMode {
-			return nil, fmt.Errorf("ec2 doesn't support the following boot mode: %s", *bootMode)
-		}
-		ec2BootMode = bm
-	}
-
-	logrus.Infof("[AWS] 📥 Importing snapshot from image: %s/%s", bucket, key)
-	snapshotDescription := fmt.Sprintf("Image Builder AWS Import of %s", name)
-	importTaskOutput, err := a.ec2.ImportSnapshot(
-		context.Background(),
-		&ec2.ImportSnapshotInput{
-			Description: aws.String(snapshotDescription),
-			DiskContainer: &ec2types.SnapshotDiskContainer{
-				UserBucket: &ec2types.UserBucket{
-					S3Bucket: aws.String(bucket),
-					S3Key:    aws.String(key),
-				},
-			},
-		},
-	)
-	if err != nil {
-		logrus.Warnf("[AWS] error importing snapshot: %s", err)
-		return nil, err
-	}
-
-	logrus.Infof("[AWS] 🚚 Waiting for snapshot to finish importing: %s", *importTaskOutput.ImportTaskId)
-
-	// importTaskOutput.
-	snapWaiter := ec2.NewSnapshotImportedWaiter(a.ec2)
-	snapWaitOutput, err := snapWaiter.WaitForOutput(
-		context.Background(),
-		&ec2.DescribeImportSnapshotTasksInput{
-			ImportTaskIds: []string{
-				*importTaskOutput.ImportTaskId,
-			},
-		},
-		time.Hour*24,
-	)
-	if err != nil {
-		return nil, err
-	}
-
-	snapshotTaskStatus := *snapWaitOutput.ImportSnapshotTasks[0].SnapshotTaskDetail.Status
-	if snapshotTaskStatus != "completed" {
-		return nil, fmt.Errorf("Unable to import snapshot, task result: %v, msg: %v", snapshotTaskStatus, *snapWaitOutput.ImportSnapshotTasks[0].SnapshotTaskDetail.StatusMessage)
-	}
-
-	// we no longer need the object in s3, let's just delete it
-	logrus.Infof("[AWS] 🧹 Deleting image from S3: %s/%s", bucket, key)
-	_, err = a.s3.DeleteObject(
-		context.Background(),
-		&s3.DeleteObjectInput{
-			Bucket: aws.String(bucket),
-			Key:    aws.String(key),
-		},
-	)
-	if err != nil {
-		return nil, err
-	}
-
-	snapshotID := *snapWaitOutput.ImportSnapshotTasks[0].SnapshotTaskDetail.SnapshotId
-	// Tag the snapshot with the image name.
-	_, err = a.ec2.CreateTags(
-		context.Background(),
-		&ec2.CreateTagsInput{
-			Resources: []string{snapshotID},
-			Tags: []ec2types.Tag{
-				{
-					Key:   aws.String("Name"),
-					Value: aws.String(name),
-				},
-			},
-		},
-	)
-	if err != nil {
-		return nil, err
-	}
-
-	logrus.Infof("[AWS] 📋 Registering AMI from imported snapshot: %s", snapshotID)
-	registerOutput, err := a.ec2.RegisterImage(
-		context.Background(),
-		&ec2.RegisterImageInput{
-			Architecture:       ec2Arch,
-			BootMode:           ec2BootMode,
-			VirtualizationType: aws.String("hvm"),
-			Name:               aws.String(name),
-			RootDeviceName:     aws.String("/dev/sda1"),
-			EnaSupport:         aws.Bool(true),
-			BlockDeviceMappings: []ec2types.BlockDeviceMapping{
-				{
-					DeviceName: aws.String("/dev/sda1"),
-					Ebs: &ec2types.EbsBlockDevice{
-						SnapshotId: aws.String(snapshotID),
-					},
-				},
-			},
-		},
-	)
-	if err != nil {
-		return nil, err
-	}
-
-	logrus.Infof("[AWS] 🎉 AMI registered: %s", *registerOutput.ImageId)
-
-	// Tag the image with the image name.
-	_, err = a.ec2.CreateTags(
-		context.Background(),
-		&ec2.CreateTagsInput{
-			Resources: []string{*registerOutput.ImageId},
-			Tags: []ec2types.Tag{
-				{
-					Key:   aws.String("Name"),
-					Value: aws.String(name),
-				},
-			},
-		},
-	)
-	if err != nil {
-		return nil, err
-	}
-
-	if len(shareWith) > 0 {
-		err = a.shareSnapshot(snapshotID, shareWith)
-		if err != nil {
-			return nil, err
-		}
-		err = a.shareImage(registerOutput.ImageId, shareWith)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	return registerOutput.ImageId, nil
-}
-
 // target region is determined by the region configured in the aws session
 func (a *AWS) CopyImage(name, ami, sourceRegion string) (string, error) {
 	result, err := a.ec2.CopyImage(

internal/cloud/awscloud/awscloud_test.go

@@ -6,40 +6,8 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/osbuild/osbuild-composer/internal/cloud/awscloud"
-	"github.com/osbuild/osbuild-composer/internal/common"
 )
 
-func TestEC2Register(t *testing.T) {
-	m := newEc2Mock(t)
-	aws := awscloud.NewForTest(m, nil, &s3mock{t, "bucket", "object-key"}, nil, nil)
-	require.NotNil(t, aws)
-
-	// Image without share
-	imageId, err := aws.Register("image-name", "bucket", "object-key", []string{}, "x86_64", common.ToPtr("uefi-preferred"))
-	require.NoError(t, err)
-	require.Equal(t, "image-id", *imageId)
-	// basic image import operations
-	require.Equal(t, 1, m.calledFn["ImportSnapshot"])
-	require.Equal(t, 1, m.calledFn["RegisterImage"])
-	// sharing operations
-	require.Equal(t, 0, m.calledFn["ModifyImageAttribute"])
-	require.Equal(t, 0, m.calledFn["ModifySnapshotAttribute"])
-
-	// Image with share
-	imageId, err = aws.Register("image-name", "bucket", "object-key", []string{"share-with-user"}, "x86_64", common.ToPtr("uefi-preferred"))
-	require.NoError(t, err)
-	require.Equal(t, "image-id", *imageId)
-	// basic image import operations
-	require.Equal(t, 2, m.calledFn["ImportSnapshot"])
-	require.Equal(t, 2, m.calledFn["RegisterImage"])
-	// sharing operations
-	require.Equal(t, 1, m.calledFn["ModifyImageAttribute"])
-	require.Equal(t, 1, m.calledFn["ModifySnapshotAttribute"])
-
-	// 2 snapshots, 2 images
-	require.Equal(t, 4, m.calledFn["CreateTags"])
-}
-
 func TestEC2CopyImage(t *testing.T) {
 	m := newEc2Mock(t)
 	aws := awscloud.NewForTest(m, nil, &s3mock{t, "bucket", "object-key"}, nil, nil)

internal/cloud/awscloud/client-interfaces.go

@@ -42,7 +42,6 @@ type EC2 interface {
 
 	// Images
 	CopyImage(context.Context, *ec2.CopyImageInput, ...func(*ec2.Options)) (*ec2.CopyImageOutput, error)
-	RegisterImage(context.Context, *ec2.RegisterImageInput, ...func(*ec2.Options)) (*ec2.RegisterImageOutput, error)
 	DeregisterImage(context.Context, *ec2.DeregisterImageInput, ...func(*ec2.Options)) (*ec2.DeregisterImageOutput, error)
 	DescribeImages(context.Context, *ec2.DescribeImagesInput, ...func(*ec2.Options)) (*ec2.DescribeImagesOutput, error)
 	ModifyImageAttribute(context.Context, *ec2.ModifyImageAttributeInput, ...func(*ec2.Options)) (*ec2.ModifyImageAttributeOutput, error)
@@ -50,7 +49,6 @@ type EC2 interface {
 	// Snapshots
 	DeleteSnapshot(context.Context, *ec2.DeleteSnapshotInput, ...func(*ec2.Options)) (*ec2.DeleteSnapshotOutput, error)
 	DescribeImportSnapshotTasks(context.Context, *ec2.DescribeImportSnapshotTasksInput, ...func(*ec2.Options)) (*ec2.DescribeImportSnapshotTasksOutput, error)
-	ImportSnapshot(context.Context, *ec2.ImportSnapshotInput, ...func(*ec2.Options)) (*ec2.ImportSnapshotOutput, error)
 	ModifySnapshotAttribute(context.Context, *ec2.ModifySnapshotAttributeInput, ...func(*ec2.Options)) (*ec2.ModifySnapshotAttributeOutput, error)
 
 	// Tags

internal/cloud/awscloud/mocks_test.go

@@ -331,13 +331,6 @@ func (m *ec2mock) CopyImage(ctx context.Context, input *ec2.CopyImageInput, optf
 	}, nil
 }
 
-func (m *ec2mock) RegisterImage(ctx context.Context, input *ec2.RegisterImageInput, optfns ...func(*ec2.Options)) (*ec2.RegisterImageOutput, error) {
-	m.calledFn["RegisterImage"] += 1
-	return &ec2.RegisterImageOutput{
-		ImageId: &m.imageId,
-	}, nil
-}
-
 func (m *ec2mock) DeregisterImage(ctx context.Context, input *ec2.DeregisterImageInput, optfns ...func(*ec2.Options)) (*ec2.DeregisterImageOutput, error) {
 	m.calledFn["DeregisterImage"] += 1
 	return nil, nil
@@ -387,13 +380,6 @@ func (m *ec2mock) DescribeImportSnapshotTasks(ctx context.Context, input *ec2.De
 	}, nil
 }
 
-func (m *ec2mock) ImportSnapshot(ctx context.Context, input *ec2.ImportSnapshotInput, optfns ...func(*ec2.Options)) (*ec2.ImportSnapshotOutput, error) {
-	m.calledFn["ImportSnapshot"] += 1
-	return &ec2.ImportSnapshotOutput{
-		ImportTaskId: aws.String("import-task-id"),
-	}, nil
-}
-
 func (m *ec2mock) ModifySnapshotAttribute(ctx context.Context, input *ec2.ModifySnapshotAttributeInput, optfns ...func(*ec2.Options)) (*ec2.ModifySnapshotAttributeOutput, error) {
 	m.calledFn["ModifySnapshotAttribute"] += 1
 	return nil, nil