diff --git a/examples/preflight/host/tcp-load-balancer.yaml b/examples/preflight/host/tcp-load-balancer.yaml index 4a8dcdf2..0813bf3a 100644 --- a/examples/preflight/host/tcp-load-balancer.yaml +++ b/examples/preflight/host/tcp-load-balancer.yaml @@ -12,6 +12,9 @@ spec: - tcpLoadBalancer: collectorName: loadbalancer outcomes: + - fail: + when: "invalid-address" + message: The Load Balancer address is not valid. - fail: when: "connection-refused" message: Connection to port 7443 via load balancer was refused. diff --git a/pkg/collect/host_network.go b/pkg/collect/host_network.go index 44d990fa..e16e0809 100644 --- a/pkg/collect/host_network.go +++ b/pkg/collect/host_network.go @@ -3,12 +3,15 @@ package collect import ( "bytes" "net" + "regexp" + "strconv" "strings" "time" "github.com/pkg/errors" "github.com/replicatedhq/troubleshoot/pkg/debug" "github.com/segmentio/ksuid" + validation "k8s.io/apimachinery/pkg/util/validation" ) type NetworkStatus string @@ -20,13 +23,59 @@ const ( NetworkStatusConnected = "connected" NetworkStatusErrorOther = "error" NetworkStatusBindPermissionDenied = "bind-permission-denied" + NetworkStatusInvalidAddress = "invalid-address" ) type NetworkStatusResult struct { - Status NetworkStatus `json:"status"` + Status NetworkStatus `json:"status"` + Message string `json:"message"` +} + +var ipRegexp = regexp.MustCompile(`^[0-9.]+$`) + +func isValidLoadBalancerAddress(address string) bool { + splitString := strings.Split(address, ":") + + if len(splitString) != 2 { // should be hostAddress:port + return false + } + hostAddress := splitString[0] + port, err := strconv.Atoi(splitString[1]) + if err != nil { + return false + } + portErrors := validation.IsValidPortNum(port) + + if len(portErrors) > 0 { + return false + } + + // Checking for uppercase letters + if strings.ToLower(hostAddress) != hostAddress { + return false + } + + // Checking if it's all numbers and . + if ipRegexp.MatchString(hostAddress) { + + // Check for isValidIP + + test := validation.IsValidIP(hostAddress) + return len(test) == 0 + + } + + errs := validation.IsQualifiedName(hostAddress) + + return len(errs) == 0 } func checkTCPConnection(progressChan chan<- interface{}, listenAddress string, dialAddress string, timeout time.Duration) (NetworkStatus, error) { + + if !isValidLoadBalancerAddress(dialAddress) { + return NetworkStatusInvalidAddress, errors.Errorf("Invalid Load Balancer Address: %v", dialAddress) + } + lstn, err := net.Listen("tcp", listenAddress) if err != nil { if strings.Contains(err.Error(), "address already in use") { @@ -42,7 +91,6 @@ func checkTCPConnection(progressChan chan<- interface{}, listenAddress string, d // token until the server responds with its token. requestToken := ksuid.New().Bytes() responseToken := ksuid.New().Bytes() - go func() { for { conn, err := lstn.Accept() @@ -60,14 +108,19 @@ func checkTCPConnection(progressChan chan<- interface{}, listenAddress string, d for { if time.Now().After(stopAfter) { + debug.Printf("Timeout") + return NetworkStatusConnectionTimeout, nil } conn, err := net.DialTimeout("tcp", dialAddress, 50*time.Millisecond) if err != nil { + debug.Printf("Error: %s", err) + if strings.Contains(err.Error(), "i/o timeout") { progressChan <- err time.Sleep(time.Second) + continue } if strings.Contains(err.Error(), "connection refused") { @@ -83,6 +136,7 @@ func checkTCPConnection(progressChan chan<- interface{}, listenAddress string, d progressChan <- errors.New("failed to verify connection to server") time.Sleep(time.Second) } + } func handleTestConnection(conn net.Conn, requestToken []byte, responseToken []byte) bool { diff --git a/pkg/collect/host_network_test.go b/pkg/collect/host_network_test.go new file mode 100644 index 00000000..d9e2ea32 --- /dev/null +++ b/pkg/collect/host_network_test.go @@ -0,0 +1,103 @@ +package collect + +import ( + "testing" +) + +func Test_isValidLoadBalancerAddress(t *testing.T) { + type args struct { + address string + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "Valid IP and Port", + args: args{address: "1.2.3.4:6443"}, + want: true, + }, + { + name: "Too many :'s in address", + args: args{address: "1.2.3.4:64:6443"}, + want: false, + }, + { + name: "Valid domain and Port ", + args: args{address: "replicated.com:80"}, + want: true, + }, + { + name: "Valid subdomain and Port ", + args: args{address: "sub.replicated.com:80"}, + want: true, + }, + { + name: "Valid subdomain with '-' and Port ", + args: args{address: "sub-domain.replicated.com:80"}, + want: true, + }, + { + name: "Special Character", + args: args{address: "sw!$$.com:80"}, + want: false, + }, + { + name: "Too many characters", + args: args{address: "howlongcanwemakethiswithoutrunningoutofwordsbecasueweneedtohitatleast64.com:80"}, + want: false, + }, + { + name: "Capital Letters", + args: args{address: "testDomain.com:80"}, + want: false, + }, + { + name: "Invalid IP", + args: args{address: "55.555.51.23:80"}, + want: false, + }, + { + name: "Too many consecutive .", + args: args{address: "55..55.51.23:80"}, + want: false, + }, + { + name: "Invalid Port too low", + args: args{address: "55.55.51.23:0"}, + want: false, + }, + { + name: "Invalid Port too large", + args: args{address: "55.55.51.23:999990"}, + want: false, + }, + { + name: "Invalid Port Character", + args: args{address: "55.55.51.23:port"}, + want: false, + }, + { + name: "Invalid Port Number", + args: args{address: "55.55.51.23:32.5"}, + want: false, + }, + { + name: "Codes in addresses", + args: args{address: "192.168.0.1"}, + want: false, + }, { + name: "Codes in addresses", + args: args{address: "\033[34m192.168.0.1\033[00m\n "}, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isValidLoadBalancerAddress(tt.args.address); got != tt.want { + t.Errorf("checkValidTCPAddress() = %v, want %v for %v", got, tt.want, tt.args.address) + } + }) + } +} diff --git a/pkg/collect/host_tcploadbalancer.go b/pkg/collect/host_tcploadbalancer.go index 29467629..fa43b611 100644 --- a/pkg/collect/host_tcploadbalancer.go +++ b/pkg/collect/host_tcploadbalancer.go @@ -26,20 +26,34 @@ func (c *CollectHostTCPLoadBalancer) Collect(progressChan chan<- interface{}) (m listenAddress := fmt.Sprintf("0.0.0.0:%d", c.hostCollector.Port) dialAddress := c.hostCollector.Address + name := path.Join("tcpLoadBalancer", "tcpLoadBalancer.json") + if c.hostCollector.CollectorName != "" { + name = path.Join("tcpLoadBalancer", fmt.Sprintf("%s.json", c.hostCollector.CollectorName)) + } + timeout := 60 * time.Minute if c.hostCollector.Timeout != "" { var err error timeout, err = time.ParseDuration(c.hostCollector.Timeout) if err != nil { - return nil, errors.Wrap(err, "failed to parse durection") + return nil, errors.Wrap(err, "failed to parse duration") } } - networkStatus, err := checkTCPConnection(progressChan, listenAddress, dialAddress, timeout) if err != nil { - return nil, err - } + result := NetworkStatusResult{ + Status: networkStatus, + Message: err.Error(), + } + b, err := json.Marshal(result) + if err != nil { + return nil, errors.Wrap(err, "failed to marshal result") + } + return map[string][]byte{ + name: b, + }, err + } result := NetworkStatusResult{ Status: networkStatus, } @@ -49,11 +63,6 @@ func (c *CollectHostTCPLoadBalancer) Collect(progressChan chan<- interface{}) (m return nil, errors.Wrap(err, "failed to marshal result") } - name := path.Join("tcpLoadBalancer", "tcpLoadBalancer.json") - if c.hostCollector.CollectorName != "" { - name = path.Join("tcpLoadBalancer", fmt.Sprintf("%s.json", c.hostCollector.CollectorName)) - } - return map[string][]byte{ name: b, }, nil