Fix regression in v0.11.0 (#250)

* Fix regression in v0.11.0

Change the default for writer opt 'IgnoreNoName' to be
true. This was set to false, and causing the example to
not run properly.

A new option 'WithErrorOnNoName' has been added to be
used in cases when this behaviour is expected. The existing
option 'WithIgnoreNoName' has now been deprecated.

A simple bash-based acceptance test has been added to
prevent this issue in the future. This starts a registry,
compiles the example into a binary, and makes sure it
runs properly. This test is now run in CI.

Signed-off-by: Josh Dolitsky <[email protected]>

* Allow hostname override for CI

Signed-off-by: Josh Dolitsky <[email protected]>

* use plain http for example..

Signed-off-by: Josh Dolitsky <[email protected]>

* Add logic to wait for registry to come up

Signed-off-by: Josh Dolitsky <[email protected]>

.github/workflows/build-pr.yml

@@ -17,6 +17,10 @@ jobs:
           go-version: '1.16.2'
       - name: run unit tests
         run: make test
+      - name: run acceptance tests
+        run: |
+          export LOCAL_REGISTRY_HOSTNAME="$(hostname -I | awk '{print $1}')"
+          make acceptance
       - name: upload coverage report
         uses: actions/upload-artifact@master
         with:

.github/workflows/build.yml

@@ -14,6 +14,12 @@ jobs:
         uses: actions/setup-go@v1
         with:
           go-version: '1.16.2'
+      - name: run unit tests
+        run: make test
+      - name: run acceptance tests
+        run: |
+          export LOCAL_REGISTRY_HOSTNAME="$(hostname -I | awk '{print $1}')"
+          make acceptance
       - name: upload coverage report
         uses: actions/upload-artifact@master
         with:

Makefile

@@ -21,6 +21,10 @@ LDFLAGS += -X $(PROJECT_PKG)/internal/version.GitTreeState=${GIT_DIRTY}
 test: vendor check-encoding
 	./scripts/test.sh
 
+.PHONY: acceptance
+acceptance:
+	./scripts/acceptance.sh
+
 .PHONY: covhtml
 covhtml:
 	open .cover/coverage.html

examples/simple_push_pull.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"fmt"
+	"os"
 
 	"github.com/deislabs/oras/pkg/content"
 	"github.com/deislabs/oras/pkg/oras"
@@ -17,14 +18,22 @@ func check(e error) {
 	}
 }
 
+func getLocalRegistryHostname() string {
+	hostname := "localhost"
+	if v := os.Getenv("LOCAL_REGISTRY_HOSTNAME"); v != "" {
+		hostname = v
+	}
+	return hostname
+}
+
 func main() {
-	ref := "localhost:5000/oras:test"
+	ref := fmt.Sprintf("%s:5000/oras:test", getLocalRegistryHostname())
 	fileName := "hello.txt"
 	fileContent := []byte("Hello World!\n")
 	customMediaType := "my.custom.media.type"
 
 	ctx := context.Background()
-	resolver := docker.NewResolver(docker.ResolverOptions{})
+	resolver := docker.NewResolver(docker.ResolverOptions{PlainHTTP: true})
 
 	// Push file(s) w custom mediatype to registry
 	memoryStore := content.NewMemoryStore()

pkg/content/content_test.go

@@ -49,7 +49,7 @@ func (suite *ContentTestSuite) SetupSuite() {
 	os.Remove(testFileName)
 	err := ioutil.WriteFile(testFileName, testContent, 0644)
 	suite.Nil(err, "no error creating test file on disk")
-	testFileStore := NewFileStore(testDirRoot)
+	testFileStore := NewFileStore(testDirRoot, WithErrorOnNoName())
 	_, err = testFileStore.Add(testRef, "", testFileName)
 	suite.Nil(err, "no error adding item to file store")
 	suite.TestFileStore = testFileStore

pkg/content/file.go

@@ -41,7 +41,7 @@ type FileStore struct {
 // NewFileStore creats a new file store
 func NewFileStore(rootPath string, opts ...WriterOpt) *FileStore {
 	// we have to process the opts to find if they told us to change defaults
-	var wOpts WriterOpts
+	wOpts := DefaultWriterOpts()
 	for _, opt := range opts {
 		if err := opt(&wOpts); err != nil {
 			continue

pkg/content/file_test.go

@@ -25,8 +25,8 @@ func TestFileStoreNoName(t *testing.T) {
 		opts []content.WriterOpt
 		err  error
 	}{
-		{nil, content.ErrNoName},
-		{[]content.WriterOpt{content.WithIgnoreNoName()}, nil},
+		{nil, nil},
+		{[]content.WriterOpt{content.WithErrorOnNoName()}, content.ErrNoName},
 	}
 	for _, tt := range tests {
 		rootPath, err := ioutil.TempDir("", "oras_filestore_test")

pkg/content/opts.go

@@ -21,7 +21,7 @@ func DefaultWriterOpts() WriterOpts {
 		InputHash:    nil,
 		OutputHash:   nil,
 		Blocksize:    DefaultBlocksize,
-		IgnoreNoName: false,
+		IgnoreNoName: true,
 	}
 }
 
@@ -74,9 +74,21 @@ func WithMultiWriterIngester() WriterOpt {
 	}
 }
 
+// WithErrorOnNoName some ingesters, when creating a Writer, do not return an error if
+// the descriptor does not have a valid name on the descriptor. Passing WithErrorOnNoName
+// tells the writer to return an error instead of passing the data to a nil writer.
+func WithErrorOnNoName() WriterOpt {
+	return func(w *WriterOpts) error {
+		w.IgnoreNoName = false
+		return nil
+	}
+}
+
 // WithIgnoreNoName some ingesters, when creating a Writer, return an error if
 // the descriptor does not have a valid name on the descriptor. Passing WithIgnoreNoName
 // tells the writer not to return an error, but rather to pass the data to a nil writer.
+//
+// Deprecated: Use WithErrorOnNoName
 func WithIgnoreNoName() WriterOpt {
 	return func(w *WriterOpts) error {
 		w.IgnoreNoName = true

pkg/oras/oras_test.go

@@ -362,7 +362,7 @@ func (suite *ORASTestSuite) Test_4_GHSA_g5v4_5x39_vwhx() {
 			suite.FailNow("error creating temp directory", err)
 		}
 		defer os.RemoveAll(tempDir)
-		store := orascontent.NewFileStore(tempDir, orascontent.WithIgnoreNoName())
+		store := orascontent.NewFileStore(tempDir)
 		defer store.Close()
 		ref = fmt.Sprintf("%s/evil:%s", suite.DockerRegistryHost, tag)
 		_, _, err = Pull(newContext(), newResolver(), ref, store)

scripts/acceptance.sh

@@ -0,0 +1,43 @@
+#!/bin/bash -ex
+
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+cd $DIR/../
+
+LOCAL_REGISTRY_HOSTNAME="${LOCAL_REGISTRY_HOSTNAME:-localhost}"
+
+# Cleanup from previous runs
+rm -f hello.txt
+rm -f bin/oras-acceptance || true
+docker rm -f oras-acceptance-registry || true
+
+# Build the example into a binary
+CGO_ENABLED=0 go build -v -o bin/oras-acceptance ./examples/
+
+# Run a test registry and expose at localhost:5000
+trap "docker rm -f oras-acceptance-registry" EXIT
+docker run -d -p 5000:5000 \
+  --name oras-acceptance-registry \
+  index.docker.io/registry
+
+# Wait for a connection to port 5000 (timeout after 1 minute)
+WAIT_TIME=0
+while true; do
+  if nc -w 1 -z "${LOCAL_REGISTRY_HOSTNAME}" 5000; then
+    echo "Able to connect to ${LOCAL_REGISTRY_HOSTNAME} port 5000"
+    break
+  else
+    if (( ${WAIT_TIME} >= 60 )); then
+      echo "Timed out waiting for connection to ${LOCAL_REGISTRY_HOSTNAME} on port 5000. Exiting."
+      exit 1
+    fi
+    echo "Waiting to connect to ${LOCAL_REGISTRY_HOSTNAME} on port 5000. Sleeping 5 seconds.."
+    sleep 5
+    WAIT_TIME=$((WAIT_TIME + 5))
+  fi
+done
+
+# Run the example binary
+bin/oras-acceptance
+
+# Ensure hello.txt exists and contains expected content
+grep '^Hello World!$' hello.txt