refactor: cleanup types, names, gateway

Author: hacdias

CHANGELOG.md

@@ -17,6 +17,8 @@ The following emojis are used to highlight certain changes:
 ### Added
 
 * ✨ The `routing/http` implements Delegated Peer Routing introduced in [IPIP-417](https://github.com/ipfs/specs/pull/417).
+* The gateway now sets a `Cache-Control` header for requests under the `/ipns/` namespace
+  if the TTL for the corresponding IPNS Records or DNSLink entities is known.
 
 ### Changed
 
@@ -27,14 +29,28 @@ The following emojis are used to highlight certain changes:
   * `ReadBitswapProviderRecord` has been renamed to `BitswapRecord` and marked as deprecated.
     From now on, please use the protocol-agnostic `PeerRecord` for most use cases. The new
     Peer Schema has been introduced in [IPIP-417](https://github.com/ipfs/specs/pull/417).
-
+* 🛠 The `namesys` package has been refactored. The following are the largest modifications:
+  * The options in `coreiface/options/namesys` have been moved to `namesys` and their names
+    have been made more consistent.
+  * Many of the exported structs and functions have been renamed in order to be consistent with
+    the remaining packages.
+  * `namesys.Resolver.Resolve` now returns a TTL, in addition to the resolved path. If the
+    TTL is unknown, 0 is returned. `IPNSResolver` is able to resolve a TTL, while `DNSResolver`
+    is not.
+  * `namesys/resolver.ResolveIPNS` has been moved to `namesys.ResolveIPNS` and now returns a TTL
+    in addition to the resolved path.
+* 🛠 The `gateway`'s `IPFSBackend.ResolveMutable` is now expected to return a TTL in addition to
+    the resolved path. If the TTL is unknown, 0 should be returned.
+ 
 ### Removed
 
 * 🛠 The `routing/http` package experienced following removals:
   * Server and client no longer support the experimental `Provide` method.
     `ProvideBitswap` is still usable, but marked as deprecated. A protocol-agnostic 
     provide mechanism is being worked on in [IPIP-378](https://github.com/ipfs/specs/pull/378).
   * Server no longer exports `FindProvidersPath` and `ProvidePath`.
+* 🛠 The `coreiface/options/namesys` package has been removed.
+* 🛠 The `namesys.StartSpan` function is no longer exported.
 
 ### Fixed
 

gateway/blocks_backend.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	gopath "path"
 	"strings"
+	"time"
 
 	"github.com/ipfs/boxo/blockservice"
 	blockstore "github.com/ipfs/boxo/blockstore"
@@ -19,8 +20,8 @@ import (
 	"github.com/ipfs/boxo/ipld/merkledag"
 	ufile "github.com/ipfs/boxo/ipld/unixfs/file"
 	uio "github.com/ipfs/boxo/ipld/unixfs/io"
+	"github.com/ipfs/boxo/ipns"
 	"github.com/ipfs/boxo/namesys"
-	"github.com/ipfs/boxo/namesys/resolve"
 	ipfspath "github.com/ipfs/boxo/path"
 	"github.com/ipfs/boxo/path/resolver"
 	blocks "github.com/ipfs/go-block-format"
@@ -40,7 +41,6 @@ import (
 	"github.com/ipld/go-ipld-prime/traversal/selector"
 	selectorparse "github.com/ipld/go-ipld-prime/traversal/selector/parse"
 	routinghelpers "github.com/libp2p/go-libp2p-routing-helpers"
-	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/libp2p/go-libp2p/core/routing"
 	mc "github.com/multiformats/go-multicodec"
 
@@ -556,32 +556,32 @@ func (bb *BlocksBackend) getPathRoots(ctx context.Context, contentPath Immutable
 	return pathRoots, lastPath, nil
 }
 
-func (bb *BlocksBackend) ResolveMutable(ctx context.Context, p ifacepath.Path) (ImmutablePath, error) {
+func (bb *BlocksBackend) ResolveMutable(ctx context.Context, p ifacepath.Path) (ImmutablePath, time.Duration, error) {
 	err := p.IsValid()
 	if err != nil {
-		return ImmutablePath{}, err
+		return ImmutablePath{}, 0, err
 	}
 
 	ipath := ipfspath.Path(p.String())
 	switch ipath.Segments()[0] {
 	case "ipns":
-		ipath, err = resolve.ResolveIPNS(ctx, bb.namesys, ipath)
+		ipath, ttl, err := namesys.ResolveIPNS(ctx, bb.namesys, ipath)
 		if err != nil {
-			return ImmutablePath{}, err
+			return ImmutablePath{}, 0, err
 		}
 		imPath, err := NewImmutablePath(ifacepath.New(ipath.String()))
 		if err != nil {
-			return ImmutablePath{}, err
+			return ImmutablePath{}, 0, err
 		}
-		return imPath, nil
+		return imPath, ttl, nil
 	case "ipfs":
 		imPath, err := NewImmutablePath(ifacepath.New(ipath.String()))
 		if err != nil {
-			return ImmutablePath{}, err
+			return ImmutablePath{}, 0, err
 		}
-		return imPath, nil
+		return imPath, 0, nil
 	default:
-		return ImmutablePath{}, NewErrorStatusCode(fmt.Errorf("unsupported path namespace: %s", p.Namespace()), http.StatusNotImplemented)
+		return ImmutablePath{}, 0, NewErrorStatusCode(fmt.Errorf("unsupported path namespace: %s", p.Namespace()), http.StatusNotImplemented)
 	}
 }
 
@@ -590,19 +590,12 @@ func (bb *BlocksBackend) GetIPNSRecord(ctx context.Context, c cid.Cid) ([]byte,
 		return nil, NewErrorStatusCode(errors.New("IPNS Record responses are not supported by this gateway"), http.StatusNotImplemented)
 	}
 
-	// Fails fast if the CID is not an encoded Libp2p Key, avoids wasteful
-	// round trips to the remote routing provider.
-	if mc.Code(c.Type()) != mc.Libp2pKey {
-		return nil, NewErrorStatusCode(errors.New("cid codec must be libp2p-key"), http.StatusBadRequest)
-	}
-
-	// The value store expects the key itself to be encoded as a multihash.
-	id, err := peer.FromCid(c)
+	name, err := ipns.NameFromCid(c)
 	if err != nil {
-		return nil, err
+		return nil, NewErrorStatusCode(err, http.StatusBadRequest)
 	}
 
-	return bb.routing.GetValue(ctx, "/ipns/"+string(id))
+	return bb.routing.GetValue(ctx, string(name.RoutingKey()))
 }
 
 func (bb *BlocksBackend) GetDNSLinkRecord(ctx context.Context, hostname string) (ifacepath.Path, error) {
@@ -651,7 +644,7 @@ func (bb *BlocksBackend) resolvePath(ctx context.Context, p ifacepath.Path) (ifa
 
 	ipath := ipfspath.Path(p.String())
 	if ipath.Segments()[0] == "ipns" {
-		ipath, err = resolve.ResolveIPNS(ctx, bb.namesys, ipath)
+		ipath, _, err = namesys.ResolveIPNS(ctx, bb.namesys, ipath)
 		if err != nil {
 			return nil, err
 		}

gateway/gateway.go

@@ -8,6 +8,7 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/ipfs/boxo/coreiface/path"
 	"github.com/ipfs/boxo/files"
@@ -335,11 +336,11 @@ type IPFSBackend interface {
 	GetIPNSRecord(context.Context, cid.Cid) ([]byte, error)
 
 	// ResolveMutable takes a mutable path and resolves it into an immutable one. This means recursively resolving any
-	// DNSLink or IPNS records.
+	// DNSLink or IPNS records. It should also return a TTL. If the TTL is unknown, 0 should be returned.
 	//
 	// For example, given a mapping from `/ipns/dnslink.tld -> /ipns/ipns-id/mydirectory` and `/ipns/ipns-id` to
 	// `/ipfs/some-cid`, the result of passing `/ipns/dnslink.tld/myfile` would be `/ipfs/some-cid/mydirectory/myfile`.
-	ResolveMutable(context.Context, path.Path) (ImmutablePath, error)
+	ResolveMutable(context.Context, path.Path) (ImmutablePath, time.Duration, error)
 
 	// GetDNSLinkRecord returns the DNSLink TXT record for the provided FQDN.
 	// Unlike ResolvePath, it does not perform recursive resolution. It only

gateway/gateway_test.go

@@ -29,19 +29,19 @@ func TestGatewayGet(t *testing.T) {
 	k, err := backend.resolvePathNoRootsReturned(ctx, ipath.Join(ipath.IpfsPath(root), "subdir", "fnord"))
 	require.NoError(t, err)
 
-	backend.namesys["/ipns/example.com"] = path.FromCid(k.Cid())
-	backend.namesys["/ipns/working.example.com"] = path.FromString(k.String())
-	backend.namesys["/ipns/double.example.com"] = path.FromString("/ipns/working.example.com")
-	backend.namesys["/ipns/triple.example.com"] = path.FromString("/ipns/double.example.com")
-	backend.namesys["/ipns/broken.example.com"] = path.FromString("/ipns/" + k.Cid().String())
+	backend.namesys["/ipns/example.com"] = newMockNamesysItem(path.FromCid(k.Cid()), 0)
+	backend.namesys["/ipns/working.example.com"] = newMockNamesysItem(path.FromString(k.String()), 0)
+	backend.namesys["/ipns/double.example.com"] = newMockNamesysItem(path.FromString("/ipns/working.example.com"), 0)
+	backend.namesys["/ipns/triple.example.com"] = newMockNamesysItem(path.FromString("/ipns/double.example.com"), 0)
+	backend.namesys["/ipns/broken.example.com"] = newMockNamesysItem(path.FromString("/ipns/"+k.Cid().String()), 0)
 	// We picked .man because:
 	// 1. It's a valid TLD.
 	// 2. Go treats it as the file extension for "man" files (even though
 	//    nobody actually *uses* this extension, AFAIK).
 	//
 	// Unfortunately, this may not work on all platforms as file type
 	// detection is platform dependent.
-	backend.namesys["/ipns/example.man"] = path.FromString(k.String())
+	backend.namesys["/ipns/example.man"] = newMockNamesysItem(path.FromString(k.String()), 0)
 
 	for _, test := range []struct {
 		host   string
@@ -90,7 +90,7 @@ func TestPretty404(t *testing.T) {
 	t.Logf("test server url: %s", ts.URL)
 
 	host := "example.net"
-	backend.namesys["/ipns/"+host] = path.FromCid(root)
+	backend.namesys["/ipns/"+host] = newMockNamesysItem(path.FromCid(root), 0)
 
 	for _, test := range []struct {
 		path   string
@@ -150,7 +150,56 @@ func TestHeaders(t *testing.T) {
 		dagCborRoots = dirRoots + "," + dagCborCID
 	)
 
-	t.Run("Cache-Control is not immutable on generated /ipfs/  HTML dir listings", func(t *testing.T) {
+	t.Run("Cache-Control uses TTL for /ipns/ when it is known", func(t *testing.T) {
+		t.Parallel()
+
+		ts, backend, root := newTestServerAndNode(t, nil, "ipns-hostname-redirects.car")
+		backend.namesys["/ipns/example.net"] = newMockNamesysItem(path.FromCid(root), time.Second*30)
+
+		t.Run("UnixFS generated directory listing without index.html has no Cache-Control", func(t *testing.T) {
+			req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipns/example.net/", nil)
+			res := mustDoWithoutRedirect(t, req)
+			require.Empty(t, res.Header["Cache-Control"])
+		})
+
+		t.Run("UnixFS directory with index.html has Cache-Control", func(t *testing.T) {
+			req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipns/example.net/foo/", nil)
+			res := mustDoWithoutRedirect(t, req)
+			require.Equal(t, "public, max-age=30", res.Header.Get("Cache-Control"))
+		})
+
+		t.Run("UnixFS file has Cache-Control", func(t *testing.T) {
+			req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipns/example.net/foo/index.html", nil)
+			res := mustDoWithoutRedirect(t, req)
+			require.Equal(t, "public, max-age=30", res.Header.Get("Cache-Control"))
+		})
+
+		t.Run("Raw block has Cache-Control", func(t *testing.T) {
+			req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipns/example.net?format=raw", nil)
+			res := mustDoWithoutRedirect(t, req)
+			require.Equal(t, "public, max-age=30", res.Header.Get("Cache-Control"))
+		})
+
+		t.Run("DAG-JSON block has Cache-Control", func(t *testing.T) {
+			req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipns/example.net?format=dag-json", nil)
+			res := mustDoWithoutRedirect(t, req)
+			require.Equal(t, "public, max-age=30", res.Header.Get("Cache-Control"))
+		})
+
+		t.Run("DAG-CBOR block has Cache-Control", func(t *testing.T) {
+			req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipns/example.net?format=dag-cbor", nil)
+			res := mustDoWithoutRedirect(t, req)
+			require.Equal(t, "public, max-age=30", res.Header.Get("Cache-Control"))
+		})
+
+		t.Run("CAR block has Cache-Control", func(t *testing.T) {
+			req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipns/example.net?format=car", nil)
+			res := mustDoWithoutRedirect(t, req)
+			require.Equal(t, "public, max-age=30", res.Header.Get("Cache-Control"))
+		})
+	})
+
+	t.Run("Cache-Control is not immutable on generated /ipfs/ HTML dir listings", func(t *testing.T) {
 		req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipfs/"+rootCID+"/", nil)
 		res := mustDoWithoutRedirect(t, req)
 
@@ -492,7 +541,7 @@ func TestRedirects(t *testing.T) {
 		t.Parallel()
 
 		ts, backend, root := newTestServerAndNode(t, nil, "ipns-hostname-redirects.car")
-		backend.namesys["/ipns/example.net"] = path.FromCid(root)
+		backend.namesys["/ipns/example.net"] = newMockNamesysItem(path.FromCid(root), 0)
 
 		// make request to directory containing index.html
 		req := mustNewRequest(t, http.MethodGet, ts.URL+"/foo", nil)
@@ -527,7 +576,7 @@ func TestRedirects(t *testing.T) {
 		t.Parallel()
 
 		backend, root := newMockBackend(t, "redirects-spa.car")
-		backend.namesys["/ipns/example.com"] = path.FromCid(root)
+		backend.namesys["/ipns/example.com"] = newMockNamesysItem(path.FromCid(root), 0)
 
 		ts := newTestServerWithConfig(t, backend, Config{
 			Headers:   map[string][]string{},
@@ -664,8 +713,8 @@ func TestDeserializedResponses(t *testing.T) {
 		t.Parallel()
 
 		backend, root := newMockBackend(t, "fixtures.car")
-		backend.namesys["/ipns/trustless.com"] = path.FromCid(root)
-		backend.namesys["/ipns/trusted.com"] = path.FromCid(root)
+		backend.namesys["/ipns/trustless.com"] = newMockNamesysItem(path.FromCid(root), 0)
+		backend.namesys["/ipns/trusted.com"] = newMockNamesysItem(path.FromCid(root), 0)
 
 		ts := newTestServerWithConfig(t, backend, Config{
 			Headers:   map[string][]string{},
@@ -727,8 +776,8 @@ func (mb *errorMockBackend) GetCAR(ctx context.Context, path ImmutablePath, para
 	return ContentPathMetadata{}, nil, mb.err
 }
 
-func (mb *errorMockBackend) ResolveMutable(ctx context.Context, path ipath.Path) (ImmutablePath, error) {
-	return ImmutablePath{}, mb.err
+func (mb *errorMockBackend) ResolveMutable(ctx context.Context, path ipath.Path) (ImmutablePath, time.Duration, error) {
+	return ImmutablePath{}, 0, mb.err
 }
 
 func (mb *errorMockBackend) GetIPNSRecord(ctx context.Context, c cid.Cid) ([]byte, error) {
@@ -811,7 +860,7 @@ func (mb *panicMockBackend) GetCAR(ctx context.Context, immutablePath ImmutableP
 	panic("i am panicking")
 }
 
-func (mb *panicMockBackend) ResolveMutable(ctx context.Context, p ipath.Path) (ImmutablePath, error) {
+func (mb *panicMockBackend) ResolveMutable(ctx context.Context, p ipath.Path) (ImmutablePath, time.Duration, error) {
 	panic("i am panicking")
 }
 

gateway/handler.go

@@ -34,7 +34,7 @@ var log = logging.Logger("boxo/gateway")
 
 const (
 	ipfsPathPrefix        = "/ipfs/"
-	ipnsPathPrefix        = "/ipns/"
+	ipnsPathPrefix        = ipns.NamespacePrefix
 	immutableCacheControl = "public, max-age=29030400, immutable"
 )
 
@@ -188,6 +188,7 @@ type requestData struct {
 
 	// Defined for non IPNS Record requests.
 	immutablePath ImmutablePath
+	ttl           time.Duration
 
 	// Defined if resolution has already happened.
 	pathMetadata *ContentPathMetadata
@@ -279,7 +280,7 @@ func (i *handler) getOrHeadHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if contentPath.Mutable() {
-		rq.immutablePath, err = i.backend.ResolveMutable(r.Context(), contentPath)
+		rq.immutablePath, rq.ttl, err = i.backend.ResolveMutable(r.Context(), contentPath)
 		if err != nil {
 			err = fmt.Errorf("failed to resolve %s: %w", debugStr(contentPath.String()), err)
 			i.webError(w, r, err, http.StatusInternalServerError)
@@ -409,32 +410,30 @@ func panicHandler(w http.ResponseWriter) {
 	}
 }
 
-func addCacheControlHeaders(w http.ResponseWriter, r *http.Request, contentPath ipath.Path, cid cid.Cid, responseFormat string) (modtime time.Time) {
+func addCacheControlHeaders(w http.ResponseWriter, r *http.Request, contentPath ipath.Path, ttl time.Duration, cid cid.Cid, responseFormat string) (modtime time.Time) {
 	// Best effort attempt to set an Etag based on the CID and response format.
 	// Setting an ETag is handled separately for CARs and IPNS records.
 	if etag := getEtag(r, cid, responseFormat); etag != "" {
 		w.Header().Set("Etag", etag)
 	}
 
-	// Set Cache-Control and Last-Modified based on contentPath properties
+	// Set Cache-Control and Last-Modified based on contentPath properties.
 	if contentPath.Mutable() {
-		// mutable namespaces such as /ipns/ can't be cached forever
-
-		// For now we set Last-Modified to Now() to leverage caching heuristics built into modern browsers:
-		// https://github.com/ipfs/kubo/pull/8074#pullrequestreview-645196768
-		// but we should not set it to fake values and use Cache-Control based on TTL instead
-		modtime = time.Now()
-
-		// TODO: set Cache-Control based on TTL of IPNS/DNSLink: https://github.com/ipfs/kubo/issues/1818#issuecomment-1015849462
-		// TODO: set Last-Modified based on /ipns/ publishing timestamp?
+		if ttl > 0 {
+			// When we know the TTL, set the Cache-Control header and disable Last-Modified.
+			w.Header().Set("Cache-Control", fmt.Sprintf("public, max-age=%d", int(ttl.Seconds())))
+			modtime = noModtime
+		} else {
+			// Otherwise, we set Last-Modified to the current time to leverage caching heuristics
+			// built into modern browsers: https://github.com/ipfs/kubo/pull/8074#pullrequestreview-645196768
+			modtime = time.Now()
+		}
 	} else {
-		// immutable! CACHE ALL THE THINGS, FOREVER! wolololol
 		w.Header().Set("Cache-Control", immutableCacheControl)
+		modtime = noModtime // disable Last-Modified
 
-		// Set modtime to 'zero time' to disable Last-Modified header (superseded by Cache-Control)
-		modtime = noModtime
-
-		// TODO: set Last-Modified? - TBD - /ipfs/ modification metadata is present in unixfs 1.5 https://github.com/ipfs/kubo/issues/6920?
+		// TODO: consider setting Last-Modified if UnixFS V1.5 ever gets released
+		// with metadata: https://github.com/ipfs/kubo/issues/6920
 	}
 
 	return modtime

gateway/handler_block.go

@@ -34,7 +34,7 @@ func (i *handler) serveRawBlock(ctx context.Context, w http.ResponseWriter, r *h
 	setContentDispositionHeader(w, name, "attachment")
 
 	// Set remaining headers
-	modtime := addCacheControlHeaders(w, r, rq.contentPath, blockCid, rawResponseFormat)
+	modtime := addCacheControlHeaders(w, r, rq.contentPath, rq.ttl, blockCid, rawResponseFormat)
 	w.Header().Set("Content-Type", rawResponseFormat)
 	w.Header().Set("X-Content-Type-Options", "nosniff") // no funny business in the browsers :^)
 

gateway/handler_car.go

@@ -56,7 +56,7 @@ func (i *handler) serveCAR(ctx context.Context, w http.ResponseWriter, r *http.R
 	setContentDispositionHeader(w, name, "attachment")
 
 	// Set Cache-Control (same logic as for a regular files)
-	addCacheControlHeaders(w, r, rq.contentPath, rootCid, carResponseFormat)
+	addCacheControlHeaders(w, r, rq.contentPath, rq.ttl, rootCid, carResponseFormat)
 
 	// Generate the CAR Etag.
 	etag := getCarEtag(rq.immutablePath, params, rootCid)