From 08bd18231dd30f7564fcf5dbb205b97ea0a283db Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Tue, 19 May 2020 15:27:11 +0200 Subject: [PATCH] Introduce `Normalize` and `Check` in compose model lifecycle Signed-off-by: Nicolas De Loof --- ecs/pkg/amazon/check.go | 42 ++++ .../{validate_test.go => check_test.go} | 4 +- ecs/pkg/amazon/cloudformation.go | 8 +- ecs/pkg/amazon/compatibility.go | 179 ++++++++++++++++++ ecs/pkg/amazon/convert.go | 27 ++- ecs/pkg/amazon/validate.go | 35 ---- ecs/pkg/compose/normalize.go | 53 ++++++ ecs/pkg/compose/project.go | 5 + 8 files changed, 311 insertions(+), 42 deletions(-) create mode 100644 ecs/pkg/amazon/check.go rename ecs/pkg/amazon/{validate_test.go => check_test.go} (63%) create mode 100644 ecs/pkg/amazon/compatibility.go delete mode 100644 ecs/pkg/amazon/validate.go create mode 100644 ecs/pkg/compose/normalize.go diff --git a/ecs/pkg/amazon/check.go b/ecs/pkg/amazon/check.go new file mode 100644 index 000000000..d249afe4a --- /dev/null +++ b/ecs/pkg/amazon/check.go @@ -0,0 +1,42 @@ +package amazon + +import ( + "github.com/compose-spec/compose-go/types" + "github.com/docker/ecs-plugin/pkg/compose" +) + +type Warning string +type Warnings []string + +type CompatibilityChecker interface { + CheckService(service *types.ServiceConfig) + CheckCapAdd(service *types.ServiceConfig) + CheckDependsOn(service *types.ServiceConfig) + CheckDNS(service *types.ServiceConfig) + CheckDNSOpts(service *types.ServiceConfig) + CheckDNSSearch(service *types.ServiceConfig) + CheckDomainName(service *types.ServiceConfig) + CheckExtraHosts(service *types.ServiceConfig) + CheckHostname(service *types.ServiceConfig) + CheckIpc(service *types.ServiceConfig) + CheckLabels(service *types.ServiceConfig) + CheckLinks(service *types.ServiceConfig) + CheckLogging(service *types.ServiceConfig) + CheckMacAddress(service *types.ServiceConfig) + CheckNetworkMode(service *types.ServiceConfig) + CheckPid(service *types.ServiceConfig) + CheckSysctls(service *types.ServiceConfig) + CheckTmpfs(service *types.ServiceConfig) + CheckUserNSMode(service *types.ServiceConfig) + Errors() []error +} + +// Check the compose model do not use unsupported features and inject sane defaults for ECS deployment +func Check(project *compose.Project) []error { + c := FargateCompatibilityChecker{} + for i, service := range project.Services { + c.CheckService(&service) + project.Services[i] = service + } + return c.errors +} diff --git a/ecs/pkg/amazon/validate_test.go b/ecs/pkg/amazon/check_test.go similarity index 63% rename from ecs/pkg/amazon/validate_test.go rename to ecs/pkg/amazon/check_test.go index e0cdd6507..3f1859548 100644 --- a/ecs/pkg/amazon/validate_test.go +++ b/ecs/pkg/amazon/check_test.go @@ -8,6 +8,6 @@ import ( func TestInvalidNetworkMode(t *testing.T) { project := load(t, "testdata/invalid_network_mode.yaml") - err := Validate(project) - assert.Error(t, err, "ECS do not support NetworkMode \"bridge\"") + err := Check(project) + assert.Error(t, err[0], "'network_mode' \"bridge\" is not supported") } diff --git a/ecs/pkg/amazon/cloudformation.go b/ecs/pkg/amazon/cloudformation.go index 93ed2e095..2eb6f1df8 100644 --- a/ecs/pkg/amazon/cloudformation.go +++ b/ecs/pkg/amazon/cloudformation.go @@ -4,6 +4,8 @@ import ( "fmt" "strings" + "github.com/sirupsen/logrus" + cloudmapapi "github.com/aws/aws-sdk-go/service/servicediscovery" ecsapi "github.com/aws/aws-sdk-go/service/ecs" @@ -26,9 +28,9 @@ const ( // Convert a compose project into a CloudFormation template func (c client) Convert(project *compose.Project) (*cloudformation.Template, error) { - err := Validate(project) - if err != nil { - return nil, err + warnings := Check(project) + for _, w := range warnings { + logrus.Warn(w) } template := cloudformation.NewTemplate() diff --git a/ecs/pkg/amazon/compatibility.go b/ecs/pkg/amazon/compatibility.go new file mode 100644 index 000000000..6049a4d4e --- /dev/null +++ b/ecs/pkg/amazon/compatibility.go @@ -0,0 +1,179 @@ +package amazon + +import ( + "fmt" + + "github.com/aws/aws-sdk-go/service/ecs" + "github.com/compose-spec/compose-go/types" +) + +type FargateCompatibilityChecker struct { + errors []error +} + +func (c *FargateCompatibilityChecker) error(message string, args ...interface{}) { + c.errors = append(c.errors, fmt.Errorf(message, args...)) +} + +func (c *FargateCompatibilityChecker) Errors() []error { + return c.errors +} + +func (c *FargateCompatibilityChecker) CheckService(service *types.ServiceConfig) { + c.CheckCapAdd(service) + c.CheckDependsOn(service) + c.CheckDNS(service) + c.CheckDNSOpts(service) + c.CheckDNSSearch(service) + c.CheckDomainName(service) + c.CheckExtraHosts(service) + c.CheckHostname(service) + c.CheckIpc(service) + c.CheckLabels(service) + c.CheckLinks(service) + c.CheckLogging(service) + c.CheckMacAddress(service) + c.CheckNetworkMode(service) + c.CheckPid(service) + c.CheckSysctls(service) + c.CheckTmpfs(service) + c.CheckUserNSMode(service) +} + +func (c *FargateCompatibilityChecker) CheckNetworkMode(service *types.ServiceConfig) { + if service.NetworkMode != "" && service.NetworkMode != ecs.NetworkModeAwsvpc { + c.error("'network_mode' %q is not supported", service.NetworkMode) + } + service.NetworkMode = ecs.NetworkModeAwsvpc +} + +func (c *FargateCompatibilityChecker) CheckDependsOn(service *types.ServiceConfig) { + if len(service.DependsOn) != 0 { + c.error("'depends_on' is not supported") + service.DependsOn = nil + } +} + +func (c *FargateCompatibilityChecker) CheckLinks(service *types.ServiceConfig) { + if len(service.Links) != 0 { + c.error("'links' is not supported") + service.Links = nil + } +} + +func (c *FargateCompatibilityChecker) CheckLogging(service *types.ServiceConfig) { + c.CheckLoggingDriver(service) +} + +func (c *FargateCompatibilityChecker) CheckLoggingDriver(service *types.ServiceConfig) { + if service.LogDriver != "" && service.LogDriver != ecs.LogDriverAwslogs { + c.error("'log_driver' %q is not supported", service.LogDriver) + service.LogDriver = ecs.LogDriverAwslogs + } +} + +func (c *FargateCompatibilityChecker) CheckPid(service *types.ServiceConfig) { + if service.Pid != "" { + c.error("'pid' is not supported") + service.Pid = "" + } +} + +func (c *FargateCompatibilityChecker) CheckUserNSMode(service *types.ServiceConfig) { + if service.UserNSMode != "" { + c.error("'userns_mode' is not supported") + service.UserNSMode = "" + } +} + +func (c *FargateCompatibilityChecker) CheckIpc(service *types.ServiceConfig) { + if service.Ipc != "" { + c.error("'ipc' is not supported") + service.Ipc = "" + } +} + +func (c *FargateCompatibilityChecker) CheckMacAddress(service *types.ServiceConfig) { + if service.MacAddress != "" { + c.error("'mac_address' is not supported") + service.MacAddress = "" + } +} + +func (c *FargateCompatibilityChecker) CheckHostname(service *types.ServiceConfig) { + if service.Hostname != "" { + c.error("'hostname' is not supported") + service.Hostname = "" + } +} + +func (c *FargateCompatibilityChecker) CheckDomainName(service *types.ServiceConfig) { + if service.DomainName != "" { + c.error("'domainname' is not supported") + service.DomainName = "" + } +} + +func (c *FargateCompatibilityChecker) CheckDNSSearch(service *types.ServiceConfig) { + if len(service.DNSSearch) > 0 { + c.error("'dns_search' is not supported") + service.DNSSearch = nil + } +} + +func (c *FargateCompatibilityChecker) CheckDNS(service *types.ServiceConfig) { + if len(service.DNS) > 0 { + c.error("'dns' is not supported") + service.DNS = nil + } +} + +func (c *FargateCompatibilityChecker) CheckDNSOpts(service *types.ServiceConfig) { + if len(service.DNSOpts) > 0 { + c.error("'dns_opt' is not supported") + service.DNSOpts = nil + } +} + +func (c *FargateCompatibilityChecker) CheckExtraHosts(service *types.ServiceConfig) { + if len(service.ExtraHosts) > 0 { + c.error("'extra_hosts' is not supported") + service.ExtraHosts = nil + } +} + +func (c *FargateCompatibilityChecker) CheckCapAdd(service *types.ServiceConfig) { + for i, v := range service.CapAdd { + if v != "SYS_PTRACE" { + c.error("'cap_add' %s is not supported", v) + l := len(service.CapAdd) + service.CapAdd[i] = service.CapAdd[l-1] + service.CapAdd = service.CapAdd[:l-1] + } + } +} + +func (c *FargateCompatibilityChecker) CheckTmpfs(service *types.ServiceConfig) { + if len(service.Tmpfs) > 0 { + c.error("'tmpfs' is not supported") + service.Tmpfs = nil + } +} + +func (c *FargateCompatibilityChecker) CheckSysctls(service *types.ServiceConfig) { + if len(service.Sysctls) > 0 { + c.error("'sysctls' is not supported") + service.Sysctls = nil + } +} + +func (c *FargateCompatibilityChecker) CheckLabels(service *types.ServiceConfig) { + for k, v := range service.Labels { + if v == "" { + c.error("'labels' with an empty value is not supported") + delete(service.Labels, k) + } + } +} + +var _ CompatibilityChecker = &FargateCompatibilityChecker{} diff --git a/ecs/pkg/amazon/convert.go b/ecs/pkg/amazon/convert.go index 13e90690a..7c8724c47 100644 --- a/ecs/pkg/amazon/convert.go +++ b/ecs/pkg/amazon/convert.go @@ -10,6 +10,7 @@ import ( ecsapi "github.com/aws/aws-sdk-go/service/ecs" "github.com/awslabs/goformation/v4/cloudformation" "github.com/awslabs/goformation/v4/cloudformation/ecs" + "github.com/awslabs/goformation/v4/cloudformation/tags" "github.com/compose-spec/compose-go/types" "github.com/docker/cli/opts" "github.com/docker/ecs-plugin/pkg/compose" @@ -68,7 +69,7 @@ func Convert(project *compose.Project, service types.ServiceConfig) (*ecs.TaskDe ResourceRequirements: nil, StartTimeout: 0, StopTimeout: durationToInt(service.StopGracePeriod), - SystemControls: nil, + SystemControls: toSystemControls(service.Sysctls), Ulimits: toUlimits(service.Ulimits), User: service.User, VolumesFrom: nil, @@ -84,10 +85,32 @@ func Convert(project *compose.Project, service types.ServiceConfig) (*ecs.TaskDe PlacementConstraints: toPlacementConstraints(service.Deploy), ProxyConfiguration: nil, RequiresCompatibilities: []string{ecsapi.LaunchTypeFargate}, - Tags: nil, + Tags: toTags(service.Labels), }, nil } +func toTags(labels types.Labels) []tags.Tag { + t := []tags.Tag{} + for n, v := range labels { + t = append(t, tags.Tag{ + Key: n, + Value: v, + }) + } + return t +} + +func toSystemControls(sysctls types.Mapping) []ecs.TaskDefinition_SystemControl { + sys := []ecs.TaskDefinition_SystemControl{} + for k, v := range sysctls { + sys = append(sys, ecs.TaskDefinition_SystemControl{ + Namespace: k, + Value: v, + }) + } + return sys +} + func toLimits(service types.ServiceConfig) (string, string, error) { // All possible cpu/mem values for Fargate cpuToMem := map[int64][]types.UnitBytes{ diff --git a/ecs/pkg/amazon/validate.go b/ecs/pkg/amazon/validate.go deleted file mode 100644 index 4e7918fc0..000000000 --- a/ecs/pkg/amazon/validate.go +++ /dev/null @@ -1,35 +0,0 @@ -package amazon - -import ( - "fmt" - - "github.com/compose-spec/compose-go/types" - "github.com/docker/ecs-plugin/pkg/compose" -) - -// Validate check the compose model do not use unsupported features and inject sane defaults for ECS deployment -func Validate(project *compose.Project) error { - if len(project.Networks) == 0 { - // Compose application model implies a default network if none is explicitly set. - // FIXME move this to compose-go - project.Networks["default"] = types.NetworkConfig{ - Name: "default", - } - } - - for i, service := range project.Services { - if len(service.Networks) == 0 { - // Service without explicit network attachment are implicitly exposed on default network - // FIXME move this to compose-go - service.Networks = map[string]*types.ServiceNetworkConfig{"default": nil} - project.Services[i] = service - } - - if service.NetworkMode != "" && service.NetworkMode != "awsvpc" { - return fmt.Errorf("ECS do not support NetworkMode %q", service.NetworkMode) - } - } - - // Here we can check for incompatible attributes, inject sane defaults, etc - return nil -} diff --git a/ecs/pkg/compose/normalize.go b/ecs/pkg/compose/normalize.go new file mode 100644 index 000000000..0061a1f1f --- /dev/null +++ b/ecs/pkg/compose/normalize.go @@ -0,0 +1,53 @@ +package compose + +import ( + "fmt" + + "github.com/compose-spec/compose-go/types" + "github.com/sirupsen/logrus" +) + +// Normalize a compose-go model to move deprecated attributes to canonical position, and introduce implicit defaults +// FIXME move this to compose-go +func Normalize(model *types.Config) error { + if len(model.Networks) == 0 { + // Compose application model implies a default network if none is explicitly set. + model.Networks["default"] = types.NetworkConfig{ + Name: "default", + } + } + + for i, s := range model.Services { + if len(s.Networks) == 0 { + // Service without explicit network attachment are implicitly exposed on default network + s.Networks = map[string]*types.ServiceNetworkConfig{"default": nil} + } + + if s.LogDriver != "" { + logrus.Warn("`log_driver` is deprecated. Use the `logging` attribute") + if s.Logging == nil { + s.Logging = &types.LoggingConfig{} + } + if s.Logging.Driver == "" { + s.Logging.Driver = s.LogDriver + } else { + return fmt.Errorf("can't use both 'log_driver' (deprecated) and 'logging.driver'") + } + } + if len(s.LogOpt) != 0 { + logrus.Warn("`log_opts` is deprecated. Use the `logging` attribute") + if s.Logging == nil { + s.Logging = &types.LoggingConfig{} + } + for k, v := range s.LogOpt { + if _, ok := s.Logging.Options[k]; !ok { + s.Logging.Options[k] = v + } else { + return fmt.Errorf("can't use both 'log_opt' (deprecated) and 'logging.options'") + } + } + } + model.Services[i] = s + } + return nil +} diff --git a/ecs/pkg/compose/project.go b/ecs/pkg/compose/project.go index 5c244b2c4..a90bba071 100644 --- a/ecs/pkg/compose/project.go +++ b/ecs/pkg/compose/project.go @@ -25,6 +25,11 @@ func NewProject(config types.ConfigDetails, name string) (*Project, error) { return nil, err } + err = Normalize(model) + if err != nil { + return nil, err + } + p := Project{ Config: *model, projectDir: config.WorkingDir,