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.

Sign up for GitHub

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

Jump to bottom

Feat: support ssl_verification_mode => 'full' / 'none' #126

Merged
merged 14 commits into from
Feb 1, 2022
Merged

Feat: support ssl_verification_mode => 'full' / 'none' #126

merged 14 commits into from
Feb 1, 2022

Conversation

kares
Copy link
Contributor

@kares kares commented Jan 25, 2022

the feature itself comes from the mixin: logstash-plugins/logstash-mixin-http_client#39

@kares kares linked an issue Jan 25, 2022 that may be closed by this pull request
support (disabling) verify mode for TLS connection #119
Closed
kares
kares commented Jan 25, 2022
View reviewed changes
logstash-output-http.gemspec
@@ -20,7 +20,7 @@ Gem::Specification.new do |s|

# Gem dependencies
s.add_runtime_dependency "logstash-core-plugin-api", ">= 1.60", "<= 2.99"
s.add_runtime_dependency "logstash-mixin-http_client", ">= 6.0.0", "< 8.0.0"
s.add_runtime_dependency "logstash-mixin-http_client", ">= 7.1.0", "< 8.0.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

7.1.0 contains support for the ssl_verification_mode option

@kares kares marked this pull request as ready for review January 25, 2022 13:55
@kares
Copy link
Contributor Author

kares commented Jan 25, 2022

🔴 8.x seems to be hitting a JNR bug (triggered by WEBrick used during testing):

logstash_1_ef554d540a5f | Jan 25, 2022 1:54:43 PM jnr.netdb.NativeProtocolsDB load
logstash_1_ef554d540a5f | WARNING: Failed to load native protocols db
logstash_1_ef554d540a5f | java.lang.RuntimeException: getprotobyname_r failed
logstash_1_ef554d540a5f | 	at org.jruby.dist/jnr.netdb.NativeProtocolsDB$LinuxNativeProtocolsDB.getProtocolByName(NativeProtocolsDB.java:180)
logstash_1_ef554d540a5f | 	at org.jruby.dist/jnr.netdb.NativeProtocolsDB.load(NativeProtocolsDB.java:80)

@kares kares self-assigned this Jan 25, 2022
kaisecheng
kaisecheng reviewed Jan 25, 2022
View reviewed changes
docs/index.asciidoc

* Value type is <<string,string>>
* Supported values are: `full`, `none`
* Default value is `full`
Copy link
Contributor

@kaisecheng kaisecheng Jan 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I am thinking what does full mean. Beats has a nice doc
Maybe a good reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, thought I would keep it short but we could copy the description from Beats' docs ...

kares
kares commented Jan 25, 2022
View reviewed changes
docs/index.asciidoc Outdated Show resolved Hide resolved
@kares kares requested a review from kaisecheng January 26, 2022 07:01
kaisecheng
kaisecheng approved these changes Jan 26, 2022
View reviewed changes
Copy link
Contributor

@kaisecheng kaisecheng left a comment

Choose a reason for hiding this comment

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

code, test and doc look good 🍎

@kares kares requested a review from karenzone January 26, 2022 10:32
karenzone
karenzone reviewed Feb 1, 2022
View reviewed changes
docs/index.asciidoc Outdated Show resolved Hide resolved
karenzone
karenzone approved these changes Feb 1, 2022
View reviewed changes
Copy link
Contributor

@karenzone karenzone left a comment

Choose a reason for hiding this comment

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

Left minor suggestion inline for your consideration. Otherwise, LGTM.

@kares @karenzone
Update docs/index.asciidoc
582892e 
Co-authored-by: Karen Metts <[email protected]>
@kares kares merged commit f279aac into logstash-plugins:main Feb 1, 2022
@kares kares mentioned this pull request Feb 1, 2022
Merged
@kares kares linked an issue Feb 3, 2022 that may be closed by this pull request
Issues with SSL certificate validation in logstash output http plugin #21
Closed
This was referenced Feb 3, 2022
Closed
Closed
Closed
@b2az
Copy link

b2az commented Feb 5, 2022

@kares u my hero

@douniwan5788
Copy link

How to disable ssl certificate verification in input?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaisecheng kaisecheng kaisecheng approved these changes

@karenzone karenzone karenzone approved these changes

Assignees

@kares kares

Labels
None yet
Projects
None yet
Milestone
No milestone
6 participants
@kares @b2az @douniwan5788 @karenzone @kaisecheng @roaksoax