refactor(image): add image reference interface

- Add image.Reference interface and SimpleReference implementation as
  parameters of image.Registry methods (in place of strings)
- Refactor image.Registry tests into suite
- Add registry implementation notes
- Remove buildah from builds (Note: should be re-added once it's
  complete)

Author: njhale

.gitignore

@@ -454,5 +454,10 @@ bin
 # Ignore vscode
 .vscode
 
-# Igore apprclient meta
+# Ignore apprclient meta
 pkg/apprclient/openapi/git_push.sh
+
+# Never ignore testdata
+!pkg/**/testdata/**
+!test/**testdata/**
+

go.mod

@@ -3,40 +3,50 @@ module github.com/operator-framework/operator-registry
 go 1.13
 
 require (
+	github.com/Microsoft/hcsshim v0.8.7 // indirect
 	github.com/antihax/optional v0.0.0-20180407024304-ca021399b1a6
 	github.com/blang/semver v3.5.0+incompatible
 	github.com/containerd/containerd v1.3.2
 	github.com/containerd/continuity v0.0.0-20200228182428-0f16d7a0959c // indirect
-	github.com/containers/buildah v1.14.3
 	github.com/docker/cli v0.0.0-20200130152716-5d0cf8839492
 	github.com/docker/distribution v2.7.1+incompatible
 	github.com/docker/docker v1.4.2-0.20200203170920-46ec8731fbce
+	github.com/docker/docker-credential-helpers v0.6.3 // indirect
 	github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect
+	github.com/docker/go-metrics v0.0.1 // indirect
+	github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 // indirect
 	github.com/ghodss/yaml v1.0.0
 	github.com/gogo/protobuf v1.3.1 // indirect
 	github.com/golang-migrate/migrate/v4 v4.6.2
 	github.com/golang/mock v1.3.1
 	github.com/golang/protobuf v1.3.2
+	github.com/google/go-cmp v0.4.0 // indirect
+	github.com/gorilla/mux v1.7.4 // indirect
 	github.com/grpc-ecosystem/grpc-health-probe v0.2.1-0.20181220223928-2bf0a5b182db
+	github.com/imdario/mergo v0.3.8 // indirect
 	github.com/mattn/go-sqlite3 v1.10.0
 	github.com/maxbrunsfeld/counterfeiter/v6 v6.2.2
 	github.com/morikuni/aec v1.0.0 // indirect
 	github.com/onsi/ginkgo v1.12.0
 	github.com/onsi/gomega v1.9.0
 	github.com/opencontainers/image-spec v1.0.2-0.20190823105129-775207bd45b6
+	github.com/opencontainers/runc v1.0.0-rc9 // indirect
+	github.com/opencontainers/runtime-spec v0.1.2-0.20190618234442-a950415649c7 // indirect
 	github.com/operator-framework/api v0.1.1
 	github.com/otiai10/copy v1.0.2
 	github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2
 	github.com/pkg/errors v0.9.1
 	github.com/sirupsen/logrus v1.4.2
 	github.com/spf13/cobra v0.0.6
 	github.com/stretchr/testify v1.5.1
-	go.etcd.io/bbolt v1.3.3
+	go.etcd.io/bbolt v1.3.4
+	golang.org/x/crypto v0.0.0-20200220183623-bac4c82f6975 // indirect
 	golang.org/x/mod v0.2.0
 	golang.org/x/net v0.0.0-20191028085509-fe3aa8a45271
 	golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45
 	golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e // indirect
 	google.golang.org/grpc v1.24.0
+	gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
 	gopkg.in/yaml.v2 v2.2.8
 	k8s.io/api v0.17.3
 	k8s.io/apiextensions-apiserver v0.17.3

go.sum

@@ -1,21 +1,15 @@
+// +build ignore
 package buildahregistry
 
 import (
-	"github.com/containers/storage"
-	"github.com/containers/storage/pkg/idtools"
 	"io/ioutil"
 	"os"
 	"path"
 	"path/filepath"
 	"sync"
 
-	//"sync"
-	//
-	//contentlocal "github.com/containerd/containerd/content/local"
-	//"github.com/containerd/containerd/metadata"
-	//"github.com/containerd/containerd/platforms"
+	"github.com/containers/storage"
 	"github.com/sirupsen/logrus"
-	//bolt "go.etcd.io/bbolt"
 )
 
 type RegistryConfig struct {
@@ -55,53 +49,47 @@ func defaultConfig() *RegistryConfig {
 	return config
 }
 
-func NewRegistry(options ...RegistryOption) (*Registry, error) {
+func NewRegistry(options ...RegistryOption) (registry *Registry, destroy func() error, err error) {
 	config := defaultConfig()
 	config.apply(options)
-	if err := config.complete(); err != nil {
-		return nil, err
+	if err = config.complete(); err != nil {
+		return
 	}
 
-	var (
-		once   sync.Once
-		closed bool
-	)
-	closeFunc := func() error {
-		defer func() {
-			once.Do(func() {
-				closed = true
-			})
-		}()
-		if closed {
-			// Already closed, no-op
-			return nil
-		}
-
-		if config.PreserveCache {
-			return nil
-		}
-		return os.RemoveAll(config.CacheDir)
+	var once sync.Once
+	destroy = func() (destroyErr error) {
+		once.Do(func() {
+			if config.PreserveCache {
+				return
+			}
+
+			destroyErr = os.RemoveAll(config.CacheDir)
+		})
+
+		return
 	}
 
 	// TODO: at this point we've overwritten all the defaults, may as well not use this
-	storeOpts, err := storage.DefaultStoreOptionsAutoDetectUID()
+	var storeOpts storage.StoreOptions
+	storeOpts, err = storage.DefaultStoreOptionsAutoDetectUID()
 	if err != nil {
-		return nil, err
+		return
 	}
 	storeOpts.RootlessStoragePath = config.CacheDir
 	storeOpts.RunRoot = config.CacheDir
 	storeOpts.GraphRoot = config.CacheDir
 	storeOpts.GraphDriverName = "vfs"
-	storeOpts.UIDMap = []idtools.IDMap{
-		{ContainerID: 0, HostID: os.Getuid()},
-	}
-	storeOpts.GIDMap = []idtools.IDMap{
-		{ContainerID: 0, HostID: os.Getgid()},
-	}
-
-	store, err := storage.GetStore(storeOpts)
+	// storeOpts.UIDMap = []idtools.IDMap{
+	// 	{ContainerID: 0, HostID: os.Getuid()},
+	// }
+	// storeOpts.GIDMap = []idtools.IDMap{
+	// 	{ContainerID: 0, HostID: os.Getgid()},
+	// }
+
+	var store storage.Store
+	store, err = storage.GetStore(storeOpts)
 	if err != nil {
-		return nil, err
+		return
 	}
 
 	// TODO: probably don't want the signature policy to be here
@@ -122,13 +110,12 @@ func NewRegistry(options ...RegistryOption) (*Registry, error) {
 }
 `), os.ModePerm)
 
-	r := &Registry{
+	registry = &Registry{
 		Store:    store,
 		CacheDir: config.CacheDir,
 		log:      config.Log,
-		close:    closeFunc,
 	}
-	return r, nil
+	return
 }
 
 type RegistryOption func(config *RegistryConfig)

pkg/image/buildahregistry/options.go renamed to pkg/image/buildahregistry/_options.go

@@ -0,0 +1,64 @@
+// +build ignore
+package buildahregistry
+
+import (
+	"context"
+	"path"
+
+	"github.com/containers/buildah"
+	"github.com/containers/image/v5/types"
+	"github.com/containers/storage"
+	"github.com/sirupsen/logrus"
+
+	"github.com/operator-framework/operator-registry/pkg/image"
+)
+
+// Registry enables manipulation of images via containerd modules.
+// TODO: Finish buildah Registry implementation.
+type Registry struct {
+	storage.Store
+
+	CacheDir string
+	SkipTLS  bool
+
+	log *logrus.Entry
+}
+
+var _ image.Registry = &Registry{}
+
+// Pull fetches and stores an image by reference.
+func (r *Registry) Pull(ctx context.Context, ref image.Reference) error {
+	img, err := buildah.Pull(ctx, ref.String(), buildah.PullOptions{
+		SignaturePolicyPath: path.Join(r.CacheDir, "policy.json"),
+		ReportWriter:        r.log.Writer(),
+		Store:               r.Store,
+		SystemContext: &types.SystemContext{
+			// TODO: auth stuff goes here too
+			// TODO: if we're okay with buildah's cobra args, there's a function to build this from the standard args
+			SignaturePolicyPath:         path.Join(r.CacheDir, "policy.json"),
+			OCIInsecureSkipTLSVerify:    r.SkipTLS,
+			DockerInsecureSkipTLSVerify: types.NewOptionalBool(r.SkipTLS),
+		},
+		BlobDirectory:    r.CacheDir,
+		AllTags:          false,
+		RemoveSignatures: false,
+		MaxRetries:       0,
+		RetryDelay:       0,
+	})
+
+	r.log.Info(img)
+	return err
+}
+
+// Unpack writes the unpackaged content of an image to a directory.
+// If the referenced image does not exist in the registry, an error is returned.
+func (r *Registry) Unpack(ctx context.Context, ref image.Reference, dir string) error {
+	img, err := r.Image(ref.String())
+	if err != nil {
+		return err
+	}
+
+	r.log.Infof("img: %v", img)
+
+	return nil
+}

pkg/image/buildahregistry/_registry.go

@@ -0,0 +1,76 @@
+# Daemonless Notes
+
+## Goal
+
+Drive the following stories:
+
+- [OLM-1666](https://issues.redhat.com/browse/OLM-1666)
+- [OLM-1679](https://issues.redhat.com/browse/OLM-167://issues.redhat.com/browse/OLM-1679)
+
+TL;DR, we want a supportable way to pull and unpack container images:
+
+- in an unprivileged containerized environment (CI constraint)
+- without relying on a external tool (docker, podman, buildah, etc.)
+
+## How does oc unpack images?
+
+The `oc` client needs to pull and unpack images. So we thought it would be useful to see how it achieves this and if we can make use of it.
+
+A quick check of the [extract command](https://github.com/openshift/oc/blob/master/pkg/cli/image/extract/extract.go) reveals several imports relating to images:
+
+```go
+ import (
+    //...
+	"github.com/docker/distribution"
+	dockerarchive "github.com/docker/docker/pkg/archive"
+	digest "github.com/opencontainers/go-digest"
+    //...
+	"github.com/openshift/library-go/pkg/image/dockerv1client"
+	"github.com/openshift/library-go/pkg/image/registryclient"
+	"github.com/openshift/oc/pkg/cli/image/archive"
+	"github.com/openshift/oc/pkg/cli/image/imagesource"
+	imagemanifest "github.com/openshift/oc/pkg/cli/image/manifest"
+	"github.com/openshift/oc/pkg/cli/image/workqueue"
+)
+```
+
+Right off the bat we can see some direct use of `docker` packages.
+
+Also interesting are a set of private [utility functions for modifying tar headers](https://github.com/openshift/oc/blob/e97def4832ab6489a4237633652ac049f09e685b/pkg/cli/image/extract/extract.go#L554) as well as some for [pulling, walking, and unpacking image layers](https://github.com/openshift/oc/blob/e97def4832ab6489a4237633652ac049f09e685b/pkg/cli/image/extract/extract.go#L326).
+
+The `opencontainers` module is imported for go-digest type.
+
+There are also some OpenShift-specific library packages pulled in. The `openshift/library-go` module provides clients for docker and OpenShift registries as well as some helper types for expressing image manifests for each registry. It also contains various types and utilities for parsing image references.
+
+Some layer unpacking functions [are copied directly from docker](https://github.com/openshift/oc/blob/master/pkg/cli/image/archive/archive.go#L82).
+
+`oc` does support [referencing images from non-registry sources](https://github.com/openshift/oc/blob/e97def4832ab6489a4237633652ac049f09e685b/pkg/cli/image/imagesource/options.go#L22) such as `file` and `s3`, but it's questionable whether we need this in `opm`.
+
+`containers/image`, an upstream module for manipulating images, is used in [image signature verification](https://github.com/openshift/oc/blob/9fd38891f0/pkg/cli/admin/verifyimagesignature/verify-signature.go), though I can't find any indication it's being used for signing in `oc`.
+
+## Options
+
+In lieu of importing `oc` or `openshift/library-go` as a dependency, we took a look at several upstream options.
+
+### Containers Packages
+
+- Serious RedHat presence
+- Can be quickly patched by involded RedHatters
+- Interfaces feel fine-grained, require glue
+
+### Buildah Packages
+
+[Buildah](https://github.com/containers/buildah) builds on the core `containers` modules. Pulling seems to require some special `chmod`, and may require more investigation.
+
+### Containerd Packages
+
+- No serious RedHat presence
+- May need staging repo or in-tree patches for CVEs
+- Implements CRI (https://kubernetes.io/docs/setup/production-environment/container-runtimes/#containerd)
+- Widespread use/adoption
+- Relatively simple interfaces
+- Already implemented and working in an [opm branch](https://github.com/njhale/operator-registry/blob/daemonless/pkg/image/containerdregistry/registry.go)
+
+## CRI-O
+
+[CRI-O](https://github.com/cri-o/cri-o/blob/master/server/image_pull.go) needs to pull images and set up container filesystems as well. We haven't looked at this closely yet.