Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mutate: Add() should tell you what it added #344

Merged
merged 3 commits into from
Aug 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/umoci/insert.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func insert(ctx *cli.Context) error {

// TODO: We should add a flag to allow for a new layer to be made
// non-distributable.
if err := mutator.Add(context.Background(), reader, history); err != nil {
if _, err := mutator.Add(context.Background(), reader, history); err != nil {
return errors.Wrap(err, "add diff layer")
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/umoci/raw-add-layer.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func rawAddLayer(ctx *cli.Context) error {

// TODO: We should add a flag to allow for a new layer to be made
// non-distributable.
if err := mutator.Add(context.Background(), newLayer, history); err != nil {
if _, err := mutator.Add(context.Background(), newLayer, history); err != nil {
return errors.Wrap(err, "add diff layer")
}

Expand Down
39 changes: 27 additions & 12 deletions mutate/mutate.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,17 @@ func (m *Mutator) Config(ctx context.Context) (ispec.ImageConfig, error) {
return m.config.Config, nil
}

// Manifest returns the current (cached) image manifest. This is what will be
// appended to when any additional Add() calls are made, and what will be
// Commit()ed if no further changes are made.
func (m *Mutator) Manifest(ctx context.Context) (ispec.Manifest, error) {
if err := m.cache(ctx); err != nil {
return ispec.Manifest{}, errors.Wrap(err, "getting cache failed")
}

return *m.manifest, nil
}

// Meta returns the current (cached) image metadata, which should be used as
// the source for any modifications of the configuration using Set.
func (m *Mutator) Meta(ctx context.Context) (Meta, error) {
Expand Down Expand Up @@ -280,46 +291,50 @@ func (m *Mutator) add(ctx context.Context, reader io.Reader, history *ispec.Hist
// generate the DiffIDs for the image metatadata. The provided history entry is
// appended to the image's history and should correspond to what operations
// were made to the configuration.
func (m *Mutator) Add(ctx context.Context, r io.Reader, history *ispec.History) error {
func (m *Mutator) Add(ctx context.Context, r io.Reader, history *ispec.History) (ispec.Descriptor, error) {
desc := ispec.Descriptor{}
if err := m.cache(ctx); err != nil {
return errors.Wrap(err, "getting cache failed")
return desc, errors.Wrap(err, "getting cache failed")
}

digest, size, err := m.add(ctx, r, history)
if err != nil {
return errors.Wrap(err, "add layer")
return desc, errors.Wrap(err, "add layer")
}

// Append to layers.
m.manifest.Layers = append(m.manifest.Layers, ispec.Descriptor{
desc = ispec.Descriptor{
// TODO: Detect whether the layer is gzip'd or not...
MediaType: ispec.MediaTypeImageLayerGzip,
Digest: digest,
Size: size,
})
return nil
}
m.manifest.Layers = append(m.manifest.Layers, desc)
return desc, nil
}

// AddNonDistributable is the same as Add, except it adds a non-distributable
tych0 marked this conversation as resolved.
Show resolved Hide resolved
// layer to the image.
func (m *Mutator) AddNonDistributable(ctx context.Context, r io.Reader, history *ispec.History) error {
func (m *Mutator) AddNonDistributable(ctx context.Context, r io.Reader, history *ispec.History) (ispec.Descriptor, error) {
desc := ispec.Descriptor{}
if err := m.cache(ctx); err != nil {
return errors.Wrap(err, "getting cache failed")
return desc, errors.Wrap(err, "getting cache failed")
}

digest, size, err := m.add(ctx, r, history)
if err != nil {
return errors.Wrap(err, "add non-distributable layer")
return desc, errors.Wrap(err, "add non-distributable layer")
}

// Append to layers.
m.manifest.Layers = append(m.manifest.Layers, ispec.Descriptor{
desc = ispec.Descriptor{
// TODO: Detect whether the layer is gzip'd or not...
MediaType: ispec.MediaTypeImageLayerNonDistributableGzip,
Digest: digest,
Size: size,
})
return nil
}
m.manifest.Layers = append(m.manifest.Layers, desc)
return desc, nil
}

// Commit writes all of the temporary changes made to the configuration,
Expand Down
26 changes: 22 additions & 4 deletions mutate/mutate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,10 @@ func TestMutateAdd(t *testing.T) {
buffer := bytes.NewBufferString("contents")

// Add a new layer.
if err := mutator.Add(context.Background(), buffer, &ispec.History{
newLayerDesc, err := mutator.Add(context.Background(), buffer, &ispec.History{
Comment: "new layer",
}); err != nil {
})
if err != nil {
t.Fatalf("unexpected error adding layer: %+v", err)
}

Expand Down Expand Up @@ -251,6 +252,19 @@ func TestMutateAdd(t *testing.T) {
t.Errorf("manifest.Layers[1].Digest is not the same!")
}

if mutator.manifest.Layers[1].Digest != newLayerDesc.Digest {
t.Fatalf("unexpected digest for new layer: %v %v", mutator.manifest.Layers[1].Digest, newLayerDesc.Digest)
}

manifestFromFunction, err := mutator.Manifest(context.Background())
if err != nil {
t.Fatalf("unexpected error getting manifest: %+v", err)
}

if !reflect.DeepEqual(manifestFromFunction, *mutator.manifest) {
t.Fatalf("mutator.Manifest() didn't return the cached manifest")
}

// Check layer was added.
if len(mutator.manifest.Layers) != 2 {
t.Errorf("manifest.Layers was not updated")
Expand Down Expand Up @@ -295,9 +309,10 @@ func TestMutateAddNonDistributable(t *testing.T) {
buffer := bytes.NewBufferString("contents")

// Add a new layer.
if err := mutator.AddNonDistributable(context.Background(), buffer, &ispec.History{
newLayerDesc, err := mutator.AddNonDistributable(context.Background(), buffer, &ispec.History{
Comment: "new layer",
}); err != nil {
})
if err != nil {
t.Fatalf("unexpected error adding layer: %+v", err)
}

Expand Down Expand Up @@ -330,6 +345,9 @@ func TestMutateAddNonDistributable(t *testing.T) {
if mutator.manifest.Layers[1].Digest == expectedLayerDigest {
t.Errorf("manifest.Layers[1].Digest is not the same!")
}
if mutator.manifest.Layers[1].Digest != newLayerDesc.Digest {
t.Fatalf("unexpected digest for new layer: %v %v", mutator.manifest.Layers[1].Digest, newLayerDesc.Digest)
}

// Check layer was added.
if len(mutator.manifest.Layers) != 2 {
Expand Down
2 changes: 1 addition & 1 deletion repack.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func Repack(engineExt casext.Engine, tagName string, bundlePath string, meta Met

// TODO: We should add a flag to allow for a new layer to be made
// non-distributable.
if err := mutator.Add(context.Background(), reader, history); err != nil {
if _, err := mutator.Add(context.Background(), reader, history); err != nil {
return errors.Wrap(err, "add diff layer")
}
}
Expand Down