From 828e56912eaaaf5660b68729e739cd2a691cb8a5 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Thu, 13 Apr 2023 11:47:29 +0100 Subject: [PATCH 1/5] Allow customizing audit broadcast with env When invoking the audit broadcast command, provide a few environment variables so that people can customize the format of the message if they want. We currently provide `MRSK_PERFORMER`, `MRSK_ROLE`, `MRSK_DESTINATION` and `MRSK_EVENT`. Also adds the destination to the default message, which we continue to send as the first argument as before. --- README.md | 9 +++++++++ lib/mrsk/cli/base.rb | 4 +++- lib/mrsk/commands/auditor.rb | 21 +++++++++++++++++++-- test/commands/auditor_test.rb | 23 ++++++++++++++++++++--- 4 files changed, 51 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 23fc8ad6..5da301cc 100644 --- a/README.md +++ b/README.md @@ -677,6 +677,15 @@ That'll post a line like follows to a preconfigured chatbot in Basecamp: [My App] [dhh] Rolled back to version d264c4e92470ad1bd18590f04466787262f605de ``` +In addition to the formatted message, MRSK sets a number of environment variables with the components of the broadcast. +You can use these (rather than the command argument) if you want more control over how the message is formatted. +MRSK currently sets: + +- `MRSK_PERFORMER` - the user performing the command +- `MRSK_DESTINATION` - the destination +- `MRSK_ROLE` - the specific role being targetted, if any +- `MRSK_EVENT` - text of the action (e.g. "Deployed app@150b24f") + ### Custom healthcheck MRSK defaults to checking the health of your application again `/up` on port 3000 up to 7 times. You can tailor the behaviour with the `healthcheck` setting: diff --git a/lib/mrsk/cli/base.rb b/lib/mrsk/cli/base.rb index f772b821..22c4a846 100644 --- a/lib/mrsk/cli/base.rb +++ b/lib/mrsk/cli/base.rb @@ -73,7 +73,9 @@ module Mrsk::Cli end def audit_broadcast(line) - run_locally { execute *MRSK.auditor.broadcast(line), verbosity: :debug } + if broadcast = MRSK.auditor.broadcast(line) + system(MRSK.auditor.broadcast_environment(line), broadcast) + end end def with_lock diff --git a/lib/mrsk/commands/auditor.rb b/lib/mrsk/commands/auditor.rb index 6915a521..deafa95f 100644 --- a/lib/mrsk/commands/auditor.rb +++ b/lib/mrsk/commands/auditor.rb @@ -22,6 +22,15 @@ class Mrsk::Commands::Auditor < Mrsk::Commands::Base end end + def broadcast_environment(line) + { + "MRSK_PERFORMER" => performer, + "MRSK_ROLE" => role, + "MRSK_DESTINATION" => config.destination, + "MRSK_EVENT" => line + } + end + def reveal [ :tail, "-n", 50, audit_log_file ] end @@ -36,7 +45,7 @@ class Mrsk::Commands::Auditor < Mrsk::Commands::Base end def tagged_broadcast_line(line) - tagged_line performer_tag, role_tag, line + tagged_line performer_tag, role_tag, destination_tag, line end def tagged_line(*tags_and_line) @@ -47,11 +56,19 @@ class Mrsk::Commands::Auditor < Mrsk::Commands::Base "[#{Time.now.to_fs(:db)}]" end + def performer + `whoami`.strip + end + def performer_tag - "[#{`whoami`.strip}]" + "[#{performer}]" end def role_tag "[#{role}]" if role end + + def destination_tag + "[#{config.destination}]" if config.destination + end end diff --git a/test/commands/auditor_test.rb b/test/commands/auditor_test.rb index 55c3a32e..6068acef 100644 --- a/test/commands/auditor_test.rb +++ b/test/commands/auditor_test.rb @@ -31,9 +31,26 @@ class CommandsAuditorTest < ActiveSupport::TestCase end test "broadcast" do - assert_match \ - /bin\/audit_broadcast '\[.*\] app removed container'/, - new_command.broadcast("app removed container").join(" ") + Mrsk::Commands::Auditor.any_instance.stubs(:performer).returns("bob") + @role = "web" + @destination = "staging" + + assert_equal \ + ["bin/audit_broadcast", "'[bob] [web] [staging] app removed container'"], + new_command.broadcast("app removed container") + end + + test "broadcast environment" do + Mrsk::Commands::Auditor.any_instance.stubs(:performer).returns("bob") + @role = "web" + @destination = "staging" + + env = new_command.broadcast_environment("app removed container") + + assert_equal "bob", env["MRSK_PERFORMER"] + assert_equal "web", env["MRSK_ROLE"] + assert_equal "staging", env["MRSK_DESTINATION"] + assert_equal "app removed container", env["MRSK_EVENT"] end private From 99fe31d4b43e721204d23e5df3cb38b9ece90a03 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 14 Apr 2023 16:11:42 +0100 Subject: [PATCH 2/5] Rename MRSK_EVENT -> MRSK_MESSAGE It's a better name, and frees up `MRSK_EVENT` to be used later. --- lib/mrsk/commands/auditor.rb | 2 +- test/commands/auditor_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mrsk/commands/auditor.rb b/lib/mrsk/commands/auditor.rb index deafa95f..1ae51181 100644 --- a/lib/mrsk/commands/auditor.rb +++ b/lib/mrsk/commands/auditor.rb @@ -27,7 +27,7 @@ class Mrsk::Commands::Auditor < Mrsk::Commands::Base "MRSK_PERFORMER" => performer, "MRSK_ROLE" => role, "MRSK_DESTINATION" => config.destination, - "MRSK_EVENT" => line + "MRSK_MESSAGE" => line } end diff --git a/test/commands/auditor_test.rb b/test/commands/auditor_test.rb index 6068acef..6ba1c5e8 100644 --- a/test/commands/auditor_test.rb +++ b/test/commands/auditor_test.rb @@ -50,7 +50,7 @@ class CommandsAuditorTest < ActiveSupport::TestCase assert_equal "bob", env["MRSK_PERFORMER"] assert_equal "web", env["MRSK_ROLE"] assert_equal "staging", env["MRSK_DESTINATION"] - assert_equal "app removed container", env["MRSK_EVENT"] + assert_equal "app removed container", env["MRSK_MESSAGE"] end private From aceabb382441be4fda7763d7089fedef579b298d Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Fri, 14 Apr 2023 16:13:59 +0100 Subject: [PATCH 3/5] Update README with env name change --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5da301cc..26503f85 100644 --- a/README.md +++ b/README.md @@ -684,7 +684,7 @@ MRSK currently sets: - `MRSK_PERFORMER` - the user performing the command - `MRSK_DESTINATION` - the destination - `MRSK_ROLE` - the specific role being targetted, if any -- `MRSK_EVENT` - text of the action (e.g. "Deployed app@150b24f") +- `MRSK_MESSAGE` - full text of the action (e.g. "Deployed app@150b24f") ### Custom healthcheck From 048aecf352a2e8f1a8558bbc66369479799f0179 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 2 May 2023 11:42:05 -0700 Subject: [PATCH 4/5] Audit details (#1) Audit details * Audit logs and broadcasts accept `details` whose values are included as log tags and MRSK_* env vars passed to the broadcast command * Commands may return execution options to the CLI in their args list * Introduce `mrsk broadcast` helper for sending audit broadcasts * Report UTC time, not local time, in audit logs. Standardize on ISO 8601 format --- README.md | 20 ++++++--- lib/mrsk/cli/app.rb | 8 ++-- lib/mrsk/cli/base.rb | 4 +- lib/mrsk/cli/main.rb | 7 +++ lib/mrsk/commander.rb | 4 +- lib/mrsk/commands/auditor.rb | 59 +++++++++--------------- lib/mrsk/commands/lock.rb | 4 +- lib/mrsk/sshkit_with_ext.rb | 33 ++++++++++++++ test/cli/main_test.rb | 13 ++++++ test/commands/auditor_test.rb | 80 ++++++++++++++++++--------------- test/fixtures/deploy_simple.yml | 1 + 11 files changed, 143 insertions(+), 90 deletions(-) diff --git a/README.md b/README.md index 1229b022..da920a26 100644 --- a/README.md +++ b/README.md @@ -677,14 +677,20 @@ That'll post a line like follows to a preconfigured chatbot in Basecamp: [My App] [dhh] Rolled back to version d264c4e92470ad1bd18590f04466787262f605de ``` -In addition to the formatted message, MRSK sets a number of environment variables with the components of the broadcast. -You can use these (rather than the command argument) if you want more control over how the message is formatted. -MRSK currently sets: +`MRSK_*` environment variables are available to the broadcast command for +fine-grained audit reporting, e.g. for triggering deployment reports or +firing a JSON webhook. These variables include: +- `MRSK_RECORDED_AT` - UTC timestamp in ISO 8601 format, e.g. `2023-04-14T17:07:31Z` +- `MRSK_PERFORMER` - the local user performing the command (from `whoami`) +- `MRSK_MESSAGE` - the full audit message, e.g. "Deployed app@150b24f" +- `MRSK_DESTINATION` - optional: destination, e.g. "staging" +- `MRSK_ROLE` - optional: role targeted, e.g. "web" -- `MRSK_PERFORMER` - the user performing the command -- `MRSK_DESTINATION` - the destination -- `MRSK_ROLE` - the specific role being targetted, if any -- `MRSK_MESSAGE` - full text of the action (e.g. "Deployed app@150b24f") +Use `mrsk broadcast` to test and troubleshoot your broadcast command: + +```bash +mrsk broadcast -m "test audit message" +``` ### Healthcheck diff --git a/lib/mrsk/cli/app.rb b/lib/mrsk/cli/app.rb index 9e783efb..0d13267e 100644 --- a/lib/mrsk/cli/app.rb +++ b/lib/mrsk/cli/app.rb @@ -60,7 +60,7 @@ class Mrsk::Cli::App < Mrsk::Cli::Base roles = MRSK.roles_on(host) roles.each do |role| - execute *MRSK.auditor(role: role).record("Stopped app"), verbosity: :debug + execute *MRSK.auditor.record("Stopped app", role: role), verbosity: :debug execute *MRSK.app(role: role).stop, raise_on_non_zero_exit: false end end @@ -107,7 +107,7 @@ class Mrsk::Cli::App < Mrsk::Cli::Base roles = MRSK.roles_on(host) roles.each do |role| - execute *MRSK.auditor(role: role).record("Executed cmd '#{cmd}' on app version #{version}"), verbosity: :debug + execute *MRSK.auditor.record("Executed cmd '#{cmd}' on app version #{version}", role: role), verbosity: :debug puts_by_host host, capture_with_info(*MRSK.app(role: role).execute_in_existing_container(cmd)) end end @@ -214,7 +214,7 @@ class Mrsk::Cli::App < Mrsk::Cli::Base roles = MRSK.roles_on(host) roles.each do |role| - execute *MRSK.auditor(role: role).record("Removed app container with version #{version}"), verbosity: :debug + execute *MRSK.auditor.record("Removed app container with version #{version}", role: role), verbosity: :debug execute *MRSK.app(role: role).remove_container(version: version) end end @@ -228,7 +228,7 @@ class Mrsk::Cli::App < Mrsk::Cli::Base roles = MRSK.roles_on(host) roles.each do |role| - execute *MRSK.auditor(role: role).record("Removed all app containers"), verbosity: :debug + execute *MRSK.auditor.record("Removed all app containers", role: role), verbosity: :debug execute *MRSK.app(role: role).remove_containers end end diff --git a/lib/mrsk/cli/base.rb b/lib/mrsk/cli/base.rb index 4617fe61..76f4f326 100644 --- a/lib/mrsk/cli/base.rb +++ b/lib/mrsk/cli/base.rb @@ -73,9 +73,7 @@ module Mrsk::Cli end def audit_broadcast(line) - if broadcast = MRSK.auditor.broadcast(line) - system(MRSK.auditor.broadcast_environment(line), broadcast) - end + run_locally { execute *MRSK.auditor.broadcast(line), verbosity: :debug } end def with_lock diff --git a/lib/mrsk/cli/main.rb b/lib/mrsk/cli/main.rb index a7832e73..aaba7cd8 100644 --- a/lib/mrsk/cli/main.rb +++ b/lib/mrsk/cli/main.rb @@ -200,6 +200,13 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base end end + desc "broadcast", "Broadcast an audit message" + option :message, aliases: "-m", type: :string, desc: "Audit mesasge", required: true + def broadcast + say "Broadcast: #{options[:message]}", :magenta + audit_broadcast options[:message] + end + desc "version", "Show MRSK version" def version puts Mrsk::VERSION diff --git a/lib/mrsk/commander.rb b/lib/mrsk/commander.rb index 99a0f7bb..4ef45f0a 100644 --- a/lib/mrsk/commander.rb +++ b/lib/mrsk/commander.rb @@ -84,8 +84,8 @@ class Mrsk::Commander Mrsk::Commands::Accessory.new(config, name: name) end - def auditor(role: nil) - Mrsk::Commands::Auditor.new(config, role: role) + def auditor(**details) + Mrsk::Commands::Auditor.new(config, **details) end def builder diff --git a/lib/mrsk/commands/auditor.rb b/lib/mrsk/commands/auditor.rb index 1ae51181..69ee03cd 100644 --- a/lib/mrsk/commands/auditor.rb +++ b/lib/mrsk/commands/auditor.rb @@ -1,36 +1,27 @@ -require "active_support/core_ext/time/conversions" +require "time" class Mrsk::Commands::Auditor < Mrsk::Commands::Base - attr_reader :role + attr_reader :details - def initialize(config, role: nil) + def initialize(config, **details) super(config) - @role = role + @details = default_details.merge(details) end # Runs remotely - def record(line) + def record(line, **details) append \ - [ :echo, tagged_record_line(line) ], + [ :echo, *audit_tags(**details), line ], audit_log_file end # Runs locally - def broadcast(line) + def broadcast(line, **details) if broadcast_cmd = config.audit_broadcast_cmd - [ broadcast_cmd, tagged_broadcast_line(line) ] + [ broadcast_cmd, *broadcast_args(line, **details), env: env_for(event: line, **details) ] end end - def broadcast_environment(line) - { - "MRSK_PERFORMER" => performer, - "MRSK_ROLE" => role, - "MRSK_DESTINATION" => config.destination, - "MRSK_MESSAGE" => line - } - end - def reveal [ :tail, "-n", 50, audit_log_file ] end @@ -40,35 +31,29 @@ class Mrsk::Commands::Auditor < Mrsk::Commands::Base [ "mrsk", config.service, config.destination, "audit.log" ].compact.join("-") end - def tagged_record_line(line) - tagged_line recorded_at_tag, performer_tag, role_tag, line + def default_details + { recorded_at: Time.now.utc.iso8601, + performer: `whoami`.chomp, + destination: config.destination } end - def tagged_broadcast_line(line) - tagged_line performer_tag, role_tag, destination_tag, line + def audit_tags(**details) + tags_for **self.details.merge(details) end - def tagged_line(*tags_and_line) - "'#{tags_and_line.compact.join(" ")}'" + def broadcast_args(line, **details) + "'#{broadcast_tags(**details).join(" ")} #{line}'" end - def recorded_at_tag - "[#{Time.now.to_fs(:db)}]" + def broadcast_tags(**details) + tags_for **self.details.merge(details).except(:recorded_at) end - def performer - `whoami`.strip + def tags_for(**details) + details.compact.values.map { |value| "[#{value}]" } end - def performer_tag - "[#{performer}]" - end - - def role_tag - "[#{role}]" if role - end - - def destination_tag - "[#{config.destination}]" if config.destination + def env_for(**details) + self.details.merge(details).compact.transform_keys { |detail| "MRSK_#{detail.upcase}" } end end diff --git a/lib/mrsk/commands/lock.rb b/lib/mrsk/commands/lock.rb index c8870238..6f84a5cc 100644 --- a/lib/mrsk/commands/lock.rb +++ b/lib/mrsk/commands/lock.rb @@ -1,5 +1,5 @@ require "active_support/duration" -require "active_support/core_ext/numeric/time" +require "time" class Mrsk::Commands::Lock < Mrsk::Commands::Base def acquire(message, version) @@ -49,7 +49,7 @@ class Mrsk::Commands::Lock < Mrsk::Commands::Base def lock_details(message, version) <<~DETAILS.strip - Locked by: #{locked_by} at #{Time.now.gmtime} + Locked by: #{locked_by} at #{Time.now.utc.iso8601} Version: #{version} Message: #{message} DETAILS diff --git a/lib/mrsk/sshkit_with_ext.rb b/lib/mrsk/sshkit_with_ext.rb index 075c2643..bc8de309 100644 --- a/lib/mrsk/sshkit_with_ext.rb +++ b/lib/mrsk/sshkit_with_ext.rb @@ -1,5 +1,6 @@ require "sshkit" require "sshkit/dsl" +require "active_support/core_ext/hash/deep_merge" class SSHKit::Backend::Abstract def capture_with_info(*args, **kwargs) @@ -9,4 +10,36 @@ class SSHKit::Backend::Abstract def puts_by_host(host, output, type: "App") puts "#{type} Host: #{host}\n#{output}\n\n" end + + # Our execution pattern is for the CLI execute args lists returned + # from commands, but this doesn't support returning execution options + # from the command. + # + # Support this by using kwargs for CLI options and merging with the + # args-extracted options. + module CommandEnvMerge + private + + # Override to merge options returned by commands in the args list with + # options passed by the CLI and pass them along as kwargs. + def command(*args_and_options) + options, args = args_and_options.partition { |a| a.is_a? Hash } + build_command(*args, **options.reduce(:deep_merge)) + end + + # Destructure options to pluck out env for merge + def build_command(args, env: nil, **options) + # Rely on native Ruby kwargs precedence rather than explicit Hash merges + SSHKit::Command.new(*args, **default_command_options, **options, env: env_for(env)) + end + + def default_command_options + { in: pwd_path, host: @host, user: @user, group: @group } + end + + def env_for(env) + @env.to_h.merge(env.to_h) + end + end + prepend CommandEnvMerge end diff --git a/test/cli/main_test.rb b/test/cli/main_test.rb index 746b9c9a..0e984f58 100644 --- a/test/cli/main_test.rb +++ b/test/cli/main_test.rb @@ -321,6 +321,19 @@ class CliMainTest < CliTestCase end end + test "broadcast" do + SSHKit::Backend::Abstract.any_instance.expects(:execute).with do |command, line, options, verbosity:| + command == "bin/audit_broadcast" && + line =~ /\A'\[[^\]]+\] message'\z/ && + options[:env].keys == %w[ MRSK_RECORDED_AT MRSK_PERFORMER MRSK_EVENT ] && + verbosity == :debug + end.returns("Broadcast audit message: message") + + run_command("broadcast", "-m", "message").tap do |output| + assert_match "Broadcast: message", output + end + end + test "version" do version = stdouted { Mrsk::Cli::Main.new.version } assert_equal Mrsk::VERSION, version diff --git a/test/commands/auditor_test.rb b/test/commands/auditor_test.rb index 6ba1c5e8..bcf62929 100644 --- a/test/commands/auditor_test.rb +++ b/test/commands/auditor_test.rb @@ -6,55 +6,65 @@ class CommandsAuditorTest < ActiveSupport::TestCase service: "app", image: "dhh/app", registry: { "username" => "dhh", "password" => "secret" }, servers: [ "1.1.1.1" ], audit_broadcast_cmd: "bin/audit_broadcast" } + + @auditor = new_command end test "record" do - assert_match \ - /echo '.* app removed container' >> mrsk-app-audit.log/, - new_command.record("app removed container").join(" ") + assert_equal [ + :echo, + "[#{@auditor.details[:recorded_at]}]", "[#{@auditor.details[:performer]}]", + "app removed container", + ">>", "mrsk-app-audit.log" + ], @auditor.record("app removed container") end test "record with destination" do - @destination = "staging" - - assert_match \ - /echo '.* app removed container' >> mrsk-app-staging-audit.log/, - new_command.record("app removed container").join(" ") + new_command(destination: "staging").tap do |auditor| + assert_equal [ + :echo, + "[#{auditor.details[:recorded_at]}]", "[#{auditor.details[:performer]}]", "[#{auditor.details[:destination]}]", + "app removed container", + ">>", "mrsk-app-staging-audit.log" + ], auditor.record("app removed container") + end end - test "record with role" do - @role = "web" + test "record with command details" do + new_command(role: "web").tap do |auditor| + assert_equal [ + :echo, + "[#{auditor.details[:recorded_at]}]", "[#{auditor.details[:performer]}]", "[#{auditor.details[:role]}]", + "app removed container", + ">>", "mrsk-app-audit.log" + ], auditor.record("app removed container") + end + end - assert_match \ - /echo '.* \[web\] app removed container' >> mrsk-app-audit.log/, - new_command.record("app removed container").join(" ") + test "record with arg details" do + assert_equal [ + :echo, + "[#{@auditor.details[:recorded_at]}]", "[#{@auditor.details[:performer]}]", "[value]", + "app removed container", + ">>", "mrsk-app-audit.log" + ], @auditor.record("app removed container", detail: "value") end test "broadcast" do - Mrsk::Commands::Auditor.any_instance.stubs(:performer).returns("bob") - @role = "web" - @destination = "staging" - - assert_equal \ - ["bin/audit_broadcast", "'[bob] [web] [staging] app removed container'"], - new_command.broadcast("app removed container") - end - - test "broadcast environment" do - Mrsk::Commands::Auditor.any_instance.stubs(:performer).returns("bob") - @role = "web" - @destination = "staging" - - env = new_command.broadcast_environment("app removed container") - - assert_equal "bob", env["MRSK_PERFORMER"] - assert_equal "web", env["MRSK_ROLE"] - assert_equal "staging", env["MRSK_DESTINATION"] - assert_equal "app removed container", env["MRSK_MESSAGE"] + assert_equal [ + "bin/audit_broadcast", + "'[#{@auditor.details[:performer]}] [value] app removed container'", + env: { + "MRSK_RECORDED_AT" => @auditor.details[:recorded_at], + "MRSK_PERFORMER" => @auditor.details[:performer], + "MRSK_EVENT" => "app removed container", + "MRSK_DETAIL" => "value" + } + ], @auditor.broadcast("app removed container", detail: "value") end private - def new_command - Mrsk::Commands::Auditor.new(Mrsk::Configuration.new(@config, destination: @destination, version: "123"), role: @role) + def new_command(destination: nil, **details) + Mrsk::Commands::Auditor.new(Mrsk::Configuration.new(@config, destination: destination, version: "123"), **details) end end diff --git a/test/fixtures/deploy_simple.yml b/test/fixtures/deploy_simple.yml index 520c138e..6c3a34ba 100644 --- a/test/fixtures/deploy_simple.yml +++ b/test/fixtures/deploy_simple.yml @@ -6,3 +6,4 @@ servers: registry: username: user password: pw +audit_broadcast_cmd: "bin/audit_broadcast" From 36a3b13bf49b35372fd26fe2d6c62d2af33258bb Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 4 May 2023 08:58:18 -0700 Subject: [PATCH 5/5] Fix SSHKit #command override args mangling --- lib/mrsk/sshkit_with_ext.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/mrsk/sshkit_with_ext.rb b/lib/mrsk/sshkit_with_ext.rb index bc8de309..cbd5dc23 100644 --- a/lib/mrsk/sshkit_with_ext.rb +++ b/lib/mrsk/sshkit_with_ext.rb @@ -22,9 +22,11 @@ class SSHKit::Backend::Abstract # Override to merge options returned by commands in the args list with # options passed by the CLI and pass them along as kwargs. - def command(*args_and_options) - options, args = args_and_options.partition { |a| a.is_a? Hash } - build_command(*args, **options.reduce(:deep_merge)) + def command(args, options) + more_options, args = args.partition { |a| a.is_a? Hash } + more_options << options + + build_command(args, **more_options.reduce(:deep_merge)) end # Destructure options to pluck out env for merge