Skip to content
This repository has been archived by the owner on Jan 29, 2022. It is now read-only.

Clean up configuration code #18

Merged
merged 2 commits into from
Nov 10, 2017
Merged

Clean up configuration code #18

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4b5bdf4
Clean up configuration code
Nov 3, 2017
b94ea29
Allow for configuration file to be merged with defaults
Nov 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion bin/puppet_webhook
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,15 @@ require 'webrick'
require 'webrick/https'
require 'puppet_webhook'

config_file '../config.yml'
server_conf = File.join(__dir__, '..', 'config', 'server.yml')

if File.exist?('/etc/puppet-webhook/server.yml')
Copy link
Member

Choose a reason for hiding this comment

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

I'd also like a command line argument which you can use to point it at.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we want to add it as part of this PR or another one?

Copy link
Member

Choose a reason for hiding this comment

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

Separate PR is fine.

Copy link
Member

Choose a reason for hiding this comment

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

The only problem with a conditional like this is it's all or nothing. To override a single setting, you have to override all. My preferred solution is to load the defaults first, then load & merge the custom config if it exists.

Copy link
Member Author

@dhollinger dhollinger Nov 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I agree with you on that. I'll have to look into and see if Sinatra::ConfigFile allows for merging of config from multiple files. Not sure it does.

config_file '/etc/puppet-webhook/server.yml'
elsif File.exist?(server_conf)
config_file server_conf
else
raise "Can't find server.yml. No such file or directory\n"
end

PIDFILE = settings.pidfile
LOCKFILE = settings.lockfile
Expand Down
38 changes: 0 additions & 38 deletions config.yml
View file
Open in desktop

This file was deleted.

31 changes: 31 additions & 0 deletions config/app.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
enable_mutex_lock: false
user: puppet
pass: puppet
protected: true
client_cfg: "/var/lib/peadmin/.mcollective"
client_timeout: "120"
use_mco_ruby: false
use_mcollective: false
slack_webhook: false
default_branch: production
discovery_timeout: '10'
ignore_environments: []
prefix: false
prefix_command: "/bin/echo example"
r10k_deploy_arguments: "-pv"
allow_uppercase: true
github_secret:
repository_events:
server_type: simple
logfile: /var/log/puppet-webhook/access.log
pidfile: /var/run/puppet-webhook/webhook.pid
lockfile: /var/run/puppet-webhook/webhook.lock
approot: './'
server_software: PuppetWebhook
bind_address: 0.0.0.0
port: 8088
enable_ssl: false
verify_ssl: false
public_key_path: ''
private_key_path: ''
command_prefix: 'umask 0022;'
13 changes: 13 additions & 0 deletions config/server.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
server_type: simple
logfile: /var/log/puppet-webhook/access.log
pidfile: /var/run/puppet-webhook/webhook.pid
lockfile: /var/run/puppet-webhook/webhook.lock
approot: './'
server_software: PuppetWebhook
bind_address: 0.0.0.0
port: 8088
enable_ssl: false
verify_ssl: false
public_key_path: ''
private_key_path: ''
command_prefix: 'umask 0022;'
11 changes: 10 additions & 1 deletion lib/puppet_webhook.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,20 @@
require 'parsers/webhook_json_parser'

class PuppetWebhook < Sinatra::Base # rubocop:disable Style/Documentation
set :root, File.dirname(__FILE__)
use Rack::Parser,
parsers: { 'application/json' => Sinatra::Parsers::WebhookJsonParser.new },
handlers: { 'application/json' => proc { |e, type| [400, { 'Content-Type' => type }, [{ error: e.to_s }.to_json]] } }
register Sinatra::ConfigFile
config_file '../config.yml'

app_conf = File.join(__dir__, '..', 'config', 'app.yml')
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above, I'd prefer to load defaults, then merge overrides.

if File.exist?('/etc/puppet-webhook/app.yml')
Copy link
Member

Choose a reason for hiding this comment

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

Should we have a CONFIG_DIR setting somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Would it provide us with anything useful?

Seems like it would just be another config option just to support the config files. Seems a bit redundant. Unless you mean to set it to one of [ static value, CLI passed arg ]

Copy link
Member

Choose a reason for hiding this comment

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

Running from a git checkout might be easier, or as non-root user. FreeBSD has /usr/local/etc instead. There are some odd setups :)

config_file '/etc/puppet-webhook/app.yml'
elsif File.exist?(app_conf)
config_file app_conf
else
raise "Can't load app.yml: No such file or directory\n"
end

set :static, false
set :lock, true if settings.enable_mutex_lock
Expand Down
4 changes: 2 additions & 2 deletions puppet-webhook.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ Gem::Specification.new do |spec|
'CHANGELOG.md',
'README.md',
'LICENSE',
'config.yml',
'config/*',
'lib/**/*',
'bin/*'
]
spec.homepage = 'https://github.com/voxpupuli/puppet-webhook'
spec.license = 'apache'
spec.executables = ['puppet_webhook']
spec.require_paths = ['lib']
spec.require_paths = %w[lib config]
spec.add_runtime_dependency 'json'
spec.add_runtime_dependency 'mcollective-client'
spec.add_runtime_dependency 'rack-parser'
Expand Down