Skip to content

Commit

Permalink
manifest: change the serializeStart() to take a new Inputs arg
Browse files Browse the repository at this point in the history
The signature of serializeStart() is a bit unwieldy and when
adding librepo based sources this prompted for this proposed
cleanup. The idea is to just pass a single "Inputs" struct
to the serialization so that if we add field we don't need
to change the signature all over the place. It also makes
the calls easier to read (IMHO).
  • Loading branch information
mvo5 authored and ondrejbudai committed Jan 14, 2025
1 parent afcde39 commit 5fafb37
Show file tree
Hide file tree
Showing 17 changed files with 95 additions and 96 deletions.
8 changes: 3 additions & 5 deletions pkg/manifest/anaconda_installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@ import (

"github.com/osbuild/images/internal/common"
"github.com/osbuild/images/pkg/arch"
"github.com/osbuild/images/pkg/container"
"github.com/osbuild/images/pkg/customizations/fsnode"
"github.com/osbuild/images/pkg/customizations/kickstart"
"github.com/osbuild/images/pkg/customizations/users"
"github.com/osbuild/images/pkg/osbuild"
"github.com/osbuild/images/pkg/ostree"
"github.com/osbuild/images/pkg/platform"
"github.com/osbuild/images/pkg/rpmmd"
)
Expand Down Expand Up @@ -196,15 +194,15 @@ func (p *AnacondaInstaller) getPackageSpecs() []rpmmd.PackageSpec {
return p.packageSpecs
}

func (p *AnacondaInstaller) serializeStart(packages []rpmmd.PackageSpec, _ []container.Spec, _ []ostree.CommitSpec, rpmRepos []rpmmd.RepoConfig) {
func (p *AnacondaInstaller) serializeStart(inputs Inputs) {
if len(p.packageSpecs) > 0 {
panic("double call to serializeStart()")
}
p.packageSpecs = packages
p.packageSpecs = inputs.Packages
if p.kernelName != "" {
p.kernelVer = rpmmd.GetVerStrFromPackageSpecListPanic(p.packageSpecs, p.kernelName)
}
p.repos = append(p.repos, rpmRepos...)
p.repos = append(p.repos, inputs.RpmRepos...)
}

func (p *AnacondaInstaller) serializeEnd() {
Expand Down
15 changes: 7 additions & 8 deletions pkg/manifest/anaconda_installer_iso_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/osbuild/images/pkg/disk"
"github.com/osbuild/images/pkg/osbuild"
"github.com/osbuild/images/pkg/ostree"
"github.com/osbuild/images/pkg/rpmmd"
)

type RootfsType uint64
Expand Down Expand Up @@ -227,25 +226,25 @@ func (p *AnacondaInstallerISOTree) NewErofsStage() *osbuild.Stage {
return osbuild.NewErofsStage(&erofsOptions, p.anacondaPipeline.Name())
}

func (p *AnacondaInstallerISOTree) serializeStart(_ []rpmmd.PackageSpec, containers []container.Spec, commits []ostree.CommitSpec, _ []rpmmd.RepoConfig) {
func (p *AnacondaInstallerISOTree) serializeStart(inputs Inputs) {
if p.ostreeCommitSpec != nil || p.containerSpec != nil {
panic("double call to serializeStart()")
}

if len(commits) > 1 {
if len(inputs.Commits) > 1 {
panic("pipeline supports at most one ostree commit")
}

if len(containers) > 1 {
if len(inputs.Containers) > 1 {
panic("pipeline supports at most one container")
}

if len(commits) > 0 {
p.ostreeCommitSpec = &commits[0]
if len(inputs.Commits) > 0 {
p.ostreeCommitSpec = &inputs.Commits[0]
}

if len(containers) > 0 {
p.containerSpec = &containers[0]
if len(inputs.Containers) > 0 {
p.containerSpec = &inputs.Containers[0]
}
}

Expand Down
60 changes: 30 additions & 30 deletions pkg/manifest/anaconda_installer_iso_tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,11 +285,11 @@ func TestAnacondaISOTreePayloadsBad(t *testing.T) {

assert.PanicsWithValue(
"pipeline supports at most one ostree commit",
func() { pipeline.serializeStart(nil, nil, make([]ostree.CommitSpec, 2), nil) },
func() { pipeline.serializeStart(Inputs{Commits: make([]ostree.CommitSpec, 2)}) },
)
assert.PanicsWithValue(
"pipeline supports at most one container",
func() { pipeline.serializeStart(nil, make([]container.Spec, 2), nil, nil) },
func() { pipeline.serializeStart(Inputs{Containers: make([]container.Spec, 2)}) },
)
}

Expand All @@ -309,7 +309,7 @@ func TestAnacondaISOTreeSerializeWithOS(t *testing.T) {
t.Run("plain", func(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.OSPipeline = osPayload
pipeline.serializeStart(nil, nil, nil, nil)
pipeline.serializeStart(Inputs{})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, payloadStages,
Expand All @@ -322,7 +322,7 @@ func TestAnacondaISOTreeSerializeWithOS(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.OSPipeline = osPayload
pipeline.Kickstart = &kickstart.Options{Path: testKsPath}
pipeline.serializeStart(nil, nil, nil, nil)
pipeline.serializeStart(Inputs{})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, append(payloadStages, "org.osbuild.kickstart"),
Expand All @@ -335,7 +335,7 @@ func TestAnacondaISOTreeSerializeWithOS(t *testing.T) {
pipeline.OSPipeline = osPayload
pipeline.Kickstart = &kickstart.Options{Path: testKsPath}
pipeline.ISOLinux = true
pipeline.serializeStart(nil, nil, nil, nil)
pipeline.serializeStart(Inputs{})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, append(payloadStages, "org.osbuild.isolinux", "org.osbuild.kickstart"),
Expand All @@ -347,7 +347,7 @@ func TestAnacondaISOTreeSerializeWithOS(t *testing.T) {
pipeline.OSPipeline = osPayload
pipeline.Kickstart = &kickstart.Options{Path: testKsPath, Unattended: true}
pipeline.ISOLinux = true
pipeline.serializeStart(nil, nil, nil, nil)
pipeline.serializeStart(Inputs{})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, append(payloadStages, "org.osbuild.isolinux", "org.osbuild.kickstart"),
Expand All @@ -364,7 +364,7 @@ func TestAnacondaISOTreeSerializeWithOS(t *testing.T) {
SudoNopasswd: []string{`%wheel`, `%sudo`},
}
pipeline.ISOLinux = true
pipeline.serializeStart(nil, nil, nil, nil)
pipeline.serializeStart(Inputs{})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, append(payloadStages, "org.osbuild.isolinux", "org.osbuild.kickstart"),
Expand All @@ -384,7 +384,7 @@ func TestAnacondaISOTreeSerializeWithOS(t *testing.T) {
},
}
pipeline.ISOLinux = true
pipeline.serializeStart(nil, nil, nil, nil)
pipeline.serializeStart(Inputs{})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, append(payloadStages, "org.osbuild.isolinux", "org.osbuild.kickstart"),
Expand All @@ -404,7 +404,7 @@ func TestAnacondaISOTreeSerializeWithOS(t *testing.T) {
},
}
pipeline.ISOLinux = true
pipeline.serializeStart(nil, nil, nil, nil)
pipeline.serializeStart(Inputs{})
assert.Panics(t, func() { pipeline.serialize() })
})

Expand All @@ -420,15 +420,15 @@ func TestAnacondaISOTreeSerializeWithOS(t *testing.T) {
},
}
pipeline.ISOLinux = true
pipeline.serializeStart(nil, nil, nil, nil)
pipeline.serializeStart(Inputs{})
assert.Panics(t, func() { pipeline.serialize() })
})

t.Run("plain+squashfs-rootfs", func(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.OSPipeline = osPayload
pipeline.RootfsType = SquashfsRootfs
pipeline.serializeStart(nil, nil, nil, nil)
pipeline.serializeStart(Inputs{})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, payloadStages,
Expand All @@ -440,7 +440,7 @@ func TestAnacondaISOTreeSerializeWithOS(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.OSPipeline = osPayload
pipeline.RootfsType = ErofsRootfs
pipeline.serializeStart(nil, nil, nil, nil)
pipeline.serializeStart(Inputs{})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages,
Expand Down Expand Up @@ -472,7 +472,7 @@ func TestAnacondaISOTreeSerializeWithOSTree(t *testing.T) {
t.Run("plain", func(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.Kickstart = &kickstart.Options{Path: testKsPath, OSTree: &kickstart.OSTree{}}
pipeline.serializeStart(nil, nil, []ostree.CommitSpec{ostreeCommit}, nil)
pipeline.serializeStart(Inputs{Commits: []ostree.CommitSpec{ostreeCommit}})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, payloadStages,
Expand All @@ -484,7 +484,7 @@ func TestAnacondaISOTreeSerializeWithOSTree(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.Kickstart = &kickstart.Options{Path: testKsPath, OSTree: &kickstart.OSTree{}}
pipeline.ISOLinux = true
pipeline.serializeStart(nil, nil, []ostree.CommitSpec{ostreeCommit}, nil)
pipeline.serializeStart(Inputs{Commits: []ostree.CommitSpec{ostreeCommit}})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, append(payloadStages, "org.osbuild.isolinux"), variantStages))
Expand All @@ -494,7 +494,7 @@ func TestAnacondaISOTreeSerializeWithOSTree(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.Kickstart = &kickstart.Options{Path: testKsPath, Unattended: true, OSTree: &kickstart.OSTree{}}
pipeline.ISOLinux = true
pipeline.serializeStart(nil, nil, []ostree.CommitSpec{ostreeCommit}, nil)
pipeline.serializeStart(Inputs{Commits: []ostree.CommitSpec{ostreeCommit}})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, append(payloadStages, "org.osbuild.isolinux"), variantStages))
Expand All @@ -510,7 +510,7 @@ func TestAnacondaISOTreeSerializeWithOSTree(t *testing.T) {
OSTree: &kickstart.OSTree{},
}
pipeline.ISOLinux = true
pipeline.serializeStart(nil, nil, []ostree.CommitSpec{ostreeCommit}, nil)
pipeline.serializeStart(Inputs{Commits: []ostree.CommitSpec{ostreeCommit}})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, append(payloadStages, "org.osbuild.isolinux"), variantStages))
Expand All @@ -529,7 +529,7 @@ func TestAnacondaISOTreeSerializeWithOSTree(t *testing.T) {
OSTree: &kickstart.OSTree{},
}
pipeline.ISOLinux = true
pipeline.serializeStart(nil, nil, []ostree.CommitSpec{ostreeCommit}, nil)
pipeline.serializeStart(Inputs{Commits: []ostree.CommitSpec{ostreeCommit}})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, append(payloadStages, "org.osbuild.isolinux"), variantStages))
Expand All @@ -548,7 +548,7 @@ func TestAnacondaISOTreeSerializeWithOSTree(t *testing.T) {
OSTree: &kickstart.OSTree{},
}
pipeline.ISOLinux = true
pipeline.serializeStart(nil, nil, []ostree.CommitSpec{ostreeCommit}, nil)
pipeline.serializeStart(Inputs{Commits: []ostree.CommitSpec{ostreeCommit}})
assert.Panics(t, func() { pipeline.serialize() })
})

Expand All @@ -565,15 +565,15 @@ func TestAnacondaISOTreeSerializeWithOSTree(t *testing.T) {
OSTree: &kickstart.OSTree{},
}
pipeline.ISOLinux = true
pipeline.serializeStart(nil, nil, []ostree.CommitSpec{ostreeCommit}, nil)
pipeline.serializeStart(Inputs{Commits: []ostree.CommitSpec{ostreeCommit}})
assert.Panics(t, func() { pipeline.serialize() })
})

t.Run("plain+squashfs-rootfs", func(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.RootfsType = SquashfsRootfs
pipeline.Kickstart = &kickstart.Options{Path: testKsPath, OSTree: &kickstart.OSTree{}}
pipeline.serializeStart(nil, nil, []ostree.CommitSpec{ostreeCommit}, nil)
pipeline.serializeStart(Inputs{Commits: []ostree.CommitSpec{ostreeCommit}})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, payloadStages,
Expand All @@ -585,7 +585,7 @@ func TestAnacondaISOTreeSerializeWithOSTree(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.RootfsType = ErofsRootfs
pipeline.Kickstart = &kickstart.Options{Path: testKsPath, OSTree: &kickstart.OSTree{}}
pipeline.serializeStart(nil, nil, []ostree.CommitSpec{ostreeCommit}, nil)
pipeline.serializeStart(Inputs{Commits: []ostree.CommitSpec{ostreeCommit}})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages,
Expand Down Expand Up @@ -621,7 +621,7 @@ func TestAnacondaISOTreeSerializeWithContainer(t *testing.T) {
t.Run("kspath", func(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.Kickstart = &kickstart.Options{Path: testKsPath}
pipeline.serializeStart(nil, []container.Spec{containerPayload}, nil, nil)
pipeline.serializeStart(Inputs{Containers: []container.Spec{containerPayload}})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, payloadStages,
Expand All @@ -633,7 +633,7 @@ func TestAnacondaISOTreeSerializeWithContainer(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.Kickstart = &kickstart.Options{Path: testKsPath}
pipeline.ISOLinux = true
pipeline.serializeStart(nil, []container.Spec{containerPayload}, nil, nil)
pipeline.serializeStart(Inputs{Containers: []container.Spec{containerPayload}})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, append(payloadStages, "org.osbuild.isolinux"), variantStages))
Expand All @@ -646,7 +646,7 @@ func TestAnacondaISOTreeSerializeWithContainer(t *testing.T) {
Unattended: true,
KernelOptionsAppend: []string{"kernel.opt=1", "debug"},
}
pipeline.serializeStart(nil, []container.Spec{containerPayload}, nil, nil)
pipeline.serializeStart(Inputs{Containers: []container.Spec{containerPayload}})
sp := pipeline.serialize()
pipeline.serializeEnd()
kickstartSt := findStage("org.osbuild.kickstart", sp.Stages)
Expand All @@ -658,7 +658,7 @@ func TestAnacondaISOTreeSerializeWithContainer(t *testing.T) {
t.Run("network-on-boot", func(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.Kickstart = &kickstart.Options{Path: testKsPath, NetworkOnBoot: true}
pipeline.serializeStart(nil, []container.Spec{containerPayload}, nil, nil)
pipeline.serializeStart(Inputs{Containers: []container.Spec{containerPayload}})
sp := pipeline.serialize()
pipeline.serializeEnd()
kickstartSt := findStage("org.osbuild.kickstart", sp.Stages)
Expand All @@ -678,7 +678,7 @@ func TestAnacondaISOTreeSerializeWithContainer(t *testing.T) {
},
}
pipeline.ISOLinux = true
pipeline.serializeStart(nil, []container.Spec{containerPayload}, nil, nil)
pipeline.serializeStart(Inputs{Containers: []container.Spec{containerPayload}})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, append(payloadStages, "org.osbuild.isolinux"), variantStages))
Expand All @@ -689,7 +689,7 @@ func TestAnacondaISOTreeSerializeWithContainer(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.Kickstart = &kickstart.Options{Path: testKsPath}
pipeline.PayloadRemoveSignatures = true
pipeline.serializeStart(nil, []container.Spec{containerPayload}, nil, nil)
pipeline.serializeStart(Inputs{Containers: []container.Spec{containerPayload}})
sp := pipeline.serialize()
pipeline.serializeEnd()
skopeoStage := findStage("org.osbuild.skopeo", sp.Stages)
Expand All @@ -701,7 +701,7 @@ func TestAnacondaISOTreeSerializeWithContainer(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.RootfsType = SquashfsRootfs
pipeline.Kickstart = &kickstart.Options{Path: testKsPath}
pipeline.serializeStart(nil, []container.Spec{containerPayload}, nil, nil)
pipeline.serializeStart(Inputs{Containers: []container.Spec{containerPayload}})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages, payloadStages,
Expand All @@ -713,7 +713,7 @@ func TestAnacondaISOTreeSerializeWithContainer(t *testing.T) {
pipeline := newTestAnacondaISOTree()
pipeline.RootfsType = ErofsRootfs
pipeline.Kickstart = &kickstart.Options{Path: testKsPath}
pipeline.serializeStart(nil, []container.Spec{containerPayload}, nil, nil)
pipeline.serializeStart(Inputs{Containers: []container.Spec{containerPayload}})
sp := pipeline.serialize()
pipeline.serializeEnd()
assert.NoError(t, checkISOTreeStages(sp.Stages,
Expand Down Expand Up @@ -745,7 +745,7 @@ restorecon -rvF /etc/sudoers.d

func stagesFrom(pipeline Pipeline) []*osbuild.Stage {
containerPayload := makeFakeContainerPayload()
pipeline.serializeStart(nil, []container.Spec{containerPayload}, nil, nil)
pipeline.serializeStart(Inputs{Containers: []container.Spec{containerPayload}})
sp := pipeline.serialize()
pipeline.serializeEnd()
return sp.Stages
Expand Down
2 changes: 1 addition & 1 deletion pkg/manifest/anaconda_installer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func TestAnacondaInstallerModules(t *testing.T) {
installerPipeline.UseLegacyAnacondaConfig = legacy
installerPipeline.AdditionalAnacondaModules = tc.enable
installerPipeline.DisabledAnacondaModules = tc.disable
installerPipeline.serializeStart(pkgs, nil, nil, nil)
installerPipeline.serializeStart(Inputs{Packages: pkgs})
pipeline := installerPipeline.serialize()

require := require.New(t)
Expand Down
11 changes: 5 additions & 6 deletions pkg/manifest/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (

"github.com/osbuild/images/pkg/container"
"github.com/osbuild/images/pkg/osbuild"
"github.com/osbuild/images/pkg/ostree"
"github.com/osbuild/images/pkg/rpmmd"
"github.com/osbuild/images/pkg/runner"
)
Expand Down Expand Up @@ -99,12 +98,12 @@ func (p *BuildrootFromPackages) getPackageSpecs() []rpmmd.PackageSpec {
return p.packageSpecs
}

func (p *BuildrootFromPackages) serializeStart(packages []rpmmd.PackageSpec, _ []container.Spec, _ []ostree.CommitSpec, rpmRepos []rpmmd.RepoConfig) {
func (p *BuildrootFromPackages) serializeStart(inputs Inputs) {
if len(p.packageSpecs) > 0 {
panic("double call to serializeStart()")
}
p.packageSpecs = packages
p.repos = append(p.repos, rpmRepos...)
p.packageSpecs = inputs.Packages
p.repos = append(p.repos, inputs.RpmRepos...)
}

func (p *BuildrootFromPackages) serializeEnd() {
Expand Down Expand Up @@ -199,11 +198,11 @@ func (p *BuildrootFromContainer) getContainerSpecs() []container.Spec {
return p.containerSpecs
}

func (p *BuildrootFromContainer) serializeStart(_ []rpmmd.PackageSpec, containerSpecs []container.Spec, _ []ostree.CommitSpec, _ []rpmmd.RepoConfig) {
func (p *BuildrootFromContainer) serializeStart(inputs Inputs) {
if len(p.containerSpecs) > 0 {
panic("double call to serializeStart()")
}
p.containerSpecs = containerSpecs
p.containerSpecs = inputs.Containers
}

func (p *BuildrootFromContainer) serializeEnd() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/manifest/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func TestNewBuildFromContainerSpecs(t *testing.T) {
}
// containerSpecs is "nil" until serializeStart populates it
require.Nil(t, build.getContainerSpecs())
build.serializeStart(nil, fakeContainerSpecs, nil, nil)
build.serializeStart(Inputs{Containers: fakeContainerSpecs})
assert.Equal(t, build.getContainerSpecs(), fakeContainerSpecs)

osbuildPipeline := build.serialize()
Expand Down
Loading

0 comments on commit 5fafb37

Please sign in to comment.