Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg: make seed in ImageType.Manifest() random by default #1107

Merged
merged 1 commit into from
Dec 20, 2024
Merged

pkg: make seed in ImageType.Manifest() random by default #1107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/build/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func makeManifest(
return nil, err
}

manifest, warnings, err := imgType.Manifest(&bp, options, repos, seedArg)
manifest, warnings, err := imgType.Manifest(&bp, options, repos, &seedArg)
if err != nil {
return nil, fmt.Errorf("[ERROR] manifest generation failed: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/gen-manifests/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func makeManifestJob(
}()
msgq <- fmt.Sprintf("Starting job %s", filename)

manifest, _, err := imgType.Manifest(&bp, options, repos, seedArg)
manifest, _, err := imgType.Manifest(&bp, options, repos, &seedArg)
if err != nil {
err = fmt.Errorf("[%s] failed: %s", filename, err)
return
Expand Down
2 changes: 1 addition & 1 deletion cmd/osbuild-package-sets/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func main() {
URL: "https://example.com", // required by some image types
}
}
manifest, _, err := image.Manifest(&blueprint.Blueprint{}, options, nil, 0)
manifest, _, err := image.Manifest(&blueprint.Blueprint{}, options, nil, nil)
if err != nil {
panic(err)
}
Expand Down
13 changes: 12 additions & 1 deletion pkg/distro/distro.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package distro

import (
"time"

"github.com/osbuild/images/pkg/blueprint"
"github.com/osbuild/images/pkg/customizations/subscription"
"github.com/osbuild/images/pkg/disk"
Expand Down Expand Up @@ -120,7 +122,9 @@ type ImageType interface {
// specified in the given blueprint; it also returns any warnings (e.g.
// deprecation notices) generated by the manifest.
// The packageSpecSets must be labelled in the same way as the originating PackageSets.
Manifest(bp *blueprint.Blueprint, options ImageOptions, repos []rpmmd.RepoConfig, seed int64) (*manifest.Manifest, []string, error)
// A custom seed for the rng can be specified, if nil the seed will
// be random.
Comment on lines +125 to +126
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can be more accurate and informative here and say that it will be derived from the current time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'm happy to do that, I was thinking of using crypto.Rand to seed the generator but maybe I'm overthinking it. Happy to either document or change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not important. Can do later or not at all.

Manifest(bp *blueprint.Blueprint, options ImageOptions, repos []rpmmd.RepoConfig, seed *int64) (*manifest.Manifest, []string, error)
}

// The ImageOptions specify options for a specific image build
Expand Down Expand Up @@ -155,3 +159,10 @@ func ExportsFallback() []string {
func PayloadPackageSets() []string {
return []string{}
}

func SeedFrom(p *int64) int64 {
if p == nil {
return time.Now().UnixNano()
}
return *p
}
8 changes: 4 additions & 4 deletions pkg/distro/distro_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func TestImageTypePipelineNames(t *testing.T) {
repoSets[plName] = repos
}

m, _, err := imageType.Manifest(&bp, options, repos, seed)
m, _, err := imageType.Manifest(&bp, options, repos, &seed)
assert.NoError(err)

containers := make(map[string][]container.Spec, 0)
Expand Down Expand Up @@ -416,7 +416,7 @@ func TestPipelineRepositories(t *testing.T) {
}

repos := tCase.repos
manifest, _, err := imageType.Manifest(&bp, options, repos, 0)
manifest, _, err := imageType.Manifest(&bp, options, repos, nil)
require.NoError(err)
packageSets := manifest.GetPackageSetChains()

Expand Down Expand Up @@ -569,7 +569,7 @@ func TestDistro_ManifestFIPSWarning(t *testing.T) {
if strings.HasSuffix(imgTypeName, "simplified-installer") {
bp.Customizations.InstallationDevice = "/dev/dummy"
}
_, warn, err := imgType.Manifest(&bp, imgOpts, nil, 0)
_, warn, err := imgType.Manifest(&bp, imgOpts, nil, nil)
if err != nil {
assert.True(t, slices.Contains(noCustomizableImages, imgTypeName))
assert.Equal(t, err, fmt.Errorf(distro.NoCustomizationsAllowedError, imgTypeName))
Expand Down Expand Up @@ -626,7 +626,7 @@ func TestOSTreeOptionsErrorForNonOSTreeImgTypes(t *testing.T) {
},
}

_, _, err = imageType.Manifest(&bp, options, nil, 0)
_, _, err = imageType.Manifest(&bp, options, nil, nil)
if imageType.OSTreeRef() == "" {
assert.Errorf(err,
"OSTree options should not be allowed for non-OSTree image type %s/%s/%s",
Expand Down
14 changes: 7 additions & 7 deletions pkg/distro/distro_test_common/distro_test_common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func kernelCount(imgType distro.ImageType, bp blueprint.Blueprint) int {
}
}

manifest, _, err := imgType.Manifest(&bp, distro.ImageOptions{OSTree: ostreeOptions}, nil, 0)
manifest, _, err := imgType.Manifest(&bp, distro.ImageOptions{OSTree: ostreeOptions}, nil, nil)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -220,7 +220,7 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) {
}
options := distro.ImageOptions{OSTree: &ostreeOptions}

m, _, err := imgType.Manifest(bp, options, nil, 0)
m, _, err := imgType.Manifest(bp, options, nil, nil)
assert.NoError(err)

nrefs := 0
Expand Down Expand Up @@ -274,7 +274,7 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) {
ostreeOptions.URL = "https://example.com/repo"
}
options := distro.ImageOptions{OSTree: &ostreeOptions}
m, _, err := imgType.Manifest(bp, options, nil, 0)
m, _, err := imgType.Manifest(bp, options, nil, nil)
assert.NoError(err)

nrefs := 0
Expand Down Expand Up @@ -320,7 +320,7 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) {
URL: "https://example.com/repo",
}
options := distro.ImageOptions{OSTree: &ostreeOptions}
m, _, err := imgType.Manifest(bp, options, nil, 0)
m, _, err := imgType.Manifest(bp, options, nil, nil)
assert.NoError(err)

nrefs := 0
Expand Down Expand Up @@ -364,7 +364,7 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) {
URL: "https://example.com/repo",
}
options := distro.ImageOptions{OSTree: &ostreeOptions}
m, _, err := imgType.Manifest(bp, options, nil, 0)
m, _, err := imgType.Manifest(bp, options, nil, nil)
assert.NoError(err)

nrefs := 0
Expand Down Expand Up @@ -418,7 +418,7 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) {
URL: "https://example.com/repo",
}
options := distro.ImageOptions{OSTree: &ostreeOptions}
m, _, err := imgType.Manifest(bp, options, nil, 0)
m, _, err := imgType.Manifest(bp, options, nil, nil)
assert.NoError(err)

nrefs := 0
Expand Down Expand Up @@ -470,7 +470,7 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) {
ParentRef: "test/x86_64/02",
}
options := distro.ImageOptions{OSTree: &ostreeOptions}
_, _, err = imgType.Manifest(bp, options, nil, 0)
_, _, err = imgType.Manifest(bp, options, nil, nil)
assert.Error(err)
}
}
Expand Down
20 changes: 10 additions & 10 deletions pkg/distro/fedora/distro_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func TestImageType_BuildPackages(t *testing.T) {
if assert.NoErrorf(t, err, "d.GetArch(%v) returned err = %v; expected nil", archLabel, err) {
continue
}
manifest, _, err := itStruct.Manifest(&blueprint.Blueprint{}, distro.ImageOptions{}, nil, 0)
manifest, _, err := itStruct.Manifest(&blueprint.Blueprint{}, distro.ImageOptions{}, nil, nil)
assert.NoError(t, err)
buildPkgs := manifest.GetPackageSetChains()["build"]
assert.NotNil(t, buildPkgs)
Expand Down Expand Up @@ -507,7 +507,7 @@ func TestDistro_ManifestError(t *testing.T) {
imgOpts := distro.ImageOptions{
Size: imgType.Size(0),
}
_, _, err := imgType.Manifest(&bp, imgOpts, nil, 0)
_, _, err := imgType.Manifest(&bp, imgOpts, nil, nil)
if imgTypeName == "iot-commit" || imgTypeName == "iot-container" || imgTypeName == "iot-bootable-container" {
assert.EqualError(t, err, "kernel boot parameter customizations are not supported for ostree types")
} else if imgTypeName == "iot-installer" || imgTypeName == "iot-simplified-installer" {
Expand Down Expand Up @@ -738,7 +738,7 @@ func TestDistro_CustomFileSystemManifestError(t *testing.T) {
arch, _ := fedoraDistro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if imgTypeName == "iot-commit" || imgTypeName == "iot-container" || imgTypeName == "iot-bootable-container" {
assert.EqualError(t, err, "Custom mountpoints and partitioning are not supported for ostree types")
} else if imgTypeName == "iot-raw-image" || imgTypeName == "iot-qcow2-image" {
Expand Down Expand Up @@ -772,7 +772,7 @@ func TestDistro_TestRootMountPoint(t *testing.T) {
arch, _ := fedoraDistro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if imgTypeName == "iot-commit" || imgTypeName == "iot-container" || imgTypeName == "iot-bootable-container" {
assert.EqualError(t, err, "Custom mountpoints and partitioning are not supported for ostree types")
} else if imgTypeName == "iot-raw-image" || imgTypeName == "iot-qcow2-image" {
Expand Down Expand Up @@ -810,7 +810,7 @@ func TestDistro_CustomFileSystemSubDirectories(t *testing.T) {
arch, _ := fedoraDistro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if strings.HasPrefix(imgTypeName, "iot-") || strings.HasPrefix(imgTypeName, "image-") {
continue
} else if imgTypeName == "live-installer" {
Expand Down Expand Up @@ -852,7 +852,7 @@ func TestDistro_MountpointsWithArbitraryDepthAllowed(t *testing.T) {
arch, _ := fedoraDistro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if strings.HasPrefix(imgTypeName, "iot-") || strings.HasPrefix(imgTypeName, "image-") {
continue
} else if imgTypeName == "live-installer" {
Expand Down Expand Up @@ -890,7 +890,7 @@ func TestDistro_DirtyMountpointsNotAllowed(t *testing.T) {
arch, _ := fedoraDistro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if strings.HasPrefix(imgTypeName, "iot-") || strings.HasPrefix(imgTypeName, "image-") {
continue
} else if imgTypeName == "live-installer" {
Expand Down Expand Up @@ -920,7 +920,7 @@ func TestDistro_CustomUsrPartitionNotLargeEnough(t *testing.T) {
arch, _ := fedoraDistro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if imgTypeName == "iot-commit" || imgTypeName == "iot-container" || imgTypeName == "iot-bootable-container" {
assert.EqualError(t, err, "Custom mountpoints and partitioning are not supported for ostree types")
} else if imgTypeName == "iot-raw-image" || imgTypeName == "iot-qcow2-image" {
Expand Down Expand Up @@ -965,7 +965,7 @@ func TestDistro_PartitioningConflict(t *testing.T) {
arch, _ := fedoraDistro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if imgTypeName == "iot-commit" || imgTypeName == "iot-container" || imgTypeName == "iot-bootable-container" {
assert.EqualError(t, err, "Custom mountpoints and partitioning are not supported for ostree types")
} else if imgTypeName == "iot-raw-image" || imgTypeName == "iot-qcow2-image" {
Expand Down Expand Up @@ -1063,7 +1063,7 @@ func TestDistro_DiskCustomizationRunsValidateLayoutConstraints(t *testing.T) {
imgOpts := distro.ImageOptions{
Size: imgType.Size(0),
}
_, _, err := imgType.Manifest(&bp, imgOpts, nil, 0)
_, _, err := imgType.Manifest(&bp, imgOpts, nil, nil)
assert.EqualError(t, err, "multiple LVM volume groups are not yet supported")
})
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/distro/fedora/imagetype.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ func (t *imageType) PartitionType() disk.PartitionTableType {
func (t *imageType) Manifest(bp *blueprint.Blueprint,
options distro.ImageOptions,
repos []rpmmd.RepoConfig,
seed int64) (*manifest.Manifest, []string, error) {
seedp *int64) (*manifest.Manifest, []string, error) {
seed := distro.SeedFrom(seedp)

warnings, err := t.checkOptions(bp, options)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion pkg/distro/rhel/imagetype.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ func (t *ImageType) PartitionType() disk.PartitionTableType {
func (t *ImageType) Manifest(bp *blueprint.Blueprint,
options distro.ImageOptions,
repos []rpmmd.RepoConfig,
seed int64) (*manifest.Manifest, []string, error) {
seedp *int64) (*manifest.Manifest, []string, error) {
seed := distro.SeedFrom(seedp)

if t.Workload != nil {
// For now, if an image type defines its own workload, don't allow any
Expand Down
20 changes: 10 additions & 10 deletions pkg/distro/rhel/rhel10/distro_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func TestImageType_BuildPackages(t *testing.T) {
if assert.NoErrorf(t, err, "d.GetArch(%v) returned err = %v; expected nil", archLabel, err) {
continue
}
manifest, _, err := itStruct.Manifest(&blueprint.Blueprint{}, distro.ImageOptions{}, nil, 0)
manifest, _, err := itStruct.Manifest(&blueprint.Blueprint{}, distro.ImageOptions{}, nil, nil)
assert.NoError(t, err)
buildPkgs := manifest.GetPackageSetChains()["build"]
assert.NotNil(t, buildPkgs)
Expand Down Expand Up @@ -257,7 +257,7 @@ func TestDistro_ManifestError(t *testing.T) {
imgOpts := distro.ImageOptions{
Size: imgType.Size(0),
}
_, _, err := imgType.Manifest(&bp, imgOpts, nil, 0)
_, _, err := imgType.Manifest(&bp, imgOpts, nil, nil)
assert.NoError(t, err)
}
}
Expand Down Expand Up @@ -427,7 +427,7 @@ func TestDistro_CustomFileSystemManifestError(t *testing.T) {
arch, _ := r10distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
assert.EqualError(t, err, "The following errors occurred while setting up custom mountpoints:\npath \"/etc\" is not allowed")
}
}
Expand All @@ -449,7 +449,7 @@ func TestDistro_TestRootMountPoint(t *testing.T) {
arch, _ := r10distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
assert.NoError(t, err)
}
}
Expand All @@ -475,7 +475,7 @@ func TestDistro_CustomFileSystemSubDirectories(t *testing.T) {
arch, _ := r10distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if strings.HasPrefix(imgTypeName, "edge-") {
continue
} else {
Expand Down Expand Up @@ -513,7 +513,7 @@ func TestDistro_MountpointsWithArbitraryDepthAllowed(t *testing.T) {
arch, _ := r10distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if strings.HasPrefix(imgTypeName, "edge-") {
continue
} else {
Expand Down Expand Up @@ -547,7 +547,7 @@ func TestDistro_DirtyMountpointsNotAllowed(t *testing.T) {
arch, _ := r10distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
assert.EqualError(t, err, "The following errors occurred while setting up custom mountpoints:\npath \"//\" must be canonical\npath \"/var//\" must be canonical\npath \"/var//log/audit/\" must be canonical")
}
}
Expand All @@ -569,7 +569,7 @@ func TestDistro_CustomUsrPartitionNotLargeEnough(t *testing.T) {
arch, _ := r10distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
assert.NoError(t, err)
}
}
Expand Down Expand Up @@ -618,7 +618,7 @@ func TestDiskAndFilesystemCustomizationsError(t *testing.T) {
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
options := distro.ImageOptions{}
_, _, err := imgType.Manifest(&bp, options, nil, 0)
_, _, err := imgType.Manifest(&bp, options, nil, nil)
if skipTest[imgTypeName] {
continue
}
Expand Down Expand Up @@ -664,7 +664,7 @@ func TestNoDiskCustomizationsNoError(t *testing.T) {
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
options := distro.ImageOptions{}
_, _, err := imgType.Manifest(&bp, options, nil, 0)
_, _, err := imgType.Manifest(&bp, options, nil, nil)
if skipTest[imgTypeName] {
continue
}
Expand Down
Loading
Loading