Adjust for pr feedback

- use string slice for parsing - update help text for new flags
digitalocean · Apr 3, 2024 · 3a4c0cb · 3a4c0cb
1 parent be101e7
commit 3a4c0cb
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 33 deletions.
diff --git a/args.go b/args.go
@@ -343,8 +343,8 @@ const (
 	ArgGlobalLoadBalancerSettings = "glb-settings"
 	// ArgGlobalLoadBalancerCDNSettings is global load balancer CDN settings.
 	ArgGlobalLoadBalancerCDNSettings = "glb-cdn-settings"
-	// ArgLoadBalancerIDs is a list of load balancer IDs.
-	ArgLoadBalancerIDs = "lb-ids"
+	// ArgTargetLoadBalancerIDs is a list of target load balancer IDs.
+	ArgTargetLoadBalancerIDs = "target-lb-ids"
 
 	// ArgFirewallName is a name of the firewall.
 	ArgFirewallName = "name"

diff --git a/commands/load_balancers.go b/commands/load_balancers.go
@@ -92,14 +92,14 @@ With the load-balancer command, you can list, create, or delete load balancers,
 		"A comma-separated list of ALLOW rules for the load balancer, e.g.: `ip:1.2.3.4,cidr:1.2.0.0/16`")
 	AddStringSliceFlag(cmdLoadBalancerCreate, doctl.ArgDenyList, "", []string{},
 		"A comma-separated list of DENY rules for the load balancer, e.g.: `ip:1.2.3.4,cidr:1.2.0.0/16`")
-	AddStringFlag(cmdLoadBalancerCreate, doctl.ArgLoadBalancerDomains, "", "",
-		"A comma-separated list of domains required to ingress traffic to a global load balancer")
+	AddStringSliceFlag(cmdLoadBalancerCreate, doctl.ArgLoadBalancerDomains, "", []string{},
+		"A comma-separated list of domains required to ingress traffic to a global load balancer, e.g.: `name:test-domain,is_managed:true,certificate_id:test-cert-id`")
 	AddStringFlag(cmdLoadBalancerCreate, doctl.ArgGlobalLoadBalancerSettings, "", "",
-		"Global load balancer settings")
+		"Target protocol and port settings for ingressing traffic to a global load balancer, e.g.: `target_protocol:http,target_port:80`")
 	AddStringFlag(cmdLoadBalancerCreate, doctl.ArgGlobalLoadBalancerCDNSettings, "", "",
-		"Global load balancer CDN settings")
-	AddStringFlag(cmdLoadBalancerCreate, doctl.ArgLoadBalancerIDs, "", "",
-		"A comma-separated list of Load Balancer IDs to add to the global load balancer")
+		"CDN cache settings global load balancer, e.g.: `is_enabled:true`")
+	AddStringSliceFlag(cmdLoadBalancerCreate, doctl.ArgTargetLoadBalancerIDs, "", []string{},
+		"A comma-separated list of Load Balancer IDs to add as target to the global load balancer")
 	cmdLoadBalancerCreate.Flags().MarkHidden(doctl.ArgLoadBalancerType)
 
 	cmdRecordUpdate := CmdBuilder(cmd, RunLoadBalancerUpdate, "update <id>",
@@ -137,14 +137,14 @@ With the load-balancer command, you can list, create, or delete load balancers,
 		"A comma-separated list of ALLOW rules for the load balancer, e.g.: `ip:1.2.3.4,cidr:1.2.0.0/16`")
 	AddStringSliceFlag(cmdRecordUpdate, doctl.ArgDenyList, "", nil,
 		"A comma-separated list of DENY rules for the load balancer, e.g.: `ip:1.2.3.4,cidr:1.2.0.0/16`")
-	AddStringFlag(cmdRecordUpdate, doctl.ArgLoadBalancerDomains, "", "",
-		"A comma-separated list of domains required to ingress traffic to a global load balancer")
+	AddStringSliceFlag(cmdRecordUpdate, doctl.ArgLoadBalancerDomains, "", []string{},
+		"A comma-separated list of domains required to ingress traffic to a global load balancer, e.g.: `name:test-domain,is_managed:true,certificate_id:test-cert-id`")
 	AddStringFlag(cmdRecordUpdate, doctl.ArgGlobalLoadBalancerSettings, "", "",
-		"Global load balancer settings")
+		"Target protocol and port settings for ingressing traffic to a global load balancer, e.g.: `target_protocol:http,target_port:80`")
 	AddStringFlag(cmdRecordUpdate, doctl.ArgGlobalLoadBalancerCDNSettings, "", "",
-		"Global load balancer CDN settings")
-	AddStringFlag(cmdRecordUpdate, doctl.ArgLoadBalancerIDs, "", "",
-		"A comma-separated list of Load Balancer IDs to add to the global load balancer")
+		"CDN cache settings global load balancer, e.g.: `is_enabled:true`")
+	AddStringSliceFlag(cmdRecordUpdate, doctl.ArgTargetLoadBalancerIDs, "", []string{},
+		"A comma-separated list of Load Balancer IDs to add as target to the global load balancer")
 
 	CmdBuilder(cmd, RunLoadBalancerList, "list", "List load balancers", "Use this command to get a list of the load balancers on your account, including the following information for each:\n\n"+lbDetail, Writer,
 		aliasOpt("ls"), displayerType(&displayers.LoadBalancer{}))
@@ -424,14 +424,12 @@ func extractForwardingRules(s string) (forwardingRules []godo.ForwardingRule, er
 	return forwardingRules, err
 }
 
-func extractDomains(s string) (domains []*godo.LBDomain, err error) {
+func extractDomains(s []string) (domains []*godo.LBDomain, err error) {
 	if len(s) == 0 {
 		return domains, err
 	}
 
-	list := strings.Split(s, " ")
-
-	for _, v := range list {
+	for _, v := range s {
 		domain := new(godo.LBDomain)
 		if err := fillStructFromStringSliceArgs(domain, v); err != nil {
 			return nil, err
@@ -484,7 +482,6 @@ func fillStructFromStringSliceArgs(obj any, s string) error {
 			case reflect.String:
 				f.Set(reflect.ValueOf(val))
 			default:
-				fmt.Println("debug kv pairs", jv, val)
 				return fmt.Errorf("Unexpected type for struct field %v", val)
 			}
 		}
@@ -644,7 +641,7 @@ func buildRequestFromArgs(c *CmdConfig, r *godo.LoadBalancerRequest) error {
 		r.Firewall = firewall
 	}
 
-	dms, err := c.Doit.GetString(c.NS, doctl.ArgLoadBalancerDomains)
+	dms, err := c.Doit.GetStringSlice(c.NS, doctl.ArgLoadBalancerDomains)
 	if err != nil {
 		return err
 	}
@@ -681,7 +678,7 @@ func buildRequestFromArgs(c *CmdConfig, r *godo.LoadBalancerRequest) error {
 		r.GLBSettings.CDN = cdnSettings
 	}
 
-	lbIDsList, err := c.Doit.GetStringSlice(c.NS, doctl.ArgLoadBalancerIDs)
+	lbIDsList, err := c.Doit.GetStringSlice(c.NS, doctl.ArgTargetLoadBalancerIDs)
 	if err != nil {
 		return err
 	}

diff --git a/commands/load_balancers_test.go b/commands/load_balancers_test.go
@@ -164,11 +164,22 @@ func TestLoadBalancerCreateGLB(t *testing.T) {
 				CDN:            &godo.CDNSettings{IsEnabled: true},
 			},
 			DropletIDs: []int{1, 2},
-			Domains: []*godo.LBDomain{{
-				Name:          "test-domain-1",
-				IsManaged:     true,
-				CertificateID: "test-cert-id-1",
-			}},
+			Domains: []*godo.LBDomain{
+				{
+					Name:          "test-domain-1",
+					IsManaged:     true,
+					CertificateID: "test-cert-id-1",
+				},
+				{
+					Name:          "test-domain-2",
+					IsManaged:     false,
+					CertificateID: "test-cert-id-2",
+				},
+			},
+			TargetLoadBalancerIDs: []string{
+				"019cb059-603f-4828-8be4-641a20f25006",
+				"023da268-bc81-468f-aa4d-9abdc4f69935",
+			},
 		}
 		disableLetsEncryptDNSRecords := true
 		r.DisableLetsEncryptDNSRecords = &disableLetsEncryptDNSRecords
@@ -181,8 +192,15 @@ func TestLoadBalancerCreateGLB(t *testing.T) {
 		config.Doit.Set(config.NS, doctl.ArgGlobalLoadBalancerSettings, "target_protocol:http,target_port:80")
 		config.Doit.Set(config.NS, doctl.ArgGlobalLoadBalancerCDNSettings, "is_enabled:true")
 		config.Doit.Set(config.NS, doctl.ArgDropletIDs, []string{"1", "2"})
-		config.Doit.Set(config.NS, doctl.ArgLoadBalancerDomains, "name:test-domain-1,is_managed:true,certificate_id:test-cert-id-1")
+		config.Doit.Set(config.NS, doctl.ArgLoadBalancerDomains, []string{
+			"name:test-domain-1,is_managed:true,certificate_id:test-cert-id-1",
+			"name:test-domain-2,is_managed:false,certificate_id:test-cert-id-2",
+		})
 		config.Doit.Set(config.NS, doctl.ArgDisableLetsEncryptDNSRecords, true)
+		config.Doit.Set(config.NS, doctl.ArgTargetLoadBalancerIDs, []string{
+			"019cb059-603f-4828-8be4-641a20f25006",
+			"023da268-bc81-468f-aa4d-9abdc4f69935",
+		})
 
 		err := RunLoadBalancerCreate(config)
 		assert.NoError(t, err)
@@ -273,11 +291,22 @@ func TestLoadBalancerUpdateGLB(t *testing.T) {
 				CDN:            &godo.CDNSettings{IsEnabled: true},
 			},
 			DropletIDs: []int{1, 2},
-			Domains: []*godo.LBDomain{{
-				Name:          "test-domain-1",
-				IsManaged:     true,
-				CertificateID: "test-cert-id-1",
-			}},
+			Domains: []*godo.LBDomain{
+				{
+					Name:          "test-domain-1",
+					IsManaged:     true,
+					CertificateID: "test-cert-id-1",
+				},
+				{
+					Name:          "test-domain-2",
+					IsManaged:     false,
+					CertificateID: "test-cert-id-2",
+				},
+			},
+			TargetLoadBalancerIDs: []string{
+				"019cb059-603f-4828-8be4-641a20f25006",
+				"023da268-bc81-468f-aa4d-9abdc4f69935",
+			},
 		}
 		disableLetsEncryptDNSRecords := true
 		r.DisableLetsEncryptDNSRecords = &disableLetsEncryptDNSRecords
@@ -291,8 +320,15 @@ func TestLoadBalancerUpdateGLB(t *testing.T) {
 		config.Doit.Set(config.NS, doctl.ArgGlobalLoadBalancerSettings, "target_protocol:http,target_port:80")
 		config.Doit.Set(config.NS, doctl.ArgGlobalLoadBalancerCDNSettings, "is_enabled:true")
 		config.Doit.Set(config.NS, doctl.ArgDropletIDs, []string{"1", "2"})
-		config.Doit.Set(config.NS, doctl.ArgLoadBalancerDomains, "name:test-domain-1,is_managed:true,certificate_id:test-cert-id-1")
+		config.Doit.Set(config.NS, doctl.ArgLoadBalancerDomains, []string{
+			"name:test-domain-1,is_managed:true,certificate_id:test-cert-id-1",
+			"name:test-domain-2,is_managed:false,certificate_id:test-cert-id-2",
+		})
 		config.Doit.Set(config.NS, doctl.ArgDisableLetsEncryptDNSRecords, true)
+		config.Doit.Set(config.NS, doctl.ArgTargetLoadBalancerIDs, []string{
+			"019cb059-603f-4828-8be4-641a20f25006",
+			"023da268-bc81-468f-aa4d-9abdc4f69935",
+		})
 
 		err := RunLoadBalancerUpdate(config)
 		assert.NoError(t, err)