From 1d58f16d2932f2ad4b60bdc6bb6fc41750af581a Mon Sep 17 00:00:00 2001 From: Justin SB Date: Fri, 17 Jan 2020 01:47:46 -0500 Subject: [PATCH] Fix & test docker package versions as well as hashes Extend the existing "unit" test to check package versions, because some of the docker packages now have a '5:' prefix. Also correct the package versions that didn't have the prefix. --- nodeup/pkg/model/BUILD.bazel | 1 + nodeup/pkg/model/containerd_test.go | 71 +++---------------- nodeup/pkg/model/docker.go | 36 +++++----- nodeup/pkg/model/docker_test.go | 90 +++++++++++++++++++------ upup/pkg/fi/nodeup/nodetasks/package.go | 2 +- 5 files changed, 97 insertions(+), 103 deletions(-) diff --git a/nodeup/pkg/model/BUILD.bazel b/nodeup/pkg/model/BUILD.bazel index 86ff79501e..acb16f8fdf 100644 --- a/nodeup/pkg/model/BUILD.bazel +++ b/nodeup/pkg/model/BUILD.bazel @@ -102,6 +102,7 @@ go_test( "//upup/pkg/fi:go_default_library", "//upup/pkg/fi/nodeup/nodetasks:go_default_library", "//util/pkg/exec:go_default_library", + "//util/pkg/hashing:go_default_library", "//vendor/github.com/blang/semver:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library", diff --git a/nodeup/pkg/model/containerd_test.go b/nodeup/pkg/model/containerd_test.go index 03f8ba3664..cdd52c78bd 100644 --- a/nodeup/pkg/model/containerd_test.go +++ b/nodeup/pkg/model/containerd_test.go @@ -17,13 +17,8 @@ limitations under the License. package model import ( - "crypto/sha1" - "encoding/hex" - "io" - "net/http" "os" "path" - "strings" "testing" "k8s.io/kops/pkg/apis/kops" @@ -38,47 +33,14 @@ func TestContainerdPackageNames(t *testing.T) { continue } - sanityCheckContainerdPackageName(t, containerdVersion.Source, containerdVersion.Version, containerdVersion.Name) + sanityCheckPackageName(t, containerdVersion.Source, containerdVersion.Version, containerdVersion.Name) for k, p := range containerdVersion.ExtraPackages { - sanityCheckContainerdPackageName(t, p.Source, p.Version, k) + sanityCheckPackageName(t, p.Source, p.Version, k) } } } -func sanityCheckContainerdPackageName(t *testing.T, u string, version string, name string) { - filename := u - lastSlash := strings.LastIndex(filename, "/") - if lastSlash != -1 { - filename = filename[lastSlash+1:] - } - - expectedNames := []string{} - // Match known RPM formats - for _, v := range []string{"-1.", "-2.", "-3.", "-3.2."} { - for _, d := range []string{"el7", "el7.centos", "el7_6"} { - for _, a := range []string{"noarch", "x86_64"} { - expectedNames = append(expectedNames, name+"-"+version+v+d+"."+a+".rpm") - } - } - } - - // Match known DEB formats - for _, a := range []string{"amd64", "armhf"} { - expectedNames = append(expectedNames, name+"_"+version+"_"+a+".deb") - } - - found := false - for _, s := range expectedNames { - if s == filename { - found = true - } - } - if !found { - t.Errorf("unexpected name=%q, version=%q for %s", name, version, u) - } -} - func TestContainerdPackageHashes(t *testing.T) { if os.Getenv("VERIFY_HASHES") == "" { t.Skip("VERIFY_HASHES not set, won't download & verify docker hashes") @@ -86,36 +48,19 @@ func TestContainerdPackageHashes(t *testing.T) { for _, containerdVersion := range containerdVersions { t.Run(containerdVersion.Source, func(t *testing.T) { - verifyContainerdPackageHash(t, containerdVersion.Source, containerdVersion.Hash) + if err := verifyPackageHash(containerdVersion.Source, containerdVersion.Hash, containerdVersion.Version); err != nil { + t.Errorf("error verifying package %q: %v", containerdVersion.Source, err) + } for _, p := range containerdVersion.ExtraPackages { - verifyContainerdPackageHash(t, p.Source, p.Hash) + if err := verifyPackageHash(p.Source, p.Hash, p.Version); err != nil { + t.Errorf("error verifying package %q: %v", p.Source, err) + } } }) } } -func verifyContainerdPackageHash(t *testing.T, u string, hash string) { - resp, err := http.Get(u) - if err != nil { - t.Errorf("%s: error fetching: %v", u, err) - return - } - defer resp.Body.Close() - - hasher := sha1.New() - if _, err := io.Copy(hasher, resp.Body); err != nil { - t.Errorf("%s: error reading: %v", u, err) - return - } - - actualHash := hex.EncodeToString(hasher.Sum(nil)) - if hash != actualHash { - t.Errorf("%s: hash was %q", u, actualHash) - return - } -} - func TestContainerdBuilder_Simple(t *testing.T) { runContainerdBuilderTest(t, "simple") } diff --git a/nodeup/pkg/model/docker.go b/nodeup/pkg/model/docker.go index 3395fc8892..42212c9f46 100644 --- a/nodeup/pkg/model/docker.go +++ b/nodeup/pkg/model/docker.go @@ -626,12 +626,12 @@ var dockerVersions = []packageVersion{ Name: "docker-ce", Distros: []distros.Distribution{distros.DistributionDebian9}, Architectures: []Architecture{ArchitectureAmd64}, - Version: "18.09.3~3-0~debian-stretch", + Version: "5:18.09.3~3-0~debian-stretch", Source: "https://download.docker.com/linux/debian/dists/stretch/pool/stable/amd64/docker-ce_18.09.3~3-0~debian-stretch_amd64.deb", Hash: "009b9a2d8bfaa97c74773fe4ec25b6bb396b10d0", ExtraPackages: map[string]packageInfo{ "docker-ce-cli": { - Version: "18.09.3~3-0~debian-stretch", + Version: "5:18.09.3~3-0~debian-stretch", Source: "https://download.docker.com/linux/debian/dists/stretch/pool/stable/amd64/docker-ce-cli_18.09.3~3-0~debian-stretch_amd64.deb", Hash: "557f868ec63e5251639ebd1d8669eb0c61dd555c", }, @@ -739,12 +739,12 @@ var dockerVersions = []packageVersion{ Name: "docker-ce", Distros: []distros.Distribution{distros.DistributionDebian9}, Architectures: []Architecture{ArchitectureAmd64}, - Version: "18.09.9~3-0~debian-stretch", + Version: "5:18.09.9~3-0~debian-stretch", Source: "https://download.docker.com/linux/debian/dists/stretch/pool/stable/amd64/docker-ce_18.09.9~3-0~debian-stretch_amd64.deb", Hash: "9d564b56f5531a08e24c8c7724445d128742572e", ExtraPackages: map[string]packageInfo{ "docker-ce-cli": { - Version: "18.09.9~3-0~debian-stretch", + Version: "5:18.09.9~3-0~debian-stretch", Source: "https://download.docker.com/linux/debian/dists/stretch/pool/stable/amd64/docker-ce-cli_18.09.9~3-0~debian-stretch_amd64.deb", Hash: "88f8f3103d2e5011e2f1a73b9e6dbf03d6e6698a", }, @@ -758,12 +758,12 @@ var dockerVersions = []packageVersion{ Name: "docker-ce", Distros: []distros.Distribution{distros.DistributionDebian10}, Architectures: []Architecture{ArchitectureAmd64}, - Version: "18.09.9~3-0~debian-buster", + Version: "5:18.09.9~3-0~debian-buster", Source: "https://download.docker.com/linux/debian/dists/buster/pool/stable/amd64/docker-ce_18.09.9~3-0~debian-buster_amd64.deb", Hash: "97620eede9ca9fd379eef41b9d14347fe1d82ded", ExtraPackages: map[string]packageInfo{ "docker-ce-cli": { - Version: "18.09.9~3-0~debian-buster", + Version: "5:18.09.9~3-0~debian-buster", Source: "https://download.docker.com/linux/debian/dists/buster/pool/stable/amd64/docker-ce-cli_18.09.9~3-0~debian-buster_amd64.deb", Hash: "510eee5b6884867be0d2b360f8ff8cf7f0c0d11a", }, @@ -777,12 +777,12 @@ var dockerVersions = []packageVersion{ Name: "docker-ce", Distros: []distros.Distribution{distros.DistributionXenial}, Architectures: []Architecture{ArchitectureAmd64}, - Version: "18.09.9~3-0~ubuntu-xenial", + Version: "5:18.09.9~3-0~ubuntu-xenial", Source: "https://download.docker.com/linux/ubuntu/dists/xenial/pool/stable/amd64/docker-ce_18.09.9~3-0~ubuntu-xenial_amd64.deb", Hash: "959a1193ff148cbf98c357e096dafca44f497520", ExtraPackages: map[string]packageInfo{ "docker-ce-cli": { - Version: "18.09.9~3-0~ubuntu-xenial", + Version: "5:18.09.9~3-0~ubuntu-xenial", Source: "https://download.docker.com/linux/ubuntu/dists/xenial/pool/stable/amd64/docker-ce-cli_18.09.9~3-0~ubuntu-xenial_amd64.deb", Hash: "b79b8958f041249bbff0afbfeded794a9e42463f", }, @@ -796,12 +796,12 @@ var dockerVersions = []packageVersion{ Name: "docker-ce", Distros: []distros.Distribution{distros.DistributionBionic}, Architectures: []Architecture{ArchitectureAmd64}, - Version: "18.09.9~3-0~ubuntu-bionic", + Version: "5:18.09.9~3-0~ubuntu-bionic", Source: "https://download.docker.com/linux/ubuntu/dists/bionic/pool/stable/amd64/docker-ce_18.09.9~3-0~ubuntu-bionic_amd64.deb", Hash: "edabe6602521927b6e9ad70fc7650329333b51a3", ExtraPackages: map[string]packageInfo{ "docker-ce-cli": { - Version: "18.09.9~3-0~ubuntu-bionic", + Version: "5:18.09.9~3-0~ubuntu-bionic", Source: "https://download.docker.com/linux/ubuntu/dists/bionic/pool/stable/amd64/docker-ce-cli_18.09.9~3-0~ubuntu-bionic_amd64.deb", Hash: "bca089a50ea22f02abe88f68d7ca35c26be9967b", }, @@ -855,12 +855,12 @@ var dockerVersions = []packageVersion{ Name: "docker-ce", Distros: []distros.Distribution{distros.DistributionDebian9}, Architectures: []Architecture{ArchitectureAmd64}, - Version: "19.03.4~3-0~debian-stretch", + Version: "5:19.03.4~3-0~debian-stretch", Source: "https://download.docker.com/linux/debian/dists/stretch/pool/stable/amd64/docker-ce_19.03.4~3-0~debian-stretch_amd64.deb", Hash: "2b8dcb2d75334fab29242ac069d1fbcfb65e88e3", ExtraPackages: map[string]packageInfo{ "docker-ce-cli": { - Version: "19.03.4~3-0~debian-stretch", + Version: "5:19.03.4~3-0~debian-stretch", Source: "https://download.docker.com/linux/debian/dists/stretch/pool/stable/amd64/docker-ce-cli_19.03.4~3-0~debian-stretch_amd64.deb", Hash: "57f71ee764abb19a0b4c580ff14b1eb3de3a9e08", }, @@ -874,12 +874,12 @@ var dockerVersions = []packageVersion{ Name: "docker-ce", Distros: []distros.Distribution{distros.DistributionDebian10}, Architectures: []Architecture{ArchitectureAmd64}, - Version: "19.03.4~3-0~debian-buster", + Version: "5:19.03.4~3-0~debian-buster", Source: "https://download.docker.com/linux/debian/dists/buster/pool/stable/amd64/docker-ce_19.03.4~3-0~debian-buster_amd64.deb", Hash: "492a70f29ceffd315ee9712b33004491c6f59e49", ExtraPackages: map[string]packageInfo{ "docker-ce-cli": { - Version: "19.03.4~3-0~debian-buster", + Version: "5:19.03.4~3-0~debian-buster", Source: "https://download.docker.com/linux/debian/dists/buster/pool/stable/amd64/docker-ce-cli_19.03.4~3-0~debian-buster_amd64.deb", Hash: "2549a364f0e5ce489c79b292b78e349751385dd5", }, @@ -893,12 +893,12 @@ var dockerVersions = []packageVersion{ Name: "docker-ce", Distros: []distros.Distribution{distros.DistributionXenial}, Architectures: []Architecture{ArchitectureAmd64}, - Version: "19.03.4~3-0~ubuntu-xenial", + Version: "5:19.03.4~3-0~ubuntu-xenial", Source: "https://download.docker.com/linux/ubuntu/dists/xenial/pool/stable/amd64/docker-ce_19.03.4~3-0~ubuntu-xenial_amd64.deb", Hash: "d9f5855413a5efcca4e756613dafb744b6cae8d2", ExtraPackages: map[string]packageInfo{ "docker-ce-cli": { - Version: "19.03.4~3-0~ubuntu-xenial", + Version: "5:19.03.4~3-0~ubuntu-xenial", Source: "https://download.docker.com/linux/ubuntu/dists/xenial/pool/stable/amd64/docker-ce-cli_19.03.4~3-0~ubuntu-xenial_amd64.deb", Hash: "3e0164dfef612b533c12dec6cd39da93bedd7e8c", }, @@ -912,12 +912,12 @@ var dockerVersions = []packageVersion{ Name: "docker-ce", Distros: []distros.Distribution{distros.DistributionBionic}, Architectures: []Architecture{ArchitectureAmd64}, - Version: "19.03.4~3-0~ubuntu-bionic", + Version: "5:19.03.4~3-0~ubuntu-bionic", Source: "https://download.docker.com/linux/ubuntu/dists/bionic/pool/stable/amd64/docker-ce_19.03.4~3-0~ubuntu-bionic_amd64.deb", Hash: "ee640d9258fd4d3f4c7017ab2a71da63cbbead55", ExtraPackages: map[string]packageInfo{ "docker-ce-cli": { - Version: "19.03.4~3-0~ubuntu-bionic", + Version: "5:19.03.4~3-0~ubuntu-bionic", Source: "https://download.docker.com/linux/ubuntu/dists/bionic/pool/stable/amd64/docker-ce-cli_19.03.4~3-0~ubuntu-bionic_amd64.deb", Hash: "09402bf5dac40f0c50f1071b17f38f6584a42ad1", }, diff --git a/nodeup/pkg/model/docker_test.go b/nodeup/pkg/model/docker_test.go index 3597477658..82f37fdd59 100644 --- a/nodeup/pkg/model/docker_test.go +++ b/nodeup/pkg/model/docker_test.go @@ -17,12 +17,11 @@ limitations under the License. package model import ( - "crypto/sha1" - "encoding/hex" - "io" - "net/http" + "fmt" "os" + "os/exec" "path" + "path/filepath" "strings" "testing" @@ -30,6 +29,7 @@ import ( "k8s.io/kops/pkg/flagbuilder" "k8s.io/kops/pkg/testutils" "k8s.io/kops/upup/pkg/fi" + "k8s.io/kops/util/pkg/hashing" ) func TestDockerPackageNames(t *testing.T) { @@ -66,6 +66,8 @@ func sanityCheckPackageName(t *testing.T, u string, version string, name string) // Match known DEB formats for _, a := range []string{"amd64", "armhf"} { expectedNames = append(expectedNames, name+"_"+version+"_"+a+".deb") + // Not entirely clear why some (docker) debian packages have this '5:' prefix + expectedNames = append(expectedNames, name+"_"+strings.TrimPrefix(version, "5:")+"_"+a+".deb") } found := false @@ -86,34 +88,80 @@ func TestDockerPackageHashes(t *testing.T) { for _, dockerVersion := range dockerVersions { t.Run(dockerVersion.Source, func(t *testing.T) { - verifyPackageHash(t, dockerVersion.Source, dockerVersion.Hash) + if err := verifyPackageHash(dockerVersion.Source, dockerVersion.Hash, dockerVersion.Version); err != nil { + t.Errorf("error verifying package %q: %v", dockerVersion.Source, err) + } for _, p := range dockerVersion.ExtraPackages { - verifyPackageHash(t, p.Source, p.Hash) + if err := verifyPackageHash(p.Source, p.Hash, p.Version); err != nil { + t.Errorf("error verifying package %q: %v", p.Source, err) + } } }) } } -func verifyPackageHash(t *testing.T, u string, hash string) { - resp, err := http.Get(u) +func verifyPackageHash(u string, hash string, expectedVersion string) error { + name := path.Base(u) + p := filepath.Join("/tmp", name) + + expectedHash, err := hashing.FromString(hash) if err != nil { - t.Errorf("%s: error fetching: %v", u, err) - return - } - defer resp.Body.Close() - - hasher := sha1.New() - if _, err := io.Copy(hasher, resp.Body); err != nil { - t.Errorf("%s: error reading: %v", u, err) - return + return err } - actualHash := hex.EncodeToString(hasher.Sum(nil)) - if hash != actualHash { - t.Errorf("%s: hash was %q", u, actualHash) - return + if _, err := fi.DownloadURL(u, p, expectedHash); err != nil { + return err } + + actualHash, err := hashing.HashAlgorithmSHA1.HashFile(p) + if err != nil { + return fmt.Errorf("error hashing file: %v", err) + } + + if hash != actualHash.Hex() { + return fmt.Errorf("hash was %q, expected %q", actualHash.Hex(), hash) + } + + if strings.HasSuffix(u, ".deb") { + cmd := exec.Command("dpkg-deb", "-I", p) + out, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("error running 'dpkg-deb -I %s': %v", p, err) + } + + version := "" + for _, line := range strings.Split(string(out), "\n") { + line = strings.TrimSpace(line) + if !strings.HasPrefix(line, "Version: ") { + continue + } + version += strings.TrimPrefix(line, "Version: ") + } + if expectedVersion != version { + return fmt.Errorf("unexpected version, actual=%q, expected=%q", version, expectedVersion) + } + + } else if strings.HasSuffix(u, ".rpm") { + cmd := exec.Command("rpm", "-qp", "--queryformat", "%{VERSION}", "--nosignature", p) + out, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("error running rpm %s: %v", strings.Join(cmd.Args, " "), err) + } + + version := strings.TrimSpace(string(out)) + if expectedVersion != version { + return fmt.Errorf("unexpected version, actual=%q, expected=%q", version, expectedVersion) + } + } else if strings.HasSuffix(u, ".tgz") || strings.HasSuffix(u, ".tar.gz") { + if expectedVersion != "" { + return fmt.Errorf("did not expect version for tgz / tar.gz package") + } + } else { + return fmt.Errorf("unexpected suffix for file (known: .rpm .deb .tar.gz .tgz)") + } + + return nil } func TestDockerBuilder_Simple(t *testing.T) { diff --git a/upup/pkg/fi/nodeup/nodetasks/package.go b/upup/pkg/fi/nodeup/nodetasks/package.go index 5d78805bec..aa31df832f 100644 --- a/upup/pkg/fi/nodeup/nodetasks/package.go +++ b/upup/pkg/fi/nodeup/nodetasks/package.go @@ -389,7 +389,7 @@ func (_ *Package) RenderLocal(t *local.LocalTarget, a, e, changes *Package) erro } if !reflect.DeepEqual(changes, &Package{}) { - klog.Warningf("cannot apply package changes for %q: %v", e.Name, changes) + klog.Warningf("cannot apply package changes for %q: %+v", e.Name, changes) } }