VyOS v1.4+ chronyd conf support #357
Conversation
| $""", | ||
| re.VERBOSE, | ||
| ), | ||
| "setval": "system ntp allow-clients address {{allow_clients}}", | ||
| "setval": "path ntp allow-clients address {{allow_clients}}", |
There was a problem hiding this comment.
Do these path references need to be {{path}}? I wouldn’t expect the variable substitution to work without it.
There was a problem hiding this comment.
I am still working on more elegant way of abstraction version discrepancies, so I appreciate there might be a better way. Meantime, this is a placeholder, so I can then replace path by either 'system' or 'service' in the resultant self.commands list - keep looking though
There was a problem hiding this comment.
Yes, I figured, was just wondering if you wanted to use the replacement tool that was already being used to replace the path.
There was a problem hiding this comment.
I may have missed in myriad of lines of code - can you point me to any existing solutions you, guys, already have? thanks
There was a problem hiding this comment.
If I am to speculate and preempt here, and the replacement tool is something that I coded, then {{path}}? is getval part and required for changed status checks, otherwise, if only the replacement is left, then it will always be changed = true
There was a problem hiding this comment.
Do these
pathreferences need to be{{path}}? I wouldn’t expect the variable substitution to work without it.
No, it did not work for me - that said, I am still checking if the solution can be down purely in the parser code
There was a problem hiding this comment.
Correct, it’s not a mechanism that will pull directly from the current namespace, you would need to insert the path into the dictionary that is being rendered, which might be tricky to find the right place. There may not be a suitable way to inject the data into the underlying render code.
If not, I’d recommend that you consider something a bit more distinct, maybe “%%path%%” as the token you’re trying to replace in order to ensure that the code doesn’t replace a portion of an argument (such as a hostname) and create an unexpected result if an NTP host is named “tick.empath.com” for example. That would become “tick.emsystem.com” or “tick.enservice.com” using the current code.
So
"setval": "path ntp allow-clients address {{allow_clients}}",
Would become:
"setval": "%%path%% ntp allow-clients address {{allow_clients}}",
And make the corresponding change in the replace operation you added.
|
I’ve not touched any of the “new-style” parser modules, so interested to see how this works. I put a comment in about the |
Change Summary
Types of changes
Related Task(s)
Related PR(s)
Proposed changes
How to test
Checklist: