From d7a5bb99c1c50cccfedef929398c24ac25dea07e Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Thu, 1 Jun 2023 13:25:20 +0200 Subject: [PATCH] ostree: replace FetchChecksum with ParentRef on ostree.ImageOptions The FetchChecksum on ostree.ImageOptions was the resolved commit ID of the parent ref to be pulled (for ostree commits and containers) or the commit ID of the content ref (for ostree installers and raw images). With the new process of manifest creation and serialisation, using the image options to transport resolved content references is bad and confusing. Image options should only reflect user and image type options before any references are resolved. With this change, the ostree.ImageOptions should only reflect the ostree-related options specified by the user. Commit IDs will only be available after the manifest is initialised when the commit sources are resolved (before serialisation). --- cmd/gen-manifests/main.go | 8 ++++---- cmd/osbuild-dnf-json-tests/main_test.go | 2 +- cmd/osbuild-package-sets/main.go | 6 +++--- cmd/osbuild-pipeline/main.go | 6 +++--- internal/cloudapi/v2/server.go | 6 +++--- internal/distro/distro_test.go | 18 +++++++++--------- .../distro_test_common/distro_test_common.go | 14 +++++++------- internal/distro/fedora/images.go | 12 ++++++------ internal/distro/fedora/imagetype.go | 2 +- internal/distro/rhel8/images.go | 14 +++++++------- internal/distro/rhel8/imagetype.go | 2 +- internal/distro/rhel9/images.go | 14 +++++++------- internal/distro/rhel9/imagetype.go | 2 +- internal/ostree/ostree.go | 7 +++---- internal/weldr/api.go | 2 +- 15 files changed, 57 insertions(+), 58 deletions(-) diff --git a/cmd/gen-manifests/main.go b/cmd/gen-manifests/main.go index 0f4eafda6..7e59a1d27 100644 --- a/cmd/gen-manifests/main.go +++ b/cmd/gen-manifests/main.go @@ -124,10 +124,10 @@ func makeManifestJob(name string, imgType distro.ImageType, cr composeRequest, d options := distro.ImageOptions{Size: 0} if cr.OSTree != nil { options.OSTree = &ostree.ImageOptions{ - URL: cr.OSTree.URL, - ImageRef: cr.OSTree.Ref, - FetchChecksum: cr.OSTree.Parent, - RHSM: cr.OSTree.RHSM, + URL: cr.OSTree.URL, + ImageRef: cr.OSTree.Ref, + ParentRef: cr.OSTree.Parent, + RHSM: cr.OSTree.RHSM, } } else { // use default OSTreeRef for image type diff --git a/cmd/osbuild-dnf-json-tests/main_test.go b/cmd/osbuild-dnf-json-tests/main_test.go index 8569a2413..62f255be1 100644 --- a/cmd/osbuild-dnf-json-tests/main_test.go +++ b/cmd/osbuild-dnf-json-tests/main_test.go @@ -60,7 +60,7 @@ func TestCrossArchDepsolve(t *testing.T) { OSTree: &ostree.ImageOptions{ URL: "foo", ImageRef: "bar", - FetchChecksum: "baz", + ParentRef: "baz", }, }, repos[archStr], 0) diff --git a/cmd/osbuild-package-sets/main.go b/cmd/osbuild-package-sets/main.go index 549686008..4e7a2339d 100644 --- a/cmd/osbuild-package-sets/main.go +++ b/cmd/osbuild-package-sets/main.go @@ -50,9 +50,9 @@ func main() { encoder.SetIndent("", " ") options := distro.ImageOptions{ OSTree: &ostree.ImageOptions{ - URL: "foo", - ImageRef: "bar", - FetchChecksum: "baz", + URL: "foo", + ImageRef: "bar", + ParentRef: "baz", }, } manifest, _, err := image.Manifest(&blueprint.Blueprint{}, options, nil, 0) diff --git a/cmd/osbuild-pipeline/main.go b/cmd/osbuild-pipeline/main.go index 68d61ec76..07e13a004 100644 --- a/cmd/osbuild-pipeline/main.go +++ b/cmd/osbuild-pipeline/main.go @@ -181,9 +181,9 @@ func main() { options := distro.ImageOptions{ Size: imageType.Size(0), OSTree: &ostree.ImageOptions{ - ImageRef: composeRequest.OSTree.Ref, - FetchChecksum: composeRequest.OSTree.Parent, - URL: composeRequest.OSTree.URL, + ImageRef: composeRequest.OSTree.Ref, + ParentRef: composeRequest.OSTree.Parent, + URL: composeRequest.OSTree.URL, }, } diff --git a/internal/cloudapi/v2/server.go b/internal/cloudapi/v2/server.go index ef0d26b51..dc3eda341 100644 --- a/internal/cloudapi/v2/server.go +++ b/internal/cloudapi/v2/server.go @@ -482,9 +482,9 @@ func generateManifest(ctx context.Context, workers *worker.Server, depsolveJobID } options.OSTree = &ostree.ImageOptions{ - ImageRef: result.Specs[0].Ref, - FetchChecksum: result.Specs[0].Checksum, - URL: result.Specs[0].URL, + ImageRef: result.Specs[0].Ref, + ParentRef: result.Specs[0].Checksum, + URL: result.Specs[0].URL, } } diff --git a/internal/distro/distro_test.go b/internal/distro/distro_test.go index 2c3462d28..0110f197b 100644 --- a/internal/distro/distro_test.go +++ b/internal/distro/distro_test.go @@ -56,9 +56,9 @@ func TestImageType_PackageSetsChains(t *testing.T) { } options := distro.ImageOptions{ OSTree: &ostree.ImageOptions{ - URL: "foo", - ImageRef: "bar", - FetchChecksum: "baz", + URL: "foo", + ImageRef: "bar", + ParentRef: "baz", }, } manifest, _, err := imageType.Manifest(&bp, options, nil, 0) @@ -147,9 +147,9 @@ func TestImageTypePipelineNames(t *testing.T) { // Add ostree options for image types that require them options.OSTree = &ostree.ImageOptions{ - ImageRef: imageType.OSTreeRef(), - URL: "https://example.com/repo", - FetchChecksum: "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + ImageRef: imageType.OSTreeRef(), + URL: "https://example.com/repo", + ParentRef: "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", } // Pipelines that require package sets will fail if none @@ -461,9 +461,9 @@ func TestPipelineRepositories(t *testing.T) { // Add ostree options for image types that require them options.OSTree = &ostree.ImageOptions{ - ImageRef: imageType.OSTreeRef(), - URL: "https://example.com/repo", - FetchChecksum: "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + ImageRef: imageType.OSTreeRef(), + URL: "https://example.com/repo", + ParentRef: "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", } repos := tCase.repos diff --git a/internal/distro/distro_test_common/distro_test_common.go b/internal/distro/distro_test_common/distro_test_common.go index 58d6df1e8..1c5117095 100644 --- a/internal/distro/distro_test_common/distro_test_common.go +++ b/internal/distro/distro_test_common/distro_test_common.go @@ -105,10 +105,10 @@ func TestDistro_Manifest(t *testing.T, pipelinePath string, prefix string, regis if ref := imageType.OSTreeRef(); ref != "" { if tt.ComposeRequest.OSTree != nil { ostreeOptions = &ostree.ImageOptions{ - ImageRef: tt.ComposeRequest.OSTree.Ref, - FetchChecksum: tt.ComposeRequest.OSTree.Parent, - URL: tt.ComposeRequest.OSTree.URL, - RHSM: tt.ComposeRequest.OSTree.RHSM, + ImageRef: tt.ComposeRequest.OSTree.Ref, + ParentRef: tt.ComposeRequest.OSTree.Parent, + URL: tt.ComposeRequest.OSTree.URL, + RHSM: tt.ComposeRequest.OSTree.RHSM, } } } @@ -217,9 +217,9 @@ var knownKernels = []string{"kernel", "kernel-debug", "kernel-rt"} // Returns the number of known kernels in the package list func kernelCount(imgType distro.ImageType, bp blueprint.Blueprint) int { ostreeOptions := &ostree.ImageOptions{ - URL: "foo", - ImageRef: "bar", - FetchChecksum: "baz", + URL: "foo", + ImageRef: "bar", + ParentRef: "baz", } manifest, _, err := imgType.Manifest(&bp, distro.ImageOptions{OSTree: ostreeOptions}, nil, 0) if err != nil { diff --git a/internal/distro/fedora/images.go b/internal/distro/fedora/images.go index 4f2ebdf04..352b7a396 100644 --- a/internal/distro/fedora/images.go +++ b/internal/distro/fedora/images.go @@ -297,9 +297,9 @@ func iotCommitImage(workload workload.Workload, img.Environment = t.environment img.Workload = workload - if options.OSTree.FetchChecksum != "" && options.OSTree.URL != "" { + if options.OSTree.ParentRef != "" && options.OSTree.URL != "" { img.OSTreeParent = &ostree.CommitSpec{ - Checksum: options.OSTree.FetchChecksum, + Checksum: options.OSTree.ParentRef, URL: options.OSTree.URL, ContentURL: options.OSTree.ContentURL, } @@ -328,9 +328,9 @@ func iotContainerImage(workload workload.Workload, img.Environment = t.environment img.Workload = workload - if options.OSTree.FetchChecksum != "" && options.OSTree.URL != "" { + if options.OSTree.ParentRef != "" && options.OSTree.URL != "" { img.OSTreeParent = &ostree.CommitSpec{ - Checksum: options.OSTree.FetchChecksum, + Checksum: options.OSTree.ParentRef, URL: options.OSTree.URL, ContentURL: options.OSTree.ContentURL, } @@ -359,7 +359,7 @@ func iotInstallerImage(workload workload.Workload, Ref: options.OSTree.ImageRef, URL: options.OSTree.URL, ContentURL: options.OSTree.ContentURL, - Checksum: options.OSTree.FetchChecksum, + Checksum: options.OSTree.ParentRef, } img := image.NewOSTreeInstaller(commit) @@ -399,7 +399,7 @@ func iotRawImage(workload workload.Workload, Ref: options.OSTree.ImageRef, URL: options.OSTree.URL, ContentURL: options.OSTree.ContentURL, - Checksum: options.OSTree.FetchChecksum, + Checksum: options.OSTree.ParentRef, } img := image.NewOSTreeRawImage(commit) diff --git a/internal/distro/fedora/imagetype.go b/internal/distro/fedora/imagetype.go index bb87b4fec..4a5313efd 100644 --- a/internal/distro/fedora/imagetype.go +++ b/internal/distro/fedora/imagetype.go @@ -249,7 +249,7 @@ func (t *imageType) checkOptions(bp *blueprint.Blueprint, options distro.ImageOp ostreeChecksum := "" if options.OSTree != nil { - ostreeChecksum = options.OSTree.FetchChecksum + ostreeChecksum = options.OSTree.ParentRef } if t.bootISO && t.rpmOstree { diff --git a/internal/distro/rhel8/images.go b/internal/distro/rhel8/images.go index 2496be05e..ad7e49916 100644 --- a/internal/distro/rhel8/images.go +++ b/internal/distro/rhel8/images.go @@ -320,9 +320,9 @@ func edgeCommitImage(workload workload.Workload, img.Environment = t.environment img.Workload = workload - if options.OSTree.FetchChecksum != "" && options.OSTree.URL != "" { + if options.OSTree.ParentRef != "" && options.OSTree.URL != "" { img.OSTreeParent = &ostree.CommitSpec{ - Checksum: options.OSTree.FetchChecksum, + Checksum: options.OSTree.ParentRef, URL: options.OSTree.URL, ContentURL: options.OSTree.ContentURL, } @@ -354,9 +354,9 @@ func edgeContainerImage(workload workload.Workload, img.Environment = t.environment img.Workload = workload - if options.OSTree.FetchChecksum != "" && options.OSTree.URL != "" { + if options.OSTree.ParentRef != "" && options.OSTree.URL != "" { img.OSTreeParent = &ostree.CommitSpec{ - Checksum: options.OSTree.FetchChecksum, + Checksum: options.OSTree.ParentRef, URL: options.OSTree.URL, ContentURL: options.OSTree.ContentURL, } @@ -388,7 +388,7 @@ func edgeInstallerImage(workload workload.Workload, Ref: options.OSTree.ImageRef, URL: options.OSTree.URL, ContentURL: options.OSTree.ContentURL, - Checksum: options.OSTree.FetchChecksum, + Checksum: options.OSTree.ParentRef, } if options.OSTree.RHSM { commit.Secrets = "org.osbuild.rhsm.consumer" @@ -432,7 +432,7 @@ func edgeRawImage(workload workload.Workload, Ref: options.OSTree.ImageRef, URL: options.OSTree.URL, ContentURL: options.OSTree.ContentURL, - Checksum: options.OSTree.FetchChecksum, + Checksum: options.OSTree.ParentRef, } img := image.NewOSTreeRawImage(commit) @@ -478,7 +478,7 @@ func edgeSimplifiedInstallerImage(workload workload.Workload, Ref: options.OSTree.ImageRef, URL: options.OSTree.URL, ContentURL: options.OSTree.ContentURL, - Checksum: options.OSTree.FetchChecksum, + Checksum: options.OSTree.ParentRef, } rawImg := image.NewOSTreeRawImage(commit) diff --git a/internal/distro/rhel8/imagetype.go b/internal/distro/rhel8/imagetype.go index 43f053275..9d1c25469 100644 --- a/internal/distro/rhel8/imagetype.go +++ b/internal/distro/rhel8/imagetype.go @@ -315,7 +315,7 @@ func (t *imageType) checkOptions(bp *blueprint.Blueprint, options distro.ImageOp ostreeChecksum := "" if options.OSTree != nil { - ostreeChecksum = options.OSTree.FetchChecksum + ostreeChecksum = options.OSTree.ParentRef } if t.bootISO && t.rpmOstree { diff --git a/internal/distro/rhel9/images.go b/internal/distro/rhel9/images.go index 8bc938e05..cf70ac0b0 100644 --- a/internal/distro/rhel9/images.go +++ b/internal/distro/rhel9/images.go @@ -262,9 +262,9 @@ func edgeCommitImage(workload workload.Workload, img.Environment = t.environment img.Workload = workload - if options.OSTree.FetchChecksum != "" && options.OSTree.URL != "" { + if options.OSTree.ParentRef != "" && options.OSTree.URL != "" { img.OSTreeParent = &ostree.CommitSpec{ - Checksum: options.OSTree.FetchChecksum, + Checksum: options.OSTree.ParentRef, URL: options.OSTree.URL, ContentURL: options.OSTree.ContentURL, } @@ -299,9 +299,9 @@ func edgeContainerImage(workload workload.Workload, img.Environment = t.environment img.Workload = workload - if options.OSTree.FetchChecksum != "" && options.OSTree.URL != "" { + if options.OSTree.ParentRef != "" && options.OSTree.URL != "" { img.OSTreeParent = &ostree.CommitSpec{ - Checksum: options.OSTree.FetchChecksum, + Checksum: options.OSTree.ParentRef, URL: options.OSTree.URL, ContentURL: options.OSTree.ContentURL, } @@ -333,7 +333,7 @@ func edgeInstallerImage(workload workload.Workload, Ref: options.OSTree.ImageRef, URL: options.OSTree.URL, ContentURL: options.OSTree.ContentURL, - Checksum: options.OSTree.FetchChecksum, + Checksum: options.OSTree.ParentRef, } if options.OSTree.RHSM { commit.Secrets = "org.osbuild.rhsm.consumer" @@ -382,7 +382,7 @@ func edgeRawImage(workload workload.Workload, Ref: options.OSTree.ImageRef, URL: options.OSTree.URL, ContentURL: options.OSTree.ContentURL, - Checksum: options.OSTree.FetchChecksum, + Checksum: options.OSTree.ParentRef, } img := image.NewOSTreeRawImage(commit) if !common.VersionLessThan(t.arch.distro.osVersion, "9.2") || t.arch.distro.osVersion == "9-stream" { @@ -445,7 +445,7 @@ func edgeSimplifiedInstallerImage(workload workload.Workload, Ref: options.OSTree.ImageRef, URL: options.OSTree.URL, ContentURL: options.OSTree.ContentURL, - Checksum: options.OSTree.FetchChecksum, + Checksum: options.OSTree.ParentRef, } rawImg := image.NewOSTreeRawImage(commit) if !common.VersionLessThan(t.arch.distro.osVersion, "9.2") || t.arch.distro.osVersion == "9-stream" { diff --git a/internal/distro/rhel9/imagetype.go b/internal/distro/rhel9/imagetype.go index b64cd7869..0843d9a6a 100644 --- a/internal/distro/rhel9/imagetype.go +++ b/internal/distro/rhel9/imagetype.go @@ -286,7 +286,7 @@ func (t *imageType) checkOptions(bp *blueprint.Blueprint, options distro.ImageOp ostreeChecksum := "" if options.OSTree != nil { - ostreeChecksum = options.OSTree.FetchChecksum + ostreeChecksum = options.OSTree.ParentRef } if t.bootISO && t.rpmOstree { diff --git a/internal/ostree/ostree.go b/internal/ostree/ostree.go index 75234ece6..d3ed10474 100644 --- a/internal/ostree/ostree.go +++ b/internal/ostree/ostree.go @@ -56,11 +56,10 @@ type ImageOptions struct { // embedded in the installer or deployed in the image. ImageRef string - // For ostree commit and container types: The FetchChecksum specifies the parent + // For ostree commit and container types: The ParentRef specifies the parent // ostree commit that the new commit will be based on. - // For ostree installers and raw images: The FetchChecksum specifies the commit - // ID that will be embedded in the installer or deployed in the image. - FetchChecksum string + // For ostree installers and raw images: The ParentRef does not apply. + ParentRef string // The URL from which to fetch the commit specified by the checksum. URL string diff --git a/internal/weldr/api.go b/internal/weldr/api.go index cf449b644..6463e1321 100644 --- a/internal/weldr/api.go +++ b/internal/weldr/api.go @@ -2466,7 +2466,7 @@ func (api *API) composeHandler(writer http.ResponseWriter, request *http.Request return } ostreeOptions.ImageRef = ref - ostreeOptions.FetchChecksum = checksum + ostreeOptions.ParentRef = checksum } var size uint64