diff --git a/Justfile b/Justfile index 62e456b..9f18a8a 100644 --- a/Justfile +++ b/Justfile @@ -10,15 +10,11 @@ test: # Lint code lint: - #!/bin/bash - golangci-lint run & - actionlint & - wait + golangci-lint run + actionlint # Format code fmt: - #!/bin/bash - just --unstable --fmt & - golangci-lint fmt & - go mod tidy & - wait + just --unstable --fmt + golangci-lint fmt + go mod tidy diff --git a/cmd/cachewd/main.go b/cmd/cachewd/main.go index 6b5c10a..e55785b 100644 --- a/cmd/cachewd/main.go +++ b/cmd/cachewd/main.go @@ -36,7 +36,8 @@ func main() { ctx := context.Background() logger, ctx := logging.Configure(ctx, cli.LoggingConfig) - switch { + // Commands + switch { //nolint:gocritic case cli.Schema: schema := config.Schema() slices.SortStableFunc(schema.Entries, func(a, b hcl.Entry) int { @@ -49,7 +50,7 @@ func main() { err = quick.Highlight(os.Stdout, string(text), "terraform", "terminal256", "solarized") kctx.FatalIfErrorf(err) } else { - fmt.Printf("%s\n", text) + fmt.Printf("%s\n", text) //nolint:forbidigo } return } diff --git a/internal/cache/api.go b/internal/cache/api.go index d69179c..b7dd930 100644 --- a/internal/cache/api.go +++ b/internal/cache/api.go @@ -33,7 +33,7 @@ func Register[Config any, C Cache](id, description string, factory Factory[Confi if err != nil { panic(err) } - block := schema.Entries[0].(*hcl.Block) + block := schema.Entries[0].(*hcl.Block) //nolint:errcheck // This seems spurious block.Comments = hcl.CommentList{description} registry[id] = registryEntry{ schema: block, @@ -124,6 +124,7 @@ type Cache interface { // Open an existing file in the cache. // // Expired files MUST NOT be returned. + // The returned headers MUST include a Last-Modified header. // Must return os.ErrNotExist if the file does not exist. Open(ctx context.Context, key Key) (io.ReadCloser, textproto.MIMEHeader, error) // Create a new file in the cache. diff --git a/internal/cache/cachetest/suite.go b/internal/cache/cachetest/suite.go index 9bdc6cf..ed4e2c9 100644 --- a/internal/cache/cachetest/suite.go +++ b/internal/cache/cachetest/suite.go @@ -3,6 +3,7 @@ package cachetest import ( "context" "io" + "net/http" "net/textproto" "os" "testing" @@ -51,6 +52,10 @@ func Suite(t *testing.T, newCache func(t *testing.T) cache.Cache) { t.Run("ContextCancellation", func(t *testing.T) { testContextCancellation(t, newCache(t)) }) + + t.Run("LastModified", func(t *testing.T) { + testLastModified(t, newCache(t)) + }) } func testCreateAndOpen(t *testing.T, c cache.Cache) { @@ -235,8 +240,13 @@ func testHeaders(t *testing.T, c cache.Cache) { assert.NoError(t, err) assert.Equal(t, "test data with headers", string(data)) - // Verify headers - assert.Equal(t, headers, returnedHeaders) + // Verify headers that were passed in are present + assert.Equal(t, "application/json", returnedHeaders.Get("Content-Type")) + assert.Equal(t, "max-age=3600", returnedHeaders.Get("Cache-Control")) + assert.Equal(t, "custom-value", returnedHeaders.Get("X-Custom-Field")) + + // Verify Last-Modified header was added + assert.NotZero(t, returnedHeaders.Get("Last-Modified")) } func testContextCancellation(t *testing.T, c cache.Cache) { @@ -267,3 +277,56 @@ func testContextCancellation(t *testing.T, c cache.Cache) { _, _, err = c.Open(ctx, key) assert.IsError(t, err, os.ErrNotExist) } + +func testLastModified(t *testing.T, c cache.Cache) { + defer c.Close() + ctx := t.Context() + + key := cache.NewKey("test-last-modified") + + // Create an object without specifying Last-Modified + writer, err := c.Create(ctx, key, nil, time.Hour) + assert.NoError(t, err) + + _, err = writer.Write([]byte("test data")) + assert.NoError(t, err) + + err = writer.Close() + assert.NoError(t, err) + + // Open and verify Last-Modified header is present + reader, headers, err := c.Open(ctx, key) + assert.NoError(t, err) + defer reader.Close() + + lastModified := headers.Get("Last-Modified") + assert.NotZero(t, lastModified, "Last-Modified header should be set") + + // Verify it can be parsed as an HTTP date + parsedTime, err := http.ParseTime(lastModified) + assert.NoError(t, err) + assert.True(t, parsedTime.Before(time.Now().Add(time.Second)), "Last-Modified should be in the past") + + // Test with explicit Last-Modified header + key2 := cache.NewKey("test-last-modified-explicit") + explicitTime := time.Date(2023, 1, 15, 12, 30, 0, 0, time.UTC) + explicitHeaders := textproto.MIMEHeader{ + "Last-Modified": []string{explicitTime.Format(http.TimeFormat)}, + } + + writer2, err := c.Create(ctx, key2, explicitHeaders, time.Hour) + assert.NoError(t, err) + + _, err = writer2.Write([]byte("test data 2")) + assert.NoError(t, err) + + err = writer2.Close() + assert.NoError(t, err) + + // Verify explicit Last-Modified is preserved + reader2, headers2, err := c.Open(ctx, key2) + assert.NoError(t, err) + defer reader2.Close() + + assert.Equal(t, explicitTime.Format(http.TimeFormat), headers2.Get("Last-Modified")) +} diff --git a/internal/cache/disk.go b/internal/cache/disk.go index f97dd1e..4e1b7d2 100644 --- a/internal/cache/disk.go +++ b/internal/cache/disk.go @@ -5,6 +5,8 @@ import ( "io" "io/fs" "log/slog" + "maps" + "net/http" "net/textproto" "os" "path/filepath" @@ -133,6 +135,14 @@ func (d *Disk) Create(ctx context.Context, key Key, headers textproto.MIMEHeader ttl = d.config.MaxTTL } + now := time.Now() + // Clone headers to avoid concurrent map writes + clonedHeaders := make(textproto.MIMEHeader) + maps.Copy(clonedHeaders, headers) + if clonedHeaders.Get("Last-Modified") == "" { + clonedHeaders.Set("Last-Modified", now.UTC().Format(http.TimeFormat)) + } + path := d.keyToPath(key) fullPath := filepath.Join(d.config.Root, path) @@ -147,7 +157,7 @@ func (d *Disk) Create(ctx context.Context, key Key, headers textproto.MIMEHeader return nil, errors.Errorf("failed to create temp file: %w", err) } - expiresAt := time.Now().Add(ttl) + expiresAt := now.Add(ttl) return &diskWriter{ disk: d, @@ -156,7 +166,7 @@ func (d *Disk) Create(ctx context.Context, key Key, headers textproto.MIMEHeader path: fullPath, tempPath: tempPath, expiresAt: expiresAt, - headers: headers, + headers: clonedHeaders, ctx: ctx, }, nil } diff --git a/internal/cache/memory.go b/internal/cache/memory.go index 703183c..4e0ad89 100644 --- a/internal/cache/memory.go +++ b/internal/cache/memory.go @@ -5,6 +5,8 @@ import ( "context" "fmt" "io" + "maps" + "net/http" "net/textproto" "os" "sync" @@ -88,12 +90,20 @@ func (m *Memory) Create(ctx context.Context, key Key, headers textproto.MIMEHead ttl = m.config.MaxTTL } + now := time.Now() + // Clone headers to avoid concurrent map writes + clonedHeaders := make(textproto.MIMEHeader) + maps.Copy(clonedHeaders, headers) + if clonedHeaders.Get("Last-Modified") == "" { + clonedHeaders.Set("Last-Modified", now.UTC().Format(http.TimeFormat)) + } + writer := &memoryWriter{ cache: m, key: key, buf: &bytes.Buffer{}, - expiresAt: time.Now().Add(ttl), - headers: headers, + expiresAt: now.Add(ttl), + headers: clonedHeaders, ctx: ctx, } diff --git a/internal/cache/s3.go b/internal/cache/s3.go index 03d6ef8..b66d2b1 100644 --- a/internal/cache/s3.go +++ b/internal/cache/s3.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "log/slog" + "maps" "net/http" "net/textproto" "os" @@ -196,16 +197,50 @@ func (s *S3) Stat(ctx context.Context, key Key) (textproto.MIMEHeader, error) { } } + // Add Last-Modified header from S3 object metadata if not already present + if headers.Get("Last-Modified") == "" && !objInfo.LastModified.IsZero() { + headers.Set("Last-Modified", objInfo.LastModified.UTC().Format(http.TimeFormat)) + } + return headers, nil } func (s *S3) Open(ctx context.Context, key Key) (io.ReadCloser, textproto.MIMEHeader, error) { - headers, err := s.Stat(ctx, key) + objectName := s.keyToPath(key) + + // Get object info to retrieve metadata and check expiration + objInfo, err := s.client.StatObject(ctx, s.config.Bucket, objectName, minio.StatObjectOptions{}) if err != nil { - return nil, nil, errors.WithStack(err) + errResponse := minio.ToErrorResponse(err) + if errResponse.Code == "NoSuchKey" { + return nil, nil, os.ErrNotExist + } + return nil, nil, errors.Errorf("failed to stat object: %w", err) } - objectName := s.keyToPath(key) + // Check if object has expired + expiresAtStr := objInfo.UserMetadata["Expires-At"] + if expiresAtStr != "" { + var expiresAt time.Time + if err := expiresAt.UnmarshalText([]byte(expiresAtStr)); err == nil { + if time.Now().After(expiresAt) { + return nil, nil, errors.Join(os.ErrNotExist, s.Delete(ctx, key)) + } + } + } + + // Retrieve headers from metadata + headers := make(textproto.MIMEHeader) + if headersJSON := objInfo.UserMetadata["Headers"]; headersJSON != "" { + if err := json.Unmarshal([]byte(headersJSON), &headers); err != nil { + return nil, nil, errors.Errorf("failed to unmarshal headers: %w", err) + } + } + + // Add Last-Modified header from S3 object metadata if not already present + if headers.Get("Last-Modified") == "" && !objInfo.LastModified.IsZero() { + headers.Set("Last-Modified", objInfo.LastModified.UTC().Format(http.TimeFormat)) + } // Get object obj, err := s.client.GetObject(ctx, s.config.Bucket, objectName, minio.GetObjectOptions{}) @@ -221,6 +256,10 @@ func (s *S3) Create(ctx context.Context, key Key, headers textproto.MIMEHeader, ttl = s.config.MaxTTL } + // Clone headers to avoid concurrent access issues + clonedHeaders := make(textproto.MIMEHeader) + maps.Copy(clonedHeaders, headers) + expiresAt := time.Now().Add(ttl) pr, pw := io.Pipe() @@ -230,7 +269,7 @@ func (s *S3) Create(ctx context.Context, key Key, headers textproto.MIMEHeader, key: key, pipe: pw, expiresAt: expiresAt, - headers: headers, + headers: clonedHeaders, ctx: ctx, errCh: make(chan error, 1), } diff --git a/internal/strategy/api.go b/internal/strategy/api.go index f00308f..c802a83 100644 --- a/internal/strategy/api.go +++ b/internal/strategy/api.go @@ -36,7 +36,7 @@ func Register[Config any, S Strategy](id, description string, factory Factory[Co if err != nil { panic(err) } - block := schema.Entries[0].(*hcl.Block) + block := schema.Entries[0].(*hcl.Block) //nolint:errcheck // This seems spurious block.Comments = hcl.CommentList{description} registry[id] = registryEntry{ schema: block, diff --git a/internal/strategy/gomod.go b/internal/strategy/gomod.go index 2380f85..7b066e9 100644 --- a/internal/strategy/gomod.go +++ b/internal/strategy/gomod.go @@ -55,7 +55,7 @@ type GoMod struct { var _ Strategy = (*GoMod)(nil) // NewGoMod creates a new Go module proxy strategy. -func NewGoMod(ctx context.Context, config GoModConfig, scheduler jobscheduler.Scheduler, cache cache.Cache, mux Mux) (*GoMod, error) { +func NewGoMod(ctx context.Context, config GoModConfig, _ jobscheduler.Scheduler, cache cache.Cache, mux Mux) (*GoMod, error) { parsedURL, err := url.Parse(config.Proxy) if err != nil { return nil, fmt.Errorf("invalid proxy URL: %w", err)