Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Additional aws credential providers #11

Merged
merged 8 commits into from
Jul 22, 2015

Conversation

ajohnstone
Copy link
Contributor

i2eaqzi

Problem

Credentials for AWS keys must be exposed on instances, when I do not have to provide credentials for instances that have AWS Instance Profiles attached to them. Credentials can be removed altogether _(Improves security)_. Arguments are also passed in fixed form.

Solution

Add additional methods for providing security to Aws ruby client, as well as changing the argument fields to accommodate differing credentials #10

More information on using instance profiles can be found here.
http://docs.aws.amazon.com/IAM/latest/UserGuide/roles-usingrole-ec2instance.html

Just to note I did not implement the following, however if necessary can be added if required.

@ajohnstone
Copy link
Contributor Author

Resolved #10

@ajohnstone
Copy link
Contributor Author

@DaveBlooman / @revett any chance of a review/merge?

Thanks

@revett
Copy link
Contributor

revett commented Jun 8, 2015

@ajohnstone - we're crazy busy at the moment, so will have to wait until later on this week for a review. Sorry.

To speed things up could you add a Problem/Solution description to the PR? Example: https://github.com/DaveBlooman/linkey/pull/19

@ajohnstone
Copy link
Contributor Author

Thanks @revett also added the obligatory image ;)

@ajohnstone
Copy link
Contributor Author

@revett / @DaveBlooman any chance of a review sometime this week ?

class_option :secret_access_key, :desc => 'AWS secret_key_id', :required => false
class_option :region, :desc => 'AWS region', :required => false

desc "send_metrics [metrics_file]", "Gets metrics from Cloudwatch and sends them to influx"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the CLI output should still show cloudwatch-sender send_metrics [metrics_file] [key_id] [access_key] [region]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any particular reason to still show this?

--access-key-id + --secret-access-key are optional if you use --provider=iam

bundle exec cloudwatch-sender 
Commands:
  cloudwatch-sender continuous [metrics_file] [sleep time]  # Continuously sends metrics to Influx/Cloudwatch
  cloudwatch-sender help [COMMAND]                          # Describe available commands or one specific command
  cloudwatch-sender send_metrics [metrics_file]             # Gets metrics from Cloudwatch and sends them to influx

Options:
  [--provider=PROVIDER]                    # AWS security provider
                                           # Possible values: iam, instance_profile
  [--access-key-id=ACCESS_KEY_ID]          # AWS access_key_id
  [--secret-access-key=SECRET_ACCESS_KEY]  # AWS secret_key_id
  [--region=REGION]                        # AWS region

Granted the following form does not show the parameters that are available if you specify the action without the correct arguments.

bundle exec cloudwatch-sender continuous 
ERROR: "cloudwatch-sender continuous" was called with no arguments
Usage: "cloudwatch-sender continuous [metrics_file] [sleep time]"

@dblooman
Copy link
Contributor

I was thinking something list this

require "thor"
require "logger"
require "cloudwatch/sender/ec2"
require "cloudwatch/sender/credentials"
require "cloudwatch/sender/metric_definition"
require "cloudwatch/sender/fetcher/base"
require "cloudwatch/sender/fetcher/ec2"
require "cloudwatch/sender/fetcher/sqs"
require "cloudwatch/sender/fetcher/custom"

module Cloudwatch
  module Sender
    class CLI < Thor
      include Thor::Actions

      class_option :provider, :desc => "AWS security provider", :required => false
      class_option :access_key_id, :desc => "AWS access_key_id", :required => false
      class_option :secret_access_key, :desc => "AWS secret_key_id", :required => false
      class_option :region, :desc => "AWS region", :required => false

      desc "send_metrics [metrics_file]", "Gets metrics from Cloudwatch and sends them to influx"
      def send_metrics(metrics_file, opts = {})
        setup_aws(options.merge(opts), opts["provider"])
        MetricDefinition.metric_type load_metrics(metrics_file)
      end

      desc "continuous [metrics_file] [sleep time]", "Continuously sends metrics to Influx/Cloudwatch"
      def continuous(metrics_file, sleep_time = 60, opts = {})
        logger = Logger.new(STDOUT)

        loop do
          begin
            send_metrics(metrics_file, options.merge(opts))
            sleep sleep_time.to_i
          rescue RequiredArgumentMissingError, ArgumentError => e
            logger.error("Required argument invalid or missing '#{e}'")
            exit(1)
          rescue Aws::Errors::MissingCredentialsError => e
            logger.error("#{e}")
            exit(1)
          rescue => e
            logger.debug("Unable to complete operation #{e}")
          end
        end
      end

      no_commands do
        def load_metrics(metrics_file)
          YAML.load(File.open(metrics_file))
        end

        def setup_aws(options, provider)
          SetupAwsCredentials.send("#{validate_provider(provider)}".to_sym, options)
        end

        def validate_provider(provider)
          return "access_key_id" if provider.nil?
          if %w(iam instance_profile).include? provider.downcase
            provider.downcase
          else
            fail ArgumentError.new("'--provider' invalid argument '#{options['provider']}'")
          end
        end
      end
    end
  end
end
module Cloudwatch
  module Sender
    class SetupAwsCredentials
      def self.iam(options)
        provider(options)
      end

      def self.instance_profile(options)
        provider(options)
      end

      def self.access_key_id(options)
        credentials = Aws::Credentials.new(options["access_key_id"], options["secret_access_key"])
        Aws.config.update(:region => (options["region"] || ENV["AWS_REGION"]), :credentials => credentials)
      rescue
        RequiredArgumentMissingError.new("'--access_key_id' and '--secret_access_key' required")
      end

      private

      def self.credentials
        Aws::InstanceProfileCredentials.new
      end

      def self.provider(options)
        Aws.config.update(:region => (options["region"] || ENV["AWS_REGION"]), :credentials => credentials)
      end
    end
  end
end

Could you check with an IAM profile @ajohnstone

@ajohnstone
Copy link
Contributor Author

@DaveBlooman Will look to do this week :)

@revett
Copy link
Contributor

revett commented Jun 22, 2015

👍

@ajohnstone
Copy link
Contributor Author

@DaveBlooman I've applied your changes as suggested and tested that they work correctly, which they do :)

@revett revett self-assigned this Jul 9, 2015
```

**Note** - the default `$INTERVAL` is 60 seconds.

```sh
cloudwatch-sender continuous /path/to/config.yaml --region=eu-west-1 $INTERVAL --provider=iam
Copy link
Contributor

Choose a reason for hiding this comment

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

Some more context around the two different authentication methods would be great.

@revett
Copy link
Contributor

revett commented Jul 9, 2015

@ajohnstone Thanks for the changes, I have left a few comments.

One final thing, could you run Rubocop over your changes please? Just to make styling inline with ours. All instructions can be found here github.com/BBC-News/rubocop-config

Thanks!

@ajohnstone
Copy link
Contributor Author

@revett

  • The project does not currently adhere to the rubocop rules that you point to. I've updated most of the failures. See Resolve - run rubocop on each commit on travis #15 #16
  • Removed minor debugging
  • Regarding the comment on "Some more context around the two different authentication methods would be great.", I've added documentation to the README.md file and also the aws security provider lists the options available. As described above....

--access-key-id + --secret-access-key are optional if you use --provider=iam

bundle exec cloudwatch-sender 
Commands:
  cloudwatch-sender continuous [metrics_file] [sleep time]  # Continuously sends metrics to Influx/Cloudwatch
  cloudwatch-sender help [COMMAND]                          # Describe available commands or one specific command
  cloudwatch-sender send_metrics [metrics_file]             # Gets metrics from Cloudwatch and sends them to influx

Options:
  [--provider=PROVIDER]                    # AWS security provider
                                           # Possible values: iam, instance_profile
  [--access-key-id=ACCESS_KEY_ID]          # AWS access_key_id
  [--secret-access-key=SECRET_ACCESS_KEY]  # AWS secret_key_id
  [--region=REGION]                        # AWS region

Granted the following form does not show the parameters that are available if you specify the action without the correct arguments.

bundle exec cloudwatch-sender continuous 
ERROR: "cloudwatch-sender continuous" was called with no arguments
Usage: "cloudwatch-sender continuous [metrics_file] [sleep time]"

@revett
Copy link
Contributor

revett commented Jul 13, 2015

Really nice work @ajohnstone - will aim to get this merged this week.

dblooman added a commit that referenced this pull request Jul 22, 2015
…iders

Additional aws credential providers
@dblooman dblooman merged commit db73f1a into bbc:master Jul 22, 2015
@ajohnstone ajohnstone deleted the additional-aws-credential-providers branch July 22, 2015 10:45
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants