From 0d8f4ac7b8d62c237ff93ee0b3cce5114f425c79 Mon Sep 17 00:00:00 2001 From: Manish Tomar Date: Wed, 18 Jul 2018 11:25:59 -0700 Subject: [PATCH 1/5] add tag deletion event whenever a tag is deleted an event is sent out via the regular notification channels Signed-off-by: Manish Tomar --- notifications/bridge.go | 8 ++++++++ notifications/bridge_test.go | 23 ++++++++++++++++++----- notifications/listener.go | 30 ++++++++++++++++++++++++++++++ notifications/listener_test.go | 12 ++++++++++-- 4 files changed, 66 insertions(+), 7 deletions(-) diff --git a/notifications/bridge.go b/notifications/bridge.go index 8f6386d3..bc4a90aa 100644 --- a/notifications/bridge.go +++ b/notifications/bridge.go @@ -108,6 +108,14 @@ func (b *bridge) BlobDeleted(repo reference.Named, dgst digest.Digest) error { return b.createBlobDeleteEventAndWrite(EventActionDelete, repo, dgst) } +func (b *bridge) TagDeleted(repo reference.Named, tag string) error { + event := b.createEvent(EventActionDelete) + event.Target.Repository = repo.Name() + event.Target.Tag = tag + + return b.sink.Write(*event) +} + func (b *bridge) createManifestEventAndWrite(action string, repo reference.Named, sm distribution.Manifest) error { manifestEvent, err := b.createManifestEvent(action, repo, sm) if err != nil { diff --git a/notifications/bridge_test.go b/notifications/bridge_test.go index 86350993..0f4d7736 100644 --- a/notifications/bridge_test.go +++ b/notifications/bridge_test.go @@ -97,6 +97,9 @@ func TestEventBridgeManifestPulledWithTag(t *testing.T) { func TestEventBridgeManifestDeleted(t *testing.T) { l := createTestEnv(t, testSinkFn(func(events ...Event) error { checkDeleted(t, EventActionDelete, events...) + if events[0].Target.Digest != dgst { + t.Fatalf("unexpected digest on event target: %q != %q", events[0].Target.Digest, dgst) + } return nil })) @@ -106,6 +109,21 @@ func TestEventBridgeManifestDeleted(t *testing.T) { } } +func TestEventBridgeTagDeleted(t *testing.T) { + l := createTestEnv(t, testSinkFn(func(events ...Event) error { + checkDeleted(t, EventActionDelete, events...) + if events[0].Target.Tag != m.Tag { + t.Fatalf("unexpected tag on event target: %q != %q", events[0].Target.Tag, m.Tag) + } + return nil + })) + + repoRef, _ := reference.WithName(repo) + if err := l.TagDeleted(repoRef, m.Tag); err != nil { + t.Fatalf("unexpected error notifying tag deletion: %v", err) + } +} + func createTestEnv(t *testing.T, fn testSinkFn) Listener { pk, err := libtrust.GenerateECP256PrivateKey() if err != nil { @@ -142,14 +160,9 @@ func checkDeleted(t *testing.T, action string, events ...Event) { t.Fatalf("request not equal: %#v != %#v", event.Actor, actor) } - if event.Target.Digest != dgst { - t.Fatalf("unexpected digest on event target: %q != %q", event.Target.Digest, dgst) - } - if event.Target.Repository != repo { t.Fatalf("unexpected repository: %q != %q", event.Target.Repository, repo) } - } func checkCommonManifest(t *testing.T, action string, events ...Event) { diff --git a/notifications/listener.go b/notifications/listener.go index 52ec0ee7..8cfdb67e 100644 --- a/notifications/listener.go +++ b/notifications/listener.go @@ -5,6 +5,7 @@ import ( "net/http" "github.com/docker/distribution" + dcontext "github.com/docker/distribution/context" "github.com/docker/distribution/reference" "github.com/opencontainers/go-digest" @@ -25,10 +26,16 @@ type BlobListener interface { BlobDeleted(repo reference.Named, desc digest.Digest) error } +// RepoListener describes a listener that can respond to repository related events. +type RepoListener interface { + TagDeleted(repo reference.Named, tag string) error +} + // Listener combines all repository events into a single interface. type Listener interface { ManifestListener BlobListener + RepoListener } type repositoryListener struct { @@ -214,3 +221,26 @@ func (bwl *blobWriterListener) Commit(ctx context.Context, desc distribution.Des return committed, err } + +type tagServiceListener struct { + distribution.TagService + parent *repositoryListener +} + +func (rl *repositoryListener) Tags(ctx context.Context) distribution.TagService { + return &tagServiceListener{ + TagService: rl.Repository.Tags(ctx), + parent: rl, + } +} + +func (tagSL *tagServiceListener) Untag(ctx context.Context, tag string) error { + if err := tagSL.TagService.Untag(ctx, tag); err != nil { + return err + } + if err := tagSL.parent.listener.TagDeleted(tagSL.parent.Repository.Named(), tag); err != nil { + dcontext.GetLogger(ctx).Errorf("error dispatching tag deleted to listener: %v", err) + return err + } + return nil +} diff --git a/notifications/listener_test.go b/notifications/listener_test.go index a5849807..32a7f6d9 100644 --- a/notifications/listener_test.go +++ b/notifications/listener_test.go @@ -50,12 +50,12 @@ func TestListener(t *testing.T) { "layer:push": 2, "layer:pull": 2, "layer:delete": 2, + "tag:delete": 1, } if !reflect.DeepEqual(tl.ops, expectedOps) { t.Fatalf("counts do not match:\n%v\n !=\n%v", tl.ops, expectedOps) } - } type testListener struct { @@ -64,7 +64,6 @@ type testListener struct { func (tl *testListener) ManifestPushed(repo reference.Named, m distribution.Manifest, options ...distribution.ManifestServiceOption) error { tl.ops["manifest:push"]++ - return nil } @@ -98,6 +97,11 @@ func (tl *testListener) BlobDeleted(repo reference.Named, d digest.Digest) error return nil } +func (tl *testListener) TagDeleted(repo reference.Named, tag string) error { + tl.ops["tag:delete"]++ + return nil +} + // checkExerciseRegistry takes the registry through all of its operations, // carrying out generic checks. func checkExerciseRepository(t *testing.T, repository distribution.Repository) { @@ -200,6 +204,10 @@ func checkExerciseRepository(t *testing.T, repository distribution.Repository) { if err != nil { t.Fatalf("unexpected error deleting blob: %v", err) } + } + err = repository.Tags(ctx).Untag(ctx, m.Tag) + if err != nil { + t.Fatalf("unexpected error deleting tag: %v", err) } } From 328069bb4dc4a0db3f024960ab89a5ee68d69240 Mon Sep 17 00:00:00 2001 From: Manish Tomar Date: Thu, 2 Aug 2018 22:58:52 -0700 Subject: [PATCH 2/5] add support for repo deleted event also by having another interface RepositoryRemover that is implemented by registry instance and is injected in app context for event tracking Signed-off-by: Manish Tomar --- notifications/bridge.go | 7 +++++++ notifications/bridge_test.go | 12 ++++++++++++ notifications/listener.go | 24 ++++++++++++++++++++---- notifications/listener_test.go | 22 +++++++++++++++++++--- registry.go | 5 +++++ registry/storage/catalog.go | 11 +++++++++++ registry/storage/registry.go | 2 ++ 7 files changed, 76 insertions(+), 7 deletions(-) diff --git a/notifications/bridge.go b/notifications/bridge.go index bc4a90aa..48048063 100644 --- a/notifications/bridge.go +++ b/notifications/bridge.go @@ -116,6 +116,13 @@ func (b *bridge) TagDeleted(repo reference.Named, tag string) error { return b.sink.Write(*event) } +func (b *bridge) RepoDeleted(repo reference.Named) error { + event := b.createEvent(EventActionDelete) + event.Target.Repository = repo.Name() + + return b.sink.Write(*event) +} + func (b *bridge) createManifestEventAndWrite(action string, repo reference.Named, sm distribution.Manifest) error { manifestEvent, err := b.createManifestEvent(action, repo, sm) if err != nil { diff --git a/notifications/bridge_test.go b/notifications/bridge_test.go index 0f4d7736..5c1401aa 100644 --- a/notifications/bridge_test.go +++ b/notifications/bridge_test.go @@ -124,6 +124,18 @@ func TestEventBridgeTagDeleted(t *testing.T) { } } +func TestEventBridgeRepoDeleted(t *testing.T) { + l := createTestEnv(t, testSinkFn(func(events ...Event) error { + checkDeleted(t, EventActionDelete, events...) + return nil + })) + + repoRef, _ := reference.WithName(repo) + if err := l.RepoDeleted(repoRef); err != nil { + t.Fatalf("unexpected error notifying repo deletion: %v", err) + } +} + func createTestEnv(t *testing.T, fn testSinkFn) Listener { pk, err := libtrust.GenerateECP256PrivateKey() if err != nil { diff --git a/notifications/listener.go b/notifications/listener.go index 8cfdb67e..b11f0773 100644 --- a/notifications/listener.go +++ b/notifications/listener.go @@ -26,9 +26,9 @@ type BlobListener interface { BlobDeleted(repo reference.Named, desc digest.Digest) error } -// RepoListener describes a listener that can respond to repository related events. type RepoListener interface { TagDeleted(repo reference.Named, tag string) error + RepoDeleted(repo reference.Named) error } // Listener combines all repository events into a single interface. @@ -43,12 +43,28 @@ type repositoryListener struct { listener Listener } +type removerListener struct { + distribution.RepositoryRemover + listener Listener +} + // Listen dispatches events on the repository to the listener. -func Listen(repo distribution.Repository, listener Listener) distribution.Repository { +func Listen(repo distribution.Repository, remover distribution.RepositoryRemover, listener Listener) (distribution.Repository, distribution.RepositoryRemover) { return &repositoryListener{ - Repository: repo, - listener: listener, + Repository: repo, + listener: listener, + }, &removerListener{ + RepositoryRemover: remover, + listener: listener, + } +} + +func (nl *removerListener) Remove(ctx context.Context, name reference.Named) error { + err := nl.RepositoryRemover.Remove(ctx, name) + if err != nil { + return err } + return nl.listener.RepoDeleted(name) } func (rl *repositoryListener) Manifests(ctx context.Context, options ...distribution.ManifestServiceOption) (distribution.ManifestService, error) { diff --git a/notifications/listener_test.go b/notifications/listener_test.go index 32a7f6d9..1d8b731e 100644 --- a/notifications/listener_test.go +++ b/notifications/listener_test.go @@ -38,10 +38,15 @@ func TestListener(t *testing.T) { if err != nil { t.Fatalf("unexpected error getting repo: %v", err) } - repository = Listen(repository, tl) + + remover, ok := registry.(distribution.RepositoryRemover) + if !ok { + t.Fatal("registry does not implement RepositoryRemover") + } + repository, remover = Listen(repository, remover, tl) // Now take the registry through a number of operations - checkExerciseRepository(t, repository) + checkExerciseRepository(t, repository, remover) expectedOps := map[string]int{ "manifest:push": 1, @@ -51,6 +56,7 @@ func TestListener(t *testing.T) { "layer:pull": 2, "layer:delete": 2, "tag:delete": 1, + "repo:delete": 1, } if !reflect.DeepEqual(tl.ops, expectedOps) { @@ -102,9 +108,14 @@ func (tl *testListener) TagDeleted(repo reference.Named, tag string) error { return nil } +func (tl *testListener) RepoDeleted(repo reference.Named) error { + tl.ops["repo:delete"]++ + return nil +} + // checkExerciseRegistry takes the registry through all of its operations, // carrying out generic checks. -func checkExerciseRepository(t *testing.T, repository distribution.Repository) { +func checkExerciseRepository(t *testing.T, repository distribution.Repository, remover distribution.RepositoryRemover) { // TODO(stevvooe): This would be a nice testutil function. Basically, it // takes the registry through a common set of operations. This could be // used to make cross-cutting updates by changing internals that affect @@ -210,4 +221,9 @@ func checkExerciseRepository(t *testing.T, repository distribution.Repository) { if err != nil { t.Fatalf("unexpected error deleting tag: %v", err) } + + err = remover.Remove(ctx, repository.Named()) + if err != nil { + t.Fatalf("unexpected error deleting repo: %v", err) + } } diff --git a/registry.go b/registry.go index a3a80ab8..6c321098 100644 --- a/registry.go +++ b/registry.go @@ -54,6 +54,11 @@ type RepositoryEnumerator interface { Enumerate(ctx context.Context, ingester func(string) error) error } +// RepositoryRemover removes given repository +type RepositoryRemover interface { + Remove(ctx context.Context, name reference.Named) error +} + // ManifestServiceOption is a function argument for Manifest Service methods type ManifestServiceOption interface { Apply(ManifestService) error diff --git a/registry/storage/catalog.go b/registry/storage/catalog.go index 3c1a78de..4db8bd88 100644 --- a/registry/storage/catalog.go +++ b/registry/storage/catalog.go @@ -7,6 +7,7 @@ import ( "path" "strings" + "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/storage/driver" ) @@ -70,6 +71,16 @@ func (reg *registry) Enumerate(ctx context.Context, ingester func(string) error) return err } +// Remove removes a repository from storage +func (r *registry) Remove(ctx context.Context, name reference.Named) error { + root, err := pathFor(repositoriesRootPathSpec{}) + if err != nil { + return err + } + repoDir := path.Join(root, name.Name()) + return r.driver.Delete(ctx, repoDir) +} + // lessPath returns true if one path a is less than path b. // // A component-wise comparison is done, rather than the lexical comparison of diff --git a/registry/storage/registry.go b/registry/storage/registry.go index 46b96853..70d5b8d2 100644 --- a/registry/storage/registry.go +++ b/registry/storage/registry.go @@ -23,6 +23,7 @@ type registry struct { schema1SigningKey libtrust.PrivateKey blobDescriptorServiceFactory distribution.BlobDescriptorServiceFactory manifestURLs manifestURLs + driver storagedriver.StorageDriver } // manifestURLs holds regular expressions for controlling manifest URL whitelisting @@ -133,6 +134,7 @@ func NewRegistry(ctx context.Context, driver storagedriver.StorageDriver, option }, statter: statter, resumableDigestEnabled: true, + driver: driver, } for _, option := range options { From 8f6758278d90fc2f1fba7ca42a21f5dfe34fe52d Mon Sep 17 00:00:00 2001 From: Manish Tomar Date: Thu, 2 Aug 2018 23:13:37 -0700 Subject: [PATCH 3/5] take handler update forgot to commit this earlier Signed-off-by: Manish Tomar --- registry/handlers/app.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/registry/handlers/app.go b/registry/handlers/app.go index a40a4df3..76c63080 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -58,10 +58,11 @@ type App struct { Config *configuration.Configuration - router *mux.Router // main application router, configured with dispatchers - driver storagedriver.StorageDriver // driver maintains the app global storage driver instance. - registry distribution.Namespace // registry is the primary registry backend for the app instance. - accessController auth.AccessController // main access controller for application + router *mux.Router // main application router, configured with dispatchers + driver storagedriver.StorageDriver // driver maintains the app global storage driver instance. + registry distribution.Namespace // registry is the primary registry backend for the app instance. + repoRemover distribution.RepositoryRemover // repoRemover provides ability to delete repos + accessController auth.AccessController // main access controller for application // httpHost is a parsed representation of the http.host parameter from // the configuration. Only the Scheme and Host fields are used. @@ -320,6 +321,11 @@ func NewApp(ctx context.Context, config *configuration.Configuration) *App { app.isCache = true dcontext.GetLogger(app).Info("Registry configured as a proxy cache to ", config.Proxy.RemoteURL) } + var ok bool + app.repoRemover, ok = app.registry.(distribution.RepositoryRemover) + if !ok { + dcontext.GetLogger(app).Warnf("Registry does not implement RempositoryRemover. Will not be able to delete repos and tags") + } return app } @@ -696,8 +702,9 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { } // assign and decorate the authorized repository with an event bridge. - context.Repository = notifications.Listen( + context.Repository, context.App.repoRemover = notifications.Listen( repository, + context.App.repoRemover, app.eventBridge(context, r)) context.Repository, err = applyRepoMiddleware(app, context.Repository, app.Config.Middleware["repository"]) From e4d5a0a17c37e54a1cf9ca2ae2be10ce6df89354 Mon Sep 17 00:00:00 2001 From: Manish Tomar Date: Fri, 3 Aug 2018 14:08:00 -0700 Subject: [PATCH 4/5] Add documentation Signed-off-by: Manish Tomar --- notifications/listener.go | 1 + 1 file changed, 1 insertion(+) diff --git a/notifications/listener.go b/notifications/listener.go index b11f0773..98ad8da9 100644 --- a/notifications/listener.go +++ b/notifications/listener.go @@ -26,6 +26,7 @@ type BlobListener interface { BlobDeleted(repo reference.Named, desc digest.Digest) error } +// RepoListener provides repository methods that respond to repository lifecycle type RepoListener interface { TagDeleted(repo reference.Named, tag string) error RepoDeleted(repo reference.Named) error From 8c05756141eb3b22d41860c58735bbf21024fd7e Mon Sep 17 00:00:00 2001 From: Manish Tomar Date: Mon, 6 Aug 2018 09:46:42 -0700 Subject: [PATCH 5/5] lint fix Signed-off-by: Manish Tomar --- registry/storage/catalog.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/registry/storage/catalog.go b/registry/storage/catalog.go index 4db8bd88..ebf80e05 100644 --- a/registry/storage/catalog.go +++ b/registry/storage/catalog.go @@ -72,13 +72,13 @@ func (reg *registry) Enumerate(ctx context.Context, ingester func(string) error) } // Remove removes a repository from storage -func (r *registry) Remove(ctx context.Context, name reference.Named) error { +func (reg *registry) Remove(ctx context.Context, name reference.Named) error { root, err := pathFor(repositoriesRootPathSpec{}) if err != nil { return err } repoDir := path.Join(root, name.Name()) - return r.driver.Delete(ctx, repoDir) + return reg.driver.Delete(ctx, repoDir) } // lessPath returns true if one path a is less than path b.