Skip to content

Commit

Permalink
Binding metadata directory in Z mode for selinux enabled docker
Browse files Browse the repository at this point in the history
  • Loading branch information
mythri-garaga committed Nov 14, 2019
1 parent c94547e commit d6d255e
Show file tree
Hide file tree
Showing 18 changed files with 243 additions and 18 deletions.
7 changes: 4 additions & 3 deletions agent/containermetadata/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type Manager interface {
SetAvailabilityZone(string)
SetHostPrivateIPv4Address(string)
SetHostPublicIPv4Address(string)
Create(*dockercontainer.Config, *dockercontainer.HostConfig, *apitask.Task, string) error
Create(*dockercontainer.Config, *dockercontainer.HostConfig, *apitask.Task, string, []string) error
Update(context.Context, string, *apitask.Task, string) error
Clean(string) error
}
Expand Down Expand Up @@ -113,7 +113,8 @@ func (manager *metadataManager) SetHostPublicIPv4Address(ipv4address string) {
// Create creates the metadata file and adds the metadata directory to
// the container's mounted host volumes
// Pointer hostConfig is modified directly so there is risk of concurrency errors.
func (manager *metadataManager) Create(config *dockercontainer.Config, hostConfig *dockercontainer.HostConfig, task *apitask.Task, containerName string) error {
func (manager *metadataManager) Create(config *dockercontainer.Config, hostConfig *dockercontainer.HostConfig,
task *apitask.Task, containerName string, dockerSecurityOptions []string) error {
// Create task and container directories if they do not yet exist
metadataDirectoryPath, err := getMetadataFilePath(task.Arn, containerName, manager.dataDir)
// Stop metadata creation if path is malformed for any reason
Expand All @@ -135,7 +136,7 @@ func (manager *metadataManager) Create(config *dockercontainer.Config, hostConfi

// Add the directory of this container's metadata to the container's mount binds
// Then add the destination directory as an environment variable in the container $METADATA
binds, env := createBindsEnv(hostConfig.Binds, config.Env, manager.dataDirOnHost, metadataDirectoryPath)
binds, env := createBindsEnv(hostConfig.Binds, config.Env, manager.dataDirOnHost, metadataDirectoryPath, dockerSecurityOptions)
config.Env = env
hostConfig.Binds = binds
return nil
Expand Down
6 changes: 4 additions & 2 deletions agent/containermetadata/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,10 @@ func TestCreateMalformedFilepath(t *testing.T) {
mockTaskARN := invalidTaskARN
mockTask := &apitask.Task{Arn: mockTaskARN}
mockContainerName := containerName
mockDockerSecurityOptions := types.Info{SecurityOptions: make([]string, 0)}.SecurityOptions

newManager := &metadataManager{}
err := newManager.Create(nil, nil, mockTask, mockContainerName)
err := newManager.Create(nil, nil, mockTask, mockContainerName, mockDockerSecurityOptions)
assert.Error(t, err)
}

Expand All @@ -116,6 +117,7 @@ func TestCreateMkdirAllFail(t *testing.T) {
mockTaskARN := validTaskARN
mockTask := &apitask.Task{Arn: mockTaskARN}
mockContainerName := containerName
mockDockerSecurityOptions := types.Info{SecurityOptions: make([]string, 0)}.SecurityOptions

gomock.InOrder(
mockOS.EXPECT().MkdirAll(gomock.Any(), gomock.Any()).Return(errors.New("err")),
Expand All @@ -124,7 +126,7 @@ func TestCreateMkdirAllFail(t *testing.T) {
newManager := &metadataManager{
osWrap: mockOS,
}
err := newManager.Create(nil, nil, mockTask, mockContainerName)
err := newManager.Create(nil, nil, mockTask, mockContainerName, mockDockerSecurityOptions)
assert.Error(t, err)
}

Expand Down
3 changes: 2 additions & 1 deletion agent/containermetadata/manager_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func TestCreate(t *testing.T) {
mockContainerName := containerName
mockConfig := &dockercontainer.Config{Env: make([]string, 0)}
mockHostConfig := &dockercontainer.HostConfig{Binds: make([]string, 0)}
mockDockerSecurityOptions := types.Info{SecurityOptions: make([]string, 0)}.SecurityOptions

gomock.InOrder(
mockOS.EXPECT().MkdirAll(gomock.Any(), gomock.Any()).Return(nil),
Expand All @@ -53,7 +54,7 @@ func TestCreate(t *testing.T) {
osWrap: mockOS,
ioutilWrap: mockIOUtil,
}
err := newManager.Create(mockConfig, mockHostConfig, mockTask, mockContainerName)
err := newManager.Create(mockConfig, mockHostConfig, mockTask, mockContainerName, mockDockerSecurityOptions)

assert.NoError(t, err)
assert.Equal(t, 1, len(mockConfig.Env), "Unexpected number of environment variables in config")
Expand Down
3 changes: 2 additions & 1 deletion agent/containermetadata/manager_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func TestCreate(t *testing.T) {
mockContainerName := containerName
mockConfig := &dockercontainer.Config{Env: make([]string, 0)}
mockHostConfig := &dockercontainer.HostConfig{Binds: make([]string, 0)}
mockDockerSecurityOptions := types.Info{SecurityOptions: make([]string, 0)}.SecurityOptions

gomock.InOrder(
mockOS.EXPECT().MkdirAll(gomock.Any(), gomock.Any()).Return(nil),
Expand All @@ -50,7 +51,7 @@ func TestCreate(t *testing.T) {
newManager := &metadataManager{
osWrap: mockOS,
}
err := newManager.Create(mockConfig, mockHostConfig, mockTask, mockContainerName)
err := newManager.Create(mockConfig, mockHostConfig, mockTask, mockContainerName, mockDockerSecurityOptions)

assert.NoError(t, err)
assert.Equal(t, 1, len(mockConfig.Env), "Unexpected number of environment variables in config")
Expand Down
8 changes: 4 additions & 4 deletions agent/containermetadata/mocks/containermetadata_mocks.go

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

20 changes: 17 additions & 3 deletions agent/containermetadata/write_metadata_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,34 @@ import (
"github.com/aws/amazon-ecs-agent/agent/utils/ioutilwrapper"
"github.com/aws/amazon-ecs-agent/agent/utils/oswrapper"

"github.com/cihub/seelog"
"github.com/pborman/uuid"
)

const (
mountPoint = "/opt/ecs/metadata"
tempFile = "temp_metadata_file"
mountPoint = "/opt/ecs/metadata"
tempFile = "temp_metadata_file"
bindMode = "Z"
selinuxSecurityOption = "selinux"
)

// createBindsEnv will do the appropriate formatting to add a new mount in a container's HostConfig
// and add the metadata file path as an environment variable ECS_CONTAINER_METADATA_FILE
// We add an additional uuid to the path to ensure it does not conflict with user mounts
func createBindsEnv(binds []string, env []string, dataDirOnHost string, metadataDirectoryPath string) ([]string, []string) {
func createBindsEnv(binds []string, env []string, dataDirOnHost string, metadataDirectoryPath string, dockerSecurityOptions []string) ([]string, []string) {
selinuxEnabled := false
for _, option := range dockerSecurityOptions {
if option == selinuxSecurityOption {
selinuxEnabled = true
}
}

randID := uuid.New()
instanceBind := fmt.Sprintf(`%s/%s:%s/%s`, dataDirOnHost, metadataDirectoryPath, mountPoint, randID)
if selinuxEnabled {
seelog.Info("Selinux is enabled on docker, mounting data directory in Z mode")
instanceBind = fmt.Sprintf(`%s:%s`, instanceBind, bindMode)
}
metadataEnvVariable := fmt.Sprintf("%s=%s/%s/%s", metadataEnvironmentVariable, mountPoint, randID, metadataFile)
binds = append(binds, instanceBind)
env = append(env, metadataEnvVariable)
Expand Down
38 changes: 38 additions & 0 deletions agent/containermetadata/write_metadata_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package containermetadata

import (
"errors"
"fmt"
"testing"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -90,3 +91,40 @@ func TestWriteChmodFail(t *testing.T) {
assert.Error(t, err)
assert.Equal(t, expectErrorMessage, err.Error())
}

func TestCreateBindEnv(t *testing.T) {
mockBinds := []string{}
mockEnv := []string{}
mockDataDirOnHost := ""
mockMetadataDirectoryPath := ""
expectedBindMode := fmt.Sprintf(`:%s`, bindMode)

testcases := []struct {
name string
securityOptions []string
selinuxEnabled bool
}{
{
name: "Selinux Enabled Bind Mode",
securityOptions: []string{"selinux"},
selinuxEnabled: true,
},
{
name: "Selinux Disabled Bind Mode",
securityOptions: []string{""},
selinuxEnabled: false,
},
}

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
binds, _ := createBindsEnv(mockBinds, mockEnv, mockDataDirOnHost, mockMetadataDirectoryPath, tc.securityOptions)
actualBindMode := binds[0][len(binds[0])-2:]
if tc.selinuxEnabled {
assert.Equal(t, expectedBindMode, actualBindMode)
} else {
assert.NotEqual(t, expectedBindMode, actualBindMode)
}
})
}
}
2 changes: 1 addition & 1 deletion agent/containermetadata/write_metadata_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const (

// createBindsEnv will do the appropriate formatting to add a new mount in a container's HostConfig
// and add the metadata file path as an environment variable ECS_CONTAINER_METADATA_FILE
func createBindsEnv(binds []string, env []string, dataDirOnHost string, metadataDirectoryPath string) ([]string, []string) {
func createBindsEnv(binds []string, env []string, dataDirOnHost string, metadataDirectoryPath string, dockerSecurityOptions []string) ([]string, []string) {
randID := uuid.New()
instanceBind := fmt.Sprintf(`%s:%s\%s`, metadataDirectoryPath, mountPoint, randID)
metadataEnvVariable := fmt.Sprintf(`%s=%s\%s\%s`, metadataEnvironmentVariable, mountPoint, randID, metadataFile)
Expand Down
19 changes: 19 additions & 0 deletions agent/dockerclient/dockerapi/docker_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ type DockerClient interface {

// LoadImage loads an image from an input stream. A timeout value and a context should be provided for the request.
LoadImage(context.Context, io.Reader, time.Duration) error

// Info returns the information of the Docker server.
Info(context.Context, time.Duration) (types.Info, error)
}

// DockerGoClient wraps the underlying go-dockerclient and docker/docker library.
Expand Down Expand Up @@ -1075,6 +1078,22 @@ func (dg *dockerGoClient) Version(ctx context.Context, timeout time.Duration) (s
return version, nil
}

func (dg *dockerGoClient) Info(ctx context.Context, timeout time.Duration) (types.Info, error) {
derivedCtx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()

client, err := dg.sdkDockerClient()
if err != nil {
return types.Info{}, err
}
info, infoErr := client.Info(derivedCtx)
if infoErr != nil {
return types.Info{}, infoErr
}

return info, nil
}

func (dg *dockerGoClient) getDaemonVersion() string {
dg.lock.Lock()
defer dg.lock.Unlock()
Expand Down
57 changes: 57 additions & 0 deletions agent/dockerclient/dockerapi/docker_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,63 @@ func TestDockerVersion(t *testing.T) {
assert.Equal(t, "1.6.0", str, "Got unexpected version string: "+str)
}

func TestDockerInfo(t *testing.T) {
mockDockerSDK, client, _, _, _, done := dockerClientSetup(t)
defer done()

mockDockerSDK.EXPECT().Info(gomock.Any()).Return(types.Info{SecurityOptions: []string{"selinux"}}, nil)

ctx, cancel := context.WithCancel(context.TODO())
defer cancel()
info, err := client.Info(ctx, dockerclient.InfoTimeout)

assert.NoError(t, err)
assert.Equal(t, []string{"selinux"}, info.SecurityOptions)
}

func TestDockerInfoError(t *testing.T) {
mockDockerSDK, client, _, _, _, done := dockerClientSetup(t)
defer done()

errorMsg := "Error getting docker info"

mockDockerSDK.EXPECT().Info(gomock.Any()).Return(types.Info{}, errors.New(errorMsg))

ctx, cancel := context.WithCancel(context.TODO())
defer cancel()
info, err := client.Info(ctx, dockerclient.InfoTimeout)

assert.Error(t, err, errorMsg)
assert.Equal(t, types.Info{}, info)
}

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

errorMsg := "Error getting client"

// Mock SDKFactory
mockDockerSDK := mock_sdkclient.NewMockClient(ctrl)
mockDockerSDK.EXPECT().Ping(gomock.Any()).Return(types.Ping{}, nil)
sdkFactory := mock_sdkclientfactory.NewMockFactory(ctrl)

ctx, cancel := context.WithCancel(context.TODO())
defer cancel()

// Return the Docker Go client for the first call
sdkFactory.EXPECT().GetDefaultClient().Times(1).Return(mockDockerSDK, nil)
client, err := NewDockerGoClient(sdkFactory, defaultTestConfig(), ctx)
assert.NoError(t, err)

// Throw error when `Info` tries to get the client
sdkFactory.EXPECT().GetDefaultClient().Return(nil, errors.New(errorMsg))
info, err := client.Info(ctx, dockerclient.InfoTimeout)

assert.Error(t, err, errorMsg)
assert.Equal(t, types.Info{}, info)
}

func TestDockerVersionCached(t *testing.T) {
_, client, _, _, _, done := dockerClientSetup(t)
defer done()
Expand Down
15 changes: 15 additions & 0 deletions agent/dockerclient/dockerapi/mocks/dockerapi_mocks.go

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

1 change: 1 addition & 0 deletions agent/dockerclient/sdkclient/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,5 @@ type Client interface {
VolumeInspect(ctx context.Context, volumeID string) (types.Volume, error)
VolumeRemove(ctx context.Context, volumeID string, force bool) error
ServerVersion(ctx context.Context) (types.Version, error)
Info(ctx context.Context) (types.Info, error)
}
15 changes: 15 additions & 0 deletions agent/dockerclient/sdkclient/mocks/sdkclient_mocks.go

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

3 changes: 3 additions & 0 deletions agent/dockerclient/timeout.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,7 @@ const (

// VersionTimeout is the timeout for the Version API
VersionTimeout = 10 * time.Second

// InfoTimeout is the timeout for the Info API
InfoTimeout = 10 * time.Second
)
7 changes: 6 additions & 1 deletion agent/engine/docker_task_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,12 @@ func (engine *DockerTaskEngine) createContainer(task *apitask.Task, container *a
// Create metadata directory and file then populate it with common metadata of all containers of this task
// Afterwards add this directory to the container's mounts if file creation was successful
if engine.cfg.ContainerMetadataEnabled && !container.IsInternal() {
mderr := engine.metadataManager.Create(config, hostConfig, task, container.Name)
info, infoErr := engine.client.Info(engine.ctx, dockerclient.InfoTimeout)
if infoErr != nil {
seelog.Warnf("Task engine [%s]: unable to get docker info : %v",
task.Arn, infoErr)
}
mderr := engine.metadataManager.Create(config, hostConfig, task, container.Name, info.SecurityOptions)
if mderr != nil {
seelog.Warnf("Task engine [%s]: unable to create metadata for container %s: %v",
task.Arn, container.Name, mderr)
Expand Down
Loading

0 comments on commit d6d255e

Please sign in to comment.