diff --git a/ecs-agent/netlib/model/networkinterface/networkinterface.go b/ecs-agent/netlib/model/networkinterface/networkinterface.go index 84e25eaccd3..99437a8857e 100644 --- a/ecs-agent/netlib/model/networkinterface/networkinterface.go +++ b/ecs-agent/netlib/model/networkinterface/networkinterface.go @@ -477,16 +477,26 @@ func InterfaceFromACS(acsENI *ecsacs.ElasticNetworkInterface) (*NetworkInterface ni.InterfaceAssociationProtocol = VLANInterfaceAssociationProtocol } - for _, nameserverIP := range acsENI.DomainNameServers { - ni.DomainNameServers = append(ni.DomainNameServers, aws.ToString(nameserverIP)) - } - for _, nameserverDomain := range acsENI.DomainName { - ni.DomainNameSearchList = append(ni.DomainNameSearchList, aws.ToString(nameserverDomain)) - } + ni.DomainNameServers = trimSpaceAll(acsENI.DomainNameServers) + ni.DomainNameSearchList = trimSpaceAll(acsENI.DomainName) return ni, nil } +// trimSpaceAll returns the given values with surrounding whitespace stripped from each. +// +// The DNS fields of an ENI originate from the VPC DHCP option set, which stores each value exactly +// as it was typed and performs no validation of its own. An option set configured as +// "domain-name-servers 10.0.0.2, 10.0.0.3" therefore yields a second value with a leading space, +// which is not a valid IP address by the time it reaches the task metadata response. +func trimSpaceAll(values []*string) []string { + var trimmed []string + for _, value := range values { + trimmed = append(trimmed, strings.TrimSpace(aws.ToString(value))) + } + return trimmed +} + // ValidateENI validates the NetworkInterface information sent from ACS. func ValidateENI(acsENI *ecsacs.ElasticNetworkInterface) error { var validateSubnetGatewayAddr = func(version, addr string) error { @@ -689,8 +699,8 @@ func v2nTunnelFromACS(acsENI *ecsacs.ElasticNetworkInterface) (*NetworkInterface }, }, - DomainNameServers: aws.ToStringSlice(acsENI.DomainNameServers), - DomainNameSearchList: aws.ToStringSlice(acsENI.DomainName), + DomainNameServers: trimSpaceAll(acsENI.DomainNameServers), + DomainNameSearchList: trimSpaceAll(acsENI.DomainName), TunnelProperties: &TunnelProperties{ ID: aws.ToString(acsTunnelProperties.TunnelId), DestinationIPAddress: aws.ToString(acsTunnelProperties.InterfaceIpAddress), @@ -726,8 +736,8 @@ func vethPairFromACS( // DNS related data for VETH interface will be copied from the peer interface's DNS data. // This is because if default traffic of the container needs to use the VETH interface, // domain name resolution will be based on the DNS config of the peer interface. - DomainNameServers: aws.ToStringSlice(peerInterface.DomainNameServers), - DomainNameSearchList: aws.ToStringSlice(peerInterface.DomainName), + DomainNameServers: trimSpaceAll(peerInterface.DomainNameServers), + DomainNameSearchList: trimSpaceAll(peerInterface.DomainName), VETHProperties: &VETHProperties{ PeerInterfaceName: aws.ToString(acsENI.InterfaceVethProperties.PeerInterface), }, diff --git a/ecs-agent/netlib/model/networkinterface/networkinterface_test.go b/ecs-agent/netlib/model/networkinterface/networkinterface_test.go index 867b28acc35..19f19df699d 100644 --- a/ecs-agent/netlib/model/networkinterface/networkinterface_test.go +++ b/ecs-agent/netlib/model/networkinterface/networkinterface_test.go @@ -213,3 +213,85 @@ func TestSetDeviceNameDefaultInterfaceNotFound(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "unable to find device name") } + +// getTestACSInterface returns a minimal ACS ENI that passes ValidateENI, for tests that only +// care about how InterfaceFromACS handles the DNS fields. +func getTestACSInterface() *ecsacs.ElasticNetworkInterface { + return &ecsacs.ElasticNetworkInterface{ + Ec2Id: aws.String("eni-1"), + MacAddress: aws.String(testconst.RandomMAC), + Ipv4Addresses: []*ecsacs.IPv4AddressAssignment{ + {Primary: aws.Bool(true), PrivateAddress: aws.String("1.2.3.4")}, + }, + SubnetGatewayIpv4Address: aws.String("1.2.3.1/20"), + } +} + +// TestInterfaceFromACSTrimsWhitespaceFromDomainNameServers tests that surrounding whitespace on a +// nameserver from ACS is stripped. The VPC DHCP option set stores nameservers as typed, so an +// option set configured as "domain-name-servers 10.0.0.2, 10.0.0.3" yields a padded second value. +func TestInterfaceFromACSTrimsWhitespaceFromDomainNameServers(t *testing.T) { + acsENI := getTestACSInterface() + acsENI.DomainNameServers = []*string{aws.String("10.0.0.2"), aws.String(" 10.0.0.3 ")} + + ni, err := InterfaceFromACS(acsENI) + + assert.NoError(t, err) + assert.Equal(t, []string{"10.0.0.2", "10.0.0.3"}, ni.DomainNameServers) +} + +// TestInterfaceFromACSTrimsWhitespaceFromDomainNameSearchList tests that surrounding whitespace on +// a search domain from ACS is stripped. Search domains come from the same DHCP option set as the +// nameservers and are stored just as literally. +func TestInterfaceFromACSTrimsWhitespaceFromDomainNameSearchList(t *testing.T) { + acsENI := getTestACSInterface() + acsENI.DomainName = []*string{aws.String(" us-west-2.compute.internal ")} + + ni, err := InterfaceFromACS(acsENI) + + assert.NoError(t, err) + assert.Equal(t, []string{"us-west-2.compute.internal"}, ni.DomainNameSearchList) +} + +// TestV2NTunnelFromACSTrimsWhitespaceFromDNSFields tests that a V2N tunnel interface gets the same +// whitespace trimming as a regular interface, since its DNS data comes from the same ACS payload. +func TestV2NTunnelFromACSTrimsWhitespaceFromDNSFields(t *testing.T) { + acsENI := &ecsacs.ElasticNetworkInterface{ + Ec2Id: aws.String("eni-1"), + DomainNameServers: []*string{aws.String(" 10.0.0.2")}, + DomainName: []*string{aws.String(" us-west-2.compute.internal")}, + InterfaceTunnelProperties: &ecsacs.NetworkInterfaceTunnelProperties{ + TunnelId: aws.String("42"), + InterfaceIpAddress: aws.String("10.1.2.3"), + }, + } + + ni, err := v2nTunnelFromACS(acsENI) + + assert.NoError(t, err) + assert.Equal(t, []string{"10.0.0.2"}, ni.DomainNameServers) + assert.Equal(t, []string{"us-west-2.compute.internal"}, ni.DomainNameSearchList) +} + +// TestVETHPairFromACSTrimsWhitespaceFromDNSFields tests that a VETH interface gets the same +// whitespace trimming as its peer, whose DNS data it copies from the same ACS payload. +func TestVETHPairFromACSTrimsWhitespaceFromDNSFields(t *testing.T) { + peer := &ecsacs.ElasticNetworkInterface{ + Ec2Id: aws.String("eni-1"), + Name: aws.String("peer"), + DomainNameServers: []*string{aws.String(" 10.0.0.2")}, + DomainName: []*string{aws.String(" us-west-2.compute.internal")}, + } + acsENI := &ecsacs.ElasticNetworkInterface{ + Ec2Id: aws.String("eni-2"), + InterfaceVethProperties: &ecsacs.NetworkInterfaceVethProperties{ + PeerInterface: aws.String("peer"), + }, + } + + ni, err := vethPairFromACS(acsENI, []*ecsacs.ElasticNetworkInterface{peer}) + + assert.NoError(t, err) + assert.Equal(t, []string{"10.0.0.2"}, ni.DomainNameServers) + assert.Equal(t, []string{"us-west-2.compute.internal"}, ni.DomainNameSearchList) +}