Skip to content

Commit

Permalink
cmd: add helper for seed generation
Browse files Browse the repository at this point in the history
To ensure that manifests get random(ish) and stable UUIDs we set the rng seed arg based on the filename.

This should fix the issue discovered in
osbuild/manifest-db#124

that duplicated UUIDs for xfs/btrfs can trigger random(ish) and hard to diagnose errors.

This is the same as
osbuild/osbuild-composer#4124 hopefully for the right place this time.
  • Loading branch information
mvo5 committed May 14, 2024
1 parent 9316652 commit 4df0926
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 14 deletions.
10 changes: 5 additions & 5 deletions cmd/build/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ func makeManifest(
distribution distro.Distro,
repos []rpmmd.RepoConfig,
archName string,
seedArg int64,
cacheRoot string,
) (manifest.OSBuildManifest, error) {
cacheDir := filepath.Join(cacheRoot, archName+distribution.Name())
Expand All @@ -58,6 +57,10 @@ func makeManifest(
if config.Blueprint != nil {
bp = blueprint.Blueprint(*config.Blueprint)
}
seedArg, err := cmdutil.SeedArgFor(config, imgType, distribution, archName)
if err != nil {
return nil, err
}

manifest, warnings, err := imgType.Manifest(&bp, options, repos, seedArg)
if err != nil {
Expand Down Expand Up @@ -209,9 +212,6 @@ func main() {
os.Exit(1)
}

rngSeed, err := cmdutil.NewRNGSeed()
check(err)

testedRepoRegistry, err := reporegistry.NewTestedDefault()
if err != nil {
panic(fmt.Sprintf("failed to create repo registry with tested distros: %v", err))
Expand Down Expand Up @@ -258,7 +258,7 @@ func main() {
}

fmt.Printf("Generating manifest for %s: ", config.Name)
mf, err := makeManifest(config, imgType, distribution, repos, archName, rngSeed, rpmCacheRoot)
mf, err := makeManifest(config, imgType, distribution, repos, archName, rpmCacheRoot)
check(err)
fmt.Print("DONE\n")

Expand Down
14 changes: 7 additions & 7 deletions cmd/gen-manifests/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ func makeManifestJob(
distribution distro.Distro,
repos []rpmmd.RepoConfig,
archName string,
seedArg int64,
cacheRoot string,
path string,
content map[string]bool,
Expand All @@ -190,6 +189,12 @@ func makeManifestJob(
filename := fmt.Sprintf("%s-%s-%s-%s.json", u(distroName), u(archName), u(imgType.Name()), u(name))
cacheDir := filepath.Join(cacheRoot, archName+distribution.Name())

// ensure that each file has a unique seed based on filename
seedArg, err := cmdutil.SeedArgFor(bc, imgType, distribution, archName)
if err != nil {
panic(err)
}

options := bc.Options

var bp blueprint.Blueprint
Expand Down Expand Up @@ -508,11 +513,6 @@ func main() {

flag.Parse()

rngSeed, err := cmdutil.NewRNGSeed()
if err != nil {
panic(err)
}

testedRepoRegistry, err := reporegistry.NewTestedDefault()
if err != nil {
panic(fmt.Sprintf("failed to create repo registry with tested distros: %v", err))
Expand Down Expand Up @@ -598,7 +598,7 @@ func main() {
}

for _, itConfig := range imgTypeConfigs {
job := makeManifestJob(itConfig, imgType, distribution, repos, archName, rngSeed, cacheRoot, outputDir, contentResolve, metadata)
job := makeManifestJob(itConfig, imgType, distribution, repos, archName, cacheRoot, outputDir, contentResolve, metadata)
jobs = append(jobs, job)
}
}
Expand Down
3 changes: 3 additions & 0 deletions internal/cmdutil/export_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package cmdutil

var NewRNGSeed = newRNGSeed
26 changes: 24 additions & 2 deletions internal/cmdutil/rand.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,20 @@ package cmdutil
import (
"crypto/rand"
"fmt"
"hash/fnv"
"math"
"math/big"
"os"
"strconv"

"github.com/osbuild/images/internal/buildconfig"
)

const RNG_SEED_ENV_KEY = "OSBUILD_TESTING_RNG_SEED"

// NewRNGSeed generates a random seed value unless the env var
// newRNGSeed generates a random seed value unless the env var
// OSBUILD_TESTING_RNG_SEED is set.
func NewRNGSeed() (int64, error) {
func newRNGSeed() (int64, error) {
if envSeedStr := os.Getenv(RNG_SEED_ENV_KEY); envSeedStr != "" {
envSeedInt, err := strconv.ParseInt(envSeedStr, 10, 64)
if err != nil {
Expand All @@ -28,3 +31,22 @@ func NewRNGSeed() (int64, error) {
}
return randSeed.Int64(), nil
}

type namer interface {
Name() string
}

func SeedArgFor(bc *buildconfig.BuildConfig, imgType namer, distribution namer, archName string) (int64, error) {
rngSeed, err := newRNGSeed()
if err != nil {
return 0, err
}

h := fnv.New64()
h.Write([]byte(distribution.Name()))
h.Write([]byte(archName))
h.Write([]byte(imgType.Name()))
h.Write([]byte(bc.Name))

return rngSeed + int64(h.Sum64()), nil
}
29 changes: 29 additions & 0 deletions internal/cmdutil/rand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/osbuild/images/internal/buildconfig"
"github.com/osbuild/images/internal/cmdutil"
)

Expand Down Expand Up @@ -38,3 +39,31 @@ func TestNewRNGSeed(t *testing.T) {
require.EqualError(t, err, fmt.Sprintf(`failed to parse %s: strconv.ParseInt: parsing "NaN": invalid syntax`, cmdutil.RNG_SEED_ENV_KEY))
})
}

type fakeNamer struct {
fakeName string
}

func (fn fakeNamer) Name() string {
return fn.fakeName
}

func TestSeedArgFor(t *testing.T) {
t.Setenv(cmdutil.RNG_SEED_ENV_KEY, "1234")

for _, tc := range []struct {
bcName, imgType, distro, archName string
expectedSeed int64
}{
{"bcName", "fakeImgType", "fakeDistro", "x86_64", 9170052743323116054},
{"bcName1", "fakeImgType", "fakeDistro", "x86_64", -7134826073208782961},
{"bcName", "fakeImgType1", "fakeDistro", "x86_64", 4026045880862600579},
{"bcName", "fakeImgType", "fakeDistro1", "x86_64", 3669869122697339647},
{"bcName", "fakeImgType", "fakeDistro1", "aarch64", 47752167762999679},
} {
bc := &buildconfig.BuildConfig{Name: tc.bcName}
seedArg, err := cmdutil.SeedArgFor(bc, fakeNamer{tc.imgType}, fakeNamer{tc.distro}, tc.archName)
assert.NoError(t, err)
assert.Equal(t, tc.expectedSeed, seedArg)
}
}

0 comments on commit 4df0926

Please sign in to comment.