diff --git a/cmd/osbuild-upload-aws/main.go b/cmd/osbuild-upload-aws/main.go index 61e86f1fac1..36425fb554d 100644 --- a/cmd/osbuild-upload-aws/main.go +++ b/cmd/osbuild-upload-aws/main.go @@ -58,7 +58,7 @@ func main() { bootModePtr = &bootMode } - ami, err := a.Register(imageName, bucketName, keyName, share, arch, bootModePtr) + ami, err := a.Register(imageName, bucketName, keyName, imageName, share, arch, bootModePtr) if err != nil { println(err.Error()) return diff --git a/cmd/osbuild-worker/jobimpl-osbuild.go b/cmd/osbuild-worker/jobimpl-osbuild.go index 5aadc73624d..14f077d6958 100644 --- a/cmd/osbuild-worker/jobimpl-osbuild.go +++ b/cmd/osbuild-worker/jobimpl-osbuild.go @@ -636,7 +636,12 @@ 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) + // giving a Name tag value is optional, default to ImageName if missing. + tagName := jobTarget.ImageName + if targetOptions.TagName != "" { + tagName = targetOptions.TagName + } + ami, err := a.Register(jobTarget.ImageName, bucket, targetOptions.Key, tagName, targetOptions.ShareWithAccounts, arch.Current().String(), targetOptions.BootMode) if err != nil { targetResult.TargetError = clienterrors.WorkerClientError(clienterrors.ErrorImportingImage, err.Error(), nil) break diff --git a/internal/boot/aws.go b/internal/boot/aws.go index 4b155510efb..2d39a534252 100644 --- a/internal/boot/aws.go +++ b/internal/boot/aws.go @@ -99,7 +99,7 @@ func UploadImageToAWS(c *awsCredentials, imagePath string, imageName string) err if err != nil { return fmt.Errorf("cannot upload the image: %v", err) } - _, err = uploader.Register(imageName, c.Bucket, imageName, nil, arch.Current().String(), nil) + _, err = uploader.Register(imageName, c.Bucket, imageName, imageName, nil, arch.Current().String(), nil) if err != nil { return fmt.Errorf("cannot register the image: %v", err) } diff --git a/internal/cloud/awscloud/awscloud.go b/internal/cloud/awscloud/awscloud.go index 419d0b4dc7e..8cc555fe90b 100644 --- a/internal/cloud/awscloud/awscloud.go +++ b/internal/cloud/awscloud/awscloud.go @@ -212,7 +212,7 @@ func WaitUntilImportSnapshotTaskCompletedWithContext(c *ec2.EC2, ctx aws.Context // 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) { +func (a *AWS) Register(imageName string, bucket string, s3key string, tagName string, shareWith []string, rpmArch string, bootMode *string) (*string, error) { rpmArchToEC2Arch := map[string]string{ "x86_64": "x86_64", "aarch64": "arm64", @@ -229,15 +229,18 @@ func (a *AWS) Register(name, bucket, key string, shareWith []string, rpmArch str } } - logrus.Infof("[AWS] 📥 Importing snapshot from image: %s/%s", bucket, key) - snapshotDescription := fmt.Sprintf("Image Builder AWS Import of %s", name) + // Note that the key value is only used in the upload transition phase in S3. It's deleted once the snapshot is done + // importing. So it's content has not much impact. The key is not conveyed anymore, compute a unique one based on + // the nameTag's value. + logrus.Infof("[AWS] 📥 Importing snapshot from image: %s/%s", bucket, s3key) + snapshotDescription := fmt.Sprintf("Image Builder AWS Import of %s", imageName) importTaskOutput, err := a.ec2.ImportSnapshot( &ec2.ImportSnapshotInput{ Description: aws.String(snapshotDescription), DiskContainer: &ec2.SnapshotDiskContainer{ UserBucket: &ec2.UserBucket{ S3Bucket: aws.String(bucket), - S3Key: aws.String(key), + S3Key: aws.String(s3key), }, }, }, @@ -261,10 +264,10 @@ func (a *AWS) Register(name, bucket, key string, shareWith []string, rpmArch str } // we no longer need the object in s3, let's just delete it - logrus.Infof("[AWS] 🧹 Deleting image from S3: %s/%s", bucket, key) + logrus.Infof("[AWS] 🧹 Deleting image from S3: %s/%s", bucket, s3key) _, err = a.s3.DeleteObject(&s3.DeleteObjectInput{ Bucket: aws.String(bucket), - Key: aws.String(key), + Key: aws.String(s3key), }) if err != nil { return nil, err @@ -290,7 +293,11 @@ func (a *AWS) Register(name, bucket, key string, shareWith []string, rpmArch str Tags: []*ec2.Tag{ { Key: aws.String("Name"), - Value: aws.String(name), + Value: aws.String(tagName), + }, + { + Key: aws.String("Build-by"), + Value: aws.String("osbuild-composer"), }, }, }, @@ -306,7 +313,7 @@ func (a *AWS) Register(name, bucket, key string, shareWith []string, rpmArch str Architecture: aws.String(ec2Arch), BootMode: bootMode, VirtualizationType: aws.String("hvm"), - Name: aws.String(name), + Name: aws.String(imageName), RootDeviceName: aws.String("/dev/sda1"), EnaSupport: aws.Bool(true), BlockDeviceMappings: []*ec2.BlockDeviceMapping{ @@ -332,7 +339,11 @@ func (a *AWS) Register(name, bucket, key string, shareWith []string, rpmArch str Tags: []*ec2.Tag{ { Key: aws.String("Name"), - Value: aws.String(name), + Value: aws.String(tagName), + }, + { + Key: aws.String("Build-by"), + Value: aws.String("osbuild-composer"), }, }, }, diff --git a/internal/cloudapi/v2/imagerequest.go b/internal/cloudapi/v2/imagerequest.go index fdfaac346f6..e8ae6b03ad1 100644 --- a/internal/cloudapi/v2/imagerequest.go +++ b/internal/cloudapi/v2/imagerequest.go @@ -47,11 +47,8 @@ func newAWSTarget(options UploadOptions, imageType distro.ImageType) (*target.Ta return nil, HTTPError(ErrorJSONUnMarshallingError) } - // For service maintenance, images are discovered by the "Name:composer-api-*" - // tag filter. Currently all image names in the service are generated, so they're - // guaranteed to be unique as well. If users are ever allowed to name their images, - // an extra tag should be added. - key := fmt.Sprintf("composer-api-%s", uuid.New().String()) + targetUuid := uuid.New() + key := fmt.Sprintf("composer-api-%s", targetUuid.String()) var amiBootMode *string switch imageType.BootMode() { @@ -63,16 +60,23 @@ func newAWSTarget(options UploadOptions, imageType distro.ImageType) (*target.Ta amiBootMode = common.ToPtr(ec2.BootModeValuesLegacyBios) } + // tag name is optional, not providing one will make the Name tag default to the ImageName + tagName := "" + if awsUploadOptions.SnapshotName != nil { + tagName = *awsUploadOptions.SnapshotName + } + t := target.NewAWSTarget(&target.AWSTargetOptions{ Region: awsUploadOptions.Region, Key: key, + TagName: tagName, ShareWithAccounts: awsUploadOptions.ShareWithAccounts, BootMode: amiBootMode, }) + t.ImageName = key if awsUploadOptions.SnapshotName != nil { - t.ImageName = *awsUploadOptions.SnapshotName - } else { - t.ImageName = key + // When registering an image the ImageName is used to set the ami's name. AMIs names must be unique. + t.ImageName = fmt.Sprintf("%s-%s", *awsUploadOptions.SnapshotName, targetUuid.String()) } t.OsbuildArtifact.ExportFilename = imageType.Filename() return t, nil diff --git a/internal/target/aws_target.go b/internal/target/aws_target.go index b6921a1a4be..06979151125 100644 --- a/internal/target/aws_target.go +++ b/internal/target/aws_target.go @@ -14,6 +14,9 @@ type AWSTargetOptions struct { Key string `json:"key"` ShareWithAccounts []string `json:"shareWithAccounts"` + // Optional tag value if not provided will default to the ImageName's value. + TagName string `json:"tag_name,omitempty"` + // Boot mode of the AMI (optional) // Supported values: // - ec2.BootModeValuesLegacyBios diff --git a/test/cases/api/aws.sh b/test/cases/api/aws.sh index 1a3a30bfe44..cfead3b8d01 100644 --- a/test/cases/api/aws.sh +++ b/test/cases/api/aws.sh @@ -103,7 +103,7 @@ function checkUploadStatusOptions() { function verify() { $AWS_CMD ec2 describe-images \ --owners self \ - --filters Name=name,Values="$AWS_SNAPSHOT_NAME" \ + --filters Name=name,Values="$AWS_SNAPSHOT_NAME-*" \ > "$WORKDIR/ami.json" AMI_IMAGE_ID=$(jq -r '.Images[].ImageId' "$WORKDIR/ami.json")