Skip to content

Commit

Permalink
Update service connect config validator
Browse files Browse the repository at this point in the history
  • Loading branch information
@chienhanlin
chienhanlin authored and EC2 Default User committed Oct 10, 2022
1 parent 67240c1 commit 4e8e401
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 99 deletions.
103 changes: 46 additions & 57 deletions agent/api/serviceconnect/service_connect_validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,40 +24,47 @@ import (
)

const (
BridgeNetworkMode = "bridge"
AWSVPCNetworkMode = "awsvpc"
invalidEgressConfigFormat = `no service connect %s in the egress config. %s`
portCollisionFormat = `%s port collision detected in the ingress config with the %s port=%d, and listener name=%s`
invalidIngressPortFormat = `the %s port=%d in the ingress config is not valid: %w`
warningIngressPortFormat = `Service connect config validation: %s port should not exist for %s mode in the ingress config`
invalidDnsEntryFormat = `no %s in the DNS config hostname=%s, address=%s`
BridgeNetworkMode = "bridge"
AWSVPCNetworkMode = "awsvpc"
missingContainerInTaskFormat = `service connect container name=%s does not exist in the task`
duplicateContainerInTaskFormat = `found %d duplicate service connect container name=%s in the task`
invalidCidrFormat = `CIDR=%s is not a valid %s CIDR`
portCollisionFormat = `%s port collision detected in the ingress config with %s port=%d, and listener name=%s`
invalidDnsAddressFormat = `hostname=%s, address=%s in the DNS config is not valid: %w`
noScSupportNetworkModeFormat = `service connect does not support for %s newtork mode`
missingListenerInIngressFormat = `missing listener name in the ingress config with intercept port=%d`
invalidPortRangeFormat = `port=%d is not a valid port. A valid port ranges from 1 through 65535`
invalidIpAddressFormat = `address=%s is not a valid IP address`
invalidIngressPortFormat = `%s port=%d in the ingress config is not valid: %w`
warningIngressPortFormat = `Service connect config: %s port should not exist in the ingress config for %s network mode`
missingDnsEntryFormat = `missing %s in the DNS config with hostname=%s, and address=%s`
)

// validateContainerName validates the service connect container name.
// validateContainerName validates the service connect container name exists in the task and no duplication.
func validateContainerName(scContainerName string, taskContainers []*ecsacs.Container) error {
// service connect container name is required
if scContainerName == "" {
return fmt.Errorf("missing service connect container name")
}

// validate the specified service connect container name exists in the task definition
numOfFoundSCContainer := 0
// validate the specified service connect container name exists in the task
numOfFoundScContainer := 0
for _, container := range taskContainers {
if aws.StringValue(container.Name) == scContainerName {
numOfFoundSCContainer += 1
numOfFoundScContainer += 1
}
}

if numOfFoundSCContainer == 0 {
return fmt.Errorf("service connect container name=%s does not exist in the task", scContainerName)
} else if numOfFoundSCContainer > 1 {
return fmt.Errorf("found %d duplicate service connect container name=%s exist in the task", numOfFoundSCContainer, scContainerName)
if numOfFoundScContainer == 0 {
return fmt.Errorf(missingContainerInTaskFormat, scContainerName)
} else if numOfFoundScContainer > 1 {
return fmt.Errorf(duplicateContainerInTaskFormat, numOfFoundScContainer, scContainerName)
}

return nil
}

// validateEgressConfig validates the service connect egress config.
// validateEgressConfig validates the listener name, IPv4 CIDR format and IPv6 CIDR format in the service connect egress config.
func validateEgressConfig(scEgressConfig *EgressConfig, ipv6Enabled bool) error {
// egress config can be empty for the first service since there are no other tasks that it can talk to
if scEgressConfig == nil {
Expand All @@ -66,32 +73,21 @@ func validateEgressConfig(scEgressConfig *EgressConfig, ipv6Enabled bool) error

// ListenerName is required if the egress config exists
if scEgressConfig.ListenerName == "" {
return fmt.Errorf(invalidEgressConfigFormat, "listener name", "")
}

// VIP is required if the egress config exists
// IPV4CIDR should be always required because an IPv6-only mode is not supoorted at this moment
if scEgressConfig.VIP.IPV4CIDR == "" {
return fmt.Errorf(invalidEgressConfigFormat, "VIP IPv4CIDR", "")
}

// IPV6CIDR is required when IPv6 is enabled
if ipv6Enabled && scEgressConfig.VIP.IPV6CIDR == "" {
return fmt.Errorf(invalidEgressConfigFormat, "VIP IPv6CIDR", "It must not be empty when the task is IPv6 enabled")
return fmt.Errorf("missing listener name in the egress config")
}

// validate IPV4CIDR if it exists
if scEgressConfig.VIP.IPV4CIDR != "" {
trimmedIpv4cidr := strings.TrimSpace(scEgressConfig.VIP.IPV4CIDR)
if err := validateCIDR(trimmedIpv4cidr, "IPv4"); err != nil {
trimmedIpv4Cidr := strings.TrimSpace(scEgressConfig.VIP.IPV4CIDR)
if err := validateCIDR(trimmedIpv4Cidr, "IPv4"); err != nil {
return err
}
}

// validate IPV6CIDR if it exists
if scEgressConfig.VIP.IPV6CIDR != "" {
trimmedIpv6cidr := strings.TrimSpace(scEgressConfig.VIP.IPV6CIDR)
if err := validateCIDR(trimmedIpv6cidr, "IPv6"); err != nil {
trimmedIpv6Cidr := strings.TrimSpace(scEgressConfig.VIP.IPV6CIDR)
if err := validateCIDR(trimmedIpv6Cidr, "IPv6"); err != nil {
return err
}
}
Expand All @@ -108,7 +104,7 @@ func validateCIDR(cidr, protocol string) error {
}
}

return fmt.Errorf("cidr=%s is not a valid %s CIDR", cidr, protocol)
return fmt.Errorf(invalidCidrFormat, cidr, protocol)
}

// getProtocol returns validity of the given IP based on the target protocol.
Expand All @@ -128,27 +124,22 @@ func getProtocol(ip net.IP, protocol string) bool {
return false
}

// validateDnsConfig validates the service connnect DNS config.
func validateDnsConfig(scDnsConfligList []DNSConfigEntry, scEgressConfig *EgressConfig, ipv6Enabled bool) error {
// DNS config associates to egress config
if len(scDnsConfligList) == 0 && scEgressConfig != nil {
return fmt.Errorf("no service connect DNS config. The DNS config is required when the egress config exists")
}

// validateDnsConfig validates hostnames and addresses in the service connnect DNS config.
func validateDnsConfig(scDnsConfligList []DNSConfigEntry) error {
for _, dnsEntry := range scDnsConfligList {
// HostName is required
if dnsEntry.HostName == "" {
return fmt.Errorf(invalidDnsEntryFormat, "hostname", dnsEntry.HostName, dnsEntry.Address)
return fmt.Errorf(missingDnsEntryFormat, "hostname", dnsEntry.HostName, dnsEntry.Address)
}

// Address is required
if dnsEntry.Address == "" {
return fmt.Errorf(invalidDnsEntryFormat, "address", dnsEntry.HostName, dnsEntry.Address)
return fmt.Errorf(missingDnsEntryFormat, "address", dnsEntry.HostName, dnsEntry.Address)
}

// validate the address is a valid IPv4/IPv6 address
if err := validateAddress(dnsEntry.Address); err != nil {
return fmt.Errorf("invalid address in the DNS config hostname=%s, address=%s: %w", dnsEntry.HostName, dnsEntry.Address, err)
return fmt.Errorf(invalidDnsAddressFormat, dnsEntry.HostName, dnsEntry.Address, err)
}
}

Expand All @@ -158,7 +149,7 @@ func validateDnsConfig(scDnsConfligList []DNSConfigEntry, scEgressConfig *Egress
// validateAddress validates the passed address is a valid IPv4/IPv6 address.
func validateAddress(address string) error {
if ip := net.ParseIP(address); ip == nil {
return fmt.Errorf("address=%s is not a valid IP address", address)
return fmt.Errorf(invalidIpAddressFormat, address)
}
return nil
}
Expand All @@ -176,7 +167,7 @@ func validateIngressConfig(scIngressConfigList []IngressConfigEntry, taskNetwork
return err
}
default:
return fmt.Errorf("service connect does not support for %s newtork mode", taskNetworkMode)
return fmt.Errorf(noScSupportNetworkModeFormat, taskNetworkMode)
}

return nil
Expand Down Expand Up @@ -215,7 +206,7 @@ func validateIngressConfigEntry(scIngressConfigList []IngressConfigEntry, networ
if err := validateInterceptPort(interceptPortValue, entry.ListenerName, interceptAndListenerPortsMap); err != nil {
return err
}
// Save the listener port value
// save the listener port value
interceptAndListenerPortsMap[interceptPortValue] = true
}

Expand All @@ -225,7 +216,7 @@ func validateIngressConfigEntry(scIngressConfigList []IngressConfigEntry, networ
if err := validateListenerPort(listenerPortValue, entry.ListenerName, interceptAndListenerPortsMap); err != nil {
return err
}
// Save the listener port value
// save the listener port value
interceptAndListenerPortsMap[listenerPortValue] = true
}

Expand All @@ -235,7 +226,7 @@ func validateIngressConfigEntry(scIngressConfigList []IngressConfigEntry, networ
if err := validateHostPort(hostPortValue, entry.ListenerName, hostPortsMap); err != nil {
return err
}
// Save the host port value
// save the host port value
hostPortsMap[hostPortValue] = true
}
}
Expand All @@ -250,7 +241,7 @@ func validateInterceptPort(interceptPortValue uint16, listenerName string, inter
}

if listenerName == "" {
return fmt.Errorf("no listener name in the ingress config with the intercept port=%d", interceptPortValue)
return fmt.Errorf(missingListenerInIngressFormat, interceptPortValue)
}

if present := interceptAndListenerPortsMap[interceptPortValue]; present {
Expand Down Expand Up @@ -293,10 +284,13 @@ func validatePort(port uint16) error {
return nil
}

return fmt.Errorf("the port=%d is an invalid port. A valid port ranges from 1 through 65535", port)
return fmt.Errorf(invalidPortRangeFormat, port)
}

// ValidateSCConfig validates service connect container name, config, egress config, and ingress config.
// ValidateServiceConnectConfig validates service connect container name,
// fields in egress config, dns config and ingress config when
// 1) fields consumed and proceeded by ECS Agent
// 2) fileds with a global standard, e.g. CIDR format
func ValidateServiceConnectConfig(scConfig *Config,
taskContainers []*ecsacs.Container,
taskNetworkMode string,
Expand All @@ -305,16 +299,11 @@ func ValidateServiceConnectConfig(scConfig *Config,
return err
}

// egress config and ingress config should not both be nil/empty
if scConfig.EgressConfig == nil && len(scConfig.IngressConfig) == 0 {
return fmt.Errorf("egress config and ingress config should not both be nil/empty")
}

if err := validateEgressConfig(scConfig.EgressConfig, ipv6Enabled); err != nil {
return err
}

if err := validateDnsConfig(scConfig.DNSConfig, scConfig.EgressConfig, ipv6Enabled); err != nil {
if err := validateDnsConfig(scConfig.DNSConfig); err != nil {
return err
}

Expand Down
42 changes: 1 addition & 41 deletions agent/api/serviceconnect/service_connect_validator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,25 +253,16 @@ func TestValidateServiceConnectConfigWithEmptyConfig(t *testing.T) {
testName string
testEgressConfigIsEmpty bool
testIngressConfigIsEmpty bool
shouldError bool
}{
{
testName: "AWSVPC default case with the empty egress config and dns config",
testEgressConfigIsEmpty: true,
testIngressConfigIsEmpty: false,
shouldError: false,
},
{
testName: "AWSVPC default case with the empty ingress config",
testEgressConfigIsEmpty: false,
testIngressConfigIsEmpty: true,
shouldError: false,
},
{
testName: "AWSVPC default case with the empty ingress config and engress config",
testEgressConfigIsEmpty: true,
testIngressConfigIsEmpty: true,
shouldError: true,
},
}

Expand All @@ -298,11 +289,7 @@ func TestValidateServiceConnectConfigWithEmptyConfig(t *testing.T) {
testIngressConfig,
)
err := ValidateServiceConnectConfig(testServiceConnectConfig, testTaskContainers, AWSVPCNetworkMode, false)
if tc.shouldError {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
assert.NoError(t, err)
})
}
}
Expand Down Expand Up @@ -344,24 +331,6 @@ func TestValidateServiceConnectConfigWithError(t *testing.T) {
testDnsConfigEntry: getTestDnsConfigEntry(testHostName, testIPv4Address),
testIngressConfigEntry: getTestIngressConfigEntry(AWSVPCNetworkMode, testInboundListenerName, false, uint16(0), testAwsVpcDefaultInterceptPort, aws.Uint16(0)),
},
{
testName: "AWSVPC default case with no IPv4 CIDR in the egress config when Ipv6 is not enabled",
testNetworkMode: AWSVPCNetworkMode,
testIsIPv6Enabled: false,
testContainerName: testServiceConnectContainerName,
testEgressConfig: getTestEgressConfig(testOutboundListenerName, "", ""),
testDnsConfigEntry: getTestDnsConfigEntry(testHostName, testIPv4Address),
testIngressConfigEntry: getTestIngressConfigEntry(AWSVPCNetworkMode, testInboundListenerName, false, uint16(0), testAwsVpcDefaultInterceptPort, aws.Uint16(0)),
},
{
testName: "AWSVPC default case with no IPv6 CIDR in the egress config when IPv6 is enabled",
testNetworkMode: AWSVPCNetworkMode,
testIsIPv6Enabled: true,
testContainerName: testServiceConnectContainerName,
testEgressConfig: getTestEgressConfig(testOutboundListenerName, testIPv4Cidr, ""),
testDnsConfigEntry: getTestDnsConfigEntry(testHostName, testIPv6Address),
testIngressConfigEntry: getTestIngressConfigEntry(AWSVPCNetworkMode, testInboundListenerName, false, uint16(0), testAwsVpcDefaultInterceptPort, aws.Uint16(0)),
},
{
testName: "AWSVPC override case with the invalid IPv4 CIDR in the egress config",
testNetworkMode: AWSVPCNetworkMode,
Expand All @@ -380,15 +349,6 @@ func TestValidateServiceConnectConfigWithError(t *testing.T) {
testDnsConfigEntry: getTestDnsConfigEntry(testHostName, testIPv6Address),
testIngressConfigEntry: getTestIngressConfigEntry(AWSVPCNetworkMode, "", true, testListenerPort, aws.Uint16(0), aws.Uint16(0)),
},
{
testName: "Bridge default case with the egress config but no dns config",
testNetworkMode: BridgeNetworkMode,
testIsIPv6Enabled: false,
testContainerName: testServiceConnectContainerName,
testEgressConfig: getTestEgressConfig(testOutboundListenerName, "", testIPv6Cidr),
testDnsConfigEntry: DNSConfigEntry{},
testIngressConfigEntry: getTestIngressConfigEntry(BridgeNetworkMode, "", false, testBridgeDefaultListenerPort, aws.Uint16(0), aws.Uint16(0)),
},
{
testName: "Bridge override case with no the invalid address in dns config when IPv6 is enabled",
testNetworkMode: BridgeNetworkMode,
Expand Down
2 changes: 1 addition & 1 deletion agent/api/task/task_attachment_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func TestHandleTaskAttachmentsWithServiceConnectAttachment(t *testing.T) {
testName: "AWSVPC IPv6 enabled with error",
testServiceConnectConfig: constructTestServiceConnectConfig(
testIngressPort,
testInboundListener,
"",
testOutboundListener,
"",
testIPv6CIDR,
Expand Down

0 comments on commit 4e8e401

Please sign in to comment.