Document philosophy and expectations of pkg/registry

[imjasonh]

...or just move it to internal/registry and document that the expectation is that users should fork it if they want to use it.

Moving conversation out of #1158 into a separate issue.

Since its inception, pkg/registry has been intended mainly to support integration tests -- e.g., to ensure that pkg/v1/remote can actually push and pull against a relatively simple, lightweight, conformant registry implementation. Since then, because it's in a public package that people can depend on, external projects have come to depend on it, mainly also to support integration tests (I think).

pkg/registry used to have zero dependencies aside from stdlib, but in #798 it gained a dependency on other ggcr packages (pkg/v1, also itself very lightweight, only depending on stdlib and pkg/types)

Aside from tests, it's also useful as a basis for a simple lightweight registry implementation, and #1158 aims to make its storage mechanism pluggable, presumably to support persistent disk storage (or something else, I have no idea and don't care). I think we're all on the same page that this is useful, and pkg/registry should be extensible. The expectation that it remain free of extraneous dependencies has never really been documented though, and we should do that. Perhaps in a pkg/registry/README.md.

I would propose staking out roughly these expectations:

• pkg/registry should not depend on anything besides stdlib and go-containerregistry's own packages (I think this is non-controversial).
• storage (and maybe someday auth?) should be pluggable via interfaces, with in-memory storage using maps as the default. If other implementations are provided by ggcr, these must also only depend on stdlib and ggcr packages.
• we may consider adding tests to enforce these expectations (e.g., depcheck)

@jonjohnsonjr @mattmoor do you agree to these expectations, and am I missing anything?

[mattmoor]

The scoping of 1. LGTM (I’d be fine with narrowing the go-containerregistry scoping to avoid implementations, so that this can be used to test those), but I’d encourage us to adopt 3. aggressively to enforce.

2. I’m less keen on a blanket statement about deps. I’d say that it could be its own module if it wants to depend on more (see k8schain), but that it should probably exist separately if the dependencies are extensive.

[imjasonh]

That sounds good to me. I doubt we'll ever have another ggcr-provided implementation except maybe one that uses pkg/v1/layout and writes to disk. I could even state that directly in the README, to be even clearer.

[imjasonh]

#1167 was supposed to mark this as fixed, but I messed it up. I will keep this issue open for now, and probably add more to pkg/registry/README.md before closing this.

[joaopapereira]

I like the scoping as well, it makes sense to me.
Maybe if you want to add more dependencies, it would make sense for the registry to have an extension point, and then you could provide the dependency you want via that extension point. This would keep pkg/registry free of external dependencies and at the same time allow people to extend it if they need it.

[jonjohnsonjr]

pkg/registry should not depend on anything besides stdlib and go-containerregistry's own packages (I think this is non-controversial).

I would prefer it if we avoided depending on go-containerregistry packages as well. The ggcr dependency graph is not as clean as I'd like it to be. Also, it will be much easier to fork off and import the package if we don't have any ggcr dependencies. Also, there are a handful of opinions within most of the ggcr packages that aren't always ideal, and I'd like to avoid those becoming part of the public API.

OTOH, I'm less opposed to pulling in opencontainers packages (assuming they're the very small "leaf" packages that are defined for interop).

I've probably just been burned by go modules too many times and am over-correcting, but IMO things are way simpler if we just have a small standalone package without any external dependencies.

[imjasonh]

I would prefer it if we avoided depending on go-containerregistry packages as well. The ggcr dependency graph is not as clean as I'd like it to be.

Would you find it reasonable to explicitly list the packages that are allowed, instead of allowing any ggcr package? pkg/v1/daemon is a pretty hairy one and pkg/registry should never depend on it anyway, for example. We should make that clearer.

I think pkg/name is pretty clean and well scoped. pkg/v1 and pkg/v1/types are also very light. If we say those three are the three that pkg/registry is allowed to depend on, can we move forward from there? That could even be enforced in a test so new ones don't slip by us in the future.

I've probably just been burned by go modules too many times and am over-correcting, but IMO things are way simpler if we just have a small standalone package without any external dependencies.

It does suck that we feel we need to be so cautious here. If we limit the set of dependencies, we can decide to break them apart again later and just copy whatever we need into pkg/registry at that time, when we know what we need.

[mattmoor]

The ggcr dependency graph is not as clean as I'd like it to be

To avoid fighting phantoms here, let's be specific. I think you'll find we all agree with pkg/v1/daemon, but that seems like a far fetched thing to have creep in here.

it will be much easier to fork off and import the package if we don't have any ggcr dependencies

Is this a goal? IMO we should either aim for it to be a useful part of this library, or it shouldn't be here.

there are a handful of opinions within most of the ggcr packages that aren't always ideal, and I'd like to avoid those becoming part of the public API.

Same as my first comment, let's be specific here.

I'm less opposed to pulling in opencontainers packages

Same as my second comment. If this is going to stay here, it should IMO mesh well with the rest of the library. This comment seems inconsistent with that goal.

I think pkg/name is pretty clean and well scoped. pkg/v1 and pkg/v1/types are also very light

+1 to this whole section. depcheck has an "allow list" mode, we can just fork that and start with this handful IMO, there's no need to allow arbitrary deps into GGCR.

[imjasonh]

In an attempt to get something concrete we can argue about, I propose this statement in pkg/registry/README.md:

pkg/registry is designed to be a good fit for tests, and as a simple basis for other registry implementations. As such, it should remain free of heavy dependencies. The package itself should only depend on Go standard library packages, and a small subset of go-containerregistry packages (currently, pkg/name, pkg/v1 and pkg/v1/types). Adding any other dependencies should be done with extreme rigor, and PRs that add further dependencies may be rejected with extreme prejudice.

If this is acceptable I'll propose this in a PR, close this issue, and I can reopen #1158, which adheres to this minimalist philosophy.

Concurrently, I can add a depcheck test that fails if any other dependencies sneak in.

@jonjohnsonjr wdyt?

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Keep fresh with the 'lifecycle/frozen' label.