Merge pull request #1626 from barnumbirr/configurable-by-hash-algorithms

Make the set of Acquire-By-Hash algorithms configurable

api/publish.go

@@ -387,6 +387,9 @@ func apiPublishRepoOrSnapshot(c *gin.Context) {
 			return &task.ProcessReturnValue{Code: http.StatusBadRequest, Value: nil}, fmt.Errorf("prefix/distribution already used by another published repo: %s", duplicate)
 		}
 
+		// Which hash algorithms get Acquire-By-Hash entries. Each one is a full
+		// set of index copies per publish, which is felt most on S3-like storage.
+		published.AcquireByHashAlgorithms = context.Config().AcquireByHashAlgorithms
 		err = published.Publish(context.PackagePool(), context, taskCollectionFactory, signer, publishOutput, b.ForceOverwrite, context.SkelPath())
 		if err != nil {
 			return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to publish: %s", err)
@@ -578,6 +581,9 @@ func apiPublishUpdateSwitch(c *gin.Context) {
 			return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to update: %s", err)
 		}
 
+		// Which hash algorithms get Acquire-By-Hash entries. Each one is a full
+		// set of index copies per publish, which is felt most on S3-like storage.
+		published.AcquireByHashAlgorithms = context.Config().AcquireByHashAlgorithms
 		err = published.Publish(context.PackagePool(), context, taskCollectionFactory, signer, out, b.ForceOverwrite, context.SkelPath())
 		if err != nil {
 			return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to update: %s", err)
@@ -1245,6 +1251,9 @@ func apiPublishUpdate(c *gin.Context) {
 			return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to update: %s", err)
 		}
 
+		// Which hash algorithms get Acquire-By-Hash entries. Each one is a full
+		// set of index copies per publish, which is felt most on S3-like storage.
+		published.AcquireByHashAlgorithms = context.Config().AcquireByHashAlgorithms
 		err = published.Publish(context.PackagePool(), context, taskCollectionFactory, signer, out, b.ForceOverwrite, context.SkelPath())
 		if err != nil {
 			return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to update: %s", err)

cmd/publish_snapshot.go

@@ -178,6 +178,9 @@ func aptlyPublishSnapshotOrRepo(cmd *commander.Command, args []string) error {
 		context.Progress().ColoredPrintf("@rWARNING@|: force overwrite mode enabled, aptly might corrupt other published repositories sharing the same package pool.\n")
 	}
 
+	// Which hash algorithms get Acquire-By-Hash entries. Each one is a full
+	// set of index copies per publish, which is felt most on S3-like storage.
+	published.AcquireByHashAlgorithms = context.Config().AcquireByHashAlgorithms
 	err = published.Publish(context.PackagePool(), context, collectionFactory, signer, context.Progress(), forceOverwrite, context.SkelPath())
 	if err != nil {
 		return fmt.Errorf("unable to publish: %s", err)

cmd/publish_switch.go

@@ -111,6 +111,9 @@ func aptlyPublishSwitch(cmd *commander.Command, args []string) error {
 		published.MultiDist = context.Flags().Lookup("multi-dist").Value.Get().(bool)
 	}
 
+	// Which hash algorithms get Acquire-By-Hash entries. Each one is a full
+	// set of index copies per publish, which is felt most on S3-like storage.
+	published.AcquireByHashAlgorithms = context.Config().AcquireByHashAlgorithms
 	err = published.Publish(context.PackagePool(), context, collectionFactory, signer, context.Progress(), forceOverwrite, context.SkelPath())
 	if err != nil {
 		return fmt.Errorf("unable to publish: %s", err)

cmd/publish_update.go

@@ -80,6 +80,9 @@ func aptlyPublishUpdate(cmd *commander.Command, args []string) error {
 		published.Version = context.Flags().Lookup("version").Value.String()
 	}
 
+	// Which hash algorithms get Acquire-By-Hash entries. Each one is a full
+	// set of index copies per publish, which is felt most on S3-like storage.
+	published.AcquireByHashAlgorithms = context.Config().AcquireByHashAlgorithms
 	err = published.Publish(context.PackagePool(), context, collectionFactory, signer, context.Progress(), forceOverwrite, context.SkelPath())
 	if err != nil {
 		return fmt.Errorf("unable to publish: %s", err)

deb/index_files.go

@@ -22,6 +22,7 @@ type indexFiles struct {
 	suffix           string
 	indexes          map[string]*indexFile
 	acquireByHash    bool
+	byHashAlgorithms []string
 	skipBz2          bool
 }
 
@@ -111,7 +112,11 @@ func (file *indexFile) Finalize(signer pgp.Signer) error {
 	}
 
 	if file.acquireByHash {
-		for _, hash := range []string{"MD5Sum", "SHA1", "SHA256", "SHA512"} {
+		algorithms, algErr := file.parent.hashAlgorithms()
+		if algErr != nil {
+			return algErr
+		}
+		for _, hash := range algorithms {
 			err = file.parent.publishedStorage.MkDir(filepath.Join(filedir, "by-hash", hash))
 			if err != nil {
 				return fmt.Errorf("unable to create dir: %s", err)
@@ -133,8 +138,13 @@ func (file *indexFile) Finalize(signer pgp.Signer) error {
 
 		if file.acquireByHash {
 			sums := file.parent.generatedFiles[file.relativePath+ext]
-			for hash, sum := range map[string]string{"SHA512": sums.SHA512, "SHA256": sums.SHA256, "SHA1": sums.SHA1, "MD5Sum": sums.MD5} {
-				err = packageIndexByHash(file, ext, hash, sum)
+			all := map[string]string{"SHA512": sums.SHA512, "SHA256": sums.SHA256, "SHA1": sums.SHA1, "MD5Sum": sums.MD5}
+			algorithms, algErr := file.parent.hashAlgorithms()
+			if algErr != nil {
+				return algErr
+			}
+			for _, hash := range algorithms {
+				err = packageIndexByHash(file, ext, hash, all[hash])
 				if err != nil {
 					return fmt.Errorf("unable to build hash file: %s", err)
 				}
@@ -185,6 +195,51 @@ func (file *indexFile) Finalize(signer pgp.Signer) error {
 	return nil
 }
 
+// knownHashAlgorithms are the algorithms aptly can write Acquire-By-Hash
+// entries for, in the order the entries are written.
+var knownHashAlgorithms = []string{"MD5Sum", "SHA1", "SHA256", "SHA512"}
+
+// hashAlgorithms is the set of Acquire-By-Hash algorithms to write entries for.
+// Empty means all of them, which is what aptly did before the set was
+// configurable, so an existing config behaves exactly as before.
+//
+// The order is knownHashAlgorithms' rather than the config's, so that the
+// directories created in one loop and the entries written in another always
+// agree whatever order the user listed.
+//
+// An unrecognised name is an error rather than something to skip. Skipping it
+// would turn a typo into "no by-hash entries for that algorithm", which looks
+// identical to having configured it away.
+func (files *indexFiles) hashAlgorithms() ([]string, error) {
+	if len(files.byHashAlgorithms) == 0 {
+		return knownHashAlgorithms, nil
+	}
+
+	wanted := make(map[string]bool, len(files.byHashAlgorithms))
+	for _, want := range files.byHashAlgorithms {
+		known := false
+		for _, k := range knownHashAlgorithms {
+			if k == want {
+				known = true
+				break
+			}
+		}
+		if !known {
+			return nil, fmt.Errorf("unknown Acquire-By-Hash algorithm %q, expected one of %v",
+				want, knownHashAlgorithms)
+		}
+		wanted[want] = true
+	}
+
+	ordered := make([]string, 0, len(wanted))
+	for _, known := range knownHashAlgorithms {
+		if wanted[known] {
+			ordered = append(ordered, known)
+		}
+	}
+	return ordered, nil
+}
+
 func packageIndexByHash(file *indexFile, ext string, hash string, sum string) error {
 	src := filepath.Join(file.parent.basePath, file.relativePath)
 	indexfile := path.Base(src + ext)
@@ -235,7 +290,7 @@ func packageIndexByHash(file *indexFile, ext string, hash string, sum string) er
 	return nil
 }
 
-func newIndexFiles(publishedStorage aptly.PublishedStorage, basePath, tempDir, suffix string, acquireByHash bool, skipBz2 bool) *indexFiles {
+func newIndexFiles(publishedStorage aptly.PublishedStorage, basePath, tempDir, suffix string, acquireByHash bool, byHashAlgorithms []string, skipBz2 bool) *indexFiles {
 	return &indexFiles{
 		publishedStorage: publishedStorage,
 		basePath:         basePath,
@@ -245,6 +300,7 @@ func newIndexFiles(publishedStorage aptly.PublishedStorage, basePath, tempDir, s
 		suffix:           suffix,
 		indexes:          make(map[string]*indexFile),
 		acquireByHash:    acquireByHash,
+		byHashAlgorithms: byHashAlgorithms,
 		skipBz2:          skipBz2,
 	}
 }

deb/index_files_test.go

@@ -0,0 +1,49 @@
+package deb
+
+import (
+	. "gopkg.in/check.v1"
+)
+
+type IndexFilesSuite struct{}
+
+var _ = Suite(&IndexFilesSuite{})
+
+func (s *IndexFilesSuite) TestHashAlgorithmsDefaultsToAll(c *C) {
+	// Unset must behave exactly as aptly did before the set was configurable,
+	// so an existing config keeps writing all four.
+	for _, files := range []*indexFiles{{}, {byHashAlgorithms: []string{}}} {
+		got, err := files.hashAlgorithms()
+		c.Check(err, IsNil)
+		c.Check(got, DeepEquals, []string{"MD5Sum", "SHA1", "SHA256", "SHA512"})
+	}
+}
+
+func (s *IndexFilesSuite) TestHashAlgorithmsHonoursTheConfiguredSet(c *C) {
+	got, err := (&indexFiles{byHashAlgorithms: []string{"SHA256"}}).hashAlgorithms()
+	c.Check(err, IsNil)
+	c.Check(got, DeepEquals, []string{"SHA256"})
+
+	got, err = (&indexFiles{byHashAlgorithms: []string{"SHA256", "SHA512"}}).hashAlgorithms()
+	c.Check(err, IsNil)
+	c.Check(got, DeepEquals, []string{"SHA256", "SHA512"})
+}
+
+func (s *IndexFilesSuite) TestHashAlgorithmsIsOrderedIndependentlyOfConfig(c *C) {
+	// The directories are created in one loop and the entries written in
+	// another. If the order came from the config those two could disagree,
+	// so it is fixed here whatever order the user listed.
+	got, err := (&indexFiles{byHashAlgorithms: []string{"SHA512", "MD5Sum", "SHA256"}}).hashAlgorithms()
+	c.Check(err, IsNil)
+	c.Check(got, DeepEquals, []string{"MD5Sum", "SHA256", "SHA512"})
+}
+
+func (s *IndexFilesSuite) TestHashAlgorithmsRejectsUnknownNames(c *C) {
+	// Loudly, not by skipping. Filtering a typo would produce "no by-hash
+	// entries for that algorithm", which is indistinguishable from having
+	// deliberately configured it away.
+	_, err := (&indexFiles{byHashAlgorithms: []string{"SHA256", "SHA3"}}).hashAlgorithms()
+	c.Check(err, ErrorMatches, `unknown Acquire-By-Hash algorithm "SHA3".*`)
+
+	_, err = (&indexFiles{byHashAlgorithms: []string{"sha256"}}).hashAlgorithms()
+	c.Check(err, NotNil)
+}

deb/publish.go

@@ -76,6 +76,10 @@ type PublishedRepo struct {
 
 	// Skip bz2 compression for index files
 	SkipBz2 bool
+	// AcquireByHashAlgorithms limits which hash algorithms get Acquire-By-Hash
+	// entries. Set per invocation from the config rather than persisted, the
+	// same way SkipBz2 is. Empty means all four.
+	AcquireByHashAlgorithms []string
 
 	// True if repo is being re-published
 	rePublishing bool
@@ -907,7 +911,7 @@ func (p *PublishedRepo) Publish(packagePool aptly.PackagePool, publishedStorageP
 	}
 	defer func() { _ = os.RemoveAll(tempDir) }()
 
-	indexes := newIndexFiles(publishedStorage, basePath, tempDir, suffix, p.AcquireByHash, p.SkipBz2)
+	indexes := newIndexFiles(publishedStorage, basePath, tempDir, suffix, p.AcquireByHash, p.AcquireByHashAlgorithms, p.SkipBz2)
 
 	legacyContentIndexes := map[string]*ContentsIndex{}
 	var count int64

man/aptly.1

@@ -71,6 +71,13 @@ The legacy json configuration is still supported (and also supports comments):
   // empty array defaults to all available architectures
   "architectures": [],
 
+  // Hash algorithms to write Acquire\-By\-Hash entries for, when a publish
+  // has Acquire\-By\-Hash enabled. Each algorithm costs a full set of index
+  // copies on every publish, which is felt most on S3\-like storage where
+  // every write is a round trip. An empty array defaults to all four.
+  // Modern apt uses SHA256; MD5Sum and SHA1 are deprecated.
+  "acquireByHashAlgorithms": ["MD5Sum", "SHA1", "SHA256", "SHA512"],
+
   // Follow contents of `Suggests:` field when processing dependencies for the package
   "dependencyFollowSuggests": false,
 

man/aptly.1.ronn.tmpl

@@ -60,6 +60,13 @@ The legacy json configuration is still supported (and also supports comments):
       // empty array defaults to all available architectures
       "architectures": [],
 
+      // Hash algorithms to write Acquire-By-Hash entries for, when a publish
+      // has Acquire-By-Hash enabled. Each algorithm costs a full set of index
+      // copies on every publish, which is felt most on S3-like storage where
+      // every write is a round trip. An empty array defaults to all four.
+      // Modern apt uses SHA256; MD5Sum and SHA1 are deprecated.
+      "acquireByHashAlgorithms": ["MD5Sum", "SHA1", "SHA256", "SHA512"],
+
       // Follow contents of `Suggests:` field when processing dependencies for the package
       "dependencyFollowSuggests": false,
 

system/t02_config/ConfigShowTest_gold

@@ -4,6 +4,12 @@
     "logFormat": "default",
     "databaseOpenAttempts": 10,
     "architectures": [],
+    "acquireByHashAlgorithms": [
+        "MD5Sum",
+        "SHA1",
+        "SHA256",
+        "SHA512"
+    ],
     "skipLegacyPool": false,
     "dependencyFollowSuggests": false,
     "dependencyFollowRecommends": false,

system/t02_config/ConfigShowYAMLTest_gold

@@ -3,6 +3,11 @@ log_level: debug
 log_format: default
 database_open_attempts: 10
 architectures: []
+acquire_by_hash_algorithms:
+    - MD5Sum
+    - SHA1
+    - SHA256
+    - SHA512
 skip_legacy_pool: false
 dep_follow_suggests: false
 dep_follow_recommends: false

utils/config.go

@@ -19,6 +19,11 @@ type ConfigStructure struct { // nolint: maligned
 	LogFormat            string   `json:"logFormat"                     yaml:"log_format"`
 	DatabaseOpenAttempts int      `json:"databaseOpenAttempts"          yaml:"database_open_attempts"`
 	Architectures        []string `json:"architectures"                 yaml:"architectures"`
+	// Hash algorithms to write Acquire-By-Hash entries for. Each one costs a
+	// full set of index copies on every publish, which is felt most on S3-like
+	// storage where every write is a round trip. Empty means all four, which is
+	// what aptly did before this was configurable.
+	AcquireByHashAlgorithms []string `json:"acquireByHashAlgorithms"    yaml:"acquire_by_hash_algorithms"`
 	SkipLegacyPool       bool     `json:"skipLegacyPool"                yaml:"skip_legacy_pool"` // OBSOLETE
 
 	// Dependency following
@@ -247,6 +252,7 @@ type AzureEndpoint struct {
 // Config is configuration for aptly, shared by all modules
 var Config = ConfigStructure{
 	RootDir:                filepath.Join(os.Getenv("HOME"), ".aptly"),
+	AcquireByHashAlgorithms: []string{"MD5Sum", "SHA1", "SHA256", "SHA512"},
 	DownloadConcurrency:    4,
 	DownloadLimit:          0,
 	Downloader:             "default",

utils/config_test.go

@@ -83,6 +83,7 @@ func (s *ConfigSuite) TestSaveConfig(c *C) {
   "logFormat": "json",
   "databaseOpenAttempts": 5,
   "architectures": null,
+  "acquireByHashAlgorithms": null,
   "skipLegacyPool": false,
   "dependencyFollowSuggests": false,
   "dependencyFollowRecommends": false,
@@ -280,6 +281,7 @@ func (s *ConfigSuite) TestSaveYAML2Config(c *C) {
     "log_format: \"\"\n"+
     "database_open_attempts: 0\n"+
     "architectures: []\n"+
+    "acquire_by_hash_algorithms: []\n"+
     "skip_legacy_pool: false\n"+
     "dep_follow_suggests: false\n"+
     "dep_follow_recommends: false\n"+
@@ -340,6 +342,7 @@ database_open_attempts: 10
 architectures:
     - amd64
     - arm64
+acquire_by_hash_algorithms: []
 skip_legacy_pool: true
 dep_follow_suggests: true
 dep_follow_recommends: true