From 6f464949f5c1fec45c27ef2c66ca729eba8ac8f5 Mon Sep 17 00:00:00 2001 From: Tomas Hozza Date: Fri, 17 Jun 2022 18:09:29 +0200 Subject: [PATCH] target: move `Filename` from target options to `Target` The filename of the image as produced by osbuild for a given export is currently set in each target options type in the `Filename` struct member. However, the value is not really specific to any target type, but to the specific export used for the target. For this reason move the value form target type options to the `Target` struct inside a new struct `OsbuildArtifact` under the name`ExportFilename`. The backward compatibility with older implementations of the composer and workers is kept on the JSON (Un)mashaling level, where the JSON object is always a super-set of the old and new way of providing the export filename in the Target. --- cmd/osbuild-store-dump/main.go | 2 +- cmd/osbuild-worker/jobimpl-osbuild.go | 18 +- internal/cloudapi/v2/handler.go | 18 +- internal/cloudapi/v2/server.go | 2 +- internal/cloudapi/v2/v2_koji_test.go | 16 +- internal/store/json_test.go | 144 +++++++++------ internal/target/aws_target.go | 2 - internal/target/azure_image_target.go | 1 - internal/target/azure_target.go | 1 - internal/target/container_target.go | 1 - internal/target/gcp_target.go | 1 - internal/target/koji_target.go | 2 - internal/target/oci_target.go | 1 - internal/target/target.go | 231 +++++++++++++++++++++--- internal/target/target_test.go | 243 ++++++++++++++++++++++++++ internal/target/vmware_target.go | 1 - internal/weldr/api_test.go | 4 +- internal/weldr/upload.go | 8 +- 18 files changed, 567 insertions(+), 129 deletions(-) create mode 100644 internal/target/target_test.go diff --git a/cmd/osbuild-store-dump/main.go b/cmd/osbuild-store-dump/main.go index f6c323680..fd3f54a3d 100644 --- a/cmd/osbuild-store-dump/main.go +++ b/cmd/osbuild-store-dump/main.go @@ -106,7 +106,6 @@ func main() { } awsTarget := target.NewAWSTarget( &target.AWSTargetOptions{ - Filename: "image.ami", Region: "far-away-1", AccessKeyID: "MyKey", SecretAccessKey: "MySecret", @@ -117,6 +116,7 @@ func main() { awsTarget.Uuid = id1 awsTarget.ImageName = "My Image" awsTarget.Created = time.Now() + awsTarget.OsbuildArtifact.ExportFilename = "image.ami" d := fedora.NewF35() a, err := d.GetArch(distro.X86_64ArchName) diff --git a/cmd/osbuild-worker/jobimpl-osbuild.go b/cmd/osbuild-worker/jobimpl-osbuild.go index 6a9f2ca88..be048edee 100644 --- a/cmd/osbuild-worker/jobimpl-osbuild.go +++ b/cmd/osbuild-worker/jobimpl-osbuild.go @@ -390,7 +390,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { imageName := jobTarget.ImageName + ".vmdk" imagePath := path.Join(tempDirectory, imageName) - exportedImagePath := path.Join(outputDirectory, exportPath, targetOptions.Filename) + exportedImagePath := path.Join(outputDirectory, exportPath, jobTarget.OsbuildArtifact.ExportFilename) err = os.Symlink(exportedImagePath, imagePath) if err != nil { targetResult.TargetError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidConfig, err.Error()) @@ -420,7 +420,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { if impl.AWSBucket != "" { bucket = impl.AWSBucket } - _, err = a.Upload(path.Join(outputDirectory, exportPath, targetOptions.Filename), bucket, key) + _, err = a.Upload(path.Join(outputDirectory, exportPath, jobTarget.OsbuildArtifact.ExportFilename), bucket, key) if err != nil { targetResult.TargetError = clienterrors.WorkerClientError(clienterrors.ErrorUploadingImage, err.Error()) break @@ -449,7 +449,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { break } - url, targetError := uploadToS3(a, outputDirectory, exportPath, bucket, targetOptions.Key, targetOptions.Filename) + url, targetError := uploadToS3(a, outputDirectory, exportPath, bucket, targetOptions.Key, jobTarget.OsbuildArtifact.ExportFilename) if targetError != nil { targetResult.TargetError = targetError break @@ -475,7 +475,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { const azureMaxUploadGoroutines = 4 err = azureStorageClient.UploadPageBlob( metadata, - path.Join(outputDirectory, exportPath, targetOptions.Filename), + path.Join(outputDirectory, exportPath, jobTarget.OsbuildArtifact.ExportFilename), azureMaxUploadGoroutines, ) @@ -495,7 +495,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { } logWithId.Infof("[GCP] 🚀 Uploading image to: %s/%s", targetOptions.Bucket, targetOptions.Object) - _, err = g.StorageObjectUpload(ctx, path.Join(outputDirectory, exportPath, targetOptions.Filename), + _, err = g.StorageObjectUpload(ctx, path.Join(outputDirectory, exportPath, jobTarget.OsbuildArtifact.ExportFilename), targetOptions.Bucket, targetOptions.Object, map[string]string{gcp.MetadataKeyImageName: jobTarget.ImageName}) if err != nil { targetResult.TargetError = clienterrors.WorkerClientError(clienterrors.ErrorUploadingImage, err.Error()) @@ -623,7 +623,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { ContainerName: storageContainer, BlobName: blobName, }, - path.Join(outputDirectory, exportPath, targetOptions.Filename), + path.Join(outputDirectory, exportPath, jobTarget.OsbuildArtifact.ExportFilename), azure.DefaultUploadThreads, ) if err != nil { @@ -680,7 +680,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { } }() - file, err := os.Open(path.Join(outputDirectory, exportPath, targetOptions.Filename)) + file, err := os.Open(path.Join(outputDirectory, exportPath, jobTarget.OsbuildArtifact.ExportFilename)) if err != nil { targetResult.TargetError = clienterrors.WorkerClientError(clienterrors.ErrorKojiBuild, fmt.Sprintf("failed to open the image for reading: %v", err)) break @@ -716,7 +716,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { } logWithId.Info("[OCI] 🔑 Logged in OCI") logWithId.Info("[OCI] ⬆ Uploading the image") - file, err := os.Open(path.Join(outputDirectory, exportPath, targetOptions.Filename)) + file, err := os.Open(path.Join(outputDirectory, exportPath, jobTarget.OsbuildArtifact.ExportFilename)) if err != nil { targetResult.TargetError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidConfig, err.Error()) break @@ -758,7 +758,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { client.TlsVerify = *targetOptions.TlsVerify } - sourcePath := path.Join(outputDirectory, exportPath, targetOptions.Filename) + sourcePath := path.Join(outputDirectory, exportPath, jobTarget.OsbuildArtifact.ExportFilename) // TODO: get the container type from the metadata of the osbuild job sourceRef := fmt.Sprintf("oci-archive:%s", sourcePath) diff --git a/internal/cloudapi/v2/handler.go b/internal/cloudapi/v2/handler.go index 1a089589d..a48388625 100644 --- a/internal/cloudapi/v2/handler.go +++ b/internal/cloudapi/v2/handler.go @@ -328,7 +328,6 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { // an extra tag should be added. key := fmt.Sprintf("composer-api-%s", uuid.New().String()) t := target.NewAWSTarget(&target.AWSTargetOptions{ - Filename: imageType.Filename(), Region: awsUploadOptions.Region, Key: key, ShareWithAccounts: awsUploadOptions.ShareWithAccounts, @@ -338,6 +337,7 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { } else { t.ImageName = key } + t.OsbuildArtifact.ExportFilename = imageType.Filename() irTarget = t case ImageTypesGuestImage: @@ -363,11 +363,11 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { key := fmt.Sprintf("composer-api-%s", uuid.New().String()) t := target.NewAWSS3Target(&target.AWSS3TargetOptions{ - Filename: imageType.Filename(), - Region: awsS3UploadOptions.Region, - Key: key, + Region: awsS3UploadOptions.Region, + Key: key, }) t.ImageName = key + t.OsbuildArtifact.ExportFilename = imageType.Filename() irTarget = t case ImageTypesGcp: @@ -388,10 +388,9 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { imageName := fmt.Sprintf("composer-api-%s", uuid.New().String()) t := target.NewGCPTarget(&target.GCPTargetOptions{ - Filename: imageType.Filename(), - Region: gcpUploadOptions.Region, - Os: imageType.Arch().Distro().Name(), // not exposed in cloudapi - Bucket: gcpUploadOptions.Bucket, + Region: gcpUploadOptions.Region, + Os: imageType.Arch().Distro().Name(), // not exposed in cloudapi + Bucket: gcpUploadOptions.Bucket, // the uploaded object must have a valid extension Object: fmt.Sprintf("%s.tar.gz", imageName), ShareWithAccounts: share, @@ -402,6 +401,7 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { } else { t.ImageName = imageName } + t.OsbuildArtifact.ExportFilename = imageType.Filename() irTarget = t case ImageTypesAzure: @@ -417,7 +417,6 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { return HTTPError(ErrorJSONUnMarshallingError) } t := target.NewAzureImageTarget(&target.AzureImageTargetOptions{ - Filename: imageType.Filename(), TenantID: azureUploadOptions.TenantId, Location: azureUploadOptions.Location, SubscriptionID: azureUploadOptions.SubscriptionId, @@ -430,6 +429,7 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { // if ImageName wasn't given, generate a random one t.ImageName = fmt.Sprintf("composer-api-%s", uuid.New().String()) } + t.OsbuildArtifact.ExportFilename = imageType.Filename() irTarget = t default: diff --git a/internal/cloudapi/v2/server.go b/internal/cloudapi/v2/server.go index 40ca19ed6..7e5570066 100644 --- a/internal/cloudapi/v2/server.go +++ b/internal/cloudapi/v2/server.go @@ -181,8 +181,8 @@ func (s *Server) enqueueKojiCompose(taskID uint64, server, name, version, releas kojiTarget := target.NewKojiTarget(&target.KojiTargetOptions{ Server: server, UploadDirectory: kojiDirectory, - Filename: ir.imageType.Filename(), }) + kojiTarget.OsbuildArtifact.ExportFilename = ir.imageType.Filename() kojiTarget.ImageName = kojiFilename buildID, err := s.workers.EnqueueOSBuildAsDependency(ir.arch.Name(), &worker.OSBuildJob{ diff --git a/internal/cloudapi/v2/v2_koji_test.go b/internal/cloudapi/v2/v2_koji_test.go index 3092add74..4293142b2 100644 --- a/internal/cloudapi/v2/v2_koji_test.go +++ b/internal/cloudapi/v2/v2_koji_test.go @@ -477,7 +477,7 @@ func TestKojiCompose(t *testing.T) { require.NoError(t, err) jobTarget := osbuildJob.Targets[0].Options.(*target.KojiTargetOptions) require.Equal(t, "koji.example.com", jobTarget.Server) - require.Equal(t, "test.img", jobTarget.Filename) + require.Equal(t, "test.img", osbuildJob.Targets[0].OsbuildArtifact.ExportFilename) require.Equal(t, fmt.Sprintf("%s-%s-%s.%s.img", name, version, release, test_distro.TestArch3Name), osbuildJob.Targets[0].ImageName) require.NotEmpty(t, jobTarget.UploadDirectory) @@ -596,14 +596,14 @@ func TestKojiJobTypeValidation(t *testing.T) { buildJobIDs := make([]uuid.UUID, nImages) filenames := make([]string, nImages) for idx := 0; idx < nImages; idx++ { - fname := fmt.Sprintf("image-file-%04d", idx) + kojiTarget := target.NewKojiTarget(&target.KojiTargetOptions{ + Server: "test-server", + UploadDirectory: "koji-server-test-dir", + }) + kojiTarget.OsbuildArtifact.ExportFilename = fmt.Sprintf("image-file-%04d", idx) buildJob := worker.OSBuildJob{ ImageName: fmt.Sprintf("build-job-%04d", idx), - Targets: []*target.Target{target.NewKojiTarget(&target.KojiTargetOptions{ - Server: "test-server", - UploadDirectory: "koji-server-test-dir", - Filename: fname, - })}, + Targets: []*target.Target{kojiTarget}, // Add an empty manifest as a static job argument to make the test pass. // Becasue of a bug in the API, the test was passing even without // any manifest being attached to the job (static or dynamic). @@ -616,7 +616,7 @@ func TestKojiJobTypeValidation(t *testing.T) { buildJobs[idx] = buildJob buildJobIDs[idx] = buildID - filenames[idx] = fname + filenames[idx] = kojiTarget.OsbuildArtifact.ExportFilename } finalizeJob := worker.KojiFinalizeJob{ diff --git a/internal/store/json_test.go b/internal/store/json_test.go index 04ded98ef..bec573da9 100644 --- a/internal/store/json_test.go +++ b/internal/store/json_test.go @@ -1019,11 +1019,13 @@ func Test_newComposeV0(t *testing.T) { Name: target.TargetNameAWS, Created: MustParseTime("2020-08-12T09:21:44.427717205-07:00"), Status: common.IBWaiting, + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: "disk.qcow2", + }, Options: target.AWSTargetOptions{ - Region: "us-east-1", - Bucket: "bucket", - Key: "key", - Filename: "disk.qcow2", + Region: "us-east-1", + Bucket: "bucket", + Key: "key", }, }, }, @@ -1049,11 +1051,13 @@ func Test_newComposeV0(t *testing.T) { Name: target.TargetNameAWS, Created: MustParseTime("2020-08-12T09:21:44.427717205-07:00"), Status: common.IBWaiting, + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: "disk.qcow2", + }, Options: target.AWSTargetOptions{ - Region: "us-east-1", - Bucket: "bucket", - Key: "key", - Filename: "disk.qcow2", + Region: "us-east-1", + Bucket: "bucket", + Key: "key", }, }, }, @@ -1123,11 +1127,13 @@ func Test_newComposeFromV0(t *testing.T) { Name: target.TargetNameAWS, Created: MustParseTime("2020-08-12T09:21:44.427717205-07:00"), Status: common.IBWaiting, + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: "disk.qcow2", + }, Options: target.AWSTargetOptions{ - Region: "us-east-1", - Bucket: "bucket", - Key: "key", - Filename: "disk.qcow2", + Region: "us-east-1", + Bucket: "bucket", + Key: "key", }, }, }, @@ -1153,11 +1159,13 @@ func Test_newComposeFromV0(t *testing.T) { Name: target.TargetNameAWS, Created: MustParseTime("2020-08-12T09:21:44.427717205-07:00"), Status: common.IBWaiting, + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: "disk.qcow2", + }, Options: target.AWSTargetOptions{ - Region: "us-east-1", - Bucket: "bucket", - Key: "key", - Filename: "disk.qcow2", + Region: "us-east-1", + Bucket: "bucket", + Key: "key", }, }, }, @@ -1220,11 +1228,13 @@ func Test_newComposesV0(t *testing.T) { Name: target.TargetNameAWS, Created: MustParseTime("2020-08-12T09:21:44.427717205-07:00"), Status: common.IBWaiting, + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: "disk.qcow2", + }, Options: target.AWSTargetOptions{ - Region: "us-east-1", - Bucket: "bucket", - Key: "key", - Filename: "disk.qcow2", + Region: "us-east-1", + Bucket: "bucket", + Key: "key", }, }, }, @@ -1249,11 +1259,13 @@ func Test_newComposesV0(t *testing.T) { Name: target.TargetNameAWS, Created: MustParseTime("2020-08-12T09:21:44.427717205-07:00"), Status: common.IBWaiting, + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: "disk.qcow2", + }, Options: target.AWSTargetOptions{ - Region: "us-east-1", - Bucket: "bucket", - Key: "key", - Filename: "disk.qcow2", + Region: "us-east-1", + Bucket: "bucket", + Key: "key", }, }, }, @@ -1270,7 +1282,7 @@ func Test_newComposesV0(t *testing.T) { uuid.MustParse("f53b49c0-d321-447e-8ab8-6e827891e3f0"): { Blueprint: &bp, ImageBuilds: []imageBuildV0{ - imageBuildV0{ + { ID: 0, ImageType: test_distro.TestImageTypeName, Manifest: []byte("JSON MANIFEST GOES HERE"), @@ -1281,11 +1293,13 @@ func Test_newComposesV0(t *testing.T) { Name: target.TargetNameAWS, Created: MustParseTime("2020-08-12T09:21:44.427717205-07:00"), Status: common.IBWaiting, + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: "disk.qcow2", + }, Options: target.AWSTargetOptions{ - Region: "us-east-1", - Bucket: "bucket", - Key: "key", - Filename: "disk.qcow2", + Region: "us-east-1", + Bucket: "bucket", + Key: "key", }, }, }, @@ -1302,7 +1316,7 @@ func Test_newComposesV0(t *testing.T) { uuid.MustParse("14c454d0-26f3-4a56-8ceb-a5673aaba686"): { Blueprint: &bp, ImageBuilds: []imageBuildV0{ - imageBuildV0{ + { ID: 0, ImageType: test_distro.TestImageTypeName, Manifest: []byte("JSON MANIFEST GOES HERE"), @@ -1313,11 +1327,13 @@ func Test_newComposesV0(t *testing.T) { Name: target.TargetNameAWS, Created: MustParseTime("2020-08-12T09:21:44.427717205-07:00"), Status: common.IBWaiting, + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: "disk.qcow2", + }, Options: target.AWSTargetOptions{ - Region: "us-east-1", - Bucket: "bucket", - Key: "key", - Filename: "disk.qcow2", + Region: "us-east-1", + Bucket: "bucket", + Key: "key", }, }, }, @@ -1386,11 +1402,13 @@ func Test_newComposesFromV0(t *testing.T) { Name: target.TargetNameAWS, Created: MustParseTime("2020-08-12T09:21:44.427717205-07:00"), Status: common.IBWaiting, + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: "disk.qcow2", + }, Options: target.AWSTargetOptions{ - Region: "us-east-1", - Bucket: "bucket", - Key: "key", - Filename: "disk.qcow2", + Region: "us-east-1", + Bucket: "bucket", + Key: "key", }, }, }, @@ -1417,11 +1435,13 @@ func Test_newComposesFromV0(t *testing.T) { Name: target.TargetNameAWS, Created: MustParseTime("2020-08-12T09:21:44.427717205-07:00"), Status: common.IBWaiting, + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: "disk.qcow2", + }, Options: target.AWSTargetOptions{ - Region: "us-east-1", - Bucket: "bucket", - Key: "key", - Filename: "disk.qcow2", + Region: "us-east-1", + Bucket: "bucket", + Key: "key", }, }, }, @@ -1449,11 +1469,13 @@ func Test_newComposesFromV0(t *testing.T) { Name: target.TargetNameAWS, Created: MustParseTime("2020-08-12T09:21:44.427717205-07:00"), Status: common.IBWaiting, + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: "disk.qcow2", + }, Options: target.AWSTargetOptions{ - Region: "us-east-1", - Bucket: "bucket", - Key: "key", - Filename: "disk.qcow2", + Region: "us-east-1", + Bucket: "bucket", + Key: "key", }, }, }, @@ -1479,11 +1501,13 @@ func Test_newComposesFromV0(t *testing.T) { Name: target.TargetNameAWS, Created: MustParseTime("2020-08-12T09:21:44.427717205-07:00"), Status: common.IBWaiting, + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: "disk.qcow2", + }, Options: target.AWSTargetOptions{ - Region: "us-east-1", - Bucket: "bucket", - Key: "key", - Filename: "disk.qcow2", + Region: "us-east-1", + Bucket: "bucket", + Key: "key", }, }, }, @@ -1542,11 +1566,13 @@ func Test_newImageBuildFromV0(t *testing.T) { Name: target.TargetNameAWS, Created: MustParseTime("2020-08-12T09:21:44.427717205-07:00"), Status: common.IBWaiting, + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: "disk.qcow2", + }, Options: target.AWSTargetOptions{ - Region: "us-east-1", - Bucket: "bucket", - Key: "key", - Filename: "disk.qcow2", + Region: "us-east-1", + Bucket: "bucket", + Key: "key", }, }, }, @@ -1568,11 +1594,13 @@ func Test_newImageBuildFromV0(t *testing.T) { Name: target.TargetNameAWS, Created: MustParseTime("2020-08-12T09:21:44.427717205-07:00"), Status: common.IBWaiting, + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: "disk.qcow2", + }, Options: target.AWSTargetOptions{ - Region: "us-east-1", - Bucket: "bucket", - Key: "key", - Filename: "disk.qcow2", + Region: "us-east-1", + Bucket: "bucket", + Key: "key", }, }, }, diff --git a/internal/target/aws_target.go b/internal/target/aws_target.go index 20f10d477..4ae93c0fc 100644 --- a/internal/target/aws_target.go +++ b/internal/target/aws_target.go @@ -6,7 +6,6 @@ const ( ) type AWSTargetOptions struct { - Filename string `json:"filename"` Region string `json:"region"` AccessKeyID string `json:"accessKeyID"` SecretAccessKey string `json:"secretAccessKey"` @@ -34,7 +33,6 @@ func NewAWSTargetResult(options *AWSTargetResultOptions) *TargetResult { } type AWSS3TargetOptions struct { - Filename string `json:"filename"` Region string `json:"region"` AccessKeyID string `json:"accessKeyID"` SecretAccessKey string `json:"secretAccessKey"` diff --git a/internal/target/azure_image_target.go b/internal/target/azure_image_target.go index 00f7c41c1..e68e4050e 100644 --- a/internal/target/azure_image_target.go +++ b/internal/target/azure_image_target.go @@ -3,7 +3,6 @@ package target const TargetNameAzureImage TargetName = "org.osbuild.azure.image" type AzureImageTargetOptions struct { - Filename string `json:"filename"` TenantID string `json:"tenant_id"` Location string `json:"location"` SubscriptionID string `json:"subscription_id"` diff --git a/internal/target/azure_target.go b/internal/target/azure_target.go index be9713282..1cb9dd65e 100644 --- a/internal/target/azure_target.go +++ b/internal/target/azure_target.go @@ -3,7 +3,6 @@ package target const TargetNameAzure TargetName = "org.osbuild.azure" type AzureTargetOptions struct { - Filename string `json:"filename"` StorageAccount string `json:"storageAccount"` StorageAccessKey string `json:"storageAccessKey"` Container string `json:"container"` diff --git a/internal/target/container_target.go b/internal/target/container_target.go index 9ec5a81c1..56dc8d8b7 100644 --- a/internal/target/container_target.go +++ b/internal/target/container_target.go @@ -3,7 +3,6 @@ package target const TargetNameContainer TargetName = "org.osbuild.container" type ContainerTargetOptions struct { - Filename string `json:"filename"` Reference string `json:"reference"` Username string `json:"username,omitempty"` diff --git a/internal/target/gcp_target.go b/internal/target/gcp_target.go index ac17b357c..008591537 100644 --- a/internal/target/gcp_target.go +++ b/internal/target/gcp_target.go @@ -3,7 +3,6 @@ package target const TargetNameGCP TargetName = "org.osbuild.gcp" type GCPTargetOptions struct { - Filename string `json:"filename"` Region string `json:"region"` Os string `json:"os"` // not exposed in cloudapi for now Bucket string `json:"bucket"` diff --git a/internal/target/koji_target.go b/internal/target/koji_target.go index 5a7844043..72e58de4d 100644 --- a/internal/target/koji_target.go +++ b/internal/target/koji_target.go @@ -3,8 +3,6 @@ package target const TargetNameKoji TargetName = "org.osbuild.koji" type KojiTargetOptions struct { - // Filename of the image as produced by osbuild for a given export - Filename string `json:"filename"` UploadDirectory string `json:"upload_directory"` Server string `json:"server"` } diff --git a/internal/target/oci_target.go b/internal/target/oci_target.go index cf8e4adcf..ffe4a9bd7 100644 --- a/internal/target/oci_target.go +++ b/internal/target/oci_target.go @@ -6,7 +6,6 @@ type OCITargetOptions struct { User string `json:"user"` Tenancy string `json:"tenancy"` Region string `json:"region"` - Filename string `json:"filename"` Fingerprint string `json:"fingerprint"` PrivateKey string `json:"private_key"` Bucket string `json:"bucket"` diff --git a/internal/target/target.go b/internal/target/target.go index 745f7b8c7..e9bf1ed13 100644 --- a/internal/target/target.go +++ b/internal/target/target.go @@ -11,13 +11,25 @@ import ( type TargetName string +// OsbuildArtifact represents a configuration to produce osbuild artifact +// specific to a target. +type OsbuildArtifact struct { + // Filename of the image as produced by osbuild for a given export + ExportFilename string `json:"export_filename"` +} + type Target struct { - Uuid uuid.UUID `json:"uuid"` - ImageName string `json:"image_name"` // Desired name of the image in the target environment - Name TargetName `json:"name"` // Name of the specific target type - Created time.Time `json:"created"` - Status common.ImageBuildState `json:"status"` - Options TargetOptions `json:"options"` // Target type specific options + Uuid uuid.UUID `json:"uuid"` + // Desired name of the image in the target environment + ImageName string `json:"image_name"` + // Name of the specific target type + Name TargetName `json:"name"` + Created time.Time `json:"created"` + Status common.ImageBuildState `json:"status"` + // Target type specific options + Options TargetOptions `json:"options"` + // Configuration to produce osbuild artifact specific to this target + OsbuildArtifact OsbuildArtifact `json:"osbuild_artifact"` } func newTarget(name TargetName, options TargetOptions) *Target { @@ -41,6 +53,8 @@ type rawTarget struct { Created time.Time `json:"created"` Status common.ImageBuildState `json:"status"` Options json.RawMessage `json:"options"` + // Configuration to produce osbuild artifact specific to this target + OsbuildArtifact OsbuildArtifact `json:"osbuild_artifact"` } func (target *Target) UnmarshalJSON(data []byte) error { @@ -49,24 +63,9 @@ func (target *Target) UnmarshalJSON(data []byte) error { if err != nil { return err } - options, err := UnmarshalTargetOptions(rawTarget.Name, rawTarget.Options) - if err != nil { - return err - } - target.Uuid = rawTarget.Uuid - target.ImageName = rawTarget.ImageName - target.Name = rawTarget.Name - target.Created = rawTarget.Created - target.Status = rawTarget.Status - target.Options = options - - return nil -} - -func UnmarshalTargetOptions(targetName TargetName, rawOptions json.RawMessage) (TargetOptions, error) { var options TargetOptions - switch targetName { + switch rawTarget.Name { case TargetNameAzure: options = new(AzureTargetOptions) case TargetNameAWS: @@ -89,9 +88,191 @@ func UnmarshalTargetOptions(targetName TargetName, rawOptions json.RawMessage) ( case TargetNameContainer: options = new(ContainerTargetOptions) default: - return nil, fmt.Errorf("unexpected target name: %s", targetName) + return fmt.Errorf("unexpected target name: %s", rawTarget.Name) } - err := json.Unmarshal(rawOptions, options) - return options, err + err = json.Unmarshal(rawTarget.Options, options) + if err != nil { + return err + } + + target.Uuid = rawTarget.Uuid + target.ImageName = rawTarget.ImageName + target.OsbuildArtifact = rawTarget.OsbuildArtifact + target.Name = rawTarget.Name + target.Created = rawTarget.Created + target.Status = rawTarget.Status + target.Options = options + + type compatOptionsType struct { + // Deprecated: `Filename` is now set in the target itself as `ExportFilename`, not in its options. + Filename string `json:"filename"` + } + + var compat compatOptionsType + err = json.Unmarshal(rawTarget.Options, &compat) + if err != nil { + return err + } + + // Kept for backward compatibility + // If the `ExportTarget` is not set in the `Target`, the request is most probably + // coming from an old composer. Copy the value from the target options. + if target.OsbuildArtifact.ExportFilename == "" { + target.OsbuildArtifact.ExportFilename = compat.Filename + } + + return nil +} + +func (target Target) MarshalJSON() ([]byte, error) { + // We can't use composition of the `TargetOptions` interface into a compatibility + // structure, because the value assigned to the embedded interface type member + // would get marshaled under the name of the type. + var rawOptions []byte + var err error + if target.Options != nil { + switch t := target.Options.(type) { + case *AWSTargetOptions: + type compatOptionsType struct { + *AWSTargetOptions + // Deprecated: `Filename` is now set in the target itself as `ExportFilename`, not in its options. + Filename string `json:"filename"` + } + compat := compatOptionsType{ + AWSTargetOptions: t, + Filename: target.OsbuildArtifact.ExportFilename, + } + rawOptions, err = json.Marshal(compat) + + case *AWSS3TargetOptions: + type compatOptionsType struct { + *AWSS3TargetOptions + // Deprecated: `Filename` is now set in the target itself as `ExportFilename`, not in its options. + Filename string `json:"filename"` + } + compat := compatOptionsType{ + AWSS3TargetOptions: t, + Filename: target.OsbuildArtifact.ExportFilename, + } + rawOptions, err = json.Marshal(compat) + + case *AzureTargetOptions: + type compatOptionsType struct { + *AzureTargetOptions + // Deprecated: `Filename` is now set in the target itself as `ExportFilename`, not in its options. + Filename string `json:"filename"` + } + compat := compatOptionsType{ + AzureTargetOptions: t, + Filename: target.OsbuildArtifact.ExportFilename, + } + rawOptions, err = json.Marshal(compat) + + case *GCPTargetOptions: + type compatOptionsType struct { + *GCPTargetOptions + // Deprecated: `Filename` is now set in the target itself as `ExportFilename`, not in its options. + Filename string `json:"filename"` + } + compat := compatOptionsType{ + GCPTargetOptions: t, + Filename: target.OsbuildArtifact.ExportFilename, + } + rawOptions, err = json.Marshal(compat) + + case *AzureImageTargetOptions: + type compatOptionsType struct { + *AzureImageTargetOptions + // Deprecated: `Filename` is now set in the target itself as `ExportFilename`, not in its options. + Filename string `json:"filename"` + } + compat := compatOptionsType{ + AzureImageTargetOptions: t, + Filename: target.OsbuildArtifact.ExportFilename, + } + rawOptions, err = json.Marshal(compat) + + // Kept for backward compatibility + case *LocalTargetOptions: + type compatOptionsType struct { + *LocalTargetOptions + // Deprecated: `Filename` is now set in the target itself as `ExportFilename`, not in its options. + Filename string `json:"filename"` + } + compat := compatOptionsType{ + LocalTargetOptions: t, + Filename: target.OsbuildArtifact.ExportFilename, + } + rawOptions, err = json.Marshal(compat) + + case *KojiTargetOptions: + type compatOptionsType struct { + *KojiTargetOptions + // Deprecated: `Filename` is now set in the target itself as `ExportFilename`, not in its options. + Filename string `json:"filename"` + } + compat := compatOptionsType{ + KojiTargetOptions: t, + Filename: target.OsbuildArtifact.ExportFilename, + } + rawOptions, err = json.Marshal(compat) + + case *VMWareTargetOptions: + type compatOptionsType struct { + *VMWareTargetOptions + // Deprecated: `Filename` is now set in the target itself as `ExportFilename`, not in its options. + Filename string `json:"filename"` + } + compat := compatOptionsType{ + VMWareTargetOptions: t, + Filename: target.OsbuildArtifact.ExportFilename, + } + rawOptions, err = json.Marshal(compat) + + case *OCITargetOptions: + type compatOptionsType struct { + *OCITargetOptions + // Deprecated: `Filename` is now set in the target itself as `ExportFilename`, not in its options. + Filename string `json:"filename"` + } + compat := compatOptionsType{ + OCITargetOptions: t, + Filename: target.OsbuildArtifact.ExportFilename, + } + rawOptions, err = json.Marshal(compat) + + case *ContainerTargetOptions: + type compatOptionsType struct { + *ContainerTargetOptions + // Deprecated: `Filename` is now set in the target itself as `ExportFilename`, not in its options. + Filename string `json:"filename"` + } + compat := compatOptionsType{ + ContainerTargetOptions: t, + Filename: target.OsbuildArtifact.ExportFilename, + } + rawOptions, err = json.Marshal(compat) + + default: + return nil, fmt.Errorf("unexpected target options type: %t", t) + } + + // check error from marshaling + if err != nil { + return nil, err + } + } + + alias := rawTarget{ + Uuid: target.Uuid, + ImageName: target.ImageName, + OsbuildArtifact: target.OsbuildArtifact, + Name: target.Name, + Created: target.Created, + Status: target.Status, + Options: json.RawMessage(rawOptions), + } + + return json.Marshal(alias) } diff --git a/internal/target/target_test.go b/internal/target/target_test.go new file mode 100644 index 000000000..6007aa8f3 --- /dev/null +++ b/internal/target/target_test.go @@ -0,0 +1,243 @@ +package target + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +// Test that `Filename` set in the `Target` options gets set also in the +// `Target.ExportFilename`. +// This covers the case when new worker receives a job from old composer. +// This covers the case when new worker receives a job from new composer. +func TestTargetOptionsFilenameCompatibilityUnmarshal(t *testing.T) { + testCases := []struct { + targetJSON []byte + expectedTarget *Target + }{ + // Test that Filename set in the target options gets set also in the ExportFilename + { + targetJSON: []byte(`{"image_name":"my-image","name":"org.osbuild.aws","options":{"filename":"image.qcow2"}}`), + expectedTarget: &Target{ + ImageName: "my-image", + OsbuildArtifact: OsbuildArtifact{ + ExportFilename: "image.qcow2", + }, + Name: TargetNameAWS, + Options: &AWSTargetOptions{}, + }, + }, + { + targetJSON: []byte(`{"image_name":"my-image","name":"org.osbuild.aws.s3","options":{"region":"eu","accessKeyID":"id","secretAccessKey":"key","sessionToken":"token","bucket":"bkt","key":"key","endpoint":"endpoint","ca_bundle":"bundle","skip_ssl_verification":true,"filename":"image.qcow2"}}`), + expectedTarget: &Target{ + ImageName: "my-image", + OsbuildArtifact: OsbuildArtifact{ + ExportFilename: "image.qcow2", + }, + Name: TargetNameAWSS3, + Options: &AWSS3TargetOptions{ + Region: "eu", + AccessKeyID: "id", + SecretAccessKey: "key", + SessionToken: "token", + Bucket: "bkt", + Key: "key", + Endpoint: "endpoint", + CABundle: "bundle", + SkipSSLVerification: true, + }, + }, + }, + { + targetJSON: []byte(`{"image_name":"my-image","name":"org.osbuild.azure","options":{"storageAccount":"account","storageAccessKey":"key","container":"container","filename":"image.qcow2"}}`), + expectedTarget: &Target{ + ImageName: "my-image", + OsbuildArtifact: OsbuildArtifact{ + ExportFilename: "image.qcow2", + }, + Name: TargetNameAzure, + Options: &AzureTargetOptions{ + StorageAccount: "account", + StorageAccessKey: "key", + Container: "container", + }, + }, + }, + { + targetJSON: []byte(`{"image_name":"my-image","name":"org.osbuild.azure.image","options":{"tenant_id":"tenant","location":"location","subscription_id":"id","resource_group":"group","filename":"image.qcow2"}}`), + expectedTarget: &Target{ + ImageName: "my-image", + OsbuildArtifact: OsbuildArtifact{ + ExportFilename: "image.qcow2", + }, + Name: TargetNameAzureImage, + Options: &AzureImageTargetOptions{ + TenantID: "tenant", + Location: "location", + SubscriptionID: "id", + ResourceGroup: "group", + }, + }, + }, + { + targetJSON: []byte(`{"image_name":"my-image","name":"org.osbuild.gcp","options":{"region":"eu","os":"rhel-8","bucket":"bkt","object":"obj","shareWithAccounts":["account@domain.org"],"credentials":"","filename":"image.qcow2"}}`), + expectedTarget: &Target{ + ImageName: "my-image", + OsbuildArtifact: OsbuildArtifact{ + ExportFilename: "image.qcow2", + }, + Name: TargetNameGCP, + Options: &GCPTargetOptions{ + Region: "eu", + Os: "rhel-8", + Bucket: "bkt", + Object: "obj", + ShareWithAccounts: []string{"account@domain.org"}, + Credentials: []byte(""), + }, + }, + }, + { + targetJSON: []byte(`{"image_name":"my-image","name":"org.osbuild.koji","options":{"upload_directory":"koji-dir","server":"koji.example.org","filename":"image.qcow2"}}`), + expectedTarget: &Target{ + ImageName: "my-image", + OsbuildArtifact: OsbuildArtifact{ + ExportFilename: "image.qcow2", + }, + Name: TargetNameKoji, + Options: &KojiTargetOptions{ + UploadDirectory: "koji-dir", + Server: "koji.example.org", + }, + }, + }, + { + targetJSON: []byte(`{"image_name":"my-image","name":"org.osbuild.oci","options":{"user":"user","tenancy":"tenant","region":"eu","fingerprint":"finger","private_key":"key","bucket":"bkt","namespace":"space","compartment_id":"compartment","filename":"image.qcow2"}}`), + expectedTarget: &Target{ + ImageName: "my-image", + OsbuildArtifact: OsbuildArtifact{ + ExportFilename: "image.qcow2", + }, + Name: TargetNameOCI, + Options: &OCITargetOptions{ + User: "user", + Tenancy: "tenant", + Region: "eu", + Fingerprint: "finger", + PrivateKey: "key", + Bucket: "bkt", + Namespace: "space", + Compartment: "compartment", + }, + }, + }, + { + targetJSON: []byte(`{"image_name":"my-image","name":"org.osbuild.vmware","options":{"host":"example.org","username":"user","password":"pass","datacenter":"center","cluster":"cluster","datastore":"store","filename":"image.qcow2"}}`), + expectedTarget: &Target{ + ImageName: "my-image", + OsbuildArtifact: OsbuildArtifact{ + ExportFilename: "image.qcow2", + }, + Name: TargetNameVMWare, + Options: &VMWareTargetOptions{ + Host: "example.org", + Username: "user", + Password: "pass", + Datacenter: "center", + Cluster: "cluster", + Datastore: "store", + }, + }, + }, + { + targetJSON: []byte(`{"image_name":"my-image","name":"org.osbuild.container","options":{"reference":"ref","username":"user","password":"pass","filename":"image.qcow2"}}`), + expectedTarget: &Target{ + ImageName: "my-image", + OsbuildArtifact: OsbuildArtifact{ + ExportFilename: "image.qcow2", + }, + Name: TargetNameContainer, + Options: &ContainerTargetOptions{ + Reference: "ref", + Username: "user", + Password: "pass", + }, + }, + }, + // Test that the job as Marshalled by the current compatibility code is also acceptable. + // Such job has Filename set in the Target options, as well in the ExportFilename. + { + targetJSON: []byte(`{"image_name":"my-image","name":"org.osbuild.aws","export_filename":"image.qcow2","options":{"filename":"image.qcow2"}}`), + expectedTarget: &Target{ + ImageName: "my-image", + OsbuildArtifact: OsbuildArtifact{ + ExportFilename: "image.qcow2", + }, + Name: TargetNameAWS, + Options: &AWSTargetOptions{}, + }, + }, + // Test the case if the compatibility code for Filename in the target options was dropped. + { + targetJSON: []byte(`{"image_name":"my-image","name":"org.osbuild.aws","osbuild_artifact":{"export_filename":"image.qcow2"},"options":{}}`), + expectedTarget: &Target{ + ImageName: "my-image", + OsbuildArtifact: OsbuildArtifact{ + ExportFilename: "image.qcow2", + }, + Name: TargetNameAWS, + Options: &AWSTargetOptions{}, + }, + }, + } + + for idx, testCase := range testCases { + t.Run(fmt.Sprintf("Case #%d", idx), func(t *testing.T) { + gotTarget := Target{} + err := json.Unmarshal(testCase.targetJSON, &gotTarget) + assert.NoError(t, err) + assert.EqualValues(t, testCase.expectedTarget, &gotTarget) + }) + } +} + +// Test that that ExportFilename set in the Target get added to the options +// as Filename. +// This enables old worker to still pick and be able to handle jobs from new composer. +func TestTargetOptionsFilenameCompatibilityMarshal(t *testing.T) { + testCases := []struct { + targetJSON []byte + target *Target + }{ + { + targetJSON: []byte(`{"uuid":"00000000-0000-0000-0000-000000000000","image_name":"my-image","name":"org.osbuild.aws","created":"0001-01-01T00:00:00Z","status":"WAITING","options":{"region":"us","accessKeyID":"id","secretAccessKey":"key","sessionToken":"token","bucket":"bkt","key":"key","shareWithAccounts":["123456789"],"filename":"image.qcow2"},"osbuild_artifact":{"export_filename":"image.qcow2"}}`), + target: &Target{ + ImageName: "my-image", + OsbuildArtifact: OsbuildArtifact{ + ExportFilename: "image.qcow2", + }, + Name: TargetNameAWS, + Options: &AWSTargetOptions{ + Region: "us", + AccessKeyID: "id", + SecretAccessKey: "key", + SessionToken: "token", + Bucket: "bkt", + Key: "key", + ShareWithAccounts: []string{"123456789"}, + }, + }, + }, + } + + for idx, testCase := range testCases { + t.Run(fmt.Sprintf("Case #%d", idx), func(t *testing.T) { + gotJSON, err := json.Marshal(testCase.target) + assert.Nil(t, err) + t.Logf("%s\n", gotJSON) + assert.EqualValues(t, testCase.targetJSON, gotJSON) + }) + } +} diff --git a/internal/target/vmware_target.go b/internal/target/vmware_target.go index 84153d0fe..72f8be877 100644 --- a/internal/target/vmware_target.go +++ b/internal/target/vmware_target.go @@ -3,7 +3,6 @@ package target const TargetNameVMWare TargetName = "org.osbuild.vmware" type VMWareTargetOptions struct { - Filename string `json:"filename"` Host string `json:"host"` Username string `json:"username"` Password string `json:"password"` diff --git a/internal/weldr/api_test.go b/internal/weldr/api_test.go index abfcd481c..8a8dbaebd 100644 --- a/internal/weldr/api_test.go +++ b/internal/weldr/api_test.go @@ -706,8 +706,10 @@ func TestCompose(t *testing.T) { Name: target.TargetNameAWS, Status: common.IBWaiting, ImageName: "test_upload", + OsbuildArtifact: target.OsbuildArtifact{ + ExportFilename: imgType.Filename(), + }, Options: &target.AWSTargetOptions{ - Filename: "test.img", Region: "frankfurt", AccessKeyID: "accesskey", SecretAccessKey: "secretkey", diff --git a/internal/weldr/upload.go b/internal/weldr/upload.go index 9603c11cf..fbceff5ab 100644 --- a/internal/weldr/upload.go +++ b/internal/weldr/upload.go @@ -243,6 +243,7 @@ func uploadRequestToTarget(u uploadRequest, imageType distro.ImageType) *target. t.Uuid = uuid.New() t.ImageName = u.ImageName + t.OsbuildArtifact.ExportFilename = imageType.Filename() t.Status = common.IBWaiting t.Created = time.Now() @@ -250,7 +251,6 @@ func uploadRequestToTarget(u uploadRequest, imageType distro.ImageType) *target. case *awsUploadSettings: t.Name = target.TargetNameAWS t.Options = &target.AWSTargetOptions{ - Filename: imageType.Filename(), Region: options.Region, AccessKeyID: options.AccessKeyID, SecretAccessKey: options.SecretAccessKey, @@ -261,7 +261,6 @@ func uploadRequestToTarget(u uploadRequest, imageType distro.ImageType) *target. case *awsS3UploadSettings: t.Name = target.TargetNameAWSS3 t.Options = &target.AWSS3TargetOptions{ - Filename: imageType.Filename(), Region: options.Region, AccessKeyID: options.AccessKeyID, SecretAccessKey: options.SecretAccessKey, @@ -275,7 +274,6 @@ func uploadRequestToTarget(u uploadRequest, imageType distro.ImageType) *target. case *azureUploadSettings: t.Name = target.TargetNameAzure t.Options = &target.AzureTargetOptions{ - Filename: imageType.Filename(), StorageAccount: options.StorageAccount, StorageAccessKey: options.StorageAccessKey, Container: options.Container, @@ -300,7 +298,6 @@ func uploadRequestToTarget(u uploadRequest, imageType distro.ImageType) *target. } t.Options = &target.GCPTargetOptions{ - Filename: imageType.Filename(), Region: options.Region, Os: imageType.Arch().Distro().Name(), Bucket: options.Bucket, @@ -310,7 +307,6 @@ func uploadRequestToTarget(u uploadRequest, imageType distro.ImageType) *target. case *vmwareUploadSettings: t.Name = target.TargetNameVMWare t.Options = &target.VMWareTargetOptions{ - Filename: imageType.Filename(), Username: options.Username, Password: options.Password, Host: options.Host, @@ -324,7 +320,6 @@ func uploadRequestToTarget(u uploadRequest, imageType distro.ImageType) *target. User: options.User, Tenancy: options.Tenancy, Region: options.Region, - Filename: imageType.Filename(), PrivateKey: options.PrivateKey, Fingerprint: options.Fingerprint, Bucket: options.Bucket, @@ -337,7 +332,6 @@ func uploadRequestToTarget(u uploadRequest, imageType distro.ImageType) *target. Username: options.Username, Password: options.Password, - Filename: imageType.Filename(), TlsVerify: options.TlsVerify, } }