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

(PE-36344) Stop using fork of ntlm gem #705

Merged
merged 2 commits into from
Jul 20, 2023

Conversation

donoghuc
Copy link
Member

We are now shipping openssl runtimes with the required crypto algorithms. This commit stops using the ntlm fork that provided a ruby implementation for the crypto algs.

We are now shipping openssl runtimes with the required crypto algorithms. This commit stops using the ntlm fork that provided a ruby implementation for the crypto algs.
@donoghuc donoghuc requested review from a team as code owners July 20, 2023 17:53
@@ -176,6 +176,7 @@
proj.component('rubygem-net-ssh')
proj.component('rubygem-net-ssh-krb')
proj.component('rubygem-nori')
proj.component('rubygem-rubyntlm')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, when I reverted the usage of the ntlm fork in pe-installer-runtime-main (d993409) I didn't see a dependency on 'rubygem-rubyntlm' added. Does something similar need to be done to pe-instlaler?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not now, when i was looking at this i realized for installer we actually use rubygems to do dependency resolution. This is not ideal because https://tickets.puppetlabs.com/browse/PE-36494 We will explicitly package that when we do that ticket but for now rubygems will find the latest rubyntmlm and we will be good to go.

@@ -11,9 +11,6 @@
proj.setting(:openssl_version, '3.0')

instance_eval File.read(File.join(File.dirname(__FILE__), '_shared-pe-bolt-server_with_ruby.rb'))

# TODO: Work around PE-36078 by using forked non-optimal solution
proj.component('rubygem-rubyntlm-fork')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops! I should have removed this when I added the proj.setting(:use_legacy_openssl_algos, true) in the bolt server, sorry.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, yeah i'll add it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, lol, i hadn't noticed that Sean didn't include my update for :use_legacy_openssl_algos. Thanks, Cas!

Copy link
Member

@justinstoller justinstoller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, just have a quick question if this has further implications for pe-installer-runtime-main

@mcdonaldseanp mcdonaldseanp merged commit 17416f1 into puppetlabs:master Jul 20, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants