From db431a565d6f0aac64651e634e59d4fd99279ec6 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Fri, 5 May 2023 15:11:39 +0200 Subject: [PATCH] ostree: move OSTreeImageOptions to the ostree package Move the ostree image options to the ostree package and rename the type to ImageOptions (ostree.ImageOptions). --- cmd/gen-manifests/main.go | 13 +++++---- cmd/osbuild-dnf-json-tests/main_test.go | 3 +- cmd/osbuild-package-sets/main.go | 3 +- cmd/osbuild-pipeline/main.go | 3 +- internal/cloudapi/v2/handler.go | 2 ++ internal/cloudapi/v2/server.go | 9 ++++-- internal/distro/distro.go | 29 ++----------------- internal/distro/distro_test.go | 7 +++-- .../distro_test_common/distro_test_common.go | 14 +++++---- internal/distro/fedora/imagetype.go | 22 ++++++++++---- internal/distro/rhel8/imagetype.go | 24 ++++++++++----- internal/distro/rhel9/imagetype.go | 24 ++++++++++----- internal/ostree/ostree.go | 28 ++++++++++++++++++ internal/weldr/api.go | 2 +- 14 files changed, 114 insertions(+), 69 deletions(-) diff --git a/cmd/gen-manifests/main.go b/cmd/gen-manifests/main.go index 2c237ae8f..ebb86b9fc 100644 --- a/cmd/gen-manifests/main.go +++ b/cmd/gen-manifests/main.go @@ -22,6 +22,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/distroregistry" "github.com/osbuild/osbuild-composer/internal/dnfjson" "github.com/osbuild/osbuild-composer/internal/manifest" + "github.com/osbuild/osbuild-composer/internal/ostree" "github.com/osbuild/osbuild-composer/internal/rhsm/facts" "github.com/osbuild/osbuild-composer/internal/rpmmd" ) @@ -122,12 +123,17 @@ func makeManifestJob(name string, imgType distro.ImageType, cr composeRequest, d options := distro.ImageOptions{Size: 0} if cr.OSTree != nil { - options.OSTree = distro.OSTreeImageOptions{ + options.OSTree = &ostree.ImageOptions{ URL: cr.OSTree.URL, ImageRef: cr.OSTree.Ref, FetchChecksum: cr.OSTree.Parent, RHSM: cr.OSTree.RHSM, } + } else { + // use default OSTreeRef for image type + options.OSTree = &ostree.ImageOptions{ + ImageRef: imgType.OSTreeRef(), + } } // add RHSM fact to detect changes @@ -155,11 +161,6 @@ func makeManifestJob(name string, imgType distro.ImageType, cr composeRequest, d return fmt.Errorf("[%s] container resolution failed: %s", filename, err.Error()) } - if options.OSTree.ImageRef == "" { - // use default OSTreeRef for image type - options.OSTree.ImageRef = imgType.OSTreeRef() - } - packageSpecs, err := depsolve(cacheDir, imgType, bp, options, repos, distribution, archName) if err != nil { err = fmt.Errorf("[%s] depsolve failed: %s", filename, err.Error()) diff --git a/cmd/osbuild-dnf-json-tests/main_test.go b/cmd/osbuild-dnf-json-tests/main_test.go index dab64d755..0623cfdd6 100644 --- a/cmd/osbuild-dnf-json-tests/main_test.go +++ b/cmd/osbuild-dnf-json-tests/main_test.go @@ -14,6 +14,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/distro" rhel "github.com/osbuild/osbuild-composer/internal/distro/rhel8" "github.com/osbuild/osbuild-composer/internal/dnfjson" + "github.com/osbuild/osbuild-composer/internal/ostree" "github.com/osbuild/osbuild-composer/internal/platform" "github.com/osbuild/osbuild-composer/internal/rpmmd" ) @@ -47,7 +48,7 @@ func TestCrossArchDepsolve(t *testing.T) { packages := imgType.PackageSets(blueprint.Blueprint{}, distro.ImageOptions{ - OSTree: distro.OSTreeImageOptions{ + OSTree: &ostree.ImageOptions{ URL: "foo", ImageRef: "bar", FetchChecksum: "baz", diff --git a/cmd/osbuild-package-sets/main.go b/cmd/osbuild-package-sets/main.go index ade46548c..b31d318cf 100644 --- a/cmd/osbuild-package-sets/main.go +++ b/cmd/osbuild-package-sets/main.go @@ -11,6 +11,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/blueprint" "github.com/osbuild/osbuild-composer/internal/distro" "github.com/osbuild/osbuild-composer/internal/distroregistry" + "github.com/osbuild/osbuild-composer/internal/ostree" ) func main() { @@ -48,7 +49,7 @@ func main() { encoder := json.NewEncoder(os.Stdout) encoder.SetIndent("", " ") pkgset := image.PackageSets(blueprint.Blueprint{}, distro.ImageOptions{ - OSTree: distro.OSTreeImageOptions{ + OSTree: &ostree.ImageOptions{ URL: "foo", ImageRef: "bar", FetchChecksum: "baz", diff --git a/cmd/osbuild-pipeline/main.go b/cmd/osbuild-pipeline/main.go index 20ba526b8..ec0815812 100644 --- a/cmd/osbuild-pipeline/main.go +++ b/cmd/osbuild-pipeline/main.go @@ -13,6 +13,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/distro" "github.com/osbuild/osbuild-composer/internal/distroregistry" "github.com/osbuild/osbuild-composer/internal/dnfjson" + "github.com/osbuild/osbuild-composer/internal/ostree" "github.com/osbuild/osbuild-composer/internal/blueprint" "github.com/osbuild/osbuild-composer/internal/rpmmd" @@ -179,7 +180,7 @@ func main() { options := distro.ImageOptions{ Size: imageType.Size(0), - OSTree: distro.OSTreeImageOptions{ + OSTree: &ostree.ImageOptions{ ImageRef: composeRequest.OSTree.Ref, FetchChecksum: composeRequest.OSTree.Parent, URL: composeRequest.OSTree.URL, diff --git a/internal/cloudapi/v2/handler.go b/internal/cloudapi/v2/handler.go index e71cf5e8f..563b12521 100644 --- a/internal/cloudapi/v2/handler.go +++ b/internal/cloudapi/v2/handler.go @@ -462,6 +462,8 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { var ostreeOptions *ostree.SourceSpec // assume it's an ostree image if the type has a default ostree ref if imageType.OSTreeRef() != "" { + imageOptions.OSTree = &ostree.ImageOptions{} + ostreeOptions = &ostree.SourceSpec{} if ir.Ostree != nil { if ir.Ostree.Ref != nil { diff --git a/internal/cloudapi/v2/server.go b/internal/cloudapi/v2/server.go index 5bb70e0a2..34534afd7 100644 --- a/internal/cloudapi/v2/server.go +++ b/internal/cloudapi/v2/server.go @@ -23,6 +23,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/container" "github.com/osbuild/osbuild-composer/internal/distro" "github.com/osbuild/osbuild-composer/internal/distroregistry" + "github.com/osbuild/osbuild-composer/internal/ostree" "github.com/osbuild/osbuild-composer/internal/prometheus" "github.com/osbuild/osbuild-composer/internal/rpmmd" "github.com/osbuild/osbuild-composer/internal/target" @@ -454,9 +455,11 @@ func generateManifest(ctx context.Context, workers *worker.Server, depsolveJobID return } - options.OSTree.ImageRef = result.Specs[0].Ref - options.OSTree.FetchChecksum = result.Specs[0].Checksum - options.OSTree.URL = result.Specs[0].URL + options.OSTree = &ostree.ImageOptions{ + ImageRef: result.Specs[0].Ref, + FetchChecksum: result.Specs[0].Checksum, + URL: result.Specs[0].URL, + } } manifest, _, err := imageType.Manifest(b, options, repos, depsolveResults.PackageSpecs, containerSpecs, seed) diff --git a/internal/distro/distro.go b/internal/distro/distro.go index cb66c50f3..a2891f544 100644 --- a/internal/distro/distro.go +++ b/internal/distro/distro.go @@ -7,6 +7,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/container" "github.com/osbuild/osbuild-composer/internal/disk" "github.com/osbuild/osbuild-composer/internal/manifest" + "github.com/osbuild/osbuild-composer/internal/ostree" "github.com/osbuild/osbuild-composer/internal/rhsm/facts" "github.com/osbuild/osbuild-composer/internal/rpmmd" "github.com/osbuild/osbuild-composer/internal/subscription" @@ -138,37 +139,11 @@ type ImageType interface { // The ImageOptions specify options for a specific image build type ImageOptions struct { Size uint64 - OSTree OSTreeImageOptions + OSTree *ostree.ImageOptions Subscription *subscription.ImageOptions Facts *facts.ImageOptions } -// The OSTreeImageOptions specify an ostree ref, checksum, URL, ContentURL, and RHSM. The meaning of -// each parameter depends on the image type being built. -type OSTreeImageOptions struct { - // For ostree commit and container types: The ref of the new commit to be - // built. - // For ostree installers and raw images: The ref of the commit being - // embedded in the installer or deployed in the image. - ImageRef string - - // For ostree commit and container types: The FetchChecksum 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 - - // The URL from which to fetch the commit specified by the checksum. - URL string - - // If specified, the URL will be used only for metadata. - ContentURL string - - // Indicate if the 'org.osbuild.rhsm.consumer' secret should be added when pulling from the - // remote. - RHSM bool -} - type BasePartitionTableMap map[string]disk.PartitionTable // Fallbacks: When a new method is added to an interface to provide to provide diff --git a/internal/distro/distro_test.go b/internal/distro/distro_test.go index abd2279ec..0bddbdafb 100644 --- a/internal/distro/distro_test.go +++ b/internal/distro/distro_test.go @@ -12,6 +12,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/distro" "github.com/osbuild/osbuild-composer/internal/distro/distro_test_common" "github.com/osbuild/osbuild-composer/internal/distroregistry" + "github.com/osbuild/osbuild-composer/internal/ostree" "github.com/osbuild/osbuild-composer/internal/rpmmd" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -44,7 +45,7 @@ func TestImageType_PackageSetsChains(t *testing.T) { require.Nil(t, err) imagePkgSets := imageType.PackageSets(blueprint.Blueprint{}, distro.ImageOptions{ - OSTree: distro.OSTreeImageOptions{ + OSTree: &ostree.ImageOptions{ URL: "foo", ImageRef: "bar", FetchChecksum: "baz", @@ -133,7 +134,7 @@ func TestImageTypePipelineNames(t *testing.T) { seed := int64(0) // Add ostree options for image types that require them - options.OSTree = distro.OSTreeImageOptions{ + options.OSTree = &ostree.ImageOptions{ ImageRef: imageType.OSTreeRef(), URL: "https://example.com/repo", FetchChecksum: "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", @@ -412,7 +413,7 @@ func TestPipelineRepositories(t *testing.T) { options := distro.ImageOptions{} // Add ostree options for image types that require them - options.OSTree = distro.OSTreeImageOptions{ + options.OSTree = &ostree.ImageOptions{ ImageRef: imageType.OSTreeRef(), URL: "https://example.com/repo", FetchChecksum: "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", diff --git a/internal/distro/distro_test_common/distro_test_common.go b/internal/distro/distro_test_common/distro_test_common.go index e154c6c76..71b501ecb 100644 --- a/internal/distro/distro_test_common/distro_test_common.go +++ b/internal/distro/distro_test_common/distro_test_common.go @@ -101,10 +101,10 @@ func TestDistro_Manifest(t *testing.T, pipelinePath string, prefix string, regis return } - var ostreeOptions distro.OSTreeImageOptions + var ostreeOptions *ostree.ImageOptions if ref := imageType.OSTreeRef(); ref != "" { if tt.ComposeRequest.OSTree != nil { - ostreeOptions = distro.OSTreeImageOptions{ + ostreeOptions = &ostree.ImageOptions{ ImageRef: tt.ComposeRequest.OSTree.Ref, FetchChecksum: tt.ComposeRequest.OSTree.Parent, URL: tt.ComposeRequest.OSTree.URL, @@ -112,8 +112,10 @@ func TestDistro_Manifest(t *testing.T, pipelinePath string, prefix string, regis } } } - if ostreeOptions.ImageRef == "" { // set image type default if not specified in request - ostreeOptions.ImageRef = imageType.OSTreeRef() + if ostreeOptions == nil { // set image type default if not specified in request + ostreeOptions = &ostree.ImageOptions{ + ImageRef: imageType.OSTreeRef(), + } } options := distro.ImageOptions{ @@ -344,14 +346,14 @@ func GetTestingPackageSpecSets(packageName, arch string, pkgSetNames []string) m func GetTestingImagePackageSpecSets(packageName string, i distro.ImageType) map[string][]rpmmd.PackageSpec { arch := i.Arch().Name() imagePackageSets := make([]string, 0, len(i.PackageSets(blueprint.Blueprint{}, distro.ImageOptions{ - OSTree: distro.OSTreeImageOptions{ + OSTree: &ostree.ImageOptions{ URL: "foo", ImageRef: "bar", FetchChecksum: "baz", }, }, nil))) for pkgSetName := range i.PackageSets(blueprint.Blueprint{}, distro.ImageOptions{ - OSTree: distro.OSTreeImageOptions{ + OSTree: &ostree.ImageOptions{ URL: "foo", ImageRef: "bar", FetchChecksum: "baz", diff --git a/internal/distro/fedora/imagetype.go b/internal/distro/fedora/imagetype.go index ef2e13d39..758d982a7 100644 --- a/internal/distro/fedora/imagetype.go +++ b/internal/distro/fedora/imagetype.go @@ -14,6 +14,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/image" "github.com/osbuild/osbuild-composer/internal/manifest" "github.com/osbuild/osbuild-composer/internal/oscap" + "github.com/osbuild/osbuild-composer/internal/ostree" "github.com/osbuild/osbuild-composer/internal/pathpolicy" "github.com/osbuild/osbuild-composer/internal/platform" "github.com/osbuild/osbuild-composer/internal/rpmmd" @@ -110,6 +111,15 @@ func (t *imageType) PackageSets(bp blueprint.Blueprint, options distro.ImageOpti } } + // For iot-commit and iot-container, we need to set an ImageRef if one + // isn't defined already in order to properly initialize the manifest and + // package selection. + if options.OSTree == nil { + options.OSTree = &ostree.ImageOptions{ + ImageRef: t.OSTreeRef(), + } + } + // In case of Cloud API, this method is called before the ostree commit // is resolved. Unfortunately, initializeManifest when called for // an ostree installer returns an error. @@ -128,11 +138,6 @@ func (t *imageType) PackageSets(bp blueprint.Blueprint, options distro.ImageOpti logrus.Warn("FIXME: Requesting package sets for iot-installer without a resolved ostree ref. Faking one.") } - // Similar to above, for edge-commit and edge-container, we need to set an - // ImageRef in order to properly initialize the manifest and package - // selection. - options.OSTree.ImageRef = t.OSTreeRef() - // create a temporary container spec array with the info from the blueprint // to initialize the manifest containers := make([]container.Spec, len(bp.Containers)) @@ -337,9 +342,14 @@ func (t *imageType) checkOptions(bp *blueprint.Blueprint, options distro.ImageOp return nil, fmt.Errorf("embedding containers is not supported for %s on %s", t.name, t.arch.distro.name) } + ostreeChecksum := "" + if options.OSTree != nil { + ostreeChecksum = options.OSTree.FetchChecksum + } + if t.bootISO && t.rpmOstree { // check the checksum instead of the URL, because the URL should have been used to resolve the checksum and we need both - if options.OSTree.FetchChecksum == "" { + if ostreeChecksum == "" { return nil, fmt.Errorf("boot ISO image type %q requires specifying a URL from which to retrieve the OSTree commit", t.name) } } diff --git a/internal/distro/rhel8/imagetype.go b/internal/distro/rhel8/imagetype.go index c7a94af30..8c0267804 100644 --- a/internal/distro/rhel8/imagetype.go +++ b/internal/distro/rhel8/imagetype.go @@ -18,6 +18,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/image" "github.com/osbuild/osbuild-composer/internal/manifest" "github.com/osbuild/osbuild-composer/internal/oscap" + "github.com/osbuild/osbuild-composer/internal/ostree" "github.com/osbuild/osbuild-composer/internal/pathpolicy" "github.com/osbuild/osbuild-composer/internal/platform" "github.com/osbuild/osbuild-composer/internal/rpmmd" @@ -282,6 +283,15 @@ func (t *imageType) PackageSets(bp blueprint.Blueprint, options distro.ImageOpti } } + // For edge-commit and edge-container, we need to set an ImageRef if one + // isn't defined already in order to properly initialize the manifest and + // package selection. + if options.OSTree == nil { + options.OSTree = &ostree.ImageOptions{ + ImageRef: t.OSTreeRef(), + } + } + // In case of Cloud API, this method is called before the ostree commit // is resolved. Unfortunately, initializeManifest when called for // an ostree installer returns an error. @@ -300,11 +310,6 @@ func (t *imageType) PackageSets(bp blueprint.Blueprint, options distro.ImageOpti logrus.Warn("FIXME: Requesting package sets for iot-installer without a resolved ostree ref. Faking one.") } - // Similar to above, for edge-commit and edge-container, we need to set an - // ImageRef in order to properly initialize the manifest and package - // selection. - options.OSTree.ImageRef = t.OSTreeRef() - // create a temporary container spec array with the info from the blueprint // to initialize the manifest containers := make([]container.Spec, len(bp.Containers)) @@ -410,9 +415,14 @@ func (t *imageType) checkOptions(bp *blueprint.Blueprint, options distro.ImageOp return warnings, fmt.Errorf("embedding containers is not supported for %s on %s", t.name, t.arch.distro.name) } + ostreeChecksum := "" + if options.OSTree != nil { + ostreeChecksum = options.OSTree.FetchChecksum + } + if t.bootISO && t.rpmOstree { // check the checksum instead of the URL, because the URL should have been used to resolve the checksum and we need both - if options.OSTree.FetchChecksum == "" { + if ostreeChecksum == "" { return warnings, fmt.Errorf("boot ISO image type %q requires specifying a URL from which to retrieve the OSTree commit", t.name) } @@ -453,7 +463,7 @@ func (t *imageType) checkOptions(bp *blueprint.Blueprint, options distro.ImageOp if t.name == "edge-raw-image" { // check the checksum instead of the URL, because the URL should have been used to resolve the checksum and we need both - if options.OSTree.FetchChecksum == "" { + if ostreeChecksum == "" { return warnings, fmt.Errorf("edge raw images require specifying a URL from which to retrieve the OSTree commit") } diff --git a/internal/distro/rhel9/imagetype.go b/internal/distro/rhel9/imagetype.go index b84b74a95..4560a66c4 100644 --- a/internal/distro/rhel9/imagetype.go +++ b/internal/distro/rhel9/imagetype.go @@ -18,6 +18,7 @@ import ( "github.com/osbuild/osbuild-composer/internal/image" "github.com/osbuild/osbuild-composer/internal/manifest" "github.com/osbuild/osbuild-composer/internal/oscap" + "github.com/osbuild/osbuild-composer/internal/ostree" "github.com/osbuild/osbuild-composer/internal/pathpolicy" "github.com/osbuild/osbuild-composer/internal/platform" "github.com/osbuild/osbuild-composer/internal/rpmmd" @@ -282,6 +283,15 @@ func (t *imageType) PackageSets(bp blueprint.Blueprint, options distro.ImageOpti } } + // For edge-commit and edge-container, we need to set an ImageRef if one + // isn't defined already in order to properly initialize the manifest and + // package selection. + if options.OSTree == nil { + options.OSTree = &ostree.ImageOptions{ + ImageRef: t.OSTreeRef(), + } + } + // In case of Cloud API, this method is called before the ostree commit // is resolved. Unfortunately, initializeManifest when called for // an ostree installer returns an error. @@ -300,11 +310,6 @@ func (t *imageType) PackageSets(bp blueprint.Blueprint, options distro.ImageOpti logrus.Warn("FIXME: Requesting package sets for iot-installer without a resolved ostree ref. Faking one.") } - // Similar to above, for edge-commit and edge-container, we need to set an - // ImageRef in order to properly initialize the manifest and package - // selection. - options.OSTree.ImageRef = t.OSTreeRef() - // create a temporary container spec array with the info from the blueprint // to initialize the manifest containers := make([]container.Spec, len(bp.Containers)) @@ -380,9 +385,14 @@ func (t *imageType) checkOptions(bp *blueprint.Blueprint, options distro.ImageOp return warnings, fmt.Errorf("embedding containers is not supported for %s on %s", t.name, t.arch.distro.name) } + ostreeChecksum := "" + if options.OSTree != nil { + ostreeChecksum = options.OSTree.FetchChecksum + } + if t.bootISO && t.rpmOstree { // check the checksum instead of the URL, because the URL should have been used to resolve the checksum and we need both - if options.OSTree.FetchChecksum == "" { + if ostreeChecksum == "" { return warnings, fmt.Errorf("boot ISO image type %q requires specifying a URL from which to retrieve the OSTree commit", t.name) } @@ -434,7 +444,7 @@ func (t *imageType) checkOptions(bp *blueprint.Blueprint, options distro.ImageOp if t.name == "edge-raw-image" { // check the checksum instead of the URL, because the URL should have been used to resolve the checksum and we need both - if options.OSTree.FetchChecksum == "" { + if ostreeChecksum == "" { return warnings, fmt.Errorf("edge raw images require specifying a URL from which to retrieve the OSTree commit") } diff --git a/internal/ostree/ostree.go b/internal/ostree/ostree.go index 22b2d3952..75234ece6 100644 --- a/internal/ostree/ostree.go +++ b/internal/ostree/ostree.go @@ -45,6 +45,34 @@ type CommitSpec struct { Checksum string } +// ImageOptions specify an ostree ref, checksum, URL, ContentURL, and RHSM. The +// meaning of each parameter depends on the image type being built. The type +// is used to specify ostree-related image options when initializing a +// Manifest. +type ImageOptions struct { + // For ostree commit and container types: The ref of the new commit to be + // built. + // For ostree installers and raw images: The ref of the commit being + // embedded in the installer or deployed in the image. + ImageRef string + + // For ostree commit and container types: The FetchChecksum 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 + + // The URL from which to fetch the commit specified by the checksum. + URL string + + // If specified, the URL will be used only for metadata. + ContentURL string + + // Indicate if the 'org.osbuild.rhsm.consumer' secret should be added when pulling from the + // remote. + RHSM bool +} + // Remote defines the options that can be set for an OSTree Remote configuration. type Remote struct { Name string diff --git a/internal/weldr/api.go b/internal/weldr/api.go index 3b10b2225..8a10ccf08 100644 --- a/internal/weldr/api.go +++ b/internal/weldr/api.go @@ -2452,7 +2452,7 @@ func (api *API) composeHandler(writer http.ResponseWriter, request *http.Request } testMode := q.Get("test") - ostreeOptions := distro.OSTreeImageOptions{ + ostreeOptions := &ostree.ImageOptions{ URL: cr.OSTree.URL, } if testMode == "1" || testMode == "2" {