cli: set -hcl2-strict to false if -hcl1 is defined (#14426)

These options are mutually exclusive but, since `-hcl2-strict` defaults
to `true` users had to explicitily set it to `false` when using `-hcl1`.

Also return `255` when job plan fails validation as this is the expected 
code in this situation.

.changelog/14426.txt

@@ -0,0 +1,7 @@
+```release-note:improvement
+cli: ignore `-hcl2-strict` when -hcl1 is set.
+```
+
+```release-note:bug
+cli: return exit code `255` when `nomad job plan` fails job validation.
+```

command/job_plan.go

@@ -87,12 +87,12 @@ Plan Options:
     used as the job.
 
   -hcl1
-    Parses the job file as HCLv1.
+    Parses the job file as HCLv1. Takes precedence over "-hcl2-strict".
 
   -hcl2-strict
     Whether an error should be produced from the HCL2 parser where a variable
     has been supplied which is not defined within the root variables. Defaults
-    to true.
+    to true, but ignored if "-hcl1" is also defined.
 
   -policy-override
     Sets the flag to force override any soft mandatory Sentinel policies.
@@ -183,9 +183,13 @@ func (c *JobPlanCommand) Run(args []string) int {
 		return 255
 	}
 
+	if c.JobGetter.HCL1 {
+		c.JobGetter.Strict = false
+	}
+
 	if err := c.JobGetter.Validate(); err != nil {
 		c.Ui.Error(fmt.Sprintf("Invalid job options: %s", err))
-		return 1
+		return 255
 	}
 
 	path := args[0]

command/job_plan_test.go

@@ -114,6 +114,25 @@ job "job1" {
 	}
 }
 
+func TestPlanCommand_hcl1_hcl2_strict(t *testing.T) {
+	ci.Parallel(t)
+
+	_, _, addr := testServer(t, false, nil)
+
+	t.Run("-hcl1 implies -hcl2-strict is false", func(t *testing.T) {
+		ui := cli.NewMockUi()
+		cmd := &JobPlanCommand{Meta: Meta{Ui: ui}}
+		got := cmd.Run([]string{
+			"-hcl1", "-hcl2-strict",
+			"-address", addr,
+			"assets/example-short.nomad",
+		})
+		// Exit code 1 here means that an alloc will be created, which is
+		// expected.
+		require.Equal(t, 1, got)
+	})
+}
+
 func TestPlanCommand_From_STDIN(t *testing.T) {
 	ci.Parallel(t)
 	stdinR, stdinW, err := os.Pipe()

command/job_run.go

@@ -95,12 +95,12 @@ Run Options:
     used as the job.
 
   -hcl1
-    Parses the job file as HCLv1.
+    Parses the job file as HCLv1. Takes precedence over "-hcl2-strict".
 
   -hcl2-strict
     Whether an error should be produced from the HCL2 parser where a variable
     has been supplied which is not defined within the root variables. Defaults
-    to true.
+    to true, but ignored if "-hcl1" is also defined.
 
   -output
     Output the JSON that would be submitted to the HTTP API without submitting
@@ -223,6 +223,10 @@ func (c *JobRunCommand) Run(args []string) int {
 		return 1
 	}
 
+	if c.JobGetter.HCL1 {
+		c.JobGetter.Strict = false
+	}
+
 	if err := c.JobGetter.Validate(); err != nil {
 		c.Ui.Error(fmt.Sprintf("Invalid job options: %s", err))
 		return 1

command/job_run_test.go

@@ -54,6 +54,24 @@ job "job1" {
 	}
 }
 
+func TestRunCommand_hcl1_hcl2_strict(t *testing.T) {
+	ci.Parallel(t)
+
+	_, _, addr := testServer(t, false, nil)
+
+	t.Run("-hcl1 implies -hcl2-strict is false", func(t *testing.T) {
+		ui := cli.NewMockUi()
+		cmd := &JobRunCommand{Meta: Meta{Ui: ui}}
+		got := cmd.Run([]string{
+			"-hcl1", "-hcl2-strict",
+			"-address", addr,
+			"-detach",
+			"assets/example-short.nomad",
+		})
+		require.Equal(t, 0, got, ui.ErrorWriter.String())
+	})
+}
+
 func TestRunCommand_Fails(t *testing.T) {
 	ci.Parallel(t)
 

command/job_validate.go

@@ -48,12 +48,12 @@ Validate Options:
     used as the job.
 
   -hcl1
-    Parses the job file as HCLv1.
+    Parses the job file as HCLv1. Takes precedence over "-hcl2-strict".
 
   -hcl2-strict
     Whether an error should be produced from the HCL2 parser where a variable
     has been supplied which is not defined within the root variables. Defaults
-    to true.
+    to true, but ignored if "-hcl1" is also defined.
 
   -vault-token
     Used to validate if the user submitting the job has permission to run the job
@@ -130,6 +130,10 @@ func (c *JobValidateCommand) Run(args []string) int {
 		return 1
 	}
 
+	if c.JobGetter.HCL1 {
+		c.JobGetter.Strict = false
+	}
+
 	if err := c.JobGetter.Validate(); err != nil {
 		c.Ui.Error(fmt.Sprintf("Invalid job options: %s", err))
 		return 1

command/job_validate_test.go

@@ -68,6 +68,22 @@ func TestValidateCommand_Files(t *testing.T) {
 		require.Equal(t, 1, code)
 	})
 }
+func TestValidateCommand_hcl1_hcl2_strict(t *testing.T) {
+	ci.Parallel(t)
+
+	_, _, addr := testServer(t, false, nil)
+
+	t.Run("-hcl1 implies -hcl2-strict is false", func(t *testing.T) {
+		ui := cli.NewMockUi()
+		cmd := &JobValidateCommand{Meta: Meta{Ui: ui}}
+		got := cmd.Run([]string{
+			"-hcl1", "-hcl2-strict",
+			"-address", addr,
+			"assets/example-short.nomad",
+		})
+		require.Equal(t, 0, got, ui.ErrorWriter.String())
+	})
+}
 
 func TestValidateCommand_Fails(t *testing.T) {
 	ci.Parallel(t)

website/content/docs/commands/job/plan.mdx

@@ -71,11 +71,12 @@ capability for the job's namespace.
   such as from "nomad job inspect" or "nomad run -output", the value of the
   field is used as the job.
 
-- `-hcl1`: If set, HCL1 parser is used for parsing the job spec.
+- `-hcl1`: If set, HCL1 parser is used for parsing the job spec. Takes
+  precedence over `-hcl2-strict`.
 
 - `-hcl2-strict`: Whether an error should be produced from the HCL2 parser where
   a variable has been supplied which is not defined within the root variables.
-  Defaults to true.
+  Defaults to true, but ignored if `-hcl1` is defined.
 
 - `-vault-token`: Used to validate if the user submitting the job has
   permission to run the job according to its Vault policies. A Vault token must

website/content/docs/commands/job/run.mdx

@@ -78,11 +78,12 @@ that volume.
   such as from "nomad job inspect" or "nomad run -output", the value of the
   field is used as the job. See [JSON Jobs] for details.
 
-- `-hcl1`: If set, HCL1 parser is used for parsing the job spec.
+- `-hcl1`: If set, HCL1 parser is used for parsing the job spec. Takes
+  precedence over `-hcl2-strict`.
 
 - `-hcl2-strict`: Whether an error should be produced from the HCL2 parser where
   a variable has been supplied which is not defined within the root variables.
-  Defaults to true.
+  Defaults to true, but ignored if `-hcl1` is defined.
 
 - `-output`: Output the JSON that would be submitted to the HTTP API without
   submitting the job.

website/content/docs/commands/job/validate.mdx

@@ -46,11 +46,12 @@ capability for the job's namespace.
   such as from "nomad job inspect" or "nomad run -output", the value of the
   field is used as the job.
 
-- `-hcl1`: If set, HCL1 parser is used for parsing the job spec.
+- `-hcl1`: If set, HCL1 parser is used for parsing the job spec. Takes
+  precedence over `-hcl2-strict`.
 
 - `-hcl2-strict`: Whether an error should be produced from the HCL2 parser where
-a variable has been supplied which is not defined within the root variables.
-Defaults to true.
+  a variable has been supplied which is not defined within the root variables.
+  Defaults to true, but ignored if `-hcl1` is defined.
 
 - `-vault-token`: Used to validate if the user submitting the job has
   permission to run the job according to its Vault policies. A Vault token must
@@ -98,4 +99,4 @@ Job validation successful
 [`go-getter`]: https://github.com/hashicorp/go-getter
 [job specification]: /docs/job-specification
 [`vault` stanza `allow_unauthenticated`]: /docs/configuration/vault#allow_unauthenticated
-[`vault_token`]: /docs/job-specification/job#vault_token
+[`vault_token`]: /docs/job-specification/job#vault_token