Merge pull request #1408 from dhiltgen/fix_panic

andrewhsu · web-flow · commit 984bc7411e55 · 2018-10-02T15:41:22.000-07:00
[18.09] Fix panic in display only case for license
diff --git a/cli/command/engine/activate.go b/cli/command/engine/activate.go
@@ -16,19 +16,21 @@ import (
 )
 
 type activateOptions struct {
-	licenseFile    string
-	version        string
-	registryPrefix string
-	format         string
-	image          string
-	quiet          bool
-	displayOnly    bool
-	sockPath       string
+	licenseFile      string
+	version          string
+	registryPrefix   string
+	format           string
+	image            string
+	quiet            bool
+	displayOnly      bool
+	sockPath         string
+	licenseLoginFunc func(ctx context.Context, authConfig *types.AuthConfig) (licenseutils.HubUser, error)
 }
 
 // newActivateCommand creates a new `docker engine activate` command
 func newActivateCommand(dockerCli command.Cli) *cobra.Command {
 	var options activateOptions
+	options.licenseLoginFunc = licenseutils.Login
 
 	cmd := &cobra.Command{
 		Use:   "activate [OPTIONS]",
@@ -60,7 +62,7 @@ https://hub.docker.com/ then specify the file with the '--license' flag.
 	flags.StringVar(&options.registryPrefix, "registry-prefix", clitypes.RegistryPrefix, "Override the default location where engine images are pulled")
 	flags.StringVar(&options.image, "engine-image", "", "Specify engine image")
 	flags.StringVar(&options.format, "format", "", "Pretty-print licenses using a Go template")
-	flags.BoolVar(&options.displayOnly, "display-only", false, "only display the available licenses and exit")
+	flags.BoolVar(&options.displayOnly, "display-only", false, "only display license information and exit")
 	flags.BoolVar(&options.quiet, "quiet", false, "Only display available licenses by ID")
 	flags.StringVar(&options.sockPath, "containerd", "", "override default location of containerd endpoint")
 
@@ -90,6 +92,9 @@ func runActivate(cli command.Cli, options activateOptions) error {
 		if license, err = getLicenses(ctx, authConfig, cli, options); err != nil {
 			return err
 		}
+		if options.displayOnly {
+			return nil
+		}
 	} else {
 		if license, err = licenseutils.LoadLocalIssuedLicense(ctx, options.licenseFile); err != nil {
 			return err
@@ -136,7 +141,7 @@ Restart docker with 'systemctl restart docker' to complete the activation.`)
 }
 
 func getLicenses(ctx context.Context, authConfig *types.AuthConfig, cli command.Cli, options activateOptions) (*model.IssuedLicense, error) {
-	user, err := licenseutils.Login(ctx, authConfig)
+	user, err := options.licenseLoginFunc(ctx, authConfig)
 	if err != nil {
 		return nil, err
 	}
diff --git a/cli/command/engine/activate_test.go b/cli/command/engine/activate_test.go
@@ -1,13 +1,18 @@
 package engine
 
 import (
+	"context"
 	"fmt"
 	"testing"
+	"time"
 
+	"github.com/docker/cli/internal/licenseutils"
 	"github.com/docker/cli/internal/test"
 	clitypes "github.com/docker/cli/types"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/client"
+	"github.com/docker/licensing"
+	"github.com/docker/licensing/model"
 	"gotest.tools/assert"
 	"gotest.tools/fs"
 	"gotest.tools/golden"
@@ -69,3 +74,73 @@ func TestActivateExpiredLicenseDryRun(t *testing.T) {
 	assert.NilError(t, err)
 	golden.Assert(t, c.OutBuffer().String(), "expired-license-display-only.golden")
 }
+
+type mockLicenseClient struct{}
+
+func (c mockLicenseClient) LoginViaAuth(ctx context.Context, username, password string) (authToken string, err error) {
+	return "", fmt.Errorf("not implemented")
+}
+
+func (c mockLicenseClient) GetHubUserOrgs(ctx context.Context, authToken string) (orgs []model.Org, err error) {
+	return nil, fmt.Errorf("not implemented")
+}
+func (c mockLicenseClient) GetHubUserByName(ctx context.Context, username string) (user *model.User, err error) {
+	return nil, fmt.Errorf("not implemented")
+}
+func (c mockLicenseClient) VerifyLicense(ctx context.Context, license model.IssuedLicense) (res *model.CheckResponse, err error) {
+	return nil, fmt.Errorf("not implemented")
+}
+func (c mockLicenseClient) GenerateNewTrialSubscription(ctx context.Context, authToken, dockerID, email string) (subscriptionID string, err error) {
+	return "", fmt.Errorf("not implemented")
+}
+func (c mockLicenseClient) ListSubscriptions(ctx context.Context, authToken, dockerID string) (response []*model.Subscription, err error) {
+	expires := time.Date(2010, time.January, 1, 0, 0, 0, 0, time.UTC)
+	return []*model.Subscription{
+		{
+			State:   "active",
+			Expires: &expires,
+		},
+	}, nil
+}
+func (c mockLicenseClient) ListSubscriptionsDetails(ctx context.Context, authToken, dockerID string) (response []*model.SubscriptionDetail, err error) {
+	return nil, fmt.Errorf("not implemented")
+}
+func (c mockLicenseClient) DownloadLicenseFromHub(ctx context.Context, authToken, subscriptionID string) (license *model.IssuedLicense, err error) {
+	return nil, fmt.Errorf("not implemented")
+}
+func (c mockLicenseClient) ParseLicense(license []byte) (parsedLicense *model.IssuedLicense, err error) {
+	return nil, fmt.Errorf("not implemented")
+}
+func (c mockLicenseClient) StoreLicense(ctx context.Context, dclnt licensing.WrappedDockerClient, licenses *model.IssuedLicense, localRootDir string) error {
+	return fmt.Errorf("not implemented")
+}
+func (c mockLicenseClient) LoadLocalLicense(ctx context.Context, dclnt licensing.WrappedDockerClient) (*model.Subscription, error) {
+	return nil, fmt.Errorf("not implemented")
+}
+func (c mockLicenseClient) SummarizeLicense(res *model.CheckResponse, keyID string) *model.Subscription {
+	return nil
+}
+func TestActivateDisplayOnlyHub(t *testing.T) {
+	isRoot = func() bool { return true }
+	c := test.NewFakeCli(&verClient{client.Client{}, types.Version{}, nil, types.Info{}, nil})
+	c.SetContainerizedEngineClient(
+		func(string) (clitypes.ContainerizedClient, error) {
+			return &fakeContainerizedEngineClient{}, nil
+		},
+	)
+
+	hubUser := licenseutils.HubUser{
+		Client: mockLicenseClient{},
+	}
+	options := activateOptions{
+		licenseLoginFunc: func(ctx context.Context, authConfig *types.AuthConfig) (licenseutils.HubUser, error) {
+			return hubUser, nil
+		},
+		displayOnly: true,
+	}
+	c.OutBuffer().Reset()
+	err := runActivate(c, options)
+
+	assert.NilError(t, err)
+	golden.Assert(t, c.OutBuffer().String(), "expired-hub-license-display-only.golden")
+}
diff --git a/cli/command/engine/testdata/expired-hub-license-display-only.golden b/cli/command/engine/testdata/expired-hub-license-display-only.golden
@@ -0,0 +1,3 @@
+Looking for existing licenses for ...
+NUM                 OWNER               PRODUCT ID          EXPIRES                         PRICING COMPONENTS
+0                                                           2010-01-01 00:00:00 +0000 UTC   
diff --git a/internal/licenseutils/utils.go b/internal/licenseutils/utils.go
@@ -20,7 +20,7 @@ import (
 // HubUser wraps a licensing client and holds key information
 // for a user to avoid multiple lookups
 type HubUser struct {
-	client licensing.Client
+	Client licensing.Client
 	token  string
 	User   model.User
 	Orgs   []model.Org
@@ -73,7 +73,7 @@ func Login(ctx context.Context, authConfig *types.AuthConfig) (HubUser, error) {
 		return HubUser{}, err
 	}
 	return HubUser{
-		client: lclient,
+		Client: lclient,
 		token:  token,
 		User:   *user,
 		Orgs:   orgs,
@@ -83,12 +83,12 @@ func Login(ctx context.Context, authConfig *types.AuthConfig) (HubUser, error) {
 
 // GetAvailableLicenses finds all available licenses for a given account and their orgs
 func (u HubUser) GetAvailableLicenses(ctx context.Context) ([]LicenseDisplay, error) {
-	subs, err := u.client.ListSubscriptions(ctx, u.token, u.User.ID)
+	subs, err := u.Client.ListSubscriptions(ctx, u.token, u.User.ID)
 	if err != nil {
 		return nil, err
 	}
 	for _, org := range u.Orgs {
-		orgSub, err := u.client.ListSubscriptions(ctx, u.token, org.ID)
+		orgSub, err := u.Client.ListSubscriptions(ctx, u.token, org.ID)
 		if err != nil {
 			return nil, err
 		}
@@ -134,16 +134,16 @@ func (u HubUser) GetAvailableLicenses(ctx context.Context) ([]LicenseDisplay, er
 
 // GenerateTrialLicense will generate a new trial license for the specified user or org
 func (u HubUser) GenerateTrialLicense(ctx context.Context, targetID string) (*model.IssuedLicense, error) {
-	subID, err := u.client.GenerateNewTrialSubscription(ctx, u.token, targetID, u.User.Email)
+	subID, err := u.Client.GenerateNewTrialSubscription(ctx, u.token, targetID, u.User.Email)
 	if err != nil {
 		return nil, err
 	}
-	return u.client.DownloadLicenseFromHub(ctx, u.token, subID)
+	return u.Client.DownloadLicenseFromHub(ctx, u.token, subID)
 }
 
 // GetIssuedLicense will download a license by ID
 func (u HubUser) GetIssuedLicense(ctx context.Context, ID string) (*model.IssuedLicense, error) {
-	return u.client.DownloadLicenseFromHub(ctx, u.token, ID)
+	return u.Client.DownloadLicenseFromHub(ctx, u.token, ID)
 }
 
 // LoadLocalIssuedLicense will load a local license file
diff --git a/internal/licenseutils/utils_test.go b/internal/licenseutils/utils_test.go
@@ -43,7 +43,7 @@ func TestGetOrgByID(t *testing.T) {
 func TestGetAvailableLicensesListFail(t *testing.T) {
 	ctx := context.Background()
 	user := HubUser{
-		client: &fakeLicensingClient{
+		Client: &fakeLicensingClient{
 			listSubscriptionsFunc: func(ctx context.Context, authToken, dockerID string) (response []*model.Subscription, err error) {
 				return nil, fmt.Errorf("list subscriptions error")
 			},
@@ -59,7 +59,7 @@ func TestGetAvailableLicensesOrgFail(t *testing.T) {
 		Orgs: []model.Org{
 			{ID: "orgid"},
 		},
-		client: &fakeLicensingClient{
+		Client: &fakeLicensingClient{
 			listSubscriptionsFunc: func(ctx context.Context, authToken, dockerID string) (response []*model.Subscription, err error) {
 				if dockerID == "orgid" {
 					return nil, fmt.Errorf("list subscriptions org error")
@@ -86,7 +86,7 @@ func TestGetAvailableLicensesHappy(t *testing.T) {
 				Orgname: "orgname",
 			},
 		},
-		client: &fakeLicensingClient{
+		Client: &fakeLicensingClient{
 			listSubscriptionsFunc: func(ctx context.Context, authToken, dockerID string) (response []*model.Subscription, err error) {
 				if dockerID == "orgid" {
 					return []*model.Subscription{
@@ -146,7 +146,7 @@ func TestGetAvailableLicensesHappy(t *testing.T) {
 func TestGenerateTrialFail(t *testing.T) {
 	ctx := context.Background()
 	user := HubUser{
-		client: &fakeLicensingClient{
+		Client: &fakeLicensingClient{
 			generateNewTrialSubscriptionFunc: func(ctx context.Context, authToken, dockerID, email string) (subscriptionID string, err error) {
 				return "", fmt.Errorf("generate trial failure")
 			},
@@ -160,7 +160,7 @@ func TestGenerateTrialFail(t *testing.T) {
 func TestGenerateTrialHappy(t *testing.T) {
 	ctx := context.Background()
 	user := HubUser{
-		client: &fakeLicensingClient{
+		Client: &fakeLicensingClient{
 			generateNewTrialSubscriptionFunc: func(ctx context.Context, authToken, dockerID, email string) (subscriptionID string, err error) {
 				return "subid", nil
 			},
@@ -174,7 +174,7 @@ func TestGenerateTrialHappy(t *testing.T) {
 func TestGetIssuedLicense(t *testing.T) {
 	ctx := context.Background()
 	user := HubUser{
-		client: &fakeLicensingClient{},
+		Client: &fakeLicensingClient{},
 	}
 	id := "idgoeshere"
 	_, err := user.GetIssuedLicense(ctx, id)

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+Looking for existing licenses for ...`
	`2`	`+NUM OWNER PRODUCT ID EXPIRES PRICING COMPONENTS`
	`3`	`+0 2010-01-01 00:00:00 +0000 UTC`