Add support for choosing the package's release

Up until now, the appregistry-server always pulled the latest
version of the requested packages, which makes sense for production,
but not for testing and release phases of the packages.

During testing, multiple version of the same package can be
tested at the same time, and when releasing, it's not mandatory
that the different releases will be released on the same date.

When combining the appregistry-server logic, with the properties of the
test and release phases, there is a need to create a complex automation
for merging multiple versions (so they can be tested at the time) into
a single bundle, and omitting non-ready releases from bundles,
after testing and before the release.

This change adds the possibility to specify the requested release after
the package's name (e.g "Kubevirt:10.0.0"). If the version is not specified,
the server will download the latest version from the appregistry.

Signed-off-by: gbenhaim <[email protected]>

Author: gbenhaim

cmd/appregistry-server/main.go

@@ -37,7 +37,7 @@ func main() {
 	rootCmd.Flags().StringP("download-folder", "f", "downloaded", "directory where downloaded nested operator bundle(s) will be stored to be processed further")
 	rootCmd.Flags().StringP("database", "d", "bundles.db", "name of db to output")
 	rootCmd.Flags().StringSliceP("registry", "r", []string{}, "pipe delimited operator source - {base url with cnr prefix}|{quay registry namespace}|{secret namespace/secret name}")
-	rootCmd.Flags().StringP("packages", "o", "", "comma separated list of package(s) to be downloaded from the specified operator source(s)")
+	rootCmd.Flags().StringP("packages", "o", "", "comma separated list of package(s) to be downloaded from the specified operator source(s). The requested release can be appended to the package name, delimited with a colone (e.g some-pkg:1.1.0)")
 	rootCmd.Flags().StringP("port", "p", "50051", "port number to serve on")
 	rootCmd.Flags().StringP("termination-log", "t", "/dev/termination-log", "path to a container termination log file")
 	rootCmd.Flags().Bool("strict", false, "fail on registry load errors")

go.sum

@@ -487,6 +487,7 @@ github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An
 github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48=
 github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE=
 github.com/stretchr/testify v0.0.0-20151208002404-e3a8ff8ce365/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w=

pkg/apprclient/client.go

@@ -64,7 +64,8 @@ func (c *client) ListPackages(namespace string) ([]*RegistryMetadata, error) {
 			Name:      repository,
 
 			// 'Default' points to the latest release pushed.
-			Release: pkg.Default,
+			Release:  pkg.Default,
+			Releases: pkg.Releases,
 
 			// Getting 'Digest' would require an additional call to the app
 			// registry, so it is being defaulted.

pkg/apprclient/metadata.go

@@ -31,9 +31,12 @@ type RegistryMetadata struct {
 	// registry.
 	Name string
 
-	// Release represents the version number of the given operator manifest.
+	// Release represents the latest version number of the given operator manifest.
 	Release string
 
+	// Releases represents all the available releases of the given operator manifest
+	Releases []string
+
 	// Digest is the sha256 hash value that uniquely corresponds to the blob
 	// associated with this particular release of the operator manifest.
 	Digest string
@@ -47,3 +50,15 @@ func (rm *RegistryMetadata) ID() string {
 func (rm *RegistryMetadata) String() string {
 	return fmt.Sprintf("%s/%s:%s", rm.Namespace, rm.Name, rm.Release)
 }
+
+// ReleaseMap returns a map between all the available releases of a package to
+// a bool, usefull for checking is some release is available for a package.
+func (rm *RegistryMetadata) ReleaseMap() map[string]bool {
+	releases := map[string]bool{}
+
+	for _, release := range rm.Releases {
+		releases[release] = false
+	}
+
+	return releases
+}

pkg/appregistry/downloader.go

@@ -20,10 +20,13 @@ type downloadItem struct {
 
 	// Source refers to the remote appregistry URL and remote registry namespace.
 	Source *Source
+
+	// Release refers to the release number the user requested
+	Release string
 }
 
 func (d *downloadItem) String() string {
-	return fmt.Sprintf("%s", d.RepositoryMetadata)
+	return fmt.Sprintf("%s:%s", d.RepositoryMetadata.Name, d.Release)
 }
 
 type registryOptionsGetter interface {
@@ -73,11 +76,18 @@ func (d *downloader) Download(input *Input) (manifests []*apprclient.OperatorMet
 		d.logger.Errorf("the following error(s) occurred while preparing the download list: %v", err)
 
 		if len(items) == 0 {
-			d.logger.Infof("download list is empty, bailing out: %s", input.Packages)
+			d.logger.Infof("download list is empty, bailing out: %v", input.Packages)
 			return
 		}
 	}
 
+	for _, item := range items {
+		d.logger.Infof(
+			"the following releases are available for package %s -> %s",
+			item.RepositoryMetadata.Name,
+			item.RepositoryMetadata.Releases,
+		)
+	}
 	d.logger.Infof("resolved the following packages: %s", items)
 
 	manifests, err = d.DownloadRepositories(items)
@@ -95,7 +105,7 @@ func (d *downloader) Download(input *Input) (manifests []*apprclient.OperatorMet
 // log it and move on.
 func (d *downloader) Prepare(input *Input) (items []*downloadItem, err error) {
 	packageMap := input.PackagesToMap()
-	itemMap := map[string]*downloadItem{}
+	itemMap := map[Package]*downloadItem{}
 	allErrors := []error{}
 
 	for _, source := range input.Sources {
@@ -112,22 +122,36 @@ func (d *downloader) Prepare(input *Input) (items []*downloadItem, err error) {
 			continue
 		}
 
+		repositoryMap := map[string]*apprclient.RegistryMetadata{}
 		for _, metadata := range repositoryList {
-			// Repository name has a one to one mapping to operator/package name.
-			// We use this as the key.
-			key := metadata.Name
-
-			if _, ok := packageMap[key]; ok {
-				// The package specified has been resolved to this repository
-				// name in remote registry.
-				itemMap[key] = &downloadItem{
-					RepositoryMetadata: metadata,
-					Source:             source,
+			repositoryMap[metadata.Name] = metadata
+		}
+
+		for _, pkg := range input.Packages {
+			metadata, ok := repositoryMap[pkg.Name]
+			if !ok {
+				// The package is not in the current source
+				continue
+			}
+			// If a specific release was requrested, download it
+			release := pkg.Release
+			if release != "" {
+				releaseMap := metadata.ReleaseMap()
+				if _, ok := releaseMap[pkg.Release]; !ok {
+					// We have the package, but not the requested release
+					continue
 				}
+			} else {
+				// default to the latest
+				release = metadata.Release
+			}
 
-				// Remove the package specified since it has been resolved.
-				delete(packageMap, key)
+			itemMap[*pkg] = &downloadItem{
+				RepositoryMetadata: metadata,
+				Release:            release,
+				Source:             source,
 			}
+			delete(packageMap, *pkg)
 		}
 	}
 
@@ -154,28 +178,28 @@ func (d *downloader) DownloadRepositories(items []*downloadItem) (manifests []*a
 	for _, item := range items {
 		endpoint := item.Source.Endpoint
 
-		d.logger.Infof("downloading repository: %s from %s", item.RepositoryMetadata, endpoint)
+		d.logger.Infof("downloading repository: %s from %s", item, endpoint)
 
 		options, err := d.regOptionGetter.GetRegistryOptions(item.Source)
 		if err != nil {
 			allErrors = append(allErrors, err)
-			d.logger.Infof("skipping repository: %s", item.RepositoryMetadata)
+			d.logger.Infof("skipping repository: %s", item)
 
 			continue
 		}
 
 		client, err := apprclient.New(*options)
 		if err != nil {
 			allErrors = append(allErrors, err)
-			d.logger.Infof("skipping repository: %s", item.RepositoryMetadata)
+			d.logger.Infof("skipping repository: %s", item)
 
 			continue
 		}
 
-		manifest, err := client.RetrieveOne(item.RepositoryMetadata.ID(), item.RepositoryMetadata.Release)
+		manifest, err := client.RetrieveOne(item.RepositoryMetadata.ID(), item.Release)
 		if err != nil {
 			allErrors = append(allErrors, err)
-			d.logger.Infof("skipping repository: %s", item.RepositoryMetadata)
+			d.logger.Infof("skipping repository: %s", item)
 
 			continue
 		}

pkg/appregistry/downloader_test.go

@@ -25,9 +25,9 @@ var testPrepare = []struct {
 					RegistryNamespace: "",
 				},
 			},
-			Packages: []string{
-				"Kubevirt",
-				"etcd",
+			Packages: []*Package{
+				&Package{"Kubevirt", ""},
+				&Package{"etcd", ""},
 			},
 		},
 		sourceQuerier: &fakeSourceQuerier{
@@ -43,8 +43,9 @@ var testPrepare = []struct {
 		},
 		expectedDownloadItems: []*downloadItem{
 			&downloadItem{
-				&apprclient.RegistryMetadata{Name: "Kubevirt"},
-				&Source{Endpoint: "quay.io", RegistryNamespace: ""},
+				RepositoryMetadata: &apprclient.RegistryMetadata{Name: "Kubevirt"},
+				Source:             &Source{Endpoint: "quay.io", RegistryNamespace: ""},
+				Release:            "",
 			},
 		},
 		expectedError: nil,
@@ -61,9 +62,9 @@ var testPrepare = []struct {
 					RegistryNamespace: "",
 				},
 			},
-			Packages: []string{
-				"Kubevirt",
-				"etcd",
+			Packages: []*Package{
+				&Package{"Kubevirt", ""},
+				&Package{"etcd", ""},
 			},
 		},
 		sourceQuerier: &fakeSourceQuerier{
@@ -84,8 +85,9 @@ var testPrepare = []struct {
 		},
 		expectedDownloadItems: []*downloadItem{
 			&downloadItem{
-				&apprclient.RegistryMetadata{Name: "Kubevirt"},
-				&Source{Endpoint: "quay.io", RegistryNamespace: ""},
+				RepositoryMetadata: &apprclient.RegistryMetadata{Name: "Kubevirt"},
+				Source:             &Source{Endpoint: "quay.io", RegistryNamespace: ""},
+				Release:            "",
 			},
 		},
 		expectedError: utilerrors.NewAggregate(
@@ -94,6 +96,91 @@ var testPrepare = []struct {
 			},
 		),
 	},
+	{
+		input: &Input{
+			Sources: []*Source{
+				{
+					Endpoint:          "quay.io",
+					RegistryNamespace: "",
+				},
+			},
+			Packages: []*Package{
+				&Package{"Kubevirt", "10.0.0"},
+				&Package{"etcd", ""},
+			},
+		},
+		sourceQuerier: &fakeSourceQuerier{
+			map[Source][]*apprclient.RegistryMetadata{
+				Source{
+					Endpoint:          "quay.io",
+					RegistryNamespace: "",
+				}: {
+					&apprclient.RegistryMetadata{
+						Name:     "Kubevirt",
+						Release:  "11.0.0",
+						Releases: []string{"10.0.0", "11.0.0"},
+					},
+					&apprclient.RegistryMetadata{
+						Name:     "etcd",
+						Release:  "2.0.0",
+						Releases: []string{"1.0.0", "2.0.0"},
+					},
+				},
+			},
+			map[Source]error{},
+		},
+		expectedDownloadItems: []*downloadItem{
+			&downloadItem{
+				RepositoryMetadata: &apprclient.RegistryMetadata{
+					Name:     "Kubevirt",
+					Release:  "11.0.0",
+					Releases: []string{"10.0.0", "11.0.0"},
+				},
+				Source:  &Source{Endpoint: "quay.io", RegistryNamespace: ""},
+				Release: "10.0.0",
+			},
+			&downloadItem{
+				RepositoryMetadata: &apprclient.RegistryMetadata{
+					Name:     "etcd",
+					Release:  "2.0.0",
+					Releases: []string{"1.0.0", "2.0.0"},
+				},
+				Source:  &Source{Endpoint: "quay.io", RegistryNamespace: ""},
+				Release: "2.0.0",
+			},
+		},
+		expectedError: nil,
+	},
+	{
+		input: &Input{
+			Sources: []*Source{
+				{
+					Endpoint:          "quay.io",
+					RegistryNamespace: "",
+				},
+			},
+			Packages: []*Package{
+				&Package{"Kubevirt", "10.0.0"},
+			},
+		},
+		sourceQuerier: &fakeSourceQuerier{
+			map[Source][]*apprclient.RegistryMetadata{
+				Source{
+					Endpoint:          "quay.io",
+					RegistryNamespace: "",
+				}: {
+					&apprclient.RegistryMetadata{
+						Name:     "Kubevirt",
+						Release:  "11.0.0",
+						Releases: []string{"11.0.0"},
+					},
+				},
+			},
+			map[Source]error{},
+		},
+		expectedDownloadItems: []*downloadItem{},
+		expectedError:         nil,
+	},
 }
 
 type fakeSourceQuerier struct {
@@ -122,7 +209,9 @@ func TestPrepare(t *testing.T) {
 		}
 
 		downloadItems, err := d.Prepare(testItem.input)
-		assert.Equal(t, testItem.expectedDownloadItems, downloadItems)
+		// Since downloadItems are stored in a map inside of the Prepare
+		// function the order isn't guaranteed
+		assert.ElementsMatch(t, testItem.expectedDownloadItems, downloadItems)
 		if testItem.expectedError != nil {
 			assert.Equal(t, testItem.expectedError, err)
 		} else {

pkg/appregistry/input.go

@@ -1,7 +1,10 @@
 package appregistry
 
 import (
+	"fmt"
 	"strings"
+
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 )
 
 // OperatorSourceSpecifier interface provides capability to have different ways
@@ -17,18 +20,32 @@ type Input struct {
 	Sources []*Source
 
 	// Packages is the set of package name(s) specified.
-	Packages []string
+	Packages []*Package
+}
+
+type Package struct {
+	// The name of the package
+	Name string
+	// The release number of the package
+	Release string
+}
+
+func (p *Package) String() string {
+	if p.Release == "" {
+		return fmt.Sprintf("%s", p.Name)
+	}
+	return fmt.Sprintf("%s:%s", p.Name, p.Release)
 }
 
 func (i *Input) IsGoodToProceed() bool {
 	return len(i.Sources) > 0 && len(i.Packages) > 0
 }
 
-func (i *Input) PackagesToMap() map[string]bool {
-	packages := map[string]bool{}
+func (i *Input) PackagesToMap() map[Package]bool {
+	packages := map[Package]bool{}
 
 	for _, pkg := range i.Packages {
-		packages[pkg] = false
+		packages[*pkg] = false
 	}
 
 	return packages
@@ -56,7 +73,7 @@ func (p *inputParser) Parse(csvSources []string, csvPackages string) (*Input, er
 		return nil, err
 	}
 
-	packages := sanitizePackageList(strings.Split(csvPackages, ","))
+	packages, err := sanitizePackageList(strings.Split(csvPackages, ","))
 
 	return &Input{
 		Sources:  sources,
@@ -66,18 +83,49 @@ func (p *inputParser) Parse(csvSources []string, csvPackages string) (*Input, er
 
 // sanitizePackageList sanitizes the set of package(s) specified. It removes
 // duplicates and ignores empty string.
-func sanitizePackageList(in []string) []string {
-	out := make([]string, 0)
-
+func sanitizePackageList(in []string) ([]*Package, error) {
+	out := make([]*Package, 0)
+	allErrors := []error{}
 	inMap := map[string]bool{}
+
 	for _, item := range in {
-		if _, ok := inMap[item]; ok || item == "" {
+		name, release, err := getNameAndRelease(item)
+		if err != nil {
+			allErrors = append(allErrors, err)
+			continue
+		}
+
+		if _, ok := inMap[name]; ok || name == "" {
 			continue
 		}
 
-		inMap[item] = true
-		out = append(out, item)
+		inMap[name] = true
+		out = append(out, &Package{Name: name, Release: release})
 	}
 
-	return out
+	err := utilerrors.NewAggregate(allErrors)
+	return out, err
+}
+
+func getNameAndRelease(in string) (string, string, error) {
+	inWithoutSpaces := strings.Map(
+		func(r rune) rune {
+			if r == ' ' {
+				return -1
+			}
+			return r
+		},
+		in,
+	)
+	parts := strings.Split(inWithoutSpaces, ":")
+
+	switch len(parts) {
+	case 1:
+		// release wasn't specified
+		return parts[0], "", nil
+	case 2:
+		return parts[0], parts[1], nil
+	default:
+		return "", "", fmt.Errorf("Failed to parse package %s", in)
+	}
 }

pkg/appregistry/input_test.go

@@ -6,11 +6,73 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestDuplicatePackage(t *testing.T) {
+type testInput struct {
+	csvSources       string
+	expectedPackages []*Package
+}
+
+var testGoodInput = []testInput{
+	{
+		"Jaeger,Jaeger",
+		[]*Package{
+			&Package{Name: "Jaeger", Release: ""},
+		},
+	},
+	{
+		"Jaeger,Kubevirt:10.0.0",
+		[]*Package{
+			&Package{Name: "Jaeger", Release: ""},
+			&Package{Name: "Kubevirt", Release: "10.0.0"},
+		},
+	},
+	{
+		"Jaeger,Kubevirt:10.0.0,Kubevirt",
+		[]*Package{
+			&Package{Name: "Jaeger", Release: ""},
+			&Package{Name: "Kubevirt", Release: "10.0.0"},
+		},
+	},
+	{
+		"Jaeger :2.0.0,  Kubevirt: 10.0.0, Kubevirt",
+		[]*Package{
+			&Package{Name: "Jaeger", Release: "2.0.0"},
+			&Package{Name: "Kubevirt", Release: "10.0.0"},
+		},
+	},
+	{
+		"",
+		[]*Package{},
+	},
+}
+
+func TestGoodInput(t *testing.T) {
+	parser := inputParser{sourceSpecifier: &registrySpecifier{}}
+
+	for _, goodInput := range testGoodInput {
+		actual, err := parser.Parse(
+			[]string{"https://quay.io/cnr|community-operator|"},
+			goodInput.csvSources,
+		)
+		assert.NoError(t, err)
+		assert.Equal(t, goodInput.expectedPackages, actual.Packages)
+	}
+}
+
+var testFaultyInput = []testInput{
+	{
+		"Jaeger,Kubevirt:10.0.0:11.0.0",
+		nil,
+	},
+}
+
+func TestFaultyInput(t *testing.T) {
 	parser := inputParser{sourceSpecifier: &registrySpecifier{}}
-	expectedPackages := []string{"Jaeger"}
 
-	actual, err := parser.Parse([]string{"https://quay.io/cnr|community-operator|"}, "Jaeger,Jaeger")
-	assert.NoError(t, err)
-	assert.Equal(t, expectedPackages, actual.Packages)
+	for _, mfInput := range testFaultyInput {
+		_, err := parser.Parse(
+			[]string{"https://quay.io/cnr|community-operator|"},
+			mfInput.csvSources,
+		)
+		assert.Error(t, err)
+	}
 }