Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update consul-template to v0.24.1 and remove deprecated vault grace #7170

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
IMPROVEMENTS:

* consul: Added support for configuring `enable_tag_override` on service stanzas. [[GH-2057](https://github.com/hashicorp/nomad/issues/2057)]
* client: Updated consul-template library to v0.24.1 - added support for working with consul connect. [Deprecated vault_grace](https://nomadproject.io/guides/upgrade/upgrade-specific/#nomad-0110) [[GH-7170](https://github.com/hashicorp/nomad/pull/7170)]

BUG FIXES:

Expand Down
3 changes: 0 additions & 3 deletions api/jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,6 @@ func TestJobs_Canonicalize(t *testing.T) {
EmbeddedTmpl: stringToPtr("FOO=bar\n"),
DestPath: stringToPtr("local/file.env"),
Envvars: boolToPtr(true),
VaultGrace: timeToPtr(3 * time.Second),
},
},
},
Expand Down Expand Up @@ -532,7 +531,6 @@ func TestJobs_Canonicalize(t *testing.T) {
LeftDelim: stringToPtr("{{"),
RightDelim: stringToPtr("}}"),
Envvars: boolToPtr(false),
VaultGrace: timeToPtr(15 * time.Second),
},
{
SourcePath: stringToPtr(""),
Expand All @@ -545,7 +543,6 @@ func TestJobs_Canonicalize(t *testing.T) {
LeftDelim: stringToPtr("{{"),
RightDelim: stringToPtr("}}"),
Envvars: boolToPtr(true),
VaultGrace: timeToPtr(3 * time.Second),
},
},
},
Expand Down
3 changes: 0 additions & 3 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -750,9 +750,6 @@ func (tmpl *Template) Canonicalize() {
if tmpl.Envvars == nil {
tmpl.Envvars = boolToPtr(false)
}
if tmpl.VaultGrace == nil {
tmpl.VaultGrace = timeToPtr(15 * time.Second)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this is probably the source of panics/issues. It ensured VaultGrace was always initialized (never nil) in the HTTP API.

Removing this code is the right thing to do. We just need to fix any code that depended on VaultGrace always being non-nil. I'll investigate and try to help.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry haven't found time today to fix this up. The really sneaky place that converts public api structs to internal nomad/structs structs is hidden in the job endpoint here: https://github.com/hashicorp/nomad/blob/v0.10.4/command/agent/job_endpoint.go#L620

That may need updating to convert nil api VaultGrace values to 0 nomad/struct VaultGrace values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment below.

}
}

type Vault struct {
Expand Down
12 changes: 0 additions & 12 deletions client/allocrunner/taskrunner/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -611,17 +611,6 @@ func newRunnerConfig(config *TaskTemplateManagerConfig,
}
conf.Templates = &flat

// Go through the templates and determine the minimum Vault grace
vaultGrace := time.Duration(-1)
for _, tmpl := range templateMapping {
// Initial condition
if vaultGrace < 0 {
vaultGrace = tmpl.VaultGrace
} else if tmpl.VaultGrace < vaultGrace {
vaultGrace = tmpl.VaultGrace
}
}

// Force faster retries
if config.retryRate != 0 {
rate := config.retryRate
Expand Down Expand Up @@ -666,7 +655,6 @@ func newRunnerConfig(config *TaskTemplateManagerConfig,
if cc.VaultConfig != nil && cc.VaultConfig.IsEnabled() {
conf.Vault.Address = &cc.VaultConfig.Addr
conf.Vault.Token = &config.VaultToken
conf.Vault.Grace = helper.TimeToPtr(vaultGrace)
if config.ClientConfig.VaultConfig.Namespace != "" {
conf.Vault.Namespace = &config.ClientConfig.VaultConfig.Namespace
}
Expand Down
46 changes: 0 additions & 46 deletions client/allocrunner/taskrunner/template/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1340,51 +1340,6 @@ func TestTaskTemplateManager_Config_ServerName(t *testing.T) {
}
}

// TestTaskTemplateManager_Config_VaultGrace asserts the vault_grace setting is
// propagated to consul-template's configuration.
func TestTaskTemplateManager_Config_VaultGrace(t *testing.T) {
t.Parallel()
assert := assert.New(t)
c := config.DefaultConfig()
c.Node = mock.Node()
c.VaultConfig = &sconfig.VaultConfig{
Enabled: helper.BoolToPtr(true),
Addr: "https://localhost/",
TLSServerName: "notlocalhost",
}

alloc := mock.Alloc()
config := &TaskTemplateManagerConfig{
ClientConfig: c,
VaultToken: "token",

// Make a template that will render immediately
Templates: []*structs.Template{
{
EmbeddedTmpl: "bar",
DestPath: "foo",
ChangeMode: structs.TemplateChangeModeNoop,
VaultGrace: 10 * time.Second,
},
{
EmbeddedTmpl: "baz",
DestPath: "bam",
ChangeMode: structs.TemplateChangeModeNoop,
VaultGrace: 100 * time.Second,
},
},
EnvBuilder: taskenv.NewBuilder(c.Node, alloc, alloc.Job.TaskGroups[0].Tasks[0], c.Region),
}

ctmplMapping, err := parseTemplateConfigs(config)
assert.Nil(err, "Parsing Templates")

ctconf, err := newRunnerConfig(config, ctmplMapping)
assert.Nil(err, "Building Runner Config")
assert.NotNil(ctconf.Vault.Grace, "Vault Grace Pointer")
assert.Equal(10*time.Second, *ctconf.Vault.Grace, "Vault Grace Value")
}

// TestTaskTemplateManager_Config_VaultNamespace asserts the Vault namespace setting is
// propagated to consul-template's configuration.
func TestTaskTemplateManager_Config_VaultNamespace(t *testing.T) {
Expand Down Expand Up @@ -1413,7 +1368,6 @@ func TestTaskTemplateManager_Config_VaultNamespace(t *testing.T) {

ctconf, err := newRunnerConfig(config, ctmplMapping)
assert.Nil(err, "Building Runner Config")
assert.NotNil(ctconf.Vault.Grace, "Vault Grace Pointer")
assert.Equal(testNS, *ctconf.Vault.Namespace, "Vault Namespace Value")
}

Expand Down
2 changes: 0 additions & 2 deletions command/agent/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1706,7 +1706,6 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
LeftDelim: helper.StringToPtr("abc"),
RightDelim: helper.StringToPtr("def"),
Envvars: helper.BoolToPtr(true),
VaultGrace: helper.TimeToPtr(3 * time.Second),
},
},
DispatchPayload: &api.DispatchPayloadConfig{
Expand Down Expand Up @@ -2054,7 +2053,6 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
LeftDelim: "abc",
RightDelim: "def",
Envvars: true,
VaultGrace: 3 * time.Second,
},
},
DispatchPayload: &structs.DispatchPayloadConfig{
Expand Down
3 changes: 2 additions & 1 deletion jobspec/parse_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ func parseTemplates(result *[]*api.Template, list *ast.ObjectList) error {
"source",
"splay",
"env",
"vault_grace",
"vault_grace", //COMPAT(0.12) not used; emits warning in 0.11.
}
if err := helper.CheckHCLKeys(o.Val, valid); err != nil {
return err
Expand All @@ -374,6 +374,7 @@ func parseTemplates(result *[]*api.Template, list *ast.ObjectList) error {
ChangeMode: helper.StringToPtr("restart"),
Splay: helper.TimeToPtr(5 * time.Second),
Perms: helper.StringToPtr("0644"),
VaultGrace: helper.TimeToPtr(0),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added default value in parser. Time.period can not be nil.
This allows for silent parsing when vault_grace is not set or set to 0. Warning when set to a non zero value.

}

dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
Expand Down
1 change: 0 additions & 1 deletion jobspec/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,6 @@ func TestParse(t *testing.T) {
Splay: helper.TimeToPtr(10 * time.Second),
Perms: helper.StringToPtr("0644"),
Envvars: helper.BoolToPtr(true),
VaultGrace: helper.TimeToPtr(33 * time.Second),
},
{
SourcePath: helper.StringToPtr("bar"),
Expand Down
3 changes: 2 additions & 1 deletion jobspec/test-fixtures/service-enable-tag-override.hcl
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
job "service_eto" {
type = "service"

group "group" {
task "task" {
service {
name = "example"
name = "example"
enable_tag_override = true
}
}
Expand Down
2 changes: 1 addition & 1 deletion jobspec/test-fixtures/tg-service-enable-tag-override.hcl
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
job "group_service_eto" {
group "group" {
service {
name = "example"
name = "example"
enable_tag_override = true
}
}
Expand Down
16 changes: 0 additions & 16 deletions nomad/structs/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5666,7 +5666,6 @@ func TestTaskDiff(t *testing.T) {
ChangeSignal: "SIGHUP",
Splay: 1,
Perms: "0644",
VaultGrace: 3 * time.Second,
},
{
SourcePath: "foo2",
Expand All @@ -5677,7 +5676,6 @@ func TestTaskDiff(t *testing.T) {
Splay: 2,
Perms: "0666",
Envvars: true,
VaultGrace: 5 * time.Second,
},
},
},
Expand All @@ -5691,7 +5689,6 @@ func TestTaskDiff(t *testing.T) {
ChangeSignal: "SIGHUP",
Splay: 1,
Perms: "0644",
VaultGrace: 3 * time.Second,
},
{
SourcePath: "foo3",
Expand All @@ -5701,7 +5698,6 @@ func TestTaskDiff(t *testing.T) {
ChangeSignal: "SIGHUP3",
Splay: 3,
Perms: "0776",
VaultGrace: 10 * time.Second,
},
},
},
Expand Down Expand Up @@ -5760,12 +5756,6 @@ func TestTaskDiff(t *testing.T) {
Old: "",
New: "3",
},
{
Type: DiffTypeAdded,
Name: "VaultGrace",
Old: "",
New: "10000000000",
},
},
},
{
Expand Down Expand Up @@ -5820,12 +5810,6 @@ func TestTaskDiff(t *testing.T) {
Old: "2",
New: "",
},
{
Type: DiffTypeDeleted,
Name: "VaultGrace",
Old: "5000000000",
New: "",
},
},
},
},
Expand Down
23 changes: 19 additions & 4 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -5829,6 +5829,24 @@ func (t *Task) Warnings() error {
mErr.Errors = append(mErr.Errors, fmt.Errorf("IOPS has been deprecated as of Nomad 0.9.0. Please remove IOPS from resource stanza."))
}

for idx, tmpl := range t.Templates {
if err := tmpl.Warnings(); err != nil {
err = multierror.Prefix(err, fmt.Sprintf("Template[%d]", idx))
mErr.Errors = append(mErr.Errors, err)
}
}

return mErr.ErrorOrNil()
}

func (t *Template) Warnings() error {
var mErr multierror.Error

// Deprecation notice for vault_grace
if t.VaultGrace > 0 {
mErr.Errors = append(mErr.Errors, fmt.Errorf("VaultGrace has been deprecated as of Nomad 0.11 and ignored since Vault 0.5. Please remove VaultGrace / vault_grace from template stanza."))
}

return mErr.ErrorOrNil()
}

Expand Down Expand Up @@ -5964,6 +5982,7 @@ type Template struct {
// VaultGrace is the grace duration between lease renewal and reacquiring a
// secret. If the lease of a secret is less than the grace, a new secret is
// acquired.
// COMPAT(0.12) VaultGrace has been ignored by Vault since Vault v0.5.
VaultGrace time.Duration
fredrikhgrelland marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down Expand Up @@ -6038,10 +6057,6 @@ func (t *Template) Validate() error {
}
}

if t.VaultGrace.Nanoseconds() < 0 {
fredrikhgrelland marked this conversation as resolved.
Show resolved Hide resolved
multierror.Append(&mErr, fmt.Errorf("Vault grace must be greater than zero: %v < 0", t.VaultGrace))
}

return mErr.ErrorOrNil()
}

Expand Down
31 changes: 31 additions & 0 deletions vendor/github.com/hashicorp/consul-template/CHANGELOG.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions vendor/github.com/hashicorp/consul-template/Makefile

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading