From decd9d44ee3e38bddb0655d37905020e617e1f5c Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 8 May 2024 13:36:45 +0200 Subject: [PATCH] cmd: add helper for seed generation 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. --- cmd/build/main.go | 10 +++++----- cmd/gen-manifests/main.go | 14 +++++++------- internal/cmdutil/export_test.go | 3 +++ internal/cmdutil/rand.go | 26 ++++++++++++++++++++++++-- internal/cmdutil/rand_test.go | 29 +++++++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 14 deletions(-) create mode 100644 internal/cmdutil/export_test.go diff --git a/cmd/build/main.go b/cmd/build/main.go index 9f6180c351..eb50638dc6 100644 --- a/cmd/build/main.go +++ b/cmd/build/main.go @@ -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()) @@ -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 { @@ -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)) @@ -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") diff --git a/cmd/gen-manifests/main.go b/cmd/gen-manifests/main.go index fede29450b..9f4c9ce570 100644 --- a/cmd/gen-manifests/main.go +++ b/cmd/gen-manifests/main.go @@ -179,7 +179,6 @@ func makeManifestJob( distribution distro.Distro, repos []rpmmd.RepoConfig, archName string, - seedArg int64, cacheRoot string, path string, content map[string]bool, @@ -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 @@ -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)) @@ -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) } } diff --git a/internal/cmdutil/export_test.go b/internal/cmdutil/export_test.go new file mode 100644 index 0000000000..d400c554d6 --- /dev/null +++ b/internal/cmdutil/export_test.go @@ -0,0 +1,3 @@ +package cmdutil + +var NewRNGSeed = newRNGSeed diff --git a/internal/cmdutil/rand.go b/internal/cmdutil/rand.go index 4f22514aa5..a4a9b9a999 100644 --- a/internal/cmdutil/rand.go +++ b/internal/cmdutil/rand.go @@ -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 { @@ -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 +} diff --git a/internal/cmdutil/rand_test.go b/internal/cmdutil/rand_test.go index 6da4b4eb6e..9c16c91682 100644 --- a/internal/cmdutil/rand_test.go +++ b/internal/cmdutil/rand_test.go @@ -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" ) @@ -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) + } +}