Skip to content

Commit

Permalink
updated docs and validation to further prohibit null chars in region,…
Browse files Browse the repository at this point in the history
… datacenter, and job name
  • Loading branch information
cgbaker committed Oct 5, 2020
1 parent a6fd467 commit a048500
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 7 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ IMPROVEMENTS:
* jobspec: Lowered minimum CPU allowed from 10 to 1. [[GH-8996](https://github.com/hashicorp/nomad/issues/8996)]

__BACKWARDS INCOMPATIBILITIES:__
* core: job IDs, task group names, and task names are not allowed to contain null characters [[GH-9020](https://github.com/hashicorp/nomad/issues/9020)]
* core: null characters are prohibited in region, datacenter, job name/ID, task group name, and task name [[GH-9020
](https://github.com/hashicorp/nomad/issues/9020)]
* driver/docker: Tasks are now issued SIGTERM instead of SIGINT when stopping [[GH-8932](https://github.com/hashicorp/nomad/issues/8932)]

BUG FIXES:
Expand Down
12 changes: 12 additions & 0 deletions command/agent/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,18 @@ func (c *Command) isValidConfig(config, cmdConfig *Config) bool {
return false
}

// Check that the region does not contain invalid characters
if strings.ContainsAny(config.Region, "\000") {
c.Ui.Error("Region contains invalid characters")
return false
}

// Check that the datacenter name does not contain invalid characters
if strings.ContainsAny(config.Datacenter, "\000") {
c.Ui.Error("Datacenter contains invalid characters")
return false
}

// Set up the TLS configuration properly if we have one.
// XXX chelseakomlo: set up a TLSConfig New method which would wrap
// constructor-type actions like this.
Expand Down
101 changes: 100 additions & 1 deletion command/agent/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import (
"strings"
"testing"

"github.com/hashicorp/nomad/version"
"github.com/mitchellh/cli"

"github.com/hashicorp/nomad/version"
)

func TestCommand_Implements(t *testing.T) {
Expand Down Expand Up @@ -142,3 +143,101 @@ func TestCommand_MetaConfigValidation(t *testing.T) {
}
}
}

func TestCommand_NullCharInDatacenter(t *testing.T) {
tmpDir, err := ioutil.TempDir("", "nomad")
if err != nil {
t.Fatalf("err: %s", err)
}
defer os.RemoveAll(tmpDir)

tcases := []string{
"char-\\000-in-the-middle",
"ends-with-\\000",
"\\000-at-the-beginning",
}
for _, tc := range tcases {
configFile := filepath.Join(tmpDir, "conf1.hcl")
err = ioutil.WriteFile(configFile, []byte(`
datacenter = "`+tc+`"
client{
enabled = true
}`), 0600)
if err != nil {
t.Fatalf("err: %s", err)
}

// Make a new command. We preemptively close the shutdownCh
// so that the command exits immediately instead of blocking.
ui := new(cli.MockUi)
shutdownCh := make(chan struct{})
close(shutdownCh)
cmd := &Command{
Version: version.GetVersion(),
Ui: ui,
ShutdownCh: shutdownCh,
}

// To prevent test failures on hosts whose hostname resolves to
// a loopback address, we must append a bind address
args := []string{"-client", "-data-dir=" + tmpDir, "-config=" + configFile, "-bind=169.254.0.1"}
if code := cmd.Run(args); code != 1 {
t.Fatalf("args: %v\nexit: %d\n", args, code)
}

out := ui.ErrorWriter.String()
exp := "Datacenter contains invalid characters"
if !strings.Contains(out, exp) {
t.Fatalf("expect to find %q\n\n%s", exp, out)
}
}
}

func TestCommand_NullCharInRegion(t *testing.T) {
tmpDir, err := ioutil.TempDir("", "nomad")
if err != nil {
t.Fatalf("err: %s", err)
}
defer os.RemoveAll(tmpDir)

tcases := []string{
"char-\\000-in-the-middle",
"ends-with-\\000",
"\\000-at-the-beginning",
}
for _, tc := range tcases {
configFile := filepath.Join(tmpDir, "conf1.hcl")
err = ioutil.WriteFile(configFile, []byte(`
region = "`+tc+`"
client{
enabled = true
}`), 0600)
if err != nil {
t.Fatalf("err: %s", err)
}

// Make a new command. We preemptively close the shutdownCh
// so that the command exits immediately instead of blocking.
ui := new(cli.MockUi)
shutdownCh := make(chan struct{})
close(shutdownCh)
cmd := &Command{
Version: version.GetVersion(),
Ui: ui,
ShutdownCh: shutdownCh,
}

// To prevent test failures on hosts whose hostname resolves to
// a loopback address, we must append a bind address
args := []string{"-client", "-data-dir=" + tmpDir, "-config=" + configFile, "-bind=169.254.0.1"}
if code := cmd.Run(args); code != 1 {
t.Fatalf("args: %v\nexit: %d\n", args, code)
}

out := ui.ErrorWriter.String()
exp := "Region contains invalid characters"
if !strings.Contains(out, exp) {
t.Fatalf("expect to find %q\n\n%s", exp, out)
}
}
}
2 changes: 2 additions & 0 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -3948,6 +3948,8 @@ func (j *Job) Validate() error {
}
if j.Name == "" {
mErr.Errors = append(mErr.Errors, errors.New("Missing job name"))
} else if strings.Contains(j.Name, "\000") {
mErr.Errors = append(mErr.Errors, errors.New("Job Name contains a null chararacter"))
}
if j.Namespace == "" {
mErr.Errors = append(mErr.Errors, errors.New("Job must be in a namespace"))
Expand Down
7 changes: 6 additions & 1 deletion nomad/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,13 @@ func TestJob_ValidateNullChar(t *testing.T) {
job.ID = "id_with\000null_character"
assert.Error(job.Validate(), "null character in job ID should not validate")

// task group name should not allow null characters
// job name should not allow null characters
job.ID = "happy_little_job_id"
job.Name = "my job name with \000 characters"
assert.Error(job.Validate(), "null character in job name should not validate")

// task group name should not allow null characters
job.Name = "my job"
job.TaskGroups[0].Name = "oh_no_another_\000_char"
assert.Error(job.Validate(), "null character in task group name should not validate")

Expand Down
10 changes: 6 additions & 4 deletions website/pages/docs/upgrade/upgrade-specific.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ When stopping tasks running with the Docker task driver, Nomad documents that a
versions of Nomad would issue `SIGINT` instead. Starting again with Nomad v0.13.0
`SIGTERM` will be sent by default when stopping Docker tasks.

### Null characters in job, task group, and task IDs
### Null characters in region, datacenter, job name/ID, task group name, and task names

Starting with Nomad v0.13.0, job will fail validation if any of the following
contain null character: the job ID, the task group name, or the task name. Any
jobs meeting this requirement should be modified before an update to v0.13.0.
Starting with Nomad v0.13.0, jobs will fail validation if any of the following
contain null character: the job ID or name, the task group name, or the task name. Any
jobs meeting this requirement should be modified before an update to v0.13.0. Similarly,
client and server config validation will prohibit either the region or the datacenter
from containing null characters.

## Nomad 0.12.0

Expand Down

0 comments on commit a048500

Please sign in to comment.