Add backup_retention_policy tests and prompt when reducing backups

README.md

@@ -36,7 +36,7 @@ Commands:
   aptible backup:purge BACKUP_ID                                                                                                                                                                                                                                                                                                                                # Permanently delete a backup and any copies of it
   aptible backup:restore BACKUP_ID [--environment ENVIRONMENT_HANDLE] [--handle HANDLE] [--container-size SIZE_MB] [--disk-size SIZE_GB] [--container-profile PROFILE] [--iops IOPS] [--key-arn KEY_ARN]                                                                                                                                                        # Restore a backup
   aptible backup_retention_policy [ENVIRONMENT_HANDLE]                                                                                                                                                                                                                                                                                                          # Show the current backup retention policy for the environment
-  aptible backup_retention_policy:set [ENVIRONMENT_HANDLE] [--daily DAILY_BACKUPS] [--monthly MONTHLY_BACKUPS] [--yearly YEARLY_BACKUPS] [--make-copy|--no-make-copy] [--keep-final|--no-keep-final]                                                                                                                                                            # Change the environment's backup retention policy
+  aptible backup_retention_policy:set [ENVIRONMENT_HANDLE] [--daily DAILY_BACKUPS] [--monthly MONTHLY_BACKUPS] [--yearly YEARLY_BACKUPS] [--make-copy|--no-make-copy] [--keep-final|--no-keep-final] [--force]                                                                                                                                                  # Change the environment's backup retention policy
   aptible config                                                                                                                                                                                                                                                                                                                                                # Print an app's current configuration
   aptible config:add [VAR1=VAL1] [VAR2=VAL2] [...]                                                                                                                                                                                                                                                                                                              # Add an ENV variable to an app
   aptible config:get [VAR1]                                                                                                                                                                                                                                                                                                                                     # Print a specific key within an app's current configuration

lib/aptible/cli/subcommands/backup_retention_policy.rb

@@ -33,7 +33,7 @@ def self.included(thor)
             desc 'backup_retention_policy:set [ENVIRONMENT_HANDLE] ' \
                  '[--daily DAILY_BACKUPS] [--monthly MONTHLY_BACKUPS] ' \
                  '[--yearly YEARLY_BACKUPS] [--make-copy|--no-make-copy] ' \
-                 '[--keep-final|--no-keep-final]',
+                 '[--keep-final|--no-keep-final] [--force]',
                  "Change the environment's backup retention policy"
             option :daily, type: :numeric,
                            desc: 'Number of daily backups to retain'
@@ -43,12 +43,14 @@ def self.included(thor)
                             desc: 'Number of yearly backups to retain'
             option :make_copy, type: :boolean,
                                desc: 'If backup copies should be created'
-            option(
-              :keep_final,
-              type: :boolean,
-              desc: 'If final backups should be kept when databases are ' \
-                    'deprovisioned'
-            )
+            option :keep_final,
+                   type: :boolean,
+                   desc: 'If final backups should be kept when databases are ' \
+                         'deprovisioned'
+            option :force,
+                   type: :boolean,
+                   desc: 'Do not prompt for confirmation if the new policy ' \
+                         'retains fewer backups than the current policy'
             define_method 'backup_retention_policy:set' do |env|
               if options.empty?
                 raise Thor::Error,
@@ -74,6 +76,37 @@ def self.included(thor)
                                    'specify all attributes to create one.'
               end
 
+              # Check if the number of backups over any period have been reduced
+              do_confirm = %i(daily monthly yearly).any? do |a|
+                cur = current_policy.try(a)
+                next if cur.nil?
+                cur > attrs[a]
+              end
+
+              # Check if any of the boolean fields have been disabled
+              do_confirm ||= %i(make_copy keep_final).any? do |a|
+                cur = current_policy.try(a)
+                next if cur.nil?
+                cur && !attrs[a]
+              end
+
+              if do_confirm && !options[:force]
+                m = 'The specified backup retention policy retains fewer ' \
+                    "backups than the environment's current policy. This may " \
+                    'result in the deletion of existing, automated backups ' \
+                    "if they exceed the new policy's rules and it may " \
+                    "violate your company's internal compliance controls. " \
+                    'For more information, see https://www.aptible.com/docs' \
+                    '/core-concepts/managed-databases/managing-databases' \
+                    '/database-backups.'
+
+                m = set_color(m, :yellow)
+                print_wrapped m
+                confirmation = yes?('Do you want to proceed [y/N]:')
+                puts ''
+                raise Thor::Error, 'Aborting' unless confirmation
+              end
+
               new_policy = account.create_backup_retention_policy!(**attrs)
 
               Formatter.render(Renderer.current) do |root|

spec/aptible/cli/subcommands/backup_retention_policy_spec.rb

@@ -0,0 +1,115 @@
+require 'spec_helper'
+
+describe Aptible::CLI::Agent do
+  let(:token) { 'some-token' }
+  let(:account) { Fabricate(:account, handle: 'test') }
+  let(:database) { Fabricate(:database, account: account, handle: 'some-db') }
+  let!(:policy) do
+    # created_at: 2016-06-14 13:24:11 +0000
+    Fabricate(:backup_retention_policy, account: account)
+  end
+
+  let(:default_handle) { 'some-db-at-2016-06-14-13-24-11' }
+
+  before do
+    allow(subject).to receive(:fetch_token).and_return(token)
+    allow(Aptible::Api::Account).to receive(:all) { [account] }
+  end
+
+  describe '#backup_retention_policy' do
+    it 'raises an error if the environment has no policy' do
+      allow(account).to receive(:backup_retention_policies).and_return([])
+      expect { subject.backup_retention_policy('test') }
+        .to raise_error(/does not have a custom backup retention policy/)
+    end
+
+    it "prints the enviroment's current policy" do
+      subject.backup_retention_policy('test')
+      out = captured_output_text
+      expect(out).to match(/daily: 30/i)
+      expect(out).to match(/monthly: 12/i)
+      expect(out).to match(/yearly: 6/i)
+      expect(out).to match(/make copy: true/i)
+      expect(out).to match(/keep final: true/i)
+      expect(out).to match(/environment: test/i)
+    end
+  end
+
+  describe '#backup_retention_policy:set' do
+    it 'requires all attributes if the environment has no policy' do
+      allow(account).to receive(:backup_retention_policies).and_return([])
+      opts = {
+        daily: 3,
+        monthly: 2,
+        yearly: 1,
+        make_copy: false,
+        keep_final: true
+      }
+
+      opts.each_key do |k|
+        missing_opts = opts.clone
+        missing_opts.delete(k)
+
+        subject.options = missing_opts
+        expect { subject.send('backup_retention_policy:set', 'test') }
+          .to raise_error(/please specify all attributes/i)
+      end
+
+      expect(account).to receive(:create_backup_retention_policy!)
+        .with(**opts).and_return(Fabricate(:backup_retention_policy))
+      subject.options = opts
+      subject.send('backup_retention_policy:set', 'test')
+    end
+
+    it 'merges provided options with the current policy' do
+      expected_opts = {
+        daily: 5,
+        monthly: policy.monthly,
+        yearly: policy.yearly,
+        make_copy: policy.make_copy,
+        keep_final: false
+      }
+
+      expect(account).to receive(:create_backup_retention_policy!)
+        .with(**expected_opts).and_return(Fabricate(:backup_retention_policy))
+      subject.options = { daily: 5, keep_final: false, force: true }
+      subject.send('backup_retention_policy:set', 'test')
+    end
+
+    it 'prompts the user if the new policy retains fewer backups' do
+      subject.options = { daily: 0 }
+
+      # Reject Prompt
+      expect(subject).to receive(:yes?).with(/do you want to proceed/i)
+
+      expect { subject.send('backup_retention_policy:set', 'test') }
+        .to raise_error(/aborting/i)
+
+      # Accept Prompt
+      expect(subject).to receive(:yes?).with(/do you want to proceed/i)
+        .and_return(true)
+
+      expect(account).to receive(:create_backup_retention_policy!)
+        .and_return(Fabricate(:backup_retention_policy))
+
+      subject.send('backup_retention_policy:set', 'test')
+    end
+
+    it '--force skips the confirmation promt' do
+      subject.options = { make_copy: false }
+
+      # Reject Prompt
+      expect(subject).to receive(:yes?).with(/do you want to proceed/i)
+
+      expect { subject.send('backup_retention_policy:set', 'test') }
+        .to raise_error(/aborting/i)
+
+      # --force
+      subject.options[:force] = true
+      expect(account).to receive(:create_backup_retention_policy!)
+        .and_return(Fabricate(:backup_retention_policy))
+
+      subject.send('backup_retention_policy:set', 'test')
+    end
+  end
+end

spec/fabricators/account_fabricator.rb

@@ -28,5 +28,6 @@ def each_certificate(&block)
   certificates { [] }
   log_drains { [] }
   metric_drains { [] }
+  backup_retention_policies { [] }
   created_at { Time.now }
 end

spec/fabricators/backup_retention_policy_fabricator.rb

@@ -0,0 +1,18 @@
+class StubBackupRetentionPolicy < OpenStruct
+  def reload
+    self
+  end
+end
+
+Fabricator(:backup_retention_policy, from: :stub_backup_retention_policy) do
+  id { sequence(:backup_retention_policy_id) }
+  created_at { Time.now }
+  daily { 30 }
+  monthly { 12 }
+  yearly { 6 }
+  make_copy { true }
+  keep_final { true }
+  account
+
+  after_create { |policy| policy.account.backup_retention_policies << policy }
+end