diff --git a/cmd/osbuild-worker/jobimpl-koji-finalize.go b/cmd/osbuild-worker/jobimpl-koji-finalize.go index ab4850fa3..70abe8dbc 100644 --- a/cmd/osbuild-worker/jobimpl-koji-finalize.go +++ b/cmd/osbuild-worker/jobimpl-koji-finalize.go @@ -187,15 +187,24 @@ func (impl *KojiFinalizeJobImpl) Run(job worker.Job) error { Arch: buildArgs.Arch, BootMode: buildArgs.ImageBootMode, } - imgOutputsExtraInfo[args.KojiFilenames[i]] = imgOutputExtraInfo + + // The image filename is now set in the KojiTargetResultOptions. + // For backward compatibility, if the filename is not set in the + // options, use the filename from the KojiTargetOptions. + imageFilename := kojiTargetOptions.Image.Filename + if imageFilename == "" { + imageFilename = args.KojiFilenames[i] + } + + imgOutputsExtraInfo[imageFilename] = imgOutputExtraInfo outputs = append(outputs, koji.BuildOutput{ BuildRootID: uint64(i), - Filename: args.KojiFilenames[i], - FileSize: kojiTargetOptions.ImageSize, + Filename: imageFilename, + FileSize: kojiTargetOptions.Image.Size, Arch: buildArgs.Arch, - ChecksumType: koji.ChecksumTypeMD5, - Checksum: kojiTargetOptions.ImageMD5, + ChecksumType: koji.ChecksumType(kojiTargetOptions.Image.ChecksumType), + Checksum: kojiTargetOptions.Image.Checksum, Type: koji.BuildOutputTypeImage, RPMs: imageRPMs, Extra: &koji.BuildOutputExtra{ diff --git a/cmd/osbuild-worker/jobimpl-osbuild.go b/cmd/osbuild-worker/jobimpl-osbuild.go index 3625fc0b8..c9128f73b 100644 --- a/cmd/osbuild-worker/jobimpl-osbuild.go +++ b/cmd/osbuild-worker/jobimpl-osbuild.go @@ -872,8 +872,12 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error { } logWithId.Info("[Koji] 🎉 Image successfully uploaded") targetResult.Options = &target.KojiTargetResultOptions{ - ImageMD5: imageHash, - ImageSize: imageSize, + Image: &target.KojiOutputInfo{ + Filename: jobTarget.ImageName, + ChecksumType: target.ChecksumTypeMD5, + Checksum: imageHash, + Size: imageSize, + }, } case *target.OCITargetOptions: diff --git a/internal/cloudapi/v2/v2_koji_test.go b/internal/cloudapi/v2/v2_koji_test.go index 69d5ce31c..c4463457b 100644 --- a/internal/cloudapi/v2/v2_koji_test.go +++ b/internal/cloudapi/v2/v2_koji_test.go @@ -50,8 +50,12 @@ func TestKojiCompose(t *testing.T) { Arch: test_distro.TestArchName, HostOS: test_distro.TestDistroName, TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ - ImageMD5: "browns", - ImageSize: 42, + Image: &target.KojiOutputInfo{ + Filename: "test.img", + ChecksumType: target.ChecksumTypeMD5, + Checksum: "browns", + Size: 42, + }, })}, OSBuildOutput: &osbuild.Result{ Success: true, @@ -87,8 +91,12 @@ func TestKojiCompose(t *testing.T) { Arch: test_distro.TestArchName, HostOS: test_distro.TestDistroName, TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ - ImageMD5: "browns", - ImageSize: 42, + Image: &target.KojiOutputInfo{ + Filename: "test.img", + ChecksumType: target.ChecksumTypeMD5, + Checksum: "browns", + Size: 42, + }, })}, OSBuildOutput: &osbuild.Result{ Success: true, @@ -124,8 +132,12 @@ func TestKojiCompose(t *testing.T) { Arch: test_distro.TestArchName, HostOS: test_distro.TestDistroName, TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ - ImageMD5: "browns", - ImageSize: 42, + Image: &target.KojiOutputInfo{ + Filename: "test.img", + ChecksumType: target.ChecksumTypeMD5, + Checksum: "browns", + Size: 42, + }, })}, OSBuildOutput: &osbuild.Result{ Success: true, @@ -160,8 +172,12 @@ func TestKojiCompose(t *testing.T) { Arch: test_distro.TestArchName, HostOS: test_distro.TestDistroName, TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ - ImageMD5: "browns", - ImageSize: 42, + Image: &target.KojiOutputInfo{ + Filename: "test.img", + ChecksumType: target.ChecksumTypeMD5, + Checksum: "browns", + Size: 42, + }, })}, OSBuildOutput: &osbuild.Result{ Success: false, @@ -198,8 +214,12 @@ func TestKojiCompose(t *testing.T) { Arch: test_distro.TestArchName, HostOS: test_distro.TestDistroName, TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ - ImageMD5: "browns", - ImageSize: 42, + Image: &target.KojiOutputInfo{ + Filename: "test.img", + ChecksumType: target.ChecksumTypeMD5, + Checksum: "browns", + Size: 42, + }, })}, OSBuildOutput: &osbuild.Result{ Success: true, @@ -247,8 +267,12 @@ func TestKojiCompose(t *testing.T) { Arch: test_distro.TestArchName, HostOS: test_distro.TestDistroName, TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ - ImageMD5: "browns", - ImageSize: 42, + Image: &target.KojiOutputInfo{ + Filename: "test.img", + ChecksumType: target.ChecksumTypeMD5, + Checksum: "browns", + Size: 42, + }, })}, OSBuildOutput: &osbuild.Result{ Success: true, @@ -288,8 +312,12 @@ func TestKojiCompose(t *testing.T) { Arch: test_distro.TestArchName, HostOS: test_distro.TestDistroName, TargetResults: []*target.TargetResult{target.NewKojiTargetResult(&target.KojiTargetResultOptions{ - ImageMD5: "browns", - ImageSize: 42, + Image: &target.KojiOutputInfo{ + Filename: "test.img", + ChecksumType: target.ChecksumTypeMD5, + Checksum: "browns", + Size: 42, + }, })}, OSBuildOutput: &osbuild.Result{ Success: true, diff --git a/internal/target/koji_target.go b/internal/target/koji_target.go index 72e58de4d..6145e29b3 100644 --- a/internal/target/koji_target.go +++ b/internal/target/koji_target.go @@ -1,5 +1,10 @@ package target +import ( + "encoding/json" + "fmt" +) + const TargetNameKoji TargetName = "org.osbuild.koji" type KojiTargetOptions struct { @@ -13,9 +18,101 @@ func NewKojiTarget(options *KojiTargetOptions) *Target { return newTarget(TargetNameKoji, options) } +// ChecksumType represents the type of a checksum used for a KojiOutputInfo. +type ChecksumType string + +const ( + ChecksumTypeMD5 ChecksumType = "md5" + + // Only MD5 is supported for now to enable backwards compatibility. + // The reason is tha the old KojiTargetOptions contained only + // ImageMD5 and ImageSize fields, which mandates the use of MD5. + // TODO: uncomment the lines below when the backwards compatibility is no longer needed. + //ChecksumTypeAdler32 ChecksumType = "adler32" + //ChecksumTypeSHA256 ChecksumType = "sha256" +) + +// KojiOutputInfo represents the information about any output file uploaded to Koji +// as part of the OSBuild job. This information is then used by the KojiFinalize +// job when importing files into Koji. +type KojiOutputInfo struct { + Filename string `json:"filename"` + ChecksumType ChecksumType `json:"checksum_type"` + Checksum string `json:"checksum"` + Size uint64 `json:"size"` +} + type KojiTargetResultOptions struct { - ImageMD5 string `json:"image_md5"` - ImageSize uint64 `json:"image_size"` + Image *KojiOutputInfo `json:"image"` + Log *KojiOutputInfo `json:"log,omitempty"` + OSBuildManifest *KojiOutputInfo `json:"osbuild_manifest,omitempty"` +} + +func (o *KojiTargetResultOptions) UnmarshalJSON(data []byte) error { + type aliasType KojiTargetResultOptions + if err := json.Unmarshal(data, (*aliasType)(o)); err != nil { + return err + } + + // compatType contains deprecated fields, which are being checked + // for backwards compatibility. + type compatType struct { + // Deprecated: Use Image in KojiTargetOptions instead. + // Kept for backwards compatibility. + ImageMD5 string `json:"image_md5"` + ImageSize uint64 `json:"image_size"` + } + + var compat compatType + if err := json.Unmarshal(data, &compat); err != nil { + return err + } + + // Check if the Image data in the new struct format are set. + // If not, then the data are coming from an old composer. + if o.Image == nil { + // o.Image.Filename is kept empty, because the filename was previously + // not set as there was always only the Image file. The KojiFinalize job + // handles this case and takes the Image filename from the KojiFinalizeJob + // options. + + o.Image = &KojiOutputInfo{ + ChecksumType: ChecksumTypeMD5, + Checksum: compat.ImageMD5, + Size: compat.ImageSize, + } + } + + return nil +} + +func (o KojiTargetResultOptions) MarshalJSON() ([]byte, error) { + type alias KojiTargetResultOptions + // compatType is a super-set of the current KojiTargetResultOptions and + // old version of it. It contains deprecated fields, which are being set + // for backwards compatibility. + type compatType struct { + alias + + // Deprecated: Use Image in KojiTargetOptions instead. + // Kept for backwards compatibility. + ImageMD5 string `json:"image_md5"` + ImageSize uint64 `json:"image_size"` + } + + // Only MD5 is supported for now to enable backwards compatibility. + // TODO: remove this block when the backwards compatibility is no longer needed. + if o.Image.ChecksumType != ChecksumTypeMD5 { + return nil, fmt.Errorf("unsupported checksum type: %s", o.Image.ChecksumType) + } + + compat := compatType{ + alias: (alias)(o), + ImageMD5: o.Image.Checksum, + ImageSize: o.Image.Size, + } + + return json.Marshal(compat) } func (KojiTargetResultOptions) isTargetResultOptions() {} diff --git a/internal/target/koji_target_test.go b/internal/target/koji_target_test.go new file mode 100644 index 000000000..41b03e025 --- /dev/null +++ b/internal/target/koji_target_test.go @@ -0,0 +1,163 @@ +package target + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestKojiTargetOptionsUnmarshalJSON tests that the resulting struct +// has appropriate fields set when legacy JSON is used. +func TestKojiTargetResultOptionsUnmarshalJSON(t *testing.T) { + type testCase struct { + name string + JSON []byte + expectedResult *KojiTargetResultOptions + err bool + } + + testCases := []testCase{ + { + name: "new format", + JSON: []byte(`{"image":{"checksum_type":"md5","checksum":"hash","filename":"image.raw","size":123456}}`), + expectedResult: &KojiTargetResultOptions{ + Image: &KojiOutputInfo{ + Filename: "image.raw", + ChecksumType: "md5", + Checksum: "hash", + Size: 123456, + }, + }, + }, + { + name: "old format", + JSON: []byte(`{"image_md5":"hash","image_size":123456}`), + expectedResult: &KojiTargetResultOptions{ + Image: &KojiOutputInfo{ + ChecksumType: "md5", + Checksum: "hash", + Size: 123456, + }, + }, + }, + { + name: "full format", + JSON: []byte(`{"image":{"checksum_type":"md5","checksum":"hash","filename":"image.raw","size":123456},"log":{"checksum_type":"md5","checksum":"hash","filename":"log.txt","size":123456},"osbuild_manifest":{"checksum_type":"md5","checksum":"hash","filename":"manifest.json","size":123456}}`), + expectedResult: &KojiTargetResultOptions{ + Image: &KojiOutputInfo{ + Filename: "image.raw", + ChecksumType: "md5", + Checksum: "hash", + Size: 123456, + }, + Log: &KojiOutputInfo{ + Filename: "log.txt", + ChecksumType: "md5", + Checksum: "hash", + Size: 123456, + }, + OSBuildManifest: &KojiOutputInfo{ + Filename: "manifest.json", + ChecksumType: "md5", + Checksum: "hash", + Size: 123456, + }, + }, + }, + { + name: "invalid JSON", + JSON: []byte(`{"image_md5":"hash","image_size":123456`), + err: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var result KojiTargetResultOptions + err := json.Unmarshal(tc.JSON, &result) + if tc.err { + assert.Error(t, err) + return + } else { + assert.NoError(t, err) + } + assert.Equal(t, tc.expectedResult, &result) + }) + } +} + +// TestKojiTargetResultOptionsMarshalJSON tests that the resulting JSON +// has the legacy fields set for backwards compatibility. +func TestKojiTargetResultOptionsMarshalJSON(t *testing.T) { + type testCase struct { + name string + results *KojiTargetResultOptions + expectedJSON []byte + err bool + } + + testCases := []testCase{ + { + name: "backwards compatibility", + results: &KojiTargetResultOptions{ + Image: &KojiOutputInfo{ + Filename: "image.raw", + ChecksumType: ChecksumTypeMD5, + Checksum: "hash", + Size: 123456, + }, + }, + expectedJSON: []byte(`{"image":{"filename":"image.raw","checksum_type":"md5","checksum":"hash","size":123456},"image_md5":"hash","image_size":123456}`), + }, + { + name: "full format", + results: &KojiTargetResultOptions{ + Image: &KojiOutputInfo{ + Filename: "image.raw", + ChecksumType: ChecksumTypeMD5, + Checksum: "hash", + Size: 123456, + }, + Log: &KojiOutputInfo{ + Filename: "log.txt", + ChecksumType: ChecksumTypeMD5, + Checksum: "hash", + Size: 654321, + }, + OSBuildManifest: &KojiOutputInfo{ + Filename: "manifest.json", + ChecksumType: ChecksumTypeMD5, + Checksum: "hash", + Size: 123321, + }, + }, + expectedJSON: []byte(`{"image":{"filename":"image.raw","checksum_type":"md5","checksum":"hash","size":123456},"log":{"filename":"log.txt","checksum_type":"md5","checksum":"hash","size":654321},"osbuild_manifest":{"filename":"manifest.json","checksum_type":"md5","checksum":"hash","size":123321},"image_md5":"hash","image_size":123456}`), + }, + { + name: "invalid checksum type", + results: &KojiTargetResultOptions{ + Image: &KojiOutputInfo{ + Filename: "image.raw", + ChecksumType: "sha256", + Checksum: "hash", + Size: 123456, + }, + }, + err: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result, err := json.Marshal(tc.results) + if tc.err { + assert.Error(t, err) + return + } else { + assert.NoError(t, err) + } + assert.Equal(t, tc.expectedJSON, result) + }) + } +} diff --git a/internal/target/targetresult_test.go b/internal/target/targetresult_test.go index dcc7a884f..b92bf9785 100644 --- a/internal/target/targetresult_test.go +++ b/internal/target/targetresult_test.go @@ -58,12 +58,16 @@ func TestTargetResultUnmarshal(t *testing.T) { }, }, { - resultJSON: []byte(`{"name":"org.osbuild.koji","options":{"image_md5":"hash","image_size":123456}}`), + resultJSON: []byte(`{"name":"org.osbuild.koji","options":{"image":{"checksum_type":"md5","checksum":"hash","filename":"image.raw","size":123456}}}`), expectedResult: &TargetResult{ Name: TargetNameKoji, Options: &KojiTargetResultOptions{ - ImageMD5: "hash", - ImageSize: 123456, + Image: &KojiOutputInfo{ + Filename: "image.raw", + ChecksumType: ChecksumTypeMD5, + Checksum: "hash", + Size: 123456, + }, }, }, }, diff --git a/internal/worker/json.go b/internal/worker/json.go index ae8305f52..8b5feddad 100644 --- a/internal/worker/json.go +++ b/internal/worker/json.go @@ -124,10 +124,11 @@ type KojiInitJobResult struct { } type KojiFinalizeJob struct { - Server string `json:"server"` - Name string `json:"name"` - Version string `json:"version"` - Release string `json:"release"` + Server string `json:"server"` + Name string `json:"name"` + Version string `json:"version"` + Release string `json:"release"` + // TODO: eventually deprecate and remove KojiFilenames, since the image filenames are now set in the KojiTargetResultOptions. KojiFilenames []string `json:"koji_filenames"` KojiDirectory string `json:"koji_directory"` TaskID uint64 `json:"task_id"` /* https://pagure.io/koji/issue/215 */