From 8ddfcf6902c4dd0e1be9ae7c78abf5a738be6064 Mon Sep 17 00:00:00 2001 From: Michelle Noorali Date: Mon, 1 Apr 2019 13:38:06 -0400 Subject: [PATCH] feat(*): add digest to invocation image on build resolves #690 --- cmd/duffle/build.go | 2 +- cmd/duffle/export.go | 1 + pkg/builder/builder.go | 53 +++------- pkg/builder/builder_test.go | 99 ++++++++++--------- pkg/imagebuilder/docker/builder.go | 39 ++++---- pkg/imagebuilder/docker/digester/digester.go | 39 ++++++++ .../docker/digester/digester_test.go | 83 ++++++++++++++++ pkg/imagebuilder/imagebuilder.go | 3 +- pkg/imagebuilder/mock/builder.go | 9 +- 9 files changed, 213 insertions(+), 115 deletions(-) create mode 100644 pkg/imagebuilder/docker/digester/digester.go create mode 100644 pkg/imagebuilder/docker/digester/digester_test.go diff --git a/cmd/duffle/build.go b/cmd/duffle/build.go index 39f53d01..3d593dd4 100644 --- a/cmd/duffle/build.go +++ b/cmd/duffle/build.go @@ -133,7 +133,7 @@ func (b *buildCmd) run() (err error) { return fmt.Errorf("cannot prepare build: %v", err) } - if err := bldr.Build(ctx, app); err != nil { + if err := bldr.Build(ctx, app, bf); err != nil { return err } diff --git a/cmd/duffle/export.go b/cmd/duffle/export.go index d4032cf3..7f6ee66f 100644 --- a/cmd/duffle/export.go +++ b/cmd/duffle/export.go @@ -40,6 +40,7 @@ type exportCmd struct { verbose bool insecure bool bundleIsFile bool + signer string } func newExportCmd(w io.Writer) *cobra.Command { diff --git a/pkg/builder/builder.go b/pkg/builder/builder.go index ff4710cc..a0a5c425 100644 --- a/pkg/builder/builder.go +++ b/pkg/builder/builder.go @@ -7,8 +7,6 @@ import ( "os" "path/filepath" "strings" - "sync" - "time" "github.com/Masterminds/semver" "github.com/pkg/errors" @@ -87,11 +85,6 @@ func (b *Builder) PrepareBuild(bldr *Builder, mfst *manifest.Manifest, appDir st return nil, nil, err } - ii := bundle.InvocationImage{} - ii.Image = imb.URI() - ii.ImageType = imb.Type() - bf.InvocationImages = append(bf.InvocationImages, ii) - baseVersion := mfst.Version if baseVersion == "" { baseVersion = "0.1.0" @@ -131,45 +124,27 @@ func (b *Builder) version(baseVersion, sha string) (string, error) { } // Build passes the context of each component to its respective builder -func (b *Builder) Build(ctx context.Context, app *AppContext) error { - if err := buildInvocationImages(ctx, b.ImageBuilders, app); err != nil { +func (b *Builder) Build(ctx context.Context, app *AppContext, bf *bundle.Bundle) error { + if err := b.buildInvocationImages(ctx, app, bf); err != nil { return fmt.Errorf("error building image: %v", err) } return nil } -func buildInvocationImages(ctx context.Context, imageBuilders []imagebuilder.ImageBuilder, app *AppContext) (err error) { - errc := make(chan error) - - go func() { - defer close(errc) - var wg sync.WaitGroup - wg.Add(len(imageBuilders)) - - for _, c := range imageBuilders { - go func(c imagebuilder.ImageBuilder) { - defer wg.Done() - err = c.Build(ctx, app.Log) - if err != nil { - errc <- fmt.Errorf("error building image %v: %v", c.Name(), err) - } - }(c) - } - - wg.Wait() - }() - - for errc != nil { - select { - case err, ok := <-errc: - if !ok { - errc = nil - continue - } +func (b *Builder) buildInvocationImages(ctx context.Context, app *AppContext, bf *bundle.Bundle) (err error) { + built := []bundle.InvocationImage{} + for _, c := range b.ImageBuilders { + digest, err := c.Build(ctx, app.Log) + if err != nil { return err - default: - time.Sleep(time.Second) } + ii := bundle.InvocationImage{} + ii.Image = c.URI() + ii.ImageType = c.Type() + ii.Digest = digest + built = append(built, ii) } + bf.InvocationImages = built + return nil } diff --git a/pkg/builder/builder_test.go b/pkg/builder/builder_test.go index 421b5a4d..5f9f7076 100644 --- a/pkg/builder/builder_test.go +++ b/pkg/builder/builder_test.go @@ -3,7 +3,6 @@ package builder import ( "context" "io" - "reflect" "testing" "github.com/deislabs/duffle/pkg/bundle" @@ -11,44 +10,6 @@ import ( "github.com/deislabs/duffle/pkg/imagebuilder" ) -// testImage represents a mock invocation image -type testImage struct { - Nam string - Typ string - UR string - Diges string -} - -// Name represents the name of a mock invocation image -func (tc testImage) Name() string { - return tc.Nam -} - -// Type represents the type of a mock invocation image -func (tc testImage) Type() string { - return tc.Typ -} - -// URI represents the URI of the artefact of a mock invocation image -func (tc testImage) URI() string { - return tc.UR -} - -// Digest represents the digest of a mock invocation image -func (tc testImage) Digest() string { - return tc.Diges -} - -// PrepareBuild is no-op for a mock invocation image -func (tc *testImage) PrepareBuild(appDir, registry, name string) error { - return nil -} - -// Build is no-op for a mock invocation image -func (tc testImage) Build(ctx context.Context, log io.WriteCloser) error { - return nil -} - func TestPrepareBuild(t *testing.T) { mfst := &manifest.Manifest{ Name: "foo", @@ -85,14 +46,58 @@ func TestPrepareBuild(t *testing.T) { t.Error(err) } - if len(b.InvocationImages) != 1 { - t.Fatalf("expected there to be 1 image, got %d. Full output: %v", len(b.Images), b) + if len(b.InvocationImages) != 0 { + t.Errorf("Expected 0 invocation images (no info set until images are built), got %v", len(b.InvocationImages)) } - - expected := bundle.InvocationImage{} - expected.Image = "cnab:0.1.0" - expected.ImageType = "docker" - if !reflect.DeepEqual(b.InvocationImages[0], expected) { - t.Errorf("expected %v, got %v", expected, b.InvocationImages[0]) + if len(bldr.ImageBuilders) != 1 { + t.Fatalf("Expected 1 image builder to be set, got %v", len(bldr.ImageBuilders)) + } + ib := bldr.ImageBuilders[0] + if ib.Name() != "cnab" { + t.Errorf("Expected name of invocation image to be cnab, got %s", ib.Name()) } + if ib.Type() != "docker" { + t.Errorf("Expected type of invocation image to be docker, got %s", ib.Type()) + } + if ib.URI() != "cnab:0.1.0" { + t.Errorf("Expected URI of invocation image to be cnab:0.1.0, got %s", ib.URI()) + } +} + +// testImage represents a mock invocation image +type testImage struct { + Nam string + Typ string + UR string + Diges string +} + +// Name represents the name of a mock invocation image +func (tc testImage) Name() string { + return tc.Nam +} + +// Type represents the type of a mock invocation image +func (tc testImage) Type() string { + return tc.Typ +} + +// URI represents the URI of the artefact of a mock invocation image +func (tc testImage) URI() string { + return tc.UR +} + +// Digest represents the digest of a mock invocation image +func (tc testImage) Digest() string { + return tc.Diges +} + +// PrepareBuild is no-op for a mock invocation image +func (tc *testImage) PrepareBuild(appDir, registry, name string) error { + return nil +} + +// Build is no-op for a mock invocation image +func (tc testImage) Build(ctx context.Context, log io.WriteCloser) (string, error) { + return "", nil } diff --git a/pkg/imagebuilder/docker/builder.go b/pkg/imagebuilder/docker/builder.go index f0fc5e69..6de55c02 100644 --- a/pkg/imagebuilder/docker/builder.go +++ b/pkg/imagebuilder/docker/builder.go @@ -9,9 +9,6 @@ import ( "os" "path" "path/filepath" - "strings" - - "github.com/deislabs/duffle/pkg/duffle/manifest" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/image/build" @@ -22,10 +19,11 @@ import ( "github.com/docker/docker/pkg/fileutils" "github.com/docker/docker/pkg/jsonmessage" "github.com/docker/docker/pkg/term" - "github.com/sirupsen/logrus" - "golang.org/x/net/context" + + "github.com/deislabs/duffle/pkg/duffle/manifest" + "github.com/deislabs/duffle/pkg/imagebuilder/docker/digester" ) const ( @@ -63,13 +61,6 @@ func (db Builder) URI() string { return db.Image } -// Digest returns the name of a Docker Builder, which will give the image name -// -// TODO - return the actual digest -func (db Builder) Digest() string { - return strings.Split(db.Image, ":")[1] -} - // NewBuilder returns a new Docker builder based on the manifest func NewBuilder(c *manifest.InvocationImage, cli *command.DockerCli) *Builder { return &Builder{ @@ -109,7 +100,7 @@ func (db *Builder) PrepareBuild(appDir, registry, name string) error { } // Build builds the docker images. -func (db Builder) Build(ctx context.Context, log io.WriteCloser) error { +func (db *Builder) Build(ctx context.Context, log io.WriteCloser) (string, error) { defer db.BuildContext.Close() buildOpts := types.ImageBuildOptions{ Tags: []string{db.Image}, @@ -118,23 +109,33 @@ func (db Builder) Build(ctx context.Context, log io.WriteCloser) error { resp, err := db.dockerBuilder.DockerClient.Client().ImageBuild(ctx, db.BuildContext, buildOpts) if err != nil { - return fmt.Errorf("error building image builder %v with builder %v: %v", db.Name(), db.Type(), err) + return "", fmt.Errorf("error building image builder %v with builder %v: %v", db.Name(), db.Type(), err) } - defer resp.Body.Close() + outFd, isTerm := term.GetFdInfo(db.BuildContext) if err := jsonmessage.DisplayJSONMessagesStream(resp.Body, log, outFd, isTerm, nil); err != nil { - return fmt.Errorf("error streaming messages for image builder %v with builder %v: %v", db.Name(), db.Type(), err) + return "", fmt.Errorf("error streaming messages for image builder %v with builder %v: %v", db.Name(), db.Type(), err) } if _, _, err = db.dockerBuilder.DockerClient.Client().ImageInspectWithRaw(ctx, db.Image); err != nil { if dockerclient.IsErrNotFound(err) { - return fmt.Errorf("could not locate image for %s: %v", db.Name(), err) + return "", fmt.Errorf("could not locate image for %s: %v", db.Name(), err) } - return fmt.Errorf("imageInspectWithRaw error for image builder %v: %v", db.Name(), err) + return "", fmt.Errorf("imageInspectWithRaw error for image builder %v: %v", db.Name(), err) } - return nil + d := digester.NewDigester( + db.dockerBuilder.DockerClient.Client(), + db.Image, + ctx, + ) + digestStr, err := d.Digest() + if err != nil { + return "", fmt.Errorf("Failed to calculate digest for image: %s", err) + } + + return digestStr, nil } func archiveSrc(contextPath string, b *Builder) error { diff --git a/pkg/imagebuilder/docker/digester/digester.go b/pkg/imagebuilder/docker/digester/digester.go new file mode 100644 index 00000000..372052bc --- /dev/null +++ b/pkg/imagebuilder/docker/digester/digester.go @@ -0,0 +1,39 @@ +package digester + +import ( + "golang.org/x/net/context" + + "github.com/docker/docker/client" + "github.com/opencontainers/go-digest" +) + +type Digester struct { + Client client.ImageAPIClient + Image string + Context context.Context +} + +// NewDigester returns a Digester given the args client, image, ctx +// +// client allows us to talk to the Docker client +// image is a string identifier of the image we want to compute digest of +// ctx is context to use and pass to Docker client +func NewDigester(client client.ImageAPIClient, image string, ctx context.Context) *Digester { + return &Digester{ + Client: client, + Image: image, + Context: ctx, + } +} + +// Digest returns the digest of the image tar +func (d *Digester) Digest() (string, error) { + reader, err := d.Client.ImageSave(d.Context, []string{d.Image}) + computedDigest, err := digest.Canonical.FromReader(reader) + if err != nil { + return "", err + } + defer reader.Close() + + return computedDigest.String(), nil +} diff --git a/pkg/imagebuilder/docker/digester/digester_test.go b/pkg/imagebuilder/docker/digester/digester_test.go new file mode 100644 index 00000000..7b086ebb --- /dev/null +++ b/pkg/imagebuilder/docker/digester/digester_test.go @@ -0,0 +1,83 @@ +package digester + +import ( + "bytes" + "context" + "io" + "io/ioutil" + "testing" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/api/types/image" + "github.com/docker/docker/api/types/registry" +) + +func TestDigest(t *testing.T) { + d := Digester{ + Client: &mockDockerClient{}, + Image: "mock-image", + } + digestStr, err := d.Digest() + if err != nil { + t.Errorf("Not expecting error computing digest but got error: %s", err) + } + expectedDigestStr := "sha256:5dffd8ab8b1b8db6fbb2a62f5059d930fe79f3e3d7b2e65d331af5b8de03c93c" + if digestStr != expectedDigestStr { + t.Errorf("Expected digest %s, got %s", expectedDigestStr, digestStr) + } +} + +type mockDockerClient struct{} + +func (m *mockDockerClient) ImageSave(ctx context.Context, imageIDs []string) (io.ReadCloser, error) { + return ioutil.NopCloser(bytes.NewReader([]byte("mock-context-for-digest"))), nil +} + +func (m *mockDockerClient) ImageBuild(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) { + return types.ImageBuildResponse{}, nil +} +func (m *mockDockerClient) BuildCachePrune(ctx context.Context, opts types.BuildCachePruneOptions) (*types.BuildCachePruneReport, error) { + return nil, nil +} +func (m *mockDockerClient) BuildCancel(ctx context.Context, id string) error { + return nil +} + +func (m *mockDockerClient) ImageCreate(ctx context.Context, parentReference string, options types.ImageCreateOptions) (io.ReadCloser, error) { + return nil, nil +} + +func (m *mockDockerClient) ImageHistory(ctx context.Context, image string) ([]image.HistoryResponseItem, error) { + return nil, nil +} +func (m *mockDockerClient) ImageImport(ctx context.Context, source types.ImageImportSource, ref string, options types.ImageImportOptions) (io.ReadCloser, error) { + return nil, nil +} +func (m *mockDockerClient) ImageInspectWithRaw(ctx context.Context, image string) (types.ImageInspect, []byte, error) { + return types.ImageInspect{}, nil, nil +} +func (m *mockDockerClient) ImageList(ctx context.Context, options types.ImageListOptions) ([]types.ImageSummary, error) { + return nil, nil +} +func (m *mockDockerClient) ImageLoad(ctx context.Context, input io.Reader, quiet bool) (types.ImageLoadResponse, error) { + return types.ImageLoadResponse{}, nil +} +func (m *mockDockerClient) ImagePull(ctx context.Context, ref string, options types.ImagePullOptions) (io.ReadCloser, error) { + return nil, nil +} +func (m *mockDockerClient) ImagePush(ctx context.Context, ref string, options types.ImagePushOptions) (io.ReadCloser, error) { + return nil, nil +} +func (m *mockDockerClient) ImageRemove(ctx context.Context, image string, options types.ImageRemoveOptions) ([]types.ImageDeleteResponseItem, error) { + return nil, nil +} +func (m *mockDockerClient) ImageSearch(ctx context.Context, term string, options types.ImageSearchOptions) ([]registry.SearchResult, error) { + return nil, nil +} +func (m *mockDockerClient) ImageTag(ctx context.Context, image, ref string) error { + return nil +} +func (m *mockDockerClient) ImagesPrune(ctx context.Context, pruneFilter filters.Args) (types.ImagesPruneReport, error) { + return types.ImagesPruneReport{}, nil +} diff --git a/pkg/imagebuilder/imagebuilder.go b/pkg/imagebuilder/imagebuilder.go index 56280b25..9d318547 100644 --- a/pkg/imagebuilder/imagebuilder.go +++ b/pkg/imagebuilder/imagebuilder.go @@ -10,8 +10,7 @@ type ImageBuilder interface { Name() string Type() string URI() string - Digest() string PrepareBuild(string, string, string) error - Build(context.Context, io.WriteCloser) error + Build(context.Context, io.WriteCloser) (string, error) } diff --git a/pkg/imagebuilder/mock/builder.go b/pkg/imagebuilder/mock/builder.go index 310bce27..7c16bee8 100644 --- a/pkg/imagebuilder/mock/builder.go +++ b/pkg/imagebuilder/mock/builder.go @@ -26,11 +26,6 @@ func (b Builder) URI() string { return "mock-uri:1.0.0" } -// Digest represents the digest of a mock builder -func (b Builder) Digest() string { - return "mock-digest" -} - // NewBuilder returns a new mock builder func NewBuilder(c *manifest.InvocationImage) *Builder { return &Builder{} @@ -42,6 +37,6 @@ func (b *Builder) PrepareBuild(appDir, registry, name string) error { } // Build is no-op for a mock builder -func (b Builder) Build(ctx context.Context, log io.WriteCloser) error { - return nil +func (b Builder) Build(ctx context.Context, log io.WriteCloser) (string, error) { + return "mock-digest", nil }