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

Limit /etc/pki directory appnet agent bind mount to iso regions #4448

Merged
merged 1 commit into from
Dec 6, 2024
Merged

Limit /etc/pki directory appnet agent bind mount to iso regions #4448

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
5 changes: 5 additions & 0 deletions agent/engine/ordering_integ_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,11 @@ func TestDependencyComplete(t *testing.T) {
// Container 'parent' depends on container 'dependency' to START. We ensure that the 'parent' container starts only
// after the 'dependency' container has started.
func TestDependencyStart(t *testing.T) {
// Skip these tests on WS 2016 until the failures are root-caused.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to this change, but the rest of this tests here are already skipped on WS 2016, adding this one because it is failing intermittently.

isWindows2016, err := config.IsWindows2016()
if err == nil && isWindows2016 == true {
t.Skip()
}
taskEngine, done, _, _ := setupWithDefaultConfig(t)
defer done()

Expand Down
38 changes: 37 additions & 1 deletion agent/engine/serviceconnect/manager_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"github.com/pborman/uuid"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/aws/endpoints"

apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container"
apiserviceconnect "github.com/aws/amazon-ecs-agent/agent/api/serviceconnect"
Expand Down Expand Up @@ -200,6 +202,37 @@ func defaultMkdirAllAndChown(path string, perm fs.FileMode, uid, gid int) error
return nil
}

func getRegionFromContainerInstanceARN(containerInstanceARN string) string {
// Parse the ARN
parsedARN, err := arn.Parse(containerInstanceARN)
if err != nil {
return ""
}

// Extract the region from the parsed ARN
return parsedARN.Region
}

func isIsoRegion(region string) bool {
partition, ok := endpoints.PartitionForRegion(endpoints.DefaultPartitions(), region)
if !ok {
danehlim marked this conversation as resolved.
Show resolved Hide resolved
// if partition is not found, assume it's iso
danehlim marked this conversation as resolved.
Show resolved Hide resolved
return true
}
switch partition.ID() {
case endpoints.AwsPartitionID:
return false
case endpoints.AwsUsGovPartitionID:
return false
case endpoints.AwsCnPartitionID:
return false
default:
// region partition is not 'aws', 'aws-us-gov', nor 'aws-cn', so assume it's
// an iso region.
return true
}
}

func (m *manager) initAgentDirectoryMounts(taskId string, container *apicontainer.Container, hostConfig *dockercontainer.HostConfig) (string, error) {
statusPathHost := filepath.Join(m.statusPathHostRoot, taskId)

Expand All @@ -213,7 +246,10 @@ func (m *manager) initAgentDirectoryMounts(taskId string, container *apicontaine

hostConfig.Binds = append(hostConfig.Binds, getBindMountMapping(statusPathHost, m.statusPathContainer))
hostConfig.Binds = append(hostConfig.Binds, getBindMountMapping(m.relayPathHost, m.relayPathContainer))
hostConfig.Binds = append(hostConfig.Binds, getBindMountMapping(hostPKIDirPath, hostPKIDirPath))
region := getRegionFromContainerInstanceARN(m.containerInstanceARN)
if isIsoRegion(region) {
hostConfig.Binds = append(hostConfig.Binds, getBindMountMapping(hostPKIDirPath, hostPKIDirPath))
}

// create logging directory and bind mount, if customer has not configured a logging driver
if container.GetLogDriver() == "" {
Expand Down
127 changes: 127 additions & 0 deletions agent/engine/serviceconnect/manager_linux_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,3 +182,130 @@ func TestGetSupportedAppnetInterfaceVerToCapabilities(t *testing.T) {
})
}
}

func TestGetRegionFromContainerInstanceARN(t *testing.T) {
tests := []struct {
name string
containerInstanceARN string
expectedRegion string
}{
{
name: "Valid ARN - US West 2",
containerInstanceARN: "arn:aws:ecs:us-west-2:123456789012:container-instance/12345678-1234-1234-1234-123456789012",
expectedRegion: "us-west-2",
},
{
name: "Valid ARN - EU Central 1",
containerInstanceARN: "arn:aws:ecs:eu-central-1:123456789012:container-instance/87654321-4321-4321-4321-210987654321",
expectedRegion: "eu-central-1",
},
{
name: "Valid ARN - AP Southeast 1",
containerInstanceARN: "arn:aws:ecs:ap-southeast-1:123456789012:container-instance/11223344-5566-7788-9900-112233445566",
expectedRegion: "ap-southeast-1",
},
{
name: "Valid ARN - US Gov West 1",
containerInstanceARN: "arn:aws-us-gov:ecs:us-gov-west-1:123456789012:container-instance/98765432-1234-5678-9012-123456789012",
expectedRegion: "us-gov-west-1",
},
{
name: "Valid ARN - CN North 1",
containerInstanceARN: "arn:aws-cn:ecs:cn-north-1:123456789012:container-instance/11223344-5566-7788-9900-112233445566",
expectedRegion: "cn-north-1",
},
{
name: "Invalid ARN - Missing Region",
containerInstanceARN: "arn:aws:ecs::123456789012:container-instance/12345678-1234-1234-1234-123456789012",
expectedRegion: "",
},
{
name: "Invalid ARN - Wrong Service",
containerInstanceARN: "arn:aws:ec2:us-west-2:123456789012:instance/i-1234567890abcdef0",
expectedRegion: "us-west-2",
},
{
name: "Invalid ARN Format",
containerInstanceARN: "invalid:arn:format",
expectedRegion: "",
},
{
name: "Empty ARN",
containerInstanceARN: "",
expectedRegion: "",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := getRegionFromContainerInstanceARN(tt.containerInstanceARN)
assert.Equal(t, tt.expectedRegion, result, "Unexpected region for ARN: %s", tt.containerInstanceARN)
})
}
}

func TestIsIsoRegion(t *testing.T) {
tests := []struct {
name string
region string
expectedResult bool
}{
{
name: "AWS Standard Region - US West 2",
region: "us-west-2",
expectedResult: false,
},
{
name: "AWS Standard Region - EU Central 1",
region: "eu-central-1",
expectedResult: false,
},
{
name: "AWS GovCloud Region - US Gov West 1",
region: "us-gov-west-1",
expectedResult: false,
},
{
name: "AWS GovCloud Region - US Gov East 1",
region: "us-gov-east-1",
expectedResult: false,
},
{
name: "AWS China Region - CN North 1",
region: "cn-north-1",
expectedResult: false,
},
{
name: "AWS China Region - CN Northwest 1",
region: "cn-northwest-1",
expectedResult: false,
},
{
name: "ISO Region - US ISO East 1",
region: "us-iso-east-1",
expectedResult: true,
},
{
name: "ISO Region - US ISOB East 1",
region: "us-isob-east-1",
expectedResult: true,
},
{
name: "Unknown Region",
region: "unknown-region",
expectedResult: true,
},
{
name: "Empty Region",
region: "",
expectedResult: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := isIsoRegion(tt.region)
assert.Equal(t, tt.expectedResult, result, "Unexpected result for region: %s", tt.region)
})
}
}
88 changes: 81 additions & 7 deletions agent/engine/serviceconnect/manager_linux_test_common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,17 @@ func getAWSVPCTask(t *testing.T) (*apitask.Task, *apicontainer.Container, *apico
return sleepTask, pauseContainer, serviceConnectContainer
}

func copyMap(input map[string]string, addk string, addv string) map[string]string {
output := make(map[string]string)
if len(addk) > 0 && len(addv) > 0 {
output[addk] = addv
}
for k, v := range input {
output[k] = v
}
return output
}

func testAgentContainerModificationsForServiceConnect(t *testing.T, privilegedMode bool) {
backupMkdirAllAndChown := mkdirAllAndChown
tempDir := t.TempDir()
Expand All @@ -129,14 +140,12 @@ func testAgentContainerModificationsForServiceConnect(t *testing.T, privilegedMo
fmt.Sprintf("%s/status/%s:%s", tempDir, scTask.GetID(), "/some/other/run"),
fmt.Sprintf("%s/relay:%s", tempDir, "/not/var/run"),
fmt.Sprintf("%s/log/%s:%s", tempDir, scTask.GetID(), "/some/other/log"),
"/etc/pki:/etc/pki",
}
expectedENVs := map[string]string{
"ReLaYgOeShErE": "unix:///not/var/run/relay_file_of_holiness",
"StAtUsGoEsHeRe": "/some/other/run/status_file_of_holiness",
"APPNET_AGENT_ADMIN_MODE": "uds",
"ENVOY_ENABLE_IAM_AUTH_FOR_XDS": "0",
"ECS_CONTAINER_INSTANCE_ARN": "fake_container_instance",
"APPNET_ENVOY_LOG_DESTINATION": "/some/other/log",
}

Expand All @@ -147,15 +156,80 @@ func testAgentContainerModificationsForServiceConnect(t *testing.T, privilegedMo
expectedBinds []string
expectedBindDirPerm string
expectedBindDirOwner uint32
containerInstanceARN string
}
testcases := []testCase{
{
name: "Service connect container has extra binds/ENV",
name: "Service connect container has extra binds/ENV. Commercial region has no /etc/pki mount.",
container: serviceConnectContainer,
expectedENV: expectedENVs,
expectedENV: copyMap(expectedENVs, "ECS_CONTAINER_INSTANCE_ARN", "arn:aws:ecs:us-west-2:123456789012:container-instance/12345678-test-test-test-123456789012"),
expectedBinds: expectedBinds,
expectedBindDirPerm: fs.FileMode(0700).String(),
expectedBindDirOwner: serviceconnect.AppNetUID,
containerInstanceARN: "arn:aws:ecs:us-west-2:123456789012:container-instance/12345678-test-test-test-123456789012",
},
{
name: "Service connect container has extra binds/ENV. US gov region has no /etc/pki mount.",
container: serviceConnectContainer,
expectedENV: copyMap(expectedENVs, "ECS_CONTAINER_INSTANCE_ARN", "arn:aws:ecs:us-gov-west-1:123456789012:container-instance/12345678-test-test-test-123456789012"),
expectedBinds: expectedBinds,
expectedBindDirPerm: fs.FileMode(0700).String(),
expectedBindDirOwner: serviceconnect.AppNetUID,
containerInstanceARN: "arn:aws:ecs:us-gov-west-1:123456789012:container-instance/12345678-test-test-test-123456789012",
},
{
name: "Service connect container has extra binds/ENV. China region has no /etc/pki mount.",
container: serviceConnectContainer,
expectedENV: copyMap(expectedENVs, "ECS_CONTAINER_INSTANCE_ARN", "arn:aws:ecs:cn-north-1:123456789012:container-instance/12345678-test-test-test-123456789012"),
expectedBinds: expectedBinds,
expectedBindDirPerm: fs.FileMode(0700).String(),
expectedBindDirOwner: serviceconnect.AppNetUID,
containerInstanceARN: "arn:aws:ecs:cn-north-1:123456789012:container-instance/12345678-test-test-test-123456789012",
},
{
name: "Service connect container has extra binds/ENV. Iso region gets extra /etc/pki bind mount.",
container: serviceConnectContainer,
expectedENV: copyMap(expectedENVs, "ECS_CONTAINER_INSTANCE_ARN", "arn:aws:ecs:us-iso-east-1:123456789012:container-instance/12345678-test-test-test-123456789012"),
expectedBinds: append(expectedBinds, "/etc/pki:/etc/pki"),
expectedBindDirPerm: fs.FileMode(0700).String(),
expectedBindDirOwner: serviceconnect.AppNetUID,
containerInstanceARN: "arn:aws:ecs:us-iso-east-1:123456789012:container-instance/12345678-test-test-test-123456789012",
},
{
name: "Service connect container has extra binds/ENV. Iso region gets extra /etc/pki bind mount.",
container: serviceConnectContainer,
expectedENV: copyMap(expectedENVs, "ECS_CONTAINER_INSTANCE_ARN", "arn:aws:ecs:eu-isoe-west-1:123456789012:container-instance/12345678-test-test-test-123456789012"),
expectedBinds: append(expectedBinds, "/etc/pki:/etc/pki"),
expectedBindDirPerm: fs.FileMode(0700).String(),
expectedBindDirOwner: serviceconnect.AppNetUID,
containerInstanceARN: "arn:aws:ecs:eu-isoe-west-1:123456789012:container-instance/12345678-test-test-test-123456789012",
},
{
name: "Service connect container has extra binds/ENV. Iso region gets extra /etc/pki bind mount.",
container: serviceConnectContainer,
expectedENV: copyMap(expectedENVs, "ECS_CONTAINER_INSTANCE_ARN", "arn:aws:ecs:us-isof-south-1:123456789012:container-instance/12345678-test-test-test-123456789012"),
expectedBinds: append(expectedBinds, "/etc/pki:/etc/pki"),
expectedBindDirPerm: fs.FileMode(0700).String(),
expectedBindDirOwner: serviceconnect.AppNetUID,
containerInstanceARN: "arn:aws:ecs:us-isof-south-1:123456789012:container-instance/12345678-test-test-test-123456789012",
},
{
name: "Service connect container has extra binds/ENV. Unknown region gets /etc/pki bind mount.",
container: serviceConnectContainer,
expectedENV: copyMap(expectedENVs, "ECS_CONTAINER_INSTANCE_ARN", "arn:aws:ecs:ap-iso-southeast-1:123456789012:container-instance/12345678-test-test-test-123456789012"),
expectedBinds: append(expectedBinds, "/etc/pki:/etc/pki"),
expectedBindDirPerm: fs.FileMode(0700).String(),
expectedBindDirOwner: serviceconnect.AppNetUID,
containerInstanceARN: "arn:aws:ecs:ap-iso-southeast-1:123456789012:container-instance/12345678-test-test-test-123456789012",
},
{
name: "Service connect container has extra binds/ENV. Invalid region gets /etc/pki bind mount.",
container: serviceConnectContainer,
expectedENV: copyMap(expectedENVs, "ECS_CONTAINER_INSTANCE_ARN", "foo-bar-invalid-arn"),
expectedBinds: append(expectedBinds, "/etc/pki:/etc/pki"),
expectedBindDirPerm: fs.FileMode(0700).String(),
expectedBindDirOwner: serviceconnect.AppNetUID,
containerInstanceARN: "foo-bar-invalid-arn",
},
}
// Add test cases for other containers expecting no modifications
Expand All @@ -179,14 +253,14 @@ func testAgentContainerModificationsForServiceConnect(t *testing.T, privilegedMo
agentContainerImageName: "container",
appnetInterfaceVersion: "v1",

containerInstanceARN: "fake_container_instance",
logPathContainer: "/some/other/log",
logPathHostRoot: filepath.Join(tempDir, "log"),
logPathContainer: "/some/other/log",
logPathHostRoot: filepath.Join(tempDir, "log"),
}

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
hostConfig := &dockercontainer.HostConfig{}
scManager.containerInstanceARN = tc.containerInstanceARN
err := scManager.AugmentTaskContainer(scTask, tc.container, hostConfig)
if err != nil {
t.Fatal(err)
Expand Down
Loading