merge branch 'add-unit-tests'

This includes a bunch of unit tests for all of the core image
components. The plan is to implement integration tests to make this all
much nicer. But at least this code checks that we don't do anything
stupid internally.

Signed-off-by: Aleksa Sarai <asarai@suse.com>

Makefile

@@ -42,20 +42,17 @@ install-deps:
 
 EPOCH_COMMIT ?= 97ecdbd53dcb72b7a0d62196df281f131dc9eb2f
 validate:
-	@echo "go-fmt"
-	@test -z "$$(gofmt -s -l . | grep -v '^vendor/' | grep -v '^third_party/' | tee /dev/stderr)"
-	@echo "go-lint"
-	@out="$$(golint $(PROJECT)/... | grep -v '/vendor/' | grep -v '/third_party/' | grep -vE 'system/utils_linux.*ALL_CAPS|system/mknod_linux.*underscores')"; \
+	test -z "$$(gofmt -s -l . | grep -v '^vendor/' | grep -v '^third_party/' | tee /dev/stderr)"
+	out="$$(golint $(PROJECT)/... | grep -v '/vendor/' | grep -v '/third_party/' | grep -vE 'system/utils_linux.*ALL_CAPS|system/mknod_linux.*underscores')"; \
 	if [ -n "$$out" ]; then \
 		echo "$$out"; \
 		exit 1; \
 	fi
-	@echo "go-vet"
-	@go vet $(shell go list $(PROJECT)/... | grep -v /vendor/ | grep -v /third_party/)
+	go vet $(shell go list $(PROJECT)/... | grep -v /vendor/ | grep -v /third_party/)
 	#@echo "git-validation"
 	#@git-validation -v -run DCO,short-subject,dangling-whitespace $(EPOCH_COMMIT)..HEAD
 
 test:
-	go test $(PROJECT)/...
+	go test -v $(PROJECT)/...
 
 ci: umoci validate test

image/cas/cas_test.go

@@ -0,0 +1,299 @@
+/*
+ * umoci: Umoci Modifies Open Containers' Images
+ * Copyright (C) 2016 SUSE LLC.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package cas
+
+import (
+	"bytes"
+	"crypto/sha256"
+	"encoding/json"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"reflect"
+	"testing"
+
+	"github.com/opencontainers/image-spec/specs-go/v1"
+	"golang.org/x/net/context"
+)
+
+// NOTE: These tests aren't really testing OCI-style manifests. It's all just
+//       example structures to make sure that the CAS acts properly.
+
+func TestCreateLayout(t *testing.T) {
+	ctx := context.Background()
+
+	root, err := ioutil.TempDir("", "umoci-TestCreateLayout")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(root)
+
+	image := filepath.Join(root, "image")
+	if err := CreateLayout(image); err != nil {
+		t.Fatalf("unexpected error creating image: %s", err)
+	}
+
+	engine, err := Open(image)
+	if err != nil {
+		t.Fatalf("unexpected error opening image: %s", err)
+	}
+	defer engine.Close()
+
+	// We should have no references or blobs.
+	if refs, err := engine.ListReferences(ctx); err != nil {
+		t.Errorf("unexpected error getting list of references: %s", err)
+	} else if len(refs) > 0 {
+		t.Errorf("got references in a newly created image: %v", refs)
+	}
+	if blobs, err := engine.ListBlobs(ctx); err != nil {
+		t.Errorf("unexpected error getting list of blobs: %s", err)
+	} else if len(blobs) > 0 {
+		t.Errorf("got blobs in a newly created image: %v", blobs)
+	}
+}
+
+func TestEngineBlob(t *testing.T) {
+	ctx := context.Background()
+
+	root, err := ioutil.TempDir("", "umoci-TestEngineBlob")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(root)
+
+	image := filepath.Join(root, "image")
+	if err := CreateLayout(image); err != nil {
+		t.Fatalf("unexpected error creating image: %s", err)
+	}
+
+	engine, err := Open(image)
+	if err != nil {
+		t.Fatalf("unexpected error opening image: %s", err)
+	}
+	defer engine.Close()
+
+	for _, test := range []struct {
+		bytes []byte
+	}{
+		{[]byte("")},
+		{[]byte("some blob")},
+		{[]byte("another blob")},
+	} {
+		hash := sha256.New()
+		if _, err := io.Copy(hash, bytes.NewReader(test.bytes)); err != nil {
+			t.Fatalf("could not hash bytes: %s", err)
+		}
+		expectedDigest := fmt.Sprintf("%s:%x", BlobAlgorithm, hash.Sum(nil))
+
+		digest, size, err := engine.PutBlob(ctx, bytes.NewReader(test.bytes))
+		if err != nil {
+			t.Errorf("PutBlob: unexpected error: %s", err)
+		}
+
+		if digest != expectedDigest {
+			t.Errorf("PutBlob: digest doesn't match: expected=%s got=%s", expectedDigest, digest)
+		}
+		if size != int64(len(test.bytes)) {
+			t.Errorf("PutBlob: length doesn't match: expected=%d got=%d", len(test.bytes), size)
+		}
+
+		blobReader, err := engine.GetBlob(ctx, digest)
+		if err != nil {
+			t.Errorf("GetBlob: unexpected error: %s", err)
+		}
+		defer blobReader.Close()
+
+		gotBytes, err := ioutil.ReadAll(blobReader)
+		if err != nil {
+			t.Errorf("GetBlob: failed to ReadAll: %s", err)
+		}
+		if !bytes.Equal(test.bytes, gotBytes) {
+			t.Errorf("GetBlob: bytes did not match: expected=%s got=%s", string(test.bytes), string(gotBytes))
+		}
+
+		if err := engine.DeleteBlob(ctx, digest); err != nil {
+			t.Errorf("DeleteBlob: unexpected error: %s", err)
+		}
+
+		if br, err := engine.GetBlob(ctx, digest); !os.IsNotExist(err) {
+			if err == nil {
+				br.Close()
+				t.Errorf("GetBlob: still got blob contents after DeleteBlob!")
+			} else {
+				t.Errorf("GetBlob: unexpected error: %s", err)
+			}
+		}
+
+		// DeleteBlob is idempotent. It shouldn't cause an error.
+		if err := engine.DeleteBlob(ctx, digest); err != nil {
+			t.Errorf("DeleteBlob: unexpected error on double-delete: %s", err)
+		}
+	}
+
+	// Should be no blobs left.
+	if blobs, err := engine.ListBlobs(ctx); err != nil {
+		t.Errorf("unexpected error getting list of blobs: %s", err)
+	} else if len(blobs) > 0 {
+		t.Errorf("got blobs in a clean image: %v", blobs)
+	}
+}
+
+func TestEngineBlobJSON(t *testing.T) {
+	ctx := context.Background()
+
+	root, err := ioutil.TempDir("", "umoci-TestEngineBlobJSON")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(root)
+
+	image := filepath.Join(root, "image")
+	if err := CreateLayout(image); err != nil {
+		t.Fatalf("unexpected error creating image: %s", err)
+	}
+
+	engine, err := Open(image)
+	if err != nil {
+		t.Fatalf("unexpected error opening image: %s", err)
+	}
+	defer engine.Close()
+
+	type object struct {
+		A string `json:"A"`
+		B int64  `json:"B,omitempty"`
+	}
+
+	for _, test := range []struct {
+		object object
+	}{
+		{object{}},
+		{object{"a value", 100}},
+		{object{"another value", 200}},
+	} {
+		digest, _, err := engine.PutBlobJSON(ctx, test.object)
+		if err != nil {
+			t.Errorf("PutBlobJSON: unexpected error: %s", err)
+		}
+
+		blobReader, err := engine.GetBlob(ctx, digest)
+		if err != nil {
+			t.Errorf("GetBlob: unexpected error: %s", err)
+		}
+		defer blobReader.Close()
+
+		gotBytes, err := ioutil.ReadAll(blobReader)
+		if err != nil {
+			t.Errorf("GetBlob: failed to ReadAll: %s", err)
+		}
+
+		var gotObject object
+		if err := json.Unmarshal(gotBytes, &gotObject); err != nil {
+			t.Errorf("GetBlob: got an invalid JSON blob: %s", err)
+		}
+		if !reflect.DeepEqual(test.object, gotObject) {
+			t.Errorf("GetBlob: got different object to original JSON. expected=%v got=%v gotBytes=%v", test.object, gotObject, gotBytes)
+		}
+
+		if err := engine.DeleteBlob(ctx, digest); err != nil {
+			t.Errorf("DeleteBlob: unexpected error: %s", err)
+		}
+
+		if br, err := engine.GetBlob(ctx, digest); !os.IsNotExist(err) {
+			if err == nil {
+				br.Close()
+				t.Errorf("GetBlob: still got blob contents after DeleteBlob!")
+			} else {
+				t.Errorf("GetBlob: unexpected error: %s", err)
+			}
+		}
+
+		// DeleteBlob is idempotent. It shouldn't cause an error.
+		if err := engine.DeleteBlob(ctx, digest); err != nil {
+			t.Errorf("DeleteBlob: unexpected error on double-delete: %s", err)
+		}
+	}
+
+	// Should be no blobs left.
+	if blobs, err := engine.ListBlobs(ctx); err != nil {
+		t.Errorf("unexpected error getting list of blobs: %s", err)
+	} else if len(blobs) > 0 {
+		t.Errorf("got blobs in a clean image: %v", blobs)
+	}
+}
+
+func TestEngineReference(t *testing.T) {
+	ctx := context.Background()
+
+	root, err := ioutil.TempDir("", "umoci-TestEngineReference")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(root)
+
+	image := filepath.Join(root, "image")
+	if err := CreateLayout(image); err != nil {
+		t.Fatalf("unexpected error creating image: %s", err)
+	}
+
+	engine, err := Open(image)
+	if err != nil {
+		t.Fatalf("unexpected error opening image: %s", err)
+	}
+	defer engine.Close()
+
+	for _, test := range []struct {
+		name       string
+		descriptor v1.Descriptor
+	}{
+		{"ref1", v1.Descriptor{}},
+		{"ref2", v1.Descriptor{MediaType: v1.MediaTypeImageConfig, Digest: "sha256:032581de4629652b8653e4dbb2762d0733028003f1fc8f9edd61ae8181393a15", Size: 100}},
+		{"ref3", v1.Descriptor{MediaType: v1.MediaTypeImageLayerNonDistributable, Digest: "sha256:3c968ad60d3a2a72a12b864fa1346e882c32690cbf3bf3bc50ee0d0e4e39f342", Size: 8888}},
+	} {
+		if err := engine.PutReference(ctx, test.name, &test.descriptor); err != nil {
+			t.Errorf("PutReference: unexpected error: %s", err)
+		}
+
+		gotDescriptor, err := engine.GetReference(ctx, test.name)
+		if err != nil {
+			t.Errorf("GetReference: unexpected error: %s", err)
+		}
+
+		if !reflect.DeepEqual(test.descriptor, *gotDescriptor) {
+			t.Errorf("GetReference: got different descriptor to original: expected=%v got=%v", test.descriptor, gotDescriptor)
+		}
+
+		if err := engine.DeleteReference(ctx, test.name); err != nil {
+			t.Errorf("DeleteReference: unexpected error: %s", err)
+		}
+
+		if _, err := engine.GetReference(ctx, test.name); !os.IsNotExist(err) {
+			if err == nil {
+				t.Errorf("GetReference: still got reference descriptor after DeleteReference!")
+			} else {
+				t.Errorf("GetReference: unexpected error: %s", err)
+			}
+		}
+
+		// DeleteBlob is idempotent. It shouldn't cause an error.
+		if err := engine.DeleteReference(ctx, test.name); err != nil {
+			t.Errorf("DeleteReference: unexpected error on double-delete: %s", err)
+		}
+	}
+}

image/cas/dir.go

@@ -65,6 +65,9 @@ func CreateLayout(path string) error {
 	if err := os.Mkdir(filepath.Join(path, blobDirectory), 0755); err != nil {
 		return err
 	}
+	if err := os.Mkdir(filepath.Join(path, blobDirectory, BlobAlgorithm), 0755); err != nil {
+		return err
+	}
 	if err := os.Mkdir(filepath.Join(path, refDirectory), 0755); err != nil {
 		return err
 	}

image/layer/tar_extract.go

@@ -68,10 +68,6 @@ func applyMetadata(path string, hdr *tar.Header) error {
 		atime = mtime
 	}
 
-	if err := system.Lutimes(path, atime, mtime); err != nil {
-		return fmt.Errorf("apply metadata: %s: %s", path, err)
-	}
-
 	// Apply xattrs. In order to make sure that we *only* have the xattr set we
 	// want, we first clear the set of xattrs from the file then apply the ones
 	// set in the tar.Header.
@@ -85,6 +81,10 @@ func applyMetadata(path string, hdr *tar.Header) error {
 		}
 	}
 
+	if err := system.Lutimes(path, atime, mtime); err != nil {
+		return fmt.Errorf("apply metadata: %s: %s", path, err)
+	}
+
 	// TODO.
 	return nil
 }
@@ -194,6 +194,18 @@ func unpackEntry(root string, hdr *tar.Header, r io.Reader) error {
 		}
 	}
 
+	// Attempt to create the parent directory of the path we're unpacking.
+	// We do a MkdirAll here because even though you need to have a tar entry
+	// for every component of a new path, applyMetadata will correct any
+	// inconsistencies.
+	//
+	// FIXME: We have to make this consistent, since if the tar archive doesn't
+	//        have entries for some of these components we won't be able to
+	//        verify that we have consistent results during unpacking.
+	if err := os.MkdirAll(dir, 0777); err != nil {
+		return err
+	}
+
 	// Now create or otherwise modify the state of the path. Right now, either
 	// the type of path matches hdr or the path doesn't exist. Note that we
 	// don't care about umasks or the initial mode here, since applyMetadata