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

Require net/https for use of 'OpenSSL::SSL...' constants #1471

Merged
merged 2 commits into from
Sep 12, 2018

Conversation

micro128390
Copy link
Contributor

Description

Current issue:
When using an http source and secure: false, oxidized gives error:

# oxidized
I, [2018-07-30T01:54:56.900233 #128]  INFO -- : Oxidized starting, running as pid 128
I, [2018-07-30T01:54:56.923148 #128]  INFO -- : lib/oxidized/nodes.rb: Loading nodes
F, [2018-07-30T01:54:56.923327 #128] FATAL -- : Oxidized crashed, crashfile written in /root/.config/oxidized/crash
uninitialized constant Oxidized::HTTP::OpenSSL

Fix

We require the 'openssl' package.

@codecov-io
Copy link

codecov-io commented Jul 30, 2018

Codecov Report

Merging #1471 into master will decrease coverage by 0.11%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1471      +/-   ##
=========================================
- Coverage   62.72%   62.6%   -0.12%     
=========================================
  Files          30      30              
  Lines        1481    1487       +6     
=========================================
+ Hits          929     931       +2     
- Misses        552     556       +4
Impacted Files Coverage Δ
lib/oxidized/source/http.rb 38% <100%> (+1.26%) ⬆️
lib/oxidized/config.rb 32.07% <0%> (-1.26%) ⬇️
lib/oxidized/node.rb 71.52% <0%> (-1.01%) ⬇️
lib/oxidized/nodes.rb 49.51% <0%> (ø) ⬆️
lib/oxidized/worker.rb 17.14% <0%> (ø) ⬆️
lib/oxidized/cli.rb 39.68% <0%> (ø) ⬆️
lib/oxidized/core.rb 30.76% <0%> (ø) ⬆️
lib/oxidized/model/model.rb 84.31% <0%> (ø) ⬆️
lib/oxidized/input/ssh.rb 54.34% <0%> (+0.5%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 868cb67...fe5a588. Read the comment docs.

@wk
Copy link
Contributor

wk commented Jul 30, 2018

@micro128390
This would also require:

  • Adding the gem into the oxidized.gemspec gemspec so it's pulled and installed before being required.
  • Adding the gem into the Dockerfile so that docker container users will have functional HTTPS support.

@ytti, @micro128390
openssl gem requires ruby >= 2.3.0, while the current intention is for Oxidized to support 2.0.0; what's a good way forward here?

@micro128390
Copy link
Contributor Author

Just a thought:

  • Installing oxidized on Centos:

    [root@vagrant-centos ~]# ruby --version
    ruby 2.0.0p648 (2015-12-16) [x86_64-linux]
    
    [root@vagrant-centos ~]# gem install oxidized
    Building native extensions.  This could take a while...
    Successfully installed rugged-0.27.2
    Fetching: slop-3.6.0.gem (100%)
    Successfully installed slop-3.6.0
    Fetching: net-telnet-0.2.0.gem (100%)
    ERROR:  Error installing oxidized:
        net-telnet requires Ruby version >= 2.3.0.
    

    ... so do oxidized dependencies already require a version of ruby >= 2.3.0 or did I do something wrong?

@ytti
Copy link
Owner

ytti commented Jul 31, 2018

Is there full crash file? I'd like to see where the openssl dependency comes from and if we have options.
I'd like to delay jumping to 2.3.0, but we will need to do it eventually anyhow.

@wk
Copy link
Contributor

wk commented Jul 31, 2018

@micro128390
The net-telnet dependency on 2.3 is unintentional, and the result of a recent (2018-07-25) release of net-telnet 0.2.0. An issue for this already exists (#1472) and I've just created a PR to resolve this (#1475).

@micro128390
Copy link
Contributor Author

@ytti https://github.com/ytti/oxidized/blob/master/lib/oxidized/source/http.rb#L59 here it is using OpenSSL constant which can not be used without the require.

@wk
Copy link
Contributor

wk commented Aug 13, 2018

@micro128390 could you check if requiring "net/https" in addition to "net/http" (but without "openssl") works in your environment?

@micro128390
Copy link
Contributor Author

Yes, requiring net/https fixes it (because net/https requires openssl internally). Tested and works. Feel free to delete this PR and just make this change elsewhere.

Thanks very much to the authors/contributors for useful software.

@wk wk merged commit ef48dbf into ytti:master Sep 12, 2018
@wk
Copy link
Contributor

wk commented Sep 12, 2018

@micro128390 thanks for working on this! I've modified your commit to reference "net/https" instead and merged it in.

@wk wk changed the title Require openssl for use of 'OpenSSL::SSL...' constants Require net/https for use of 'OpenSSL::SSL...' constants Sep 12, 2018
@morgens
Copy link

morgens commented Sep 13, 2018

So if i'm trying to upgrade from 0.21 to 0.24 and get this, what should I do?
2018-09-13_161012
It's on CentOS 7, where latest Ruby version is 2.0.0.648

@zkiroel
Copy link

zkiroel commented Nov 8, 2018

So if i'm trying to upgrade from 0.21 to 0.24 and get this, what should I do?
2018-09-13_161012
It's on CentOS 7, where latest Ruby version is 2.0.0.648

have you find solution? thank you

@morgens
Copy link

morgens commented Nov 8, 2018

So if i'm trying to upgrade from 0.21 to 0.24 and get this, what should I do?
2018-09-13_161012
It's on CentOS 7, where latest Ruby version is 2.0.0.648

have you find solution? thank you

Nope. Just waiting for a new release. Hope this will get fixed.

@zkiroel
Copy link

zkiroel commented Nov 8, 2018

Nope. Just waiting for a new release. Hope this will get fixed.

try this (without cfg file listings), work for me
main lines is 34 & 35 i guess
#history
4 sudo yum -y update
6 sudo yum -y install epel-release
10 sudo systemctl stop firewalld
11 sudo systemctl disable firewalld
13 sudo yum -y install git yum wget mc htop iftop ncdu screen httpd nginx httpd net-tools httpd-tools open-vm-tools vim
12 sudo vim /etc/selinux/config
15 sudo reboot
16 sudo vim /etc/nginx/nginx.conf
18 sudo htpasswd -c /etc/nginx/.htpasswd oxidized
19 sudo systemctl restart nginx
20 sudo systemctl enable nginx
21 sudo yum -y install curl gcc-c++ patch readline readline-devel zlib zlib-devel
22 sudo yum -y install libyaml-devel libffi-devel openssl-devel make cmake
23 sudo yum install bzip2 autoconf automake libtool bison iconv-devel libssh2-devel libicu-devel -y
26 curl -sSL https://rvm.io/mpapis.asc | gpg2 --import -
27 curl -L get.rvm.io | bash -s stable
28 source /home/leorikz/.rvm/scripts/rvm
29 gem
31 rvm install 2.5.1
32 rvm use --default 2.5.1
33 rvm list
34 gem update --system
35 gem update
36 gem install oxidized
37 gem install oxidized-script oxidized-web
38 oxidized
39 sudo vim .config/oxidized/config
41 touch /home/leorikz/.config/oxidized/router.db
42 sudo vim /home/leorikz/.config/oxidized/router.db
43 oxidized

2018-11-08_164000

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants