From c36b64c10bd942413b32c2f8ce91c07b9c185157 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 10 Jul 2020 15:27:41 +0200 Subject: [PATCH 1/4] Fixing subscription-id parameter not passed to backend... --- cli/cmd/context/createaci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/cmd/context/createaci.go b/cli/cmd/context/createaci.go index 68c690dc1..d1c171c02 100644 --- a/cli/cmd/context/createaci.go +++ b/cli/cmd/context/createaci.go @@ -75,7 +75,7 @@ func getAciContextData(ctx context.Context, opts aciCreateOpts) (interface{}, st func convertAciOpts(opts aciCreateOpts) map[string]string { return map[string]string{ - "aciSubscriptionId": opts.subscriptionID, + "aciSubscriptionID": opts.subscriptionID, "aciResourceGroup": opts.resourceGroup, "aciLocation": opts.location, "description": opts.description, From 97d408d25d7775cc3ab9c7b07f7455a142530ba1 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 10 Jul 2020 16:39:09 +0200 Subject: [PATCH 2/4] Replaced map[string] string by existing aciCreateOpts struct for context create --- azure/backend.go | 7 +++++-- azure/context.go | 22 ++++++++++++---------- azure/context_test.go | 12 +++++++----- cli/cmd/context/createaci.go | 36 ++++++++++++++---------------------- context/cloud/api.go | 4 ++-- 5 files changed, 40 insertions(+), 41 deletions(-) diff --git a/azure/backend.go b/azure/backend.go index b043e5975..e6c2e78e6 100644 --- a/azure/backend.go +++ b/azure/backend.go @@ -23,6 +23,8 @@ import ( "strconv" "strings" + acicontext "github.com/docker/api/cli/cmd/context" + "github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance" "github.com/Azure/go-autorest/autorest/to" "github.com/compose-spec/compose-go/cli" @@ -359,7 +361,8 @@ func (cs *aciCloudService) Logout(ctx context.Context) error { return cs.loginService.Logout(ctx) } -func (cs *aciCloudService) CreateContextData(ctx context.Context, params map[string]string) (interface{}, string, error) { +func (cs *aciCloudService) CreateContextData(ctx context.Context, params interface{}) (interface{}, string, error) { contextHelper := newContextCreateHelper() - return contextHelper.createContextData(ctx, params) + createOpts := params.(acicontext.AciCreateOpts) + return contextHelper.createContextData(ctx, createOpts) } diff --git a/azure/context.go b/azure/context.go index 4c0b0e31f..20205d42b 100644 --- a/azure/context.go +++ b/azure/context.go @@ -21,6 +21,8 @@ import ( "fmt" "os" + acicontext "github.com/docker/api/cli/cmd/context" + "github.com/AlecAivazis/survey/v2" "github.com/Azure/azure-sdk-for-go/profiles/preview/preview/subscription/mgmt/subscription" "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources" @@ -43,10 +45,10 @@ func newContextCreateHelper() contextCreateACIHelper { } } -func (helper contextCreateACIHelper) createContextData(ctx context.Context, opts map[string]string) (interface{}, string, error) { +func (helper contextCreateACIHelper) createContextData(ctx context.Context, opts acicontext.AciCreateOpts) (interface{}, string, error) { var subscriptionID string - if opts["aciSubscriptionID"] != "" { - subscriptionID = opts["aciSubscriptionID"] + if opts.SubscriptionID != "" { + subscriptionID = opts.SubscriptionID } else { subs, err := helper.resourceGroupHelper.GetSubscriptionIDs(ctx) if err != nil { @@ -61,10 +63,10 @@ func (helper contextCreateACIHelper) createContextData(ctx context.Context, opts var group resources.Group var err error - if opts["aciResourceGroup"] != "" { - group, err = helper.resourceGroupHelper.GetGroup(ctx, subscriptionID, opts["aciResourceGroup"]) + if opts.ResourceGroup != "" { + group, err = helper.resourceGroupHelper.GetGroup(ctx, subscriptionID, opts.ResourceGroup) if err != nil { - return nil, "", errors.Wrapf(err, "Could not find resource group %q", opts["aciResourceGroup"]) + return nil, "", errors.Wrapf(err, "Could not find resource group %q", opts.ResourceGroup) } } else { groups, err := helper.resourceGroupHelper.ListGroups(ctx, subscriptionID) @@ -80,8 +82,8 @@ func (helper contextCreateACIHelper) createContextData(ctx context.Context, opts location := *group.Location description := fmt.Sprintf("%s@%s", *group.Name, location) - if opts["description"] != "" { - description = fmt.Sprintf("%s (%s)", opts["description"], description) + if opts.Description != "" { + description = fmt.Sprintf("%s (%s)", opts.Description, description) } return store.AciContext{ @@ -108,7 +110,7 @@ func (helper contextCreateACIHelper) createGroup(ctx context.Context, subscripti return g, nil } -func (helper contextCreateACIHelper) chooseGroup(ctx context.Context, subscriptionID string, opts map[string]string, groups []resources.Group) (resources.Group, error) { +func (helper contextCreateACIHelper) chooseGroup(ctx context.Context, subscriptionID string, opts acicontext.AciCreateOpts, groups []resources.Group) (resources.Group, error) { groupNames := []string{"create a new resource group"} for _, g := range groups { groupNames = append(groupNames, fmt.Sprintf("%s (%s)", *g.Name, *g.Location)) @@ -124,7 +126,7 @@ func (helper contextCreateACIHelper) chooseGroup(ctx context.Context, subscripti } if group == 0 { - return helper.createGroup(ctx, subscriptionID, opts["aciLocation"]) + return helper.createGroup(ctx, subscriptionID, opts.Location) } return groups[group-1], nil diff --git a/azure/context_test.go b/azure/context_test.go index 87fd3166a..17de73aff 100644 --- a/azure/context_test.go +++ b/azure/context_test.go @@ -20,6 +20,8 @@ import ( "context" "testing" + acicontext "github.com/docker/api/cli/cmd/context" + "github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/resources/mgmt/resources" "github.com/Azure/azure-sdk-for-go/profiles/preview/preview/subscription/mgmt/subscription" "github.com/Azure/go-autorest/autorest/to" @@ -175,11 +177,11 @@ func aciContext(subscriptionID string, resourceGroupName string, location string } } -func options(subscriptionID string, resourceGroupName string) map[string]string { - return map[string]string{ - "aciSubscriptionID": subscriptionID, - "aciResourceGroup": resourceGroupName, - "aciLocation": "eastus", +func options(subscriptionID string, resourceGroupName string) acicontext.AciCreateOpts { + return acicontext.AciCreateOpts{ + SubscriptionID: subscriptionID, + ResourceGroup: resourceGroupName, + Location: "eastus", } } diff --git a/cli/cmd/context/createaci.go b/cli/cmd/context/createaci.go index d1c171c02..cdff27945 100644 --- a/cli/cmd/context/createaci.go +++ b/cli/cmd/context/createaci.go @@ -27,15 +27,16 @@ import ( "github.com/docker/api/errdefs" ) -type aciCreateOpts struct { - description string - location string - subscriptionID string - resourceGroup string +// AciCreateOpts options for creating ACI context +type AciCreateOpts struct { + Description string + Location string + SubscriptionID string + ResourceGroup string } func createAciCommand() *cobra.Command { - var opts aciCreateOpts + var opts AciCreateOpts cmd := &cobra.Command{ Use: "aci CONTEXT [flags]", Short: "Create a context for Azure Container Instances", @@ -45,15 +46,15 @@ func createAciCommand() *cobra.Command { }, } - addDescriptionFlag(cmd, &opts.description) - cmd.Flags().StringVar(&opts.location, "location", "eastus", "Location") - cmd.Flags().StringVar(&opts.subscriptionID, "subscription-id", "", "Location") - cmd.Flags().StringVar(&opts.resourceGroup, "resource-group", "", "Resource group") + addDescriptionFlag(cmd, &opts.Description) + cmd.Flags().StringVar(&opts.Location, "location", "eastus", "Location") + cmd.Flags().StringVar(&opts.SubscriptionID, "subscription-id", "", "Location") + cmd.Flags().StringVar(&opts.ResourceGroup, "resource-group", "", "Resource group") return cmd } -func runCreateAci(ctx context.Context, contextName string, opts aciCreateOpts) error { +func runCreateAci(ctx context.Context, contextName string, opts AciCreateOpts) error { if contextExists(ctx, contextName) { return errors.Wrapf(errdefs.ErrAlreadyExists, "context %s", contextName) } @@ -65,19 +66,10 @@ func runCreateAci(ctx context.Context, contextName string, opts aciCreateOpts) e } -func getAciContextData(ctx context.Context, opts aciCreateOpts) (interface{}, string, error) { +func getAciContextData(ctx context.Context, opts AciCreateOpts) (interface{}, string, error) { cs, err := client.GetCloudService(ctx, store.AciContextType) if err != nil { return nil, "", errors.Wrap(err, "cannot connect to ACI backend") } - return cs.CreateContextData(ctx, convertAciOpts(opts)) -} - -func convertAciOpts(opts aciCreateOpts) map[string]string { - return map[string]string{ - "aciSubscriptionID": opts.subscriptionID, - "aciResourceGroup": opts.resourceGroup, - "aciLocation": opts.location, - "description": opts.description, - } + return cs.CreateContextData(ctx, opts) } diff --git a/context/cloud/api.go b/context/cloud/api.go index d6bb755e7..87d4c40f8 100644 --- a/context/cloud/api.go +++ b/context/cloud/api.go @@ -29,7 +29,7 @@ type Service interface { // Logout logout from cloud provider Logout(ctx context.Context) error // CreateContextData create data for cloud context - CreateContextData(ctx context.Context, params map[string]string) (contextData interface{}, description string, err error) + CreateContextData(ctx context.Context, params interface{}) (contextData interface{}, description string, err error) } // NotImplementedCloudService to use for backend that don't provide cloud services @@ -49,6 +49,6 @@ func (cs notImplementedCloudService) Login(ctx context.Context, params map[strin return errdefs.ErrNotImplemented } -func (cs notImplementedCloudService) CreateContextData(ctx context.Context, params map[string]string) (interface{}, string, error) { +func (cs notImplementedCloudService) CreateContextData(ctx context.Context, params interface{}) (interface{}, string, error) { return nil, "", errdefs.ErrNotImplemented } From 43b54ef75a1c4f73cdfeef310306ffd057b5e7dd Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 10 Jul 2020 16:55:44 +0200 Subject: [PATCH 3/4] Also remove map[string]string for azure login (tenantId param) --- azure/backend.go | 7 +++++-- azure/login/login.go | 3 --- cli/cmd/login/azurelogin.go | 13 ++++++------- cli/cmd/login/login.go | 2 +- context/cloud/api.go | 4 ++-- 5 files changed, 14 insertions(+), 15 deletions(-) diff --git a/azure/backend.go b/azure/backend.go index e6c2e78e6..d8b7fb482 100644 --- a/azure/backend.go +++ b/azure/backend.go @@ -23,6 +23,8 @@ import ( "strconv" "strings" + clilogin "github.com/docker/api/cli/cmd/login" + acicontext "github.com/docker/api/cli/cmd/context" "github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance" @@ -353,8 +355,9 @@ type aciCloudService struct { loginService login.AzureLoginService } -func (cs *aciCloudService) Login(ctx context.Context, params map[string]string) error { - return cs.loginService.Login(ctx, params[login.TenantIDLoginParam]) +func (cs *aciCloudService) Login(ctx context.Context, params interface{}) error { + createOpts := params.(clilogin.AzureLoginOpts) + return cs.loginService.Login(ctx, createOpts.TenantID) } func (cs *aciCloudService) Logout(ctx context.Context) error { diff --git a/azure/login/login.go b/azure/login/login.go index 7117ccb76..59dd3cd87 100644 --- a/azure/login/login.go +++ b/azure/login/login.go @@ -45,9 +45,6 @@ const ( // v1 scope like "https://management.azure.com/.default" for ARM access scopes = "offline_access https://management.azure.com/.default" clientID = "04b07795-8ddb-461a-bbee-02f9e1bf7b46" // Azure CLI client id - - // TenantIDLoginParam - TenantIDLoginParam = "tenantId" ) type ( diff --git a/cli/cmd/login/azurelogin.go b/cli/cmd/login/azurelogin.go index 8c7e7824f..00d795003 100644 --- a/cli/cmd/login/azurelogin.go +++ b/cli/cmd/login/azurelogin.go @@ -2,27 +2,26 @@ package login import ( "github.com/spf13/cobra" - - "github.com/docker/api/azure/login" ) -type azureLoginOpts struct { - tenantID string +// AzureLoginOpts azure login options +type AzureLoginOpts struct { + TenantID string } // AzureLoginCommand returns the azure login command func AzureLoginCommand() *cobra.Command { - opts := azureLoginOpts{} + opts := AzureLoginOpts{} cmd := &cobra.Command{ Use: "azure", Short: "Log in to azure", Args: cobra.MaximumNArgs(0), RunE: func(cmd *cobra.Command, args []string) error { - return cloudLogin(cmd, "aci", map[string]string{login.TenantIDLoginParam: opts.tenantID}) + return cloudLogin(cmd, "aci", opts) }, } flags := cmd.Flags() - flags.StringVar(&opts.tenantID, "tenant-id", "", "Specify tenant ID to use from your azure account") + flags.StringVar(&opts.TenantID, "tenant-id", "", "Specify tenant ID to use from your azure account") return cmd } diff --git a/cli/cmd/login/login.go b/cli/cmd/login/login.go index 79dc912f4..0fee72222 100644 --- a/cli/cmd/login/login.go +++ b/cli/cmd/login/login.go @@ -59,7 +59,7 @@ func runLogin(cmd *cobra.Command, args []string) error { return mobycli.ExecCmd(cmd) } -func cloudLogin(cmd *cobra.Command, backendType string, params map[string]string) error { +func cloudLogin(cmd *cobra.Command, backendType string, params interface{}) error { ctx := cmd.Context() cs, err := client.GetCloudService(ctx, backendType) if err != nil { diff --git a/context/cloud/api.go b/context/cloud/api.go index 87d4c40f8..84c2c242f 100644 --- a/context/cloud/api.go +++ b/context/cloud/api.go @@ -25,7 +25,7 @@ import ( // Service cloud specific services type Service interface { // Login login to cloud provider - Login(ctx context.Context, params map[string]string) error + Login(ctx context.Context, params interface{}) error // Logout logout from cloud provider Logout(ctx context.Context) error // CreateContextData create data for cloud context @@ -45,7 +45,7 @@ func (cs notImplementedCloudService) Logout(ctx context.Context) error { return errdefs.ErrNotImplemented } -func (cs notImplementedCloudService) Login(ctx context.Context, params map[string]string) error { +func (cs notImplementedCloudService) Login(ctx context.Context, params interface{}) error { return errdefs.ErrNotImplemented } From 84dbd1467d22463d2ebeda1a734b06949119df57 Mon Sep 17 00:00:00 2001 From: Djordje Lukic Date: Mon, 13 Jul 2020 10:38:29 +0200 Subject: [PATCH 4/4] Move the struct for creating and aci context to azure package --- azure/backend.go | 21 +++++++++++++++------ azure/context.go | 6 ++---- azure/context_test.go | 6 ++---- cli/cmd/context/createaci.go | 15 ++++----------- cli/cmd/login/azurelogin.go | 9 +++------ 5 files changed, 26 insertions(+), 31 deletions(-) diff --git a/azure/backend.go b/azure/backend.go index d8b7fb482..4def23cb6 100644 --- a/azure/backend.go +++ b/azure/backend.go @@ -23,10 +23,6 @@ import ( "strconv" "strings" - clilogin "github.com/docker/api/cli/cmd/login" - - acicontext "github.com/docker/api/cli/cmd/context" - "github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance" "github.com/Azure/go-autorest/autorest/to" "github.com/compose-spec/compose-go/cli" @@ -54,6 +50,19 @@ const ( // ErrNoSuchContainer is returned when the mentioned container does not exist var ErrNoSuchContainer = errors.New("no such container") +// ContextParams options for creating ACI context +type ContextParams struct { + Description string + Location string + SubscriptionID string + ResourceGroup string +} + +// LoginParams azure login options +type LoginParams struct { + TenantID string +} + func init() { backend.Register("aci", "aci", service, getCloudService) } @@ -356,7 +365,7 @@ type aciCloudService struct { } func (cs *aciCloudService) Login(ctx context.Context, params interface{}) error { - createOpts := params.(clilogin.AzureLoginOpts) + createOpts := params.(LoginParams) return cs.loginService.Login(ctx, createOpts.TenantID) } @@ -366,6 +375,6 @@ func (cs *aciCloudService) Logout(ctx context.Context) error { func (cs *aciCloudService) CreateContextData(ctx context.Context, params interface{}) (interface{}, string, error) { contextHelper := newContextCreateHelper() - createOpts := params.(acicontext.AciCreateOpts) + createOpts := params.(ContextParams) return contextHelper.createContextData(ctx, createOpts) } diff --git a/azure/context.go b/azure/context.go index 20205d42b..41af99706 100644 --- a/azure/context.go +++ b/azure/context.go @@ -21,8 +21,6 @@ import ( "fmt" "os" - acicontext "github.com/docker/api/cli/cmd/context" - "github.com/AlecAivazis/survey/v2" "github.com/Azure/azure-sdk-for-go/profiles/preview/preview/subscription/mgmt/subscription" "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources" @@ -45,7 +43,7 @@ func newContextCreateHelper() contextCreateACIHelper { } } -func (helper contextCreateACIHelper) createContextData(ctx context.Context, opts acicontext.AciCreateOpts) (interface{}, string, error) { +func (helper contextCreateACIHelper) createContextData(ctx context.Context, opts ContextParams) (interface{}, string, error) { var subscriptionID string if opts.SubscriptionID != "" { subscriptionID = opts.SubscriptionID @@ -110,7 +108,7 @@ func (helper contextCreateACIHelper) createGroup(ctx context.Context, subscripti return g, nil } -func (helper contextCreateACIHelper) chooseGroup(ctx context.Context, subscriptionID string, opts acicontext.AciCreateOpts, groups []resources.Group) (resources.Group, error) { +func (helper contextCreateACIHelper) chooseGroup(ctx context.Context, subscriptionID string, opts ContextParams, groups []resources.Group) (resources.Group, error) { groupNames := []string{"create a new resource group"} for _, g := range groups { groupNames = append(groupNames, fmt.Sprintf("%s (%s)", *g.Name, *g.Location)) diff --git a/azure/context_test.go b/azure/context_test.go index 17de73aff..f61a90745 100644 --- a/azure/context_test.go +++ b/azure/context_test.go @@ -20,8 +20,6 @@ import ( "context" "testing" - acicontext "github.com/docker/api/cli/cmd/context" - "github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/resources/mgmt/resources" "github.com/Azure/azure-sdk-for-go/profiles/preview/preview/subscription/mgmt/subscription" "github.com/Azure/go-autorest/autorest/to" @@ -177,8 +175,8 @@ func aciContext(subscriptionID string, resourceGroupName string, location string } } -func options(subscriptionID string, resourceGroupName string) acicontext.AciCreateOpts { - return acicontext.AciCreateOpts{ +func options(subscriptionID string, resourceGroupName string) ContextParams { + return ContextParams{ SubscriptionID: subscriptionID, ResourceGroup: resourceGroupName, Location: "eastus", diff --git a/cli/cmd/context/createaci.go b/cli/cmd/context/createaci.go index cdff27945..61e68cd80 100644 --- a/cli/cmd/context/createaci.go +++ b/cli/cmd/context/createaci.go @@ -22,21 +22,14 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "github.com/docker/api/azure" "github.com/docker/api/client" "github.com/docker/api/context/store" "github.com/docker/api/errdefs" ) -// AciCreateOpts options for creating ACI context -type AciCreateOpts struct { - Description string - Location string - SubscriptionID string - ResourceGroup string -} - func createAciCommand() *cobra.Command { - var opts AciCreateOpts + var opts azure.ContextParams cmd := &cobra.Command{ Use: "aci CONTEXT [flags]", Short: "Create a context for Azure Container Instances", @@ -54,7 +47,7 @@ func createAciCommand() *cobra.Command { return cmd } -func runCreateAci(ctx context.Context, contextName string, opts AciCreateOpts) error { +func runCreateAci(ctx context.Context, contextName string, opts azure.ContextParams) error { if contextExists(ctx, contextName) { return errors.Wrapf(errdefs.ErrAlreadyExists, "context %s", contextName) } @@ -66,7 +59,7 @@ func runCreateAci(ctx context.Context, contextName string, opts AciCreateOpts) e } -func getAciContextData(ctx context.Context, opts AciCreateOpts) (interface{}, string, error) { +func getAciContextData(ctx context.Context, opts azure.ContextParams) (interface{}, string, error) { cs, err := client.GetCloudService(ctx, store.AciContextType) if err != nil { return nil, "", errors.Wrap(err, "cannot connect to ACI backend") diff --git a/cli/cmd/login/azurelogin.go b/cli/cmd/login/azurelogin.go index 00d795003..363b6cbf4 100644 --- a/cli/cmd/login/azurelogin.go +++ b/cli/cmd/login/azurelogin.go @@ -2,16 +2,13 @@ package login import ( "github.com/spf13/cobra" -) -// AzureLoginOpts azure login options -type AzureLoginOpts struct { - TenantID string -} + "github.com/docker/api/azure" +) // AzureLoginCommand returns the azure login command func AzureLoginCommand() *cobra.Command { - opts := AzureLoginOpts{} + opts := azure.LoginParams{} cmd := &cobra.Command{ Use: "azure", Short: "Log in to azure",