Skip to content

Commit

Permalink
EC2 task networking on Windows: gMSA support, adding additional local…
Browse files Browse the repository at this point in the history
… routes and IMDS route in the task namespace. (aws#2876)

* Add IMDS route to the task namespace if task is allowed to have IMDS access.c

* Add support for adding additional local routes in awsvpc network mode on Windows.

* Removed the workflow for getting Primary ipv4 address of the vpc.

This is not required as we can use the dns settings of the primary instance ENI.

* Added gSMA support while using awsvpc network mode.

We use the instance ENIs DNS settings since both the ENIs would be in the same VPC and would be additionally beneficial during domain join.

* PR review changes: Reverted netwrapper to platform agnostic package

* Changes to json format for vpc-eni plugin

* Bug fix to rectify the scenario when pause container sends nil network stats.

Initially, without any network, pause container returns nil network stats. When container stats are collected with pause namespace in place, lastStats.NetworkStats is nil which causes agent to crash and restart.

* Minor changes: Comment changes, logfile path consolidation and variable name change
  • Loading branch information
rawahars authored and Harsh Rawat committed Jun 23, 2021
1 parent 3fc6c96 commit 7c90d47
Show file tree
Hide file tree
Showing 25 changed files with 258 additions and 193 deletions.
19 changes: 13 additions & 6 deletions agent/app/agent_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,16 @@ import (
"sync"
"time"

fsxfactory "github.com/aws/amazon-ecs-agent/agent/fsx/factory"

asmfactory "github.com/aws/amazon-ecs-agent/agent/asm/factory"
"github.com/aws/amazon-ecs-agent/agent/credentials"
"github.com/aws/amazon-ecs-agent/agent/data"
"github.com/aws/amazon-ecs-agent/agent/ecs_client/model/ecs"
"github.com/aws/amazon-ecs-agent/agent/ecscni"
"github.com/aws/amazon-ecs-agent/agent/engine"
"github.com/aws/amazon-ecs-agent/agent/engine/dockerstate"
"github.com/aws/amazon-ecs-agent/agent/eni/networkutils"
"github.com/aws/amazon-ecs-agent/agent/eni/watcher"
fsxfactory "github.com/aws/amazon-ecs-agent/agent/fsx/factory"
s3factory "github.com/aws/amazon-ecs-agent/agent/s3/factory"
"github.com/aws/amazon-ecs-agent/agent/sighandlers"
"github.com/aws/amazon-ecs-agent/agent/sighandlers/exitcodes"
Expand Down Expand Up @@ -76,12 +76,18 @@ func (agent *ecsAgent) initializeTaskENIDependencies(state dockerstate.TaskEngin
return err, false
}

// Query the VPC's primary IPv4 CIDR using IMDS
primaryIPv4VPCCIDR, err := agent.ec2MetadataClient.PrimaryIPV4VPCCIDR(agent.mac)
// The instance ENI and the task ENI on ECS EC2 Windows will belong to the same VPC, and therefore,
// have the same DNS server list. Hence, we store the DNS server list of the instance ENI during
// agent startup and use the same during config creation for setting up task ENI.
// Another intrinsic benefit of this approach is that any DNS servers added for Active Directory
// will be added to the task ENI, allowing tasks in awsvpc network mode to support gMSA.
dnsServerList, err := agent.resourceFields.NetworkUtils.GetDNSServerAddressList(agent.mac)
if err != nil {
return fmt.Errorf("unable to get primary ipv4 cidr of the vpc: %v", err), false
// An error at this point is terminal as the tasks launched with awsvpc network mode
// require the DNS entries.
return fmt.Errorf("unable to get dns server addresses of instance eni: %v", err), true
}
agent.cfg.PrimaryIPv4VPCCIDR = primaryIPv4VPCCIDR
agent.cfg.InstanceENIDNSServerList = dnsServerList

return nil, false
}
Expand Down Expand Up @@ -305,6 +311,7 @@ func (agent *ecsAgent) initializeResourceFields(credentialsManager credentials.M
Ctx: agent.ctx,
DockerClient: agent.dockerClient,
S3ClientCreator: s3factory.NewS3ClientCreator(),
NetworkUtils: networkutils.New(),
}
}

Expand Down
7 changes: 3 additions & 4 deletions agent/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
package config

import (
"net"
"time"

"github.com/aws/amazon-ecs-agent/agent/dockerclient"
Expand Down Expand Up @@ -334,7 +333,7 @@ type Config struct {
// External specifies whether agent is running on external compute capacity (i.e. outside of aws).
External BooleanDefaultFalse

// PrimaryIPv4VPCCIDR stores the primary IPv4 CIDR of the VPC in which agent is running
// Currently, this field is only populated for Windows and is used during task networking setup
PrimaryIPv4VPCCIDR *net.IPNet
// InstanceENIDNSServerList stores the list of DNS servers for the primary instance ENI.
// Currently, this field is only populated for Windows and is used during task networking setup.
InstanceENIDNSServerList []string
}
5 changes: 0 additions & 5 deletions agent/ec2/blackhole_ec2_metadata_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ package ec2

import (
"errors"
"net"

"github.com/aws/aws-sdk-go/aws/ec2metadata"
)
Expand Down Expand Up @@ -85,7 +84,3 @@ func (blackholeMetadataClient) SpotInstanceAction() (string, error) {
func (blackholeMetadataClient) OutpostARN() (string, error) {
return "", errors.New("blackholed")
}

func (c blackholeMetadataClient) PrimaryIPV4VPCCIDR(mac string) (*net.IPNet, error) {
return nil, errors.New("blackholed")
}
17 changes: 0 additions & 17 deletions agent/ec2/ec2_metadata_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"encoding/json"
"errors"
"fmt"
"net"
"strings"
"time"

Expand Down Expand Up @@ -83,7 +82,6 @@ type EC2MetadataClient interface {
PublicIPv4Address() (string, error)
SpotInstanceAction() (string, error)
OutpostARN() (string, error)
PrimaryIPV4VPCCIDR(mac string) (*net.IPNet, error)
}

type ec2MetadataClientImpl struct {
Expand Down Expand Up @@ -205,18 +203,3 @@ func (c *ec2MetadataClientImpl) SpotInstanceAction() (string, error) {
func (c *ec2MetadataClientImpl) OutpostARN() (string, error) {
return c.client.GetMetadata(OutpostARN)
}

// PrimaryIPV4VPCCIDR returns the primary IPV4 block associated with the VPC
func (c *ec2MetadataClientImpl) PrimaryIPV4VPCCIDR(mac string) (*net.IPNet, error) {
vpcCIDR, err := c.client.GetMetadata(fmt.Sprintf(PrimaryIPV4VPCCIDRResourceFormat, mac))
if err != nil {
return nil, err
}

_, parsedCIDR, err := net.ParseCIDR(vpcCIDR)
if err != nil {
return nil, err
}

return parsedCIDR, nil
}
48 changes: 8 additions & 40 deletions agent/ec2/ec2_metadata_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,14 @@ import (
)

const (
testRoleName = "test-role"
mac = "01:23:45:67:89:ab"
macs = "01:23:45:67:89:ab/\n01:23:45:67:89:ac"
vpcID = "vpc-1234"
subnetID = "subnet-1234"
iidRegion = "us-east-1"
privateIP = "127.0.0.1"
publicIP = "127.0.0.1"
vpcPrimaryCIDR = "10.0.0.0/16"
incorrectVPCPrimaryCIDR = "10.0.0.0\n/16"
testRoleName = "test-role"
mac = "01:23:45:67:89:ab"
macs = "01:23:45:67:89:ab/\n01:23:45:67:89:ac"
vpcID = "vpc-1234"
subnetID = "subnet-1234"
iidRegion = "us-east-1"
privateIP = "127.0.0.1"
publicIP = "127.0.0.1"
)

func makeTestRoleCredentials() ec2.RoleCredentials {
Expand Down Expand Up @@ -254,33 +252,3 @@ func TestSpotInstanceActionError(t *testing.T) {
assert.Error(t, err)
assert.Equal(t, "", resp)
}

func TestPrimaryIPV4VPCCIDRSuccess(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

mockGetter := mock_ec2.NewMockHttpClient(ctrl)
testClient := ec2.NewEC2MetadataClient(mockGetter)

mockGetter.EXPECT().GetMetadata(fmt.Sprintf(ec2.PrimaryIPV4VPCCIDRResourceFormat, mac)).
Return(vpcPrimaryCIDR, nil)
resp, err := testClient.PrimaryIPV4VPCCIDR(mac)

assert.NoError(t, err)
assert.Equal(t, vpcPrimaryCIDR, resp.String())
}

func TestPrimaryIPV4VPCCIDRError(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

mockGetter := mock_ec2.NewMockHttpClient(ctrl)
testClient := ec2.NewEC2MetadataClient(mockGetter)

mockGetter.EXPECT().GetMetadata(fmt.Sprintf(ec2.PrimaryIPV4VPCCIDRResourceFormat, mac)).
Return(incorrectVPCPrimaryCIDR, nil)
resp, err := testClient.PrimaryIPV4VPCCIDR(mac)

assert.Error(t, err)
assert.Nil(t, resp)
}
16 changes: 0 additions & 16 deletions agent/ec2/mocks/ec2_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions agent/ecscni/mocks/namespace_helper_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion agent/ecscni/namespace_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ import (
type execCmdExecutorFnType func(commands []string, separator string) error

// NamespaceHelper defines the methods for performing additional actions to setup/clean the task namespace.
// Task namespace in awsvpc network mode is configured using pause container which is the first container
// launched for the task. These commands are executed inside that container.
type NamespaceHelper interface {
ConfigureTaskNamespaceRouting(ctx context.Context, config *Config, result *current.Result) error
ConfigureTaskNamespaceRouting(ctx context.Context, taskENI *apieni.ENI, config *Config, result *current.Result) error
ConfigureFirewallForTaskNSSetup(taskENI *apieni.ENI, config *Config) error
ConfigureFirewallForTaskNSCleanup(taskENI *apieni.ENI, config *Config) error
}
Expand Down
2 changes: 1 addition & 1 deletion agent/ecscni/namespace_helper_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var execCmdExecutorFn execCmdExecutorFnType = nil

// ConfigureTaskNamespaceRouting executes the commands required for setting up appropriate routing inside task namespace.
// This is applicable only for Windows.
func (nsHelper *helper) ConfigureTaskNamespaceRouting(ctx context.Context, config *Config, result *current.Result) error {
func (nsHelper *helper) ConfigureTaskNamespaceRouting(ctx context.Context, taskENI *apieni.ENI, config *Config, result *current.Result) error {
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion agent/ecscni/namespace_helper_unsupported.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var execCmdExecutorFn execCmdExecutorFnType = nil

// ConfigureTaskNamespaceRouting executes the commands required for setting up appropriate routing inside task namespace.
// This is applicable only for Windows.
func (nsHelper *helper) ConfigureTaskNamespaceRouting(ctx context.Context, config *Config, result *current.Result) error {
func (nsHelper *helper) ConfigureTaskNamespaceRouting(ctx context.Context, taskENI *apieni.ENI, config *Config, result *current.Result) error {
return nil
}

Expand Down
36 changes: 34 additions & 2 deletions agent/ecscni/namespace_helper_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,11 @@ const (
// credentialsEndpointRoute is the route of credentials endpoint for accessing task iam roles/task metadata.
credentialsEndpointRoute = "169.254.170.2/32"
// imdsEndpointIPAddress is the IP address of the endpoint for accessing IMDS.
imdsEndpointIPAddress = "169.254.169.254"
imdsEndpointIPAddress = "169.254.169.254/32"
// ecsBridgeEndpointNameFormat is the name format of the ecs-bridge endpoint in the task namespace.
ecsBridgeEndpointNameFormat = "%s-ep-%s"
// taskPrimaryEndpointNameFormat is the name format of the primary endpoint in the task namespace.
taskPrimaryEndpointNameFormat = "%s-br-%s-ep-%s"
// blockIMDSFirewallRuleNameFormat is the format of firewall rule name for blocking IMDS from task namespace.
blockIMDSFirewallRuleNameFormat = "Disable IMDS for %s"
// ecsBridgeRouteAddCmdFormat is the format of command for adding route entry through ECS Bridge.
Expand All @@ -59,7 +61,13 @@ const (
var execCmdExecutorFn execCmdExecutorFnType = execCmdExecutor

// ConfigureTaskNamespaceRouting executes the commands required for setting up appropriate routing inside task namespace.
func (nsHelper *helper) ConfigureTaskNamespaceRouting(ctx context.Context, config *Config, result *current.Result) error {
// The commands currently executed are-
// netsh interface ipv4 delete route prefix=0.0.0.0/0 interface="vEthernet (nat-ep-<container_id>)
// netsh interface ipv4 delete route prefix=<ecs-bridge-subnet-cidr> interface="vEthernet (nat-ep-<container_id>)
// netsh interface ipv4 add route prefix=169.254.170.2/32 interface="vEthernet (nat-ep-<container_id>)
// netsh interface ipv4 add route prefix=169.254.169.254/32 interface="vEthernet (task-br-<mac>-ep-<container_id>)
// netsh interface ipv4 add route prefix=<local-route> interface="vEthernet (nat-ep-<container_id>)
func (nsHelper *helper) ConfigureTaskNamespaceRouting(ctx context.Context, taskENI *apieni.ENI, config *Config, result *current.Result) error {
// Obtain the ecs-bridge endpoint's subnet IP address from the CNI plugin execution result.
ecsBridgeSubnetIPAddress := &net.IPNet{
IP: result.IPs[0].Address.IP.Mask(result.IPs[0].Address.Mask),
Expand All @@ -74,6 +82,26 @@ func (nsHelper *helper) ConfigureTaskNamespaceRouting(ctx context.Context, confi
credentialsAddressRouteAdditionCmd := fmt.Sprintf(ecsBridgeRouteAddCmdFormat, credentialsEndpointRoute, ecsBridgeEndpointName)
commands := []string{defaultRouteDeletionCmd, defaultSubnetRouteDeletionCmd, credentialsAddressRouteAdditionCmd}

if !config.BlockInstanceMetadata {
// This naming convention is drawn from the way CNI plugin names the endpoints.
// https://github.com/aws/amazon-vpc-cni-plugins/blob/master/plugins/vpc-eni/network/network_windows.go
taskPrimaryEndpointId := strings.Replace(strings.ToLower(taskENI.MacAddress), ":", "", -1)
taskPrimaryEndpointName := fmt.Sprintf(taskPrimaryEndpointNameFormat, TaskHNSNetworkNamePrefix,
taskPrimaryEndpointId, config.ContainerID)
imdsRouteAdditionCmd := fmt.Sprintf(ecsBridgeRouteAddCmdFormat, imdsEndpointIPAddress, taskPrimaryEndpointName)
commands = append(commands, imdsRouteAdditionCmd)
}

// Add any additional route which needs to be routed via ecs-bridge.
for _, route := range config.AdditionalLocalRoutes {
ipRoute := &net.IPNet{
IP: route.IP,
Mask: route.Mask,
}
additionalRouteAdditionCmd := fmt.Sprintf(ecsBridgeRouteAddCmdFormat, ipRoute.String(), ecsBridgeEndpointName)
commands = append(commands, additionalRouteAdditionCmd)
}

// Invoke the generated commands inside the task namespace.
err := nsHelper.invokeCommandsInsideContainer(ctx, config.ContainerID, commands, " && ")
if err != nil {
Expand All @@ -83,6 +111,8 @@ func (nsHelper *helper) ConfigureTaskNamespaceRouting(ctx context.Context, confi
}

// ConfigureFirewallForTaskNSSetup executes the commands, if required, to setup firewall rules for disabling IMDS access from task.
// The commands executed are-
// netsh advfirewall firewall add rule name="Disable IMDS for <ENI IP>" dir=out localip=<ENI IP> remoteip=169.254.170.2/32 action=block
func (nsHelper *helper) ConfigureFirewallForTaskNSSetup(taskENI *apieni.ENI, config *Config) error {
if config.BlockInstanceMetadata {
if taskENI == nil {
Expand All @@ -107,6 +137,8 @@ func (nsHelper *helper) ConfigureFirewallForTaskNSSetup(taskENI *apieni.ENI, con
}

// ConfigureFirewallForTaskNSCleanup executes the commands, if required, to cleanup the firewall rules created during setup.
// The commands executed are-
// netsh advfirewall firewall delete rule name="Disable IMDS for <ENI IP>" dir=out
func (nsHelper *helper) ConfigureFirewallForTaskNSCleanup(taskENI *apieni.ENI, config *Config) error {
if config.BlockInstanceMetadata {
if taskENI == nil {
Expand Down
17 changes: 15 additions & 2 deletions agent/ecscni/namespace_helper_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
"testing"
"time"

cnitypes "github.com/containernetworking/cni/pkg/types"

"github.com/docker/docker/api/types"
"github.com/stretchr/testify/assert"

Expand Down Expand Up @@ -55,12 +57,23 @@ func TestConfigureTaskNamespaceRouting(t *testing.T) {

dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
cniConig := getCNIConfig()
taskENI := getTaskENI()

cniConig.AdditionalLocalRoutes = append(cniConig.AdditionalLocalRoutes, cnitypes.IPNet{
net.ParseIP("10.0.0.0"),
net.CIDRMask(24, 32),
})

bridgeEpName := fmt.Sprintf(ecsBridgeEndpointNameFormat, ECSBridgeNetworkName, containerID)
taskEpId := strings.Replace(strings.ToLower(taskENI.MacAddress), ":", "", -1)
taskEPName := fmt.Sprintf(taskPrimaryEndpointNameFormat, TaskHNSNetworkNamePrefix, taskEpId, containerID)

cmd1 := fmt.Sprintf(ecsBridgeRouteDeleteCmdFormat, windowsDefaultRoute, bridgeEpName)
cmd2 := fmt.Sprintf(ecsBridgeRouteDeleteCmdFormat, "10.0.0.0/24", bridgeEpName)
cmd3 := fmt.Sprintf(ecsBridgeRouteAddCmdFormat, credentialsEndpointRoute, bridgeEpName)
finalCmd := strings.Join([]string{cmd1, cmd2, cmd3}, " && ")
cmd4 := fmt.Sprintf(ecsBridgeRouteAddCmdFormat, imdsEndpointIPAddress, taskEPName)
cmd5 := fmt.Sprintf(ecsBridgeRouteAddCmdFormat, "10.0.0.0/24", bridgeEpName)
finalCmd := strings.Join([]string{cmd1, cmd2, cmd3, cmd4, cmd5}, " && ")

gomock.InOrder(
dockerClient.EXPECT().CreateContainerExec(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Do(
Expand All @@ -83,7 +96,7 @@ func TestConfigureTaskNamespaceRouting(t *testing.T) {
)

nsHelper := NewNamespaceHelper(dockerClient)
err := nsHelper.ConfigureTaskNamespaceRouting(ctx, cniConig, getECSBridgeResult())
err := nsHelper.ConfigureTaskNamespaceRouting(ctx, taskENI, cniConig, getECSBridgeResult())
assert.NoError(t, err)
}

Expand Down
Loading

0 comments on commit 7c90d47

Please sign in to comment.