Merge pull request #8360 from justinsb/check_package_hashes

Test package versions as well as hashes

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",

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")
 }

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",
 			},

nodeup/pkg/model/docker_test.go

@@ -17,12 +17,11 @@ limitations under the License.
 package model
 
 import (
-	"crypto/sha1"
+	"fmt"
-	"encoding/hex"
-	"io"
-	"net/http"
 	"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) {
+func verifyPackageHash(u string, hash string, expectedVersion string) error {
-	resp, err := http.Get(u)
+	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 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 _, err := fi.DownloadURL(u, p, expectedHash); err != nil {
-	if hash != actualHash {
+		return err
-		t.Errorf("%s: hash was %q", u, actualHash)
-		return
 	}
+
+	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) {

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)
 		}
 	}