(PE-28877, PE-28941) Add option to compile-ast endpoint to load bolt types

[donoghuc]

Previously when the compile endpoint was asked to compile AST with references to Bolt objects the compile would fail. This commit updates the compiler to accept serialized pcore and to deserialize it for compilation. This adds the requirement of some Bolt library code (and some supporting gems [addressable, public_suffix]) as well as the boltlib module. Part of making the serialized types perform as expected the compiler now creates specialized bolt inventory to back operations on bolt Target options. This work depends on two new additions to the jruby-puppet configuration section for pe-puppetserver. The first is appending the path to the bolt library code in the ruby-load-path array. The second is a new setting called boltlib-path which is an array pointing to the path of any bolt modules to prepend to the modulepath for compilation. The compile request now accepts an option to load bolt types. The default behavior is to not attempt to load bolt types.

[donoghuc]

Orch changes: https://github.com/puppetlabs/orchestrator/pull/1340
Ticket @mcdonaldseanp is working on to ship the required gems and add new settings to enterprise modules https://tickets.puppetlabs.com/browse/PE-28881
Ticket i'm working on to add acceptance tests in orch. https://tickets.puppetlabs.com/browse/PE-28597

[mwaggett]

Do we usually mention PE stuff in FOSS commit messages? I know the puppet repo doesn't like non-PUP tickets, but not sure if we have a similar policy here.

[justinstoller]

fwiw, I had these two concerns about this approach that I outlined here: #2329 (comment) I was fine merging the current approach in since we were trying to make the 6.15.0 deadline. I'm curious, since we didn't make that deadline, can we attempt those approaches now?

[adreyer]

Regarding the two questions raised in the old PR

1. AFAICT, Bolt resource compilation is the primary, quite probably only, consumer of PAL. It looks like currently Bolt has a few hacks around the PAL API to enable what it needs. Since those changes aren't upstreamed into PAL proper those hacks need to be copy-pasted here. I would love to see Puppet's version of PAL allow setting variables like you need to, as well as compiling ast code as you need it, and having setting defaults that you can use by default.

I agree we should clean this up. I'm not sure whether that should happen with immediately on as part of Puppet 7. I'm still concerned about the time crunch with the LTS approaching but doing this immediately post-LTS will definitely increase the maintenance burden of the LTS. I also don't know how stable PAL needs to be. Should we set up a meeting to figure this out?

2. We are now loading all of Bolt to, afaict, have the apply_target and apply_result PCore types available. Does this mean that functions like file::readable are now available? I have a lot of reservations about loading an entire other product within Puppet Server (where will its logging go? etc). Puppet Server's already kind of a Turducken, and now we've added the Christmas Ham... I would much prefer we separate out the PCore types into a Puppet module that server can load, though I understand that disentangling the PCore types from Bolt may be involved.

We could extract the boltlib module with it's PCore types pretty easily but some of the ruby implementations are used without PAL so that would be messier. I think the question of whether to load the bolt gem into puppet-server is a question of how we want to scale plans in general. Long term we need to horizontally scale all of bolt in pe not just catalog compilation. Do we want to set up a new service for scaling plan execution and ssh/winrm or put it in puppet-server to keep our footprint simple? It seems like ssh and winrm probably do belong in puppet-server so we have a single "edge server" for users to install in network partitions which will mean pulling bolt in anyway.

[mwaggett]

@donoghuc If this is ready from your perspective, can you remove the DO NOT MERGE label?

This seems sane to me, but I'd like someone with more puppetserver experience to take a look.

[donoghuc]

Is DO NOT MERGE lable discouraging review? I added it because this is still blocked on getting the pse bits merged/promoted. It is also dependent on some orchestrator changes.

[mwaggett]

oh, I thought this was one of the ones you said was ready. if it's still blocked, you can leave the label. 😊

[donoghuc]

To be clear, It is ready for review, we just need to be thoughtful on merge order.

[Magisus]

This looks fine, pending Molly's comments. At some point I'd kind of like us to make a decision about the non-Bolt code path here. If no use case for it comes up, I think it should ultimately get deleted.

[mwaggett]

@Magisus or @donoghuc do one of you want to make a ticket for making a decision about the non-Bolt code path?

otherwise, looks like this is ready to go?

[Magisus]

No. If the code being there ever gets annoying we can revisit it.

[justinstoller]

I know this was pretty arduous, I really appreciate all the work you have done on this.