New check for checking HTTP responses for each instances on a ELB #239
Conversation
|
Thanks for the contribution! I will review it shortly. |
CHANGELOG.md
Outdated
| @@ -5,6 +5,10 @@ This CHANGELOG follows the format listed at [Keep A Changelog](http://keepachang | |||
|
|
|||
| ## [Unreleased] | |||
|
|
|||
| ## [8.4.0] - 2017-09-29 | |||
There was a problem hiding this comment.
Please leave the version bumping and dating to the maintainers. All changes should stay under ## [Unreleased] until a maintainer is ready to create a release. https://github.com/sensu-plugins/community/blob/master/HOW_WE_CHANGELOG.md#unreleased-change
| # | ||
|
|
||
| require 'sensu-plugin/check/cli' | ||
| require 'aws-sdk-v1' |
There was a problem hiding this comment.
with this being a new check we should write this against the v2 api. Eventually we will be going through and updating the checks for v2 and dropping v1 support due to version conflicts with newer versions of some gems (json if I recall correctly) with newer sensu setups.
There was a problem hiding this comment.
yes this is a really big issue which prevents us from upgrading to 1.x
There was a problem hiding this comment.
Can we re-write this to be against v2?
|
|
||
| # check each node by calling a http endpoint | ||
| class HTTPCheckELBNodes < Sensu::Plugin::Check::CLI | ||
| option :aws_access_key, |
There was a problem hiding this comment.
We are trending towards not passing the key on the CLI at all due to the number of people not properly using token substitution and lack of redaction. If you use the v2 and: https://github.com/sensu-plugins/sensu-plugins-aws/blob/8.3.0/lib/sensu-plugins-aws/common.rb you will get env and IAM which are the two methods we are going to use going forward.
| description: "AWS Access Key. or use ENV['AWS_ACCESS_KEY']", | ||
| default: ENV['AWS_ACCESS_KEY'] | ||
|
|
||
| option :aws_secret_access_key, |
bin/check-elb-instances-endpoints.rb
Outdated
| description: 'path to check', | ||
| required: true | ||
|
|
||
| option :use_https, |
There was a problem hiding this comment.
I'd prefer to default to a secure setup and have people override when they want to be insecure. I'd also accept just exposing an option such as:
option :protocol,
long: '--protocol PROTO',
in: %(http https),
default: 'https'
majormoses
added
Status: Awaiting Response
Type: Enhancement
and removed
Enhancement
labels
|
Just checking if you still planned on re-writing it against sdk v2 as we are in the process of re-writing existing ones and will not accept a new check with v1. You can read the meta issue here: #240 |
|
@majormoses yep, I would like to. |
|
Just a friendly reminder this is still open and if possibly I'd like to get this across the finish line but it's in your court. |
|
👍 |
Pull Request Checklist
Is this in reference to an existing issue?
General
Update Changelog following the conventions laid out on Keep A Changelog
Update README with any necessary configuration snippets
Binstubs are created if needed
RuboCop passes
Existing tests pass
New Plugins
Tests
Add the plugin to the README
Does it have a complete header as outlined here
Purpose
Check a HTTP endpoint for a HTTP200 for each instance attached to a ELB. I plan on using a separate endpoint for non critical health issues.
Known Compatibility Issues