fix(registry): use RepositoryV2 displayer for list-v2 commands (#1797 / #1799)

commands/databases.go

@@ -2535,6 +2535,7 @@ func RunDatabaseFirewallRulesAppend(c *CmdConfig) error {
 		firewallRule.Value = rule.Value
 		firewallRule.ClusterUUID = rule.ClusterUUID
 		firewallRule.UUID = rule.UUID
+		firewallRule.Description = rule.Description
 
 		allRules = append(allRules, firewallRule)
 	}
@@ -2581,6 +2582,7 @@ func RunDatabaseFirewallRulesRemove(c *CmdConfig) error {
 				ClusterUUID: rule.ClusterUUID,
 				Type:        rule.Type,
 				Value:       rule.Value,
+				Description: rule.Description,
 			})
 		}
 	}

commands/databases_test.go

@@ -1985,3 +1985,93 @@ func TestDatabaseConfigurationUpdate(t *testing.T) {
 		assert.Error(t, err)
 	})
 }
+
+func TestDatabaseFirewallRulesAppend(t *testing.T) {
+	withTestClient(t, func(config *CmdConfig, tm *tcMocks) {
+		existingRules := do.DatabaseFirewallRules{
+			{
+				DatabaseFirewallRule: &godo.DatabaseFirewallRule{
+					UUID:        "rule-1-uuid",
+					ClusterUUID: testDBCluster.ID,
+					Type:        "ip_addr",
+					Value:       "10.0.0.5",
+					Description: "Staging App",
+				},
+			},
+		}
+
+		tm.databases.EXPECT().GetFirewallRules(testDBCluster.ID).Return(existingRules, nil).Times(2)
+
+		expectedRequest := &godo.DatabaseUpdateFirewallRulesRequest{
+			Rules: []*godo.DatabaseFirewallRule{
+				{
+					Type:        "ip_addr",
+					Value:       "1.2.3.4",
+					ClusterUUID: testDBCluster.ID,
+				},
+				{
+					UUID:        "rule-1-uuid",
+					ClusterUUID: testDBCluster.ID,
+					Type:        "ip_addr",
+					Value:       "10.0.0.5",
+					Description: "Staging App",
+				},
+			},
+		}
+
+		tm.databases.EXPECT().UpdateFirewallRules(testDBCluster.ID, expectedRequest).Return(nil)
+
+		config.Args = append(config.Args, testDBCluster.ID)
+		config.Doit.Set(config.NS, doctl.ArgDatabaseFirewallRule, "ip_addr:1.2.3.4")
+
+		err := RunDatabaseFirewallRulesAppend(config)
+		assert.NoError(t, err)
+	})
+}
+
+func TestDatabaseFirewallRulesRemove(t *testing.T) {
+	withTestClient(t, func(config *CmdConfig, tm *tcMocks) {
+		existingRules := do.DatabaseFirewallRules{
+			{
+				DatabaseFirewallRule: &godo.DatabaseFirewallRule{
+					UUID:        "rule-1-uuid",
+					ClusterUUID: testDBCluster.ID,
+					Type:        "ip_addr",
+					Value:       "10.0.0.5",
+					Description: "Staging App",
+				},
+			},
+			{
+				DatabaseFirewallRule: &godo.DatabaseFirewallRule{
+					UUID:        "rule-2-uuid",
+					ClusterUUID: testDBCluster.ID,
+					Type:        "ip_addr",
+					Value:       "10.0.0.6",
+					Description: "Prod App",
+				},
+			},
+		}
+
+		tm.databases.EXPECT().GetFirewallRules(testDBCluster.ID).Return(existingRules, nil).Times(2)
+
+		expectedRequest := &godo.DatabaseUpdateFirewallRulesRequest{
+			Rules: []*godo.DatabaseFirewallRule{
+				{
+					UUID:        "rule-2-uuid",
+					ClusterUUID: testDBCluster.ID,
+					Type:        "ip_addr",
+					Value:       "10.0.0.6",
+					Description: "Prod App",
+				},
+			},
+		}
+
+		tm.databases.EXPECT().UpdateFirewallRules(testDBCluster.ID, expectedRequest).Return(nil)
+
+		config.Args = append(config.Args, testDBCluster.ID)
+		config.Doit.Set(config.NS, doctl.ArgDatabaseFirewallRuleUUID, "rule-1-uuid")
+
+		err := RunDatabaseFirewallRulesRemove(config)
+		assert.NoError(t, err)
+	})
+}

commands/displayers/database.go

@@ -736,6 +736,7 @@ func (dr *DatabaseFirewallRules) Cols() []string {
 		"ClusterUUID",
 		"Type",
 		"Value",
+		"Description",
 	}
 }
 
@@ -746,6 +747,7 @@ func (dr *DatabaseFirewallRules) ColMap() map[string]string {
 		"ClusterUUID": "ClusterUUID",
 		"Type":        "Type",
 		"Value":       "Value",
+		"Description": "Description",
 	}
 }
 
@@ -758,6 +760,7 @@ func (dr *DatabaseFirewallRules) KV() []map[string]any {
 			"ClusterUUID": r.ClusterUUID,
 			"Type":        r.Type,
 			"Value":       r.Value,
+			"Description": r.Description,
 		}
 		out = append(out, o)
 	}

commands/displayers/registry.go

@@ -113,7 +113,7 @@ type RepositoryV2 struct {
 	Repositories []do.RepositoryV2
 }
 
-var _ Displayable = &Repository{}
+var _ Displayable = &RepositoryV2{}
 
 func (r *RepositoryV2) JSON(out io.Writer) error {
 	return writeJSON(r.Repositories, out)

commands/registry.go

@@ -181,7 +181,7 @@ func Repository() *Command {
 		cmd,
 		RunListRepositoriesV2, "list-v2",
 		"List repositories for a container registry", listRepositoriesV2Desc,
-		Writer, aliasOpt("ls2"), displayerType(&displayers.Repository{}),
+		Writer, aliasOpt("ls2"), displayerType(&displayers.RepositoryV2{}), overrideCmdNS(overrideNS),
 	)
 	cmdListRepositoriesV2.overrideNS = overrideNS
 	addRegistryFlag(cmdListRepositoriesV2)
@@ -1581,7 +1581,7 @@ func RegistriesRepository() *Command {
 		cmd,
 		RunRegistriesListRepositoriesV2, "list-v2 <registry-name>",
 		"List repositories for a container registry", listRepositoriesV2Desc,
-		Writer, aliasOpt("ls2"), displayerType(&displayers.Repository{}),
+		Writer, aliasOpt("ls2"), displayerType(&displayers.RepositoryV2{}), overrideCmdNS(overrideNS),
 	)
 	cmdListRepositoriesV2.overrideNS = overrideNS
 	cmdListRepositoriesV2.Example = `The following example lists repositories in a registry named ` + "`" + `example-registry` + "`" + ` and uses the ` + "`" + `--format` + "`" + ` flag to return only the name and update time of each repository: doctl registries repository list-v2 example-registry --format Name,UpdatedAt`

commands/registry_test.go

@@ -23,6 +23,7 @@ import (
 	"time"
 
 	"github.com/digitalocean/doctl"
+	"github.com/digitalocean/doctl/commands/displayers"
 	"github.com/digitalocean/doctl/do"
 	"github.com/digitalocean/doctl/do/mocks"
 	"github.com/digitalocean/godo"
@@ -151,6 +152,27 @@ func TestRepositoryCommand(t *testing.T) {
 	cmd := Repository()
 	assert.NotNil(t, cmd)
 	assertCommandNames(t, cmd, "list", "list-v2", "list-manifests", "list-tags", "delete-manifest", "delete-tag")
+
+	for _, child := range cmd.ChildCommands() {
+		if child.Name() == "list-v2" {
+			assert.Equal(t, (&displayers.RepositoryV2{}).Cols(), child.fmtCols)
+		} else if child.Name() == "list" {
+			assert.Equal(t, (&displayers.Repository{}).Cols(), child.fmtCols)
+		}
+	}
+}
+
+func TestRegistriesRepositoryCommand(t *testing.T) {
+	cmd := RegistriesRepository()
+	assert.NotNil(t, cmd)
+
+	for _, child := range cmd.ChildCommands() {
+		if strings.HasPrefix(child.Name(), "list-v2") {
+			assert.Equal(t, (&displayers.RepositoryV2{}).Cols(), child.fmtCols)
+		} else if strings.HasPrefix(child.Name(), "list ") {
+			assert.Equal(t, (&displayers.Repository{}).Cols(), child.fmtCols)
+		}
+	}
 }
 
 func TestGarbageCollectionCommand(t *testing.T) {
@@ -334,6 +356,24 @@ func TestRepositoryListV2(t *testing.T) {
 			assert.False(t, strings.Contains(output, testRepositoryV2NoTags.LatestManifest.Blobs[0].Digest))
 		})
 	})
+	t.Run("with custom V2 format fields", func(t *testing.T) {
+		withTestClient(t, func(config *CmdConfig, tm *tcMocks) {
+			tm.registry.EXPECT().Get().Return(&testRegistry, nil)
+			tm.registry.EXPECT().ListRepositoriesV2(testRepositoryV2.RegistryName).Return([]do.RepositoryV2{testRepositoryV2}, nil)
+
+			config.Doit.Set(config.NS, doctl.ArgFormat, "Name,ManifestCount,UpdatedAt,LatestManifest,LatestTag,TagCount")
+
+			var buf bytes.Buffer
+			config.Out = &buf
+			err := RunListRepositoriesV2(config)
+			assert.NoError(t, err)
+
+			output := buf.String()
+			assert.True(t, strings.Contains(output, testRepositoryV2.Name))
+			assert.True(t, strings.Contains(output, fmt.Sprintf("%d", testRepositoryV2.ManifestCount)))
+			assert.True(t, strings.Contains(output, testRepositoryV2.LatestManifest.Digest))
+		})
+	})
 }
 
 func TestRepositoryListTags(t *testing.T) {
@@ -1289,3 +1329,46 @@ func TestRegistriesList(t *testing.T) {
 		assert.Contains(t, output, testRegistry.Name)
 	})
 }
+
+func TestRegistriesListRepositoriesV2(t *testing.T) {
+	t.Run("default format", func(t *testing.T) {
+		withTestClient(t, func(config *CmdConfig, tm *tcMocks) {
+			config.Args = append(config.Args, testRegistryName)
+			config.Registries = func() do.RegistriesService {
+				return tm.registries
+			}
+			tm.registries.EXPECT().ListRepositoriesV2(testRegistryName).Return([]do.RepositoryV2{testRepositoryV2}, nil)
+
+			var buf bytes.Buffer
+			config.Out = &buf
+			err := RunRegistriesListRepositoriesV2(config)
+			assert.NoError(t, err)
+
+			output := buf.String()
+			assert.True(t, strings.Contains(output, testRepositoryV2.Name))
+			assert.True(t, strings.Contains(output, testRepositoryV2.LatestManifest.Digest))
+		})
+	})
+
+	t.Run("with custom V2 format fields", func(t *testing.T) {
+		withTestClient(t, func(config *CmdConfig, tm *tcMocks) {
+			config.Args = append(config.Args, testRegistryName)
+			config.Registries = func() do.RegistriesService {
+				return tm.registries
+			}
+			tm.registries.EXPECT().ListRepositoriesV2(testRegistryName).Return([]do.RepositoryV2{testRepositoryV2}, nil)
+
+			config.Doit.Set(config.NS, doctl.ArgFormat, "Name,ManifestCount,UpdatedAt,LatestManifest,LatestTag,TagCount")
+
+			var buf bytes.Buffer
+			config.Out = &buf
+			err := RunRegistriesListRepositoriesV2(config)
+			assert.NoError(t, err)
+
+			output := buf.String()
+			assert.True(t, strings.Contains(output, testRepositoryV2.Name))
+			assert.True(t, strings.Contains(output, fmt.Sprintf("%d", testRepositoryV2.ManifestCount)))
+			assert.True(t, strings.Contains(output, testRepositoryV2.LatestManifest.Digest))
+		})
+	})
+}

integration/database_firewall_add_test.go

@@ -136,22 +136,19 @@ var _ = suite("database/firewalls", func(t *testing.T, when spec.G, it spec.S) {
 			output, err := cmd.CombinedOutput()
 			expect.NoError(err, fmt.Sprintf("received error output: %s", output))
 
-			expected := strings.TrimSpace(string(output))
-			actual := strings.TrimSpace(databasesAddFirewallRuleOutput)
+			expected := strings.TrimSpace(databasesAddFirewallRuleOutput)
+			actual := strings.TrimSpace(string(output))
 
 			expect.Equal(expected, actual)
-
-			fmt.Println(expected)
-			fmt.Println(actual)
 		})
 	})
 
 })
 
 const (
 	databasesAddFirewallRuleOutput = `
-UUID                                    ClusterUUID                             Type    Value
-cdb689c2-56e6-48e6-869d-306c85af178d    d168d635-1c88-4616-b9b4-793b7c573927    tag     new-firewall-tag
+UUID                                    ClusterUUID                             Type    Value               Description
+cdb689c2-56e6-48e6-869d-306c85af178d    d168d635-1c88-4616-b9b4-793b7c573927    tag     new-firewall-tag    
 cdb689c2-56e6-48e6-869d-306c85af178d    d168d635-1c88-4616-b9b4-793b7c573927    tag     old-firewall-tag
 `
 )

integration/database_firewall_list_test.go

@@ -69,7 +69,7 @@ var _ = suite("database/firewalls", func(t *testing.T, when spec.G, it spec.S) {
 
 const (
 	databasesFirewallRuleOutput = `
-UUID                                    ClusterUUID                             Type       Value
+UUID                                    ClusterUUID                             Type       Value            Description
 cdb689c2-56e6-48e6-869d-306c85af178d    d168d635-1c88-4616-b9b4-793b7c573927    ip_addr    107.13.36.145
 `
 	databasesListFirewallRuleResponse = `

integration/database_firewall_remove_test.go

@@ -114,20 +114,17 @@ var _ = suite("database/firewalls", func(t *testing.T, when spec.G, it spec.S) {
 			output, err := cmd.CombinedOutput()
 			expect.NoError(err, fmt.Sprintf("received error output: %s", output))
 
-			expected := strings.TrimSpace(string(output))
-			actual := strings.TrimSpace(databasesRemoveFirewallRuleOutput)
+			expected := strings.TrimSpace(databasesRemoveFirewallRuleOutput)
+			actual := strings.TrimSpace(string(output))
 
 			expect.Equal(expected, actual)
-
-			fmt.Println(expected)
-			fmt.Println(actual)
 		})
 	})
 
 })
 
 const (
 	databasesRemoveFirewallRuleOutput = `
-UUID                                    ClusterUUID                             Type    Value
+UUID                                    ClusterUUID                             Type    Value               Description
 cdb689c2-56e6-48e6-869d-306c85af178d    d168d635-1c88-4616-b9b4-793b7c573927    tag     new-firewall-tag`
 )

integration/database_firewall_update_test.go

@@ -80,7 +80,7 @@ var _ = suite("database/firewalls", func(t *testing.T, when spec.G, it spec.S) {
 const (
 	databasesUpdateFirewallUpdateRequest = `{"rules":[{"uuid":"","cluster_uuid":"","type":"ip_addr","value":"192.168.1.1","created_at":"0001-01-01T00:00:00Z"}]}`
 	databasesUpdateFirewallRuleOutput    = `
-UUID                                    ClusterUUID                             Type       Value
+UUID                                    ClusterUUID                             Type       Value          Description
 82ebbbd4-437c-4e11-bfd2-644ccb555de0    d168d635-1c88-4616-b9b4-793b7c573927    ip_addr    192.168.1.1`
 	databasesUpdateFirewallRuleResponse = `{
 		"rules":[

integration/registries_list_repositories_test.go

@@ -102,6 +102,25 @@ var _ = suite("registries/list-repositories", func(t *testing.T, when spec.G, it
 			expect.Contains(string(output), "test-repo-1")
 			expect.Contains(string(output), "test-repo-2")
 		})
+
+		it("lists repositories using registries command with custom V2 format fields", func() {
+			cmd := exec.Command(builtBinaryPath,
+				"-t", "some-magic-token",
+				"-u", server.URL,
+				"registries",
+				"repository",
+				"list-v2",
+				"test-registry",
+				"--format", "Name,ManifestCount,UpdatedAt",
+			)
+
+			output, err := cmd.CombinedOutput()
+			expect.NoError(err, fmt.Sprintf("received error output: %s", output))
+			expect.Contains(string(output), "Name")
+			expect.Contains(string(output), "Manifest Count")
+			expect.Contains(string(output), "test-repo-1")
+			expect.Contains(string(output), "test-repo-2")
+		})
 	})
 
 	it.After(func() {

integration/registry_repo_list_v2_test.go

@@ -76,6 +76,27 @@ var _ = suite("registry/repository/list-v2", func(t *testing.T, when spec.G, it
 
 		expect.Equal(strings.TrimSpace(repositoryListV2Output), strings.TrimSpace(string(output)))
 	})
+
+	it("returns list of repositories in registry with custom format including V2 fields", func() {
+		cmd := exec.Command(builtBinaryPath,
+			"-t", "some-magic-token",
+			"-u", server.URL,
+			"registry",
+			"repository",
+			"list-v2",
+			"--format", "Name,ManifestCount,UpdatedAt",
+		)
+
+		output, err := cmd.CombinedOutput()
+		expect.NoError(err, "Output: %s", output)
+
+		expectedOutput := `
+Name      Manifest Count    Updated At
+repo-1    82                2021-04-09 23:54:25 +0000 UTC
+repo-2    82                <nil>
+`
+		expect.Equal(strings.TrimSpace(expectedOutput), strings.TrimSpace(string(output)))
+	})
 })
 
 var (