jinja: provide and document jinja-safe key aliases in instance-data (SC-622)#1123
Merged
Conversation
blackboxsw
changed the title
blackboxsw
force-pushed
the
lxd-ds
branch
3 times, most recently
from
fbd3394 to
f6cf805
Compare
TheRealFalcon
suggested changes
Dec 2, 2021
Contributor
TheRealFalcon
left a comment
TheRealFalcon
left a comment
There was a problem hiding this comment.
Looks good overall! I left mostly minor comments inline with the only major thing being removing the unused parameter.
| break | ||
| if walked_key_path: | ||
| walked_key_path += "." | ||
| walked_key_path += key_path_part |
Contributor
There was a problem hiding this comment.
I think this section is hard to follow in a function that's already fairly long. It might help to break it down some or maybe even just add some more comments, but I don't think it needs to block merging.
Collaborator
Author
There was a problem hiding this comment.
118 line long function... I've done worse, but could do a lot better. breaking this down into a couple of separate functions for readability.
TheRealFalcon
approved these changes
Dec 2, 2021
Contributor
TheRealFalcon
left a comment
TheRealFalcon
left a comment
There was a problem hiding this comment.
I'll leave the existing comments up to you, but I think this is good as-is.
Without a leading non-integer character jinja's lexer gets confused trying to parse underscore-delimited variable names such as 1_0 as the lexer starts to interpret an integer and chokes on the underscore. This prevents cloud-init from providing a simple jinja dot-notation variable alias such as ds.1_0 because the lexer barfs claiminig invalid variable end "_0". Providing the leading character allows the lexer to interpret the variable as a string type and properly interpret v1_0 as the whole variable text.
Provide additional aliased keys in instance-data.json which have replaced any jinja operator characters in key names with underscores. This provides a simple mechanism for "unsafe" key names to be used in #cloud-config user-data or cloud-init query commands using jinja's dot-notation for dict attributes versus the more verbose square-backet accessor and quoting the unsafe key name. For example: ds.1_0.config.user_data instead of ds["1.0"].config["user-data"]
Accept a include_key_aliases param to convert_jinja_instance_data. When true, add key aliases to the instance-data dict returned. Hide jinja variable aliases from simple cloud-init query --all but Use aliases when rendering jinja template files from query --format command or #cloud-config userdata.
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Commit Message
Additional Context
Test Steps
Checklist: