registry: add default expiry to login command (#1388)

* registry: add default expiry to login command

Adds a default expiry to the login command. By default, the API tokens
created by logging into the registry will now expire after 30 days. To
create a token that doesn't expire, you must pass the `--never-expire
true` flag.

* registry: fix integration test case

* docr: show message when default expiry applied

* docr: update logged string in integration test

args.go

@@ -392,6 +392,8 @@ const (
 	ArgRegistryExpirySeconds = "expiry-seconds"
 	// ArgRegistryReadOnly indicates that a generated registry API token should be read-only.
 	ArgRegistryReadOnly = "read-only"
+	// ArgRegistryNeverExpire indicates that a generated registry API token should never expire.
+	ArgRegistryNeverExpire = "never-expire"
 	// ArgSubscriptionTier is a subscription tier slug.
 	ArgSubscriptionTier = "subscription-tier"
 	// ArgGCIncludeUntaggedManifests indicates that a garbage collection should delete

commands/registry.go

@@ -16,6 +16,7 @@ package commands
 import (
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -45,8 +46,14 @@ const (
 	DOSecretOperatorAnnotation = "digitalocean.com/dosecret-identifier"
 
 	oauthTokenRevokeEndpoint = "https://cloud.digitalocean.com/v1/oauth/revoke"
+
+	// defaultRegistryAPITokenExpirySeconds is the default number of seconds before a registry API
+	// token expires. 2592000 is 30 days in seconds.
+	defaultRegistryAPITokenExpirySeconds = 2592000
 )
 
+var errExpiryTimeAndNeverExpire = errors.New("the generated registry API token cannot have both the expiry time and never-expire set")
+
 // Registry creates the registry command
 func Registry() *Command {
 	cmd := &Command{
@@ -84,9 +91,11 @@ func Registry() *Command {
 	cmdRegistryLogin := CmdBuilder(cmd, RunRegistryLogin, "login", "Log in Docker to a container registry",
 		loginRegDesc, Writer)
 	AddIntFlag(cmdRegistryLogin, doctl.ArgRegistryExpirySeconds, "", 0,
-		"The length of time the registry credentials will be valid for in seconds. By default, the credentials do not expire.")
+		"The length of time the registry credentials will be valid for in seconds. By default, the credentials expire after 30 days.")
 	AddBoolFlag(cmdRegistryLogin, doctl.ArgRegistryReadOnly, "", false,
 		"If true, the DigitalOcean API token generated by the login command will be read-only, causing any push operations to fail. By default, the API token is read-write.")
+	AddBoolFlag(cmdRegistryLogin, doctl.ArgRegistryNeverExpire, "", false,
+		"If true, the DigitalOcean API token generated by the login command will never expire. By default, this is false.")
 
 	logoutRegDesc := "This command logs Docker out of the private container registry, revoking access to it."
 	cmdRunRegistryLogout := CmdBuilder(cmd, RunRegistryLogout, "logout", "Log out Docker from a container registry",
@@ -382,23 +391,37 @@ func RunRegistryLogin(c *CmdConfig) error {
 	if err != nil {
 		return err
 	}
+	neverExpire, err := c.Doit.GetBool(c.NS, doctl.ArgRegistryNeverExpire)
+	if err != nil {
+		return err
+	}
+	if neverExpire && expirySeconds > 0 {
+		return errExpiryTimeAndNeverExpire
+	}
 	readOnly, err := c.Doit.GetBool(c.NS, doctl.ArgRegistryReadOnly)
 	if err != nil {
 		return err
 	}
 
 	regCredReq := godo.RegistryDockerCredentialsRequest{
-		ReadWrite: !readOnly,
+		ReadWrite:     !readOnly,
+		ExpirySeconds: godo.Int(defaultRegistryAPITokenExpirySeconds),
 	}
 	if expirySeconds != 0 {
 		regCredReq.ExpirySeconds = godo.Int(expirySeconds)
 	}
+	if neverExpire {
+		regCredReq.ExpirySeconds = nil
+	}
 
 	fmt.Printf("Logging Docker in to %s\n", c.Registry().Endpoint())
 	creds, err := c.Registry().DockerCredentials(&regCredReq)
 	if err != nil {
 		return err
 	}
+	if expirySeconds == 0 && !neverExpire {
+		notice("Login valid for 30 days. Use the --expiry-seconds flag to set a shorter expiration or --never-expire for no expiration.")
+	}
 
 	var dc dockerConfig
 	err = json.Unmarshal(creds.DockerConfigJSON, &dc)

commands/registry_test.go

@@ -640,11 +640,14 @@ func TestRegistryLogin(t *testing.T) {
 		name          string
 		expirySeconds int
 		readOnly      bool
+		neverExpire   bool
+		err           error
 		expect        func(m *mocks.MockRegistryService)
 	}{
 		{
 			name:          "no-expiry",
 			expirySeconds: 0,
+			neverExpire:   true,
 			expect: func(m *mocks.MockRegistryService) {
 				m.EXPECT().Endpoint().Return(do.RegistryHostname)
 				m.EXPECT().DockerCredentials(&godo.RegistryDockerCredentialsRequest{
@@ -670,10 +673,18 @@ func TestRegistryLogin(t *testing.T) {
 			expect: func(m *mocks.MockRegistryService) {
 				m.EXPECT().Endpoint().Return(do.RegistryHostname)
 				m.EXPECT().DockerCredentials(&godo.RegistryDockerCredentialsRequest{
-					ReadWrite: false,
+					ReadWrite:     false,
+					ExpirySeconds: godo.Int(defaultRegistryAPITokenExpirySeconds),
 				}).Return(testDockerCredentials, nil)
 			},
 		},
+		{
+			name:          "with-expiry-and-never-expire",
+			expirySeconds: 3600,
+			neverExpire:   true,
+			err:           errExpiryTimeAndNeverExpire,
+			expect:        func(m *mocks.MockRegistryService) {},
+		},
 	}
 
 	for _, test := range tests {
@@ -684,11 +695,16 @@ func TestRegistryLogin(t *testing.T) {
 				}
 
 				config.Doit.Set(config.NS, doctl.ArgRegistryExpirySeconds, test.expirySeconds)
+				config.Doit.Set(config.NS, doctl.ArgRegistryNeverExpire, test.neverExpire)
 				config.Doit.Set(config.NS, doctl.ArgRegistryReadOnly, test.readOnly)
 
 				config.Out = os.Stderr
 				err := RunRegistryLogin(config)
-				assert.NoError(t, err)
+				if test.err != nil {
+					assert.Error(t, test.err, err)
+				} else {
+					assert.NoError(t, err)
+				}
 			})
 		})
 	}

integration/registry_login_test.go

@@ -49,7 +49,7 @@ var _ = suite("registry/login", func(t *testing.T, when spec.G, it spec.S) {
 
 				readWriteParam := req.URL.Query().Get("read_write")
 				expiryParam := req.URL.Query().Get("expiry_seconds")
-				if expiryParam == "3600" {
+				if expiryParam == "3600" || expiryParam == "2592000" {
 					w.Write([]byte(registryDockerCredentialsExpiryResponse))
 				} else if expiryParam == "" {
 					if readWriteParam == "false" {
@@ -96,9 +96,9 @@ var _ = suite("registry/login", func(t *testing.T, when spec.G, it spec.S) {
 			err = json.Unmarshal(fileBytes, &dc)
 			expect.NoError(err)
 
-			expect.Equal("Logging Docker in to registry.digitalocean.com\n", string(output))
+			expect.Equal("Logging Docker in to registry.digitalocean.com\nNotice: Login valid for 30 days. Use the --expiry-seconds flag to set a shorter expiration or --never-expire for no expiration.\n", string(output))
 			for host := range dc.Auths {
-				expect.Equal("registry.digitalocean.com", host)
+				expect.Equal("expiring.registry.com", host)
 			}
 		})
 	})
@@ -137,7 +137,7 @@ var _ = suite("registry/login", func(t *testing.T, when spec.G, it spec.S) {
 		})
 	})
 
-	when("read-only flag is passed", func() {
+	when("read-only flag is passed and the token doesn't expire", func() {
 		it("add the correct query parameter", func() {
 			tmpDir := t.TempDir()
 
@@ -150,6 +150,8 @@ var _ = suite("registry/login", func(t *testing.T, when spec.G, it spec.S) {
 				"login",
 				"--read-only",
 				"true",
+				"--never-expire",
+				"true",
 			)
 			cmd.Env = os.Environ()
 			cmd.Env = append(cmd.Env, fmt.Sprintf("DOCKER_CONFIG=%s", tmpDir))