8000 Use reference package internally by aaronlehmann · Pull Request #1270 · distribution/distribution · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Use reference package internally #1270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Use reference package internally #1270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
8000
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 21 additions & 15 deletions notifications/bridge.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/docker/distribution/context"
"github.com/docker/distribution/digest"
"github.com/docker/distribution/manifest/schema1"
"github.com/docker/distribution/reference"
"github.com/docker/distribution/uuid"
)

Expand All @@ -23,8 +24,8 @@ var _ Listener = &bridge{}

// URLBuilder defines a subset of url builder to be used by the event listener.
type URLBuilder interface {
BuildManifestURL(name, tag string) (string, error)
BuildBlobURL(name string, dgst digest.Digest) (string, error)
BuildManifestURL(name reference.Named, tag string) (string, error)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be reference.Tagged?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should take reference.NamedTagged, and BuildBlobURL should take reference.Canonical. I'll make these changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've implemented this change at https://github.com/aaronlehmann/distribution/tree/use-reference-package-v2. It depends on #1268 and #1271, so I'm going to close this PR for now and reopen it when those PRs are merged.

(Slight correction to the above: BuildManifestURL will take a single reference.Named that may include a digest or tag.)

BuildBlobURL(name reference.Named, dgst digest.Digest) (string, error)
}

// NewBridge returns a notification listener that writes records to sink,
Expand Down Expand Up @@ -53,31 +54,31 @@ func NewRequestRecord(id string, r *http.Request) RequestRecord {
}
}

func (b *bridge) ManifestPushed(repo string, sm *schema1.SignedManifest) error {
func (b *bridge) ManifestPushed(repo reference.Named, sm *schema1.SignedManifest) error {
return b.createManifestEventAndWrite(EventActionPush, repo, sm)
}

func (b *bridge) ManifestPulled(repo string, sm *schema1.SignedManifest) error {
func (b *bridge) ManifestPulled(repo reference.Named, sm *schema1.SignedManifest) error {
return b.createManifestEventAndWrite(EventActionPull, repo, sm)
}

func (b *bridge) ManifestDeleted(repo string, sm *schema1.SignedManifest) error {
func (b *bridge) ManifestDeleted(repo reference.Named, sm *schema1.SignedManifest) error {
return b.createManifestEventAndWrite(EventActionDelete, repo, sm)
}

func (b *bridge) BlobPushed(repo string, desc distribution.Descriptor) error {
func (b *bridge) BlobPushed(repo reference.Named, desc distribution.Descriptor) error {
return b.createBlobEventAndWrite(EventActionPush, repo, desc)
}

func (b *bridge) BlobPulled(repo string, desc distribution.Descriptor) error {
func (b *bridge) BlobPulled(repo reference.Named, desc distribution.Descriptor) error {
return b.createBlobEventAndWrite(EventActionPull, repo, desc)
}

func (b *bridge) BlobDeleted(repo string, desc distribution.Descriptor) error {
func (b *bridge) BlobDeleted(repo reference.Named, desc distribution.Descriptor) error {
return b.createBlobEventAndWrite(EventActionDelete, repo, desc)
}

func (b *bridge) createManifestEventAndWrite(action string, repo string, sm *schema1.SignedManifest) error {
func (b *bridge) createManifestEventAndWrite(action string, repo reference.Named, sm *schema1.SignedManifest) error {
manifestEvent, err := b.createManifestEvent(action, repo, sm)
if err != nil {
return err
Expand All @@ -86,10 +87,10 @@ func (b *bridge) createManifestEventAndWrite(action string, repo string, sm *sch
return b.sink.Write(*manifestEvent)
}

func (b *bridge) createManifestEvent(action string, repo string, sm *schema1.SignedManifest) (*Event, error) {
func (b *bridge) createManifestEvent(action string, repo reference.Named, sm *schema1.SignedManifest) (*Event, error) {
event := b.createEvent(action)
event.Target.MediaType = schema1.ManifestMediaType
event.Target.Repository = repo
event.Target.Repository = repo.String()

p, err := sm.Payload()
if err != nil {
Expand All @@ -103,15 +104,20 @@ func (b *bridge) createManifestEvent(action string, repo string, sm *schema1.Sig
return nil, err
}

event.Target.URL, err = b.ub.BuildManifestURL(sm.Name, event.Target.Digest.String())
nameRef, err := reference.ParseNamed(sm.Name)
if err != nil {
return nil, err
}

event.Target.URL, err = b.ub.BuildManifestURL(nameRef, event.Target.Digest.String())
if err != nil {
return nil, err
}

return event, nil
}

func (b *bridge) createBlobEventAndWrite(action string, repo string, desc distribution.Descriptor) error {
func (b *bridge) createBlobEventAndWrite(action string, repo reference.Named, desc distribution.Descriptor) error {
event, err := b.createBlobEvent(action, repo, desc)
if err != nil {
return err
Expand All @@ -120,11 +126,11 @@ func (b *bridge) createBlobEventAndWrite(action string, repo string, desc distri
return b.sink.Write(*event)
}

func (b *bridge) createBlobEvent(action string, repo string, desc distribution.Descriptor) (*Event, error) {
func (b *bridge) createBlobEvent(action string, repo reference.Named, desc distribution.Descriptor) (*Event, error) {
event := b.createEvent(action)
event.Target.Descriptor = desc
event.Target.Length = desc.Size
event.Target.Repository = repo
event.Target.Repository = repo.String()

var err error
event.Target.URL, err = b.ub.BuildBlobURL(repo, desc.Digest)
Expand Down
14 changes: 9 additions & 5 deletions notifications/bridge_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"

"github.com/docker/distribution/digest"
"github.com/docker/distribution/reference"

"github.com/docker/libtrust"

Expand Down Expand Up @@ -38,14 +39,14 @@ var (
)

func TestEventBridgeManifestPulled(t *testing.T) {

l := createTestEnv(t, testSinkFn(func(events ...Event) error {
checkCommonManifest(t, EventActionPull, events...)

return nil
}))

if err := l.ManifestPulled(repo, sm); err != nil {
repoRef, _ := reference.ParseNamed(repo)
if err := l.ManifestPulled(repoRef, sm); err != nil {
t.Fatalf("unexpected error notifying manifest pull: %v", err)
}
}
Expand All @@ -57,7 +58,8 @@ func TestEventBridgeManifestPushed(t *testing.T) {
return nil
}))

if err := l.ManifestPushed(repo, sm); err != nil {
repoRef, _ := reference.ParseNamed(repo)
if err := l.ManifestPushed(repoRef, sm); err != nil {
t.Fatalf("unexpected error notifying manifest pull: %v", err)
}
}
Expand All @@ -69,7 +71,8 @@ func TestEventBridgeManifestDeleted(t *testing.T) {
return nil
}))

if err := l.ManifestDeleted(repo, sm); err != nil {
repoRef, _ := reference.ParseNamed(repo)
if err := l.ManifestDeleted(repoRef, sm); err != nil {
t.Fatalf("unexpected error notifying manifest pull: %v", err)
}
}
Expand Down Expand Up @@ -106,7 +109,8 @@ func checkCommonManifest(t *testing.T, action string, events ...Event) {
t.Fatalf("unexpected event action: %q != %q", event.Action, action)
}

u, err := ub.BuildManifestURL(repo, dgst.String())
repoRef, _ := reference.ParseNamed(repo)
u, err := ub.BuildManifestURL(repoRef, dgst.String())
if err != nil {
t.Fatalf("error building expected url: %v", err)
}
Expand Down
13 changes: 7 additions & 6 deletions notifications/listener.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,29 @@ import (
"github.com/docker/distribution/context"
"github.com/docker/distribution/digest"
"github.com/docker/distribution/manifest/schema1"
"github.com/docker/distribution/reference"
)

// ManifestListener describes a set of methods for listening to events related to manifests.
type ManifestListener interface {
ManifestPushed(repo string, sm *schema1.SignedManifest) error
ManifestPulled(repo string, sm *schema1.SignedManifest) error
ManifestPushed(repo reference.Named, sm *schema1.SignedManifest) error
ManifestPulled(repo reference.Named, sm *schema1.SignedManifest) error

// TODO(stevvooe): Please note that delete support is still a little shaky
// and we'll need to propagate these in the future.

ManifestDeleted(repo string, sm *schema1.SignedManifest) error
ManifestDeleted(repo reference.Named, sm *schema1.SignedManifest) error
}

// BlobListener describes a listener that can respond to layer related events.
type BlobListener interface {
BlobPushed(repo string, desc distribution.Descriptor) error
BlobPulled(repo string, desc distribution.Descriptor) error
BlobPushed(repo reference.Named, desc distribution.Descriptor) error
BlobPulled(repo reference.Named, desc distribution.Descriptor) error

// TODO(stevvooe): Please note that delete support is still a little shaky
// and we'll need to propagate these in the future.

BlobDeleted(repo string, desc distribution.Descriptor) error
BlobDeleted(repo reference.Named, desc distribution.Descriptor) error
}

// Listener combines all repository events into a single interface.
Expand Down
18 changes: 10 additions & 8 deletions notifications/listener_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/docker/distribution/digest"
"github.com/docker/distribution/manifest"
"github.com/docker/distribution/manifest/schema1"
"github.com/docker/distribution/reference"
"github.com/docker/distribution/registry/storage"
"github.com/docker/distribution/registry/storage/cache/memory"
"github.com/docker/distribution/registry/storage/driver/inmemory"
Expand All @@ -27,7 +28,8 @@ func TestListener(t *testing.T) {
ops: make(map[string]int),
}

repository, err := registry.Repository(ctx, "foo/bar")
repoRef, _ := reference.ParseNamed("foo/bar")
repository, err := registry.Repository(ctx, repoRef)
if err != nil {
t.Fatalf("unexpected error getting repo: %v", err)
}
Expand Down Expand Up @@ -55,33 +57,33 @@ type testListener struct {
ops map[string]int
}

func (tl *testListener) ManifestPushed(repo string, sm *schema1.SignedManifest) error {
func (tl *testListener) ManifestPushed(repo reference.Named, sm *schema1.SignedManifest) error {
tl.ops["manifest:push"]++

return nil
}

func (tl *testListener) ManifestPulled(repo string, sm *schema1.SignedManifest) error {
func (tl *testListener) ManifestPulled(repo reference.Named, sm *schema1.SignedManifest) error {
tl.ops["manifest:pull"]++
return nil
}

func (tl *testListener) ManifestDeleted(repo string, sm *schema1.SignedManifest) error {
func (tl *testListener) ManifestDeleted(repo reference.Named, sm *schema1.SignedManifest) error {
tl.ops["manifest:delete"]++
return nil
}

func (tl *testListener) BlobPushed(repo string, desc distribution.Descriptor) error {
func (tl *testListener) BlobPushed(repo reference.Named, desc distribution.Descriptor) error {
tl.ops["layer:push"]++
return nil
}

func (tl *testListener) BlobPulled(repo string, desc distribution.Descriptor) error {
func (tl *testListener) BlobPulled(repo reference.Named, desc distribution.Descriptor) error {
tl.ops["layer:pull"]++
return nil
}

func (tl *testListener) BlobDeleted(repo string, desc distribution.Descriptor) error {
func (tl *testListener) BlobDeleted(repo reference.Named, desc distribution.Descriptor) error {
tl.ops["layer:delete"]++
return nil
}
Expand All @@ -99,7 +101,7 @@ func checkExerciseRepository(t *testing.T, repository distribution.Repository) {
Versioned: manifest.Versioned{
SchemaVersion: 1,
},
Name: repository.Name(),
Name: repository.Name().String(),
Tag: tag,
}

Expand Down
5 changes: 3 additions & 2 deletions registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"github.com/docker/distribution/context"
"github.com/docker/distribution/digest"
"github.com/docker/distribution/manifest/schema1"
"github.com/docker/distribution/reference"
)

// Scope defines the set of items that match a namespace.
Expand Down Expand Up @@ -34,7 +35,7 @@ type Namespace interface {
// Repository should return a reference to the named repository. The
// registry may or may not have the repository but should always return a
// reference.
Repository(ctx context.Context, name string) (Repository, error)
Repository(ctx context.Context, name reference.Named) (Repository, error)

// Repositories fills 'repos' with a lexigraphically sorted catalog of repositories
// up to the size of 'repos' and returns the value 'n' for the number of entries
Expand All @@ -49,7 +50,7 @@ type ManifestServiceOption func(ManifestService) error
// Repository is a named collection of manifests and layers.
type Repository interface {
// Name returns the name of the repository.
Name() string
Name() reference.Named

// Manifests returns a reference to this repository's manifest service.
// with the supplied options applied.
Expand Down
21 changes: 11 additions & 10 deletions registry/api/v2/urls.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"

"github.com/docker/distribution/digest"
"github.com/docker/distribution/reference"
"github.com/gorilla/mux"
)

Expand Down Expand Up @@ -113,10 +114,10 @@ func (ub *URLBuilder) BuildCatalogURL(values ...url.Values) (string, error) {
}

// BuildTagsURL constructs a url to list the tags in the named repository.
func (ub *URLBuilder) BuildTagsURL(name string) (string, error) {
func (ub *URLBuilder) BuildTagsURL(name reference.Named) (string, error) {
route := ub.cloneRoute(RouteNameTags)

tagsURL, err := route.URL("name", name)
tagsURL, err := route.URL("name", name.String())
if err != nil {
return "", err
}
Expand All @@ -126,10 +127,10 @@ func (ub *URLBuilder) BuildTagsURL(name string) (string, error) {

// BuildManifestURL constructs a url for the manifest identified by name and
// reference. The argument reference may be either a tag or digest.
func (ub *URLBuilder) BuildManifestURL(name, reference string) (string, error) {
func (ub *URLBuilder) BuildManifestURL(name reference.Named, reference string) (string, error) {
route := ub.cloneRoute(RouteNameManifest)

manifestURL, err := route.URL("name", name, "reference", reference)
manifestURL, err := route.URL("name", name.String(), "reference", reference)
if err != nil {
return "", err
}
Expand All @@ -138,10 +139,10 @@ func (ub *URLBuilder) BuildManifestURL(name, reference string) (string, error) {
}

// BuildBlobURL const 2851 ructs the url for the blob identified by name and dgst.
func (ub *URLBuilder) BuildBlobURL(name string, dgst digest.Digest) (string, error) {
func (ub *URLBuilder) BuildBlobURL(name reference.Named, dgst digest.Digest) (string, error) {
route := ub.cloneRoute(RouteNameBlob)

layerURL, err := route.URL("name", name, "digest", dgst.String())
layerURL, err := route.URL("name", name.String(), "digest", dgst.String())
if err != nil {
return "", err
}
Expand All @@ -151,10 +152,10 @@ func (ub *URLBuilder) BuildBlobURL(name string, dgst digest.Digest) (string, err

// BuildBlobUploadURL constructs a url to begin a blob upload in the
// repository identified by name.
func (ub *URLBuilder) BuildBlobUploadURL(name string, values ...url.Values) (string, error) {
func (ub *URLBuilder) BuildBlobUploadURL(name reference.Named, values ...url.Values) (string, error) {
route := ub.cloneRoute(RouteNameBlobUpload)

uploadURL, err := route.URL("name", name)
uploadURL, err := route.URL("name", name.String())
if err != nil {
return "", err
}
Expand All @@ -166,10 +167,10 @@ func (ub *URLBuilder) BuildBlobUploadURL(name string, values ...url.Values) (str
// including any url values. This should generally not be used by clients, as
// this url is provided by server implementations during the blob upload
// process.
func (ub *URLBuilder) BuildBlobUploadChunkURL(name, uuid string, values ...url.Values) (string, error) {
func (ub *URLBuilder) BuildBlobUploadChunkURL(name reference.Named, uuid string, values ...url.Values) (string, error) {
route := ub.cloneRoute(RouteNameBlobUploadChunk)

uploadURL, err := route.URL("name", name, "uuid", uuid)
uploadURL, err := route.URL("name", name.String(), "uuid", uuid)
if err != nil {
return "", err
}
Expand Down
Loading
0