From add4ea684a85219a4aca2fe0b52663b61b778cac Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Wed, 18 Jan 2017 14:46:30 +0100 Subject: [PATCH] *: pluggable transports Signed-off-by: Antonio Murdaca --- directory/directory_transport.go | 5 ++ docker/daemon/daemon_transport.go | 5 ++ docker/docker_transport.go | 5 ++ oci/layout/oci_transport.go | 5 ++ openshift/openshift_transport.go | 5 ++ signature/policy_config.go | 8 +- signature/policy_config_test.go | 4 +- storage/storage_transport.go | 5 ++ transports/alltransports/alltransports.go | 31 ++++++++ .../alltransports_test.go} | 11 +-- transports/transports.go | 74 +++++++++---------- 11 files changed, 105 insertions(+), 53 deletions(-) create mode 100644 transports/alltransports/alltransports.go rename transports/{transports_test.go => alltransports/alltransports_test.go} (86%) diff --git a/directory/directory_transport.go b/directory/directory_transport.go index 89d2565aac..34f7428935 100644 --- a/directory/directory_transport.go +++ b/directory/directory_transport.go @@ -10,10 +10,15 @@ import ( "github.com/containers/image/directory/explicitfilepath" "github.com/containers/image/docker/reference" "github.com/containers/image/image" + "github.com/containers/image/transports" "github.com/containers/image/types" "github.com/opencontainers/go-digest" ) +func init() { + transports.Register(Transport) +} + // Transport is an ImageTransport for directory paths. var Transport = dirTransport{} diff --git a/docker/daemon/daemon_transport.go b/docker/daemon/daemon_transport.go index d64f088f00..41ccd1f197 100644 --- a/docker/daemon/daemon_transport.go +++ b/docker/daemon/daemon_transport.go @@ -5,10 +5,15 @@ import ( "github.com/containers/image/docker/reference" "github.com/containers/image/image" + "github.com/containers/image/transports" "github.com/containers/image/types" "github.com/opencontainers/go-digest" ) +func init() { + transports.Register(Transport) +} + // Transport is an ImageTransport for images managed by a local Docker daemon. var Transport = daemonTransport{} diff --git a/docker/docker_transport.go b/docker/docker_transport.go index a8f511a0a5..15d68e993c 100644 --- a/docker/docker_transport.go +++ b/docker/docker_transport.go @@ -6,10 +6,15 @@ import ( "github.com/containers/image/docker/policyconfiguration" "github.com/containers/image/docker/reference" + "github.com/containers/image/transports" "github.com/containers/image/types" "github.com/pkg/errors" ) +func init() { + transports.Register(Transport) +} + // Transport is an ImageTransport for Docker registry-hosted images. var Transport = dockerTransport{} diff --git a/oci/layout/oci_transport.go b/oci/layout/oci_transport.go index 734af87c0b..007798b4cd 100644 --- a/oci/layout/oci_transport.go +++ b/oci/layout/oci_transport.go @@ -9,11 +9,16 @@ import ( "github.com/containers/image/directory/explicitfilepath" "github.com/containers/image/docker/reference" "github.com/containers/image/image" + "github.com/containers/image/transports" "github.com/containers/image/types" "github.com/opencontainers/go-digest" "github.com/pkg/errors" ) +func init() { + transports.Register(Transport) +} + // Transport is an ImageTransport for OCI directories. var Transport = ociTransport{} diff --git a/openshift/openshift_transport.go b/openshift/openshift_transport.go index 119385bbc3..108e110232 100644 --- a/openshift/openshift_transport.go +++ b/openshift/openshift_transport.go @@ -8,10 +8,15 @@ import ( "github.com/containers/image/docker/policyconfiguration" "github.com/containers/image/docker/reference" genericImage "github.com/containers/image/image" + "github.com/containers/image/transports" "github.com/containers/image/types" "github.com/pkg/errors" ) +func init() { + transports.Register(Transport) +} + // Transport is an ImageTransport for OpenShift registry-hosted images. var Transport = openshiftTransport{} diff --git a/signature/policy_config.go b/signature/policy_config.go index ace24fec88..ae9b1df959 100644 --- a/signature/policy_config.go +++ b/signature/policy_config.go @@ -122,10 +122,8 @@ func (m *policyTransportsMap) UnmarshalJSON(data []byte) error { // So, use a temporary map of pointers-to-slices and convert. tmpMap := map[string]*PolicyTransportScopes{} if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { - transport, ok := transports.KnownTransports[key] - if !ok { - return nil - } + // transport can be nil + transport := transports.Get(key) // paranoidUnmarshalJSONObject detects key duplication for us, check just to be safe. if _, ok := tmpMap[key]; ok { return nil @@ -174,7 +172,7 @@ func (m *policyTransportScopesWithTransport) UnmarshalJSON(data []byte) error { if _, ok := tmpMap[key]; ok { return nil } - if key != "" { + if key != "" && m.transport != nil { if err := m.transport.ValidatePolicyConfigurationScope(key); err != nil { return nil } diff --git a/signature/policy_config_test.go b/signature/policy_config_test.go index c7b3278f39..85152950da 100644 --- a/signature/policy_config_test.go +++ b/signature/policy_config_test.go @@ -9,6 +9,7 @@ import ( "github.com/containers/image/directory" "github.com/containers/image/docker" + _ "github.com/containers/image/openshift" "github.com/containers/image/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -269,9 +270,6 @@ func TestPolicyUnmarshalJSON(t *testing.T) { func(v mSI) { v["transports"] = []string{} }, // "default" is an invalid PolicyRequirements func(v mSI) { v["default"] = PolicyRequirements{} }, - // A key in "transports" is an invalid transport name - func(v mSI) { x(v, "transports")["this is unknown"] = x(v, "transports")["docker"] }, - func(v mSI) { x(v, "transports")[""] = x(v, "transports")["docker"] }, } for _, fn := range breakFns { err = tryUnmarshalModifiedPolicy(t, &p, validJSON, fn) diff --git a/storage/storage_transport.go b/storage/storage_transport.go index 78c7ef651a..e9982175fa 100644 --- a/storage/storage_transport.go +++ b/storage/storage_transport.go @@ -9,12 +9,17 @@ import ( "github.com/Sirupsen/logrus" "github.com/containers/image/docker/reference" + "github.com/containers/image/transports" "github.com/containers/image/types" "github.com/containers/storage/storage" "github.com/opencontainers/go-digest" ddigest "github.com/opencontainers/go-digest" ) +func init() { + transports.Register(Transport) +} + var ( // Transport is an ImageTransport that uses either a default // storage.Store or one that's it's explicitly told to use. diff --git a/transports/alltransports/alltransports.go b/transports/alltransports/alltransports.go new file mode 100644 index 0000000000..eb5ddcd984 --- /dev/null +++ b/transports/alltransports/alltransports.go @@ -0,0 +1,31 @@ +package alltransports + +import ( + "strings" + + // register all known transports + // NOTE: Make sure docs/policy.json.md is updated when adding or updating + // a transport. + _ "github.com/containers/image/directory" + _ "github.com/containers/image/docker" + _ "github.com/containers/image/docker/daemon" + _ "github.com/containers/image/oci/layout" + _ "github.com/containers/image/openshift" + _ "github.com/containers/image/storage" + "github.com/containers/image/transports" + "github.com/containers/image/types" + "github.com/pkg/errors" +) + +// ParseImageName converts a URL-like image name to a types.ImageReference. +func ParseImageName(imgName string) (types.ImageReference, error) { + parts := strings.SplitN(imgName, ":", 2) + if len(parts) != 2 { + return nil, errors.Errorf(`Invalid image name "%s", expected colon-separated transport:reference`, imgName) + } + transport := transports.Get(parts[0]) + if transport == nil { + return nil, errors.Errorf(`Invalid image name "%s", unknown transport "%s"`, imgName, parts[0]) + } + return transport.ParseReference(parts[1]) +} diff --git a/transports/transports_test.go b/transports/alltransports/alltransports_test.go similarity index 86% rename from transports/transports_test.go rename to transports/alltransports/alltransports_test.go index 2daec0ddfa..8401d5d933 100644 --- a/transports/transports_test.go +++ b/transports/alltransports/alltransports_test.go @@ -1,17 +1,13 @@ -package transports +package alltransports import ( "testing" + "github.com/containers/image/transports" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestKnownTransports(t *testing.T) { - assert.NotNil(t, KnownTransports) // Ensure that the initialization has actually been run - assert.True(t, len(KnownTransports) > 1) -} - func TestParseImageName(t *testing.T) { // This primarily tests error handling, TestImageNameHandling is a table-driven // test for the expected values. @@ -36,11 +32,12 @@ func TestImageNameHandling(t *testing.T) { {"docker-daemon", "busybox:latest", "busybox:latest"}, {"oci", "/etc:sometag", "/etc:sometag"}, // "atomic" not tested here because it depends on per-user configuration for the default cluster. + // "containers-storage" not tested here because it needs to initialize various directories on the fs. } { fullInput := c.transport + ":" + c.input ref, err := ParseImageName(fullInput) require.NoError(t, err, fullInput) - s := ImageName(ref) + s := transports.ImageName(ref) assert.Equal(t, c.transport+":"+c.roundtrip, s, fullInput) } } diff --git a/transports/transports.go b/transports/transports.go index 04826c047b..dfc1fdefa8 100644 --- a/transports/transports.go +++ b/transports/transports.go @@ -2,52 +2,50 @@ package transports import ( "fmt" - "strings" - - "github.com/containers/image/directory" - "github.com/containers/image/docker" - "github.com/containers/image/docker/daemon" - ociLayout "github.com/containers/image/oci/layout" - "github.com/containers/image/openshift" - "github.com/containers/image/storage" + "sync" + "github.com/containers/image/types" - "github.com/pkg/errors" ) -// KnownTransports is a registry of known ImageTransport instances. -var KnownTransports map[string]types.ImageTransport +// knownTransports is a registry of known ImageTransport instances. +type knownTransports struct { + transports map[string]types.ImageTransport + mu sync.Mutex +} -func init() { - KnownTransports = make(map[string]types.ImageTransport) - // NOTE: Make sure docs/policy.json.md is updated when adding or updating - // a transport. - for _, t := range []types.ImageTransport{ - directory.Transport, - docker.Transport, - daemon.Transport, - ociLayout.Transport, - openshift.Transport, - storage.Transport, - } { - name := t.Name() - if _, ok := KnownTransports[name]; ok { - panic(fmt.Sprintf("Duplicate image transport name %s", name)) - } - KnownTransports[name] = t - } +func (kt *knownTransports) Get(k string) types.ImageTransport { + kt.mu.Lock() + t := kt.transports[k] + kt.mu.Unlock() + return t } -// ParseImageName converts a URL-like image name to a types.ImageReference. -func ParseImageName(imgName string) (types.ImageReference, error) { - parts := strings.SplitN(imgName, ":", 2) - if len(parts) != 2 { - return nil, errors.Errorf(`Invalid image name "%s", expected colon-separated transport:reference`, imgName) +func (kt *knownTransports) Add(t types.ImageTransport) { + kt.mu.Lock() + name := t.Name() + if t := kt.transports[name]; t != nil { + panic(fmt.Sprintf("Duplicate image transport name %s", name)) } - transport, ok := KnownTransports[parts[0]] - if !ok { - return nil, errors.Errorf(`Invalid image name "%s", unknown transport "%s"`, imgName, parts[0]) + kt.transports[name] = t + kt.mu.Unlock() +} + +var kt *knownTransports + +func init() { + kt = &knownTransports{ + transports: make(map[string]types.ImageTransport), } - return transport.ParseReference(parts[1]) +} + +// Get TODO(runcom) +func Get(name string) types.ImageTransport { + return kt.Get(name) +} + +// Register TODO(runcom) +func Register(t types.ImageTransport) { + kt.Add(t) } // ImageName converts a types.ImageReference into an URL-like image name, which MUST be such that