From 0c15ab69528059d21395c166b3ec3c72ec297c69 Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Wed, 6 Apr 2016 17:01:30 -0700 Subject: [PATCH 1/2] Remove signature store from registry. Return a generated signature for manifest pull. Signed-off-by: Richard Scothern --- configuration/configuration.go | 5 - docs/configuration.md | 19 ---- manifests.go | 6 - notifications/listener_test.go | 7 +- registry/handlers/api_test.go | 4 +- registry/handlers/app.go | 10 +- registry/proxy/proxymanifeststore_test.go | 17 ++- registry/root.go | 2 +- registry/storage/blobstore.go | 1 - registry/storage/garbagecollect.go | 17 --- registry/storage/garbagecollect_test.go | 24 ++-- registry/storage/manifeststore.go | 47 -------- registry/storage/manifeststore_test.go | 43 +------ registry/storage/paths.go | 54 +-------- registry/storage/paths_test.go | 17 +-- registry/storage/registry.go | 22 +--- registry/storage/signaturestore.go | 131 ---------------------- registry/storage/signedmanifesthandler.go | 22 ---- 18 files changed, 39 insertions(+), 409 deletions(-) delete mode 100644 registry/storage/signaturestore.go diff --git a/configuration/configuration.go b/configuration/configuration.go index eee83f8a..59c90fde 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -157,11 +157,6 @@ type Configuration struct { // TrustKey is the signing key to use for adding the signature to // schema1 manifests. TrustKey string `yaml:"signingkeyfile,omitempty"` - - // DisableSignatureStore will cause all signatures attached to schema1 manifests - // to be ignored. Signatures will be generated on all schema1 manifest requests - // rather than only requests which converted schema2 to schema1. - DisableSignatureStore bool `yaml:"disablesignaturestore,omitempty"` } `yaml:"schema1,omitempty"` } `yaml:"compatibility,omitempty"` } diff --git a/docs/configuration.md b/docs/configuration.md index 6a1afb79..c215647d 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -243,7 +243,6 @@ information about each option that appears later in this page. compatibility: schema1: signingkeyfile: /etc/registry/key.json - disablesignaturestore: true In some instances a configuration option is **optional** but it contains child options marked as **required**. This indicates that you can omit the parent with @@ -1730,7 +1729,6 @@ To enable pulling private repositories (e.g. `batman/robin`) a username and pass compatibility: schema1: signingkeyfile: /etc/registry/key.json - disablesignaturestore: true Configure handling of older and deprecated features. Each subsection defines a such a feature with configurable behavior. @@ -1756,23 +1754,6 @@ defines a such a feature with configurable behavior. startup. - - - disablesignaturestore - - - no - - - Disables storage of signatures attached to schema1 manifests. By default - signatures are detached from schema1 manifests, stored, and reattached - when the manifest is requested. When this is true, the storage is disabled - and a new signature is always generated for schema1 manifests using the - schema1 signing key. Disabling signature storage will cause all newly - uploaded signatures to be discarded. Existing stored signatures will not - be removed but they will not be re-attached to the corresponding manifest. - - ## Example: Development configuration diff --git a/manifests.go b/manifests.go index 3bf912a6..2ac7c8f2 100644 --- a/manifests.go +++ b/manifests.go @@ -61,12 +61,6 @@ type ManifestEnumerator interface { Enumerate(ctx context.Context, ingester func(digest.Digest) error) error } -// SignaturesGetter provides an interface for getting the signatures of a schema1 manifest. If the digest -// referred to is not a schema1 manifest, an error should be returned. -type SignaturesGetter interface { - GetSignatures(ctx context.Context, manifestDigest digest.Digest) ([]digest.Digest, error) -} - // Describable is an interface for descriptors type Describable interface { Descriptor() Descriptor diff --git a/notifications/listener_test.go b/notifications/listener_test.go index d16a4560..c7db5944 100644 --- a/notifications/listener_test.go +++ b/notifications/listener_test.go @@ -20,7 +20,12 @@ import ( func TestListener(t *testing.T) { ctx := context.Background() - registry, err := storage.NewRegistry(ctx, inmemory.New(), storage.BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), storage.EnableDelete, storage.EnableRedirect) + k, err := libtrust.GenerateECP256PrivateKey() + if err != nil { + t.Fatal(err) + } + + registry, err := storage.NewRegistry(ctx, inmemory.New(), storage.BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), storage.EnableDelete, storage.EnableRedirect, storage.Schema1SigningKey(k)) if err != nil { t.Fatalf("error creating registry: %v", err) } diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go index 523ecca2..01fd4f4c 100644 --- a/registry/handlers/api_test.go +++ b/registry/handlers/api_test.go @@ -1067,13 +1067,13 @@ func testManifestAPISchema1(t *testing.T, env *testEnv, imageName reference.Name t.Fatalf("error decoding fetched manifest: %v", err) } - // check two signatures were roundtripped + // check only 1 signature is returned signatures, err = fetchedManifestByDigest.Signatures() if err != nil { t.Fatal(err) } - if len(signatures) != 2 { + if len(signatures) != 1 { t.Fatalf("expected 2 signature from manifest, got: %d", len(signatures)) } diff --git a/registry/handlers/app.go b/registry/handlers/app.go index 4bda082b..384a61d6 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -155,6 +155,7 @@ func NewApp(ctx context.Context, config *configuration.Configuration) *App { app.configureRedis(config) app.configureLogHook(config) + options := registrymiddleware.GetRegistryOptions() if config.Compatibility.Schema1.TrustKey != "" { app.trustKey, err = libtrust.LoadKeyFile(config.Compatibility.Schema1.TrustKey) if err != nil { @@ -169,6 +170,8 @@ func NewApp(ctx context.Context, config *configuration.Configuration) *App { } } + options = append(options, storage.Schema1SigningKey(app.trustKey)) + if config.HTTP.Host != "" { u, err := url.Parse(config.HTTP.Host) if err != nil { @@ -177,17 +180,10 @@ func NewApp(ctx context.Context, config *configuration.Configuration) *App { app.httpHost = *u } - options := registrymiddleware.GetRegistryOptions() - if app.isCache { options = append(options, storage.DisableDigestResumption) } - if config.Compatibility.Schema1.DisableSignatureStore { - options = append(options, storage.DisableSchema1Signatures) - options = append(options, storage.Schema1SigningKey(app.trustKey)) - } - // configure deletion if d, ok := config.Storage["delete"]; ok { e, ok := d["enabled"] diff --git a/registry/proxy/proxymanifeststore_test.go b/registry/proxy/proxymanifeststore_test.go index 1069d66c..0d6b7171 100644 --- a/registry/proxy/proxymanifeststore_test.go +++ b/registry/proxy/proxymanifeststore_test.go @@ -60,12 +60,6 @@ func (sm statsManifest) Put(ctx context.Context, manifest distribution.Manifest, return sm.manifests.Put(ctx, manifest) } -/*func (sm statsManifest) Enumerate(ctx context.Context, manifests []distribution.Manifest, last distribution.Manifest) (n int, err error) { - sm.stats["enumerate"]++ - return sm.manifests.Enumerate(ctx, manifests, last) -} -*/ - type mockChallenger struct { sync.Mutex count int @@ -75,7 +69,6 @@ type mockChallenger struct { func (m *mockChallenger) tryEstablishChallenges(context.Context) error { m.Lock() defer m.Unlock() - m.count++ return nil } @@ -93,9 +86,15 @@ func newManifestStoreTestEnv(t *testing.T, name, tag string) *manifestStoreTestE if err != nil { t.Fatalf("unable to parse reference: %s", err) } + k, err := libtrust.GenerateECP256PrivateKey() + if err != nil { + t.Fatal(err) + } ctx := context.Background() - truthRegistry, err := storage.NewRegistry(ctx, inmemory.New(), storage.BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider())) + truthRegistry, err := storage.NewRegistry(ctx, inmemory.New(), + storage.BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), + storage.Schema1SigningKey(k)) if err != nil { t.Fatalf("error creating registry: %v", err) } @@ -117,7 +116,7 @@ func newManifestStoreTestEnv(t *testing.T, name, tag string) *manifestStoreTestE t.Fatalf(err.Error()) } - localRegistry, err := storage.NewRegistry(ctx, inmemory.New(), storage.BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), storage.EnableRedirect, storage.DisableDigestResumption) + localRegistry, err := storage.NewRegistry(ctx, inmemory.New(), storage.BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), storage.EnableRedirect, storage.DisableDigestResumption, storage.Schema1SigningKey(k)) if err != nil { t.Fatalf("error creating registry: %v", err) } diff --git a/registry/root.go b/registry/root.go index 7a7d44cb..5d3005c2 100644 --- a/registry/root.go +++ b/registry/root.go @@ -69,7 +69,7 @@ var GCCmd = &cobra.Command{ os.Exit(1) } - registry, err := storage.NewRegistry(ctx, driver, storage.DisableSchema1Signatures, storage.Schema1SigningKey(k)) + registry, err := storage.NewRegistry(ctx, driver, storage.Schema1SigningKey(k)) if err != nil { fmt.Fprintf(os.Stderr, "failed to construct registry: %v", err) os.Exit(1) diff --git a/registry/storage/blobstore.go b/registry/storage/blobstore.go index 9034cb68..84f6660f 100644 --- a/registry/storage/blobstore.go +++ b/registry/storage/blobstore.go @@ -75,7 +75,6 @@ func (bs *blobStore) Put(ctx context.Context, mediaType string, p []byte) (distr } // TODO(stevvooe): Write out mediatype here, as well. - return distribution.Descriptor{ Size: int64(len(p)), diff --git a/registry/storage/garbagecollect.go b/registry/storage/garbagecollect.go index be64b847..bc340416 100644 --- a/registry/storage/garbagecollect.go +++ b/registry/storage/garbagecollect.go @@ -6,7 +6,6 @@ import ( "github.com/docker/distribution" "github.com/docker/distribution/context" "github.com/docker/distribution/digest" - "github.com/docker/distribution/manifest/schema1" "github.com/docker/distribution/manifest/schema2" "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/storage/driver" @@ -71,22 +70,6 @@ func MarkAndSweep(ctx context.Context, storageDriver driver.StorageDriver, regis } switch manifest.(type) { - case *schema1.SignedManifest: - signaturesGetter, ok := manifestService.(distribution.SignaturesGetter) - if !ok { - return fmt.Errorf("unable to convert ManifestService into SignaturesGetter") - } - signatures, err := signaturesGetter.GetSignatures(ctx, dgst) - if err != nil { - return fmt.Errorf("failed to get signatures for signed manifest: %v", err) - } - for _, signatureDigest := range signatures { - if dryRun { - emit("%s: marking signature %s", repoName, signatureDigest) - } - markSet[signatureDigest] = struct{}{} - } - break case *schema2.DeserializedManifest: config := manifest.(*schema2.DeserializedManifest).Config if dryRun { diff --git a/registry/storage/garbagecollect_test.go b/registry/storage/garbagecollect_test.go index a0ba154b..86fc175a 100644 --- a/registry/storage/garbagecollect_test.go +++ b/registry/storage/garbagecollect_test.go @@ -12,6 +12,7 @@ import ( "github.com/docker/distribution/registry/storage/driver" "github.com/docker/distribution/registry/storage/driver/inmemory" "github.com/docker/distribution/testutil" + "github.com/docker/libtrust" ) type image struct { @@ -22,7 +23,11 @@ type image struct { func createRegistry(t *testing.T, driver driver.StorageDriver) distribution.Namespace { ctx := context.Background() - registry, err := NewRegistry(ctx, driver, EnableDelete) + k, err := libtrust.GenerateECP256PrivateKey() + if err != nil { + t.Fatal(err) + } + registry, err := NewRegistry(ctx, driver, EnableDelete, Schema1SigningKey(k)) if err != nil { t.Fatalf("Failed to construct namespace") } @@ -139,13 +144,13 @@ func TestNoDeletionNoEffect(t *testing.T) { ctx := context.Background() inmemoryDriver := inmemory.New() - registry := createRegistry(t, inmemoryDriver) + registry := createRegistry(t, inmemory.New()) repo := makeRepository(t, registry, "palailogos") manifestService, err := repo.Manifests(ctx) image1 := uploadRandomSchema1Image(t, repo) image2 := uploadRandomSchema1Image(t, repo) - image3 := uploadRandomSchema2Image(t, repo) + uploadRandomSchema2Image(t, repo) // construct manifestlist for fun. blobstatter := registry.BlobStatter() @@ -160,20 +165,17 @@ func TestNoDeletionNoEffect(t *testing.T) { t.Fatalf("Failed to add manifest list: %v", err) } + before := allBlobs(t, registry) + // Run GC err = MarkAndSweep(context.Background(), inmemoryDriver, registry, false) if err != nil { t.Fatalf("Failed mark and sweep: %v", err) } - blobs := allBlobs(t, registry) - - // the +1 at the end is for the manifestList - // the first +3 at the end for each manifest's blob - // the second +3 at the end for each manifest's signature/config layer - totalBlobCount := len(image1.layers) + len(image2.layers) + len(image3.layers) + 1 + 3 + 3 - if len(blobs) != totalBlobCount { - t.Fatalf("Garbage collection affected storage") + after := allBlobs(t, registry) + if len(before) != len(after) { + t.Fatalf("Garbage collection affected storage: %d != %d", len(before), len(after)) } } diff --git a/registry/storage/manifeststore.go b/registry/storage/manifeststore.go index 5a9165f9..68483c95 100644 --- a/registry/storage/manifeststore.go +++ b/registry/storage/manifeststore.go @@ -2,7 +2,6 @@ package storage import ( "fmt" - "path" "encoding/json" "github.com/docker/distribution" @@ -12,7 +11,6 @@ import ( "github.com/docker/distribution/manifest/manifestlist" "github.com/docker/distribution/manifest/schema1" "github.com/docker/distribution/manifest/schema2" - "github.com/docker/distribution/registry/storage/driver" ) // A ManifestHandler gets and puts manifests of a particular type. @@ -141,48 +139,3 @@ func (ms *manifestStore) Enumerate(ctx context.Context, ingester func(digest.Dig }) return err } - -// Only valid for schema1 signed manifests -func (ms *manifestStore) GetSignatures(ctx context.Context, manifestDigest digest.Digest) ([]digest.Digest, error) { - // sanity check that digest refers to a schema1 digest - manifest, err := ms.Get(ctx, manifestDigest) - if err != nil { - return nil, err - } - - if _, ok := manifest.(*schema1.SignedManifest); !ok { - return nil, fmt.Errorf("digest %v is not for schema1 manifest", manifestDigest) - } - - signaturesPath, err := pathFor(manifestSignaturesPathSpec{ - name: ms.repository.Named().Name(), - revision: manifestDigest, - }) - if err != nil { - return nil, err - } - - var digests []digest.Digest - alg := string(digest.SHA256) - signaturePaths, err := ms.blobStore.driver.List(ctx, path.Join(signaturesPath, alg)) - - switch err.(type) { - case nil: - break - case driver.PathNotFoundError: - // Manifest may have been pushed with signature store disabled - return digests, nil - default: - return nil, err - } - - for _, sigPath := range signaturePaths { - sigdigest, err := digest.ParseDigest(alg + ":" + path.Base(sigPath)) - if err != nil { - // merely found not a digest - continue - } - digests = append(digests, sigdigest) - } - return digests, nil -} diff --git a/registry/storage/manifeststore_test.go b/registry/storage/manifeststore_test.go index fcb5adf9..cbd30c04 100644 --- a/registry/storage/manifeststore_test.go +++ b/registry/storage/manifeststore_test.go @@ -52,15 +52,11 @@ func newManifestStoreTestEnv(t *testing.T, name reference.Named, tag string, opt } func TestManifestStorage(t *testing.T) { - testManifestStorage(t, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect) -} - -func TestManifestStorageDisabledSignatures(t *testing.T) { k, err := libtrust.GenerateECP256PrivateKey() if err != nil { t.Fatal(err) } - testManifestStorage(t, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect, DisableSchema1Signatures, Schema1SigningKey(k)) + testManifestStorage(t, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect, Schema1SigningKey(k)) } func testManifestStorage(t *testing.T, options ...RegistryOption) { @@ -71,7 +67,6 @@ func testManifestStorage(t *testing.T, options ...RegistryOption) { if err != nil { t.Fatal(err) } - equalSignatures := env.registry.(*registry).schema1SignaturesEnabled m := schema1.Manifest{ Versioned: manifest.Versioned{ @@ -175,12 +170,6 @@ func testManifestStorage(t *testing.T, options ...RegistryOption) { t.Fatalf("fetched payload does not match original payload: %q != %q", fetchedManifest.Canonical, sm.Canonical) } - if equalSignatures { - if !reflect.DeepEqual(fetchedManifest, sm) { - t.Fatalf("fetched manifest not equal: %#v != %#v", fetchedManifest.Manifest, sm.Manifest) - } - } - _, pl, err := fetchedManifest.Payload() if err != nil { t.Fatalf("error getting payload %#v", err) @@ -223,12 +212,6 @@ func testManifestStorage(t *testing.T, options ...RegistryOption) { t.Fatalf("fetched manifest not equal: %q != %q", byDigestManifest.Canonical, fetchedManifest.Canonical) } - if equalSignatures { - if !reflect.DeepEqual(fetchedByDigest, fetchedManifest) { - t.Fatalf("fetched manifest not equal: %#v != %#v", fetchedByDigest, fetchedManifest) - } - } - sigs, err := fetchedJWS.Signatures() if err != nil { t.Fatalf("unable to extract signatures: %v", err) @@ -285,17 +268,6 @@ func testManifestStorage(t *testing.T, options ...RegistryOption) { t.Fatalf("unexpected error verifying manifest: %v", err) } - // Assemble our payload and two signatures to get what we expect! - expectedJWS, err := libtrust.NewJSONSignature(payload, sigs[0], sigs2[0]) - if err != nil { - t.Fatalf("unexpected error merging jws: %v", err) - } - - expectedSigs, err := expectedJWS.Signatures() - if err != nil { - t.Fatalf("unexpected error getting expected signatures: %v", err) - } - _, pl, err = fetched.Payload() if err != nil { t.Fatalf("error getting payload %#v", err) @@ -315,19 +287,6 @@ func testManifestStorage(t *testing.T, options ...RegistryOption) { t.Fatalf("payloads are not equal") } - if equalSignatures { - receivedSigs, err := receivedJWS.Signatures() - if err != nil { - t.Fatalf("error getting signatures: %v", err) - } - - for i, sig := range receivedSigs { - if !bytes.Equal(sig, expectedSigs[i]) { - t.Fatalf("mismatched signatures from remote: %v != %v", string(sig), string(expectedSigs[i])) - } - } - } - // Test deleting manifests err = ms.Delete(ctx, dgst) if err != nil { diff --git a/registry/storage/paths.go b/registry/storage/paths.go index 8985f043..1b142b88 100644 --- a/registry/storage/paths.go +++ b/registry/storage/paths.go @@ -30,8 +30,6 @@ const ( // revisions // -> // -> link -// -> signatures -// //link // tags/ // -> current/link // -> index @@ -62,8 +60,7 @@ const ( // // The third component of the repository directory is the manifests store, // which is made up of a revision store and tag store. Manifests are stored in -// the blob store and linked into the revision store. Signatures are separated -// from the manifest payload data and linked into the blob store, as well. +// the blob store and linked into the revision store. // While the registry can save all revisions of a manifest, no relationship is // implied as to the ordering of changes to a manifest. The tag store provides // support for name, tag lookups of manifests, using "current/link" under a @@ -77,8 +74,6 @@ const ( // manifestRevisionsPathSpec: /v2/repositories//_manifests/revisions/ // manifestRevisionPathSpec: /v2/repositories//_manifests/revisions/// // manifestRevisionLinkPathSpec: /v2/repositories//_manifests/revisions///link -// manifestSignaturesPathSpec: /v2/repositories//_manifests/revisions///signatures/ -// manifestSignatureLinkPathSpec: /v2/repositories//_manifests/revisions///signatures///link // // Tags: // @@ -148,33 +143,6 @@ func pathFor(spec pathSpec) (string, error) { } return path.Join(root, "link"), nil - case manifestSignaturesPathSpec: - root, err := pathFor(manifestRevisionPathSpec{ - name: v.name, - revision: v.revision, - }) - - if err != nil { - return "", err - } - - return path.Join(root, "signatures"), nil - case manifestSignatureLinkPathSpec: - root, err := pathFor(manifestSignaturesPathSpec{ - name: v.name, - revision: v.revision, - }) - - if err != nil { - return "", err - } - - signatureComponents, err := digestPathComponents(v.signature, false) - if err != nil { - return "", err - } - - return path.Join(root, path.Join(append(signatureComponents, "link")...)), nil case manifestTagsPathSpec: return path.Join(append(repoPrefix, v.name, "_manifests", "tags")...), nil case manifestTagPathSpec: @@ -325,26 +293,6 @@ type manifestRevisionLinkPathSpec struct { func (manifestRevisionLinkPathSpec) pathSpec() {} -// manifestSignaturesPathSpec describes the path components for the directory -// containing all the signatures for the target blob. Entries are named with -// the underlying key id. -type manifestSignaturesPathSpec struct { - name string - revision digest.Digest -} - -func (manifestSignaturesPathSpec) pathSpec() {} - -// manifestSignatureLinkPathSpec describes the path components used to look up -// a signature file by the hash of its blob. -type manifestSignatureLinkPathSpec struct { - name string - revision digest.Digest - signature digest.Digest -} - -func (manifestSignatureLinkPathSpec) pathSpec() {} - // manifestTagsPathSpec describes the path elements required to point to the // manifest tags directory. type manifestTagsPathSpec struct { diff --git a/registry/storage/paths_test.go b/registry/storage/paths_test.go index 91004bd4..f739552a 100644 --- a/registry/storage/paths_test.go +++ b/registry/storage/paths_test.go @@ -26,21 +26,6 @@ func TestPathMapper(t *testing.T) { }, expected: "/docker/registry/v2/repositories/foo/bar/_manifests/revisions/sha256/abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789/link", }, - { - spec: manifestSignatureLinkPathSpec{ - name: "foo/bar", - revision: "sha256:abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789", - signature: "sha256:abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789", - }, - expected: "/docker/registry/v2/repositories/foo/bar/_manifests/revisions/sha256/abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789/signatures/sha256/abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789/link", - }, - { - spec: manifestSignaturesPathSpec{ - name: "foo/bar", - revision: "sha256:abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789", - }, - expected: "/docker/registry/v2/repositories/foo/bar/_manifests/revisions/sha256/abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789/signatures", - }, { spec: manifestTagsPathSpec{ name: "foo/bar", @@ -113,7 +98,7 @@ func TestPathMapper(t *testing.T) { // Add a few test cases to ensure we cover some errors // Specify a path that requires a revision and get a digest validation error. - badpath, err := pathFor(manifestSignaturesPathSpec{ + badpath, err := pathFor(manifestRevisionPathSpec{ name: "foo/bar", }) diff --git a/registry/storage/registry.go b/registry/storage/registry.go index 3fe4ac68..94034b26 100644 --- a/registry/storage/registry.go +++ b/registry/storage/registry.go @@ -18,7 +18,6 @@ type registry struct { blobDescriptorCacheProvider cache.BlobDescriptorCacheProvider deleteEnabled bool resumableDigestEnabled bool - schema1SignaturesEnabled bool schema1SigningKey libtrust.PrivateKey blobDescriptorServiceFactory distribution.BlobDescriptorServiceFactory } @@ -47,17 +46,8 @@ func DisableDigestResumption(registry *registry) error { return nil } -// DisableSchema1Signatures is a functional option for NewRegistry. It disables -// signature storage and ensures all schema1 manifests will only be returned -// with a signature from a provided signing key. -func DisableSchema1Signatures(registry *registry) error { - registry.schema1SignaturesEnabled = false - return nil -} - // Schema1SigningKey returns a functional option for NewRegistry. It sets the -// signing key for adding a signature to all schema1 manifests. This should be -// used in conjunction with disabling signature store. +// key for signing all schema1 manifests. func Schema1SigningKey(key libtrust.PrivateKey) RegistryOption { return func(registry *registry) error { registry.schema1SigningKey = key @@ -116,9 +106,8 @@ func NewRegistry(ctx context.Context, driver storagedriver.StorageDriver, option statter: statter, pathFn: bs.path, }, - statter: statter, - resumableDigestEnabled: true, - schema1SignaturesEnabled: true, + statter: statter, + resumableDigestEnabled: true, } for _, option := range options { @@ -231,11 +220,6 @@ func (repo *repository) Manifests(ctx context.Context, options ...distribution.M ctx: ctx, repository: repo, blobStore: blobStore, - signatures: &signatureStore{ - ctx: ctx, - repository: repo, - blobStore: repo.blobStore, - }, }, schema2Handler: &schema2ManifestHandler{ ctx: ctx, diff --git a/registry/storage/signaturestore.go b/registry/storage/signaturestore.go deleted file mode 100644 index 2940e041..00000000 --- a/registry/storage/signaturestore.go +++ /dev/null @@ -1,131 +0,0 @@ -package storage - -import ( - "path" - "sync" - - "github.com/docker/distribution/context" - "github.com/docker/distribution/digest" -) - -type signatureStore struct { - repository *repository - blobStore *blobStore - ctx context.Context -} - -func (s *signatureStore) Get(dgst digest.Digest) ([][]byte, error) { - signaturesPath, err := pathFor(manifestSignaturesPathSpec{ - name: s.repository.Named().Name(), - revision: dgst, - }) - - if err != nil { - return nil, err - } - - // Need to append signature digest algorithm to path to get all items. - // Perhaps, this should be in the pathMapper but it feels awkward. This - // can be eliminated by implementing listAll on drivers. - signaturesPath = path.Join(signaturesPath, "sha256") - - signaturePaths, err := s.blobStore.driver.List(s.ctx, signaturesPath) - if err != nil { - return nil, err - } - - var wg sync.WaitGroup - type result struct { - index int - signature []byte - err error - } - ch := make(chan result) - - bs := s.linkedBlobStore(s.ctx, dgst) - for i, sigPath := range signaturePaths { - sigdgst, err := digest.ParseDigest("sha256:" + path.Base(sigPath)) - if err != nil { - context.GetLogger(s.ctx).Errorf("could not get digest from path: %q, skipping", sigPath) - continue - } - - wg.Add(1) - go func(idx int, sigdgst digest.Digest) { - defer wg.Done() - context.GetLogger(s.ctx). - Debugf("fetching signature %q", sigdgst) - - r := result{index: idx} - - if p, err := bs.Get(s.ctx, sigdgst); err != nil { - context.GetLogger(s.ctx). - Errorf("error fetching signature %q: %v", sigdgst, err) - r.err = err - } else { - r.signature = p - } - - ch <- r - }(i, sigdgst) - } - done := make(chan struct{}) - go func() { - wg.Wait() - close(done) - }() - - // aggregrate the results - signatures := make([][]byte, len(signaturePaths)) -loop: - for { - select { - case result := <-ch: - signatures[result.index] = result.signature - if result.err != nil && err == nil { - // only set the first one. - err = result.err - } - case <-done: - break loop - } - } - - return signatures, err -} - -func (s *signatureStore) Put(dgst digest.Digest, signatures ...[]byte) error { - bs := s.linkedBlobStore(s.ctx, dgst) - for _, signature := range signatures { - if _, err := bs.Put(s.ctx, "application/json", signature); err != nil { - return err - } - } - return nil -} - -// linkedBlobStore returns the namedBlobStore of the signatures for the -// manifest with the given digest. Effectively, each signature link path -// layout is a unique linked blob store. -func (s *signatureStore) linkedBlobStore(ctx context.Context, revision digest.Digest) *linkedBlobStore { - linkpath := func(name string, dgst digest.Digest) (string, error) { - return pathFor(manifestSignatureLinkPathSpec{ - name: name, - revision: revision, - signature: dgst, - }) - - } - - return &linkedBlobStore{ - ctx: ctx, - repository: s.repository, - blobStore: s.blobStore, - blobAccessController: &linkedBlobStatter{ - blobStore: s.blobStore, - repository: s.repository, - linkPathFns: []linkPathFunc{linkpath}, - }, - linkPathFns: []linkPathFunc{linkpath}, - } -} diff --git a/registry/storage/signedmanifesthandler.go b/registry/storage/signedmanifesthandler.go index 8e13dd93..df6369f3 100644 --- a/registry/storage/signedmanifesthandler.go +++ b/registry/storage/signedmanifesthandler.go @@ -18,7 +18,6 @@ type signedManifestHandler struct { repository *repository blobStore *linkedBlobStore ctx context.Context - signatures *signatureStore } var _ ManifestHandler = &signedManifestHandler{} @@ -30,13 +29,6 @@ func (ms *signedManifestHandler) Unmarshal(ctx context.Context, dgst digest.Dige signatures [][]byte err error ) - if ms.repository.schema1SignaturesEnabled { - // Fetch the signatures for the manifest - signatures, err = ms.signatures.Get(dgst) - if err != nil { - return nil, err - } - } jsig, err := libtrust.NewJSONSignature(content, signatures...) if err != nil { @@ -47,8 +39,6 @@ func (ms *signedManifestHandler) Unmarshal(ctx context.Context, dgst digest.Dige if err := jsig.Sign(ms.repository.schema1SigningKey); err != nil { return nil, err } - } else if !ms.repository.schema1SignaturesEnabled { - return nil, fmt.Errorf("missing signing key with signature store disabled") } // Extract the pretty JWS @@ -90,18 +80,6 @@ func (ms *signedManifestHandler) Put(ctx context.Context, manifest distribution. return "", err } - if ms.repository.schema1SignaturesEnabled { - // Grab each json signature and store them. - signatures, err := sm.Signatures() - if err != nil { - return "", err - } - - if err := ms.signatures.Put(revision.Digest, signatures...); err != nil { - return "", err - } - } - return revision.Digest, nil } From febcee65645cf56a1f359485f738ac0a573b9a81 Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Fri, 27 May 2016 11:30:42 -0700 Subject: [PATCH 2/2] Add a deprecation document detailing signature store removal Signed-off-by: Richard Scothern --- cmd/registry/config-dev.yml | 2 +- docs/deprecated.md | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 docs/deprecated.md diff --git a/cmd/registry/config-dev.yml b/cmd/registry/config-dev.yml index b6438be5..42c9dc16 100644 --- a/cmd/registry/config-dev.yml +++ b/cmd/registry/config-dev.yml @@ -24,7 +24,7 @@ storage: cache: blobdescriptor: redis filesystem: - rootdirectory: /var/lib/registry + rootdirectory: /var/lib/registry-hello-world-clean maintenance: uploadpurging: enabled: false diff --git a/docs/deprecated.md b/docs/deprecated.md new file mode 100644 index 00000000..796d3fa5 --- /dev/null +++ b/docs/deprecated.md @@ -0,0 +1,24 @@ + + +# Docker Registry Deprecation + +This document details functionality or components which are deprecated within +the registry. + +### v2.5.0 + +The signature store has been removed from the registry. Since `v2.4.0` it has +been possible to configure the registry to generate manifest signatures rather +than load them from storage. In this version of the registry this becomes +the default behavior. Signatures which are attached to manifests on put are +not stored in the registry. This does not alter the functional behavior of +the registry. + +Old signatures blobs can be removed from the registry storage by running the +garbage-collect subcommand.