From 92d6796ad982d13b680aee64006284c04ff7cb74 Mon Sep 17 00:00:00 2001 From: Catherine Date: Thu, 4 Dec 2025 00:54:19 +0000 Subject: [PATCH] Return both `LastModified` and `ETag` in manifest metadata. NFCI --- src/backend.go | 11 ++++++++++- src/backend_fs.go | 6 ++++-- src/backend_s3.go | 19 +++++++++++-------- src/collect.go | 12 ++++++------ src/main.go | 4 ++-- src/observe.go | 4 ++-- src/pages.go | 27 +++++++++++++++------------ src/update.go | 4 ++-- 8 files changed, 52 insertions(+), 35 deletions(-) diff --git a/src/backend.go b/src/backend.go index 444fee8..cf72c96 100644 --- a/src/backend.go +++ b/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 diff --git a/src/backend_fs.go b/src/backend_fs.go index a916744..52ad790 100644 --- a/src/backend_fs.go +++ b/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 { diff --git a/src/backend_s3.go b/src/backend_s3.go index 6f27055..a9e470b 100644 --- a/src/backend_s3.go +++ b/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 } } diff --git a/src/collect.go b/src/collect.go index b76cec5..4c16f37 100644 --- a/src/collect.go +++ b/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 diff --git a/src/main.go b/src/main.go index 5ac2d48..d365fac 100644 --- a/src/main.go +++ b/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 { diff --git a/src/observe.go b/src/observe.go index 5e06f0b..09c43aa 100644 --- a/src/observe.go +++ b/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() diff --git a/src/pages.go b/src/pages.go index 649eead..92e9bf3 100644 --- a/src/pages.go +++ b/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) diff --git a/src/update.go b/src/update.go index f0b54df..c0bcec1 100644 --- a/src/update.go +++ b/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.