Change the way a context is stored

Initially we stored the context data in the `Metadata` of the context
but in hindsight this data would be better of in the `Endpoints` because
that's what it is used for.

Before:
```json
{
  "Name": "aci",
  "Metadata": {
    "Type": "aci",
    "Data": {
      "key": "value"
    }
  },
  "Endpoints": {
      "docker": {}
  }
}
```

After:
```json
{
  "Name": "aci",
  "Type": "aci",
  "Metadata": {},
  "Endpoints": {
      "aci": {
          "key": "value"
      },
      "docker": {}
  }
}
```

With this change the contexts that we create are more in line with the contexts the docker cli creates.

It also makes the code less complicated since we don't need to marsal twice any more. The API is nicer too:

```go
// Get a context:
c, err := store.Get(contextName)

// Get the stored endpoint:
var aciContext store.AciContext
if err := contextStore.GetEndpoint(currentContext, &aciContext); err != nil {
	return nil, err
}
```

.github/PULL_REQUEST_TEMPLATE.md

@@ -1,8 +1,6 @@
 **What I did**
 
 **Related issue**
-<-- If this is a bug fix, make sure your description includes "fixes #xxxx", or
-"closes #xxxx"
--->
+<-- If this is a bug fix, make sure your description includes "fixes #xxxx", or "closes #xxxx" -->
 
 **(not mandatory) A picture of a cute animal, if possible in relation with what you did**

azure/backend.go

@@ -36,19 +36,15 @@ func init() {
 	})
 }
 
-func getter() interface{} {
-	return &store.AciContext{}
-}
-
 // New creates a backend that can manage containers
 func New(ctx context.Context) (backend.Service, error) {
 	currentContext := apicontext.CurrentContext(ctx)
 	contextStore := store.ContextStore(ctx)
-	metadata, err := contextStore.Get(currentContext, getter)
-	if err != nil {
-		return nil, errors.Wrap(err, "wrong context type")
+
+	var aciContext store.AciContext
+	if err := contextStore.GetEndpoint(currentContext, &aciContext); err != nil {
+		return nil, err
 	}
-	aciContext, _ := metadata.Metadata.Data.(store.AciContext)
 
 	auth, _ := login.NewAuthorizerFromLogin()
 	containerGroupsClient := containerinstance.NewContainerGroupsClient(aciContext.SubscriptionID)

cli/cmd/context/create.go

@@ -67,9 +67,7 @@ func runCreate(ctx context.Context, opts createOpts, name string, contextType st
 		return createACIContext(ctx, name, opts)
 	default:
 		s := store.ContextStore(ctx)
-		return s.Create(name, store.TypedContext{
-			Type:        contextType,
-			Description: opts.description,
-		})
+		// TODO: we need to implement different contexts for known backends
+		return s.Create(name, contextType, opts.description, store.ExampleContext{})
 	}
 }

cli/cmd/context/createaci.go

@@ -29,19 +29,27 @@ package context
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/docker/api/context/store"
 )
 
 func createACIContext(ctx context.Context, name string, opts createOpts) error {
 	s := store.ContextStore(ctx)
-	return s.Create(name, store.TypedContext{
-		Type:        "aci",
-		Description: opts.description,
-		Data: store.AciContext{
+
+	description := fmt.Sprintf("%s@%s", opts.aciResourceGroup, opts.aciLocation)
+	if opts.description != "" {
+		description = fmt.Sprintf("%s (%s)", opts.description, description)
+	}
+
+	return s.Create(
+		name,
+		store.AciContextType,
+		description,
+		store.AciContext{
 			SubscriptionID: opts.aciSubscriptionID,
 			Location:       opts.aciLocation,
 			ResourceGroup:  opts.aciResourceGroup,
 		},
-	})
+	)
 }

cli/cmd/context/ls.go

@@ -79,7 +79,7 @@ func runList(ctx context.Context) error {
 		fmt.Fprintf(w,
 			format,
 			contextName,
-			c.Metadata.Type,
+			c.Type,
 			c.Metadata.Description,
 			getEndpoint("docker", c.Endpoints),
 			getEndpoint("kubernetes", c.Endpoints),
@@ -89,15 +89,19 @@ func runList(ctx context.Context) error {
 	return w.Flush()
 }
 
-func getEndpoint(name string, meta map[string]store.Endpoint) string {
-	d, ok := meta[name]
+func getEndpoint(name string, meta map[string]interface{}) string {
+	endpoints, ok := meta[name]
+	if !ok {
+		return ""
+	}
+	data, ok := endpoints.(store.Endpoint)
 	if !ok {
 		return ""
 	}
 
-	result := d.Host
-	if d.DefaultNamespace != "" {
-		result += fmt.Sprintf(" (%s)", d.DefaultNamespace)
+	result := data.Host
+	if data.DefaultNamespace != "" {
+		result += fmt.Sprintf(" (%s)", data.DefaultNamespace)
 	}
 
 	return result

cli/cmd/context/show.go

@@ -53,7 +53,7 @@ func runShow(ctx context.Context) error {
 	// Match behavior of existing CLI
 	if name != store.DefaultContextName {
 		s := store.ContextStore(ctx)
-		if _, err := s.Get(name, nil); err != nil {
+		if _, err := s.Get(name); err != nil {
 			return err
 		}
 	}

cli/cmd/context/use.go

@@ -52,7 +52,7 @@ func runUse(ctx context.Context, name string) error {
 	s := store.ContextStore(ctx)
 	// Match behavior of existing CLI
 	if name != store.DefaultContextName {
-		if _, err := s.Get(name, nil); err != nil {
+		if _, err := s.Get(name); err != nil {
 			return err
 		}
 	}

cli/cmd/serve.go

@@ -77,7 +77,7 @@ func (cs *cliServer) Contexts(ctx context.Context, request *cliv1.ContextsReques
 	for _, c := range contexts {
 		result.Contexts = append(result.Contexts, &cliv1.Context{
 			Name:        c.Name,
-			ContextType: c.Metadata.Type,
+			ContextType: c.Type,
 		})
 	}
 	return result, nil

cli/main.go

@@ -182,7 +182,7 @@ func execMoby(ctx context.Context) {
 	currentContext := apicontext.CurrentContext(ctx)
 	s := store.ContextStore(ctx)
 
-	_, err := s.Get(currentContext, nil)
+	_, err := s.Get(currentContext)
 	// Only run original docker command if the current context is not
 	// ours.
 	if err != nil {

client/client.go

@@ -44,19 +44,18 @@ func New(ctx context.Context) (*Client, error) {
 	currentContext := apicontext.CurrentContext(ctx)
 	s := store.ContextStore(ctx)
 
-	cc, err := s.Get(currentContext, nil)
+	cc, err := s.Get(currentContext)
 	if err != nil {
 		return nil, err
 	}
-	contextType := s.GetType(cc)
 
-	service, err := backend.Get(ctx, contextType)
+	service, err := backend.Get(ctx, cc.Type)
 	if err != nil {
 		return nil, err
 	}
 
 	return &Client{
-		backendType: contextType,
+		backendType: cc.Type,
 		bs:          service,
 	}, nil
 

context/store/store.go

@@ -72,18 +72,66 @@ func ContextStore(ctx context.Context) Store {
 type Store interface {
 	// Get returns the context with name, it returns an error if the  context
 	// doesn't exist
-	Get(name string, getter func() interface{}) (*Metadata, error)
-	// GetType returns the type of the context (docker, aci etc)
-	GetType(meta *Metadata) string
+	Get(name string) (*Metadata, error)
+	// GetEndpoint sets the `v` parameter to the value of the endpoint for a
+	// particular context type
+	GetEndpoint(name string, v interface{}) error
 	// Create creates a new context, it returns an error if a context with the
 	// same name exists already.
-	Create(name string, data TypedContext) error
+	Create(name string, contextType string, description string, data interface{}) error
 	// List returns the list of created contexts
 	List() ([]*Metadata, error)
 	// Remove removes a context by name from the context store
 	Remove(name string) error
 }
 
+// Endpoint holds the Docker or the Kubernetes endpoint, they both have the
+// `Host` property, only kubernetes will have the `DefaultNamespace`
+type Endpoint struct {
+	Host             string `json:",omitempty"`
+	DefaultNamespace string `json:",omitempty"`
+}
+
+const (
+	// AciContextType is the endpoint key in the context endpoints for an ACI
+	// backend
+	AciContextType = "aci"
+	// MobyContextType is the endpoint key in the context endpoints for a moby
+	// backend
+	MobyContextType = "moby"
+	// ExampleContextType is the endpoint key in the context endpoints for an
+	// example backend
+	ExampleContextType = "example"
+)
+
+// Metadata represents the docker context metadata
+type Metadata struct {
+	Name      string                 `json:",omitempty"`
+	Type      string                 `json:",omitempty"`
+	Metadata  ContextMetadata        `json:",omitempty"`
+	Endpoints map[string]interface{} `json:",omitempty"`
+}
+
+// ContextMetadata is represtentation of the data we put in a context
+// metadata
+type ContextMetadata struct {
+	Description       string `json:",omitempty"`
+	StackOrchestrator string `json:",omitempty"`
+}
+
+// AciContext is the context for the ACI backend
+type AciContext struct {
+	SubscriptionID string `json:",omitempty"`
+	Location       string `json:",omitempty"`
+	ResourceGroup  string `json:",omitempty"`
+}
+
+// MobyContext is the context for the moby backend
+type MobyContext struct{}
+
+// ExampleContext is the context for the example backend
+type ExampleContext struct{}
+
 type store struct {
 	root string
 }
@@ -127,9 +175,9 @@ func New(opts ...Opt) (Store, error) {
 }
 
 // Get returns the context with the given name
-func (s *store) Get(name string, getter func() interface{}) (*Metadata, error) {
+func (s *store) Get(name string) (*Metadata, error) {
 	meta := filepath.Join(s.root, contextsDir, metadataDir, contextDirOf(name), metaFile)
-	m, err := read(meta, getter)
+	m, err := read(meta)
 	if os.IsNotExist(err) {
 		return nil, errors.Wrap(errdefs.ErrNotFound, objectName(name))
 	} else if err != nil {
@@ -139,73 +187,75 @@ func (s *store) Get(name string, getter func() interface{}) (*Metadata, error) {
 	return m, nil
 }
 
-func read(meta string, getter func() interface{}) (*Metadata, error) {
+func (s *store) GetEndpoint(name string, data interface{}) error {
+	meta, err := s.Get(name)
+	if err != nil {
+		return err
+	}
+	if _, ok := meta.Endpoints[meta.Type]; !ok {
+		return errors.Wrapf(errdefs.ErrNotFound, "endpoint of type %q", meta.Type)
+	}
+
+	dstPtrValue := reflect.ValueOf(data)
+	dstValue := reflect.Indirect(dstPtrValue)
+
+	val := reflect.ValueOf(meta.Endpoints[meta.Type])
+	valIndirect := reflect.Indirect(val)
+
+	if dstValue.Type() != valIndirect.Type() {
+		return errdefs.ErrWrongContextType
+	}
+
+	dstValue.Set(valIndirect)
+
+	return nil
+}
+
+func read(meta string) (*Metadata, error) {
 	bytes, err := ioutil.ReadFile(meta)
 	if err != nil {
 		return nil, err
 	}
 
-	var um untypedMetadata
-	if err := json.Unmarshal(bytes, &um); err != nil {
+	var metadata Metadata
+	if err := json.Unmarshal(bytes, &metadata); err != nil {
 		return nil, err
 	}
 
-	var uc untypedContext
-	if err := json.Unmarshal(um.Metadata, &uc); err != nil {
+	metadata.Endpoints, err = toTypedEndpoints(metadata.Endpoints)
+	if err != nil {
 		return nil, err
 	}
-	if uc.Type == "" {
-		uc.Type = "docker"
-	}
 
-	var data interface{}
-	if uc.Data != nil {
-		data, err = parse(uc.Data, getter)
+	return &metadata, nil
+}
+
+func toTypedEndpoints(endpoints map[string]interface{}) (map[string]interface{}, error) {
+	result := map[string]interface{}{}
+	for k, v := range endpoints {
+		bytes, err := json.Marshal(v)
 		if err != nil {
 			return nil, err
 		}
-	}
+		typeGetters := getters()
+		if _, ok := typeGetters[k]; !ok {
+			result[k] = v
+			continue
+		}
 
-	return &Metadata{
-		Name:      um.Name,
-		Endpoints: um.Endpoints,
-		Metadata: TypedContext{
-			StackOrchestrator: uc.StackOrchestrator,
-			Description:       uc.Description,
-			Type:              uc.Type,
-			Data:              data,
-		},
-	}, nil
-}
-
-func parse(payload []byte, getter func() interface{}) (interface{}, error) {
-	if getter == nil {
-		var res map[string]interface{}
-		if err := json.Unmarshal(payload, &res); err != nil {
+		val := typeGetters[k]()
+		err = json.Unmarshal(bytes, &val)
+		if err != nil {
 			return nil, err
 		}
-		return res, nil
+
+		result[k] = val
 	}
 
-	typed := getter()
-	if err := json.Unmarshal(payload, &typed); err != nil {
-		return nil, err
-	}
-
-	return reflect.ValueOf(typed).Elem().Interface(), nil
+	return result, nil
 }
 
-func (s *store) GetType(meta *Metadata) string {
-	for k := range meta.Endpoints {
-		if k != dockerEndpointKey {
-			return k
-		}
-	}
-
-	return dockerEndpointKey
-}
-
-func (s *store) Create(name string, data TypedContext) error {
+func (s *store) Create(name string, contextType string, description string, data interface{}) error {
 	if name == DefaultContextName {
 		return errors.Wrap(errdefs.ErrAlreadyExists, objectName(name))
 	}
@@ -220,16 +270,15 @@ func (s *store) Create(name string, data TypedContext) error {
 		return err
 	}
 
-	if data.Data == nil {
-		data.Data = dummyContext{}
-	}
-
 	meta := Metadata{
-		Name:     name,
-		Metadata: data,
-		Endpoints: map[string]Endpoint{
-			(dockerEndpointKey): {},
-			(data.Type):         {},
+		Name: name,
+		Type: contextType,
+		Metadata: ContextMetadata{
+			Description: description,
+		},
+		Endpoints: map[string]interface{}{
+			(dockerEndpointKey): data,
+			(contextType):       data,
 		},
 	}
 
@@ -252,7 +301,7 @@ func (s *store) List() ([]*Metadata, error) {
 	for _, fi := range c {
 		if fi.IsDir() {
 			meta := filepath.Join(root, fi.Name(), metaFile)
-			r, err := read(meta, nil)
+			r, err := read(meta)
 			if err != nil {
 				return nil, err
 			}
@@ -303,45 +352,19 @@ func createDirIfNotExist(dir string) error {
 	return nil
 }
 
-type dummyContext struct{}
-
-// Endpoint holds the Docker or the Kubernetes endpoint
-type Endpoint struct {
-	Host             string `json:",omitempty"`
-	DefaultNamespace string `json:",omitempty"`
-}
-
-// Metadata represents the docker context metadata
-type Metadata struct {
-	Name      string              `json:",omitempty"`
-	Metadata  TypedContext        `json:",omitempty"`
-	Endpoints map[string]Endpoint `json:",omitempty"`
-}
-
-type untypedMetadata struct {
-	Name      string              `json:",omitempty"`
-	Metadata  json.RawMessage     `json:",omitempty"`
-	Endpoints map[string]Endpoint `json:",omitempty"`
-}
-
-type untypedContext struct {
-	StackOrchestrator string          `json:",omitempty"`
-	Type              string          `json:",omitempty"`
-	Description       string          `json:",omitempty"`
-	Data              json.RawMessage `json:",omitempty"`
-}
-
-// TypedContext is a context with a type (moby, aci, etc...)
-type TypedContext struct {
-	StackOrchestrator string      `json:",omitempty"`
-	Type              string      `json:",omitempty"`
-	Description       string      `json:",omitempty"`
-	Data              interface{} `json:",omitempty"`
-}
-
-// AciContext is the context for ACI
-type AciContext struct {
-	SubscriptionID string `json:",omitempty"`
-	Location       string `json:",omitempty"`
-	ResourceGroup  string `json:",omitempty"`
+// Different context types managed by the store.
+// TODO(rumpl): we should make this extensible in the future if we want to
+// be able to manage other contexts.
+func getters() map[string]func() interface{} {
+	return map[string]func() interface{}{
+		"aci": func() interface{} {
+			return &AciContext{}
+		},
+		"moby": func() interface{} {
+			return &MobyContext{}
+		},
+		"example": func() interface{} {
+			return &ExampleContext{}
+		},
+	}
 }

context/store/store_test.go

@@ -33,6 +33,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
@@ -62,42 +63,55 @@ func (suite *StoreTestSuite) AfterTest(suiteName, testName string) {
 }
 
 func (suite *StoreTestSuite) TestCreate() {
-	err := suite.store.Create("test", TypedContext{})
+	err := suite.store.Create("test", "test", "description", ContextMetadata{})
 	require.Nil(suite.T(), err)
 
-	err = suite.store.Create("test", TypedContext{})
+	err = suite.store.Create("test", "test", "descrsiption", ContextMetadata{})
 	require.EqualError(suite.T(), err, `context "test": already exists`)
 	require.True(suite.T(), errdefs.IsAlreadyExistsError(err))
 }
 
+func (suite *StoreTestSuite) TestGetEndpoint() {
+	err := suite.store.Create("aci", "aci", "description", AciContext{
+		Location: "eu",
+	})
+	require.Nil(suite.T(), err)
+
+	var ctx AciContext
+	err = suite.store.GetEndpoint("aci", &ctx)
+	assert.Equal(suite.T(), nil, err)
+	assert.Equal(suite.T(), "eu", ctx.Location)
+
+	var exampleCtx ExampleContext
+	err = suite.store.GetEndpoint("aci", &exampleCtx)
+	assert.EqualError(suite.T(), err, "wrong context type")
+}
+
 func (suite *StoreTestSuite) TestGetUnknown() {
-	meta, err := suite.store.Get("unknown", nil)
+	meta, err := suite.store.Get("unknown")
 	require.Nil(suite.T(), meta)
 	require.EqualError(suite.T(), err, `context "unknown": not found`)
 	require.True(suite.T(), errdefs.IsNotFoundError(err))
 }
 
 func (suite *StoreTestSuite) TestGet() {
-	err := suite.store.Create("test", TypedContext{
-		Type:        "type",
-		Description: "description",
-	})
+	err := suite.store.Create("test", "type", "description", ContextMetadata{})
 	require.Nil(suite.T(), err)
 
-	meta, err := suite.store.Get("test", nil)
+	meta, err := suite.store.Get("test")
 	require.Nil(suite.T(), err)
 	require.NotNil(suite.T(), meta)
 	require.Equal(suite.T(), "test", meta.Name)
 
 	require.Equal(suite.T(), "description", meta.Metadata.Description)
-	require.Equal(suite.T(), "type", meta.Metadata.Type)
+	require.Equal(suite.T(), "type", meta.Type)
 }
 
 func (suite *StoreTestSuite) TestList() {
-	err := suite.store.Create("test1", TypedContext{})
+	err := suite.store.Create("test1", "type", "description", ContextMetadata{})
 	require.Nil(suite.T(), err)
 
-	err = suite.store.Create("test2", TypedContext{})
+	err = suite.store.Create("test2", "type", "description", ContextMetadata{})
 	require.Nil(suite.T(), err)
 
 	contexts, err := suite.store.List()
@@ -116,7 +130,7 @@ func (suite *StoreTestSuite) TestRemoveNotFound() {
 }
 
 func (suite *StoreTestSuite) TestRemove() {
-	err := suite.store.Create("testremove", TypedContext{})
+	err := suite.store.Create("testremove", "type", "description", ContextMetadata{})
 	require.Nil(suite.T(), err)
 	contexts, err := suite.store.List()
 	require.Nil(suite.T(), err)

context/store/storedefault.go

@@ -10,7 +10,7 @@ import (
 
 // Represents a context as created by the docker cli
 type defaultContext struct {
-	Metadata  TypedContext
+	Metadata  ContextMetadata
 	Endpoints endpoints
 }
 
@@ -54,20 +54,19 @@ func dockerDefaultContext() (*Metadata, error) {
 
 	meta := Metadata{
 		Name: "default",
-		Endpoints: map[string]Endpoint{
-			"docker": {
+		Type: "docker",
+		Endpoints: map[string]interface{}{
+			"docker": Endpoint{
 				Host: defaultCtx.Endpoints.Docker.Host,
 			},
-			"kubernetes": {
+			"kubernetes": Endpoint{
 				Host:             defaultCtx.Endpoints.Kubernetes.Host,
 				DefaultNamespace: defaultCtx.Endpoints.Kubernetes.DefaultNamespace,
 			},
 		},
-		Metadata: TypedContext{
+		Metadata: ContextMetadata{
 			Description:       "Current DOCKER_HOST based configuration",
-			Type:              "docker",
 			StackOrchestrator: defaultCtx.Metadata.StackOrchestrator,
-			Data:              defaultCtx.Metadata,
 		},
 	}
 

errdefs/errors.go

@@ -47,6 +47,9 @@ var (
 	ErrNotImplemented = errors.New("not implemented")
 	// ErrParsingFailed is returned when a string cannot be parsed
 	ErrParsingFailed = errors.New("parsing failed")
+	// ErrWrongContextType is returned when the caller tries to get a context
+	// with the wrong type
+	ErrWrongContextType = errors.New("wrong context type")
 )
 
 // IsNotFoundError returns true if the unwrapped error is ErrNotFound

tests/framework/clisuite.go

@@ -35,9 +35,7 @@ func (sut *CliSuite) BeforeTest(suiteName, testName string) {
 	)
 	require.Nil(sut.T(), err)
 
-	err = s.Create("example", store.TypedContext{
-		Type: "example",
-	})
+	err = s.Create("example", "example", "", store.ContextMetadata{})
 	require.Nil(sut.T(), err)
 
 	sut.storeRoot = dir