From a1706bc910b96014cc25ab03a022a31f2fef91c6 Mon Sep 17 00:00:00 2001 From: Matt Fields Date: Mon, 6 May 2019 14:51:12 -0400 Subject: [PATCH 1/6] Replace exec with Open3.capture3 The deploy and deploy_module methods are already running in a detached forked process, and therefore running the command in the background is redundant. Additionally, by running the command in the background, we lose the ability to capture the exit code, and assume it always succeeds. Running the command in-band additionally allows us to correctly call the slack and rocketchat notifiers, which previously were silenced. --- templates/webhook.bin.erb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/templates/webhook.bin.erb b/templates/webhook.bin.erb index a2c89989..6081ead0 100755 --- a/templates/webhook.bin.erb +++ b/templates/webhook.bin.erb @@ -261,8 +261,7 @@ class Server < Sinatra::Base file.flock(File::LOCK_EX) message = "triggered: #{command}" - exec "#{command} &" - exit_status = 0 + stdout, stderr, exit_status = Open3.capture3(command) raise "#{stdout}\n#{stderr}" if exit_status != 0 end message From 4fc5b0cdcf951c4245c48eb9eb8c8bbaacf2f5f2 Mon Sep 17 00:00:00 2001 From: Matt Fields Date: Mon, 6 May 2019 14:58:11 -0400 Subject: [PATCH 2/6] Call generate_types after notifiers When generate_types is called before the slack/rocketchat notifiers, it causes the chat notifications to be in reverse order, since the generate_types command will also send its own notifications. By calling generate_types after the chat notifiers, the messages appear in the correct order in chat. --- templates/webhook.bin.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/webhook.bin.erb b/templates/webhook.bin.erb index 6081ead0..cfa2a823 100755 --- a/templates/webhook.bin.erb +++ b/templates/webhook.bin.erb @@ -434,11 +434,11 @@ class Server < Sinatra::Base end status_message = {:status => :success, :message => message.to_s, :branch => branch, :status_code => 202} $logger.info("message: #{message} branch: #{branch}") + notify_slack(status_message) if slack? + notify_rocketchat(status_message) if rocketchat? unless deleted generate_types(branch) if types? end - notify_slack(status_message) if slack? - notify_rocketchat(status_message) if rocketchat? status_message.to_json rescue => e status_message = {:status => :fail, :message => e.message, :trace => e.backtrace, :branch => branch, :status_code => 500} From b9c2a2736642f3e4371b41bcfe6bcf2e0df2bf4f Mon Sep 17 00:00:00 2001 From: Matt Fields Date: Mon, 6 May 2019 15:03:56 -0400 Subject: [PATCH 3/6] Revert chat status code back to 200 Since the chat notifiers are the only thing that actually uses this value (because the deploy and deploy_module methods are run in detached forked processes), the actual value doesn't really matter. There are check in the notify_* methods that check for either a 200 or 500 value, so values passed in should be one of these two values. Keeping the chat status codes at 202 results in broken chat messages, since the color and attachment are not defined. --- templates/webhook.bin.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/webhook.bin.erb b/templates/webhook.bin.erb index cfa2a823..63a62b99 100755 --- a/templates/webhook.bin.erb +++ b/templates/webhook.bin.erb @@ -400,7 +400,7 @@ class Server < Sinatra::Base end message = run_command(command) $logger.info("message: #{message} module_name: #{module_name}") - status_message = {:status => :success, :message => message.to_s, :module_name => module_name, :status_code => 202} + status_message = {:status => :success, :message => message.to_s, :module_name => module_name, :status_code => 200} notify_slack(status_message) if slack? notify_rocketchat(status_message) if rocketchat? status_message.to_json @@ -432,7 +432,7 @@ class Server < Sinatra::Base end message = run_command(command) end - status_message = {:status => :success, :message => message.to_s, :branch => branch, :status_code => 202} + status_message = {:status => :success, :message => message.to_s, :branch => branch, :status_code => 200} $logger.info("message: #{message} branch: #{branch}") notify_slack(status_message) if slack? notify_rocketchat(status_message) if rocketchat? From f3f155745554b85a94a96e6e1e318523b374843b Mon Sep 17 00:00:00 2001 From: Matt Fields Date: Mon, 6 May 2019 15:08:56 -0400 Subject: [PATCH 4/6] Return 202 HTTP status from payload and module endpoints Since the deploy and deploy_module methods are now detached forked processes, their return codes and output are no longer returned automatically to the payload and module endpoints, and we must manually define a return value for them. Since the work has been sent off to an out-of-band process at the time the webhook call returns, a 202 response is acceptable. --- templates/webhook.bin.erb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/templates/webhook.bin.erb b/templates/webhook.bin.erb index 63a62b99..b47b977a 100755 --- a/templates/webhook.bin.erb +++ b/templates/webhook.bin.erb @@ -123,6 +123,8 @@ class Server < Sinatra::Base module_name = sanitize_input(module_name) $logger.info("Deploying module #{module_name} in the background.") Process.detach(fork { deploy_module(module_name) }) + + return 202, {:status => :accepted, :message => 'accepted' }.to_json end # Simulate a github post: @@ -213,6 +215,8 @@ class Server < Sinatra::Base else $logger.info("Deploying environment #{env} in the background") Process.detach(fork { deploy(env, deleted) }) + + return 202, {:status => :accepted, :message => 'accepted' }.to_json end end From a76c565ac19b7855c26308ce6c22c7aa0dee83c0 Mon Sep 17 00:00:00 2001 From: Matt Fields Date: Mon, 6 May 2019 15:40:38 -0400 Subject: [PATCH 5/6] Update chat messages to be more accurate for the operation Since the generate_types method defines an :environment key rather than a :module or :branch, the target value is not defined. We fix that by looking for an :environment key and setting target. Additionally, the phrasing is not valid for generating types, so the phrasing has been updated to better reflect what actually happened. --- templates/webhook.bin.erb | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/templates/webhook.bin.erb b/templates/webhook.bin.erb index b47b977a..19e3d7fe 100755 --- a/templates/webhook.bin.erb +++ b/templates/webhook.bin.erb @@ -317,15 +317,20 @@ class Server < Sinatra::Base http_options: http_options end - if status_message[:branch] + if status_message[:environment] # generate_types + target = status_message[:environment] + msg = "Generation of Puppet resource types for #{target}" + elsif status_message[:branch] # deploy target = status_message[:branch] - elsif status_message[:module] + msg = "Deployment of Puppet environment #{target}" + elsif status_message[:module] # deploy_module target = status_message[:module] + msg = "Deployment of Puppet module #{target}" end message = { author: 'r10k for Puppet', - title: "r10k deployment of Puppet environment #{target}" + title: msg } case status_message[:status_code] @@ -343,7 +348,7 @@ class Server < Sinatra::Base ) end - notifier.post text: message[:fallback], attachments: [message] + notifier.post text: 'r10k deployment', attachments: [message] end def notify_rocketchat(status_message) @@ -363,16 +368,21 @@ class Server < Sinatra::Base notifier.username = rocketchat_user notifier.channel = rocketchat_channel - if status_message[:branch] + if status_message[:environment] # generate_types + target = status_message[:environment] + msg = "Generation of Puppet resource types for #{target}" + elsif status_message[:branch] # deploy target = status_message[:branch] - elsif status_message[:module] + msg = "Deployment of Puppet environment #{target}" + elsif status_message[:module] # deploy_module target = status_message[:module] + msg = "Deployment of Puppet module #{target}" end - message = "r10k deployment of Puppet environment/module #{target}" + message = "r10k deployment" attachments = { author: 'r10k for Puppet', - title: "r10k deployment of Puppet environment #{target}", + title: msg, } case status_message[:status_code] From fa339708c502fcb28cc56fc7db863445994bd8a8 Mon Sep 17 00:00:00 2001 From: Matt Fields Date: Mon, 6 May 2019 15:47:27 -0400 Subject: [PATCH 6/6] Add support for customizing the slack icon --- manifests/params.pp | 1 + manifests/webhook/config.pp | 2 ++ templates/webhook.bin.erb | 8 +++++++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/manifests/params.pp b/manifests/params.pp index eda3cc16..b2ae8e23 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -150,6 +150,7 @@ $webhook_slack_channel = undef $webhook_slack_username = undef $webhook_slack_proxy_url = undef + $webhook_slack_icon = undef $webhook_rocketchat_webhook = undef $webhook_rocketchat_channel = undef $webhook_rocketchat_username = undef diff --git a/manifests/webhook/config.pp b/manifests/webhook/config.pp index 162b1753..dc2d8547 100644 --- a/manifests/webhook/config.pp +++ b/manifests/webhook/config.pp @@ -39,6 +39,7 @@ $slack_channel = $r10k::params::webhook_slack_channel, $slack_username = $r10k::params::webhook_slack_username, $slack_proxy_url = $r10k::params::webhook_slack_proxy_url, + $slack_icon = $r10k::params::webhook_slack_icon, $rocketchat_webhook = $r10k::params::webhook_rocketchat_webhook, $rocketchat_channel = $r10k::params::webhook_rocketchat_channel, $rocketchat_username = $r10k::params::webhook_rocketchat_username, @@ -87,6 +88,7 @@ 'slack_channel' => $slack_channel, 'slack_username' => $slack_username, 'slack_proxy_url' => $slack_proxy_url, + 'slack_icon' => $slack_icon, 'rocketchat_webhook' => $rocketchat_webhook, 'rocketchat_channel' => $rocketchat_channel, 'rocketchat_username' => $rocketchat_username, diff --git a/templates/webhook.bin.erb b/templates/webhook.bin.erb index 19e3d7fe..835f2f6c 100755 --- a/templates/webhook.bin.erb +++ b/templates/webhook.bin.erb @@ -299,6 +299,12 @@ class Server < Sinatra::Base slack_user = 'r10k' end + if $config['slack_icon'] + slack_icon = $config['slack_icon'] + else + slack_icon = ':ocean:' + end + if $config['slack_proxy_url'] uri = URI($config['slack_proxy_url']) http_options = { @@ -313,7 +319,7 @@ class Server < Sinatra::Base notifier = Slack::Notifier.new $config['slack_webhook'] do defaults channel: slack_channel, username: slack_user, - icon_emoji: ":ocean:", + icon_emoji: slack_icon, http_options: http_options end