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

Enhancing/validating config templates #228

Closed
jonct opened this issue Jul 9, 2024 · 9 comments
Closed

Enhancing/validating config templates #228

jonct opened this issue Jul 9, 2024 · 9 comments
Labels
enhancement New feature or request

Comments

@jonct
Copy link
Contributor

jonct commented Jul 9, 2024

I just finished saying that nobody needs some newcomer to second-guess how Jailmaker is organized. So, uh… sorry. 😶‍🌫️ This one's kind of disruptive.

Summary

Might it still be reasonable to suggest adopting YAML-based templates?

Background

The current template format is clever and not hard to learn, but it brings some surprises. Among them:

  • Comments must begin at the first column, even within indented content.
  • Some multiline blocks collapse into space-delimited options; others into newline-delimited scripts.
  • Rules for escaping border on trial-and-error.
  • Lack of variable substitution makes structure and self-documentation tricky.
  • Overall it could be made clearer where syntax ends and conventions take over.
  • There's even a dynamically generated regex in the implementation that makes me squeamish.

Proposal

iX has preinstalled PyYAML for use by its middleware; it's unlikely to ever disappear. It could solve all of the above† quickly, and together.

  • Leveraging a well-known, rigorously defined standard.
  • Engaging editor support for syntax coloring and validation.
    • Especially for script blocks!
  • Validating both syntax and semantic content of PRs.
  • Validating both syntax and semantic content for CLI invocations.
  • Reducing code maintenance.

† It wouldn't do anything for variable interpolation, unfortunately. But there are conventions and unit testable starting points.

Alternatives considered

It would be easy to call for new documentation PRs. And for more eyes on the code. And for a robust suite of parser unit tests (hat tip to @Lockszmith-GH!) And for additional code to validate syntax and emit smart warnings and errors in actionable context…

I was initially only going to ask about enabling ExtendedInterpolation in the KeyValueParser. Just so that basic user-specified params (interface names, IP addresses, host paths…) could be grouped and documented at the top of a template.

But while digging in the weeds on interpolation, it occurred to me that the file structure doesn't derive from any waypoints in the systemd-nspawn world. It's just an entirely bespoke file format in apparent need of testing, tooling, and documentation.

Next steps

If I were to write a config migration function into jlmkr.py, would you consider the leap?

@jonct jonct added the enhancement New feature or request label Jul 9, 2024
@Jip-Hop
Copy link
Owner

Jip-Hop commented Jul 9, 2024

I was so happy the jlmkr.py script didn't rely on any external Python packages. 🥲 But I was also sad Python lacks built-in support for YAML. So I'd be open to migration to YAML. But I'd still prefer to minimize external dependencies (even if they are currently shipped with SCALE). Perhaps this pattern could help to still run jlmkr.py without first manually installing the Python dependencies: https://gist.github.com/Jip-Hop/15aa2b9bf9134242ad00bac5f021bc7c

But how does YAML handle comments? Preserving comments in the templates (even when some value is overridden) helps the user understand how to edit the config.

@jonct
Copy link
Contributor Author

jonct commented Jul 9, 2024

I was so happy the jlmkr.py script didn't rely on any external Python packages.

I'm with you there.

On the other hand it does still want to be installed in a specific directory. So your turnkey venv idea wouldn't be entirely out of line if it came to that.

I'd still prefer to minimize external dependencies (even if they are currently shipped with SCALE).

I truly can't imagine iX revoking PyYAML, any more than they'd revoke python3 itself or switch to an incompatible version.

But how does YAML handle comments? Preserving comments in the templates (even when some value is overridden) helps the user understand how to edit the config.

Of course YAML itself handles comments marvelously!

But you're asking about round-tripping in and out of tree representations in memory, specifically using the supplied PyYAML. Ruamel could do it; PyYAML has a patch to do it. Neither of these facts is helpful to the cause.

You probably don't want to resort to static compilation through PyInstaller or Static-Python and I don't blame you. And porting [again] to C or Go would be stark raving nuts.

I'll have to give the round-tripping question more thought. Thanks for being open to the exploration!

@Jip-Hop
Copy link
Owner

Jip-Hop commented Jul 9, 2024

Sure! But also keep in mind that iX has plans to incorporate/copy/adopt jailmaker functionality and/or code into their own codebase. If/when this happens I can imagine the config format would be evaluated, and perhaps changed. Maybe that would be the right moment to migrate to YAML and of course iX can then use PyYAML and any other Python packages they desire to implement what I tried to do with minimal dependencies...

@Jip-Hop
Copy link
Owner

Jip-Hop commented Jul 9, 2024

I'm open to exploration and I truly appreciate the interest and suggestions, newcomer or not. In my opinion what jailmaker needs right now is an extensive test suite (integration tests as well as python unit tests). I realize using well tested (external) libraries could contribute to this to some degree...

I'm really glad Lockszmith-GH has started working on #86 as it's becoming increasingly difficult to fix one thing without breaking something else. So instead of disruptive changes or new features I hope automated testing of jailmaker will take flight! How is your python and are you interested in helping out?

@jonct
Copy link
Contributor Author

jonct commented Jul 10, 2024

keep in mind that iX has plans to incorporate/copy/adopt jailmaker functionality and/or code into their own codebase.

Oh, you shouldn't say that to me. 😉 That's exactly the source of the mindset that vaulted me up into your business about architecture and ergonomics.

I take from iX's intentionally separate "sandboxes" branding that they foresee a different overview. I do, too, but I owe you full deference on this current jlmkr.py solution as we wait for whatever iX reveals and when.

I'm really glad Lockszmith-GH has started working on #86

💯 and I owe them deference too. IMHO you're exemplary leader-contributors, both. 🙇‍♂️

How is your python and are you interested in helping out?

Pythonism achieved. Not so much commercially hands-on since 2.x, but I've kept somewhat briefed on recent idioms. [As of today: including the potentially very relevant PEP 441.]

Very happy to help out. I don't have a lot of grind time to lend; big chunks of that are going into the pro bono work that necessitates this Raspberry Pi cluster (and is itself provoking me to endanger a client deadline.)

But it was important to me to pay forward my netboot investigation by submitting the router template. And then I was honored when you reached out for a hand with the test harness. Inspired, I went looking for another meaningful itch to scratch.

I really do believe that a better structured/colorized config file unlocks far more than just unit test coverage of the parser itself. Applying local reasoning throughout that tree structure would facilitate much more meaningful consistency checks, optimizations, warnings, and error messages — both in the dev pipeline and in use.

In short: way more testing, in way more places, in way less time.

And if those aspects are robust enough, then it might not even be necessary or appropriate to rewrite the file on disk? Just make context-savvy suggestions to help the user declare their needs better while they're still editing the source of truth.

It's a thought, anyway. I could pursue it in a branch and we could weigh those claims pretty quickly.

@Jip-Hop
Copy link
Owner

Jip-Hop commented Jul 10, 2024

I take from iX's intentionally separate "sandboxes" branding that they foresee a different overview.

AFAIK iX is not planning anything drastically different from jailmaker. According to Kris from iX:

Jailmaker is a nice tool. We figure its only time before we do a proper UI-based “sandboxes/jails” system in SCALE and when we do that we’d expect it to carry forward any current jailmaker managed environments fairly easily.

Glad to hear you want to help out :) Looking forward to your proposal!

@jonct
Copy link
Contributor Author

jonct commented Jul 10, 2024

So there haven't been any significant backchannel communications with iX? It looks like I've been reading these public statements a bit differently than you have.

I see your blue-sky discussion and I'll pick up over there with some thoughts as I organize them. Cheers!

@Jip-Hop
Copy link
Owner

Jip-Hop commented Jul 10, 2024

So there haven't been any significant backchannel communications with iX?

There has been some. But it's probably too early to say for sure how they're going to address it. Either way I've open sourced jailmaker with a compatible license upon their request.

@Jip-Hop
Copy link
Owner

Jip-Hop commented Jul 18, 2024

Closing as not planned due to End of Life October 2024 Announcement.

@Jip-Hop Jip-Hop closed this as completed Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants