From b394a76b893b2bd7ba574eaf39a271fc9cc86aa2 Mon Sep 17 00:00:00 2001 From: James Rasell Date: Thu, 6 Feb 2025 09:42:34 +0100 Subject: [PATCH] jobspec2: isolate package from Nomad core and BUSL. (#25021) --- GNUmakefile | 8 ++++ jobspec2/hcl_conversions.go | 3 +- jobspec2/helper_test.go | 8 ---- jobspec2/parse_job.go | 19 +++++----- jobspec2/parse_test.go | 76 ++++++++++++++++++------------------- jobspec2/util.go | 10 +++++ 6 files changed, 65 insertions(+), 59 deletions(-) delete mode 100644 jobspec2/helper_test.go create mode 100644 jobspec2/util.go diff --git a/GNUmakefile b/GNUmakefile index a56419d4f..a27728a30 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -182,6 +182,14 @@ check: ## Lint the source code @echo "==> Check API package is isolated from rest..." @cd ./api && if go list --test -f '{{ join .Deps "\n" }}' . | grep github.com/hashicorp/nomad/ | grep -v -e /nomad/api/ -e nomad/api.test; then echo " /api package depends the ^^ above internal nomad packages. Remove such dependency"; exit 1; fi + @echo "==> Check jobspec2 package is isolated from Nomad core..." + @cd ./jobspec2 && \ + if go list --test -f '{{ join .Deps "\n" }}' . | \ + grep github.com/hashicorp/nomad/ | \ + grep -v -e /nomad/jobspec2/ -e nomad/jobspec2.test | \ + grep -v -e /nomad/api ; then echo \ + " /jobspec2 package depends the ^^ above internal nomad packages. Remove such dependency"; exit 1; fi + @echo "==> Check command package does not import structs..." @cd ./command && if go list -f '{{ join .Imports "\n" }}' . | grep github.com/hashicorp/nomad/nomad/structs; then echo " /command package imports the structs pkg. Remove such import"; exit 1; fi diff --git a/jobspec2/hcl_conversions.go b/jobspec2/hcl_conversions.go index 8ce337a54..ecf5e4714 100644 --- a/jobspec2/hcl_conversions.go +++ b/jobspec2/hcl_conversions.go @@ -13,7 +13,6 @@ import ( "github.com/hashicorp/hcl/v2/gohcl" "github.com/hashicorp/hcl/v2/hcldec" "github.com/hashicorp/nomad/api" - "github.com/hashicorp/nomad/helper/pointer" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/gocty" ) @@ -120,7 +119,7 @@ func decodeAffinity(body hcl.Body, ctx *hcl.EvalContext, val interface{}) hcl.Di weight := v.GetAttr("weight") if !weight.IsNull() { w, _ := weight.AsBigFloat().Int64() - a.Weight = pointer.Of(int8(w)) + a.Weight = pointerOf(int8(w)) } // If "version" is provided, set the operand diff --git a/jobspec2/helper_test.go b/jobspec2/helper_test.go deleted file mode 100644 index a317d8661..000000000 --- a/jobspec2/helper_test.go +++ /dev/null @@ -1,8 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package jobspec2 - -func intToPtr(v int) *int { - return &v -} diff --git a/jobspec2/parse_job.go b/jobspec2/parse_job.go index 7520f40c5..4ea2edda0 100644 --- a/jobspec2/parse_job.go +++ b/jobspec2/parse_job.go @@ -8,7 +8,6 @@ import ( "time" "github.com/hashicorp/nomad/api" - "github.com/hashicorp/nomad/helper/pointer" ) func normalizeJob(jc *jobConfig) { @@ -81,13 +80,13 @@ func normalizeVault(v *api.Vault) { } if v.Env == nil { - v.Env = pointer.Of(true) + v.Env = pointerOf(true) } if v.DisableFile == nil { - v.DisableFile = pointer.Of(false) + v.DisableFile = pointerOf(false) } if v.ChangeMode == nil { - v.ChangeMode = pointer.Of("restart") + v.ChangeMode = pointerOf("restart") } } @@ -127,16 +126,16 @@ func normalizeTemplates(templates []*api.Template) { for _, t := range templates { if t.ChangeMode == nil { - t.ChangeMode = pointer.Of("restart") + t.ChangeMode = pointerOf("restart") } if t.Perms == nil { - t.Perms = pointer.Of("0644") + t.Perms = pointerOf("0644") } if t.Splay == nil { - t.Splay = pointer.Of(5 * time.Second) + t.Splay = pointerOf(5 * time.Second) } if t.ErrMissingKey == nil { - t.ErrMissingKey = pointer.Of(false) + t.ErrMissingKey = pointerOf(false) } normalizeChangeScript(t.ChangeScript) } @@ -152,10 +151,10 @@ func normalizeChangeScript(ch *api.ChangeScript) { } if ch.Timeout == nil { - ch.Timeout = pointer.Of(5 * time.Second) + ch.Timeout = pointerOf(5 * time.Second) } if ch.FailOnError == nil { - ch.FailOnError = pointer.Of(false) + ch.FailOnError = pointerOf(false) } } diff --git a/jobspec2/parse_test.go b/jobspec2/parse_test.go index 6fd3072f3..bdf093d20 100644 --- a/jobspec2/parse_test.go +++ b/jobspec2/parse_test.go @@ -10,14 +10,12 @@ import ( "time" "github.com/hashicorp/nomad/api" - "github.com/hashicorp/nomad/ci" - "github.com/hashicorp/nomad/helper/pointer" "github.com/shoenig/test/must" "github.com/stretchr/testify/require" ) func TestParse_ConnectJob(t *testing.T) { - ci.Parallel(t) + t.Parallel() name := "./test-fixtures/connect-example.hcl" f, err := os.Open(name) @@ -32,7 +30,7 @@ func TestParse_ConnectJob(t *testing.T) { } func TestParse_VarsAndFunctions(t *testing.T) { - ci.Parallel(t) + t.Parallel() hcl := ` variables { @@ -58,7 +56,7 @@ job "example" { } func TestParse_VariablesDefaultsAndSet(t *testing.T) { - ci.Parallel(t) + t.Parallel() hcl := ` variables { @@ -157,7 +155,7 @@ job "example" { // TestParse_UnknownVariables asserts that unknown variables are left intact for further processing func TestParse_UnknownVariables(t *testing.T) { - ci.Parallel(t) + t.Parallel() hcl := ` variables { @@ -192,7 +190,7 @@ job "example" { // TestParse_UnsetVariables asserts that variables that have neither types nor // values return early instead of panicking. func TestParse_UnsetVariables(t *testing.T) { - ci.Parallel(t) + t.Parallel() hcl := ` variable "region_var" {} @@ -214,7 +212,7 @@ job "example" { } func TestParse_Locals(t *testing.T) { - ci.Parallel(t) + t.Parallel() hcl := ` variables { @@ -263,7 +261,7 @@ job "example" { } func TestParse_FileOperators(t *testing.T) { - ci.Parallel(t) + t.Parallel() hcl := ` job "example" { @@ -300,7 +298,7 @@ job "example" { } func TestParseDynamic(t *testing.T) { - ci.Parallel(t) + t.Parallel() hcl := ` job "example" { @@ -363,7 +361,7 @@ job "example" { } func TestParse_InvalidHCL(t *testing.T) { - ci.Parallel(t) + t.Parallel() t.Run("invalid body", func(t *testing.T) { hcl := `invalid{hcl` @@ -408,7 +406,7 @@ job "example" { } func TestParse_InvalidScalingSyntax(t *testing.T) { - ci.Parallel(t) + t.Parallel() cases := []struct { name string @@ -574,7 +572,7 @@ job "example" { } func TestParseJob_JobWithFunctionsAndLookups(t *testing.T) { - ci.Parallel(t) + t.Parallel() hcl := ` variable "env" { @@ -613,13 +611,13 @@ job "job-webserver" { { "prod", &api.Job{ - ID: pointer.Of("job-webserver"), - Name: pointer.Of("job-webserver"), + ID: pointerOf("job-webserver"), + Name: pointerOf("job-webserver"), Datacenters: []string{"prod-dc1", "prod-dc2"}, TaskGroups: []*api.TaskGroup{ { - Name: pointer.Of("group-webserver"), - Count: pointer.Of(20), + Name: pointerOf("group-webserver"), + Count: pointerOf(20), Tasks: []*api.Task{ { @@ -639,13 +637,13 @@ job "job-webserver" { { "staging", &api.Job{ - ID: pointer.Of("job-webserver"), - Name: pointer.Of("job-webserver"), + ID: pointerOf("job-webserver"), + Name: pointerOf("job-webserver"), Datacenters: []string{"dc1"}, TaskGroups: []*api.TaskGroup{ { - Name: pointer.Of("group-webserver"), - Count: pointer.Of(3), + Name: pointerOf("group-webserver"), + Count: pointerOf(3), Tasks: []*api.Task{ { @@ -665,13 +663,13 @@ job "job-webserver" { { "unknown", &api.Job{ - ID: pointer.Of("job-webserver"), - Name: pointer.Of("job-webserver"), + ID: pointerOf("job-webserver"), + Name: pointerOf("job-webserver"), Datacenters: []string{}, TaskGroups: []*api.TaskGroup{ { - Name: pointer.Of("group-webserver"), - Count: pointer.Of(0), + Name: pointerOf("group-webserver"), + Count: pointerOf(0), Tasks: []*api.Task{ { @@ -705,7 +703,7 @@ job "job-webserver" { } func TestParse_TaskEnvs(t *testing.T) { - ci.Parallel(t) + t.Parallel() cases := []struct { name string @@ -780,7 +778,7 @@ job "example" { } func TestParse_TaskEnvs_Multiple(t *testing.T) { - ci.Parallel(t) + t.Parallel() hcl := ` job "example" { @@ -806,7 +804,7 @@ job "example" { } func Test_TaskEnvs_Invalid(t *testing.T) { - ci.Parallel(t) + t.Parallel() cases := []struct { name string @@ -856,7 +854,7 @@ job "example" { } func TestParse_Meta_Alternatives(t *testing.T) { - ci.Parallel(t) + t.Parallel() hcl := ` job "example" { group "group" { @@ -904,7 +902,7 @@ func TestParse_Meta_Alternatives(t *testing.T) { } func TestParse_Constraint_Alternatives(t *testing.T) { - ci.Parallel(t) + t.Parallel() hclOpVal := ` job "example" { @@ -1006,7 +1004,7 @@ job "example" { // TestParse_UndefinedVariables asserts that values with undefined variables are left // intact in the job representation func TestParse_UndefinedVariables(t *testing.T) { - ci.Parallel(t) + t.Parallel() cases := []string{ "plain", @@ -1050,7 +1048,7 @@ func TestParse_UndefinedVariables(t *testing.T) { } func TestParseServiceCheck(t *testing.T) { - ci.Parallel(t) + t.Parallel() hcl := ` job "group_service_check_script" { group "group" { @@ -1074,11 +1072,11 @@ func TestParseServiceCheck(t *testing.T) { require.NoError(t, err) expectedJob := &api.Job{ - ID: pointer.Of("group_service_check_script"), - Name: pointer.Of("group_service_check_script"), + ID: pointerOf("group_service_check_script"), + Name: pointerOf("group_service_check_script"), TaskGroups: []*api.TaskGroup{ { - Name: pointer.Of("group"), + Name: pointerOf("group"), Services: []*api.Service{ { Name: "foo-service", @@ -1101,7 +1099,7 @@ func TestParseServiceCheck(t *testing.T) { } func TestWaitConfig(t *testing.T) { - ci.Parallel(t) + t.Parallel() hclBytes, err := os.ReadFile("test-fixtures/template-wait-config.hcl") require.NoError(t, err) @@ -1122,7 +1120,7 @@ func TestWaitConfig(t *testing.T) { } func TestErrMissingKey(t *testing.T) { - ci.Parallel(t) + t.Parallel() hclBytes, err := os.ReadFile("test-fixtures/template-err-missing-key.hcl") require.NoError(t, err) job, err := ParseWithConfig(&ParseConfig{ @@ -1138,7 +1136,7 @@ func TestErrMissingKey(t *testing.T) { } func TestRestartRenderTemplates(t *testing.T) { - ci.Parallel(t) + t.Parallel() hclBytes, err := os.ReadFile("test-fixtures/restart-render-templates.hcl") require.NoError(t, err) job, err := ParseWithConfig(&ParseConfig{ @@ -1159,7 +1157,7 @@ func TestRestartRenderTemplates(t *testing.T) { // Identities slice to the pre-1.7 Identity field in case >=1.7 CLIs are used // with <1.7 APIs. func TestIdentity(t *testing.T) { - ci.Parallel(t) + t.Parallel() hclBytes, err := os.ReadFile("test-fixtures/identity-compat.nomad.hcl") must.NoError(t, err) job, err := ParseWithConfig(&ParseConfig{ diff --git a/jobspec2/util.go b/jobspec2/util.go new file mode 100644 index 000000000..6b8706e49 --- /dev/null +++ b/jobspec2/util.go @@ -0,0 +1,10 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package jobspec2 + +// pointerOf returns a pointer to "a". It is duplicated from the helper package +// to isolate the jobspec2 package from the rest of Nomad. +func pointerOf[A any](a A) *A { + return &a +}