issue with caching "bad" fetchers in Puller/Pusher

[smira]

See e.g.

go-containerregistry/pkg/v1/remote/puller.go

Lines 65 to 90 in c4dd792

	// this will run once per reader instance
	func (r *reader) init(ctx context.Context) error {
	r.once.Do(func() {
	r.f, r.err = makeFetcher(ctx, r.target, r.o)
	})
	return r.err
	}
	func (p *Puller) fetcher(ctx context.Context, target resource) (*fetcher, error) {
	v, _ := p.readers.LoadOrStore(target, &reader{
	target: target,
	o: p.o,
	})
	rr := v.(*reader)
	return rr.f, rr.init(ctx)
	}
	// Head is like remote.Head, but avoids re-authenticating when possible.
	func (p *Puller) Head(ctx context.Context, ref name.Reference) (*v1.Descriptor, error) {
	f, err := p.fetcher(ctx, ref.Context())
	if err != nil {
	return nil, err
	}
	return f.headManifest(ctx, ref, allManifestMediaTypes)
	}

(same issue is in the Pusher)

For the code like:

puller := remote.NewPuller() // cache the puller instance

// somewhere later

puller.Head(ctx, reference) 

In this flow, fetcher is created lazily on access, but it uses the ctx passed with the first request. If the fetcher fails to init itself because ctx got canceled, it will be cached forever in the failed state (due to sync.Once). While the next .Head request might come with non-canceled context, it will still fail with the first error.

Moreover, the ctx used to initialize the fetcher is same as ctx used for the Head request itself, so passing context.Background is not an option.

There is no interface to clean cached fetcher, or even skip the cache.

Is it by design?

The only workaround is to create Puller/Pusher on each request, which completely cancels the idea of caching/reusing fetchers.