fix(appr): use given cache dir for image builds

Author: njhale

pkg/apprclient/apprclientfakes/fake_client.go

@@ -1,3 +1,4 @@
+//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Client
 package apprclient
 
 import (

pkg/apprclient/client.go

@@ -1,3 +1,4 @@
+//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 builder.go ImageAppender
 package appregistry
 
 import (
@@ -46,6 +47,7 @@ type AppregistryImageBuilder struct {
 	CleanOutput bool
 	ManifestDir string
 	DatabaseDir string
+	client      apprclient.Client
 }
 
 func NewAppregistryImageBuilder(options ...AppregistryBuildOption) (*AppregistryImageBuilder, error) {
@@ -69,14 +71,11 @@ func NewAppregistryImageBuilder(options ...AppregistryBuildOption) (*Appregistry
 		CleanOutput:         config.CleanOutput,
 		ManifestDir:         config.ManifestDir,
 		DatabaseDir:         config.DatabaseDir,
+		client:              config.Client,
 	}, nil
 }
 
 func (b *AppregistryImageBuilder) Build() error {
-	opts := apprclient.Options{Source: b.AppRegistryEndpoint}
-	if b.AuthToken != "" {
-		opts.AuthToken = b.AuthToken
-	}
 
 	defer func() {
 		if !b.CleanOutput {
@@ -87,12 +86,7 @@ func (b *AppregistryImageBuilder) Build() error {
 		}
 	}()
 
-	client, err := apprclient.New(opts)
-	if err != nil {
-		return err
-	}
-
-	downloader := NewManifestDownloader(client)
+	downloader := NewManifestDownloader(b.client)
 	if err := downloader.DownloadManifests(b.ManifestDir, b.AppRegistryOrg); err != nil {
 		return err
 	}

pkg/appregistry/builder.go

@@ -3,7 +3,10 @@ package appregistry
 import (
 	"fmt"
 	"io/ioutil"
+	"os"
 	"path"
+
+	"github.com/operator-framework/operator-registry/pkg/apprclient"
 )
 
 type AppregistryBuildOptions struct {
@@ -20,6 +23,8 @@ type AppregistryBuildOptions struct {
 	CleanOutput bool
 	ManifestDir string
 	DatabaseDir string
+
+	Client apprclient.Client
 }
 
 func (o *AppregistryBuildOptions) Validate() error {
@@ -46,18 +51,31 @@ func (o *AppregistryBuildOptions) Validate() error {
 	if o.DatabaseDir == "" {
 		return fmt.Errorf("local database directory required")
 	}
+	if o.Client == nil {
+		return fmt.Errorf("app-registry client must not be nil")
+	}
 
 	return nil
 }
 
 func (o *AppregistryBuildOptions) Complete() error {
-	// if a user has specified a specific cache dir, don't clean it after run
-	o.CleanOutput = o.CacheDir == ""
+	// if a user hasn't specified a specific cache directory, generate a temporary one
+	if o.CacheDir == "" {
+		tmp, err := ioutil.TempDir("", "cache-")
+		if err != nil {
+			return err
+		}
+		o.CacheDir = tmp
 
-	// build a separate path for manifests and the built database, so that
-	// building is idempotent
+		// clean up temporary directories
+		o.CleanOutput = true
+	} else if err := os.MkdirAll(o.CacheDir, os.ModePerm); err != nil && !os.IsExist(err) {
+		return err
+	}
+
+	// build a separate path for manifests and the built database, so that building is idempotent
 	if o.ManifestDir == "" {
-		manifestDir, err := ioutil.TempDir("", "manifests-")
+		manifestDir, err := ioutil.TempDir(o.CacheDir, "manifests-")
 		if err != nil {
 			return err
 		}
@@ -75,6 +93,22 @@ func (o *AppregistryBuildOptions) Complete() error {
 	if o.DatabasePath == "" {
 		o.DatabasePath = path.Join(o.DatabaseDir, "bundles.db")
 	}
+
+	// create the client
+	if o.Client == nil {
+		opts := apprclient.Options{Source: o.AppRegistryEndpoint}
+		if o.AuthToken != "" {
+			opts.AuthToken = o.AuthToken
+		}
+
+		client, err := apprclient.New(opts)
+		if err != nil {
+			return err
+		}
+
+		o.Client = client
+	}
+
 	return nil
 }
 
@@ -88,7 +122,7 @@ func (c *AppregistryBuildOptions) Apply(options []AppregistryBuildOption) {
 // ToOption converts an AppregistryBuildOptions object into a function that applies
 // its current configuration to another AppregistryBuildOptions instance
 func (c *AppregistryBuildOptions) ToOption() AppregistryBuildOption {
-	return  func(o *AppregistryBuildOptions) {
+	return func(o *AppregistryBuildOptions) {
 		if c.Appender != nil {
 			o.Appender = c.Appender
 		}
@@ -116,6 +150,9 @@ func (c *AppregistryBuildOptions) ToOption() AppregistryBuildOption {
 		if c.DatabaseDir != "" {
 			o.DatabaseDir = c.DatabaseDir
 		}
+		if c.Client != nil {
+			o.Client = c.Client
+		}
 	}
 }
 
@@ -174,3 +211,9 @@ func WithCacheDir(s string) AppregistryBuildOption {
 		o.CacheDir = s
 	}
 }
+
+func WithClient(c apprclient.Client) AppregistryBuildOption {
+	return func(o *AppregistryBuildOptions) {
+		o.Client = c
+	}
+}

pkg/appregistry/builder_options.go

@@ -0,0 +1,82 @@
+package appregistry
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"github.com/operator-framework/operator-registry/pkg/apprclient"
+	"github.com/operator-framework/operator-registry/pkg/apprclient/apprclientfakes"
+)
+
+var _ = Describe("Image building", func() {
+	var (
+		options  []AppregistryBuildOption
+		builder  *AppregistryImageBuilder
+		pkg      *apprclient.RegistryMetadata
+		operator *apprclient.OperatorMetadata
+	)
+
+	JustBeforeEach(func() {
+		var err error
+		builder, err = NewAppregistryImageBuilder(options...)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(builder).ToNot(BeNil())
+		Expect(builder.Build()).To(Succeed())
+	})
+
+	BeforeEach(func() {
+		var noopAppender ImageAppendFunc = func(from, to, layer string) error {
+			return nil
+		}
+
+		client := &apprclientfakes.FakeClient{}
+		pkg = &apprclient.RegistryMetadata{
+			Namespace: "marsupials",
+			Name:      "koala",
+		}
+		client.ListPackagesReturns([]*apprclient.RegistryMetadata{pkg}, nil)
+
+		pkgBlob, err := ioutil.ReadFile("testdata/golden/marsupials_pkg.tar")
+		Expect(err).ToNot(HaveOccurred())
+
+		operator = &apprclient.OperatorMetadata{
+			RegistryMetadata: *pkg,
+			Blob:             pkgBlob,
+		}
+		client.RetrieveOneReturns(operator, nil)
+
+		options = append(options,
+			WithAppender(noopAppender),
+			WithAppRegistryOrg("metatheria"),
+			WithClient(client),
+		)
+	})
+
+	Context("with a custom cache dir", func() {
+		var (
+			cacheDir      string
+			manifestsGlob string
+		)
+
+		BeforeEach(func() {
+			cacheDir = filepath.Join("testdata", "custom-cache")
+			options = append(options, WithCacheDir(cacheDir))
+			manifestsGlob = filepath.Join(cacheDir, "manifests-*/koala/marsupials")
+		})
+
+		AfterEach(func() {
+			Expect(os.RemoveAll(cacheDir)).To(Succeed())
+		})
+
+		It("should retain unpacked operator manifests", func() {
+			Expect(cacheDir).To(BeADirectory())
+			Expect(filepath.Glob(filepath.Join(manifestsGlob, "package.yaml"))).To(HaveLen(1))
+			Expect(filepath.Glob(filepath.Join(manifestsGlob, "v1.0.0", "koala.v1.0.0.clusterserviceversion.yaml"))).To(HaveLen(1))
+		})
+
+	})
+})