From a584b2e0581c01bf4dcc9f28669ae7540b8512c8 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Fri, 11 Aug 2017 20:05:55 +0300 Subject: [PATCH] Fix bug with `PoolPath` field being overwritten on mirror update While updating mirror, if package file is already in pool path, field `PoolPath` was left as empty which results in package file being unavailable later on while publishing. --- aptly/interfaces.go | 2 +- deb/package.go | 7 ++-- deb/package_files.go | 7 +++- files/package_pool.go | 16 ++++----- files/package_pool_test.go | 33 ++++++++++++------- system/t06_publish/PublishSnapshot37Test_gold | 13 ++++++++ system/t06_publish/snapshot.py | 15 +++++++++ 7 files changed, 69 insertions(+), 24 deletions(-) create mode 100644 system/t06_publish/PublishSnapshot37Test_gold diff --git a/aptly/interfaces.go b/aptly/interfaces.go index 4761a5db6..3eb5102eb 100644 --- a/aptly/interfaces.go +++ b/aptly/interfaces.go @@ -23,7 +23,7 @@ type PackagePool interface { // // if poolPath is empty, poolPath is generated automatically based on checksum info (if available) // in any case, if function returns true, it also fills back checksums with complete information about the file in the pool - Verify(poolPath, basename string, checksums *utils.ChecksumInfo, checksumStorage ChecksumStorage) (bool, error) + Verify(poolPath, basename string, checksums *utils.ChecksumInfo, checksumStorage ChecksumStorage) (string, bool, error) // Import copies file into package pool // // - srcPath is full path to source file as it is now diff --git a/deb/package.go b/deb/package.go index 099e15510..996aef457 100644 --- a/deb/package.go +++ b/deb/package.go @@ -629,14 +629,15 @@ type PackageDownloadTask struct { func (p *Package) DownloadList(packagePool aptly.PackagePool, checksumStorage aptly.ChecksumStorage) (result []PackageDownloadTask, err error) { result = make([]PackageDownloadTask, 0, 1) - for idx, f := range p.Files() { - verified, err := f.Verify(packagePool, checksumStorage) + files := p.Files() + for idx := range files { + verified, err := files[idx].Verify(packagePool, checksumStorage) if err != nil { return nil, err } if !verified { - result = append(result, PackageDownloadTask{File: &p.Files()[idx]}) + result = append(result, PackageDownloadTask{File: &files[idx]}) } } diff --git a/deb/package_files.go b/deb/package_files.go index 6874503d3..4716219c4 100644 --- a/deb/package_files.go +++ b/deb/package_files.go @@ -27,7 +27,12 @@ type PackageFile struct { // Verify that package file is present and correct func (f *PackageFile) Verify(packagePool aptly.PackagePool, checksumStorage aptly.ChecksumStorage) (bool, error) { - return packagePool.Verify(f.PoolPath, f.Filename, &f.Checksums, checksumStorage) + generatedPoolPath, exists, err := packagePool.Verify(f.PoolPath, f.Filename, &f.Checksums, checksumStorage) + if exists && err == nil { + f.PoolPath = generatedPoolPath + } + + return exists, err } // GetPoolPath returns path to the file in the pool diff --git a/files/package_pool.go b/files/package_pool.go index 24ae9fb4f..bd2cd26c3 100644 --- a/files/package_pool.go +++ b/files/package_pool.go @@ -162,7 +162,7 @@ func (pool *PackagePool) ensureChecksums(poolPath, fullPoolPath string, checksum // // if poolPath is empty, poolPath is generated automatically based on checksum info (if available) // in any case, if function returns true, it also fills back checksums with complete information about the file in the pool -func (pool *PackagePool) Verify(poolPath, basename string, checksums *utils.ChecksumInfo, checksumStorage aptly.ChecksumStorage) (bool, error) { +func (pool *PackagePool) Verify(poolPath, basename string, checksums *utils.ChecksumInfo, checksumStorage aptly.ChecksumStorage) (string, bool, error) { possiblePoolPaths := []string{} if poolPath != "" { @@ -172,7 +172,7 @@ func (pool *PackagePool) Verify(poolPath, basename string, checksums *utils.Chec if checksums.SHA256 != "" { modernPath, err := pool.buildPoolPath(basename, checksums) if err != nil { - return false, err + return "", false, err } possiblePoolPaths = append(possiblePoolPaths, modernPath) } @@ -180,7 +180,7 @@ func (pool *PackagePool) Verify(poolPath, basename string, checksums *utils.Chec if pool.supportLegacyPaths && checksums.MD5 != "" { legacyPath, err := pool.LegacyPath(basename, checksums) if err != nil { - return false, err + return "", false, err } possiblePoolPaths = append(possiblePoolPaths, legacyPath) } @@ -193,7 +193,7 @@ func (pool *PackagePool) Verify(poolPath, basename string, checksums *utils.Chec if err != nil { if !os.IsNotExist(err) { // unable to stat target location? - return false, err + return "", false, err } // doesn't exist, skip it continue @@ -208,21 +208,21 @@ func (pool *PackagePool) Verify(poolPath, basename string, checksums *utils.Chec targetChecksums, err = pool.ensureChecksums(path, fullPoolPath, checksumStorage) if err != nil { - return false, err + return "", false, err } if checksums.MD5 != "" && targetChecksums.MD5 != checksums.MD5 || checksums.SHA256 != "" && targetChecksums.SHA256 != checksums.SHA256 { // wrong file? - return false, nil + return "", false, nil } // fill back checksums *checksums = *targetChecksums - return true, nil + return path, true, nil } - return false, nil + return "", false, nil } // Import copies file into package pool diff --git a/files/package_pool_test.go b/files/package_pool_test.go index cc824aa71..a1210e4a8 100644 --- a/files/package_pool_test.go +++ b/files/package_pool_test.go @@ -155,7 +155,8 @@ func (s *PackagePoolSuite) TestImportLegacy(c *C) { func (s *PackagePoolSuite) TestVerifyLegacy(c *C) { s.checksum.Size = 2738 // file doesn't exist yet - exists, err := s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs) + path, exists, err := s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs) + c.Check(path, Equals, "") c.Check(err, IsNil) c.Check(exists, Equals, false) @@ -164,7 +165,8 @@ func (s *PackagePoolSuite) TestVerifyLegacy(c *C) { c.Assert(err, IsNil) // check existence (and fills back checksum) - exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs) + path, exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs) + c.Check(path, Equals, "00/35/libboost-program-options-dev_1.49.0.1_i386.deb") c.Check(err, IsNil) c.Check(exists, Equals, true) c.Check(s.checksum.SHA512, Equals, "d7302241373da972aa9b9e71d2fd769b31a38f71182aa71bc0d69d090d452c69bb74b8612c002ccf8a89c279ced84ac27177c8b92d20f00023b3d268e6cec69c") @@ -172,7 +174,8 @@ func (s *PackagePoolSuite) TestVerifyLegacy(c *C) { func (s *PackagePoolSuite) TestVerify(c *C) { // file doesn't exist yet - exists, err := s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs) + ppath, exists, err := s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs) + c.Check(ppath, Equals, "") c.Check(err, IsNil) c.Check(exists, Equals, false) @@ -182,20 +185,23 @@ func (s *PackagePoolSuite) TestVerify(c *C) { c.Check(path, Equals, "c7/6b/4bd12fd92e4dfe1b55b18a67a669_libboost-program-options-dev_1.49.0.1_i386.deb") // check existence - exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs) + ppath, exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs) + c.Check(ppath, Equals, ppath) c.Check(err, IsNil) c.Check(exists, Equals, true) c.Check(s.checksum.SHA512, Equals, "d7302241373da972aa9b9e71d2fd769b31a38f71182aa71bc0d69d090d452c69bb74b8612c002ccf8a89c279ced84ac27177c8b92d20f00023b3d268e6cec69c") // check existence with fixed path - exists, err = s.pool.Verify(path, filepath.Base(s.debFile), &s.checksum, s.cs) + ppath, exists, err = s.pool.Verify(path, filepath.Base(s.debFile), &s.checksum, s.cs) + c.Check(ppath, Equals, path) c.Check(err, IsNil) c.Check(exists, Equals, true) c.Check(s.checksum.SHA512, Equals, "d7302241373da972aa9b9e71d2fd769b31a38f71182aa71bc0d69d090d452c69bb74b8612c002ccf8a89c279ced84ac27177c8b92d20f00023b3d268e6cec69c") // check existence, but with missing checksum s.checksum.SHA512 = "" - exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs) + ppath, exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs) + c.Check(ppath, Equals, path) c.Check(err, IsNil) c.Check(exists, Equals, true) // checksum is filled back based on checksum storage @@ -205,7 +211,8 @@ func (s *PackagePoolSuite) TestVerify(c *C) { ck := utils.ChecksumInfo{ Size: s.checksum.Size, } - exists, err = s.pool.Verify(path, filepath.Base(s.debFile), &ck, s.cs) + ppath, exists, err = s.pool.Verify(path, filepath.Base(s.debFile), &ck, s.cs) + c.Check(ppath, Equals, path) c.Check(err, IsNil) c.Check(exists, Equals, true) // checksum is filled back based on checksum storage @@ -213,14 +220,16 @@ func (s *PackagePoolSuite) TestVerify(c *C) { // check existence, with wrong checksum info but correct path and size available ck.SHA256 = "abc" - exists, err = s.pool.Verify(path, filepath.Base(s.debFile), &ck, s.cs) + ppath, exists, err = s.pool.Verify(path, filepath.Base(s.debFile), &ck, s.cs) + c.Check(ppath, Equals, "") c.Check(err, IsNil) c.Check(exists, Equals, false) // check existence, with missing checksum and no info in checksum storage delete(s.cs.(*mockChecksumStorage).store, path) s.checksum.SHA512 = "" - exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs) + ppath, exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs) + c.Check(ppath, Equals, path) c.Check(err, IsNil) c.Check(exists, Equals, true) // checksum is filled back based on re-calculation @@ -228,12 +237,14 @@ func (s *PackagePoolSuite) TestVerify(c *C) { // check existence, with wrong size s.checksum.Size = 13455 - exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs) + ppath, exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs) + c.Check(ppath, Equals, "") c.Check(err, IsNil) c.Check(exists, Equals, false) // check existence, with empty checksum info - exists, err = s.pool.Verify("", filepath.Base(s.debFile), &utils.ChecksumInfo{}, s.cs) + ppath, exists, err = s.pool.Verify("", filepath.Base(s.debFile), &utils.ChecksumInfo{}, s.cs) + c.Check(ppath, Equals, "") c.Check(err, IsNil) c.Check(exists, Equals, false) } diff --git a/system/t06_publish/PublishSnapshot37Test_gold b/system/t06_publish/PublishSnapshot37Test_gold new file mode 100644 index 000000000..0365afcb4 --- /dev/null +++ b/system/t06_publish/PublishSnapshot37Test_gold @@ -0,0 +1,13 @@ +Loading packages... +Generating metadata files and linking package files... +Finalizing metadata files... +Signing file 'Release' with gpg, please enter your passphrase when prompted: +Clearsigning file 'Release' with gpg, please enter your passphrase when prompted: + +Snapshot wheezy has been successfully published. +Please setup your webserver to serve directory '${HOME}/.aptly/public' with autoindexing. +Now you can add following line to apt sources: + deb http://your-server/ wheezy main +Don't forget to add your GPG key to apt with apt-key. + +You can also use `aptly serve` to publish your repositories over HTTP quickly. diff --git a/system/t06_publish/snapshot.py b/system/t06_publish/snapshot.py index 11ec88f3f..3a2af0f6e 100644 --- a/system/t06_publish/snapshot.py +++ b/system/t06_publish/snapshot.py @@ -1001,3 +1001,18 @@ def check(self): self.check_not_exists('public/dists/maverick/main/Contents-i386.gz') self.check_exists('public/dists/maverick/main/binary-amd64/Release') self.check_not_exists('public/dists/maverick/main/Contents-amd64.gz') + + +class PublishSnapshot37Test(BaseTest): + """ + publish snapshot: mirror with double mirror update + """ + fixtureGpg = True + fixtureCmds = [ + "aptly -architectures=i386,amd64 mirror create -keyring=aptlytest.gpg -filter='$$Source (gnupg)' -with-udebs wheezy http://mirror.yandex.ru/debian/ wheezy main non-free", + "aptly mirror update -keyring=aptlytest.gpg wheezy", + "aptly mirror update -keyring=aptlytest.gpg wheezy", + "aptly snapshot create wheezy from mirror wheezy", + ] + runCmd = "aptly publish snapshot -keyring=${files}/aptly.pub -secret-keyring=${files}/aptly.sec wheezy" + gold_processor = BaseTest.expand_environ