(PE-31696) Prefer builtin lib code over external gems when autoloading #8685
Conversation
This file contains 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
In PE we use the ruby interpreter shipped with puppet agent to run the ace server. The ace server is responsible for executing remote tasks. Remote tasks will often use the `puppet` library. Remote ruby tasks run in ace over the local transport (and thus bolt) execute code with the same environment variables as the ruby process running the task runner uses. In the case of ace, we ship a puppet gem in addition to many of the other gems bolt depends on. Now that puppet has moved to using require_relative there is a bug whereby a new combination of code is loaded between the puppet gem and the puppet code shipped with the agent (because we set GEM_HOME and GEM_PATH when we start the service). This commit updates the autoloader to prefer loading code from the $LOAD_PATH. This helps solve the problem of loading code from both places in ace-server for remote tasks. The only consequence it may have would be if users somehow relied on shadowing shipped puppet code with external gems, though it is hard to imagine how they would manage to reliably configure this.
|
Changing the order makes my nervous given all the different ways code can be loaded in puppet agent, server, bolt, ... Prior to relative requires, which version of puppet was ace server using (the one from puppet-agent or the gem version it depended on)? It might be good to sync up on this one, as I'm not sure I followed all of the ace logic. |
|
|
|
Is this still an issue now that https://github.com/puppetlabs/bolt-vanagon/pull/170/files is merged? |
|
Yeah, I was never able to get it to work with that change 😢 |
|
Closing this in favor of puppetlabs/ace#99 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
In PE we use the ruby interpreter shipped with puppet agent to run the ace server. The ace server is responsible for executing remote tasks. Remote tasks will often use the
puppetlibrary. Remote ruby tasks run in ace over the local transport (and thus bolt) execute code with the same environment variables as the ruby process running the task runner uses. In the case of ace, we ship a puppet gem in addition to many of the other gems bolt depends on. Now that puppet has moved to using require_relative there is a bug whereby a new combination of code is loaded between the puppet gem and the puppet code shipped with the agent (because we set GEM_HOME and GEM_PATH when we start the service). This commit updates the autoloader to prefer loading code from the $LOAD_PATH. This helps solve the problem of loading code from both places in ace-server for remote tasks. The only consequence it may have would be if users somehow relied on shadowing shipped puppet code with external gems, though it is hard to imagine how they would manage to reliably configure this.