diff --git a/client/client_export_metadata_test.go b/client/client_export_metadata_test.go index df915b742..819be230a 100644 --- a/client/client_export_metadata_test.go +++ b/client/client_export_metadata_test.go @@ -5,17 +5,12 @@ import ( "context" "encoding/json" "fmt" - "io" - "net/http" - "net/http/httptest" - "net/http/httputil" "net/url" "os" "path" "path/filepath" "strconv" "strings" - "sync" "testing" "time" @@ -673,7 +668,7 @@ func testExportAnnotationsMediaTypes(t *testing.T, sb integration.Sandbox) { require.Equal(t, ocispecs.MediaTypeImageIndex, imgs2.Index.MediaType) } -func testExportAttestations(t *testing.T, sb integration.Sandbox, ociArtifact bool, setOCIArtifact bool, strictSubjectRegistry bool) { +func testExportAttestations(t *testing.T, sb integration.Sandbox, ociArtifact bool, setOCIArtifact bool) { workers.CheckFeatureCompat(t, sb, workers.FeatureDirectPush) requiresLinux(t) c, err := New(sb.Context(), sb.Address()) @@ -686,12 +681,6 @@ func testExportAttestations(t *testing.T, sb integration.Sandbox, ociArtifact bo } require.NoError(t, err) - var strictRegistry *strictSubjectRegistryProxy - if strictSubjectRegistry { - strictRegistry = newStrictSubjectRegistryProxy(t, registry) - registry = strictRegistry.host - } - ps := []ocispecs.Platform{ platforms.MustParse("linux/amd64"), platforms.MustParse("linux/arm64"), @@ -909,10 +898,6 @@ func testExportAttestations(t *testing.T, sb integration.Sandbox, ociArtifact bo require.Equal(t, subjects, attest2.Subject) } - if strictRegistry != nil { - require.True(t, strictRegistry.sawSubjectManifest(), "expected an OCI artifact manifest with subject") - } - cdAddress := sb.ContainerdAddress() if cdAddress == "" { return @@ -929,10 +914,6 @@ func testExportAttestations(t *testing.T, sb integration.Sandbox, ociArtifact bo checkAllReleasable(t, c, sb, true) }) - if strictRegistry != nil { - return // strict registry only exercises the image push path - } - t.Run("local", func(t *testing.T) { dir := t.TempDir() _, err = c.Build(sb.Context(), SolveOpt{ @@ -1041,19 +1022,15 @@ func testExportAttestations(t *testing.T, sb integration.Sandbox, ociArtifact bo } func testExportAttestationsDefaultOCIArtifact(t *testing.T, sb integration.Sandbox) { - testExportAttestations(t, sb, false, false, false) + testExportAttestations(t, sb, false, false) } func testExportAttestationsImageManifest(t *testing.T, sb integration.Sandbox) { - testExportAttestations(t, sb, false, true, false) + testExportAttestations(t, sb, false, true) } func testExportAttestationsOCIArtifact(t *testing.T, sb integration.Sandbox) { - testExportAttestations(t, sb, true, true, false) -} - -func testExportAttestationsOCIArtifactSubjectPushOrder(t *testing.T, sb integration.Sandbox) { - testExportAttestations(t, sb, true, true, true) + testExportAttestations(t, sb, true, true) } func testImageResolveAttestationChainLocal(t *testing.T, sb integration.Sandbox) { @@ -2443,124 +2420,3 @@ func isSLSAPredicateType(v string) bool { return false } } - -type strictSubjectRegistryProxy struct { - host string - - mu sync.Mutex - manifests map[digest.Digest]struct{} - sawSubject bool -} - -func newStrictSubjectRegistryProxy(t *testing.T, registry string) *strictSubjectRegistryProxy { - t.Helper() - - target, err := url.Parse("http://" + registry) - require.NoError(t, err) - - p := &strictSubjectRegistryProxy{ - manifests: map[digest.Digest]struct{}{}, - } - proxy := &httputil.ReverseProxy{ - Rewrite: func(pr *httputil.ProxyRequest) { - pr.SetURL(target) - pr.Out.Host = target.Host - }, - } - - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPut || !strings.Contains(r.URL.Path, "/manifests/") { - proxy.ServeHTTP(w, r) - return - } - - body, err := io.ReadAll(r.Body) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - r.Body = io.NopCloser(bytes.NewReader(body)) - r.ContentLength = int64(len(body)) - - subject, err := manifestSubject(body) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - if subject != "" && !p.hasManifest(subject) { - w.WriteHeader(http.StatusBadRequest) - _, _ = fmt.Fprintf(w, "unknown: blob unknown to registry - %s", subject) - return - } - - rw := &statusRecorder{ResponseWriter: w} - proxy.ServeHTTP(rw, r) - if rw.success() { - p.recordManifest(digest.FromBytes(body), subject) - } - })) - t.Cleanup(s.Close) - - p.host = strings.TrimPrefix(s.URL, "http://") - return p -} - -func (p *strictSubjectRegistryProxy) hasManifest(dgst digest.Digest) bool { - p.mu.Lock() - defer p.mu.Unlock() - _, ok := p.manifests[dgst] - return ok -} - -func (p *strictSubjectRegistryProxy) recordManifest(dgst, subject digest.Digest) { - p.mu.Lock() - defer p.mu.Unlock() - p.manifests[dgst] = struct{}{} - p.sawSubject = p.sawSubject || subject != "" -} - -func (p *strictSubjectRegistryProxy) sawSubjectManifest() bool { - p.mu.Lock() - defer p.mu.Unlock() - return p.sawSubject -} - -func manifestSubject(dt []byte) (digest.Digest, error) { - var manifest struct { - Subject *ocispecs.Descriptor `json:"subject"` - } - if err := json.Unmarshal(dt, &manifest); err != nil { - return "", err - } - if manifest.Subject == nil { - return "", nil - } - return manifest.Subject.Digest, nil -} - -type statusRecorder struct { - http.ResponseWriter - status int -} - -func (r *statusRecorder) WriteHeader(status int) { - r.status = status - r.ResponseWriter.WriteHeader(status) -} - -func (r *statusRecorder) Write(dt []byte) (int, error) { - if r.status == 0 { - r.status = http.StatusOK - } - return r.ResponseWriter.Write(dt) -} - -func (r *statusRecorder) Flush() { - if f, ok := r.ResponseWriter.(http.Flusher); ok { - f.Flush() - } -} - -func (r *statusRecorder) success() bool { - return r.status == 0 || r.status >= http.StatusOK && r.status < http.StatusMultipleChoices -} diff --git a/client/client_test.go b/client/client_test.go index 1420fc192..d3e66f24f 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -104,7 +104,6 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){ testExportAttestationsDefaultOCIArtifact, testExportAttestationsImageManifest, testExportAttestationsOCIArtifact, - testExportAttestationsOCIArtifactSubjectPushOrder, testImageResolveAttestationChainLocal, testImageResolveAttestationChainRequiresNetwork, testImageResolveProvenanceAttestation, diff --git a/util/push/push.go b/util/push/push.go index 001bf2953..0ffe86757 100644 --- a/util/push/push.go +++ b/util/push/push.go @@ -138,13 +138,8 @@ func Push(ctx context.Context, sm *session.Manager, sid string, provider content return err } - manifestStack, err = orderManifests(ctx, provider, manifestStack) - if err != nil { - return err - } - mfstDone := progress.OneOff(ctx, fmt.Sprintf("pushing manifest for %s", ref)) - for _, desc := range manifestStack { + for _, desc := range slices.Backward(manifestStack) { if _, err := pushHandler(ctx, desc); err != nil { return mfstDone(err) } @@ -152,56 +147,6 @@ func Push(ctx context.Context, sm *session.Manager, sid string, provider content return mfstDone(nil) } -// orderManifests returns manifests in push order. It preserves the existing -// child-before-parent behavior from reversing the dispatch stack, and also -// ensures that subjects present in the same push are uploaded before the -// manifests or indexes that reference them. -func orderManifests(ctx context.Context, provider content.Provider, manifests []ocispecs.Descriptor) ([]ocispecs.Descriptor, error) { - manifestByDigest := make(map[digest.Digest]ocispecs.Descriptor, len(manifests)) - for _, desc := range manifests { - manifestByDigest[desc.Digest] = desc - } - - ordered := make([]ocispecs.Descriptor, 0, len(manifests)) - visited := make(map[digest.Digest]struct{}, len(manifests)) - - var visit func(ocispecs.Descriptor) error - visit = func(desc ocispecs.Descriptor) error { - if _, ok := visited[desc.Digest]; ok { - return nil - } - visited[desc.Digest] = struct{}{} - if images.IsManifestType(desc.MediaType) || images.IsIndexType(desc.MediaType) { - p, err := content.ReadBlob(ctx, provider, desc) - if err != nil { - return err - } - var withSubject struct { - Subject *ocispecs.Descriptor `json:"subject"` - } - if err := json.Unmarshal(p, &withSubject); err != nil { - return err - } - if withSubject.Subject != nil { - if dep, ok := manifestByDigest[withSubject.Subject.Digest]; ok { - if err := visit(dep); err != nil { - return err - } - } - } - } - ordered = append(ordered, desc) - return nil - } - - for _, desc := range slices.Backward(manifests) { - if err := visit(desc); err != nil { - return nil, err - } - } - return ordered, nil -} - // TODO: the containerd function for this is filtering too much, that needs to be fixed. // For now we just carry this. func skipNonDistributableBlobs(f images.HandlerFunc) images.HandlerFunc {