diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index bf2a8eff..5a9e5df5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,20 +15,15 @@ jobs: strategy: fail-fast: false matrix: - ruby-version: [2.3, 2.4, 2.5, 2.6, 2.7] + ruby-version: [2.3, 2.4, 2.5, 2.6, 2.7, 3.1, 3.2, 3.3, 3.4] + env: + RUBY_VERSION: ${{ matrix.ruby-version }} steps: - name: Check out code uses: actions/checkout@v4 - - name: Setup ruby - uses: ruby/setup-ruby@v1 - with: - ruby-version: ${{ matrix.ruby-version }} - bundler: 1.17.3 - bundler-cache: true - - name: Test - run: bundle exec rake + run: make test - name: Sync README run: | diff --git a/Dockerfile b/Dockerfile index c3a8ffd2..64b9e559 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,8 +1,11 @@ -FROM ruby:2.7.8 +ARG RUBY_VERSION=2.3.1 +FROM ruby:${RUBY_VERSION} -# Install and use an compatible bundler -ENV BUNDLER_VERSION=1.17.3 -RUN gem install bundler -v "${BUNDLER_VERSION}" +ARG BUNDLER_VERSION=1.17.3 +ENV BUNDLER_VERSION=${BUNDLER_VERSION} +RUN if [ "${BUNDLER_VERSION}" != "" ] ; then \ + gem install bundler -v "${BUNDLER_VERSION}" ; \ + fi # Install required gems before copying in code # to avoid re-installing gems when developing @@ -12,7 +15,7 @@ COPY Gemfile.lock /app COPY aptible-cli.gemspec /app # We reference the version, so copy that in, too -CMD mkdir -p /app/lib/aptible/cli/ +RUN mkdir -p /app/lib/aptible/cli/ COPY lib/aptible/cli/version.rb /app/lib/aptible/cli/ RUN bundle install diff --git a/Gemfile b/Gemfile index 00775107..11a1724d 100644 --- a/Gemfile +++ b/Gemfile @@ -7,5 +7,8 @@ group :test do gem 'webmock' end +gem 'aptible-api', git: 'https://github.com/aptible/aptible-api-ruby', branch: 'ext-aws-check' +gem 'aptible-auth', git: 'https://github.com/aptible/aptible-auth-ruby', branch: 'concur-ruby-version-fix' + # Specify your gem's dependencies in aptible-cli.gemspec gemspec diff --git a/Gemfile.lock b/Gemfile.lock index 9a176935..a8cd49b3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,15 +1,36 @@ +GIT + remote: https://github.com/aptible/aptible-api-ruby + revision: 8fe6db7b81c5d6fcbb5b332b58befee1886c2029 + branch: ext-aws-check + specs: + aptible-api (1.10.0) + aptible-auth + aptible-resource + gem_config + multipart-post (< 2.2.0) + +GIT + remote: https://github.com/aptible/aptible-auth-ruby + revision: e4d569863a6533507ffe7544f3961b8d89f6a520 + branch: concur-ruby-version-fix + specs: + aptible-auth (1.3.0) + aptible-resource (~> 1.0) + concurrent-ruby (~> 1.1.9) + gem_config + multipart-post (= 2.1.1) + oauth2 (= 2.0.9) + PATH remote: . specs: aptible-cli (0.25.1) activesupport (>= 4.0, < 6.0) aptible-api (~> 1.10.0) - aptible-auth (~> 1.2.5) + aptible-auth (~> 1.3) aptible-billing (~> 1.0) aptible-resource (~> 1.1) - aws-eventstream (~> 1.1.1) - aws-sdk (~> 2.0) - aws-sigv4 (~> 1.2.4) + aws-sdk-s3 (~> 1.0) bigdecimal (~> 1.3.5) cbor chronic_duration (~> 0.10.6) @@ -35,16 +56,6 @@ GEM tzinfo (~> 1.1) addressable (2.8.0) public_suffix (>= 2.0.2, < 5.0) - aptible-api (1.10.0) - aptible-auth - aptible-resource - gem_config - multipart-post (< 2.2.0) - aptible-auth (1.2.5) - aptible-resource (~> 1.0) - gem_config - multipart-post (= 2.1.1) - oauth2 (= 1.4.7) aptible-billing (1.0.1) activesupport (>= 4.0, < 6.0) aptible-resource (~> 1.0) @@ -60,16 +71,26 @@ GEM rake (>= 10, < 13.0) rubocop (= 0.42.0) ast (2.4.2) - aws-eventstream (1.1.1) - aws-sdk (2.11.632) - aws-sdk-resources (= 2.11.632) - aws-sdk-core (2.11.632) - aws-sigv4 (~> 1.0) - jmespath (~> 1.0) - aws-sdk-resources (2.11.632) - aws-sdk-core (= 2.11.632) - aws-sigv4 (1.2.4) + aws-eventstream (1.4.0) + aws-partitions (1.1182.0) + aws-sdk-core (3.237.0) + aws-eventstream (~> 1, >= 1.3.0) + aws-partitions (~> 1, >= 1.992.0) + aws-sigv4 (~> 1.9) + base64 + bigdecimal + jmespath (~> 1, >= 1.6.1) + logger + aws-sdk-kms (1.117.0) + aws-sdk-core (~> 3, >= 3.234.0) + aws-sigv4 (~> 1.5) + aws-sdk-s3 (1.203.1) + aws-sdk-core (~> 3, >= 3.234.0) + aws-sdk-kms (~> 1) + aws-sigv4 (~> 1.5) + aws-sigv4 (1.12.1) aws-eventstream (~> 1, >= 1.0.2) + base64 (0.3.0) bigdecimal (1.3.5) cbor (0.5.10.1) chronic_duration (0.10.6) @@ -93,26 +114,31 @@ GEM git (1.7.0) rchardet (~> 1.8) hashdiff (1.1.1) + hashie (5.0.0) httpclient (2.8.3) + httplog (1.5.0) + rack (>= 1.0) + rainbow (>= 2.0.0) i18n (0.9.5) concurrent-ruby (~> 1.0) jmespath (1.6.2) json (2.5.1) jwt (2.3.0) + logger (1.7.0) method_source (1.1.0) minitest (5.12.0) - multi_json (1.15.0) multi_xml (0.6.0) multipart-post (2.1.1) net-http-persistent (3.1.0) connection_pool (~> 2.2) numerizer (0.1.1) - oauth2 (1.4.7) - faraday (>= 0.8, < 2.0) + oauth2 (2.0.9) + faraday (>= 0.17.3, < 3.0) jwt (>= 1.0, < 3.0) - multi_json (~> 1.3) multi_xml (~> 0.5) - rack (>= 1.2, < 3) + rack (>= 1.2, < 4) + snaky_hash (~> 2.0) + version_gem (~> 1.1) parser (2.7.2.0) ast (~> 2.4.1) powerpack (0.1.3) @@ -146,6 +172,9 @@ GEM ruby-progressbar (~> 1.7) unicode-display_width (~> 1.0, >= 1.0.1) ruby-progressbar (1.13.0) + snaky_hash (2.0.3) + hashie (>= 0.1.0, < 6) + version_gem (>= 1.1.8, < 3) stripe (4.24.0) faraday (~> 0.13) net-http-persistent (~> 3.0) @@ -161,6 +190,7 @@ GEM thread_safe (~> 0.1) unicode-display_width (1.8.0) uri_template (0.7.0) + version_gem (1.1.9) webmock (3.16.2) addressable (>= 2.8.0) crack (>= 0.3.2) @@ -171,11 +201,14 @@ PLATFORMS DEPENDENCIES activesupport (~> 4.0) + aptible-api! + aptible-auth! aptible-cli! aptible-tasks (~> 0.5.8) bundler (~> 1.3) climate_control (= 0.0.3) fabrication (~> 2.15.2) + httplog (< 1.6) pry rack (~> 1.0) rake diff --git a/Makefile b/Makefile index 00ee47cd..4b0b305b 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,58 @@ + +export COMPOSE_IGNORE_ORPHANS ?= true +export RUBY_VERSION ?= 2.3.1 +RUBY_VERSION_MAJOR = $(word 1,$(subst ., ,$(RUBY_VERSION))) +export BUNDLER_VERSION ?= +ifeq ($(BUNDLER_VERSION),) +ifeq ($(RUBY_VERSION_MAJOR),2) +export BUNDLER_VERSION = 1.17.3 +endif +endif +export COMPOSE_PROJECT_NAME ?= aptible-cli-$(subst .,_,$(RUBY_VERSION)) + +## Build and pull docker compose images build: docker compose build --pull +## Open shell in a docker container, supports CMD= bash: build - docker compose run cli bash + $(MAKE) run CMD=bash + +CMD ?= bash +## Run command in a docker container, supports CMD= +run: + docker compose run cli $(CMD) +## Run tests in a docker container, supports ARGS= test: build - docker compose run cli bundle exec rake + $(MAKE) test-direct ARGS="$(ARGS)" + +## Run tests in a docker container without building, supports ARGS= +test-direct: + docker compose run cli bundle exec rake $(ARGS) + +## Run rubocop in a docker container, supports ARGS= +lint: build + $(MAKE) lint-direct ARGS="$(ARGS)" + +## Run rubocop in a docker container without building, supports ARGS= +lint-direct: + docker compose run cli bundle exec rake rubocop $(ARGS) + +## Clean up docker compose resources +clean: + docker compose down --remove-orphans --volumes + +## Alias for clean +down: clean + +## Show this help message +help: + @echo "\n\033[1;34mAvailable targets:\033[0m\n" + @awk 'BEGIN {FS = ":"; prev = ""} \ + /^## / {prev = substr($$0, 4); next} \ + /^[a-zA-Z_-]+:/ {if (prev != "") printf " \033[1;36m%-20s\033[0m %s\n", $$1, prev; prev = ""} \ + {prev = ""}' $(MAKEFILE_LIST) | sort + @echo .PHONY: build bash test diff --git a/aptible-cli.gemspec b/aptible-cli.gemspec index 35922d76..6a49c046 100644 --- a/aptible-cli.gemspec +++ b/aptible-cli.gemspec @@ -22,12 +22,10 @@ Gem::Specification.new do |spec| spec.add_dependency 'activesupport', '>= 4.0', '< 6.0' spec.add_dependency 'aptible-api', '~> 1.10.0' - spec.add_dependency 'aptible-auth', '~> 1.2.5' + spec.add_dependency 'aptible-auth', '~> 1.3' spec.add_dependency 'aptible-billing', '~> 1.0' spec.add_dependency 'aptible-resource', '~> 1.1' - spec.add_dependency 'aws-eventstream', '~> 1.1.1' - spec.add_dependency 'aws-sdk', '~> 2.0' - spec.add_dependency 'aws-sigv4', '~> 1.2.4' + spec.add_dependency 'aws-sdk-s3', '~> 1.0' spec.add_dependency 'bigdecimal', '~> 1.3.5' # https://github.com/ruby/bigdecimal#which-version-should-you-select spec.add_dependency 'cbor' spec.add_dependency 'chronic_duration', '~> 0.10.6' @@ -54,4 +52,5 @@ Gem::Specification.new do |spec| spec.add_development_dependency 'pry' spec.add_development_dependency 'climate_control', '= 0.0.3' spec.add_development_dependency 'fabrication', '~> 2.15.2' + spec.add_development_dependency 'httplog', '< 1.6' end diff --git a/docker-compose.yml b/docker-compose.yml index a90fd52f..72fd26f9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,6 +1,10 @@ services: cli: - build: . + build: + context: . + args: + RUBY_VERSION: ${RUBY_VERSION:-2.3.1} + BUNDLER_VERSION: ${BUNDLER_VERSION:-} volumes: - type: bind source: . diff --git a/lib/aptible/cli/agent.rb b/lib/aptible/cli/agent.rb index 2cb4da8d..183be2b4 100644 --- a/lib/aptible/cli/agent.rb +++ b/lib/aptible/cli/agent.rb @@ -90,6 +90,10 @@ def initialize(*) level = Logger::WARN debug_level = ENV['APTIBLE_DEBUG'] level = debug_level if debug_level + require 'httplog' if ENV['BUNDLER_VERSION'] && \ + ENV['APTIBLE_LOG_HTTP_REQUEST_RESPONSE'] && \ + !ENV['APTIBLE_LOG_HTTP_REQUEST_RESPONSE'] \ + .downcase.start_with?('f') conf.logger.tap { |l| l.level = level } end warn_sso_enforcement diff --git a/lib/aptible/cli/helpers/aws_account.rb b/lib/aptible/cli/helpers/aws_account.rb index 0cf58038..850dd97c 100644 --- a/lib/aptible/cli/helpers/aws_account.rb +++ b/lib/aptible/cli/helpers/aws_account.rb @@ -24,9 +24,7 @@ def aws_accounts_all end def aws_account_from_id(id) - Aptible::Api::ExternalAwsAccount.all(token: fetch_token).find do |a| - a.id.to_s == id.to_s - end + Aptible::Api::ExternalAwsAccount.find(id.to_s, token: fetch_token) end def ensure_external_aws_account(id) @@ -38,19 +36,40 @@ def ensure_external_aws_account(id) acct end + def fetch_organization_id + orgs = Aptible::Auth::Organization.all(token: fetch_token) + raise Thor::Error, 'No organizations found, specify one with ' \ + '--organization-id=ORG_ID' if orgs.empty? + raise Thor::Error, 'Multiple organizations found, indicate which ' \ + 'one to use with --organization-id=ORG_ID ' \ + "\n\tFound organization ids: #{orgs.join(', ')}" \ + if orgs.count > 1 + + orgs.first.id + end + + def organization_id_from_opts_or_auth(options) + return options[:organization_id] if options.key? :organization_id + + fetch_organization_id + end + def build_external_aws_account_attrs(options) - role_arn = options[:role_arn] || options[:arn] + discovery_role_arn = if options[:remove_discovery_role_arn] + '' + else + options[:discovery_role_arn] + end discovery_enabled = if options.key?(:discovery_enabled) options[:discovery_enabled] end attrs = { - role_arn: role_arn, account_name: options[:account_name] || options[:name], aws_account_id: options[:aws_account_id], - organization_id: options[:organization_id], aws_region_primary: options[:aws_region_primary], status: options[:status], discovery_enabled: discovery_enabled, + discovery_role_arn: discovery_role_arn, discovery_frequency: options[:discovery_frequency] } attrs.reject { |_, v| v.nil? } @@ -58,34 +77,78 @@ def build_external_aws_account_attrs(options) def create_external_aws_account!(options) attrs = build_external_aws_account_attrs(options) - Aptible::Api::ExternalAwsAccount.create( - token: fetch_token, - **attrs - ) + attrs[:organization_id] = organization_id_from_opts_or_auth(options) + begin + resource = Aptible::Api::ExternalAwsAccount.create( + token: fetch_token, + **attrs + ) + if resource.errors.any? + raise Thor::Error, resource.errors.full_messages.first + end + resource + rescue HyperResource::ClientError => e + raise Thor::Error, e.message + end end def update_external_aws_account!(id, options) ext = ensure_external_aws_account(id) attrs = build_external_aws_account_attrs(options) - ext.update!(**attrs) unless attrs.empty? - ext + begin + unless attrs.empty? + ext.update!(**attrs) + if ext.errors.any? + raise Thor::Error, ext.errors.full_messages.first + end + end + ext + rescue HyperResource::ClientError => e + raise Thor::Error, e.message + end end def delete_external_aws_account!(id) ext = ensure_external_aws_account(id) - if ext.respond_to?(:destroy!) - ext.destroy! - elsif ext.respond_to?(:destroy) - ext.destroy - elsif ext.respond_to?(:delete!) - ext.delete! - elsif ext.respond_to?(:delete) - ext.delete - else - raise Thor::Error, 'Delete is not supported for this resource' + begin + if ext.respond_to?(:destroy!) + ext.destroy! + elsif ext.respond_to?(:destroy) + ext.destroy + elsif ext.respond_to?(:delete!) + ext.delete! + elsif ext.respond_to?(:delete) + ext.delete + else + raise Thor::Error, 'Delete is not supported for this resource' + end + rescue HyperResource::ClientError => e + raise Thor::Error, e.message end true end + + def check_external_aws_account!(id) + ext = ensure_external_aws_account(id) + begin + ext.check! + rescue HyperResource::ClientError => e + raise Thor::Error, e.message + end + end + + def format_check_state(state) + case state + when 'success' + '✅ success' + when 'failed' + '❌ failed' + when 'not_run' + '⏭️ not_run' + else + state + end + end end end end diff --git a/lib/aptible/cli/helpers/s3_log_helpers.rb b/lib/aptible/cli/helpers/s3_log_helpers.rb index 5c2fb910..1bafad16 100644 --- a/lib/aptible/cli/helpers/s3_log_helpers.rb +++ b/lib/aptible/cli/helpers/s3_log_helpers.rb @@ -1,4 +1,4 @@ -require 'aws-sdk' +require 'aws-sdk-s3' require 'pathname' module Aptible diff --git a/lib/aptible/cli/subcommands/aws_accounts.rb b/lib/aptible/cli/subcommands/aws_accounts.rb index 90ea9397..62993009 100644 --- a/lib/aptible/cli/subcommands/aws_accounts.rb +++ b/lib/aptible/cli/subcommands/aws_accounts.rb @@ -11,6 +11,10 @@ def self.included(thor) include Helpers::Telemetry desc 'aws_accounts', 'List external AWS accounts', hide: true + option :organization_id, aliases: '--org-id', + type: :string, + default: nil, + desc: 'Organization ID' def aws_accounts telemetry(__method__, options) @@ -28,8 +32,8 @@ def aws_accounts aws_region_primary status discovery_enabled + discovery_role_arn discovery_frequency - role_arn account_id created_at updated_at @@ -44,15 +48,14 @@ def aws_accounts end desc 'aws_accounts:add ' \ - '[--role-arn ROLE_ARN] ' \ '[--account-name ACCOUNT_NAME] ' \ '[--aws-account-id AWS_ACCOUNT_ID] ' \ '[--org-id ORGANIZATION_ID] '\ '[--aws-region-primary AWS_REGION] ' \ '[--discovery-enabled|--no-discovery-enabled] ' \ + '[--discovery-role-arn DISCOVERY_ROLE_ARN] ' \ '[--discovery-frequency FREQ]', \ 'Add a new external AWS account', hide: true - option :role_arn, type: :string, desc: 'IAM Role ARN to assume' option :account_name, type: :string, desc: 'Display name' option :aws_account_id, type: :string, desc: 'AWS Account ID' option :organization_id, aliases: '--org-id', @@ -63,6 +66,10 @@ def aws_accounts desc: 'Primary AWS region' option :discovery_enabled, type: :boolean, desc: 'Enable resource discovery' + option :discovery_role_arn, type: :string, + desc: 'IAM Role ARN that Aptible ' \ + 'will assume to discover ' \ + 'resources in your AWS account' option :discovery_frequency, type: :string, desc: 'Discovery frequency (e.g., daily)' @@ -85,8 +92,41 @@ def aws_accounts account_name aws_region_primary discovery_enabled + discovery_role_arn + discovery_frequency + account_id + created_at + updated_at + ).each do |k| + v = rattrs[k] + node.value(k, v) unless v.nil? + end + end + end + end + + desc 'aws_accounts:show ID', + 'Show an external AWS account', \ + hide: true + define_method 'aws_accounts:show' do |id| + telemetry(__method__, options.merge(id: id)) + ext = ensure_external_aws_account(id) + Formatter.render(Renderer.current) do |root| + root.object do |node| + node.value('id', ext.id) + rattrs = + if ext.respond_to?(:attributes) + ext.attributes + else + {} + end + %w( + aws_account_id + account_name + aws_region_primary + discovery_enabled + discovery_role_arn discovery_frequency - role_arn account_id created_at updated_at @@ -114,22 +154,29 @@ def aws_accounts end end - desc 'aws_accounts:update ID [--role-arn ROLE_ARN] ' \ + desc 'aws_accounts:update ID ' \ '[--account-name ACCOUNT_NAME] ' \ '[--aws-account-id AWS_ACCOUNT_ID] ' \ - '[--organization-id ORG_ID] ' \ '[--aws-region-primary AWS_REGION] ' \ '[--discovery-enabled|--no-discovery-enabled] ' \ + '[--discovery-role-arn DISCOVERY_ROLE_ARN] ' \ + '[--remove-discovery-role-arn] ' \ '[--discovery-frequency FREQ]', \ 'Update an external AWS account', hide: true - option :role_arn, type: :string, desc: 'New IAM Role ARN to assume' option :account_name, type: :string, desc: 'New display name' option :aws_account_id, type: :string, desc: 'AWS Account ID' - option :organization_id, type: :string, desc: 'Organization ID' option :aws_region_primary, type: :string, desc: 'Primary AWS region' option :discovery_enabled, type: :boolean, desc: 'Enable resource discovery' + option :discovery_role_arn, type: :string, + desc: 'IAM Role ARN that Aptible ' \ + 'will assume to discover ' \ + 'resources in your AWS account' + option :remove_discovery_role_arn, type: :boolean, + desc: 'Remove the discovery ' \ + 'role ARN from this ' \ + 'account' option :discovery_frequency, type: :string, desc: 'Discovery frequency (e.g., daily)' @@ -152,8 +199,8 @@ def aws_accounts account_name aws_region_primary discovery_enabled + discovery_role_arn discovery_frequency - role_arn account_id created_at updated_at @@ -164,6 +211,47 @@ def aws_accounts end end end + + desc 'aws_accounts:check ID', + 'Check the connection for an external AWS account', \ + hide: true + define_method 'aws_accounts:check' do |id| + telemetry(__method__, options.merge(id: id)) + + response = check_external_aws_account!(id) + + if Renderer.format == 'json' + Formatter.render(Renderer.current) do |root| + root.object do |node| + node.value('state', response.state) + node.list('checks') do |check_list| + response.checks.each do |check| + check_list.object do |check_node| + check_node.value('name', check.check_name) + check_node.value('state', check.state) + check_node.value('details', check.details) \ + unless check.details.nil? + end + end + end + end + end + else + puts "State: #{format_check_state(response.state)}" + puts '' + puts 'Checks:' + response.checks.each do |check| + puts " Name: #{check.check_name}" + puts " State: #{format_check_state(check.state)}" + puts " Details: #{check.details}" unless check.details.nil? + puts '' + end + end + + unless response.state == 'success' + raise Thor::Error, 'AWS account check failed' + end + end end end end diff --git a/lib/aptible/cli/subcommands/logs.rb b/lib/aptible/cli/subcommands/logs.rb index dfca7fd1..e06588c4 100644 --- a/lib/aptible/cli/subcommands/logs.rb +++ b/lib/aptible/cli/subcommands/logs.rb @@ -1,4 +1,4 @@ -require 'aws-sdk' +require 'aws-sdk-s3' require 'shellwords' require 'time' diff --git a/spec/aptible/cli/subcommands/external_aws_accounts_spec.rb b/spec/aptible/cli/subcommands/external_aws_accounts_spec.rb index abe743ce..6ef58991 100644 --- a/spec/aptible/cli/subcommands/external_aws_accounts_spec.rb +++ b/spec/aptible/cli/subcommands/external_aws_accounts_spec.rb @@ -11,12 +11,12 @@ a1 = Fabricate(:external_aws_account, account_name: 'Dev', aws_account_id: '111111111111', - role_arn: 'arn:aws:iam::111111111111:role/' \ + discovery_role_arn: 'arn:aws:iam::111111111111:role/' \ 'DevRole') a2 = Fabricate(:external_aws_account, account_name: 'Prod', aws_account_id: '222222222222', - role_arn: 'arn:aws:iam::222222222222:role/' \ + discovery_role_arn: 'arn:aws:iam::222222222222:role/' \ 'ProdRole') expect(Aptible::Api::ExternalAwsAccount).to receive(:all) @@ -31,14 +31,14 @@ expect(captured_output_text).to include('Account Name: Dev') expect(captured_output_text).to include('Aws Account: 111111111111') expect(captured_output_text).to( - include('Role Arn: arn:aws:iam::111111111111:role/DevRole') + include('Discovery Role Arn: arn:aws:iam::111111111111:role/DevRole') ) expect(captured_output_text).to include("Id: #{a2.id}") expect(captured_output_text).to include('Account Name: Prod') expect(captured_output_text).to include('Aws Account: 222222222222') expect(captured_output_text).to( - include('Role Arn: arn:aws:iam::222222222222:role/ProdRole') + include('Discovery Role Arn: arn:aws:iam::222222222222:role/ProdRole') ) end @@ -54,10 +54,12 @@ end it 'renders JSON output and uses JSON href' do - a1 = Fabricate(:external_aws_account, - account_name: 'Dev', - aws_account_id: '111111111111', - role_arn: 'arn:aws:iam::111111111111:role/DevRole') + a1 = Fabricate( + :external_aws_account, + account_name: 'Dev', + aws_account_id: '111111111111', + discovery_role_arn: 'arn:aws:iam::111111111111:role/DevRole' + ) allow(Aptible::CLI::Renderer).to receive(:format).and_return('json') @@ -72,7 +74,7 @@ expect(json.first['id']).to eq(a1.id) expect(json.first['account_name']).to eq('Dev') expect(json.first['aws_account_id']).to eq('111111111111') - expect(json.first['role_arn']).to( + expect(json.first['discovery_role_arn']).to( eq('arn:aws:iam::111111111111:role/DevRole') ) end @@ -80,11 +82,15 @@ describe '#aws_accounts:add' do it 'creates an external AWS account' do + org = double('org', id: 'org-123') + allow(Aptible::Auth::Organization).to receive(:all) + .with(token: token).and_return([org]) + created = Fabricate( :external_aws_account, account_name: 'Staging', aws_account_id: '123456789012', - role_arn: 'arn:aws:iam::123456789012:role/' \ + discovery_role_arn: 'arn:aws:iam::123456789012:role/' \ 'StagingRole', discovery_enabled: true, discovery_frequency: 'daily', @@ -93,16 +99,17 @@ expect(Aptible::Api::ExternalAwsAccount).to receive(:create).with( token: token, - role_arn: 'arn:aws:iam::123456789012:role/StagingRole', + discovery_role_arn: 'arn:aws:iam::123456789012:role/StagingRole', account_name: 'Staging', aws_account_id: '123456789012', + organization_id: 'org-123', aws_region_primary: 'us-east-1', discovery_enabled: true, discovery_frequency: 'daily' ).and_return(created) subject.options = { - role_arn: 'arn:aws:iam::123456789012:role/StagingRole', + discovery_role_arn: 'arn:aws:iam::123456789012:role/StagingRole', account_name: 'Staging', aws_account_id: '123456789012', aws_region_primary: 'us-east-1', @@ -114,67 +121,77 @@ expect(captured_output_text).to include("Id: #{created.id}") expect(captured_output_text).to include('Account Name: Staging') expect(captured_output_text).to include('Aws Account: 123456789012') - expect(captured_output_text).to( - include('Role Arn: arn:aws:iam::123456789012:role/StagingRole') + expect(captured_output_text).to include( + 'Discovery Role Arn: arn:aws:iam::123456789012:role/StagingRole' ) end - it 'creates with minimal options (role_arn only)' do - created = Fabricate(:external_aws_account, - role_arn: 'arn:aws:iam::123456789012:role/' \ - 'MinRole') + it 'creates with minimal options (discovery_role_arn only)' do + org = double('org', id: 'org-123') + allow(Aptible::Auth::Organization).to receive(:all) + .with(token: token).and_return([org]) + + created = Fabricate( + :external_aws_account, + discovery_role_arn: 'arn:aws:iam::123456789012:role/MinRole' + ) expect(Aptible::Api::ExternalAwsAccount).to receive(:create).with( token: token, - role_arn: 'arn:aws:iam::123456789012:role/MinRole' + discovery_role_arn: 'arn:aws:iam::123456789012:role/MinRole', + organization_id: 'org-123' ).and_return(created) subject.options = { - role_arn: 'arn:aws:iam::123456789012:role/MinRole' + discovery_role_arn: 'arn:aws:iam::123456789012:role/MinRole' } subject.send('aws_accounts:add') expect(captured_output_text).to include("Id: #{created.id}") - expect(captured_output_text).to( - include('Role Arn: arn:aws:iam::123456789012:role/MinRole') + expect(captured_output_text).to include( + 'Discovery Role Arn: arn:aws:iam::123456789012:role/MinRole' ) end - it 'supports --arn alias for --role-arn' do - created = Fabricate(:external_aws_account, - role_arn: 'arn:aws:iam::123456789012:role/' \ - 'AliasRole') + it 'creates with organization_id provided' do + created = Fabricate( + :external_aws_account, + discovery_role_arn: 'arn:aws:iam::123456789012:role/AliasRole' + ) expect(Aptible::Api::ExternalAwsAccount).to receive(:create).with( token: token, - role_arn: 'arn:aws:iam::123456789012:role/AliasRole' + discovery_role_arn: 'arn:aws:iam::123456789012:role/AliasRole', + organization_id: 'explicit-org-id' ).and_return(created) subject.options = { - arn: 'arn:aws:iam::123456789012:role/AliasRole' + discovery_role_arn: 'arn:aws:iam::123456789012:role/AliasRole', + organization_id: 'explicit-org-id' } subject.send('aws_accounts:add') - expect(captured_output_text).to( - include('Role Arn: arn:aws:iam::123456789012:role/AliasRole') + expect(captured_output_text).to include( + 'Discovery Role Arn: arn:aws:iam::123456789012:role/AliasRole' ) end it 'creates with all optional fields' do - created = Fabricate(:external_aws_account, - account_name: 'Full', - aws_account_id: '123456789012', - role_arn: 'arn:aws:iam::123456789012:role/' \ - 'FullRole', - organization_id: 'o-123', - aws_region_primary: 'us-west-2', - discovery_enabled: false, - discovery_frequency: 'weekly', - status: 'active') + created = Fabricate( + :external_aws_account, + account_name: 'Full', + aws_account_id: '123456789012', + discovery_role_arn: 'arn:aws:iam::123456789012:role/FullRole', + organization_id: 'o-123', + aws_region_primary: 'us-west-2', + discovery_enabled: false, + discovery_frequency: 'weekly', + status: 'active' + ) expect(Aptible::Api::ExternalAwsAccount).to receive(:create).with( token: token, - role_arn: 'arn:aws:iam::123456789012:role/FullRole', + discovery_role_arn: 'arn:aws:iam::123456789012:role/FullRole', account_name: 'Full', aws_account_id: '123456789012', organization_id: 'o-123', @@ -185,7 +202,7 @@ ).and_return(created) subject.options = { - role_arn: 'arn:aws:iam::123456789012:role/FullRole', + discovery_role_arn: 'arn:aws:iam::123456789012:role/FullRole', account_name: 'Full', aws_account_id: '123456789012', organization_id: 'o-123', @@ -201,18 +218,25 @@ end it 'honors --no-discovery-enabled (false case)' do - created = Fabricate(:external_aws_account, - role_arn: 'arn:aws:iam::123456789012:role/NoDisc', - discovery_enabled: false) + org = double('org', id: 'org-123') + allow(Aptible::Auth::Organization).to receive(:all) + .with(token: token).and_return([org]) + + created = Fabricate( + :external_aws_account, + discovery_role_arn: 'arn:aws:iam::123456789012:role/NoDisc', + discovery_enabled: false + ) expect(Aptible::Api::ExternalAwsAccount).to receive(:create).with( token: token, - role_arn: 'arn:aws:iam::123456789012:role/NoDisc', + discovery_role_arn: 'arn:aws:iam::123456789012:role/NoDisc', + organization_id: 'org-123', discovery_enabled: false ).and_return(created) subject.options = { - role_arn: 'arn:aws:iam::123456789012:role/NoDisc', + discovery_role_arn: 'arn:aws:iam::123456789012:role/NoDisc', discovery_enabled: false } subject.send('aws_accounts:add') @@ -221,6 +245,10 @@ end it 'bubbles API errors during create' do + org = double('org', id: 'org-123') + allow(Aptible::Auth::Organization).to receive(:all) + .with(token: token).and_return([org]) + expect(Aptible::Api::ExternalAwsAccount).to receive(:create).and_raise( HyperResource::ClientError.new( 'Boom', @@ -228,28 +256,36 @@ ) ) - subject.options = { role_arn: 'arn:aws:iam::123456789012:role/Error' } + subject.options = { + discovery_role_arn: 'arn:aws:iam::123456789012:role/Error' + } expect { subject.send('aws_accounts:add') }.to( - raise_error(HyperResource::ClientError) + raise_error(Thor::Error, /Boom/) ) end it 'renders JSON output for create' do - created = Fabricate(:external_aws_account, - role_arn: 'arn:aws:iam::123456789012:role/' \ - 'JsonRole', - account_name: 'JsonName') + org = double('org', id: 'org-123') + allow(Aptible::Auth::Organization).to receive(:all) + .with(token: token).and_return([org]) + + created = Fabricate( + :external_aws_account, + discovery_role_arn: 'arn:aws:iam::123456789012:role/JsonRole', + account_name: 'JsonName' + ) allow(Aptible::CLI::Renderer).to receive(:format).and_return('json') expect(Aptible::Api::ExternalAwsAccount).to receive(:create).with( token: token, - role_arn: 'arn:aws:iam::123456789012:role/JsonRole', - account_name: 'JsonName' + discovery_role_arn: 'arn:aws:iam::123456789012:role/JsonRole', + account_name: 'JsonName', + organization_id: 'org-123' ).and_return(created) subject.options = { - role_arn: 'arn:aws:iam::123456789012:role/JsonRole', + discovery_role_arn: 'arn:aws:iam::123456789012:role/JsonRole', account_name: 'JsonName' } subject.send('aws_accounts:add') @@ -257,32 +293,65 @@ json = captured_output_json expect(json['id']).to eq(created.id) expect(json['account_name']).to eq('JsonName') - expect(json['role_arn']).to( + expect(json['discovery_role_arn']).to( eq('arn:aws:iam::123456789012:role/JsonRole') ) end + + it 'fails when no organizations found and no org_id provided' do + allow(Aptible::Auth::Organization).to receive(:all) + .with(token: token).and_return([]) + + subject.options = { + discovery_role_arn: 'arn:aws:iam::123456789012:role/TestRole' + } + + expect { subject.send('aws_accounts:add') }.to( + raise_error(Thor::Error, /No organizations found/) + ) + end + + it 'fails when multiple organizations found and no org_id provided' do + org1 = double('org1', id: 'org-1') + org2 = double('org2', id: 'org-2') + allow(Aptible::Auth::Organization).to receive(:all) + .with(token: token).and_return([org1, org2]) + + subject.options = { + discovery_role_arn: 'arn:aws:iam::123456789012:role/TestRole' + } + + expect { subject.send('aws_accounts:add') }.to( + raise_error(Thor::Error, /Multiple organizations found/) + ) + end end describe '#aws_accounts:update' do it 'updates an external AWS account' do - ext = double('ext', - id: 42, - attributes: { - 'account_name' => 'New Name', - 'aws_account_id' => '999999999999', - 'role_arn' => 'arn:aws:iam::999999999999:role/NewRole' - }) + errors = Aptible::Resource::Errors.new + ext = double( + 'ext', + id: 42, + errors: errors, + attributes: { + 'account_name' => 'New Name', + 'aws_account_id' => '999999999999', + 'discovery_role_arn' => + 'arn:aws:iam::999999999999:role/NewRole' + } + ) - expect(Aptible::Api::ExternalAwsAccount).to receive(:all) - .with(token: token).and_return([ext]) + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('42', token: token).and_return(ext) expect(ext).to receive(:update!).with( - role_arn: 'arn:aws:iam::999999999999:role/NewRole', + discovery_role_arn: 'arn:aws:iam::999999999999:role/NewRole', account_name: 'New Name', aws_account_id: '999999999999' ).and_return(true) subject.options = { - role_arn: 'arn:aws:iam::999999999999:role/NewRole', + discovery_role_arn: 'arn:aws:iam::999999999999:role/NewRole', account_name: 'New Name', aws_account_id: '999999999999' } @@ -292,29 +361,33 @@ expect(captured_output_text).to include('Account Name: New Name') expect(captured_output_text).to include('Aws Account: 999999999999') expect(captured_output_text).to( - include('Role Arn: arn:aws:iam::999999999999:role/NewRole') + include('Discovery Role Arn: arn:aws:iam::999999999999:role/NewRole') ) end it 'fails when account not found' do - expect(Aptible::Api::ExternalAwsAccount).to receive(:all) - .with(token: token).and_return([]) + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('999', token: token).and_return(nil) expect { subject.send('aws_accounts:update', '999') } .to raise_error(Thor::Error, /External AWS account not found: 999/) end it 'updates only one field (account_name)' do - ext = double('ext', - id: 42, - attributes: { - 'account_name' => 'Updated Name', - 'aws_account_id' => '111111111111', - 'role_arn' => 'arn:aws:iam::111111111111:role/Role' - }) + errors = Aptible::Resource::Errors.new + ext = double( + 'ext', + id: 42, + errors: errors, + attributes: { + 'account_name' => 'Updated Name', + 'aws_account_id' => '111111111111', + 'discovery_role_arn' => 'arn:aws:iam::111111111111:role/Role' + } + ) - expect(Aptible::Api::ExternalAwsAccount).to receive(:all) - .with(token: token).and_return([ext]) + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('42', token: token).and_return(ext) expect(ext).to receive(:update!).with( account_name: 'Updated Name' ).and_return(true) @@ -328,15 +401,17 @@ end it 'updates discovery settings separately' do + errors = Aptible::Resource::Errors.new ext = double('ext', id: 42, + errors: errors, attributes: { 'discovery_enabled' => true, 'discovery_frequency' => 'hourly' }) - expect(Aptible::Api::ExternalAwsAccount).to receive(:all) - .with(token: token).and_return([ext]) + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('42', token: token).and_return(ext) expect(ext).to receive(:update!).with( discovery_enabled: true, discovery_frequency: 'hourly' @@ -360,8 +435,8 @@ 'aws_account_id' => '111111111111' }) - expect(Aptible::Api::ExternalAwsAccount).to receive(:all) - .with(token: token).and_return([ext]) + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('42', token: token).and_return(ext) expect(ext).not_to receive(:update!) subject.options = {} @@ -372,8 +447,8 @@ it 'bubbles API errors during update' do ext = double('ext', id: 42) - expect(Aptible::Api::ExternalAwsAccount).to receive(:all) - .with(token: token).and_return([ext]) + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('42', token: token).and_return(ext) expect(ext).to receive(:update!).and_raise( HyperResource::ClientError.new( @@ -384,21 +459,23 @@ subject.options = { account_name: 'X' } expect { subject.send('aws_accounts:update', '42') }.to( - raise_error(HyperResource::ClientError) + raise_error(Thor::Error, /Boom/) ) end it 'renders JSON output for update' do + errors = Aptible::Resource::Errors.new ext = double('ext', id: 7, + errors: errors, attributes: { 'account_name' => 'JsonUpdated', 'aws_account_id' => '123' }) allow(Aptible::CLI::Renderer).to receive(:format).and_return('json') - expect(Aptible::Api::ExternalAwsAccount).to receive(:all) - .with(token: token).and_return([ext]) + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('7', token: token).and_return(ext) expect(ext).to( receive(:update!).with(account_name: 'JsonUpdated').and_return(true) ) @@ -410,13 +487,125 @@ expect(json['id']).to eq(7) expect(json['account_name']).to eq('JsonUpdated') end + + it 'removes discovery_role_arn with --remove-discovery-role-arn' do + errors = Aptible::Resource::Errors.new + ext = double( + 'ext', + id: 42, + errors: errors, + attributes: { + 'account_name' => 'Test', + 'aws_account_id' => '111111111111' + } + ) + + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('42', token: token).and_return(ext) + expect(ext).to receive(:update!).with( + discovery_role_arn: '' + ).and_return(true) + + subject.options = { remove_discovery_role_arn: true } + subject.send('aws_accounts:update', '42') + + expect(captured_output_text).to include('Id: 42') + end + + it 'ignores --discovery-role-arn when --remove-discovery-role-arn is set' do + errors = Aptible::Resource::Errors.new + ext = double( + 'ext', + id: 42, + errors: errors, + attributes: { + 'account_name' => 'Test', + 'aws_account_id' => '111111111111' + } + ) + + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('42', token: token).and_return(ext) + # Should send empty string, not the provided ARN + expect(ext).to receive(:update!).with( + discovery_role_arn: '' + ).and_return(true) + + subject.options = { + remove_discovery_role_arn: true, + discovery_role_arn: 'arn:aws:iam::111111111111:role/ShouldBeIgnored' + } + subject.send('aws_accounts:update', '42') + + expect(captured_output_text).to include('Id: 42') + end + end + + describe '#aws_accounts:show' do + it 'shows an external AWS account' do + ext = Fabricate( + :external_aws_account, + id: 42, + account_name: 'ShowTest', + aws_account_id: '123456789012', + discovery_role_arn: 'arn:aws:iam::123456789012:role/TestRole', + discovery_enabled: true, + discovery_frequency: 'daily' + ) + + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('42', token: token).and_return(ext) + + subject.send('aws_accounts:show', '42') + + expect(captured_output_text).to include('Id: 42') + expect(captured_output_text).to include('Account Name: ShowTest') + expect(captured_output_text).to include('Aws Account: 123456789012') + expect(captured_output_text).to( + include('Discovery Role Arn: arn:aws:iam::123456789012:role/TestRole') + ) + expect(captured_output_text).to include('Discovery Enabled: true') + expect(captured_output_text).to include('Discovery Frequency: daily') + end + + it 'fails when account not found' do + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('999', token: token).and_return(nil) + + expect { subject.send('aws_accounts:show', '999') } + .to raise_error(Thor::Error, /External AWS account not found: 999/) + end + + it 'renders JSON output' do + ext = Fabricate( + :external_aws_account, + id: 7, + account_name: 'JsonShow', + aws_account_id: '987654321098', + discovery_role_arn: 'arn:aws:iam::987654321098:role/JsonRole' + ) + + allow(Aptible::CLI::Renderer).to receive(:format).and_return('json') + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('7', token: token).and_return(ext) + + subject.send('aws_accounts:show', '7') + + json = captured_output_json + expect(json['id']).to eq(7) + expect(json['account_name']).to eq('JsonShow') + expect(json['aws_account_id']).to eq('987654321098') + expect(json['discovery_role_arn']).to( + eq('arn:aws:iam::987654321098:role/JsonRole') + ) + end end describe '#aws_accounts:delete' do it 'deletes an external AWS account' do ext = double('ext', id: 24) - expect(Aptible::Api::ExternalAwsAccount).to receive(:all) - .with(token: token).and_return([ext]) + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('24', token: token).and_return(ext) expect(ext).to receive(:destroy!).and_return(true) subject.send('aws_accounts:delete', '24') @@ -426,8 +615,8 @@ end it 'fails when account not found' do - expect(Aptible::Api::ExternalAwsAccount).to receive(:all) - .with(token: token).and_return([]) + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('999', token: token).and_return(nil) expect { subject.send('aws_accounts:delete', '999') } .to raise_error(Thor::Error, /External AWS account not found: 999/) @@ -435,8 +624,8 @@ it 'supports alternative delete methods' do ext = double('ext', id: 24) - expect(Aptible::Api::ExternalAwsAccount).to receive(:all) - .with(token: token).and_return([ext]) + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('24', token: token).and_return(ext) expect(ext).to receive(:respond_to?).with(:destroy!).and_return(false) expect(ext).to receive(:respond_to?).with(:destroy).and_return(false) @@ -451,8 +640,8 @@ it 'raises when delete is not supported' do ext = double('ext', id: 24) - expect(Aptible::Api::ExternalAwsAccount).to receive(:all) - .with(token: token).and_return([ext]) + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('24', token: token).and_return(ext) expect(ext).to receive(:respond_to?).with(:destroy!).and_return(false) expect(ext).to receive(:respond_to?).with(:destroy).and_return(false) @@ -466,8 +655,8 @@ it 'renders JSON output for delete' do ext = double('ext', id: 33) allow(Aptible::CLI::Renderer).to receive(:format).and_return('json') - expect(Aptible::Api::ExternalAwsAccount).to receive(:all) - .with(token: token).and_return([ext]) + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('33', token: token).and_return(ext) expect(ext).to receive(:destroy!).and_return(true) subject.send('aws_accounts:delete', '33') @@ -477,4 +666,73 @@ expect(json['deleted']).to eq(true) end end + + describe '#aws_accounts:check' do + it 'raises error when account not found' do + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('42', token: token).and_return(nil) + + expect { subject.send('aws_accounts:check', '42') }.to( + raise_error(Thor::Error, /External AWS account not found: 42/) + ) + end + + it 'checks an external AWS account successfully' do + ext = double('ext', id: 42) + check_result = double( + 'check_result', + state: 'success', + checks: [ + double('check', check_name: 'role_access', state: 'success', + details: nil) + ] + ) + + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('42', token: token).and_return(ext) + expect(ext).to receive(:check!).and_return(check_result) + + # check command uses puts directly (not Formatter) for non-JSON output + expect { subject.send('aws_accounts:check', '42') }.to output( + /State:.*success/m + ).to_stdout + end + + it 'raises error on check failure' do + ext = double('ext', id: 42) + check_result = double( + 'check_result', + state: 'failed', + checks: [ + double('check', check_name: 'role_access', state: 'failed', + details: 'Access denied') + ] + ) + + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('42', token: token).and_return(ext) + expect(ext).to receive(:check!).and_return(check_result) + + expect { subject.send('aws_accounts:check', '42') }.to( + raise_error(Thor::Error, /AWS account check failed/) + ) + end + + it 'handles API errors during check' do + ext = double('ext', id: 42) + + expect(Aptible::Api::ExternalAwsAccount).to receive(:find) + .with('42', token: token).and_return(ext) + expect(ext).to receive(:check!).and_raise( + HyperResource::ClientError.new( + 'Check failed', + response: Faraday::Response.new(status: 500) + ) + ) + + expect { subject.send('aws_accounts:check', '42') }.to( + raise_error(Thor::Error, /Check failed/) + ) + end + end end diff --git a/spec/fabricators/external_aws_account_fabricator.rb b/spec/fabricators/external_aws_account_fabricator.rb index 45d52cc2..4fa96bf2 100644 --- a/spec/fabricators/external_aws_account_fabricator.rb +++ b/spec/fabricators/external_aws_account_fabricator.rb @@ -7,7 +7,7 @@ def attributes 'status' => status, 'discovery_enabled' => discovery_enabled, 'discovery_frequency' => discovery_frequency, - 'role_arn' => role_arn, + 'discovery_role_arn' => discovery_role_arn, 'account_id' => account_id, 'created_at' => created_at, 'updated_at' => updated_at @@ -18,6 +18,7 @@ def attributes Fabricator(:external_aws_account, from: :stub_external_aws_account) do id { Fabricate.sequence(:external_aws_account_id) { |i| i } } account + errors { Aptible::Resource::Errors.new } account_name { |attrs| "External AWS Account #{attrs[:id]}" } aws_account_id do @@ -25,7 +26,9 @@ def attributes format('%012d', 10_000_000_000 + i) end end - role_arn { |attrs| "arn:aws:iam::#{attrs[:aws_account_id]}:role/ExampleRole" } + discovery_role_arn do |attrs| + "arn:aws:iam::#{attrs[:aws_account_id]}:role/ExampleRole" + end aws_region_primary 'us-east-1' status 'active' discovery_enabled false