The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/distrobuilder/pull/22
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === Resolves #15
From 6c4c7ca7c64be3761ecdf0f7938664c5b6b2f467 Mon Sep 17 00:00:00 2001 From: Thomas Hipp <[email protected]> Date: Wed, 28 Feb 2018 14:51:51 +0100 Subject: [PATCH 1/7] shared: Allow omission of GPG keys Signed-off-by: Thomas Hipp <[email protected]> --- shared/definition.go | 6 +----- shared/definition_test.go | 36 +++++++++++++++++++++--------------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/shared/definition.go b/shared/definition.go index 619304f..f4d1df9 100644 --- a/shared/definition.go +++ b/shared/definition.go @@ -33,7 +33,7 @@ type DefinitionImage struct { type DefinitionSource struct { Downloader string `yaml:"downloader"` URL string `yaml:"url"` - Keys []string `yaml:"keys"` + Keys []string `yaml:"keys,omitempty"` Keyserver string `yaml:"keyserver,omitempty"` } @@ -120,10 +120,6 @@ func ValidateDefinition(def Definition) error { return errors.New("source.url may not be empty") } - if len(def.Source.Keys) == 0 { - return errors.New("source.keys may not be empty") - } - validManagers := []string{ "apk", "apt", diff --git a/shared/definition_test.go b/shared/definition_test.go index 2feb677..32b1886 100644 --- a/shared/definition_test.go +++ b/shared/definition_test.go @@ -48,41 +48,45 @@ func TestValidateDefinition(t *testing.T) { false, }, { - "empty image.distribution", - Definition{}, - "image.distribution may not be empty", - true, - }, - { - "invalid source.downloader", + "valid Definition without source.keys", Definition{ Image: DefinitionImage{ Distribution: "ubuntu", Release: "artful", }, Source: DefinitionSource{ - Downloader: "foo", + Downloader: "debootstrap", + URL: "https://ubuntu.com";, + }, + Packages: DefinitionPackages{ + Manager: "apt", }, }, - "source.downloader must be one of .+", + "", + false, + }, + { + "empty image.distribution", + Definition{}, + "image.distribution may not be empty", true, }, { - "empty source.url", + "invalid source.downloader", Definition{ Image: DefinitionImage{ Distribution: "ubuntu", Release: "artful", }, Source: DefinitionSource{ - Downloader: "debootstrap", + Downloader: "foo", }, }, - "source.url may not be empty", + "source.downloader must be one of .+", true, }, { - "empty source.keys", + "empty source.url", Definition{ Image: DefinitionImage{ Distribution: "ubuntu", @@ -90,10 +94,9 @@ func TestValidateDefinition(t *testing.T) { }, Source: DefinitionSource{ Downloader: "debootstrap", - URL: "https://ubuntu.com";, }, }, - "source.keys may not be empty", + "source.url may not be empty", true, }, { @@ -123,6 +126,9 @@ func TestValidateDefinition(t *testing.T) { if !tt.shouldFail && err != nil { t.Fatalf("Validation failed: %s", err) } else if tt.shouldFail { + if err == nil { + t.Fatal("Expected failure") + } match, _ := regexp.MatchString(tt.expected, err.Error()) if !match { t.Fatalf("Validation failed: Expected '%s', got '%s'", tt.expected, err.Error()) From 19f788673f72433049d8cb90bf22a58604ce3e15 Mon Sep 17 00:00:00 2001 From: Thomas Hipp <[email protected]> Date: Wed, 28 Feb 2018 14:57:28 +0100 Subject: [PATCH 2/7] shared: Create separate function for keyring creation Signed-off-by: Thomas Hipp <[email protected]> --- shared/util.go | 35 ++++++++++++++++++++++------------- shared/util_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/shared/util.go b/shared/util.go index 1c65910..49c53db 100644 --- a/shared/util.go +++ b/shared/util.go @@ -57,29 +57,19 @@ func RunCommand(name string, arg ...string) error { // VerifyFile verifies a file using gpg. func VerifyFile(signedFile, signatureFile string, keys []string, keyserver string) (bool, error) { - var out string - - gpgDir := filepath.Join(os.TempDir(), "distrobuilder.gpg") - - err := os.MkdirAll(gpgDir, 0700) + gpgDir, err := CreateGPGKeyring(keyserver, keys) if err != nil { return false, err } defer os.RemoveAll(gpgDir) - out, err = lxd.RunCommand("gpg", append([]string{ - "--homedir", gpgDir, "--keyserver", keyserver, "--recv-keys"}, keys...)...) - if err != nil { - return false, fmt.Errorf("Failed to receive keys: %s", out) - } - if signatureFile != "" { - out, err = lxd.RunCommand("gpg", "--homedir", gpgDir, "--verify", signatureFile, signedFile) + out, err := lxd.RunCommand("gpg", "--homedir", gpgDir, "--verify", signatureFile, signedFile) if err != nil { return false, fmt.Errorf("Failed to verify: %s", out) } } else { - out, err = lxd.RunCommand("gpg", "--homedir", gpgDir, "--verify", signedFile) + out, err := lxd.RunCommand("gpg", "--homedir", gpgDir, "--verify", signedFile) if err != nil { return false, fmt.Errorf("Failed to verify: %s", out) } @@ -88,6 +78,25 @@ func VerifyFile(signedFile, signatureFile string, keys []string, keyserver strin return true, nil } +// CreateGPGKeyring creates a new GPG keyring. +func CreateGPGKeyring(keyserver string, keys []string) (string, error) { + gpgDir := filepath.Join(os.TempDir(), "distrobuilder.gpg") + + err := os.MkdirAll(gpgDir, 0700) + if err != nil { + return "", err + } + + out, err := lxd.RunCommand("gpg", append([]string{ + "--homedir", gpgDir, "--keyserver", keyserver, "--recv-keys"}, keys...)...) + if err != nil { + os.RemoveAll(gpgDir) + return "", fmt.Errorf("Failed to create keyring: %s", out) + } + + return gpgDir, nil +} + // Pack creates an xz-compressed tarball. func Pack(filename, path string, args ...string) error { return RunCommand("tar", append([]string{"-cJf", filename, "-C", path}, args...)...) diff --git a/shared/util_test.go b/shared/util_test.go index 9ec6ef5..993f3e6 100644 --- a/shared/util_test.go +++ b/shared/util_test.go @@ -5,6 +5,8 @@ import ( "os" "path/filepath" "testing" + + lxd "github.com/lxc/lxd/shared" ) func TestVerifyFile(t *testing.T) { @@ -73,6 +75,14 @@ func TestVerifyFile(t *testing.T) { keyserver, true, }, + { + "missing keyserver", + filepath.Join(testdataDir, "testfile.asc"), + "", + keys, + "", + true, + }, } for i, tt := range tests { @@ -87,3 +97,27 @@ func TestVerifyFile(t *testing.T) { } } } + +func TestCreateGPGKeyring(t *testing.T) { + gpgDir, err := CreateGPGKeyring("pgp.mit.edu", []string{"0x5DE8949A899C8D99"}) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + + if !lxd.PathExists(gpgDir) { + t.Fatalf("Failed to create gpg directory: %s", gpgDir) + } + os.RemoveAll(gpgDir) + + // This should fail running the gpg command. + gpgDir, err = CreateGPGKeyring("", []string{}) + if err == nil { + t.Fatal("Expected to fail") + } + + // The gpgDir directory should've have been cleaned up. Check this. + if lxd.PathExists(gpgDir) { + os.RemoveAll(gpgDir) + t.Fatal("Failed to clean up gpg directory") + } +} From 1ffed3de3eba81b032d708c4600422add62b54c6 Mon Sep 17 00:00:00 2001 From: Thomas Hipp <[email protected]> Date: Wed, 28 Feb 2018 15:07:49 +0100 Subject: [PATCH 3/7] sources: Support GPG keys in debootstrap Signed-off-by: Thomas Hipp <[email protected]> --- sources/debootstrap.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/sources/debootstrap.go b/sources/debootstrap.go index 156eda1..dce6652 100644 --- a/sources/debootstrap.go +++ b/sources/debootstrap.go @@ -29,6 +29,16 @@ func (s *Debootstrap) Run(source shared.DefinitionSource, release, variant, arch args = append(args, "--arch", arch) } + if len(source.Keys) > 0 { + gpgDir, err := shared.CreateGPGKeyring(source.Keyserver, source.Keys) + if err != nil { + return err + } + defer os.RemoveAll(gpgDir) + + args = append(args, "--keyring", filepath.Join(gpgDir, "pubring.kbx")) + } + args = append(args, release, filepath.Join(cacheDir, "rootfs")) if source.URL != "" { From 17bce64b03dd6d10fa2470242c0d2a7a8d6a39f3 Mon Sep 17 00:00:00 2001 From: Thomas Hipp <[email protected]> Date: Wed, 28 Feb 2018 15:58:13 +0100 Subject: [PATCH 4/7] util: Allow empty keyserver Signed-off-by: Thomas Hipp <[email protected]> --- shared/util.go | 11 +++++++++-- shared/util_test.go | 15 +++++++-------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/shared/util.go b/shared/util.go index 49c53db..d0489fe 100644 --- a/shared/util.go +++ b/shared/util.go @@ -87,8 +87,15 @@ func CreateGPGKeyring(keyserver string, keys []string) (string, error) { return "", err } - out, err := lxd.RunCommand("gpg", append([]string{ - "--homedir", gpgDir, "--keyserver", keyserver, "--recv-keys"}, keys...)...) + args := []string{"--homedir", gpgDir} + + if keyserver != "" { + args = append(args, "--keyserver", keyserver) + } + + args = append(args, append([]string{"--recv-keys"}, keys...)...) + + out, err := lxd.RunCommand("gpg", args...) if err != nil { os.RemoveAll(gpgDir) return "", fmt.Errorf("Failed to create keyring: %s", out) diff --git a/shared/util_test.go b/shared/util_test.go index 993f3e6..ab2da3d 100644 --- a/shared/util_test.go +++ b/shared/util_test.go @@ -81,7 +81,7 @@ func TestVerifyFile(t *testing.T) { "", keys, "", - true, + false, }, } @@ -109,15 +109,14 @@ func TestCreateGPGKeyring(t *testing.T) { } os.RemoveAll(gpgDir) - // This should fail running the gpg command. + // This shouldn't fail either. gpgDir, err = CreateGPGKeyring("", []string{}) - if err == nil { - t.Fatal("Expected to fail") + if err != nil { + t.Fatalf("Unexpected error: %s", err) } - // The gpgDir directory should've have been cleaned up. Check this. - if lxd.PathExists(gpgDir) { - os.RemoveAll(gpgDir) - t.Fatal("Failed to clean up gpg directory") + if !lxd.PathExists(gpgDir) { + t.Fatalf("Failed to create gpg directory: %s", gpgDir) } + os.RemoveAll(gpgDir) } From b6c153835660fa857e96944220aedc876c16fdf4 Mon Sep 17 00:00:00 2001 From: Thomas Hipp <[email protected]> Date: Wed, 28 Feb 2018 18:21:05 +0100 Subject: [PATCH 5/7] sources: Add separate variant for downloaders Signed-off-by: Thomas Hipp <[email protected]> --- distrobuilder/main.go | 4 ++-- shared/definition.go | 6 ++++++ sources/alpine-http.go | 2 +- sources/archlinux-http.go | 2 +- sources/centos-http.go | 4 ++-- sources/debootstrap.go | 6 +++--- sources/source.go | 2 +- sources/ubuntu-http.go | 2 +- 8 files changed, 17 insertions(+), 11 deletions(-) diff --git a/distrobuilder/main.go b/distrobuilder/main.go index e5baf11..dfb9e92 100644 --- a/distrobuilder/main.go +++ b/distrobuilder/main.go @@ -161,8 +161,8 @@ func run(c *cli.Context) error { } // Download the root filesystem - err = downloader.Run(def.Source, def.Image.Release, def.Image.Variant, - arch, c.GlobalString("cache-dir")) + err = downloader.Run(def.Source, def.Image.Release, arch, + c.GlobalString("cache-dir")) if err != nil { return fmt.Errorf("Error while downloading source: %s", err) } diff --git a/shared/definition.go b/shared/definition.go index f4d1df9..5d70918 100644 --- a/shared/definition.go +++ b/shared/definition.go @@ -35,6 +35,7 @@ type DefinitionSource struct { URL string `yaml:"url"` Keys []string `yaml:"keys,omitempty"` Keyserver string `yaml:"keyserver,omitempty"` + Variant string `yaml:"variant,omitempty"` } // A DefinitionTargetLXC represents LXC specific files as part of the metadata. @@ -97,6 +98,11 @@ func SetDefinitionDefaults(def *Definition) { if def.Source.Keyserver == "" { def.Source.Keyserver = "hkps.pool.sks-keyservers.net" } + + // If no Source.Variant is specified, use the one in Image.Variant. + if def.Source.Variant == "" { + def.Source.Variant = def.Image.Variant + } } // ValidateDefinition validates the given Definition. diff --git a/sources/alpine-http.go b/sources/alpine-http.go index e3fbf25..19f1155 100644 --- a/sources/alpine-http.go +++ b/sources/alpine-http.go @@ -20,7 +20,7 @@ func NewAlpineLinuxHTTP() *AlpineLinuxHTTP { } // Run downloads an Alpine Linux mini root filesystem. -func (s *AlpineLinuxHTTP) Run(source shared.DefinitionSource, release, variant, arch, cacheDir string) error { +func (s *AlpineLinuxHTTP) Run(source shared.DefinitionSource, release, arch, cacheDir string) error { fname := fmt.Sprintf("alpine-minirootfs-%s-%s.tar.gz", release, arch) tarball := fmt.Sprintf("%s/v%s/releases/%s/%s", source.URL, strings.Join(strings.Split(release, ".")[0:2], "."), arch, fname) diff --git a/sources/archlinux-http.go b/sources/archlinux-http.go index fed12a7..ef22d3d 100644 --- a/sources/archlinux-http.go +++ b/sources/archlinux-http.go @@ -19,7 +19,7 @@ func NewArchLinuxHTTP() *ArchLinuxHTTP { } // Run downloads an Arch Linux tarball. -func (s *ArchLinuxHTTP) Run(source shared.DefinitionSource, release, variant, arch, cacheDir string) error { +func (s *ArchLinuxHTTP) Run(source shared.DefinitionSource, release, arch, cacheDir string) error { fname := fmt.Sprintf("archlinux-bootstrap-%s-x86_64.tar.gz", release) tarball := fmt.Sprintf("%s/%s/%s", source.URL, release, fname) diff --git a/sources/centos-http.go b/sources/centos-http.go index e2cdc8b..e65c008 100644 --- a/sources/centos-http.go +++ b/sources/centos-http.go @@ -27,11 +27,11 @@ func NewCentOSHTTP() *CentOSHTTP { } // Run downloads the tarball and unpacks it. -func (s *CentOSHTTP) Run(source shared.DefinitionSource, release, variant, arch, cacheDir string) error { +func (s *CentOSHTTP) Run(source shared.DefinitionSource, release, arch, cacheDir string) error { s.cacheDir = cacheDir baseURL := fmt.Sprintf("%s/%s/isos/%s/", source.URL, strings.Split(release, ".")[0], arch) - s.fname = getRelease(source.URL, release, variant, arch) + s.fname = getRelease(source.URL, release, source.Variant, arch) if s.fname == "" { return fmt.Errorf("Couldn't get name of iso") } diff --git a/sources/debootstrap.go b/sources/debootstrap.go index dce6652..11b9a19 100644 --- a/sources/debootstrap.go +++ b/sources/debootstrap.go @@ -16,13 +16,13 @@ func NewDebootstrap() *Debootstrap { } // Run runs debootstrap. -func (s *Debootstrap) Run(source shared.DefinitionSource, release, variant, arch, cacheDir string) error { +func (s *Debootstrap) Run(source shared.DefinitionSource, release, arch, cacheDir string) error { var args []string os.RemoveAll(filepath.Join(cacheDir, "rootfs")) - if variant != "" { - args = append(args, "--variant", variant) + if source.Variant != "" { + args = append(args, "--variant", source.Variant) } if arch != "" { diff --git a/sources/source.go b/sources/source.go index ef44fd7..ef32d05 100644 --- a/sources/source.go +++ b/sources/source.go @@ -4,7 +4,7 @@ import "github.com/lxc/distrobuilder/shared" // A Downloader represents a source downloader. type Downloader interface { - Run(shared.DefinitionSource, string, string, string, string) error + Run(shared.DefinitionSource, string, string, string) error } // Get returns a Downloader. diff --git a/sources/ubuntu-http.go b/sources/ubuntu-http.go index 455f624..7554912 100644 --- a/sources/ubuntu-http.go +++ b/sources/ubuntu-http.go @@ -25,7 +25,7 @@ func NewUbuntuHTTP() *UbuntuHTTP { } // Run downloads the tarball and unpacks it. -func (s *UbuntuHTTP) Run(source shared.DefinitionSource, release, variant, arch, cacheDir string) error { +func (s *UbuntuHTTP) Run(source shared.DefinitionSource, release, arch, cacheDir string) error { baseURL := fmt.Sprintf("%s/releases/%s/release/", source.URL, release) if strings.ContainsAny(release, "0123456789") { From c9b245a820fd8f6fad3eec0740f38e60d8b8b060 Mon Sep 17 00:00:00 2001 From: Thomas Hipp <[email protected]> Date: Thu, 1 Mar 2018 09:03:03 +0100 Subject: [PATCH 6/7] shared: Allow empty URL Signed-off-by: Thomas Hipp <[email protected]> --- shared/definition.go | 6 +----- shared/definition_test.go | 27 +++++++++++++++------------ 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/shared/definition.go b/shared/definition.go index 5d70918..64bfd0d 100644 --- a/shared/definition.go +++ b/shared/definition.go @@ -32,7 +32,7 @@ type DefinitionImage struct { // A DefinitionSource specifies the download type and location type DefinitionSource struct { Downloader string `yaml:"downloader"` - URL string `yaml:"url"` + URL string `yaml:"url,omitempty"` Keys []string `yaml:"keys,omitempty"` Keyserver string `yaml:"keyserver,omitempty"` Variant string `yaml:"variant,omitempty"` @@ -122,10 +122,6 @@ func ValidateDefinition(def Definition) error { return fmt.Errorf("source.downloader must be one of %v", validDownloaders) } - if strings.TrimSpace(def.Source.URL) == "" { - return errors.New("source.url may not be empty") - } - validManagers := []string{ "apk", "apt", diff --git a/shared/definition_test.go b/shared/definition_test.go index 32b1886..ded182f 100644 --- a/shared/definition_test.go +++ b/shared/definition_test.go @@ -66,37 +66,40 @@ func TestValidateDefinition(t *testing.T) { false, }, { - "empty image.distribution", - Definition{}, - "image.distribution may not be empty", - true, - }, - { - "invalid source.downloader", + "valid Defintion without source.url", Definition{ Image: DefinitionImage{ Distribution: "ubuntu", Release: "artful", }, Source: DefinitionSource{ - Downloader: "foo", + Downloader: "debootstrap", + }, + Packages: DefinitionPackages{ + Manager: "apt", }, }, - "source.downloader must be one of .+", + "", + false, + }, + { + "empty image.distribution", + Definition{}, + "image.distribution may not be empty", true, }, { - "empty source.url", + "invalid source.downloader", Definition{ Image: DefinitionImage{ Distribution: "ubuntu", Release: "artful", }, Source: DefinitionSource{ - Downloader: "debootstrap", + Downloader: "foo", }, }, - "source.url may not be empty", + "source.downloader must be one of .+", true, }, { From e85a2eda7bb5bbf21bab06c276f4a862a16ce107 Mon Sep 17 00:00:00 2001 From: Thomas Hipp <[email protected]> Date: Thu, 1 Mar 2018 09:05:32 +0100 Subject: [PATCH 7/7] sources,shared: Add Suite option to Source The Suite option is used by debootstrap, and will create a temporary symlink in /usr/share/debootstrap/scripts. This allows realeased images which aren't yet in debootstrap to be used. Signed-off-by: Thomas Hipp <[email protected]> --- shared/definition.go | 1 + sources/debootstrap.go | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/shared/definition.go b/shared/definition.go index 64bfd0d..fb42f5e 100644 --- a/shared/definition.go +++ b/shared/definition.go @@ -36,6 +36,7 @@ type DefinitionSource struct { Keys []string `yaml:"keys,omitempty"` Keyserver string `yaml:"keyserver,omitempty"` Variant string `yaml:"variant,omitempty"` + Suite string `yaml:"suite,omitempty"` } // A DefinitionTargetLXC represents LXC specific files as part of the metadata. diff --git a/sources/debootstrap.go b/sources/debootstrap.go index 11b9a19..d4682bc 100644 --- a/sources/debootstrap.go +++ b/sources/debootstrap.go @@ -45,5 +45,16 @@ func (s *Debootstrap) Run(source shared.DefinitionSource, release, arch, cacheDi args = append(args, source.URL) } + // If source.Suite is set, create a symlink in /usr/share/debootstrap/scripts + // pointing release to source.Suite. + if source.Suite != "" { + link := filepath.Join("/usr/share/debootstrap/scripts", release) + err := os.Symlink(source.Suite, link) + if err != nil { + return err + } + defer os.Remove(link) + } + return shared.RunCommand("debootstrap", args...) }
_______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
