From 9c932aaf2ae3ba6547a59ad3f2a18f5883d984bd Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 24 Oct 2016 12:13:47 -0700 Subject: [PATCH] Interpolate and then validate services --- client/task_runner.go | 21 +++++++++++++++++++-- nomad/structs/structs.go | 23 +++++++++++++++++++---- nomad/structs/structs_test.go | 12 ++++++++++-- 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/client/task_runner.go b/client/task_runner.go index 050388bb0dd2..fd6eec80dacb 100644 --- a/client/task_runner.go +++ b/client/task_runner.go @@ -367,6 +367,15 @@ func (r *TaskRunner) Run() { r.logger.Printf("[DEBUG] client: starting task context for '%s' (alloc '%s')", r.task.Name, r.alloc.ID) + // Create the initial environment, this will be recreated if a Vault token + // is needed + if err := r.setTaskEnv(); err != nil { + r.setState( + structs.TaskStateDead, + structs.NewTaskEvent(structs.TaskSetupFailure).SetSetupError(err)) + return + } + if err := r.validateTask(); err != nil { r.setState( structs.TaskStateDead, @@ -417,6 +426,14 @@ func (r *TaskRunner) validateTask() error { } } + // Validate the Service names + for i, service := range r.task.Services { + name := r.taskEnv.ReplaceEnv(service.Name) + if err := service.ValidateName(name); err != nil { + mErr.Errors = append(mErr.Errors, fmt.Errorf("service (%d) failed validation: %v", i, err)) + } + } + if len(mErr.Errors) == 1 { return mErr.Errors[0] } @@ -648,7 +665,7 @@ func (r *TaskRunner) updatedTokenHandler() { if err := r.setTaskEnv(); err != nil { r.setState( structs.TaskStateDead, - structs.NewTaskEvent(structs.TaskDriverFailure).SetDriverError(err).SetFailsTask()) + structs.NewTaskEvent(structs.TaskSetupFailure).SetSetupError(err).SetFailsTask()) return } @@ -687,7 +704,7 @@ func (r *TaskRunner) prestart(resultCh chan bool) { if err := r.setTaskEnv(); err != nil { r.setState( structs.TaskStateDead, - structs.NewTaskEvent(structs.TaskDriverFailure).SetDriverError(err).SetFailsTask()) + structs.NewTaskEvent(structs.TaskSetupFailure).SetSetupError(err).SetFailsTask()) resultCh <- false return } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 8461a75cc222..e0558e0861be 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1909,13 +1909,14 @@ func (s *Service) Canonicalize(job string, taskGroup string, task string) { func (s *Service) Validate() error { var mErr multierror.Error - // Ensure the service name is valid per RFC-952 §1 - // (https://tools.ietf.org/html/rfc952), RFC-1123 §2.1 + // Ensure the service name is valid per the below RFCs but make an exception + // for our interpolation syntax + // RFC-952 §1 (https://tools.ietf.org/html/rfc952), RFC-1123 §2.1 // (https://tools.ietf.org/html/rfc1123), and RFC-2782 // (https://tools.ietf.org/html/rfc2782). - re := regexp.MustCompile(`^(?i:[a-z0-9]|[a-z0-9][a-z0-9\-]{0,61}[a-z0-9])$`) + re := regexp.MustCompile(`^(?i:[a-z0-9]|[a-z0-9\$][a-zA-Z0-9\-\$\{\}\_\.]*[a-z0-9\}])$`) if !re.MatchString(s.Name) { - mErr.Errors = append(mErr.Errors, fmt.Errorf("service name must be valid per RFC 1123 and can contain only alphanumeric characters or dashes and must be less than 63 characters long: %q", s.Name)) + mErr.Errors = append(mErr.Errors, fmt.Errorf("service name must be valid per RFC 1123 and can contain only alphanumeric characters or dashes: %q", s.Name)) } for _, c := range s.Checks { @@ -1931,6 +1932,20 @@ func (s *Service) Validate() error { return mErr.ErrorOrNil() } +// ValidateName checks if the services Name is valid and should be called after +// the name has been interpolated +func (s *Service) ValidateName(name string) error { + // Ensure the service name is valid per RFC-952 §1 + // (https://tools.ietf.org/html/rfc952), RFC-1123 §2.1 + // (https://tools.ietf.org/html/rfc1123), and RFC-2782 + // (https://tools.ietf.org/html/rfc2782). + re := regexp.MustCompile(`^(?i:[a-z0-9]|[a-z0-9][a-z0-9\-]{0,61}[a-z0-9])$`) + if !re.MatchString(name) { + return fmt.Errorf("service name must be valid per RFC 1123 and can contain only alphanumeric characters or dashes and must be less than 63 characters long: %q", name) + } + return nil +} + // Hash calculates the hash of the check based on it's content and the service // which owns it func (s *Service) Hash() string { diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index e64acaeb42aa..b29dde3be59f 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -951,7 +951,7 @@ func TestInvalidServiceCheck(t *testing.T) { Name: "service.name", PortLabel: "bar", } - if err := s.Validate(); err == nil { + if err := s.ValidateName(s.Name); err == nil { t.Fatalf("Service should be invalid (contains a dot): %v", err) } @@ -963,11 +963,19 @@ func TestInvalidServiceCheck(t *testing.T) { t.Fatalf("Service should be invalid (begins with a hyphen): %v", err) } + s = Service{ + Name: "my-service-${NOMAD_META_FOO}", + PortLabel: "bar", + } + if err := s.Validate(); err != nil { + t.Fatalf("Service should be valid: %v", err) + } + s = Service{ Name: "abcdef0123456789-abcdef0123456789-abcdef0123456789-abcdef0123456", PortLabel: "bar", } - if err := s.Validate(); err == nil { + if err := s.ValidateName(s.Name); err == nil { t.Fatalf("Service should be invalid (too long): %v", err) }