the agent identifiers pass to the proxy agent must be URL encoded

Author: charleszheng44

cmd/agent/main.go

@@ -18,13 +18,12 @@ package main
 
 import (
 	"crypto/tls"
-	"errors"
 	"flag"
 	"fmt"
 	"net"
 	"net/http"
+	"net/url"
 	"os"
-	"strings"
 	"time"
 
 	"github.com/google/uuid"
@@ -105,7 +104,7 @@ func (o *GrpcProxyAgentOptions) Flags() *pflag.FlagSet {
 	flags.DurationVar(&o.syncInterval, "sync-interval", o.syncInterval, "The initial interval by which the agent periodically checks if it has connections to all instances of the proxy server.")
 	flags.DurationVar(&o.probeInterval, "probe-interval", o.probeInterval, "The interval by which the agent periodically checks if its connections to the proxy server are ready.")
 	flags.StringVar(&o.serviceAccountTokenPath, "service-account-token-path", o.serviceAccountTokenPath, "If non-empty proxy agent uses this token to prove its identity to the proxy server.")
-	flags.StringVar(&o.agentIdentifiers, "agent-identifiers", o.agentIdentifiers, "Identifiers of the agent that will be used by the server when choosing agent. e.g.,host=localhost,host=node1.mydomain.com,cidr=127.0.0.1/16,ipv4=1.2.3.4,ipv4=5.6.7.8,ipv6=:::::")
+	flags.StringVar(&o.agentIdentifiers, "agent-identifiers", o.agentIdentifiers, "Identifiers of the agent that will be used by the server when choosing agent. N.B. the list of identifiers must be in URL encoded format. e.g.,host=localhost&host=node1.mydomain.com&cidr=127.0.0.1/16&ipv4=1.2.3.4&ipv4=5.6.7.8&ipv6=:::::")
 	return flags
 }
 
@@ -168,20 +167,18 @@ func (o *GrpcProxyAgentOptions) Validate() error {
 }
 
 func validateAgentIdentifiers(agentIdentifiers string) error {
-	entries := strings.Split(agentIdentifiers, ",")
-	for _, entry := range entries {
-		kv := strings.Split(entry, "=")
-		if len(kv) != 2 {
-			return errors.New("invalid arguments format, the valid format is " +
-				"<addressType1>=<address1>,<addressType2>=<address2>")
-		}
-		switch agent.IdentifierType(kv[0]) {
+	decoded, err := url.ParseQuery(agentIdentifiers)
+	if err != nil {
+		return err
+	}
+	for idType := range decoded {
+		switch agent.IdentifierType(idType) {
 		case agent.IPv4:
 		case agent.IPv6:
 		case agent.CIDR:
 		case agent.Host:
 		default:
-			return fmt.Errorf("unknown address type: %s", kv[0])
+			return fmt.Errorf("unknown address type: %s", idType)
 		}
 	}
 	return nil

cmd/server/main.go

@@ -294,10 +294,12 @@ func (o *ProxyRunOptions) Validate() error {
 	}
 
 	// validate the proxy strategies
-	pss := strings.Split(o.proxyStrategies, ",")
-	for _, ps := range pss {
-		if ps != string(server.ProxyStrategyDestHost) {
-			return fmt.Errorf("unknown proxy strategy: %s, available strategy is: destHost", ps)
+	if o.proxyStrategies != "" {
+		pss := strings.Split(o.proxyStrategies, ",")
+		for _, ps := range pss {
+			if ps != string(server.ProxyStrategyDestHost) {
+				return fmt.Errorf("unknown proxy strategy: %s, available strategy is: destHost", ps)
+			}
 		}
 	}
 

pkg/agent/client.go

@@ -17,11 +17,13 @@ limitations under the License.
 package agent
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"io"
 	"io/ioutil"
 	"net"
+	"net/url"
 	"strconv"
 	"strings"
 	"sync"
@@ -99,37 +101,48 @@ const (
 	UID  IdentifierType = "uid"
 )
 
+// String prints out each <identifierType>=<identifier> separately
+func (a AgentIdentifiers) String() string {
+	var buf bytes.Buffer
+	for _, ipv4 := range a.IPv4 {
+		buf.WriteString(fmt.Sprintf("ipv4=%s,", ipv4))
+	}
+	for _, ipv6 := range a.IPv6 {
+		buf.WriteString(fmt.Sprintf("ipv6=%s,", ipv6))
+	}
+	for _, host := range a.Host {
+		buf.WriteString(fmt.Sprintf("host=%s,", host))
+	}
+	for _, cidr := range a.Host {
+		buf.WriteString(fmt.Sprintf("cidr=%s,", cidr))
+	}
+	return strings.TrimSuffix(buf.String(), ",")
+}
+
 // GenAgentIdentifiers generates an AgentIdentifiers based on the input string, the
 // input string should be a comma-seprated list with each item in the format
 // of <IdentifierType>=<address>
-func GenAgentIdentifiers(addrs string) AgentIdentifiers {
+func GenAgentIdentifiers(addrs string) (AgentIdentifiers, error) {
 	var agentIDs AgentIdentifiers
-	entries := strings.Split(addrs, ",")
-	for _, entry := range entries {
-		kv := strings.Split(entry, "=")
-		if len(kv) != 2 {
-			klog.V(4).InfoS("Invalid agent address input format",
-				"got", entry, "expect", "<IdentifierType>=<address>")
-			continue
-		}
-		if kv[1] == "" {
-			continue
-		}
-		switch IdentifierType(kv[0]) {
+	decoded, err := url.ParseQuery(addrs)
+	if err != nil {
+		return agentIDs, fmt.Errorf("fail to parse url encoded string: %v", err)
+	}
+	for idType, ids := range decoded {
+		switch IdentifierType(idType) {
 		case IPv4:
-			agentIDs.IPv4 = append(agentIDs.IPv4, kv[1])
+			agentIDs.IPv4 = append(agentIDs.IPv4, ids...)
 		case IPv6:
-			agentIDs.IPv6 = append(agentIDs.IPv6, kv[1])
+			agentIDs.IPv6 = append(agentIDs.IPv6, ids...)
 		case Host:
-			agentIDs.Host = append(agentIDs.Host, kv[1])
+			agentIDs.Host = append(agentIDs.Host, ids...)
 		case CIDR:
-			agentIDs.CIDR = append(agentIDs.CIDR, kv[1])
+			agentIDs.CIDR = append(agentIDs.CIDR, ids...)
 		default:
-			klog.V(5).InfoS("Unknown address type", "Address Type", kv[0])
-			continue
+			return agentIDs, fmt.Errorf("Unknown address type: %s", idType)
 		}
 	}
-	return agentIDs
+	return agentIDs, nil
 }
 
 // AgentClient runs on the node network side. It connects to proxy server and establishes

pkg/server/backend_manager.go

@@ -168,18 +168,18 @@ func containIdType(idTypes []pkgagent.IdentifierType, idType pkgagent.Identifier
 // AddBackend adds a backend.
 func (s *DefaultBackendStorage) AddBackend(identifier string, idType pkgagent.IdentifierType, conn agent.AgentService_ConnectServer) Backend {
 	if !containIdType(s.idTypes, idType) {
-		klog.ErrorS(&ErrWrongIDType{idType, s.idTypes}, "fail to add backend")
+		klog.V(4).InfoS("fial to add backend", "backend", identifier, "error", &ErrWrongIDType{idType, s.idTypes})
 		return nil
 	}
-	klog.V(2).InfoS("Register backend for agent", "connection", conn, "agentID", agentID)
+	klog.V(2).InfoS("Register backend for agent", "connection", conn, "agentID", identifier)
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	_, ok := s.backends[identifier]
 	addedBackend := newBackend(conn)
 	if ok {
 		for _, v := range s.backends[identifier] {
 			if v.conn == conn {
-				klog.V(1).InfoS("This should not happen. Adding existing backend for agent", "connection", conn, "agentID", agentID)
+				klog.V(1).InfoS("This should not happen. Adding existing backend for agent", "connection", conn, "agentID", identifier)
 				return v
 			}
 		}

pkg/server/server.go

@@ -485,7 +485,10 @@ func getAgentIdentifiers(stream agent.AgentService_ConnectServer) (pkgagent.Agen
 		return agentIdentifiers, nil
 	}
 
-	agentIdentifiers = pkgagent.GenAgentIdentifiers(agentIDs[0])
+	agentIdentifiers, err := pkgagent.GenAgentIdentifiers(agentIDs[0])
+	if err != nil {
+		return agentIdentifiers, err
+	}
 	return agentIdentifiers, nil
 }