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

Bugfix/deep environment variables #6392

Closed
wants to merge 639 commits into from
Closed

Bugfix/deep environment variables #6392

wants to merge 639 commits into from

Conversation

emilesalem
Copy link

the fix that was added for issue #3944 is not evaluating environment variables beyond the first level of hash values, as described here

This pull request aims to correct that.

I could not add a test as I dont know anything about Ruby development and failed to run the test suites. I tested this fix running logstash from source inside a docker image based on jruby:9.1-jdk and it works with a few sample configurations. I didnt have the same success trying to run test suites.

Thank you for considering this fix.

jsvd and others added 30 commits October 7, 2016 10:39
This problem is seen here:
#6019

This change will log extra information that will be helpful to the
debugging process.

Testing this out with jdbc-input 4.1.1 (cited in #6019 above) we now see
as output:

```
~/p/l/logstash-alt (better_plugin_error_messages) $ bin/logstash -e "input { jdbc {} } output { stdout {} }"
Sending Logstash logs to /Users/andrewvc/projects/lsp/logstash-alt/logs which is now configured via log4j2.properties.
[2016-10-10T16:38:50,723][WARN ][logstash.registry        ] Problems loading a plugin with {:type=>"input", :name=>#<LogStash::Registry::Plugin:0x50ec4e78 @type="input", @name="jdbc">, :path=>"logstash/inputs/jdbc", :error_message=>"uninitialized constant LogStash::PluginMixins::Jdbc::Cabin", :error_class=>NameError, :error_backtrace=>["org/jruby/RubyModule.java:2719:in `const_missing'", "/Users/andrewvc/projects/lsp/logstash-alt/vendor/bundle/jruby/1.9/gems/logstash-input-jdbc-4.1.1/lib/logstash/plugin_mixins/jdbc.rb:11:in `Jdbc'", "/Users/andrewvc/projects/lsp/logstash-alt/vendor/bundle/jruby/1.9/gems/logstash-input-jdbc-4.1.1/lib/logstash/plugin_mixins/jdbc.rb:9:in `(root)'", "org/jruby/RubyKernel.java:1040:in `require'", "/Users/andrewvc/projects/lsp/logstash-alt/vendor/bundle/jruby/1.9/gems/polyglot-0.3.5/lib/polyglot.rb:65:in `require'", "/Users/andrewvc/projects/lsp/logstash-alt/vendor/bundle/jruby/1.9/gems/logstash-input-jdbc-4.1.1/lib/logstash/inputs/jdbc.rb:1:in `(root)'", "org/jruby/RubyKernel.java:1040:in `require'", "/Users/andrewvc/projects/lsp/logstash-alt/vendor/bundle/jruby/1.9/gems/polyglot-0.3.5/lib/polyglot.rb:65:in `require'", "/Users/andrewvc/projects/lsp/logstash-alt/vendor/bundle/jruby/1.9/gems/logstash-input-jdbc-4.1.1/lib/logstash/inputs/jdbc.rb:4:in `(root)'", "/Users/andrewvc/projects/lsp/logstash-alt/logstash-core/lib/logstash/plugins/registry.rb:1:in `(root)'", "/Users/andrewvc/projects/lsp/logstash-alt/logstash-core/lib/logstash/plugins/registry.rb:59:in `lookup'", "/Users/andrewvc/projects/lsp/logstash-alt/logstash-core/lib/logstash/plugin.rb:121:in `lookup'", "org/jruby/RubyKernel.java:1079:in `eval'", "/Users/andrewvc/projects/lsp/logstash-alt/logstash-core/lib/logstash/pipeline.rb:418:in `plugin'", "(eval):8:in `initialize'", "/Users/andrewvc/projects/lsp/logstash-alt/logstash-core/lib/logstash/pipeline.rb:90:in `initialize'", "/Users/andrewvc/projects/lsp/logstash-alt/logstash-core/lib/logstash/agent.rb:195:in `create_pipeline'", "/Users/andrewvc/projects/lsp/logstash-alt/logstash-core/lib/logstash/agent.rb:87:in `register_pipeline'", "/Users/andrewvc/projects/lsp/logstash-alt/logstash-core/lib/logstash/runner.rb:256:in `execute'", "/Users/andrewvc/projects/lsp/logstash-alt/vendor/bundle/jruby/1.9/gems/clamp-0.6.5/lib/clamp/command.rb:67:in `run'", "/Users/andrewvc/projects/lsp/logstash-alt/lib/bootstrap/environment.rb:68:in `(root)'"]}
[2016-10-10T16:38:50,735][ERROR][logstash.agent           ] fetched an invalid config {:config=>"input { jdbc {} } output { stdout {} }", :reason=>"Couldn't find any input plugin named 'jdbc'. Are you sure this is correct? Trying to load the jdbc input plugin resulted in this error: Problems loading the requested plugin named jdbc of type input. Error: NameError uninitialized constant LogStash::PluginMixins::Jdbc::Cabin"}
```

Fixes #6020
the wrong word in this sentence "Run the example desfined at line 11" .
“desfined” change to "defined".

Fixes #5987
Added new Event API doc to support breaking changes
This allows us to validate all settings after all the settings sources
have been processed (logstash.yml, flags, environment variables, etc)

NullableString is required for validation to pass on what were
previously String settings with nil defaults.

WritableDirectory's strict now defaults false to help with a problem
where the default path.data might not be writable *and* the user could
be specifying --path.data on the command line to compensate. Prior to
this, the default value would be validated and cause Logstash to
terminate on startup because of the default data directory was validated
before the flag override was applied.

To make this validate_all feature more useful, Setting#set will only
call validate if `strict` is true.

Fixes #6004

Fixes #6008
* avoid the use of sleeps
* remove unused mocking of logger
* ensure pipelines are shut down after each test

Fixes #6039
Adds remove and deprecates uninstall

Fixes #6041

Fixes #6042
Change strings and regexs to CONSTANTS.

move method missing code to event.rb

doh - put Event class in already defined Logstash module

Fixes #6045
This also makes the failure reports from WritableDirectory's validation
more specific (parent directory doesn't exist, or does exist and is
isn't writable, or the path itself isn't writable, etc).

Fixes #6023
update comments

preserve abort_on_exception state
SafeURI requires forwardable, which is included elsewhere in logstash. Plugin tests using this validation don't load logstash in the right order for this to work, so we need an explicit require here.

Fixes #5978
As this breaking change will catch the unsuspecting off guard, make it very visible in the breaking changes documentation

Fixes #6068
@emilesalem
Copy link
Author

emilesalem commented Jan 28, 2017

@suyograo @andrewvc I have updated the pull request to correct the indentation errors. Unfortunately, as I explained in my first comment, I know basically nothing of the Ruby language and do not have the time to learn it properly, so writing tests is beyond my abilities right now. Someone else will have to write the specs, I'm sorry.

jsvd and others added 16 commits January 30, 2017 12:46
this removes explicit references to the "main" pipeline,
using instead the value of the `pipeline.id` from LogStash::SETINGS

Fixes #6606
use bin/ruby instead of hardcoded vendorized jruby

without :development

remove symlink check per PR discussiom, will re-introduce in a followup PR
recover method wip

recover method wip

recevered lenght 0 is invalid

fix firstUnackedSeqNum

DRYied open and recover

DRYed and refactored to extract AbstractByteBufferPageIO from ByteBufferPageIO and MmapPageIO

better exception messages

cleanup and remove subject idiom

rename _mapFile to mapFile

added invalid state recovery tests

use log4j

renamed TestSettings methods to improve readability

duplicate code

add version check

typo

use test exceptions annotation

use parametrized tests

added uncheck() method to clean test stream

add better message todo

proper javadoc comment

typo
use new bin/ruby and bundler without :development

refactor to DRY and use expected exception

added original Apache 2.0 license and some cosmetics

exclude bin/lock from packaging

rename variables
since 5.x introduced log4j2 as the main logging mechanism, it's
necessary to be more explicit when logging complex objects.

In this case we tell the logger to use the .to_s version of the Snapshot
report generated by the Watcher.
The Snapshot#to_s calls .to_simple_hash.to_s

Fixes #6628
added usage comment
… same settings

extracted BasePipeline class to support complete config validation

minor review changes

added comment
…rs and other config problems are more specifically logged before we get to this point, and showing the full config can too easily obscure a more actional 'you used an invalid setting' kind of error.

Fixes #6654
@suyograo
Copy link
Contributor

suyograo commented Feb 8, 2017

@emilesalem I'll add tests to it. Can you please open this PR against master instead of 5.1? Typically all patches must be submitted against the master branch and we then backport to release branches such as 5.1, 5.2 etc.

untergeek and others added 5 commits February 9, 2017 12:12
This will now put ENV variables from the `startup.options` file into `/etc/default/logstash` or `/etc/sysconfig/logstash` (or whatever service name you chose), and use the updated pleaserun to ensure these are honored at start time for whichever init system you use (systemd, upstart, SysV).

fixes #6482

Fixes #6660
@karmi
Copy link

karmi commented Feb 9, 2017

Hi @emilesalem, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

1 similar comment
@karmi
Copy link

karmi commented Feb 9, 2017

Hi @emilesalem, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

@emilesalem emilesalem closed this Feb 9, 2017
@emilesalem emilesalem deleted the bugfix/deepEnVars branch February 11, 2017 18:14
@z-vr
Copy link

z-vr commented Mar 12, 2017

has this not been released?

@emilesalem
Copy link
Author

this was moved to #6694

merged in master 3 weeks ago.

No idea when they'll release.

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.

None yet