From 4df09268cf6afb920249e023ea0fa38718a115c6 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 9f6180c35..eb50638dc 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 fede29450..9f4c9ce57 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 000000000..d400c554d --- /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 4f22514aa..a4a9b9a99 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 6da4b4eb6..9c16c9168 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) + } +}