Bundler: freeze lockfile on run, and "normalize" platform on plugin changes (#13015)

This PR enables the upgrade of bundler to the latest version.

Prior to this PR, the ability to do so was blocked by bundler.setup in versions of bundler > `2.23` making runtime changes to `Gemfile.lock` (unless the lock file was `frozen`) based on the specific platform the application was being run on, overriding any platforms (including generic `java` platform) set during build time. This was in conflict with changes made in #12782, which prevented the logstash user writing to files in `/usr/share/logstash`.

This PR will freeze the lockfile when logstash is run, and unfreeze it when manipulating plugins (install, update, remove, install from offline pack) to allow new plugins to be added. While unfrozen, changes are also made to ensure that the platform list remains as the generic `java` platform, and not changed to the specific platform for the runtime JVM.

This PR also introduces a new runtime flag, `--enable-local-plugin-development`. This flag is intended for use by Logstash developers only, and enables a mode of operation where a Gemfile can be manipulated, eg

```
gem "logstash-integration-kafka", :path => '/users/developer/code/plugins/logstash-integration-kafka'
```

to facilitate quick and simple plugin testing.

This PR also sets the `silence_root_warning` flag to avoid bundler printing out alarming looking warning messages when `sudo` is used. This warning message was concerning for users - it would be printed out during normal operation of `bin/logstash-plugin install/update/remove` when run under `sudo`, which is the expected mode of operation when logstash is installed to run as a service via rpm/deb packages. 

This PR also updates the vagrant based integration tests to ensure that Logstash still runs after plugin update/install/remove operations, fixes up some regular expressions that would cause test failures, and removes some dead code from tests.

## Release notes

* Updated Bundler to latest version
* Ensured that `Gemfile.lock` are appropriately frozen
* Added new developer-only flag to facilitate local plugin development to allow unfrozen lockfile in a development environment
This commit is contained in:
Rob Bavey 2021-08-17 09:35:30 -04:00 committed by GitHub
parent 771844c070
commit 4707cbd94c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
28 changed files with 207 additions and 82 deletions

View file

@ -27,6 +27,7 @@ QA_DIR="$PWD/qa"
# Always run the halt, even if the test times out or an exit is sent
cleanup() {
cd $QA_DIR
bundle check || bundle install
bundle exec rake qa:vm:halt
@ -39,6 +40,7 @@ cleanup
if [[ $SELECTED_TEST_SUITE == $"redhat" ]]; then
echo "Generating the RPM, make sure you start with a clean environment before generating other packages."
cd $LS_HOME
./gradlew clean bootstrap
rake artifact:rpm
echo "Acceptance: Installing dependencies"
cd $QA_DIR
@ -52,6 +54,7 @@ if [[ $SELECTED_TEST_SUITE == $"redhat" ]]; then
elif [[ $SELECTED_TEST_SUITE == $"debian" ]]; then
echo "Generating the DEB, make sure you start with a clean environment before generating other packages."
cd $LS_HOME
./gradlew clean bootstrap
rake artifact:deb
echo "Acceptance: Installing dependencies"
cd $QA_DIR
@ -65,6 +68,7 @@ elif [[ $SELECTED_TEST_SUITE == $"debian" ]]; then
elif [[ $SELECTED_TEST_SUITE == $"all" ]]; then
echo "Building Logstash artifacts"
cd $LS_HOME
./gradlew clean bootstrap
rake artifact:all
echo "Acceptance: Installing dependencies"

View file

@ -223,5 +223,14 @@ With this command, Logstash concatenates three config files, `/tmp/one`, `/tmp/t
as the log4j logging configuration. This can also be set through the LS_SETTINGS_DIR environment variable.
The default is the `config` directory under Logstash home.
*`--enable-local-plugin-development`*::
This flag enables developers to update their local Gemfile without running into issues caused by a frozen lockfile.
This flag can be helpful when you are developing/testing plugins locally.
NOTE: This flag is for Logstash developers only. End users should not need it.
*`-h, --help`*::
Print help

View file

@ -73,7 +73,7 @@ module LogStash
# in the context of Bundler.setup it looks like this is useless here because Gemfile path can only be specified using
# the ENV, see https://github.com/bundler/bundler/blob/v1.8.3/lib/bundler/shared_helpers.rb#L103
::Bundler.settings.set_local(:gemfile, Environment::GEMFILE_PATH)
::Bundler.settings.set_local(:frozen, true) unless options[:allow_gemfile_changes]
::Bundler.reset!
::Bundler.setup
end
@ -95,7 +95,6 @@ module LogStash
:jobs => 12, :all => false, :package => false, :without => [:development]}.merge(options)
options[:without] = Array(options[:without])
options[:update] = Array(options[:update]) if options[:update]
::Gem.clear_paths
ENV['GEM_HOME'] = ENV['GEM_PATH'] = LogStash::Environment.logstash_gem_home
::Gem.paths = ENV
@ -128,7 +127,11 @@ module LogStash
::Bundler.settings.set_local(:gemfile, LogStash::Environment::GEMFILE_PATH)
::Bundler.settings.set_local(:without, options[:without])
::Bundler.settings.set_local(:force, options[:force])
# This env setting avoids the warning given when bundler is run as root, as is required
# to update plugins when logstash is run as a service
# Note: Using an `ENV` here because ::Bundler.settings.set_local(:silence_root_warning, true)
# does not work (set_global *does*, but that seems too drastic a change)
with_env("BUNDLE_SILENCE_ROOT_WARNING" => "true") do
if !debug?
# Will deal with transient network errors
execute_bundler_with_retry(options)
@ -137,6 +140,7 @@ module LogStash
execute_bundler(options)
end
end
end
def execute_bundler_with_retry(options)
try = 0
@ -176,6 +180,18 @@ module LogStash
::Bundler::CLI.start(bundler_arguments(options))
end
def specific_platforms(platforms=::Gem.platforms)
platforms.find_all {|plat| plat.is_a?(::Gem::Platform) && plat.os=='java' && !plat.cpu.nil?}
end
def genericize_platform
output = LogStash::Bundler.invoke!({:add_platform => 'java'})
specific_platforms.each do |platform|
output << LogStash::Bundler.invoke!({:remove_platform => platform})
end
output
end
def debug?
ENV["DEBUG"]
end
@ -185,7 +201,6 @@ module LogStash
# @return [Array<String>] Bundler::CLI.start string arguments array
def bundler_arguments(options = {})
arguments = []
if options[:install]
arguments << "install"
arguments << "--clean" if options[:clean]
@ -202,13 +217,29 @@ module LogStash
elsif options[:package]
arguments << "package"
arguments << "--all" if options[:all]
elsif options[:add_platform]
arguments << "lock"
arguments << "--add_platform"
arguments << options[:add_platform]
elsif options[:remove_platform]
arguments << "lock"
arguments << "--remove_platform"
arguments << options[:remove_platform]
end
arguments << "--verbose" if options[:verbose]
arguments.flatten
end
def with_env(modifications)
backup_env = ENV.to_hash
ENV.replace(backup_env.merge(modifications))
yield
ensure
ENV.replace(backup_env)
end
# capture any $stdout from the passed block. also trap any exception in that block, in which case the trapped exception will be returned
# @param [Proc] the code block to execute
# @return [String, Exception] the captured $stdout string and any trapped exception or nil if none

View file

@ -81,7 +81,12 @@ end
# defined and exposing the LogStash::Runner#main instance method which will be called with the current ARGV
# currently lib/logstash/runner.rb and lib/pluginmanager/main.rb are called using this.
if $0 == __FILE__
LogStash::Bundler.setup!({:without => [:build, :development]})
bundler_options = {:without => [:build, :development]}
## Check for dev flags - this needs to be done before the runner is invoked to set bundler options
if ARGV.include?("--enable-local-plugin-development")
bundler_options[:allow_gemfile_changes] = true
end
LogStash::Bundler.setup!(bundler_options)
require_relative "patches/jar_dependencies"
require ARGV.shift

View file

@ -93,6 +93,10 @@ module Bundler
builder.eval_gemfile("bundler file", gemfile.generate())
definition = builder.to_definition(lockfile_path, {})
LogStash::Bundler.specific_platforms(definition.platforms).each do |specific_platform|
definition.remove_platform(specific_platform)
end
definition.add_platform(Gem::Platform.new('java'))
definition.lock(lockfile_path)
gemfile.save
rescue => e

View file

@ -202,9 +202,12 @@ class LogStash::PluginManager::Install < LogStash::PluginManager::Command
bundler_options[:without] = [] if development?
bundler_options[:rubygems_source] = gemfile.gemset.sources
bundler_options[:local] = true if local?
output = nil
# Unfreeze the bundle when installing gems
Bundler.settings.temporary({:frozen => false}) do
output = LogStash::Bundler.invoke!(bundler_options)
output << LogStash::Bundler.genericize_platform.to_s
end
puts("Installation successful")
rescue => exception
gemfile.restore!

View file

@ -54,7 +54,7 @@ class LogStash::PluginManager::Remove < LogStash::PluginManager::Command
signal_error("This plugin has not been previously installed") unless LogStash::PluginManager.installed_plugin?(plugin, gemfile)
exit(1) unless ::Bundler::LogstashUninstall.uninstall!(plugin)
LogStash::Bundler.genericize_platform
remove_unused_locally_installed_gems!
rescue => exception
report_exception("Operation aborted, cannot remove plugin", exception)

View file

@ -80,7 +80,13 @@ class LogStash::PluginManager::Update < LogStash::PluginManager::Command
# Bundler cannot update and clean gems in one operation so we have to call the CLI twice.
options = {:update => plugins, :rubygems_source => gemfile.gemset.sources}
options[:local] = true if local?
output=nil
# Unfreeze the bundle when updating gems
Bundler.settings.temporary({:frozen => false}) do
output = LogStash::Bundler.invoke!(options)
output << LogStash::Bundler.genericize_platform unless output.nil?
end
# We currently dont removed unused gems from the logstash installation
# see: https://github.com/elastic/logstash/issues/6339
# output = LogStash::Bundler.invoke!(:clean => true)

View file

@ -67,6 +67,7 @@ module LogStash
Setting::String.new("log.level", "info", true, ["fatal", "error", "warn", "debug", "info", "trace"]),
Setting::Boolean.new("version", false),
Setting::Boolean.new("help", false),
Setting::Boolean.new("enable-local-plugin-development", false),
Setting::String.new("log.format", "plain", true, ["json", "plain"]),
Setting::Boolean.new("http.enabled", true),
Setting::String.new("http.host", "127.0.0.1"),

View file

@ -73,6 +73,11 @@ class LogStash::Runner < Clamp::StrictCommand
:attribute_name => "node.name",
:default => LogStash::SETTINGS.get_default("node.name")
option ["--enable-local-plugin-development"], :flag,
I18n.t("logstash.runner.flag.enable-local-plugin-development"),
:attribute_name => "enable-local-plugin-development",
:default => LogStash::SETTINGS.get_default("enable-local-plugin-development")
# Config Settings
option ["-f", "--path.config"], "CONFIG_PATH",
I18n.t("logstash.runner.flag.config"),

View file

@ -269,6 +269,13 @@ en:
enabled by default.
http_host: Web API binding host
http_port: Web API http port
enable-local-plugin-development: |+
Allow Gemfile to be manipulated directly
to facilitate simpler local plugin
development.
This is an advanced setting, intended
only for use by Logstash developers,
and should not be used in production.
pipeline-id: |+
Sets the ID of the pipeline.
pipeline-workers: |+

View file

@ -32,8 +32,10 @@ import java.security.PrivilegedAction;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.jruby.Ruby;
import org.jruby.RubyClass;
import org.jruby.RubyException;
import org.jruby.RubyInstanceConfig;
import org.jruby.RubyStandardError;
import org.jruby.RubySystemExit;
import org.jruby.exceptions.RaiseException;
import org.jruby.runtime.builtin.IRubyObject;
@ -73,7 +75,8 @@ public final class Logstash implements Runnable, AutoCloseable {
Throwable t = e;
String message = e.getMessage();
if (message != null) {
if (message.startsWith(UNCLEAN_SHUTDOWN_PREFIX)) {
if (message.startsWith(UNCLEAN_SHUTDOWN_PREFIX) ||
message.startsWith(MUTATED_GEMFILE_ERROR)) {
t = e.getCause(); // be less verbose with uncleanShutdown's wrapping exception
} else if (message.contains("Could not load FFI Provider")) {
message =
@ -176,6 +179,15 @@ public final class Logstash implements Runnable, AutoCloseable {
ruby.runFromMain(script, config.displayedFileName());
} catch (final RaiseException ex) {
final RubyException re = ex.getException();
// If this is a production error this signifies an issue with the Gemfile, likely
// that a logstash developer has made changes to their local Gemfile for plugin
// development, etc. If this is the case, exit with a warning giving remediating
// information for Logstash devs.
if (isProductionError(re)){
bundlerStartupError(ex);
}
if (re instanceof RubySystemExit) {
IRubyObject success = ((RubySystemExit) re).success_p();
if (!success.isTrue()) {
@ -189,6 +201,15 @@ public final class Logstash implements Runnable, AutoCloseable {
}
}
// Tests whether the RubyException is of type `Bundler::ProductionError`
private boolean isProductionError(RubyException re){
if (re instanceof RubyStandardError){
RubyClass metaClass = re.getMetaClass();
return (metaClass.getName().equals("Bundler::ProductionError"));
}
return false;
}
@Override
public void close() {
ruby.tearDown(false);
@ -233,6 +254,14 @@ public final class Logstash implements Runnable, AutoCloseable {
}
private static final String UNCLEAN_SHUTDOWN_PREFIX = "Logstash stopped processing because of an error: ";
private static final String MUTATED_GEMFILE_ERROR = "Logstash was unable to start due to an unexpected Gemfile change.\n" +
"If you are a user, this is a bug.\n" +
"If you are a logstash developer, please try restarting logstash with the " +
"`--enable-local-plugin-development` flag set.";
private static void bundlerStartupError(final Exception ex){
throw new IllegalStateException(MUTATED_GEMFILE_ERROR);
}
private static void uncleanShutdown(final Exception ex) {
throw new IllegalStateException(UNCLEAN_SHUTDOWN_PREFIX + ex.getMessage(), ex);

View file

@ -44,32 +44,50 @@ shared_examples "logstash install" do |logstash|
command = logstash.run_command_in_path("bin/logstash-plugin install #{gem_path_on_vagrant}")
expect(command).to install_successfully
expect(logstash).to have_installed?("logstash-filter-dns")
expect(logstash).not_to be_running
with_running_logstash_service(logstash) do
expect(logstash).to be_running
end
end
end
context "when fetching a gem from rubygems" do
it "successfully install the plugin" do
command = logstash.run_command_in_path("bin/logstash-plugin install logstash-filter-qatest")
expect(command).to install_successfully
expect(logstash).to have_installed?("logstash-filter-qatest")
expect(logstash).not_to be_running
with_running_logstash_service(logstash) do
expect(logstash).to be_running
end
end
it "successfully install the plugin when verification is disabled" do
command = logstash.run_command_in_path("bin/logstash-plugin install --no-verify logstash-filter-qatest")
expect(command).to install_successfully
expect(logstash).to have_installed?("logstash-filter-qatest")
expect(logstash).not_to be_running
with_running_logstash_service(logstash) do
expect(logstash).to be_running
end
end
it "fails when installing a non logstash plugin" do
command = logstash.run_command_in_path("bin/logstash-plugin install bundler")
expect(command).not_to install_successfully
expect(logstash).not_to be_running
with_running_logstash_service(logstash) do
expect(logstash).to be_running
end
end
it "allow to install a specific version" do
command = logstash.run_command_in_path("bin/logstash-plugin install --no-verify --version 0.1.0 logstash-filter-qatest")
expect(command).to install_successfully
expect(logstash).to have_installed?("logstash-filter-qatest", "0.1.0")
with_running_logstash_service(logstash) do
expect(logstash).to be_running
end
end
end
end
@ -78,6 +96,9 @@ shared_examples "logstash install" do |logstash|
it "fails to install and report an error" do
command = logstash.run_command_in_path("bin/logstash-plugin install --no-verify logstash-output-impossible-plugin")
expect(command.stderr).to match(/Plugin not found, aborting/)
with_running_logstash_service(logstash) do
expect(logstash).to be_running
end
end
end
end

View file

@ -43,6 +43,10 @@ shared_examples "logstash remove" do |logstash|
result = logstash.run_command_in_path("bin/logstash-plugin remove logstash-filter-qatest")
expect(logstash).not_to have_installed?("logstash-filter-qatest")
expect(logstash).not_to be_running
with_running_logstash_service(logstash) do
expect(logstash).to be_running
end
end
end
end

View file

@ -43,6 +43,10 @@ shared_examples "logstash uninstall" do |logstash|
result = logstash.run_command_in_path("bin/logstash-plugin uninstall logstash-filter-qatest")
expect(logstash).not_to have_installed?("logstash-filter-qatest")
expect(logstash).not_to be_running
with_running_logstash_service(logstash) do
expect(logstash).to be_running
end
end
end
end

View file

@ -33,16 +33,22 @@ shared_examples "logstash update" do |logstash|
before do
logstash.run_command_in_path("bin/logstash-plugin install --no-verify --version #{previous_version} #{plugin_name}")
logstash.run_command_in_path("bin/logstash-plugin list")
expect(logstash).to have_installed?(plugin_name, previous_version)
# Logstash won't update when we have a pinned version in the gemfile so we remove them
logstash.replace_in_gemfile(',[[:space:]]"0.1.0"', "")
expect(logstash).to have_installed?(plugin_name, previous_version)
end
context "update a specific plugin" do
it "has executed successfully" do
cmd = logstash.run_command_in_path("bin/logstash-plugin update --no-verify #{plugin_name}")
expect(cmd.stdout).to match(/Updating #{plugin_name}/)
expect(logstash).to have_installed?(plugin_name, "0.1.1")
expect(logstash).not_to have_installed?(plugin_name, previous_version)
expect(logstash).not_to be_running
with_running_logstash_service(logstash) do
expect(logstash).to be_running
end
end
end
@ -50,6 +56,10 @@ shared_examples "logstash update" do |logstash|
it "has executed successfully" do
logstash.run_command_in_path("bin/logstash-plugin update --no-verify")
expect(logstash).to have_installed?(plugin_name, "0.1.1")
expect(logstash).not_to be_running
with_running_logstash_service(logstash) do
expect(logstash).to be_running
end
end
end
end

View file

@ -31,9 +31,9 @@ RSpec.shared_examples "installable" do |logstash|
end
it "is running on #{logstash.hostname}" do
logstash.start_service
with_running_logstash_service(logstash) do
expect(logstash).to be_running
logstash.stop_service
end
end
it "is removable on #{logstash.hostname}" do

View file

@ -40,9 +40,9 @@ RSpec.shared_examples "installable_with_jdk" do |logstash|
end
it "is running on #{logstash.hostname}" do
logstash.start_service
with_running_logstash_service(logstash, "/usr/share/logstash/jdk/bin/java") do
expect(logstash).to be_running_with("/usr/share/logstash/jdk/bin/java")
logstash.stop_service
end
end
it "is removable on #{logstash.hostname}" do

View file

@ -26,9 +26,8 @@ RSpec.shared_examples "runnable" do |logstash|
end
it "is running on #{logstash.hostname}" do
logstash.start_service
with_running_logstash_service(logstash) do
expect(logstash).to be_running
logstash.stop_service
end
end
end

View file

@ -38,7 +38,8 @@ RSpec.shared_examples "updated" do |logstash|
logstash.install({:version => LOGSTASH_VERSION})
expect(logstash).to be_installed
# starts the service to be sure it runs after the upgrade
logstash.start_service
with_running_logstash_service(logstash) do
expect(logstash).to be_running
end
end
end

View file

@ -56,3 +56,20 @@ selected_boxes = if ENV.include?('LS_VAGRANT_HOST') then
SpecsHelper.configure(selected_boxes)
puts "[Acceptance specs] running on #{ServiceTester.configuration.hosts}" if !selected_boxes.empty?
def with_running_logstash_service(logstash, jdk_path=nil)
begin
logstash.start_service
Stud.try(40.times, RSpec::Expectations::ExpectationNotMetError) do
if jdk_path
expect(logstash).to be_running_with(jdk_path)
else
expect(logstash).to be_running
end
end
yield
ensure
logstash.stop_service
end
end

View file

@ -21,7 +21,6 @@ require_relative "./commands/redhat"
require_relative "./commands/suse"
require_relative "./commands/centos/centos-6"
require_relative "./commands/oel/oel-6"
require_relative "./commands/ubuntu/ubuntu-1604"
require_relative "./commands/suse/sles-11"
require "forwardable"
@ -131,11 +130,7 @@ module ServiceTester
case type
when "debian"
if host.start_with?("ubuntu")
if host == "ubuntu-1604"
return Ubuntu1604Commands.new
else
return UbuntuCommands.new
end
else
return DebianCommands.new
end

View file

@ -28,7 +28,7 @@ module ServiceTester
stdout.force_encoding(Encoding::UTF_8)
(
stdout.match(/Active: active \(running\)/) &&
stdout.match(/^\s*└─\d*\s.*#{jdk_path}/) &&
stdout.match(/^\s*(└─|`-)\d*\s.*#{jdk_path}/) &&
stdout.match(/#{package}.service - #{package}/)
)
end
@ -49,12 +49,15 @@ module ServiceTester
stdout = cmd.stdout
end
running = stdout.match(/#{package} start\/running/)
if running
pid = stdout.match(/#{package} start\/running, process (\d*)/).captures[0]
at(hosts, {in: :serial}) do |host|
cmd = sudo_exec!("ps ax | grep #{pid}")
stdout = cmd.stdout
end
(running && stdout.match(/#{jdk_path}/))
running = (running && stdout.match(/#{jdk_path}/))
end
running
end
def service_manager(service, action, host=nil)

View file

@ -19,15 +19,6 @@ require_relative "debian"
module ServiceTester
class UbuntuCommands < DebianCommands
def running?(hosts, package)
stdout = ""
at(hosts, {in: :serial}) do |host|
cmd = sudo_exec!("service #{package} status")
stdout = cmd.stdout
end
stdout.match(/^#{package} start\/running/)
end
include ::ServiceTester::SystemD
end
end

View file

@ -1,25 +0,0 @@
# Licensed to Elasticsearch B.V. under one or more contributor
# license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright
# ownership. Elasticsearch B.V. licenses this file to you under
# the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
require_relative "../base"
require_relative "../ubuntu"
module ServiceTester
class Ubuntu1604Commands < UbuntuCommands
include ::ServiceTester::SystemD
end
end

View file

@ -71,7 +71,7 @@ module LogStash
def self.reporter(io, wait_thr, &block)
Thread.new(io, wait_thr) do |_io, _wait_thr|
while (_wait_thr.status == "run")
while (_wait_thr.status == "run" || _wait_thr.status == "sleep")
begin
c = _io.read(1)
block.call(c) if c

View file

@ -46,7 +46,7 @@ module LogStash
end
def self.fetch_config
machines = CommandExecutor.run!("vagrant status").stdout.split("\n").select { |l| l.include?("running") }.map { |r| r.split(' ')[0]}
machines = CommandExecutor.run!("vagrant status --machine-readable").stdout.split("\n").select { |l| l.include?("state,running") }.map { |r| r.split(',')[1]}
CommandExecutor.run!("vagrant ssh-config #{machines.join(' ')}")
end

View file

@ -155,6 +155,7 @@ void setupJruby(File projectDir, File buildDir) {
jruby.currentDirectory = projectDir
jruby.runScriptlet("require '${projectDir}/lib/bootstrap/environment'")
jruby.runScriptlet("LogStash::Bundler.invoke!")
jruby.runScriptlet("LogStash::Bundler.genericize_platform")
}
}