Merge pull request #45 from gatewayd-io/return-errors

Return errors

Author: mostafa

.golangci.yaml

@@ -1,3 +1,6 @@
+run:
+  timeout: 5m
+  tests: false
 linters:
   enable-all: true
   disable:
@@ -20,5 +23,3 @@ linters:
     - musttag
     - nosnakecase
     - wrapcheck
-run:
-  tests: false

plugin/errors.go

@@ -0,0 +1,5 @@
+package plugin
+
+import "errors"
+
+var ErrInvalidAddressPortPair = errors.New("invalid address:port pair")

plugin/scheduler.go

@@ -21,18 +21,43 @@ func (p *Plugin) PeriodicInvalidator() {
 		p.Logger.Trace("Got proxies from GatewayD", "proxies", proxies)
 
 		// Get all the client keys and delete the ones that are not valid.
+		// TODO: use scan instead of keys.
 		for _, address := range p.RedisClient.Keys(context.Background(), "*:*").Val() {
-			if validateAddressPort(address) || validateHostPort(address) {
-				// If the connection is busy, it is not safe to delete the key.
-				if isBusy(proxies, address) {
-					p.Logger.Trace("Skipping connection because it is busy", "address", address)
+			valid := false
+
+			// Validate the address if the address is an IP address.
+			if ok, err := validateAddressPort(address); ok && err == nil {
+				valid = true
+			} else {
+				p.Logger.Trace(
+					"Skipping connection because it is invalid", "address", address, "error", err)
+			}
+
+			if !valid {
+				// Validate the address if the address is a hostname.
+				if ok, err := validateHostPort(address); ok && err == nil {
+					valid = true
+				} else {
+					p.Logger.Trace(
+						"Skipping connection because it is invalid", "address", address, "error", err)
 					continue
 				}
+			}
 
-				p.RedisClient.Del(context.Background(), address)
-				p.Logger.Trace("Deleted address", "address", address)
-				CacheDeletesCounter.Inc()
+			// If the address is not valid, skip it.
+			if !valid {
+				continue
 			}
+
+			// If the connection is busy (a client is connected), it is not safe to delete the key.
+			if isBusy(proxies, address) {
+				p.Logger.Trace("Skipping connection because it is busy", "address", address)
+				continue
+			}
+
+			p.RedisClient.Del(context.Background(), address)
+			p.Logger.Trace("Deleted stale address", "address", address)
+			CacheDeletesCounter.Inc()
 		}
 	}); err != nil {
 		p.Logger.Error("Failed to start periodic invalidator",

plugin/utils.go

@@ -152,52 +152,53 @@ func validateIP(ip net.IP) bool {
 }
 
 // validateAddressPort validates an address:port string.
-func validateAddressPort(addressPort string) bool {
+func validateAddressPort(addressPort string) (bool, error) {
 	// Split the address and port.
+	// TODO: Support IPv6.
 	data := strings.Split(strings.TrimSpace(addressPort), ":")
 	if len(data) != AddressPortPairLength {
-		return false
+		return false, ErrInvalidAddressPortPair
 	}
 
 	// Validate the port.
 	port, err := strconv.ParseUint(data[1], 10, 16)
 	if err != nil {
-		return false
+		return false, err
 	}
 
 	// Resolve the IP address, if it is a host.
 	ipAddress, err := net.ResolveIPAddr("ip", data[0])
 	if err != nil {
-		return false
+		return false, err
 	}
 
 	// Validate the IP address and port.
 	if (validateIP(net.ParseIP(data[0])) || validateIP(ipAddress.IP)) && (port > 0 && port <= 65535) {
-		return true
+		return true, nil
 	}
 
-	return false
+	return false, nil
 }
 
 // validateHostPort validates a host:port string.
 // TODO: Add support for IPv6.
-func validateHostPort(hostPort string) bool {
+func validateHostPort(hostPort string) (bool, error) {
 	data := strings.Split(hostPort, ":")
 	if len(data) != AddressPortPairLength {
-		return false
+		return false, ErrInvalidAddressPortPair
 	}
 
 	port, err := strconv.ParseUint(data[1], 10, 16)
 	if err != nil {
-		return false
+		return false, err
 	}
 
 	// FIXME: There is not much to validate on the host side.
 	if data[0] != "" && port > 0 && port <= 65535 {
-		return true
+		return true, nil
 	}
 
-	return false
+	return false, nil
 }
 
 // isBusy checks if a client address exists in cache by matching the address

plugin/utils_test.go

@@ -283,17 +283,51 @@ func Test_GetTablesFromQuery_AlterMultiple(t *testing.T) {
 }
 
 func Test_validateAddressPort_Hostname(t *testing.T) {
-	assert.True(t, validateAddressPort("localhost:5432"))
-	assert.True(t, validateAddressPort("	localhost:5432"))
-	assert.True(t, validateAddressPort("localhost:5432	"))
-	assert.True(t, validateAddressPort("	localhost:5432	"))
+	valid, err := validateAddressPort("localhost:5432")
+	assert.True(t, valid)
+	assert.Nil(t, err)
+
+	valid, err = validateAddressPort("	localhost:5432")
+	assert.True(t, valid)
+	assert.Nil(t, err)
+
+	valid, err = validateAddressPort("localhost:5432	")
+	assert.True(t, valid)
+	assert.Nil(t, err)
+
+	valid, err = validateAddressPort("	localhost:5432	")
+	assert.True(t, valid)
+	assert.Nil(t, err)
+}
+
+func Test_validateAddressPort_Fails(t *testing.T) {
+	valid, err := validateAddressPort("localhost")
+	assert.False(t, valid)
+	assert.NotNil(t, err)
+}
+
+func Test_validateHostPort_Fails(t *testing.T) {
+	valid, err := validateHostPort("127.0.0.1")
+	assert.False(t, valid)
+	assert.NotNil(t, err)
 }
 
 func Test_validateAddressPort_IPv4(t *testing.T) {
-	assert.True(t, validateAddressPort("127.0.0.1:5432"))
-	assert.True(t, validateAddressPort("    127.0.0.1:5432"))
-	assert.True(t, validateAddressPort("127.0.0.1:5432  "))
-	assert.True(t, validateAddressPort("    127.0.0.1:5432  "))
+	valid, err := validateAddressPort("127.0.0.1:5432")
+	assert.True(t, valid)
+	assert.Nil(t, err)
+
+	valid, err = validateAddressPort("    127.0.0.1:5432")
+	assert.True(t, valid)
+	assert.Nil(t, err)
+
+	valid, err = validateAddressPort("127.0.0.1:5432  ")
+	assert.True(t, valid)
+	assert.Nil(t, err)
+
+	valid, err = validateAddressPort("    127.0.0.1:5432  ")
+	assert.True(t, valid)
+	assert.Nil(t, err)
 }
 
 func Test_isBusy(t *testing.T) {