From 4913eb3306dc590f74885040cd34314bf3a5734e Mon Sep 17 00:00:00 2001 From: T Van Doren <210452321+tvdaptible@users.noreply.github.com> Date: Mon, 17 Nov 2025 13:04:07 -0800 Subject: [PATCH 01/14] chore: upgrade aws-sdk v2 -> v3 [SC-35423] * https://github.com/aws/aws-sdk-ruby/blob/version-3/V3_UPGRADING_GUIDE.md * deprecation notice: https://github.com/aws/aws-sdk-ruby/blob/version-2/README.md --- Gemfile.lock | 33 ++++++++++++++--------- Makefile | 17 ++++++++++-- aptible-cli.gemspec | 4 +-- lib/aptible/cli/helpers/s3_log_helpers.rb | 2 +- lib/aptible/cli/subcommands/logs.rb | 2 +- 5 files changed, 39 insertions(+), 19 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 9a176935..530b6b09 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,9 +7,7 @@ PATH aptible-auth (~> 1.2.5) 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) @@ -60,16 +58,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) @@ -99,6 +107,7 @@ GEM 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) diff --git a/Makefile b/Makefile index 00ee47cd..d5d46abf 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,23 @@ + +export COMPOSE_IGNORE_ORPHANS ?= true + build: docker compose build --pull bash: build - docker compose run cli bash + $(MAKE) run CMD=bash + +CMD ?= bash +run: + docker compose run cli $(CMD) test: build - docker compose run cli bundle exec rake + $(MAKE) test-direct ARGS="$(ARGS)" + +test-direct: + docker compose run cli bundle exec rake $(ARGS) + +down: + docker compose down --remove-orphans $(ARGS) .PHONY: build bash test diff --git a/aptible-cli.gemspec b/aptible-cli.gemspec index 35922d76..0e2217b0 100644 --- a/aptible-cli.gemspec +++ b/aptible-cli.gemspec @@ -25,9 +25,7 @@ Gem::Specification.new do |spec| spec.add_dependency 'aptible-auth', '~> 1.2.5' 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' 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/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' From 31f19a052b397d140506704a4ee6182cb95c04b0 Mon Sep 17 00:00:00 2001 From: T Van Doren <210452321+tvdaptible@users.noreply.github.com> Date: Mon, 17 Nov 2025 13:34:43 -0800 Subject: [PATCH 02/14] test against ruby 3.1 thru 3.4 --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index bf2a8eff..b3e5e7e0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,7 +15,7 @@ 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] steps: - name: Check out code uses: actions/checkout@v4 From 0a7aea71895f36a87f116d7f86889a95972715d9 Mon Sep 17 00:00:00 2001 From: T Van Doren <210452321+tvdaptible@users.noreply.github.com> Date: Mon, 17 Nov 2025 09:30:00 -0800 Subject: [PATCH 03/14] feat: aws_accounts:* commands fix a couple bugs in aws_accounts:* commands and add a couple new ones. --- Gemfile.lock | 4 + Makefile | 19 +- aptible-cli.gemspec | 1 + lib/aptible/cli/agent.rb | 4 + lib/aptible/cli/helpers/aws_account.rb | 28 +- lib/aptible/cli/subcommands/aws_accounts.rb | 70 +++- .../subcommands/external_aws_accounts_spec.rb | 331 +++++++++++++----- .../external_aws_account_fabricator.rb | 6 +- 8 files changed, 346 insertions(+), 117 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 9a176935..a1778601 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -94,6 +94,9 @@ GEM rchardet (~> 1.8) hashdiff (1.1.1) 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) @@ -176,6 +179,7 @@ DEPENDENCIES 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..e8410698 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,23 @@ + +export COMPOSE_IGNORE_ORPHANS ?= true + build: docker compose build --pull bash: build - docker compose run cli bash + $(MAKE) run CMD=bash + +CMD ?= bash +run: + docker compose run cli $(CMD) test: build - docker compose run cli bundle exec rake + $(MAKE) test-direct ARGS="$(ARGS)" + +test-direct: + docker compose run cli bundle exec rake $(ARGS) + +down: + docker compose down --remove-orphans $(ARGS) -.PHONY: build bash test +.PHONY: build bash test \ No newline at end of file diff --git a/aptible-cli.gemspec b/aptible-cli.gemspec index 35922d76..41316e92 100644 --- a/aptible-cli.gemspec +++ b/aptible-cli.gemspec @@ -54,4 +54,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/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..ff00ddd0 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,36 @@ 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 = options[:discovery_role_arn] 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,6 +73,7 @@ def build_external_aws_account_attrs(options) def create_external_aws_account!(options) attrs = build_external_aws_account_attrs(options) + attrs[:organization_id] = organization_id_from_opts_or_auth(options) Aptible::Api::ExternalAwsAccount.create( token: fetch_token, **attrs diff --git a/lib/aptible/cli/subcommands/aws_accounts.rb b/lib/aptible/cli/subcommands/aws_accounts.rb index 90ea9397..a969a9a5 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,25 @@ 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] ' \ '[--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 :discovery_frequency, type: :string, desc: 'Discovery frequency (e.g., daily)' @@ -152,8 +195,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 +207,15 @@ 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)) + # FIXME: implement it! + raise Thor::Error, 'not implemented yet :(' + end end end end diff --git a/spec/aptible/cli/subcommands/external_aws_accounts_spec.rb b/spec/aptible/cli/subcommands/external_aws_accounts_spec.rb index abe743ce..4d3c25c5 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) ) 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,63 @@ 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' - }) + ext = double( + 'ext', + id: 42, + 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 +359,31 @@ 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' - }) + ext = double( + 'ext', + id: 42, + 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) @@ -335,8 +404,8 @@ '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 +429,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 +441,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( @@ -397,8 +466,8 @@ }) 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) ) @@ -412,11 +481,71 @@ 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 +555,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 +564,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 +580,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 +595,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 +606,12 @@ expect(json['deleted']).to eq(true) end end + + describe '#aws_accounts:check' do + it 'raises not implemented error' do + expect { subject.send('aws_accounts:check', '42') }.to( + raise_error(Thor::Error, /not implemented yet/) + ) + end + end end diff --git a/spec/fabricators/external_aws_account_fabricator.rb b/spec/fabricators/external_aws_account_fabricator.rb index 45d52cc2..866e2f26 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 @@ -25,7 +25,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 From c2d638ca95962c785868aa62c7f5f3c6c606dbf8 Mon Sep 17 00:00:00 2001 From: T Van Doren <210452321+tvdaptible@users.noreply.github.com> Date: Wed, 10 Dec 2025 11:02:25 -0800 Subject: [PATCH 04/14] add check_external_aws_account! helper --- lib/aptible/cli/helpers/aws_account.rb | 5 +++++ lib/aptible/cli/subcommands/aws_accounts.rb | 7 +++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/aptible/cli/helpers/aws_account.rb b/lib/aptible/cli/helpers/aws_account.rb index ff00ddd0..24cfae2c 100644 --- a/lib/aptible/cli/helpers/aws_account.rb +++ b/lib/aptible/cli/helpers/aws_account.rb @@ -102,6 +102,11 @@ def delete_external_aws_account!(id) end true end + + def check_external_aws_account!(id) + ext = ensure_external_aws_account(id) + ext.check! + end end end end diff --git a/lib/aptible/cli/subcommands/aws_accounts.rb b/lib/aptible/cli/subcommands/aws_accounts.rb index a969a9a5..63be4dd0 100644 --- a/lib/aptible/cli/subcommands/aws_accounts.rb +++ b/lib/aptible/cli/subcommands/aws_accounts.rb @@ -213,8 +213,11 @@ def aws_accounts hide: true define_method 'aws_accounts:check' do |id| telemetry(__method__, options.merge(id: id)) - # FIXME: implement it! - raise Thor::Error, 'not implemented yet :(' + + response = check_external_aws_account!(id) + puts "FIXME: response=#{response}" + + raise Thor::Error, 'not done yet :(' end end end From ad6ae93fec64b676a8ec1524de294f1030a3ed4e Mon Sep 17 00:00:00 2001 From: T Van Doren <210452321+tvdaptible@users.noreply.github.com> Date: Wed, 10 Dec 2025 11:23:50 -0800 Subject: [PATCH 05/14] aws_accounts:check works now! --- lib/aptible/cli/helpers/aws_account.rb | 13 ++++++++ lib/aptible/cli/subcommands/aws_accounts.rb | 33 +++++++++++++++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/lib/aptible/cli/helpers/aws_account.rb b/lib/aptible/cli/helpers/aws_account.rb index 24cfae2c..3c8dac14 100644 --- a/lib/aptible/cli/helpers/aws_account.rb +++ b/lib/aptible/cli/helpers/aws_account.rb @@ -107,6 +107,19 @@ def check_external_aws_account!(id) ext = ensure_external_aws_account(id) ext.check! 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/subcommands/aws_accounts.rb b/lib/aptible/cli/subcommands/aws_accounts.rb index 63be4dd0..6bf925eb 100644 --- a/lib/aptible/cli/subcommands/aws_accounts.rb +++ b/lib/aptible/cli/subcommands/aws_accounts.rb @@ -215,9 +215,38 @@ def aws_accounts telemetry(__method__, options.merge(id: id)) response = check_external_aws_account!(id) - puts "FIXME: response=#{response}" - raise Thor::Error, 'not done yet :(' + 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 From 827f80aebaa9ad028ee3a2a730162eae3177fe61 Mon Sep 17 00:00:00 2001 From: T Van Doren <210452321+tvdaptible@users.noreply.github.com> Date: Wed, 10 Dec 2025 22:24:06 -0800 Subject: [PATCH 06/14] show errors when api calls fail for aws_accounts commands --- lib/aptible/cli/helpers/aws_account.rb | 59 ++++++++++---- .../subcommands/external_aws_accounts_spec.rb | 77 ++++++++++++++++++- .../external_aws_account_fabricator.rb | 1 + 3 files changed, 116 insertions(+), 21 deletions(-) diff --git a/lib/aptible/cli/helpers/aws_account.rb b/lib/aptible/cli/helpers/aws_account.rb index 3c8dac14..87898256 100644 --- a/lib/aptible/cli/helpers/aws_account.rb +++ b/lib/aptible/cli/helpers/aws_account.rb @@ -74,38 +74,63 @@ def build_external_aws_account_attrs(options) def create_external_aws_account!(options) attrs = build_external_aws_account_attrs(options) attrs[:organization_id] = organization_id_from_opts_or_auth(options) - Aptible::Api::ExternalAwsAccount.create( - token: fetch_token, - **attrs - ) + 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) - ext.check! + begin + ext.check! + rescue HyperResource::ClientError => e + raise Thor::Error, e.message + end end def format_check_state(state) diff --git a/spec/aptible/cli/subcommands/external_aws_accounts_spec.rb b/spec/aptible/cli/subcommands/external_aws_accounts_spec.rb index 4d3c25c5..a7d81295 100644 --- a/spec/aptible/cli/subcommands/external_aws_accounts_spec.rb +++ b/spec/aptible/cli/subcommands/external_aws_accounts_spec.rb @@ -260,7 +260,7 @@ 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 @@ -329,9 +329,11 @@ describe '#aws_accounts:update' do it 'updates an external AWS account' do + errors = Aptible::Resource::Errors.new ext = double( 'ext', id: 42, + errors: errors, attributes: { 'account_name' => 'New Name', 'aws_account_id' => '999999999999', @@ -372,9 +374,11 @@ end it 'updates only one field (account_name)' do + errors = Aptible::Resource::Errors.new ext = double( 'ext', id: 42, + errors: errors, attributes: { 'account_name' => 'Updated Name', 'aws_account_id' => '111111111111', @@ -397,8 +401,10 @@ 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' @@ -453,13 +459,15 @@ 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' @@ -608,9 +616,70 @@ end describe '#aws_accounts:check' do - it 'raises not implemented error' 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, /not implemented yet/) + raise_error(Thor::Error, /Check failed/) ) end end diff --git a/spec/fabricators/external_aws_account_fabricator.rb b/spec/fabricators/external_aws_account_fabricator.rb index 866e2f26..4fa96bf2 100644 --- a/spec/fabricators/external_aws_account_fabricator.rb +++ b/spec/fabricators/external_aws_account_fabricator.rb @@ -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 From 330f5903b8d0e13464c810c9a71b70930b187c5e Mon Sep 17 00:00:00 2001 From: T Van Doren <210452321+tvdaptible@users.noreply.github.com> Date: Wed, 10 Dec 2025 23:39:02 -0800 Subject: [PATCH 07/14] use aptible-api gem from branch, so we get the updated ExternalAwsAccount resources --- Gemfile | 2 ++ Gemfile.lock | 17 ++++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Gemfile b/Gemfile index 00775107..146c705d 100644 --- a/Gemfile +++ b/Gemfile @@ -7,5 +7,7 @@ group :test do gem 'webmock' end +gem 'aptible-api', git: 'https://github.com/aptible/aptible-api-ruby', branch: 'ext-aws-check' + # Specify your gem's dependencies in aptible-cli.gemspec gemspec diff --git a/Gemfile.lock b/Gemfile.lock index a1778601..66ae283b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,3 +1,14 @@ +GIT + remote: https://github.com/aptible/aptible-api-ruby + revision: a60c7e3ecaec043e9c1f4904cf1f14051909352d + branch: ext-aws-check + specs: + aptible-api (1.10.0) + aptible-auth + aptible-resource + gem_config + multipart-post (< 2.2.0) + PATH remote: . specs: @@ -35,11 +46,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 @@ -174,6 +180,7 @@ PLATFORMS DEPENDENCIES activesupport (~> 4.0) + aptible-api! aptible-cli! aptible-tasks (~> 0.5.8) bundler (~> 1.3) From f9f03cfdd74c5d9e24d90d88ce830e3a7a404305 Mon Sep 17 00:00:00 2001 From: T Van Doren <210452321+tvdaptible@users.noreply.github.com> Date: Thu, 11 Dec 2025 03:12:36 -0800 Subject: [PATCH 08/14] add --remove-discovery-role-flag to aws_accounts:update --- lib/aptible/cli/helpers/aws_account.rb | 6 ++- lib/aptible/cli/subcommands/aws_accounts.rb | 5 +- .../subcommands/external_aws_accounts_spec.rb | 52 +++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/lib/aptible/cli/helpers/aws_account.rb b/lib/aptible/cli/helpers/aws_account.rb index 87898256..850dd97c 100644 --- a/lib/aptible/cli/helpers/aws_account.rb +++ b/lib/aptible/cli/helpers/aws_account.rb @@ -55,7 +55,11 @@ def organization_id_from_opts_or_auth(options) end def build_external_aws_account_attrs(options) - discovery_role_arn = options[:discovery_role_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 diff --git a/lib/aptible/cli/subcommands/aws_accounts.rb b/lib/aptible/cli/subcommands/aws_accounts.rb index 6bf925eb..38ef023e 100644 --- a/lib/aptible/cli/subcommands/aws_accounts.rb +++ b/lib/aptible/cli/subcommands/aws_accounts.rb @@ -160,6 +160,7 @@ def aws_accounts '[--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 :account_name, type: :string, desc: 'New display name' @@ -172,7 +173,9 @@ def aws_accounts 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)' diff --git a/spec/aptible/cli/subcommands/external_aws_accounts_spec.rb b/spec/aptible/cli/subcommands/external_aws_accounts_spec.rb index a7d81295..6ef58991 100644 --- a/spec/aptible/cli/subcommands/external_aws_accounts_spec.rb +++ b/spec/aptible/cli/subcommands/external_aws_accounts_spec.rb @@ -487,6 +487,58 @@ 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 From 9030c81eece040dca0e6fa29229a91c82e6bf0dc Mon Sep 17 00:00:00 2001 From: T Van Doren <210452321+tvdaptible@users.noreply.github.com> Date: Sat, 13 Dec 2025 03:14:05 -0800 Subject: [PATCH 09/14] simplify local testing against different ruby versions ``` make build # use ruby=2.3.1, bundler=1.17.3 to build the docker image make test # run the spec tests with ruby=2.3.1, bundler=1.17.3ruby 2.3.1 and bundler 1.17.3 make lint # run lint with ruby=2.3.1, bundler=1.17.3 make test RUBY_VERSION=2.7 # ruby=2.7, bundler=1.17.3 make test RUBY_VERSION=3.3 # ruby=3.3, bundler= ``` --- Dockerfile | 13 +++++++----- Makefile | 49 ++++++++++++++++++++++++++++++++++++++++++++-- docker-compose.yml | 6 +++++- 3 files changed, 60 insertions(+), 8 deletions(-) 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/Makefile b/Makefile index 00ee47cd..f7a495e3 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,55 @@ + +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 + +## 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/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: . From ad8ef84fd46e16b98078a11ddfa3a6f35d16dc2e Mon Sep 17 00:00:00 2001 From: T Van Doren <210452321+tvdaptible@users.noreply.github.com> Date: Sat, 13 Dec 2025 03:30:57 -0800 Subject: [PATCH 10/14] upgrade aptible-auth to ~> 1.3 [SC-35150] --- aptible-cli.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aptible-cli.gemspec b/aptible-cli.gemspec index 35922d76..2c440e92 100644 --- a/aptible-cli.gemspec +++ b/aptible-cli.gemspec @@ -22,7 +22,7 @@ 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' From b1e64aae990a83d63f43642b5e0f7348f51ad433 Mon Sep 17 00:00:00 2001 From: T Van Doren <210452321+tvdaptible@users.noreply.github.com> Date: Sat, 13 Dec 2025 03:34:45 -0800 Subject: [PATCH 11/14] temporarily use aptible-auth branch with fixed concurrent-ruby version --- Gemfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Gemfile b/Gemfile index 00775107..454db9c2 100644 --- a/Gemfile +++ b/Gemfile @@ -7,5 +7,7 @@ group :test do gem 'webmock' end +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 From 78eab4dcf27f5856a8dc0bceeacd6f8062ed08c8 Mon Sep 17 00:00:00 2001 From: T Van Doren <210452321+tvdaptible@users.noreply.github.com> Date: Sat, 13 Dec 2025 04:17:58 -0800 Subject: [PATCH 12/14] pull in aptible-auth ~ 1.3 gem, and use yet another branch (for now) to handle some dependency version gymanstics --- Gemfile | 1 + Gemfile.lock | 37 +++++++++++++++++++++++++------------ 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/Gemfile b/Gemfile index 454db9c2..11a1724d 100644 --- a/Gemfile +++ b/Gemfile @@ -7,6 +7,7 @@ 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 diff --git a/Gemfile.lock b/Gemfile.lock index 66ae283b..aa8a4aaf 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,6 +1,6 @@ GIT remote: https://github.com/aptible/aptible-api-ruby - revision: a60c7e3ecaec043e9c1f4904cf1f14051909352d + revision: 8fe6db7b81c5d6fcbb5b332b58befee1886c2029 branch: ext-aws-check specs: aptible-api (1.10.0) @@ -9,13 +9,25 @@ GIT 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) @@ -46,11 +58,6 @@ GEM tzinfo (~> 1.1) addressable (2.8.0) public_suffix (>= 2.0.2, < 5.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) @@ -99,6 +106,7 @@ 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) @@ -110,18 +118,18 @@ GEM jwt (2.3.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) @@ -155,6 +163,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) @@ -170,6 +181,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) @@ -181,6 +193,7 @@ PLATFORMS DEPENDENCIES activesupport (~> 4.0) aptible-api! + aptible-auth! aptible-cli! aptible-tasks (~> 0.5.8) bundler (~> 1.3) From 865f74dcf6a13be058460a9293c8aa84ba2e0d14 Mon Sep 17 00:00:00 2001 From: T Van Doren <210452321+tvdaptible@users.noreply.github.com> Date: Sat, 13 Dec 2025 04:18:05 -0800 Subject: [PATCH 13/14] lint roller --- lib/aptible/cli/subcommands/aws_accounts.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/aptible/cli/subcommands/aws_accounts.rb b/lib/aptible/cli/subcommands/aws_accounts.rb index 38ef023e..62993009 100644 --- a/lib/aptible/cli/subcommands/aws_accounts.rb +++ b/lib/aptible/cli/subcommands/aws_accounts.rb @@ -175,7 +175,8 @@ def aws_accounts 'resources in your AWS account' option :remove_discovery_role_arn, type: :boolean, desc: 'Remove the discovery ' \ - 'role ARN from this account' + 'role ARN from this ' \ + 'account' option :discovery_frequency, type: :string, desc: 'Discovery frequency (e.g., daily)' From 69b671f3cd8cb2e259890938389eaedb97b7fcb7 Mon Sep 17 00:00:00 2001 From: T Van Doren <210452321+tvdaptible@users.noreply.github.com> Date: Sat, 13 Dec 2025 05:08:01 -0800 Subject: [PATCH 14/14] use `make test` in CI for consistency with local dev env --- .github/workflows/test.yml | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b3e5e7e0..5a9e5df5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,19 +16,14 @@ jobs: fail-fast: false matrix: 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: |