Merge pull request #147 from rumpl/feat-better-context

Change the way a context is stored

.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
 		}
-	}
-
-	return &Metadata{
-		Name:      um.Name,
-		Endpoints: um.Endpoints,
-		Metadata: TypedContext{
-			StackOrchestrator: uc.StackOrchestrator,
-			Description:       uc.Description,
-			Type:              uc.Type,
-			Data:              data,
-		},
-	}, nil
-}
+		typeGetters := getters()
+		if _, ok := typeGetters[k]; !ok {
+			result[k] = v
+			continue
+		}
 
-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
-	}
-
-	typed := getter()
-	if err := json.Unmarshal(payload, &typed); err != nil {
-		return nil, err
-	}
-
-	return reflect.ValueOf(typed).Elem().Interface(), nil
-}
 
-func (s *store) GetType(meta *Metadata) string {
-	for k := range meta.Endpoints {
-		if k != dockerEndpointKey {
-			return k
-		}
+		result[k] = val
 	}
 
-	return dockerEndpointKey
+	return result, nil
 }
 
-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