Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update service connect config validator #3424

Merged
merged 1 commit into from
Oct 10, 2022
Merged

Update service connect config validator #3424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 46 additions & 57 deletions agent/api/serviceconnect/service_connect_validator.go
View file
Open in desktop
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) fields 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
Open in desktop
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
Open in desktop
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