From b335bdefd8ea32694c28c10069476362c9144b28 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Mon, 4 Sep 2023 17:12:43 +0200 Subject: [PATCH] feat: bubble TTL out of NameSys --- gateway/blocks_backend.go | 2 +- gateway/utilities_test.go | 15 +++++------ namesys/dns_resolver.go | 21 ++++++++-------- namesys/ipns_resolver.go | 18 ++++++------- namesys/ipns_resolver_test.go | 4 +-- namesys/mpns.go | 42 ++++++++++++++++--------------- namesys/namesys.go | 3 ++- namesys/namesys_test.go | 8 +++--- namesys/republisher/repub_test.go | 4 +-- namesys/resolve/resolve.go | 2 +- namesys/utilities.go | 34 ++++++++++++------------- 11 files changed, 78 insertions(+), 75 deletions(-) diff --git a/gateway/blocks_backend.go b/gateway/blocks_backend.go index 82c580680..8f582e50f 100644 --- a/gateway/blocks_backend.go +++ b/gateway/blocks_backend.go @@ -607,7 +607,7 @@ func (bb *BlocksBackend) GetIPNSRecord(ctx context.Context, c cid.Cid) ([]byte, func (bb *BlocksBackend) GetDNSLinkRecord(ctx context.Context, hostname string) (ifacepath.Path, error) { if bb.namesys != nil { - p, err := bb.namesys.Resolve(ctx, "/ipns/"+hostname, namesys.ResolveWithDepth(1)) + p, _, err := bb.namesys.Resolve(ctx, "/ipns/"+hostname, namesys.ResolveWithDepth(1)) if err == namesys.ErrResolveRecursion { err = nil } diff --git a/gateway/utilities_test.go b/gateway/utilities_test.go index d19ec961f..0a06505cc 100644 --- a/gateway/utilities_test.go +++ b/gateway/utilities_test.go @@ -11,6 +11,7 @@ import ( "regexp" "strings" "testing" + "time" "github.com/ipfs/boxo/blockservice" ipath "github.com/ipfs/boxo/coreiface/path" @@ -53,7 +54,7 @@ func mustDo(t *testing.T, req *http.Request) *http.Response { type mockNamesys map[string]path.Path -func (m mockNamesys) Resolve(ctx context.Context, name string, opts ...namesys.ResolveOption) (value path.Path, err error) { +func (m mockNamesys) Resolve(ctx context.Context, name string, opts ...namesys.ResolveOption) (value path.Path, ttl time.Duration, err error) { cfg := namesys.DefaultResolveOptions() for _, o := range opts { o(&cfg) @@ -65,24 +66,24 @@ func (m mockNamesys) Resolve(ctx context.Context, name string, opts ...namesys.R } for strings.HasPrefix(name, "/ipns/") { if depth == 0 { - return value, namesys.ErrResolveRecursion + return value, 0, namesys.ErrResolveRecursion } depth-- var ok bool value, ok = m[name] if !ok { - return "", namesys.ErrResolveFailed + return "", 0, namesys.ErrResolveFailed } name = value.String() } - return value, nil + return value, 0, nil } func (m mockNamesys) ResolveAsync(ctx context.Context, name string, opts ...namesys.ResolveOption) <-chan namesys.ResolveResult { out := make(chan namesys.ResolveResult, 1) - v, err := m.Resolve(ctx, name, opts...) - out <- namesys.ResolveResult{Path: v, Err: err} + v, ttl, err := m.Resolve(ctx, name, opts...) + out <- namesys.ResolveResult{Path: v, TTL: ttl, Err: err} close(out) return out } @@ -162,7 +163,7 @@ func (mb *mockBackend) GetIPNSRecord(ctx context.Context, c cid.Cid) ([]byte, er func (mb *mockBackend) GetDNSLinkRecord(ctx context.Context, hostname string) (ipath.Path, error) { if mb.namesys != nil { - p, err := mb.namesys.Resolve(ctx, "/ipns/"+hostname, namesys.ResolveWithDepth(1)) + p, _, err := mb.namesys.Resolve(ctx, "/ipns/"+hostname, namesys.ResolveWithDepth(1)) if err == namesys.ErrResolveRecursion { err = nil } diff --git a/namesys/dns_resolver.go b/namesys/dns_resolver.go index 128946aa9..6972e9d37 100644 --- a/namesys/dns_resolver.go +++ b/namesys/dns_resolver.go @@ -7,6 +7,7 @@ import ( "net" gpath "path" "strings" + "time" path "github.com/ipfs/boxo/path" dns "github.com/miekg/dns" @@ -31,7 +32,7 @@ func NewDNSResolver(lookup LookupTXTFunc) *DNSResolver { return &DNSResolver{lookupTXT: lookup} } -func (r *DNSResolver) Resolve(ctx context.Context, name string, options ...ResolveOption) (path.Path, error) { +func (r *DNSResolver) Resolve(ctx context.Context, name string, options ...ResolveOption) (path.Path, time.Duration, error) { ctx, span := startSpan(ctx, "DNSResolver.Resolve") defer span.End() @@ -45,17 +46,17 @@ func (r *DNSResolver) ResolveAsync(ctx context.Context, name string, options ... return resolveAsync(ctx, r, name, ProcessResolveOptions(options)) } -func (r *DNSResolver) resolveOnceAsync(ctx context.Context, name string, options ResolveOptions) <-chan onceResult { +func (r *DNSResolver) resolveOnceAsync(ctx context.Context, name string, options ResolveOptions) <-chan ResolveResult { ctx, span := startSpan(ctx, "DNSResolver.ResolveOnceAsync") defer span.End() var fqdn string - out := make(chan onceResult, 1) + out := make(chan ResolveResult, 1) segments := strings.SplitN(name, "/", 2) domain := segments[0] if _, ok := dns.IsDomainName(domain); !ok { - out <- onceResult{err: fmt.Errorf("not a valid domain name: %s", domain)} + out <- ResolveResult{Err: fmt.Errorf("not a valid domain name: %s", domain)} close(out) return out } @@ -95,7 +96,7 @@ func (r *DNSResolver) resolveOnceAsync(ctx context.Context, name string, options } if subRes.Err == nil { p, err := appendPath(subRes.Path) - emitOnceResult(ctx, out, onceResult{value: p, err: err}) + emitOnceResult(ctx, out, ResolveResult{Path: p, Err: err}) // Return without waiting for rootRes, since this result // (for "_dnslink."+fqdn) takes precedence return @@ -108,7 +109,7 @@ func (r *DNSResolver) resolveOnceAsync(ctx context.Context, name string, options } if rootRes.Err == nil { p, err := appendPath(rootRes.Path) - emitOnceResult(ctx, out, onceResult{value: p, err: err}) + emitOnceResult(ctx, out, ResolveResult{Path: p, Err: err}) // Do not return here. Wait for subRes so that it is // output last if good, thereby giving subRes precedence. } else { @@ -125,7 +126,7 @@ func (r *DNSResolver) resolveOnceAsync(ctx context.Context, name string, options if rootResErr == ErrResolveFailed && subResErr == ErrResolveFailed { // Wrap error so that it can be tested if it is a ErrResolveFailed err := fmt.Errorf("%w: _dnslink subdomain at %q is missing a TXT record (https://docs.ipfs.tech/concepts/dnslink/)", ErrResolveFailed, gpath.Base(name)) - emitOnceResult(ctx, out, onceResult{err: err}) + emitOnceResult(ctx, out, ResolveResult{Err: err}) } return } @@ -151,20 +152,20 @@ func workDomain(ctx context.Context, r *DNSResolver, name string, res chan Resol } } // Could not look up any text records for name - res <- ResolveResult{"", err} + res <- ResolveResult{Path: "", Err: err} return } for _, t := range txt { p, err := parseEntry(t) if err == nil { - res <- ResolveResult{p, nil} + res <- ResolveResult{Path: p, Err: nil} return } } // There were no TXT records with a dnslink - res <- ResolveResult{"", ErrResolveFailed} + res <- ResolveResult{Path: "", Err: ErrResolveFailed} } func parseEntry(txt string) (path.Path, error) { diff --git a/namesys/ipns_resolver.go b/namesys/ipns_resolver.go index 4f228af19..be4b0412b 100644 --- a/namesys/ipns_resolver.go +++ b/namesys/ipns_resolver.go @@ -33,7 +33,7 @@ func NewIPNSResolver(route routing.ValueStore) *IPNSResolver { } } -func (r *IPNSResolver) Resolve(ctx context.Context, name string, options ...ResolveOption) (path.Path, error) { +func (r *IPNSResolver) Resolve(ctx context.Context, name string, options ...ResolveOption) (path.Path, time.Duration, error) { ctx, span := startSpan(ctx, "IpnsResolver.Resolve", trace.WithAttributes(attribute.String("Name", name))) defer span.End() @@ -47,11 +47,11 @@ func (r *IPNSResolver) ResolveAsync(ctx context.Context, name string, options .. return resolveAsync(ctx, r, name, ProcessResolveOptions(options)) } -func (r *IPNSResolver) resolveOnceAsync(ctx context.Context, name string, options ResolveOptions) <-chan onceResult { +func (r *IPNSResolver) resolveOnceAsync(ctx context.Context, name string, options ResolveOptions) <-chan ResolveResult { ctx, span := startSpan(ctx, "IpnsResolver.ResolveOnceAsync", trace.WithAttributes(attribute.String("Name", name))) defer span.End() - out := make(chan onceResult, 1) + out := make(chan ResolveResult, 1) log.Debugf("RoutingResolver resolving %s", name) cancel := func() {} @@ -65,7 +65,7 @@ func (r *IPNSResolver) resolveOnceAsync(ctx context.Context, name string, option pid, err := peer.Decode(name) if err != nil { log.Debugf("RoutingResolver: could not convert public key hash %s to peer ID: %s\n", name, err) - out <- onceResult{err: err} + out <- ResolveResult{Err: err} close(out) cancel() return out @@ -79,7 +79,7 @@ func (r *IPNSResolver) resolveOnceAsync(ctx context.Context, name string, option vals, err := r.routing.SearchValue(ctx, ipnsKey, dht.Quorum(int(options.DhtRecordCount))) if err != nil { log.Debugf("RoutingResolver: dht get for name %s failed: %s", name, err) - out <- onceResult{err: err} + out <- ResolveResult{Err: err} close(out) cancel() return out @@ -101,13 +101,13 @@ func (r *IPNSResolver) resolveOnceAsync(ctx context.Context, name string, option rec, err := ipns.UnmarshalRecord(val) if err != nil { log.Debugf("RoutingResolver: could not unmarshal value for name %s: %s", name, err) - emitOnceResult(ctx, out, onceResult{err: err}) + emitOnceResult(ctx, out, ResolveResult{Err: err}) return } p, err := rec.Value() if err != nil { - emitOnceResult(ctx, out, onceResult{err: err}) + emitOnceResult(ctx, out, ResolveResult{Err: err}) return } @@ -129,11 +129,11 @@ func (r *IPNSResolver) resolveOnceAsync(ctx context.Context, name string, option } default: log.Errorf("encountered error when parsing EOL: %s", err) - emitOnceResult(ctx, out, onceResult{err: err}) + emitOnceResult(ctx, out, ResolveResult{Err: err}) return } - emitOnceResult(ctx, out, onceResult{value: path.Path(p.String()), ttl: ttl}) + emitOnceResult(ctx, out, ResolveResult{Path: path.Path(p.String()), TTL: ttl}) case <-ctx.Done(): return } diff --git a/namesys/ipns_resolver_test.go b/namesys/ipns_resolver_test.go index 17c140bef..20d9909f1 100644 --- a/namesys/ipns_resolver_test.go +++ b/namesys/ipns_resolver_test.go @@ -30,7 +30,7 @@ func TestRoutingResolve(t *testing.T) { err := publisher.Publish(context.Background(), identity.PrivateKey(), h) require.NoError(t, err) - res, err := resolver.Resolve(context.Background(), identity.ID().Pretty()) + res, _, err := resolver.Resolve(context.Background(), identity.ID().Pretty()) require.NoError(t, err) require.Equal(t, h, res) } @@ -89,7 +89,7 @@ func TestPrexistingRecord(t *testing.T) { } func verifyCanResolve(r Resolver, name string, exp path.Path) error { - res, err := r.Resolve(context.Background(), name) + res, _, err := r.Resolve(context.Background(), name) if err != nil { return err } diff --git a/namesys/mpns.go b/namesys/mpns.go index c06b9f74e..9407f8880 100644 --- a/namesys/mpns.go +++ b/namesys/mpns.go @@ -122,16 +122,18 @@ func NewNameSystem(r routing.ValueStore, opts ...Option) (NameSystem, error) { } // Resolve implements Resolver. -func (ns *nameSys) Resolve(ctx context.Context, name string, options ...ResolveOption) (path.Path, error) { +func (ns *nameSys) Resolve(ctx context.Context, name string, options ...ResolveOption) (path.Path, time.Duration, error) { ctx, span := startSpan(ctx, "MPNS.Resolve", trace.WithAttributes(attribute.String("Name", name))) defer span.End() if strings.HasPrefix(name, "/ipfs/") { - return path.ParsePath(name) + p, err := path.ParsePath(name) + return p, 0, err } if !strings.HasPrefix(name, "/") { - return path.ParsePath("/ipfs/" + name) + p, err := path.ParsePath("/ipfs/" + name) + return p, 0, err } return resolve(ctx, ns, name, ProcessResolveOptions(options)) @@ -144,7 +146,7 @@ func (ns *nameSys) ResolveAsync(ctx context.Context, name string, options ...Res if strings.HasPrefix(name, "/ipfs/") { p, err := path.ParsePath(name) res := make(chan ResolveResult, 1) - res <- ResolveResult{p, err} + res <- ResolveResult{Path: p, Err: err} close(res) return res } @@ -152,7 +154,7 @@ func (ns *nameSys) ResolveAsync(ctx context.Context, name string, options ...Res if !strings.HasPrefix(name, "/") { p, err := path.ParsePath("/ipfs/" + name) res := make(chan ResolveResult, 1) - res <- ResolveResult{p, err} + res <- ResolveResult{Path: p, Err: err} close(res) return res } @@ -161,11 +163,11 @@ func (ns *nameSys) ResolveAsync(ctx context.Context, name string, options ...Res } // resolveOnce implements resolver. -func (ns *nameSys) resolveOnceAsync(ctx context.Context, name string, options ResolveOptions) <-chan onceResult { +func (ns *nameSys) resolveOnceAsync(ctx context.Context, name string, options ResolveOptions) <-chan ResolveResult { ctx, span := startSpan(ctx, "MPNS.ResolveOnceAsync") defer span.End() - out := make(chan onceResult, 1) + out := make(chan ResolveResult, 1) if !strings.HasPrefix(name, ipns.NamespacePrefix) { name = ipns.NamespacePrefix + name @@ -173,7 +175,7 @@ func (ns *nameSys) resolveOnceAsync(ctx context.Context, name string, options Re segments := strings.SplitN(name, "/", 4) if len(segments) < 3 || segments[0] != "" { log.Debugf("invalid name syntax for %s", name) - out <- onceResult{err: ErrResolveFailed} + out <- ResolveResult{Err: ErrResolveFailed} close(out) return out } @@ -194,7 +196,7 @@ func (ns *nameSys) resolveOnceAsync(ctx context.Context, name string, options Re fixedCid := cid.NewCidV1(cid.Libp2pKey, ipnsCid.Hash()).String() codecErr := fmt.Errorf("peer ID represented as CIDv1 require libp2p-key multicodec: retry with /ipns/%s", fixedCid) log.Debugf("RoutingResolver: could not convert public key hash %q to peer ID: %s\n", key, codecErr) - out <- onceResult{err: codecErr} + out <- ResolveResult{Err: codecErr} close(out) return out } @@ -213,7 +215,7 @@ func (ns *nameSys) resolveOnceAsync(ctx context.Context, name string, options Re span.SetAttributes(attribute.Bool("CacheHit", true)) span.RecordError(err) - out <- onceResult{value: p, err: err} + out <- ResolveResult{Path: p, Err: err} close(out) return out } @@ -224,37 +226,37 @@ func (ns *nameSys) resolveOnceAsync(ctx context.Context, name string, options Re } else if _, ok := dns.IsDomainName(key); ok { res = ns.dnsResolver } else { - out <- onceResult{err: fmt.Errorf("invalid IPNS root: %q", key)} + out <- ResolveResult{Err: fmt.Errorf("invalid IPNS root: %q", key)} close(out) return out } resCh := res.resolveOnceAsync(ctx, key, options) - var best onceResult + var best ResolveResult go func() { defer close(out) for { select { case res, ok := <-resCh: if !ok { - if best != (onceResult{}) { - ns.cacheSet(cacheKey, best.value, best.ttl) + if best != (ResolveResult{}) { + ns.cacheSet(cacheKey, best.Path, best.TTL) } return } - if res.err == nil { + if res.Err == nil { best = res } - p := res.value - err := res.err - ttl := res.ttl + p := res.Path + err := res.Err + ttl := res.TTL // Attach rest of the path if len(segments) > 3 { p, err = path.FromSegments("", strings.TrimRight(p.String(), "/"), segments[3]) } - emitOnceResult(ctx, out, onceResult{value: p, ttl: ttl, err: err}) + emitOnceResult(ctx, out, ResolveResult{Path: p, TTL: ttl, Err: err}) case <-ctx.Done(): return } @@ -264,7 +266,7 @@ func (ns *nameSys) resolveOnceAsync(ctx context.Context, name string, options Re return out } -func emitOnceResult(ctx context.Context, outCh chan<- onceResult, r onceResult) { +func emitOnceResult(ctx context.Context, outCh chan<- ResolveResult, r ResolveResult) { select { case outCh <- r: case <-ctx.Done(): diff --git a/namesys/namesys.go b/namesys/namesys.go index 6be310f83..4ff6b9cc0 100644 --- a/namesys/namesys.go +++ b/namesys/namesys.go @@ -69,6 +69,7 @@ type NameSystem interface { // ResolveResult is the return type for [Resolver.ResolveAsync]. type ResolveResult struct { Path path.Path + TTL time.Duration Err error } @@ -94,7 +95,7 @@ type Resolver interface { // // There is a default depth-limit to avoid infinite recursion. Most users will be fine with // this default limit, but if you need to adjust the limit you can specify it as an option. - Resolve(ctx context.Context, name string, options ...ResolveOption) (value path.Path, err error) + Resolve(ctx context.Context, name string, options ...ResolveOption) (value path.Path, ttl time.Duration, err error) // ResolveAsync performs recursive name lookup, like Resolve, but it returns entries as // they are discovered in the DHT. Each returned result is guaranteed to be "better" diff --git a/namesys/namesys_test.go b/namesys/namesys_test.go index 8a1e5a483..b51871efb 100644 --- a/namesys/namesys_test.go +++ b/namesys/namesys_test.go @@ -23,16 +23,16 @@ type mockResolver struct { func testResolution(t *testing.T, resolver Resolver, name string, depth uint, expected string, expError error) { t.Helper() - p, err := resolver.Resolve(context.Background(), name, ResolveWithDepth(depth)) + p, _, err := resolver.Resolve(context.Background(), name, ResolveWithDepth(depth)) require.ErrorIs(t, err, expError) require.Equal(t, expected, p.String()) } -func (r *mockResolver) resolveOnceAsync(ctx context.Context, name string, options ResolveOptions) <-chan onceResult { +func (r *mockResolver) resolveOnceAsync(ctx context.Context, name string, options ResolveOptions) <-chan ResolveResult { p, err := path.ParsePath(r.entries[name]) - out := make(chan onceResult, 1) - out <- onceResult{value: p, err: err} + out := make(chan ResolveResult, 1) + out <- ResolveResult{Path: p, Err: err} close(out) return out } diff --git a/namesys/republisher/repub_test.go b/namesys/republisher/repub_test.go index bc50f29d7..9387cd07e 100644 --- a/namesys/republisher/repub_test.go +++ b/namesys/republisher/repub_test.go @@ -209,7 +209,7 @@ func verifyResolution(nsystems []namesys.NameSystem, key string, exp path.Path) ctx, cancel := context.WithCancel(context.Background()) defer cancel() for _, n := range nsystems { - val, err := n.Resolve(ctx, key) + val, _, err := n.Resolve(ctx, key) if err != nil { return err } @@ -225,7 +225,7 @@ func verifyResolutionFails(nsystems []namesys.NameSystem, key string) error { ctx, cancel := context.WithCancel(context.Background()) defer cancel() for _, n := range nsystems { - _, err := n.Resolve(ctx, key) + _, _, err := n.Resolve(ctx, key) if err == nil { return errors.New("expected resolution to fail") } diff --git a/namesys/resolve/resolve.go b/namesys/resolve/resolve.go index 09ac1bf83..2e93ba6fd 100644 --- a/namesys/resolve/resolve.go +++ b/namesys/resolve/resolve.go @@ -43,7 +43,7 @@ func ResolveIPNS(ctx context.Context, ns namesys.NameSystem, p path.Path) (path. return "", err } - resolvedPath, err := ns.Resolve(ctx, resolvable.String()) + resolvedPath, _, err := ns.Resolve(ctx, resolvable.String()) if err != nil { return "", err } diff --git a/namesys/utilities.go b/namesys/utilities.go index 7a175c3dd..6e1561a80 100644 --- a/namesys/utilities.go +++ b/namesys/utilities.go @@ -12,34 +12,31 @@ import ( "go.opentelemetry.io/otel/trace" ) -type onceResult struct { - value path.Path - ttl time.Duration - err error -} - type resolver interface { - resolveOnceAsync(ctx context.Context, name string, options ResolveOptions) <-chan onceResult + resolveOnceAsync(ctx context.Context, name string, options ResolveOptions) <-chan ResolveResult } // resolve is a helper for implementing Resolver.ResolveN using resolveOnce. -func resolve(ctx context.Context, r resolver, name string, options ResolveOptions) (path.Path, error) { +func resolve(ctx context.Context, r resolver, name string, options ResolveOptions) (path.Path, time.Duration, error) { ctx, cancel := context.WithCancel(ctx) defer cancel() err := ErrResolveFailed - var p path.Path + var ( + p path.Path + ttl time.Duration + ) resCh := resolveAsync(ctx, r, name, options) for res := range resCh { - p, err = res.Path, res.Err + p, ttl, err = res.Path, res.TTL, res.Err if err != nil { break } } - return p, err + return p, ttl, err } func resolveAsync(ctx context.Context, r resolver, name string, options ResolveOptions) <-chan ResolveResult { @@ -71,18 +68,19 @@ func resolveAsync(ctx context.Context, r resolver, name string, options ResolveO break } - if res.err != nil { - emitResult(ctx, outCh, ResolveResult{Err: res.err}) + if res.Err != nil { + emitResult(ctx, outCh, ResolveResult{Err: res.Err}) return } - log.Debugf("resolved %s to %s", name, res.value.String()) - if !strings.HasPrefix(res.value.String(), ipns.NamespacePrefix) { - emitResult(ctx, outCh, ResolveResult{Path: res.value}) + + log.Debugf("resolved %s to %s", name, res.Path.String()) + if !strings.HasPrefix(res.Path.String(), ipns.NamespacePrefix) { + emitResult(ctx, outCh, ResolveResult{Path: res.Path, TTL: res.TTL}) break } if depth == 1 { - emitResult(ctx, outCh, ResolveResult{Path: res.value, Err: ErrResolveRecursion}) + emitResult(ctx, outCh, ResolveResult{Path: res.Path, TTL: res.TTL, Err: ErrResolveRecursion}) break } @@ -99,7 +97,7 @@ func resolveAsync(ctx context.Context, r resolver, name string, options ResolveO subCtx, cancelSub = context.WithCancel(ctx) _ = cancelSub - p := strings.TrimPrefix(res.value.String(), ipns.NamespacePrefix) + p := strings.TrimPrefix(res.Path.String(), ipns.NamespacePrefix) subCh = resolveAsync(subCtx, r, p, subopts) case res, ok := <-subCh: if !ok {