From a3937e99cec48a9451aff4b5cb80dbc05f5f0402 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Hozza?= Date: Wed, 6 Aug 2025 13:46:59 +0200 Subject: [PATCH] internal/awscloud: use AWS.Register() from osbuild/images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: TomΓ‘Ε‘ Hozza --- cmd/osbuild-upload-aws/main.go | 42 +++-- cmd/osbuild-worker/jobimpl-osbuild.go | 27 +++- internal/cloud/awscloud/awscloud.go | 160 ------------------- internal/cloud/awscloud/awscloud_test.go | 32 ---- internal/cloud/awscloud/client-interfaces.go | 2 - internal/cloud/awscloud/mocks_test.go | 14 -- 6 files changed, 52 insertions(+), 225 deletions(-) diff --git a/cmd/osbuild-upload-aws/main.go b/cmd/osbuild-upload-aws/main.go index 63716b116..1e246981e 100644 --- a/cmd/osbuild-upload-aws/main.go +++ b/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 bootMode string + var archOpt 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(&bootMode, "boot-mode", "", "boot mode (legacy-bios, uefi, uefi-preferred)") + flag.StringVar(&archOpt, "arch", "", "arch (x86_64 or aarch64)") + 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 - if bootMode != "" { - bootModePtr = &bootMode - } - - ami, err := a.Register(imageName, bucketName, keyName, share, arch, bootModePtr) + imgArch, err := arch.FromString(archOpt) 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) } diff --git a/cmd/osbuild-worker/jobimpl-osbuild.go b/cmd/osbuild-worker/jobimpl-osbuild.go index 0fc793333..7f60fab7b 100644 --- a/cmd/osbuild-worker/jobimpl-osbuild.go +++ b/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, } diff --git a/internal/cloud/awscloud/awscloud.go b/internal/cloud/awscloud/awscloud.go index f0fc936a8..e6980b96c 100644 --- a/internal/cloud/awscloud/awscloud.go +++ b/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( diff --git a/internal/cloud/awscloud/awscloud_test.go b/internal/cloud/awscloud/awscloud_test.go index 02f5d0d07..88adbdf23 100644 --- a/internal/cloud/awscloud/awscloud_test.go +++ b/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) diff --git a/internal/cloud/awscloud/client-interfaces.go b/internal/cloud/awscloud/client-interfaces.go index a7568e782..c8680c3f4 100644 --- a/internal/cloud/awscloud/client-interfaces.go +++ b/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 diff --git a/internal/cloud/awscloud/mocks_test.go b/internal/cloud/awscloud/mocks_test.go index 4e324ec15..77fcf7553 100644 --- a/internal/cloud/awscloud/mocks_test.go +++ b/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