Skip to content

Commit

Permalink
fix gmsa,fsx capability bug
Browse files Browse the repository at this point in the history
  • Loading branch information
arun-annamalai committed Jan 23, 2023
1 parent 17daadd commit d429efb
Show file tree
Hide file tree
Showing 15 changed files with 74 additions and 41 deletions.
2 changes: 1 addition & 1 deletion agent/app/agent_capability.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func (agent *ecsAgent) appendDockerDependentCapabilities(capabilities []*ecs.Att
}

func (agent *ecsAgent) appendGMSACapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute {
if agent.cfg.GMSACapable {
if agent.cfg.GMSACapable.Enabled() {
return appendNameOnlyAttribute(capabilities, attributePrefix+capabilityGMSA)
}

Expand Down
2 changes: 1 addition & 1 deletion agent/app/agent_capability_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1432,7 +1432,7 @@ func TestAppendGMSACapabilities(t *testing.T) {

agent := &ecsAgent{
cfg: &config.Config{
GMSACapable: true,
GMSACapable: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled},
},
}

Expand Down
2 changes: 1 addition & 1 deletion agent/app/agent_capability_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (agent *ecsAgent) appendIPv6Capability(capabilities []*ecs.Attribute) []*ec
}

func (agent *ecsAgent) appendFSxWindowsFileServerCapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute {
if agent.cfg.FSxWindowsFileServerCapable {
if agent.cfg.FSxWindowsFileServerCapable.Enabled() {
return appendNameOnlyAttribute(capabilities, attributePrefix+capabilityFSxWindowsFileServer)
}

Expand Down
6 changes: 3 additions & 3 deletions agent/app/agent_capability_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ func TestAppendGMSACapabilitiesFalse(t *testing.T) {

agent := &ecsAgent{
cfg: &config.Config{
GMSACapable: false,
GMSACapable: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled},
},
}

Expand All @@ -258,7 +258,7 @@ func TestAppendFSxWindowsFileServerCapabilities(t *testing.T) {

agent := &ecsAgent{
cfg: &config.Config{
FSxWindowsFileServerCapable: true,
FSxWindowsFileServerCapable: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled},
},
}

Expand All @@ -280,7 +280,7 @@ func TestAppendFSxWindowsFileServerCapabilitiesFalse(t *testing.T) {

agent := &ecsAgent{
cfg: &config.Config{
FSxWindowsFileServerCapable: false,
FSxWindowsFileServerCapable: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled},
},
}

Expand Down
2 changes: 1 addition & 1 deletion agent/config/config_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func DefaultConfig() Config {
NvidiaRuntime: DefaultNvidiaRuntime,
CgroupCPUPeriod: defaultCgroupCPUPeriod,
GMSACapable: parseGMSACapability(),
FSxWindowsFileServerCapable: false,
FSxWindowsFileServerCapable: BooleanDefaultFalse{Value: ExplicitlyDisabled},
RuntimeStatsLogFile: defaultRuntimeStatsLogFile,
EnableRuntimeStats: BooleanDefaultFalse{Value: NotSet},
ShouldExcludeIPv6PortBinding: BooleanDefaultTrue{Value: ExplicitlyEnabled},
Expand Down
4 changes: 2 additions & 2 deletions agent/config/config_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ func DefaultConfig() Config {
SharedVolumeMatchFullConfig: BooleanDefaultFalse{Value: ExplicitlyDisabled}, //only requiring shared volumes to match on name, which is default docker behavior
PollMetrics: BooleanDefaultFalse{Value: NotSet},
PollingMetricsWaitDuration: DefaultPollingMetricsWaitDuration,
GMSACapable: true,
FSxWindowsFileServerCapable: true,
GMSACapable: BooleanDefaultFalse{Value: ExplicitlyDisabled},
FSxWindowsFileServerCapable: BooleanDefaultFalse{Value: ExplicitlyDisabled},
PauseContainerImageName: DefaultPauseContainerImageName,
PauseContainerTag: DefaultPauseContainerTag,
CNIPluginsPath: filepath.Join(ecsBinaryDir, defaultCNIPluginDirName),
Expand Down
17 changes: 9 additions & 8 deletions agent/config/parse_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ import (
"github.com/cihub/seelog"
)

func parseGMSACapability() bool {
func parseGMSACapability() BooleanDefaultFalse {
envStatus := utils.ParseBool(os.Getenv("ECS_GMSA_SUPPORTED"), true)
if envStatus {
// Check if domain join check override is present
skipDomainJoinCheck := utils.ParseBool(os.Getenv(envSkipDomainJoinCheck), false)
if skipDomainJoinCheck {
seelog.Infof("Skipping domain join validation based on environment override")
return true
return BooleanDefaultFalse{Value: ExplicitlyEnabled}
}

// check if the credentials fetcher socket is created and exists
Expand All @@ -42,15 +42,15 @@ func parseGMSACapability() bool {
if err != nil {
if os.IsNotExist(err) {
seelog.Errorf("CREDENTIALS_FETCHER_HOST_DIR not found, err: %v", err)
return false
return BooleanDefaultFalse{Value: ExplicitlyDisabled}
}
}

//skip domain join check if the domainless gMSA is supported by setting the env variable
domainlessGMSAUser := os.Getenv("CREDENTIALS_FETCHER_SECRET_NAME_FOR_DOMAINLESS_GMSA")
if domainlessGMSAUser != "" && len(domainlessGMSAUser) > 0 {
seelog.Info("domainless gMSA support is enabled")
return true
return BooleanDefaultFalse{Value: ExplicitlyEnabled}
}

// returns true if the container instance is domain joined
Expand All @@ -59,16 +59,17 @@ func parseGMSACapability() bool {

if !isDomainJoined {
seelog.Error("gMSA on linux requires domain joined instance. Did not find expected env var ECS_DOMAIN_JOINED_LINUX_INSTANCE=true")
return BooleanDefaultFalse{Value: ExplicitlyDisabled}
}
return isDomainJoined
return BooleanDefaultFalse{Value: ExplicitlyEnabled}
}
}
seelog.Debug("env variables to support gMSA are not set")
return false
return BooleanDefaultFalse{Value: ExplicitlyDisabled}
}

func parseFSxWindowsFileServerCapability() bool {
return false
func parseFSxWindowsFileServerCapability() BooleanDefaultFalse {
return BooleanDefaultFalse{Value: ExplicitlyDisabled}
}

var IsWindows2016 = func() (bool, error) {
Expand Down
8 changes: 4 additions & 4 deletions agent/config/parse_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,25 @@ func TestParseGMSACapabilitySupported(t *testing.T) {
t.Setenv("ECS_DOMAIN_JOINED_LINUX_INSTANCE", "True")
t.Setenv("CREDENTIALS_FETCHER_HOST_DIR", "/var/run")

assert.True(t, parseGMSACapability())
assert.True(t, parseGMSACapability().Enabled())
}

func TestParseGMSACapabilityNonDomainJoined(t *testing.T) {
t.Setenv("ECS_GMSA_SUPPORTED", "True")
t.Setenv("ECS_DOMAIN_JOINED_LINUX_INSTANCE", "False")

assert.False(t, parseGMSACapability())
assert.False(t, parseGMSACapability().Enabled())
}

func TestParseGMSACapabilityUnsupported(t *testing.T) {
t.Setenv("ECS_GMSA_SUPPORTED", "False")

assert.False(t, parseGMSACapability())
assert.False(t, parseGMSACapability().Enabled())
}

func TestSkipDomainJoinCheckParseGMSACapability(t *testing.T) {
t.Setenv("ECS_GMSA_SUPPORTED", "True")
t.Setenv("ZZZ_SKIP_DOMAIN_JOIN_CHECK_NOT_SUPPORTED_IN_PRODUCTION", "True")

assert.True(t, parseGMSACapability())
assert.True(t, parseGMSACapability().Enabled())
}
8 changes: 4 additions & 4 deletions agent/config/parse_unsupported.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import (
"strings"
)

func parseGMSACapability() bool {
return false
func parseGMSACapability() BooleanDefaultFalse {
return BooleanDefaultFalse{Value: ExplicitlyDisabled}
}

func parseFSxWindowsFileServerCapability() bool {
return false
func parseFSxWindowsFileServerCapability() BooleanDefaultFalse {
return BooleanDefaultFalse{Value: ExplicitlyDisabled}
}

var IsWindows2016 = func() (bool, error) {
Expand Down
31 changes: 31 additions & 0 deletions agent/config/parse_unsupported_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//go:build !linux && !windows
// +build !linux,!windows

// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License"). You may
// not use this file except in compliance with the License. A copy of the
// License is located at
//
// httpaws.amazon.com/apache2.0/
//
// or in the "license" file accompanying this file. This file is distributed
// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
// express or implied. See the License for the specific language governing
// permissions and limitations under the License.

package config

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestParseGMSACapabilitySupported(t *testing.T) {
assert.False(t, parseGMSACapability().Enabled())
}

func TestParseFSxWindowsFileServerCapability(t *testing.T) {
assert.False(t, parseGMSACapability().Enabled())
}
21 changes: 11 additions & 10 deletions agent/config/parse_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,41 +35,42 @@ const (
)

// parseGMSACapability is used to determine if gMSA support can be enabled
func parseGMSACapability() bool {
func parseGMSACapability() BooleanDefaultFalse {
envStatus := utils.ParseBool(os.Getenv("ECS_GMSA_SUPPORTED"), true)
return checkDomainJoinWithEnvOverride(envStatus)
}

// parseFSxWindowsFileServerCapability is used to determine if fsxWindowsFileServer support can be enabled
func parseFSxWindowsFileServerCapability() bool {
func parseFSxWindowsFileServerCapability() BooleanDefaultFalse {
// fsxwindowsfileserver is not supported on Windows 2016 and non-domain-joined container instances
status, err := IsWindows2016()
if err != nil || status == true {
return false
return BooleanDefaultFalse{Value: ExplicitlyDisabled}
}

envStatus := utils.ParseBool(os.Getenv("ECS_FSX_WINDOWS_FILE_SERVER_SUPPORTED"), true)
return checkDomainJoinWithEnvOverride(envStatus)
}

func checkDomainJoinWithEnvOverride(envStatus bool) bool {
func checkDomainJoinWithEnvOverride(envStatus bool) BooleanDefaultFalse {
if envStatus {
// Check if domain join check override is present
skipDomainJoinCheck := utils.ParseBool(os.Getenv(envSkipDomainJoinCheck), false)
if skipDomainJoinCheck {
seelog.Debug("Skipping domain join validation based on environment override")
return true
return BooleanDefaultFalse{Value: ExplicitlyEnabled}
}
// check if container instance is domain joined.
// If container instance is not domain joined, explicitly disable feature configuration.
status, err := isDomainJoined()
if err == nil && status == true {
return true
if err != nil {
seelog.Errorf("Unable to determine valid domain join with err: %v", err)
}
if status == true {
return BooleanDefaultFalse{Value: ExplicitlyEnabled}
}
seelog.Errorf("Unable to determine valid domain join: %v", err)
}

return false
return BooleanDefaultFalse{Value: ExplicitlyDisabled}
}

// isDomainJoined is used to validate if container instance is part of a valid active directory.
Expand Down
4 changes: 2 additions & 2 deletions agent/config/parse_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestParseGMSACapability(t *testing.T) {
os.Setenv("ECS_GMSA_SUPPORTED", "False")
defer os.Unsetenv("ECS_GMSA_SUPPORTED")

assert.False(t, parseGMSACapability())
assert.False(t, parseGMSACapability().Enabled())
}

func TestParseBooleanEnvVar(t *testing.T) {
Expand All @@ -49,5 +49,5 @@ func TestParseFSxWindowsFileServerCapability(t *testing.T) {
os.Setenv("ECS_FSX_WINDOWS_FILE_SERVER_SUPPORTED", "False")
defer os.Unsetenv("ECS_FSX_WINDOWS_FILE_SERVER_SUPPORTED")

assert.False(t, parseFSxWindowsFileServerCapability())
assert.False(t, parseFSxWindowsFileServerCapability().Enabled())
}
4 changes: 2 additions & 2 deletions agent/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,14 +329,14 @@ type Config struct {

// GMSACapable is the config option to indicate if gMSA is supported.
// It should be enabled by default only if the container instance is part of a valid active directory domain.
GMSACapable bool
GMSACapable BooleanDefaultFalse

// VolumePluginCapabilities specifies the capabilities of the ecs volume plugin.
VolumePluginCapabilities []string

// FSxWindowsFileServerCapable is the config option to indicate if fsxWindowsFileServer is supported.
// It should be enabled by default only if the container instance is part of a valid active directory domain.
FSxWindowsFileServerCapable bool
FSxWindowsFileServerCapable BooleanDefaultFalse

// External specifies whether agent is running on external compute capacity (i.e. outside of aws).
External BooleanDefaultFalse
Expand Down
2 changes: 1 addition & 1 deletion agent/engine/engine_sudo_linux_integ_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ func TestGMSATaskFile(t *testing.T) {
cfg := defaultTestConfigIntegTest()
cfg.TaskCPUMemLimit.Value = config.ExplicitlyDisabled
cfg.TaskCleanupWaitDuration = 3 * time.Second
cfg.GMSACapable = true
cfg.GMSACapable = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}
cfg.AWSRegion = "us-west-2"

taskEngine, done, _ := setupGMSALinux(cfg, nil, t)
Expand Down
2 changes: 1 addition & 1 deletion agent/engine/engine_windows_integ_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ func defaultTestConfigIntegTestGMSA() *config.Config {
cfg.TaskCPUMemLimit.Value = config.ExplicitlyDisabled
cfg.ImagePullBehavior = config.ImagePullPreferCachedBehavior

cfg.GMSACapable = true
cfg.GMSACapable = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}
cfg.AWSRegion = "us-west-2"

return cfg
Expand Down

0 comments on commit d429efb

Please sign in to comment.