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

template: error on missing key #14002

Closed
wants to merge 14 commits into from

Conversation

DerekStrickland
Copy link
Contributor

@DerekStrickland DerekStrickland commented Aug 4, 2022

This PR adds the consul-template configuration option error_on_missing_key. This allows jobspec authors to specify that a template should fail if it references a struct or map key that does not exist. The default value is false and should be fully backward compatible.

jobspec2/parse_test.go Outdated Show resolved Hide resolved
Comment on lines 80 to 81
- `error_on_missing_key` `(bool: false)` - Specifies how the template behaves
when attempting to index a struct or map key that does not exist.
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be good to expand on what happens for each value, as the naming can be a bit misleading. Something like this perhaps (not sure if I got it right 😅):

If set to true the task will fail if a template tries to access a key that doesn't in a map , if false an empty value will be used in the template instead.

api/tasks.go Show resolved Hide resolved
@@ -2742,6 +2743,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
Min: helper.TimeToPtr(5 * time.Second),
Max: helper.TimeToPtr(10 * time.Second),
},
ErrMissingKey: pointer.Of[bool](true),
Copy link
Contributor

Choose a reason for hiding this comment

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

This will cause the backport to fail like in the other PR. Since all the other tests use the helper package already it may be good to avoid pointer here for now to get a clean merge and save some manual work.

@lgfa29 lgfa29 modified the milestones: 1.3.3, 1.3.x Aug 4, 2022
@mmcquillan mmcquillan removed this from the 1.3.x milestone Aug 31, 2022
@rodrigol-chan
Copy link

Thank you for working on this, @DerekStrickland! I've been bitten by this issue. Do you plan on working on this? If not, I can pick this up.

Co-authored-by: Luiz Aoqui <luiz@hashicorp.com>
Co-authored-by: Derek Strickland <1111455+DerekStrickland@users.noreply.github.com>
@lgfa29
Copy link
Contributor

lgfa29 commented Nov 1, 2022

Thank you for working on this, @DerekStrickland! I've been bitten by this issue. Do you plan on working on this? If not, I can pick this up.

Hi @rodrigol-chan 👋

Could you explain the scenario you had where this would be useful?

We paused work on this because this consul-template config didn't actually do what we expect it to do.

@rodrigol-chan
Copy link

I'm using the Vault integration to template some secrets into a file. Unfortunately, I forgot to add all of the key-value pairs and instead of a nice Nomad error telling me a key-value pair does not exist, I had to figure out that <no value> is being produced instead. It wasn't entirely obvious because the secret is salted and hashed, and only then put into a file, but it would be nice to have an explicit failure.

As I understand it, if the whole secret does not exist, it already fails to launch the task. But if a key in the secret does not exist, it's silently templated as the wrong value.

@angrycub
Copy link
Contributor

angrycub commented Nov 4, 2022

Closing in favor of #15141

@github-actions
Copy link

github-actions bot commented Mar 4, 2023

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 4, 2023
@angrycub angrycub deleted the f-template-err-on-missing-key branch February 15, 2024 16:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants