Return both LastModified and ETag in manifest metadata. NFCI

src/backend.go

@@ -37,10 +37,19 @@ type GetManifestOptions struct {
 	BypassCache bool
 }
 
+type ManifestMetadata struct {
+	LastModified time.Time
+	ETag         string
+}
+
 type ModifyManifestOptions struct {
 	// If non-zero, the request will only succeed if the manifest hasn't been changed since
 	// the given time. Whether this is racy or not is can be determined via `HasAtomicCAS()`.
 	IfUnmodifiedSince time.Time
+	// If non-empty, the request will only succeed if the manifest hasn't changed from
+	// the state corresponding to the ETag. Whether this is racy or not is can be determined
+	// via `HasAtomicCAS()`.
+	IfMatch string
 }
 
 type QueryAuditLogOptions struct {
@@ -81,7 +90,7 @@ type Backend interface {
 
 	// Retrieve a manifest.
 	GetManifest(ctx context.Context, name string, opts GetManifestOptions) (
-		manifest *Manifest, mtime time.Time, err error,
+		manifest *Manifest, metadata ManifestMetadata, err error,
 	)
 
 	// Stage a manifest. This operation stores a new version of a manifest, locking any blobs

src/backend_fs.go

@@ -196,7 +196,7 @@ func (fs *FSBackend) ListManifests(ctx context.Context) (manifests []string, err
 func (fs *FSBackend) GetManifest(
 	ctx context.Context, name string, opts GetManifestOptions,
 ) (
-	manifest *Manifest, mtime time.Time, err error,
+	manifest *Manifest, metadata ManifestMetadata, err error,
 ) {
 	stat, err := fs.siteRoot.Stat(name)
 	if errors.Is(err, os.ErrNotExist) {
@@ -215,7 +215,9 @@ func (fs *FSBackend) GetManifest(
 	if err != nil {
 		return
 	}
-	return manifest, stat.ModTime(), nil
+	return manifest, ManifestMetadata{
+		LastModified: stat.ModTime(),
+	}, nil
 }
 
 func stagedManifestName(manifestData []byte) string {

src/backend_s3.go

@@ -117,8 +117,7 @@ func (c *CachedBlob) Weight() uint32 { return uint32(len(c.blob)) }
 type CachedManifest struct {
 	manifest *Manifest
 	weight   uint32
-	mtime    time.Time
-	etag     string
+	metadata ManifestMetadata
 	err      error
 }
 
@@ -423,8 +422,8 @@ func (l s3ManifestLoader) load(
 
 	loader := func() (*CachedManifest, error) {
 		opts := minio.GetObjectOptions{}
-		if oldManifest != nil && oldManifest.etag != "" {
-			opts.SetMatchETagExcept(oldManifest.etag)
+		if oldManifest != nil && oldManifest.metadata.ETag != "" {
+			opts.SetMatchETagExcept(oldManifest.metadata.ETag)
 		}
 		object, err := l.s3.client.GetObject(ctx, l.s3.bucket, manifestObjectName(name), opts)
 		// Note that many errors (e.g. NoSuchKey) will be reported only after this point.
@@ -448,7 +447,11 @@ func (l s3ManifestLoader) load(
 			return nil, err
 		}
 
-		return &CachedManifest{manifest, uint32(len(data)), stat.LastModified, stat.ETag, nil}, nil
+		metadata := ManifestMetadata{
+			LastModified: stat.LastModified,
+			ETag:         stat.ETag,
+		}
+		return &CachedManifest{manifest, uint32(len(data)), metadata, nil}, nil
 	}
 
 	observer := func() (*CachedManifest, error) {
@@ -471,7 +474,7 @@ func (l s3ManifestLoader) load(
 		errResp := minio.ToErrorResponse(err)
 		if errResp.Code == "NoSuchKey" {
 			err = fmt.Errorf("%w: %s", ErrObjectNotFound, errResp.Key)
-			return &CachedManifest{nil, 1, time.Time{}, "", err}, nil
+			return &CachedManifest{nil, 1, ManifestMetadata{}, err}, nil
 		} else if errResp.StatusCode == http.StatusNotModified && oldManifest != nil {
 			return oldManifest, nil
 		} else {
@@ -485,7 +488,7 @@ func (l s3ManifestLoader) load(
 func (s3 *S3Backend) GetManifest(
 	ctx context.Context, name string, opts GetManifestOptions,
 ) (
-	manifest *Manifest, mtime time.Time, err error,
+	manifest *Manifest, metadata ManifestMetadata, err error,
 ) {
 	if opts.BypassCache {
 		entry, found := s3.siteCache.Cache.GetEntry(name)
@@ -500,7 +503,7 @@ func (s3 *S3Backend) GetManifest(
 		return
 	} else {
 		// This could be `manifest, mtime, nil` or `nil, time.Time{}, ErrObjectNotFound`.
-		manifest, mtime, err = cached.manifest, cached.mtime, cached.err
+		manifest, metadata, err = cached.manifest, cached.metadata, cached.err
 		return
 	}
 }

src/collect.go

@@ -14,7 +14,7 @@ type Flusher interface {
 
 // Inverse of `ExtractTar`.
 func CollectTar(
-	context context.Context, writer io.Writer, manifest *Manifest, manifestMtime time.Time,
+	context context.Context, writer io.Writer, manifest *Manifest, metadata ManifestMetadata,
 ) (
 	err error,
 ) {
@@ -52,13 +52,13 @@ func CollectTar(
 		case Type_Directory:
 			header.Typeflag = tar.TypeDir
 			header.Mode = 0755
-			header.ModTime = manifestMtime
+			header.ModTime = metadata.LastModified
 			err = appendFile(&header, nil, Transform_Identity)
 
 		case Type_InlineFile:
 			header.Typeflag = tar.TypeReg
 			header.Mode = 0644
-			header.ModTime = manifestMtime
+			header.ModTime = metadata.LastModified
 			err = appendFile(&header, entry.GetData(), entry.GetTransform())
 
 		case Type_ExternalFile:
@@ -78,7 +78,7 @@ func CollectTar(
 		case Type_Symlink:
 			header.Typeflag = tar.TypeSymlink
 			header.Mode = 0644
-			header.ModTime = manifestMtime
+			header.ModTime = metadata.LastModified
 			err = appendFile(&header, entry.GetData(), Transform_Identity)
 
 		default:
@@ -94,7 +94,7 @@ func CollectTar(
 			Name:     RedirectsFileName,
 			Typeflag: tar.TypeReg,
 			Mode:     0644,
-			ModTime:  manifestMtime,
+			ModTime:  metadata.LastModified,
 		}, []byte(redirects), Transform_Identity)
 		if err != nil {
 			return err
@@ -106,7 +106,7 @@ func CollectTar(
 			Name:     HeadersFileName,
 			Typeflag: tar.TypeReg,
 			Mode:     0644,
-			ModTime:  manifestMtime,
+			ModTime:  metadata.LastModified,
 		}, []byte(headers), Transform_Identity)
 		if err != nil {
 			return err

src/main.go

@@ -309,12 +309,12 @@ func Main() {
 		}
 
 		webRoot := webRootArg(*getArchive)
-		manifest, manifestMtime, err :=
+		manifest, metadata, err :=
 			backend.GetManifest(ctx, webRoot, GetManifestOptions{})
 		if err != nil {
 			logc.Fatalln(ctx, err)
 		}
-		CollectTar(ctx, fileOutputArg(), manifest, manifestMtime)
+		CollectTar(ctx, fileOutputArg(), manifest, metadata)
 
 	case *updateSite != "":
 		if backend, err = CreateBackend(&config.Storage); err != nil {

src/observe.go

@@ -383,13 +383,13 @@ func (backend *observedBackend) ListManifests(ctx context.Context) (manifests []
 func (backend *observedBackend) GetManifest(
 	ctx context.Context, name string, opts GetManifestOptions,
 ) (
-	manifest *Manifest, mtime time.Time, err error,
+	manifest *Manifest, metadata ManifestMetadata, err error,
 ) {
 	span, ctx := ObserveFunction(ctx, "GetManifest",
 		"manifest.name", name,
 		"manifest.bypass_cache", opts.BypassCache,
 	)
-	if manifest, mtime, err = backend.inner.GetManifest(ctx, name, opts); err == nil {
+	if manifest, metadata, err = backend.inner.GetManifest(ctx, name, opts); err == nil {
 		manifestsRetrievedCount.Inc()
 	}
 	span.Finish()

src/pages.go

@@ -82,7 +82,7 @@ func getPage(w http.ResponseWriter, r *http.Request) error {
 	var err error
 	var sitePath string
 	var manifest *Manifest
-	var manifestMtime time.Time
+	var metadata ManifestMetadata
 
 	cacheControl, err := cacheobject.ParseRequestCacheControl(r.Header.Get("Cache-Control"))
 	if err != nil {
@@ -101,25 +101,25 @@ func getPage(w http.ResponseWriter, r *http.Request) error {
 	}
 
 	type indexManifestResult struct {
-		manifest      *Manifest
-		manifestMtime time.Time
-		err           error
+		manifest *Manifest
+		metadata ManifestMetadata
+		err      error
 	}
 	indexManifestCh := make(chan indexManifestResult, 1)
 	go func() {
-		manifest, mtime, err := backend.GetManifest(
+		manifest, metadata, err := backend.GetManifest(
 			r.Context(), makeWebRoot(host, ".index"),
 			GetManifestOptions{BypassCache: bypassCache},
 		)
-		indexManifestCh <- (indexManifestResult{manifest, mtime, err})
+		indexManifestCh <- (indexManifestResult{manifest, metadata, err})
 	}()
 
 	err = nil
 	sitePath = strings.TrimPrefix(r.URL.Path, "/")
 	if projectName, projectPath, hasProjectSlash := strings.Cut(sitePath, "/"); projectName != "" {
 		var projectManifest *Manifest
-		var projectManifestMtime time.Time
-		projectManifest, projectManifestMtime, err = backend.GetManifest(
+		var projectMetadata ManifestMetadata
+		projectManifest, projectMetadata, err = backend.GetManifest(
 			r.Context(), makeWebRoot(host, projectName),
 			GetManifestOptions{BypassCache: bypassCache},
 		)
@@ -128,12 +128,12 @@ func getPage(w http.ResponseWriter, r *http.Request) error {
 				writeRedirect(w, http.StatusFound, r.URL.Path+"/")
 				return nil
 			}
-			sitePath, manifest, manifestMtime = projectPath, projectManifest, projectManifestMtime
+			sitePath, manifest, metadata = projectPath, projectManifest, projectMetadata
 		}
 	}
 	if manifest == nil && (err == nil || errors.Is(err, ErrObjectNotFound)) {
 		result := <-indexManifestCh
-		manifest, manifestMtime, err = result.manifest, result.manifestMtime, result.err
+		manifest, metadata, err = result.manifest, result.metadata, result.err
 		if manifest == nil && errors.Is(err, ErrObjectNotFound) {
 			if fallback != nil {
 				logc.Printf(r.Context(), "fallback: %s via %s", host, config.Fallback.ProxyTo)
@@ -165,10 +165,11 @@ func getPage(w http.ResponseWriter, r *http.Request) error {
 		return nil
 	}
 	if metadataPath, found := strings.CutPrefix(sitePath, ".git-pages/"); found {
-		lastModified := manifestMtime.UTC().Format(http.TimeFormat)
+		lastModified := metadata.LastModified.UTC().Format(http.TimeFormat)
 		switch {
 		case metadataPath == "health":
 			w.Header().Add("Last-Modified", lastModified)
+			w.Header().Add("ETag", fmt.Sprintf("\"%s\"", metadata.ETag))
 			w.WriteHeader(http.StatusOK)
 			fmt.Fprintf(w, "ok\n")
 			return nil
@@ -183,6 +184,7 @@ func getPage(w http.ResponseWriter, r *http.Request) error {
 
 			w.Header().Add("Content-Type", "application/json; charset=utf-8")
 			w.Header().Add("Last-Modified", lastModified)
+			w.Header().Add("ETag", fmt.Sprintf("\"%s-manifest\"", metadata.ETag))
 			w.WriteHeader(http.StatusOK)
 			w.Write([]byte(ManifestDebugJSON(manifest)))
 			return nil
@@ -203,6 +205,7 @@ func getPage(w http.ResponseWriter, r *http.Request) error {
 			}
 			w.Header().Add("Content-Type", "application/x-tar")
 			w.Header().Add("Last-Modified", lastModified)
+			w.Header().Add("ETag", fmt.Sprintf("\"%s-archive\"", metadata.ETag))
 			w.Header().Add("Transfer-Encoding", "chunked")
 			w.WriteHeader(http.StatusOK)
 			var iow io.Writer
@@ -214,7 +217,7 @@ func getPage(w http.ResponseWriter, r *http.Request) error {
 			case "zstd":
 				iow, _ = zstd.NewWriter(w)
 			}
-			return CollectTar(r.Context(), iow, manifest, manifestMtime)
+			return CollectTar(r.Context(), iow, manifest, metadata)
 
 		default:
 			w.WriteHeader(http.StatusNotFound)

src/update.go

@@ -165,7 +165,7 @@ func PartialUpdateFromArchive(
 	// Here the old manifest is used both as a substrate to which a patch is applied, as well
 	// as a "load linked" operation for a future "store conditional" update which, taken together,
 	// create an atomic compare-and-swap operation.
-	oldManifest, oldManifestMtime, err := backend.GetManifest(ctx, webRoot,
+	oldManifest, oldMetadata, err := backend.GetManifest(ctx, webRoot,
 		GetManifestOptions{BypassCache: true})
 	if err != nil {
 		logc.Printf(ctx, "patch %s err: %s", webRoot, err)
@@ -204,7 +204,7 @@ func PartialUpdateFromArchive(
 		result = UpdateResult{UpdateError, nil, err}
 	} else {
 		result = Update(ctx, webRoot, oldManifest, newManifest,
-			ModifyManifestOptions{IfUnmodifiedSince: oldManifestMtime})
+			ModifyManifestOptions{IfUnmodifiedSince: oldMetadata.LastModified})
 		// The `If-Unmodified-Since` precondition is internally generated here, which means its
 		// failure shouldn't be surfaced as-is in the HTTP response. If we also accepted options
 		// from the client, then that precondition failure should surface in the response.