From c80c60965f6a99a7b9f4570a40067efb0af20076 Mon Sep 17 00:00:00 2001 From: James Rasell Date: Wed, 24 Sep 2025 14:20:34 +0100 Subject: [PATCH] node pool: Allow specifying node identity ttl in HCL or JSON spec. (#26825) The node identity TTL defaults to 24hr but can be altered by setting the node identity TTL parameter. In order to allow setting and viewing the value, the field is now plumbed through the CLI and HTTP API. In order to parse the HCL, a new helper package has been created which contains generic parsing and decoding functionality for dealing with HCL that contains time durations. hclsimple can be used when this functionality is not needed. In order to parse the JSON, custom marshal and unmarshal functions have been created as used in many other places. The node pool init command has been updated to include this new parameter, although commented out, so reference. The info command now includes the TTL in its output too. --- api/node_pools.go | 50 ++++++++++++++++++++++ api/node_pools_test.go | 8 +++- command/asset/pool.nomad.hcl | 6 +++ command/asset/pool.nomad.json | 1 + command/node_pool_apply.go | 8 +++- command/node_pool_apply_test.go | 8 +++- command/node_pool_info.go | 8 +++- command/node_pool_info_test.go | 21 ++++------ command/node_pool_list_test.go | 9 +--- helper/hcl/decode.go | 63 ++++++++++++++++++++++++++++ helper/hcl/parse.go | 49 ++++++++++++++++++++++ helper/hcl/parse_test.go | 74 +++++++++++++++++++++++++++++++++ nomad/mock/mock.go | 2 +- nomad/structs/node_pool.go | 31 ++++++++++++++ 14 files changed, 311 insertions(+), 27 deletions(-) create mode 100644 helper/hcl/decode.go create mode 100644 helper/hcl/parse.go create mode 100644 helper/hcl/parse_test.go diff --git a/api/node_pools.go b/api/node_pools.go index d3f18bd87..8fd101de1 100644 --- a/api/node_pools.go +++ b/api/node_pools.go @@ -4,9 +4,11 @@ package api import ( + "encoding/json" "errors" "fmt" "net/url" + "time" ) const ( @@ -118,11 +120,59 @@ type NodePool struct { Name string `hcl:"name,label"` Description string `hcl:"description,optional"` Meta map[string]string `hcl:"meta,block"` + NodeIdentityTTL time.Duration `hcl:"node_identity_ttl,optional"` SchedulerConfiguration *NodePoolSchedulerConfiguration `hcl:"scheduler_config,block"` CreateIndex uint64 ModifyIndex uint64 } +// MarshalJSON implements the json.Marshaler interface and allows +// NodePool.NodeIdentityTTL to be marshaled correctly. +func (n *NodePool) MarshalJSON() ([]byte, error) { + type Alias NodePool + exported := &struct { + NodeIdentityTTL string + *Alias + }{ + NodeIdentityTTL: n.NodeIdentityTTL.String(), + Alias: (*Alias)(n), + } + if n.NodeIdentityTTL == 0 { + exported.NodeIdentityTTL = "" + } + return json.Marshal(exported) +} + +// UnmarshalJSON implements the json.Unmarshaler interface and allows +// NodePool.NodeIdentityTTL to be unmarshalled correctly. +func (n *NodePool) UnmarshalJSON(data []byte) (err error) { + type Alias NodePool + aux := &struct { + NodeIdentityTTL any + *Alias + }{ + Alias: (*Alias)(n), + } + + if err = json.Unmarshal(data, &aux); err != nil { + return err + } + if aux.NodeIdentityTTL != nil { + switch v := aux.NodeIdentityTTL.(type) { + case string: + if v != "" { + if n.NodeIdentityTTL, err = time.ParseDuration(v); err != nil { + return err + } + } + case float64: + n.NodeIdentityTTL = time.Duration(v) + } + + } + return nil +} + // NodePoolSchedulerConfiguration is used to serialize the scheduler // configuration of a node pool. type NodePoolSchedulerConfiguration struct { diff --git a/api/node_pools_test.go b/api/node_pools_test.go index bd8d81cef..e60596b8d 100644 --- a/api/node_pools_test.go +++ b/api/node_pools_test.go @@ -5,6 +5,7 @@ package api import ( "testing" + "time" "github.com/hashicorp/nomad/api/internal/testutil" "github.com/shoenig/test/must" @@ -139,7 +140,10 @@ func TestNodePools_Register(t *testing.T) { // Create test node pool. t.Run("create and update node pool", func(t *testing.T) { - dev1 := &NodePool{Name: "dev-1"} + dev1 := &NodePool{ + Name: "dev-1", + NodeIdentityTTL: 720 * time.Hour, + } _, err := nodePools.Register(dev1, nil) must.NoError(t, err) @@ -147,6 +151,7 @@ func TestNodePools_Register(t *testing.T) { got, _, err := nodePools.Info(dev1.Name, nil) must.NoError(t, err) must.Eq(t, dev1.Name, got.Name) + must.Eq(t, dev1.NodeIdentityTTL, got.NodeIdentityTTL) // Update test node pool. dev1.Description = "test" @@ -158,6 +163,7 @@ func TestNodePools_Register(t *testing.T) { must.NoError(t, err) must.Eq(t, dev1.Name, got.Name) must.Eq(t, dev1.Description, got.Description) + must.Eq(t, dev1.NodeIdentityTTL, got.NodeIdentityTTL) }) t.Run("missing node pool", func(t *testing.T) { diff --git a/command/asset/pool.nomad.hcl b/command/asset/pool.nomad.hcl index d242d97ae..27da8d0c2 100644 --- a/command/asset/pool.nomad.hcl +++ b/command/asset/pool.nomad.hcl @@ -9,6 +9,12 @@ node_pool "example" { owner = "sre" } + # node identity TTL is an optional parameter that sets the TTL for all node + # identities issued to nodes in this node pool. The value must be a valid + # duration string (e.g. "30m", "1h", "24h"). If not set, the default value is + # "24h". + # node_identity_ttl = "24h" + # The scheduler configuration options specific to this node pool. This block # supports a subset of the fields supported in the global scheduler # configuration as described at: diff --git a/command/asset/pool.nomad.json b/command/asset/pool.nomad.json index 16d205f26..179b5d4fb 100644 --- a/command/asset/pool.nomad.json +++ b/command/asset/pool.nomad.json @@ -5,6 +5,7 @@ "environment": "prod", "owner": "sre" }, + "NodeIdentityTTL": "24h", "SchedulerConfiguration": { "SchedulerAlgorithm": "spread", "MemoryOversubscriptionEnabled": true diff --git a/command/node_pool_apply.go b/command/node_pool_apply.go index 126609aab..1787646fa 100644 --- a/command/node_pool_apply.go +++ b/command/node_pool_apply.go @@ -10,8 +10,8 @@ import ( "os" "strings" - "github.com/hashicorp/hcl/v2/hclsimple" "github.com/hashicorp/nomad/api" + "github.com/hashicorp/nomad/helper/hcl" "github.com/posener/complete" ) @@ -112,7 +112,11 @@ func (c *NodePoolApplyCommand) Run(args []string) int { if jsonInput { err = json.Unmarshal(content, &poolSpec.NodePool) } else { - err = hclsimple.Decode(path, content, nil, &poolSpec) + hclParser := hcl.NewParser() + + if hclDiags := hclParser.Parse(content, &poolSpec, path); hclDiags.HasErrors() { + err = hclDiags + } } if err != nil { c.Ui.Error(fmt.Sprintf("Failed to parse input content: %v", err)) diff --git a/command/node_pool_apply_test.go b/command/node_pool_apply_test.go index fbd9307e8..03f5b8d3a 100644 --- a/command/node_pool_apply_test.go +++ b/command/node_pool_apply_test.go @@ -9,6 +9,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/hashicorp/cli" "github.com/hashicorp/nomad/ci" @@ -59,7 +60,8 @@ node_pool "dev" { file.Seek(0, 0) hclTestFile = ` node_pool "dev" { - description = "dev node pool" + description = "dev node pool" + node_identity_ttl = "720h" meta { test = "true" @@ -78,12 +80,14 @@ node_pool "dev" { must.NotNil(t, got) must.NotNil(t, got.Meta) must.Eq(t, "true", got.Meta["test"]) + must.Eq(t, 720*time.Hour, got.NodeIdentityTTL) // Create node pool with JSON file. jsonTestFile := ` { "Name": "prod", - "Description": "prod node pool" + "Description": "prod node pool", + "NodeIdentityTTL": "720h" }` file, err = os.CreateTemp(t.TempDir(), "node-pool-test-*.json") diff --git a/command/node_pool_info.go b/command/node_pool_info.go index b3f18c90e..747be6d8a 100644 --- a/command/node_pool_info.go +++ b/command/node_pool_info.go @@ -110,11 +110,17 @@ func (c *NodePoolInfoCommand) Run(args []string) int { return 0 } - // Print node pool information. + // Print node pool information and conditionally handle the node identity + // ttl, so we don't print if we are not talking to a version of Nomad + // that supports it. basic := []string{ fmt.Sprintf("Name|%s", pool.Name), fmt.Sprintf("Description|%s", pool.Description), } + + if pool.NodeIdentityTTL > 0 { + basic = append(basic, fmt.Sprintf("Node Identity TTL|%s", pool.NodeIdentityTTL)) + } c.Ui.Output(formatKV(basic)) c.Ui.Output(c.Colorize().Color("\n[bold]Metadata[reset]")) diff --git a/command/node_pool_info_test.go b/command/node_pool_info_test.go index 186b5c5d9..5ab22f8ab 100644 --- a/command/node_pool_info_test.go +++ b/command/node_pool_info_test.go @@ -6,6 +6,7 @@ package command import ( "strings" "testing" + "time" "github.com/hashicorp/cli" "github.com/hashicorp/nomad/api" @@ -35,13 +36,15 @@ func TestNodePoolInfoCommand_Run(t *testing.T) { Meta: map[string]string{ "env": "test", }, + NodeIdentityTTL: 720 * time.Hour, } _, err := client.NodePools().Register(dev1, nil) must.NoError(t, err) dev1Output := ` -Name = dev-1 -Description = Test pool +Name = dev-1 +Description = Test pool +Node Identity TTL = 720h0m0s Metadata env = test @@ -50,14 +53,7 @@ Scheduler Configuration No scheduler configuration` dev1JsonOutput := ` -{ - "Description": "Test pool", - "Meta": { - "env": "test" - }, - "Name": "dev-1", - "SchedulerConfiguration": null -}` +"NodeIdentityTTL":"720h0m0s","Name":"dev-1","Description":"Test pool","Meta":{"env":"test"},"SchedulerConfiguration":null` // These two node pools are used to test exact prefix match. prod1 := &api.NodePool{Name: "prod-1"} @@ -91,8 +87,9 @@ No scheduler configuration` name: "exact prefix match", args: []string{"prod-1"}, expectedOut: ` -Name = prod-1 -Description = +Name = prod-1 +Description = +Node Identity TTL = 24h0m0s Metadata No metadata diff --git a/command/node_pool_list_test.go b/command/node_pool_list_test.go index 0fe6ec9b2..c25fc4e0c 100644 --- a/command/node_pool_list_test.go +++ b/command/node_pool_list_test.go @@ -101,14 +101,7 @@ prod-1 `, "-filter", `Name == "prod-1"`, }, expectedOut: ` -[ - { - "Description": "", - "Meta": null, - "Name": "prod-1", - "SchedulerConfiguration": null - } -]`, +"NodeIdentityTTL":"24h0m0s","Name":"prod-1","Description":"","Meta":null,"SchedulerConfiguration":null`, expectedCode: 0, }, { diff --git a/helper/hcl/decode.go b/helper/hcl/decode.go new file mode 100644 index 000000000..c0c352435 --- /dev/null +++ b/helper/hcl/decode.go @@ -0,0 +1,63 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package hcl + +import ( + "fmt" + "time" + + "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/gocty" +) + +// DecodeDuration is the decode function for time.Duration types. It supports +// both string and numeric values. String values are parsed using +// time.ParseDuration. Numeric values are expected to be in nanoseconds. +// +// This function duplicates that found within the jobspec2 package to avoid +// license conflicts. +func DecodeDuration(expr hcl.Expression, ctx *hcl.EvalContext, val any) hcl.Diagnostics { + srcVal, diags := expr.Value(ctx) + + if srcVal.Type() == cty.String { + dur, err := time.ParseDuration(srcVal.AsString()) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unsuitable value type", + Detail: fmt.Sprintf("Unsuitable duration value: %s", err.Error()), + Subject: expr.StartRange().Ptr(), + Context: expr.Range().Ptr(), + }) + return diags + } + + srcVal = cty.NumberIntVal(int64(dur)) + } + + if srcVal.Type() != cty.Number { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unsuitable value type", + Detail: fmt.Sprintf("Unsuitable value: expected a string but found %s", srcVal.Type()), + Subject: expr.StartRange().Ptr(), + Context: expr.Range().Ptr(), + }) + return diags + } + + err := gocty.FromCtyValue(srcVal, val) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unsuitable value type", + Detail: fmt.Sprintf("Unsuitable value: %s", err.Error()), + Subject: expr.StartRange().Ptr(), + Context: expr.Range().Ptr(), + }) + } + + return diags +} diff --git a/helper/hcl/parse.go b/helper/hcl/parse.go new file mode 100644 index 000000000..7012a3d0b --- /dev/null +++ b/helper/hcl/parse.go @@ -0,0 +1,49 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package hcl + +import ( + "reflect" + "time" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/gohcl" + "github.com/hashicorp/hcl/v2/hclparse" +) + +type Parser struct { + parser *hclparse.Parser + decoder *gohcl.Decoder +} + +// NewParser returns a new Parser instance which supports decoding time.Duration +// parameters by default. +func NewParser() *Parser { + + // Create our base decoder, so we can register custom decoders on it. + decoder := &gohcl.Decoder{} + + // Register default custom decoders here which currently only includes + // time.Duration parsing. + dur := time.Duration(0) + decoder.RegisterExpressionDecoder(reflect.TypeOf(dur), DecodeDuration) + decoder.RegisterExpressionDecoder(reflect.TypeOf(&dur), DecodeDuration) + + return &Parser{ + decoder: decoder, + parser: hclparse.NewParser(), + } +} + +func (p *Parser) Parse(src []byte, dst any, filename string) hcl.Diagnostics { + + hclFile, parseDiag := p.parser.ParseHCL(src, filename) + + if parseDiag.HasErrors() { + return parseDiag + } + + decodeDiag := p.decoder.DecodeBody(hclFile.Body, nil, dst) + return decodeDiag +} diff --git a/helper/hcl/parse_test.go b/helper/hcl/parse_test.go new file mode 100644 index 000000000..d73917bdd --- /dev/null +++ b/helper/hcl/parse_test.go @@ -0,0 +1,74 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package hcl + +import ( + "testing" + "time" + + "github.com/shoenig/test/must" +) + +func TestParser_Parse_Duration(t *testing.T) { + + type testConfig struct { + Interval time.Duration `hcl:"interval"` + Timeout *time.Duration `hcl:"timeout,optional"` + } + + t.Run("string durations", func(t *testing.T) { + src := ` +interval = "5s" +timeout = "2m" +` + var parsedConfig testConfig + p := NewParser() + + diags := p.Parse([]byte(src), &parsedConfig, "durations.hcl") + must.False(t, diags.HasErrors()) + must.Eq(t, 5*time.Second, parsedConfig.Interval) + must.Eq(t, 2*time.Minute, *parsedConfig.Timeout) + }) + + t.Run("numeric durations (nanoseconds)", func(t *testing.T) { + // 5s and 2m expressed directly in nanoseconds + src := ` +interval = 5000000000 +timeout = 120000000000 +` + var parsedConfig testConfig + p := NewParser() + + diags := p.Parse([]byte(src), &parsedConfig, "numeric.hcl") + must.False(t, diags.HasErrors()) + must.Eq(t, 5*time.Second, parsedConfig.Interval) + must.Eq(t, 2*time.Minute, *parsedConfig.Timeout) + }) + + t.Run("invalid duration string", func(t *testing.T) { + src := ` + interval = "notaduration" + ` + var parsedConfig testConfig + p := NewParser() + + diags := p.Parse([]byte(src), &parsedConfig, "invalid_string.hcl") + must.True(t, diags.HasErrors()) + must.Len(t, 1, diags.Errs()) + must.StrContains(t, diags.Error(), "Unsuitable duration value") + }) + + t.Run("wrong type", func(t *testing.T) { + src := ` + interval = true + ` + var parsedConfig testConfig + p := NewParser() + + diags := p.Parse([]byte(src), &parsedConfig, "wrong_type.hcl") + must.True(t, diags.HasErrors()) + must.Len(t, 1, diags.Errs()) + must.StrContains(t, diags.Error(), "Unsuitable value: expected a string") + }) +} diff --git a/nomad/mock/mock.go b/nomad/mock/mock.go index cba6cb07b..10d5bcf21 100644 --- a/nomad/mock/mock.go +++ b/nomad/mock/mock.go @@ -248,7 +248,7 @@ func NodePool() *structs.NodePool { pool := &structs.NodePool{ Name: fmt.Sprintf("pool-%s", uuid.Short()), Description: "test node pool", - NodeIdentityTTL: 24 * time.Hour, + NodeIdentityTTL: 720 * time.Hour, Meta: map[string]string{"team": "test"}, } pool.SetHash() diff --git a/nomad/structs/node_pool.go b/nomad/structs/node_pool.go index 8d5138127..65d941cfa 100644 --- a/nomad/structs/node_pool.go +++ b/nomad/structs/node_pool.go @@ -4,6 +4,7 @@ package structs import ( + "encoding/json" "fmt" "maps" "regexp" @@ -207,6 +208,36 @@ func (n *NodePool) SetHash() []byte { return hashVal } +// UnmarshalJSON implements the json.Unmarshaler interface and allows +// NodePool.NodeIdentityTTL to be unmarshalled correctly. +func (n *NodePool) UnmarshalJSON(data []byte) (err error) { + type Alias NodePool + aux := &struct { + NodeIdentityTTL any + *Alias + }{ + Alias: (*Alias)(n), + } + + if err = json.Unmarshal(data, &aux); err != nil { + return err + } + if aux.NodeIdentityTTL != nil { + switch v := aux.NodeIdentityTTL.(type) { + case string: + if v != "" { + if n.NodeIdentityTTL, err = time.ParseDuration(v); err != nil { + return err + } + } + case float64: + n.NodeIdentityTTL = time.Duration(v) + } + + } + return nil +} + // NodePoolSchedulerConfiguration is the scheduler configuration applied to a // node pool. //