Repository: digitalocean/doctl
Pull request #1388 (2023-07-05 15:55:08)

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.

danaelhe: Looks good!
bentranter: @andrewsomething Thanks for the suggestion, totally forgot we'd talked about adding that when discussing the change before. I added that message right after the `Logging Docker in...` message appears.

Patch:
diff --git a/args.go b/args.go
index e2ecb0d3..a13291b9 100644
--- a/args.go
+++ b/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
diff --git a/commands/registry.go b/commands/registry.go
index d45e568e..035b3a8c 100644
--- a/commands/registry.go
+++ b/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)


Test patch:
diff --git a/commands/registry_test.go b/commands/registry_test.go
index 8279a012..e7a34d47 100644
--- a/commands/registry_test.go
+++ b/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)
+				}
 			})
 		})
 	}
diff --git a/integration/registry_login_test.go b/integration/registry_login_test.go
index 6312b096..08084992 100644
--- a/integration/registry_login_test.go
+++ b/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))