Cache puppet virtual module misses; drop remote query forwarding

docs/puppet/remote.md

@@ -5,6 +5,3 @@ Shared classification, TTL and eviction rules: [caching](../common/caching.md).
 | Type | Mutable (auto-detected) | Immutable (auto-detected) |
 |---|---|---|
 | `puppet` | `v3/modules/*`, `v3/releases*` | `.tar.gz` |
-
-`/v3/releases` forwards the `module`, `limit` and `offset` query params
-upstream; set `base_url` to the Forge API root, e.g. `https://forgeapi.puppet.com`.

docs/puppet/virtual.md

@@ -13,7 +13,7 @@ dependencies resolved across members.
 - `/v3/releases/<slug>-<version>` and `/v3/files/<file>` 302 to the member
   owning that release.
 - A member error fails the request with 502; a member without the module is
-  skipped. Merges are cached for 60s.
+  skipped. Merges, and modules no member has, are cached for 60s.
 
 ```hcl
 resource "artifactapi_virtual" "puppet" {

internal/provider/puppet/puppet.go

@@ -3,7 +3,6 @@ package puppet
 import (
 	"context"
 	"net/http"
-	"strconv"
 	"strings"
 
 	"git.unkin.net/unkin/artifactapi/internal/auth"
@@ -40,32 +39,6 @@ func (p *Provider) UpstreamURL(remote models.Remote, path string) string {
 	return strings.TrimRight(remote.BaseURL, "/") + "/" + strings.TrimLeft(path, "/")
 }
 
-// ForwardedQuery keeps the module filter and pagination on release listings.
-func (p *Provider) ForwardedQuery(path string) map[string]provider.QueryValidator {
-	if path != "v3/releases" {
-		return nil
-	}
-	return map[string]provider.QueryValidator{"module": moduleParam, "limit": pageLimit, "offset": pageOffset}
-}
-
-func moduleParam(v string) (string, bool) {
-	v = NormalizeSlug(v)
-	return v, ValidSlug(v)
-}
-
-func pageLimit(v string) (string, bool) {
-	n, err := strconv.ParseUint(v, 10, 32)
-	if err != nil || n == 0 {
-		return "", false
-	}
-	return strconv.FormatUint(min(n, 100), 10), true
-}
-
-func pageOffset(v string) (string, bool) {
-	n, err := strconv.ParseUint(v, 10, 32)
-	return strconv.FormatUint(n, 10), err == nil
-}
-
 func (p *Provider) RewritesResponse() {}
 
 func (p *Provider) RewriteResponse(body []byte, remote models.Remote, proxyBaseURL string) ([]byte, error) {

internal/provider/puppet/puppet_test.go

@@ -76,28 +76,3 @@ func TestAuthHeaders(t *testing.T) {
 		t.Error("no credentials, no header")
 	}
 }
-
-func TestForwardedQuery(t *testing.T) {
-	p := &Provider{}
-	if p.ForwardedQuery("v3/files/a-b-1.0.0.tar.gz") != nil || p.ForwardedQuery("v3/modules/a-b") != nil {
-		t.Fatal("only release listings forward a query")
-	}
-	f := p.ForwardedQuery("v3/releases")
-	for name, c := range map[string]struct {
-		in, want string
-		ok       bool
-	}{
-		"module": {"Puppetlabs/stdlib", "puppetlabs-stdlib", true},
-		"limit":  {"500", "100", true},
-		"offset": {"40", "40", true},
-	} {
-		if got, ok := f[name](c.in); got != c.want || ok != c.ok {
-			t.Errorf("%s(%q) = %q %v, want %q %v", name, c.in, got, ok, c.want, c.ok)
-		}
-	}
-	for name, bad := range map[string]string{"module": "../x", "limit": "0", "offset": "-1"} {
-		if _, ok := f[name](bad); ok {
-			t.Errorf("%s(%q) accepted", name, bad)
-		}
-	}
-}

internal/virtual/puppet_merger.go

@@ -34,11 +34,13 @@ type PuppetMember struct {
 }
 
 // PuppetDoc is a module's merged releases, newest version first, with
-// file_uri routed via the virtual; Owners maps each version to its member.
+// file_uri routed via the virtual; Owners maps each version to its member;
+// missing caches that no member has the module.
 type PuppetDoc struct {
 	Releases []map[string]any
 	Owners   map[string]string
 	body     []byte
+	missing  bool
 }
 
 func (d *PuppetDoc) index() []byte            { return d.body }
@@ -114,7 +116,11 @@ func (e *Engine) PuppetReleases(ctx context.Context, virt models.Virtual, slug s
 		if err != nil {
 			return nil, err
 		}
-		return MergePuppetReleases(slug, members)
+		d, err := MergePuppetReleases(slug, members)
+		if errors.Is(err, ErrNotFound) {
+			return &PuppetDoc{missing: true}, nil
+		}
+		return d, err
 	})
 	if err != nil {
 		return nil, err
@@ -122,7 +128,10 @@ func (e *Engine) PuppetReleases(ctx context.Context, virt models.Virtual, slug s
 	if inserted {
 		e.evictOldest(prefix, maxPuppetModules)
 	}
-	return repo.(*PuppetDoc), nil
+	if d := repo.(*PuppetDoc); !d.missing {
+		return d, nil
+	}
+	return nil, ErrNotFound
 }
 
 // PuppetRedirect maps v3/releases/<slug>-<version> or v3/files/<file> to the

internal/virtual/puppet_merger_test.go

@@ -180,3 +180,26 @@ func TestPuppetRedirect(t *testing.T) {
 		}
 	}
 }
+
+func TestPuppetReleasesCachesMissingModule(t *testing.T) {
+	var mu sync.Mutex
+	asked := map[string]int{}
+	e := puppetEngine(func(r models.Remote, p string) ([]byte, error) {
+		mu.Lock()
+		asked[r.Name]++
+		mu.Unlock()
+		if r.Name == "internal" {
+			return nil, &proxy.ProxyError{Status: 404}
+		}
+		return forgePages("unkin-nope", nil, p)
+	})
+	virt := models.Virtual{Name: "puppet", PackageType: models.PackagePuppet, Members: []string{"internal", "forge"}}
+	for range 2 {
+		if _, err := e.PuppetReleases(context.Background(), virt, "unkin-nope"); !errors.Is(err, ErrNotFound) {
+			t.Fatalf("want ErrNotFound, got %v", err)
+		}
+	}
+	if asked["internal"] != 1 || asked["forge"] != 1 {
+		t.Fatalf("a module no member has must be cached, asked %v", asked)
+	}
+}