From cd98fc4fc8ddc16548b8a6b8bebaae7052b2a453 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 24 Oct 2024 15:19:26 -0700 Subject: [PATCH 1/3] Fix custom repo test The main fix here is that the test was using the same package name/version for both test cases. Due to this, the package being tested may already be in the apt package cache when the negative test runs. This can cause unexpected results. Also some minor refactoring so that the proper `*testing.T` is used. This fixes a problem with the test build not producing any logs. Signed-off-by: Brian Goff --- test/azlinux_test.go | 154 ++++++++++++++++++++++++------------------- test/windows_test.go | 2 +- 2 files changed, 86 insertions(+), 70 deletions(-) diff --git a/test/azlinux_test.go b/test/azlinux_test.go index 37d218a32..558da372a 100644 --- a/test/azlinux_test.go +++ b/test/azlinux_test.go @@ -1415,7 +1415,7 @@ func testCustomLinuxWorker(ctx context.Context, t *testing.T, targetCfg targetCo testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) { // base package that will be used as a build dependency of the main package. depSpec := &dalec.Spec{ - Name: "dalec-test-package", + Name: "dalec-test-package-custom-worker-dep", Version: "0.0.1", Revision: "1", Description: "A basic package for various testing uses", @@ -1543,30 +1543,37 @@ EOF } func testCustomRepo(ctx context.Context, t *testing.T, cfg testLinuxConfig) { - depSpec := &dalec.Spec{ - Name: "dalec-test-package", - Version: "0.0.1", - Revision: "1", - Description: "A basic package for various testing uses", - License: "MIT", - Sources: map[string]dalec.Source{ - "version.txt": { - Inline: &dalec.SourceInline{ - File: &dalec.SourceInlineFile{ - Contents: "version: " + "0.0.1", + // provide a unique suffix per test otherwise, depending on the test case, + // you can end up with a false positive result due to apt package caching. + // e.g. there may not be a public key for the repo under test, but if the + // package is already in the package cache (due to other tests that injected + // a public key) then apt may use that package anyway. + getDepSpec := func(suffix string) *dalec.Spec { + return &dalec.Spec{ + Name: "dalec-test-package" + suffix, + Version: "0.0.1", + Revision: "1", + Description: "A basic package for various testing uses", + License: "MIT", + Sources: map[string]dalec.Source{ + "version.txt": { + Inline: &dalec.SourceInline{ + File: &dalec.SourceInlineFile{ + Contents: "version: " + "0.0.1", + }, }, }, }, - }, - Artifacts: dalec.Artifacts{ - Docs: map[string]dalec.ArtifactConfig{ - "version.txt": {}, + Artifacts: dalec.Artifacts{ + Docs: map[string]dalec.ArtifactConfig{ + "version.txt": {}, + }, }, - }, + } } - getSpec := func(keyConfig map[string]dalec.Source) *dalec.Spec { + getSpec := func(dep *dalec.Spec, keyConfig map[string]dalec.Source) *dalec.Spec { return &dalec.Spec{ Name: "dalec-test-custom-repo", Version: "0.0.1", @@ -1575,14 +1582,14 @@ func testCustomRepo(ctx context.Context, t *testing.T, cfg testLinuxConfig) { License: "MIT", Dependencies: &dalec.PackageDependencies{ Build: map[string]dalec.PackageConstraints{ - "dalec-test-package": {}, + dep.Name: {}, }, Runtime: map[string]dalec.PackageConstraints{ - "dalec-test-package": {}, + dep.Name: {}, }, Test: []string{ - "dalec-test-package", + dep.Name, "bash", "coreutils", }, @@ -1608,7 +1615,7 @@ func testCustomRepo(ctx context.Context, t *testing.T, cfg testLinuxConfig) { Build: dalec.ArtifactBuild{ Steps: []dalec.BuildStep{ { - Command: `set -x; [ "$(cat /usr/share/doc/dalec-test-package/version.txt)" = "version: 0.0.1" ]`, + Command: `set -x; [ "$(cat /usr/share/doc/` + dep.Name + `/version.txt)" = "version: 0.0.1" ]`, }, }, }, @@ -1629,7 +1636,7 @@ func testCustomRepo(ctx context.Context, t *testing.T, cfg testLinuxConfig) { } - getRepoState := func(ctx context.Context, client gwclient.Client, w llb.State, key llb.State) llb.State { + getRepoState := func(ctx context.Context, t *testing.T, client gwclient.Client, w llb.State, key llb.State, depSpec *dalec.Spec) llb.State { sr := newSolveRequest(withSpec(ctx, t, depSpec), withBuildTarget(cfg.Target.Package)) pkg := reqToState(ctx, client, sr, t) @@ -1640,68 +1647,77 @@ func testCustomRepo(ctx context.Context, t *testing.T, cfg testLinuxConfig) { return llb.Scratch().File(llb.Copy(workerWithRepo, "/opt/repo", "/", &llb.CopyInfo{CopyDirContentsOnly: true})) } - testNoPublicKey := func(ctx context.Context, gwc gwclient.Client) { - sr := newSolveRequest(withBuildTarget(cfg.Target.Worker), withSpec(ctx, t, nil)) - w := reqToState(ctx, gwc, sr, t) + t.Run("no public key", func(t *testing.T) { + t.Parallel() - // generate a gpg public/private key pair - gpgKey := generateGPGKey(w) + testNoPublicKey := func(ctx context.Context, gwc gwclient.Client) { + sr := newSolveRequest(withBuildTarget(cfg.Target.Worker), withSpec(ctx, t, nil)) + w := reqToState(ctx, gwc, sr, t) - repoState := getRepoState(ctx, gwc, w, gpgKey) + // generate a gpg public/private key pair + gpgKey := generateGPGKey(w) - sr = newSolveRequest(withSpec(ctx, t, getSpec(nil)), withBuildContext(ctx, t, "test-repo", repoState), withBuildTarget(cfg.Target.Container)) - // don't error here, the logs are intended to be checked by - // RunTestExpecting + depSpec := getDepSpec("no-public-key") + repoState := getRepoState(ctx, t, gwc, w, gpgKey, depSpec) - _, err := gwc.Solve(ctx, sr) - if err == nil { - t.Fatal("expected solve to fail") + sr = newSolveRequest( + withSpec(ctx, t, getSpec(depSpec, nil)), + withBuildContext(ctx, t, "test-repo", repoState), + withBuildTarget(cfg.Target.Container), + ) + + _, err := gwc.Solve(ctx, sr) + if err == nil { + t.Fatal("expected solve to fail") + } } - } - testWithPublicKey := func(ctx context.Context, gwc gwclient.Client) { - sr := newSolveRequest(withBuildTarget(cfg.Target.Worker), withSpec(ctx, t, nil)) - w := reqToState(ctx, gwc, sr, t) + testEnv.RunTest(ctx, t, testNoPublicKey) + }) + + t.Run("with public key", func(t *testing.T) { + t.Parallel() - // generate a gpg key to sign the repo - // under /public.key - gpgKey := generateGPGKey(w) - repoState := getRepoState(ctx, gwc, w, gpgKey) + testWithPublicKey := func(ctx context.Context, gwc gwclient.Client) { + sr := newSolveRequest(withBuildTarget(cfg.Target.Worker), withSpec(ctx, t, nil)) + w := reqToState(ctx, gwc, sr, t) - spec := getSpec(map[string]dalec.Source{ - // in the dalec spec, the public key will be passed in via build context - "public.key": { - Context: &dalec.SourceContext{ - Name: "repo-public-key", + // generate a gpg key to sign the repo + // under /public.key + gpgKey := generateGPGKey(w) + depSpec := getDepSpec("with-public-key") + repoState := getRepoState(ctx, t, gwc, w, gpgKey, depSpec) + + spec := getSpec(depSpec, map[string]dalec.Source{ + // in the dalec spec, the public key will be passed in via build context + "public.key": { + Context: &dalec.SourceContext{ + Name: "repo-public-key", + }, + Path: "public.key", }, - Path: "public.key", - }, - }) + }) - sr = newSolveRequest(withSpec(ctx, t, spec), withBuildContext(ctx, t, "test-repo", repoState), - withBuildContext(ctx, t, "repo-public-key", gpgKey), - withBuildTarget(cfg.Target.Container)) + sr = newSolveRequest( + withSpec(ctx, t, spec), + withBuildContext(ctx, t, "test-repo", repoState), + withBuildContext(ctx, t, "repo-public-key", gpgKey), + withBuildTarget(cfg.Target.Container), + ) - res := solveT(ctx, t, gwc, sr) - _, err := res.SingleRef() - if err != nil { - t.Fatal(err) + res := solveT(ctx, t, gwc, sr) + _, err := res.SingleRef() + if err != nil { + t.Fatal(err) + } } - } - t.Run("no public key", func(t *testing.T) { - t.Parallel() - testEnv.RunTest(ctx, t, testNoPublicKey) - }) - - t.Run("with public key", func(t *testing.T) { - t.Parallel() testEnv.RunTest(ctx, t, testWithPublicKey) }) } func testPinnedBuildDeps(ctx context.Context, t *testing.T, cfg testLinuxConfig) { - pkgName := "dalec-test-package" + pkgName := "dalec-test-package-pinned" getTestPackageSpec := func(version string) *dalec.Spec { depSpec := &dalec.Spec{ @@ -1794,7 +1810,7 @@ func testPinnedBuildDeps(ctx context.Context, t *testing.T, cfg testLinuxConfig) }, } - getWorker := func(ctx context.Context, client gwclient.Client) llb.State { + getWorker := func(ctx context.Context, t *testing.T, client gwclient.Client) llb.State { // Build the worker target, this will give us the worker image as an output. // Note: Currently we need to provide a dalec spec just due to how the router is setup. // The spec can be nil, though, it just needs to be parsable by yaml unmarshaller. @@ -1817,7 +1833,7 @@ func testPinnedBuildDeps(ctx context.Context, t *testing.T, cfg testLinuxConfig) t.Parallel() testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) { - worker := getWorker(ctx, gwc) + worker := getWorker(ctx, t, gwc) sr := newSolveRequest(withSpec(ctx, t, spec), withBuildContext(ctx, t, cfg.Worker.ContextName, worker), withBuildTarget(cfg.Target.Container)) res := solveT(ctx, t, gwc, sr) diff --git a/test/windows_test.go b/test/windows_test.go index 54c1469cd..3c899a479 100644 --- a/test/windows_test.go +++ b/test/windows_test.go @@ -448,7 +448,7 @@ func testCustomWindowscrossWorker(ctx context.Context, t *testing.T, targetCfg t testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) { // base package that will be used as a build dependency of the main package. depSpec := &dalec.Spec{ - Name: "dalec-test-package", + Name: "dalec-test-package-windows-worker-dep", Version: "0.0.1", Revision: "1", Description: "A basic package for various testing uses", From e8c3269f210398ab1b20d382a91c20c08725af9c Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Fri, 25 Oct 2024 15:55:46 -0700 Subject: [PATCH 2/3] jammy: Remove cached data for local repos before install We don't really need to cache these since they are locally anyway. It can also cause inconsistencies between builds if the repo config is different, for instance during tests when swithcing between signed and unsigned repos. Signed-off-by: Brian Goff --- frontend/jammy/handle_container.go | 18 +++++++++- frontend/jammy/handle_deb.go | 55 ++++++++++++++++++++++++------ 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/frontend/jammy/handle_container.go b/frontend/jammy/handle_container.go index c957c4bfd..b422e3a06 100644 --- a/frontend/jammy/handle_container.go +++ b/frontend/jammy/handle_container.go @@ -104,8 +104,24 @@ func buildImageRootfs(worker llb.State, spec *dalec.Spec, sOpt dalec.SourceOpts, debug := llb.Scratch().File(llb.Mkfile("debug", 0o644, []byte(`debug=2`)), opts...) opts = append(opts, dalec.ProgressGroup("Install spec package")) + script := llb.Scratch().File( + llb.Mkfile("install.sh", 0o755, []byte(`#!/usr/bin/env sh +set -ex + +rm -f /var/lib/apt/lists/_* +apt autoclean -y + +apt update +apt install -y /tmp/pkg/*.deb + `)), + opts..., + ) + + p := "/tmp/dalec/internal/install/container.sh" + return baseImg.Run( - dalec.ShArgs("set -x; apt update && apt install -y /tmp/pkg/*.deb"), + dalec.ShArgs(p), + llb.AddMount(p, script, llb.SourcePath("install.sh")), customRepoOpts, llb.AddEnv("DEBIAN_FRONTEND", "noninteractive"), llb.AddMount("/tmp/pkg", deb, llb.Readonly), diff --git a/frontend/jammy/handle_deb.go b/frontend/jammy/handle_deb.go index c38c97ec8..76ec3119e 100644 --- a/frontend/jammy/handle_deb.go +++ b/frontend/jammy/handle_deb.go @@ -2,7 +2,6 @@ package jammy import ( "context" - "fmt" "io/fs" "strings" @@ -145,26 +144,59 @@ func customRepoMounts(worker llb.State, repos []dalec.PackageRepositoryConfig, s return dalec.WithRunOptions(withRepos, withData, keyMounts), nil } -func installPackages(ls []string, rOpts ...llb.RunOption) llb.RunOption { +func installPackages(ls []string, opts ...llb.ConstraintsOpt) llb.RunOption { + script := llb.Scratch().File( + llb.Mkfile("install.sh", 0o755, []byte(`#!/usr/bin/env sh +set -ex + +# Make sure any cached data from local repos is purged since this should not +# be shared between builds. +rm -f /var/lib/apt/lists/_* +apt autoclean -y + +apt update +apt install -y `+strings.Join(ls, " ")+` +`, + )), + opts...) + + p := "/tmp/dalec/internal/deb/install.sh" return dalec.RunOptFunc(func(ei *llb.ExecInfo) { - // This only runs apt-get update if the pkgcache is older than 10 minutes. - dalec.ShArgs(`set -ex; apt update; apt install -y ` + strings.Join(ls, " ")).SetRunOption(ei) + llb.AddMount(p, script, llb.SourcePath("install.sh")).SetRunOption(ei) + dalec.ShArgs(p).SetRunOption(ei) dalec.WithMountedAptCache(AptCachePrefix).SetRunOption(ei) - dalec.WithRunOptions(rOpts...).SetRunOption(ei) }) } -func installWithConstraints(pkgPath string, pkgName string, rOpts ...llb.RunOption) llb.RunOption { +func installWithConstraints(pkgPath string, pkgName string, opts ...llb.ConstraintsOpt) llb.RunOption { return dalec.RunOptFunc(func(ei *llb.ExecInfo) { + script := llb.Scratch().File( + llb.Mkfile("install.sh", 0o755, []byte(`#!/usr/bin/env sh +set -ex + +# Make sure any cached data from local repos is purged since this should not +# be shared between builds. +rm -f /var/lib/apt/lists/_* +apt autoclean -y + +dpkg -i --force-depends `+pkgPath+` + +apt update +aptitude install -y -f -o "Aptitude::ProblemResolver::Hints::=reject `+pkgName+` :UNINST" +`), + ), opts...) + // The apt solver always tries to select the latest package version even when constraints specify that an older version should be installed and that older version is available in a repo. // This leads the solver to simply refuse to install our target package if the latest version of ANY dependency package is incompatible with the constraints. // To work around this we first install the .deb for the package with dpkg, specifically ignoring any dependencies so that we can avoid the constraints issue. // We then use aptitude to fix the (possibly broken) install of the package, and we pass the aptitude solver a hint to REJECT any solution that involves uninstalling the package. // This forces aptitude to find a solution that will respect the constraints even if the solution involves pinning dependency packages to older versions. - dalec.ShArgs(`set -ex; dpkg -i --force-depends ` + pkgPath + - fmt.Sprintf(`; apt update; aptitude install -y -f -o "Aptitude::ProblemResolver::Hints::=reject %s :UNINST"`, pkgName)).SetRunOption(ei) dalec.WithMountedAptCache(AptCachePrefix).SetRunOption(ei) - dalec.WithRunOptions(rOpts...).SetRunOption(ei) + + p := "/tmp/dalec/internal/deb/install-with-constraints.sh" + llb.AddMount(p, script, llb.SourcePath("install.sh")).SetRunOption(ei) + dalec.ShArgs(p).SetRunOption(ei) + }) } @@ -231,7 +263,7 @@ func basePackages(opts ...llb.ConstraintsOpt) llb.StateOption { return func(in llb.State) llb.State { opts = append(opts, dalec.ProgressGroup("Install base packages")) return in.Run( - installPackages([]string{"aptitude", "dpkg-dev", "devscripts", "equivs", "fakeroot", "dh-make", "build-essential", "dh-apparmor", "dh-make", "dh-exec", "debhelper-compat=" + deb.DebHelperCompat}), + installPackages([]string{"aptitude", "dpkg-dev", "devscripts", "equivs", "fakeroot", "dh-make", "build-essential", "dh-apparmor", "dh-make", "dh-exec", "debhelper-compat=" + deb.DebHelperCompat}, opts...), dalec.WithConstraints(opts...), ).Root() } @@ -285,7 +317,8 @@ func buildDepends(worker llb.State, sOpt dalec.SourceOpts, spec *dalec.Spec, tar ) return in.Run( - installWithConstraints(debPath+"/*.deb", depsSpec.Name, customRepoOpts), + installWithConstraints(debPath+"/*.deb", depsSpec.Name, opts...), + customRepoOpts, llb.AddMount(debPath, pkg, llb.Readonly), dalec.WithConstraints(opts...), ).Root() From 31cee317fa7b0ab77f5d1d27768a0713f65e30e2 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Sun, 27 Oct 2024 10:26:01 -0700 Subject: [PATCH 3/3] jammy: Remove duplicate code to install package Signed-off-by: Brian Goff --- frontend/jammy/handle_container.go | 18 +----------------- frontend/jammy/handle_deb.go | 10 +++++----- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/frontend/jammy/handle_container.go b/frontend/jammy/handle_container.go index b422e3a06..82c85f66a 100644 --- a/frontend/jammy/handle_container.go +++ b/frontend/jammy/handle_container.go @@ -104,24 +104,8 @@ func buildImageRootfs(worker llb.State, spec *dalec.Spec, sOpt dalec.SourceOpts, debug := llb.Scratch().File(llb.Mkfile("debug", 0o644, []byte(`debug=2`)), opts...) opts = append(opts, dalec.ProgressGroup("Install spec package")) - script := llb.Scratch().File( - llb.Mkfile("install.sh", 0o755, []byte(`#!/usr/bin/env sh -set -ex - -rm -f /var/lib/apt/lists/_* -apt autoclean -y - -apt update -apt install -y /tmp/pkg/*.deb - `)), - opts..., - ) - - p := "/tmp/dalec/internal/install/container.sh" - return baseImg.Run( - dalec.ShArgs(p), - llb.AddMount(p, script, llb.SourcePath("install.sh")), + installPackages([]string{"/tmp/pkg/*.deb"}, opts...), customRepoOpts, llb.AddEnv("DEBIAN_FRONTEND", "noninteractive"), llb.AddMount("/tmp/pkg", deb, llb.Readonly), diff --git a/frontend/jammy/handle_deb.go b/frontend/jammy/handle_deb.go index 76ec3119e..6b109fdec 100644 --- a/frontend/jammy/handle_deb.go +++ b/frontend/jammy/handle_deb.go @@ -170,6 +170,11 @@ apt install -y `+strings.Join(ls, " ")+` func installWithConstraints(pkgPath string, pkgName string, opts ...llb.ConstraintsOpt) llb.RunOption { return dalec.RunOptFunc(func(ei *llb.ExecInfo) { + // The apt solver always tries to select the latest package version even when constraints specify that an older version should be installed and that older version is available in a repo. + // This leads the solver to simply refuse to install our target package if the latest version of ANY dependency package is incompatible with the constraints. + // To work around this we first install the .deb for the package with dpkg, specifically ignoring any dependencies so that we can avoid the constraints issue. + // We then use aptitude to fix the (possibly broken) install of the package, and we pass the aptitude solver a hint to REJECT any solution that involves uninstalling the package. + // This forces aptitude to find a solution that will respect the constraints even if the solution involves pinning dependency packages to older versions. script := llb.Scratch().File( llb.Mkfile("install.sh", 0o755, []byte(`#!/usr/bin/env sh set -ex @@ -186,11 +191,6 @@ aptitude install -y -f -o "Aptitude::ProblemResolver::Hints::=reject `+pkgName+` `), ), opts...) - // The apt solver always tries to select the latest package version even when constraints specify that an older version should be installed and that older version is available in a repo. - // This leads the solver to simply refuse to install our target package if the latest version of ANY dependency package is incompatible with the constraints. - // To work around this we first install the .deb for the package with dpkg, specifically ignoring any dependencies so that we can avoid the constraints issue. - // We then use aptitude to fix the (possibly broken) install of the package, and we pass the aptitude solver a hint to REJECT any solution that involves uninstalling the package. - // This forces aptitude to find a solution that will respect the constraints even if the solution involves pinning dependency packages to older versions. dalec.WithMountedAptCache(AptCachePrefix).SetRunOption(ei) p := "/tmp/dalec/internal/deb/install-with-constraints.sh"