From 7e2855bbb3ca36f37ecc7ac420367de1218b14c4 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Tue, 6 Jun 2023 16:55:50 +0200 Subject: [PATCH] ostree: rewrite Resolve() to take SourceSpec and return CommitSpec The Resolve() function is now only responsible for resolving a SourceSpec to a CommitSpec. It only resolves a checksum if a URL is set and sets the option for the RHSM secrets. The Parent has been removed from the SourceSpec. The SourceSpec is a simple reference to a single ostree ref and has no connection with the ostree options. --- cmd/osbuild-worker/jobimpl-ostree-resolve.go | 15 +++--- internal/distro/test_distro/distro.go | 1 - internal/ostree/ostree.go | 57 +++++++------------- internal/weldr/api.go | 18 +++---- 4 files changed, 35 insertions(+), 56 deletions(-) diff --git a/cmd/osbuild-worker/jobimpl-ostree-resolve.go b/cmd/osbuild-worker/jobimpl-ostree-resolve.go index 13622dbda..1a0dde45b 100644 --- a/cmd/osbuild-worker/jobimpl-ostree-resolve.go +++ b/cmd/osbuild-worker/jobimpl-ostree-resolve.go @@ -52,13 +52,12 @@ func (impl *OSTreeResolveJobImpl) Run(job worker.Job) error { for i, s := range args.Specs { reqParams := ostree.SourceSpec{ - URL: s.URL, - Ref: s.Ref, - Parent: s.Parent, - RHSM: s.RHSM, + URL: s.URL, + Ref: s.Ref, + RHSM: s.RHSM, } - ref, checksum, err := ostree.Resolve(reqParams) + commitSpec, err := ostree.Resolve(reqParams) if err != nil { logWithId.Infof("Resolving ostree params failed: %v", err) setError(err, &result) @@ -66,9 +65,9 @@ func (impl *OSTreeResolveJobImpl) Run(job worker.Job) error { } result.Specs[i] = worker.OSTreeResolveResultSpec{ - URL: s.URL, - Ref: ref, - Checksum: checksum, + URL: commitSpec.URL, + Ref: commitSpec.Ref, + Checksum: commitSpec.Checksum, RHSM: s.RHSM, } } diff --git a/internal/distro/test_distro/distro.go b/internal/distro/test_distro/distro.go index 983a485b3..d14b0b60f 100644 --- a/internal/distro/test_distro/distro.go +++ b/internal/distro/test_distro/distro.go @@ -250,7 +250,6 @@ func (t *TestImageType) Manifest(b *blueprint.Blueprint, options distro.ImageOpt } // copy any other options that might be specified ostreeSource.URL = options.OSTree.URL - ostreeSource.Parent = options.OSTree.ParentRef ostreeSource.RHSM = options.OSTree.RHSM } ostreeSources = []ostree.SourceSpec{ostreeSource} diff --git a/internal/ostree/ostree.go b/internal/ostree/ostree.go index 61988eefa..b9ba88f4d 100644 --- a/internal/ostree/ostree.go +++ b/internal/ostree/ostree.go @@ -22,10 +22,9 @@ var ostreeRefRE = regexp.MustCompile(`^(?:[\w\d][-._\w\d]*\/)*[\w\d][-._\w\d]*$` // SourceSpec serves as input for ResolveParams, and contains all necessary // variables to resolve a ref, which can then be turned into a CommitSpec. type SourceSpec struct { - URL string - Ref string - Parent string - RHSM bool + URL string + Ref string + RHSM bool } // CommitSpec specifies an ostree commit using any combination of Ref (branch), URL (source), and Checksum (commit ID). @@ -161,50 +160,34 @@ func ResolveRef(location, ref string, consumerCerts bool, subs *rhsm.Subscriptio return checksum, nil } -// Resolve the ostree source specification into the necessary ref for the image -// build pipeline and a commit (checksum) to be fetched. +// Resolve the ostree source specification into a commit specification. // -// If a URL is defined in the source specification, the checksum of the Parent -// ref is resolved, otherwise the checksum is an empty string. Specifying -// Parent without URL results in a ParameterComboError. Failure to resolve the +// If a URL is defined in the source specification, the checksum of the ref is +// resolved, otherwise the checksum is an empty string. Failure to resolve the // checksum results in a ResolveRefError. // -// If Parent is not specified in the RequestParams, the value of Ref is used. -// -// If any ref (Ref or Parent) is malformed, the function returns with a RefError. -func Resolve(source SourceSpec) (ref, checksum string, err error) { - // TODO: return CommitSpec instead and add RHSM option - ref = source.Ref - - // Determine value of ref +// If the ref is malformed, the function returns with a RefError. +func Resolve(source SourceSpec) (CommitSpec, error) { if !VerifyRef(source.Ref) { - return "", "", NewRefError("Invalid ostree ref %q", source.Ref) + return CommitSpec{}, NewRefError("Invalid ostree ref %q", source.Ref) } - // Determine value of parentRef - parentRef := source.Parent - if parentRef != "" { - // verify format of parent ref - if !VerifyRef(source.Parent) { - return "", "", NewRefError("Invalid ostree parent ref %q", source.Parent) - } - if source.URL == "" { - // specifying parent ref also requires URL - return "", "", NewParameterComboError("ostree parent ref specified, but no URL to retrieve it") - } - } else { - // if parent is not provided, use ref - parentRef = source.Ref + commit := CommitSpec{ + Ref: source.Ref, + URL: source.URL, + } + if source.RHSM { + commit.Secrets = "org.osbuild.rhsm.consumer" } - // Resolve parent checksum + // URL set: Resolve checksum if source.URL != "" { // If a URL is specified, we need to fetch the commit at the URL. - parent, err := ResolveRef(source.URL, parentRef, source.RHSM, nil, nil) + checksum, err := ResolveRef(source.URL, source.Ref, source.RHSM, nil, nil) if err != nil { - return "", "", err // ResolveRefError + return CommitSpec{}, err // ResolveRefError } - checksum = parent + commit.Checksum = checksum } - return + return commit, nil } diff --git a/internal/weldr/api.go b/internal/weldr/api.go index 098832ef5..c7ad202da 100644 --- a/internal/weldr/api.go +++ b/internal/weldr/api.go @@ -2341,21 +2341,19 @@ func (api *API) resolveOSTreeCommits(sourceSpecs map[string][]ostree.SourceSpec, for name, sources := range sourceSpecs { commits := make([]ostree.CommitSpec, len(sources)) for idx, source := range sources { - var ref, checksum string if test { - checksum = fmt.Sprintf("%x", sha256.Sum256([]byte(source.URL+source.Ref))) - ref = source.Ref + checksum := fmt.Sprintf("%x", sha256.Sum256([]byte(source.URL+source.Ref))) + commits[idx] = ostree.CommitSpec{ + Ref: source.Ref, + URL: source.URL, + Checksum: checksum, + } } else { - var err error - ref, checksum, err = ostree.Resolve(source) + commit, err := ostree.Resolve(source) if err != nil { return nil, err } - } - commits[idx] = ostree.CommitSpec{ - Ref: ref, - URL: source.URL, - Checksum: checksum, + commits[idx] = commit } } commitSpecs[name] = commits