diff --git a/internal/cli/collection_publish_tag_failure_test.go b/internal/cli/collection_publish_tag_failure_test.go new file mode 100644 index 0000000..ad8b3c2 --- /dev/null +++ b/internal/cli/collection_publish_tag_failure_test.go @@ -0,0 +1,170 @@ +package cli + +import ( + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "path/filepath" + "slices" + "strings" + "sync" + "testing" + + coreerrors "github.com/devcontainers/cli/internal/errors" + "github.com/devcontainers/cli/internal/log" + "github.com/devcontainers/cli/internal/oci" +) + +type pushRegistry struct { + host string + failTag string + + mu sync.Mutex + blobs map[string][]byte + manifests map[string][]byte + attempts map[string][]string +} + +func newPushRegistry(t *testing.T, failTag string) *pushRegistry { + t.Helper() + r := &pushRegistry{failTag: failTag, blobs: map[string][]byte{}, manifests: map[string][]byte{}, attempts: map[string][]string{}} + srv := httptest.NewServer(http.HandlerFunc(r.serve)) + t.Cleanup(srv.Close) + // A localhost registry is plain HTTP for both CLIs. + _, port, err := net.SplitHostPort(srv.Listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + r.host = "localhost:" + port + return r +} + +func (r *pushRegistry) tagAttempts(repo string) []string { + r.mu.Lock() + defer r.mu.Unlock() + return slices.Clone(r.attempts[repo]) +} + +func (r *pushRegistry) serve(w http.ResponseWriter, req *http.Request) { + r.mu.Lock() + defer r.mu.Unlock() + path := strings.TrimPrefix(req.URL.Path, "/v2/") + notFound := func() { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"errors":[{"code":"NAME_UNKNOWN","message":"not found"}]}`)) + } + switch { + case path == "" || path == "/": + w.WriteHeader(http.StatusOK) + case strings.HasSuffix(path, "/tags/list"): + notFound() + case strings.Contains(path, "/blobs/uploads/") && req.Method == http.MethodPost: + w.Header().Set("Location", req.URL.Path+"session") + w.WriteHeader(http.StatusAccepted) + case strings.Contains(path, "/blobs/uploads/") && req.Method == http.MethodPut: + body, _ := io.ReadAll(req.Body) + digest := req.URL.Query().Get("digest") + r.blobs[digest] = body + w.Header().Set("Docker-Content-Digest", digest) + w.WriteHeader(http.StatusCreated) + case strings.Contains(path, "/blobs/"): + body, ok := r.blobs[path[strings.LastIndex(path, "/")+1:]] + if !ok { + notFound() + return + } + w.Header().Set("Content-Length", fmt.Sprint(len(body))) + if req.Method != http.MethodHead { + _, _ = w.Write(body) + } + case strings.Contains(path, "/manifests/"): + i := strings.Index(path, "/manifests/") + repo, ref := path[:i], path[i+len("/manifests/"):] + if req.Method != http.MethodPut { + body, ok := r.manifests[repo+":"+ref] + if !ok { + notFound() + return + } + w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json") + w.Header().Set("Content-Length", fmt.Sprint(len(body))) + if req.Method != http.MethodHead { + _, _ = w.Write(body) + } + return + } + r.attempts[repo] = append(r.attempts[repo], ref) + if ref == r.failTag { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"errors":[{"code":"TAG_INVALID","message":"tag rejected by test"}]}`)) + return + } + body, _ := io.ReadAll(req.Body) + r.manifests[repo+":"+ref] = body + sum := sha256.Sum256(body) + w.Header().Set("Docker-Content-Digest", "sha256:"+hex.EncodeToString(sum[:])) + w.WriteHeader(http.StatusCreated) + default: + notFound() + } +} + +func writeTagFailureCollection(t *testing.T, collectionType string) string { + t.Helper() + src := filepath.Join(t.TempDir(), "src") + if collectionType == "feature" { + writeSeamFeature(t, src, "good", "1.0.0") + return src + } + writeFile(t, filepath.Join(src, "good", "devcontainer-template.json"), `{"id":"good","version":"1.0.0","name":"Good"}`, 0o644) + writeFile(t, filepath.Join(src, "good", ".devcontainer.json"), `{"image":"alpine"}`, 0o644) + return src +} + +func TestPublishCollection_FailedTagPushExits(t *testing.T) { + for _, collectionType := range []string{"feature", "template"} { + t.Run(collectionType, func(t *testing.T) { + reg := newPushRegistry(t, "1.0") + out := &captureOutput{} + logger := log.New(log.Options{Level: log.LevelInfo, Format: "text", Writer: out.Stderr()}) + client := oci.NewClientWithAuthPolicy(logger, map[string]string{"DOCKER_CONFIG": t.TempDir()}, oci.DefaultAuthPolicy()) + + err := publishCollectionWith(t.Context(), out, client, writeTagFailureCollection(t, collectionType), reg.host, "me/coll", collectionType, "info") + if exit, ok := errors.AsType[*coreerrors.ExitCodeError](err); !ok || exit.Code != 1 { + t.Fatalf("err = %v, want exit code 1 after the rejected tag", err) + } + if out.out.Len() != 0 { + t.Errorf("stdout = %q, want no result after a failed tag push", out.out.String()) + } + + resource := reg.host + "/me/coll/good" + stderr := out.err.String() + for _, want := range []string{ + "Failed to PUT manifest for tag 1.0\n", + fmt.Sprintf("(!) ERR: Failed to publish %s: '%s'", collectionType, resource), + fmt.Sprintf("(!) ERR: Failed to publish '%s'", resource), + } { + if !strings.Contains(stderr, want) { + t.Errorf("stderr is missing %q:\n%s", want, stderr) + } + } + if strings.Contains(stderr, "Published "+collectionType) { + t.Errorf("stderr reports a successful publish:\n%s", stderr) + } + + if got, want := reg.tagAttempts("me/coll/good"), []string{"1", "1.0"}; !slices.Equal(got, want) { + t.Errorf("tag PUTs = %v, want %v: no tag is tried after the first failure", got, want) + } + if got := reg.tagAttempts("me/coll"); len(got) != 0 { + t.Errorf("collection metadata PUTs = %v, want none after a failed publish", got) + } + }) + } +} diff --git a/internal/cli/publish_parity_test.go b/internal/cli/publish_parity_test.go index 35c3e8a..44c92d2 100644 --- a/internal/cli/publish_parity_test.go +++ b/internal/cli/publish_parity_test.go @@ -1,11 +1,15 @@ package cli import ( + "bytes" + "errors" "os" "os/exec" "path/filepath" "reflect" + "slices" "sort" + "strings" "testing" "time" @@ -25,6 +29,8 @@ func TestPublishParity(t *testing.T) { if _, err := os.Stat(goCLI); err != nil { t.Skip("Go CLI not built") } + t.Run("tag failure", func(t *testing.T) { publishTagFailureParity(t, repoRoot, goCLI) }) + if !isDockerAvailable() { t.Skip("docker required") } @@ -115,3 +121,56 @@ func TestPublishParity(t *testing.T) { }) } } + +func publishTagFailureParity(t *testing.T, repoRoot, goCLI string) { + for _, collectionType := range []string{"feature", "template"} { + t.Run(collectionType, func(t *testing.T) { + reg := newPushRegistry(t, "1.0") + src := writeTagFailureCollection(t, collectionType) + type side struct { + stdout string + errLines []string + exit int + attempts []string + metadata []string + } + publish := func(cli []string, namespace string) side { + args := append(slices.Clone(cli[1:]), collectionType+"s", "publish", src, "-r", reg.host, "-n", namespace) + cmd := exec.CommandContext(t.Context(), cli[0], args...) + cmd.Dir = repoRoot + cmd.Env = append(os.Environ(), "DOCKER_CONFIG="+t.TempDir()) + var stdout, stderr bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, &stderr + err := cmd.Run() + var exitErr *exec.ExitError + if err != nil && !errors.As(err, &exitErr) { + t.Fatalf("run %v: %v", cli, err) + } + var errLines []string + for line := range strings.Lines(strings.ReplaceAll(stderr.String(), namespace, "")) { + for _, marker := range []string{"(!) ERR:", "Failed to PUT manifest for tag"} { + if i := strings.Index(line, marker); i >= 0 { + errLines = append(errLines, strings.TrimSpace(line[i:])) + } + } + } + return side{ + stdout: stdout.String(), + errLines: errLines, + exit: cmd.ProcessState.ExitCode(), + attempts: reg.tagAttempts(namespace + "/good"), + metadata: reg.tagAttempts(namespace), + } + } + + ts := publish([]string{"node", filepath.Join(repoRoot, "reference", "devcontainer.js")}, "tsns/coll") + goSide := publish([]string{goCLI}, "gons/coll") + if !reflect.DeepEqual(ts, goSide) { + t.Errorf("publish with a rejected tag differs:\n--- TS\n%+v\n--- Go\n%+v", ts, goSide) + } + if ts.exit != 1 || !slices.Equal(ts.attempts, []string{"1", "1.0"}) { + t.Errorf("TS side = %+v: the fake registry did not exercise the failed tag", ts) + } + }) + } +} diff --git a/internal/oci/push.go b/internal/oci/push.go index f6f293b..694be8b 100644 --- a/internal/oci/push.go +++ b/internal/oci/push.go @@ -79,23 +79,17 @@ func (c *Client) PushArtifact(ctx context.Context, ref *Ref, tgzPath string, tag return nil, err } - var publishedTags []string for _, tag := range tags { if err := repo.PushReference(ctx, manifestDesc, bytes.NewReader(manifestBytes), tag); err != nil { - c.log.Write(fmt.Sprintf("Failed to push tag %s: %v", tag, err), log.LevelWarning) - continue + c.log.Write(fmt.Sprintf("Failed to PUT manifest for tag %s\n%v", tag, err), log.LevelError) + return nil, fmt.Errorf("push tag %s of %s: %w", tag, ref.Resource, err) } - publishedTags = append(publishedTags, tag) c.log.Write(fmt.Sprintf("Published tag: %s", tag), log.LevelInfo) } - if len(publishedTags) == 0 { - return nil, fmt.Errorf("failed to publish any tags") - } - return &PushResult{ Digest: manifestDesc.Digest.String(), - PublishedTags: publishedTags, + PublishedTags: tags, }, nil } diff --git a/internal/oci/push_partial_test.go b/internal/oci/push_partial_test.go index f30983d..d485913 100644 --- a/internal/oci/push_partial_test.go +++ b/internal/oci/push_partial_test.go @@ -1,8 +1,12 @@ package oci import ( + "bytes" + "maps" "os" "path/filepath" + "slices" + "strings" "testing" "github.com/devcontainers/cli/internal/log" @@ -17,38 +21,45 @@ func writeTarball(t *testing.T, content string) string { return p } -func TestPushArtifact_BearerAuthAndPartialTagFailure(t *testing.T) { +func TestPushArtifact_BearerAuthAndStopsAtFirstFailedTag(t *testing.T) { const user, pass = "u", "p" reg := newFakeOCIRegistry() reg.requireBearer = true reg.wantUser, reg.wantPass = user, pass - reg.failTags = map[string]bool{"latest": true} + reg.failTags = map[string]bool{"1.0": true} registry := reg.start(t) ref, err := ParseRef(registry + "/ns/hello:1.0.0") if err != nil { t.Fatal(err) } - client := NewClient(log.Null, dockerConfigEnv(t, registry, user, pass)) + var logs bytes.Buffer + client := NewClient(log.New(log.Options{Writer: &logs, Level: log.LevelInfo}), dockerConfigEnv(t, registry, user, pass)) - res, err := client.PushArtifact(t.Context(), ref, writeTarball(t, "layer"), []string{"1.0.0", "latest"}, "feature", nil) - if err != nil { - t.Fatalf("push returned error, want partial success: %v", err) + res, err := client.PushArtifact(t.Context(), ref, writeTarball(t, "layer"), []string{"1", "1.0", "1.0.0", "latest"}, "feature", nil) + if err == nil { + t.Fatalf("push returned %+v, want an error for the rejected tag 1.0 (reference putManifestWithTags returns undefined)", res) } - - if got := res.PublishedTags; len(got) != 1 || got[0] != "1.0.0" { - t.Fatalf("PublishedTags = %v, want [1.0.0] (partial, no rollback)", got) + if res != nil { + t.Errorf("result = %+v, want nil alongside the error", res) + } + if !strings.Contains(err.Error(), "1.0") || !strings.Contains(err.Error(), ref.Resource) { + t.Errorf("error %q does not name the tag and resource", err) } + if !strings.Contains(logs.String(), "Failed to PUT manifest for tag 1.0\n") { + t.Errorf("log does not report the rejected tag:\n%s", logs.String()) + } + reg.mu.Lock() - _, has100 := reg.manifests["1.0.0"] - _, hasLatest := reg.manifests["latest"] + attempted := slices.Clone(reg.putManifestAttempt) + tagged := slices.Sorted(maps.Keys(reg.manifests)) tokenReqs := reg.tokenRequests reg.mu.Unlock() - if !has100 { - t.Error("1.0.0 tag was not published") + if want := []string{"1", "1.0"}; !slices.Equal(attempted, want) { + t.Errorf("tag PUTs attempted = %v, want %v: tags after the first failure must not be tried", attempted, want) } - if hasLatest { - t.Error("latest tag was published despite the injected failure") + if want := []string{"1"}; !slices.Equal(tagged, want) { + t.Errorf("tags in registry = %v, want %v (tags before the failure stay pushed)", tagged, want) } if tokenReqs == 0 { t.Error("token endpoint never called; the Bearer auth loop did not run") diff --git a/internal/oci/push_registry_test.go b/internal/oci/push_registry_test.go index a6239c3..5257887 100644 --- a/internal/oci/push_registry_test.go +++ b/internal/oci/push_registry_test.go @@ -27,10 +27,11 @@ type fakeOCIRegistry struct { failTags map[string]bool failAllManifests bool - tokenRequests int - putManifestOK []string - blobUploads int - blobHeadHits int + tokenRequests int + putManifestAttempt []string + putManifestOK []string + blobUploads int + blobHeadHits int } func newFakeOCIRegistry() *fakeOCIRegistry { @@ -159,6 +160,7 @@ func (r *fakeOCIRegistry) handleManifest(w http.ResponseWriter, req *http.Reques switch req.Method { case http.MethodPut: + r.putManifestAttempt = append(r.putManifestAttempt, ref) if r.failAllManifests || r.failTags[ref] { // 400 is a non-retryable client error, so oras returns immediately // (a 5xx would trigger retry backoff and slow the test), while still