[SC-35118] Support deployment without overloaded environment variables.

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    aptible-cli (0.26.0)
+    aptible-cli (0.26.1)
       activesupport (>= 4.0, < 6.0)
       aptible-api (~> 1.12)
       aptible-auth (~> 1.4)
@@ -120,7 +120,7 @@ GEM
     parser (2.7.2.0)
       ast (~> 2.4.1)
     powerpack (0.1.3)
-    pry (0.14.2)
+    pry (0.15.2)
       coderay (~> 1.1)
       method_source (~> 1.0)
     public_suffix (3.1.1)

lib/aptible/cli/subcommands/deploy.rb

@@ -2,9 +2,8 @@ module Aptible
   module CLI
     module Subcommands
       module Deploy
-        DOCKER_IMAGE_DEPLOY_ARGS = Hash[%w(
+        DEPRECATED_ENV = Hash[%w(
           APTIBLE_DOCKER_IMAGE
-          APTIBLE_PRIVATE_REGISTRY_EMAIL
           APTIBLE_PRIVATE_REGISTRY_USERNAME
           APTIBLE_PRIVATE_REGISTRY_PASSWORD
         ).map do |var|
@@ -40,11 +39,23 @@ def self.included(thor)
                                        desc: 'This option only affects new ' \
                                              'services, not existing ones. ' \
                                              'Examples: m c r'
-            DOCKER_IMAGE_DEPLOY_ARGS.each_pair do |opt, var|
-              option opt,
-                     type: :string, banner: var,
-                     desc: "Shorthand for #{var}=..."
-            end
+
+            option :docker_image,
+                   type: :string,
+                   desc: 'The docker image to deploy. If none specified, ' \
+                         'the currently deployed image will be pulled again'
+            option :private_registry_username,
+                   type: :string,
+                   desc: 'Username for Docker images located in a private ' \
+                        'repository'
+            option :private_registry_password,
+                   type: :string,
+                   desc: 'Password for Docker images located in a private ' \
+                         'repository'
+            option :private_registry_email,
+                   type: :string,
+                   desc: 'This parameter is deprecated'
+
             app_options
             def deploy(*args)
               telemetry(__method__, options)
@@ -62,20 +73,43 @@ def deploy(*args)
 
               env = extract_env(args)
 
-              DOCKER_IMAGE_DEPLOY_ARGS.each_pair do |opt, var|
+              DEPRECATED_ENV.each_pair do |opt, var|
                 val = options[opt]
+                dasherized = "--#{opt.to_s.tr('_', '-')}"
+                if env[var]
+                  m = "WARNING: The environment variable #{var} " \
+                      'will be deprecated. Use the option ' \
+                      "#{dasherized}, instead."
+                  CLI.logger.warn m
+                end
                 next unless val
                 if env[var] && env[var] != val
-                  dasherized = "--#{opt.to_s.tr('_', '-')}"
                   raise Thor::Error, "The options #{dasherized} and #{var} " \
                                      'cannot be set to different values'
                 end
-                env[var] = val
+              end
+
+              settings = {}
+              sensitive_settings = {}
+
+              if options[:docker_image]
+                settings['APTIBLE_DOCKER_IMAGE'] = options[:docker_image]
+              end
+
+              if options[:private_registry_username]
+                sensitive_settings['APTIBLE_PRIVATE_REGISTRY_USERNAME'] =
+                  options[:private_registry_username]
+              end
+              if options[:private_registry_password]
+                sensitive_settings['APTIBLE_PRIVATE_REGISTRY_PASSWORD'] =
+                  options[:private_registry_password]
               end
 
               opts = {
                 type: 'deploy',
                 env: env,
+                settings: settings,
+                sensitive_settings: sensitive_settings,
                 git_ref: git_ref,
                 container_count: options[:container_count],
                 container_size: options[:container_size],
@@ -84,7 +118,7 @@ def deploy(*args)
 
               allow_it = [
                 opts[:git_ref],
-                opts[:env].try(:[], 'APTIBLE_DOCKER_IMAGE'),
+                opts[:settings].try(:[], 'APTIBLE_DOCKER_IMAGE'),
                 app.status == 'provisioned'
               ].any? { |x| x }
 

lib/aptible/cli/version.rb

@@ -1,5 +1,5 @@
 module Aptible
   module CLI
-    VERSION = '0.26.0'.freeze
+    VERSION = '0.26.1'.freeze
   end
 end

spec/aptible/cli/subcommands/deploy_spec.rb

@@ -45,7 +45,10 @@ def stub_options(**opts)
         stub_options(docker_image: 'foobar')
 
         expect(app).to receive(:create_operation!)
-          .with(type: 'deploy', env: { 'APTIBLE_DOCKER_IMAGE' => 'foobar' })
+          .with(
+            type: 'deploy',
+            settings: { 'APTIBLE_DOCKER_IMAGE' => 'foobar' }
+          )
           .and_return(operation)
         expect(subject).to receive(:attach_to_operation_logs)
           .with(operation)
@@ -60,14 +63,13 @@ def stub_options(**opts)
           private_registry_password: 'qux'
         )
 
-        env = {
-          'APTIBLE_PRIVATE_REGISTRY_EMAIL' => 'foo',
+        sensitive_settings = {
           'APTIBLE_PRIVATE_REGISTRY_USERNAME' => 'bar',
           'APTIBLE_PRIVATE_REGISTRY_PASSWORD' => 'qux'
         }
 
         expect(app).to receive(:create_operation!)
-          .with(type: 'deploy', env: env)
+          .with(type: 'deploy', sensitive_settings: sensitive_settings)
           .and_return(operation)
         expect(subject).to receive(:attach_to_operation_logs)
           .with(operation)
@@ -96,10 +98,12 @@ def stub_options(**opts)
       end
 
       it 'allows setting configuration variables' do
-        stub_options
+        stub_options(docker_image: 'foobar')
 
         expect(app).to receive(:create_operation!)
-          .with(type: 'deploy', env: { 'FOO' => 'bar', 'BAR' => 'qux' })
+          .with(type: 'deploy',
+                env: { 'FOO' => 'bar', 'BAR' => 'qux' },
+                settings: { 'APTIBLE_DOCKER_IMAGE' => 'foobar' })
           .and_return(operation)
         expect(subject).to receive(:attach_to_operation_logs)
           .with(operation)
@@ -137,14 +141,45 @@ def stub_options(**opts)
         stub_options(docker_image: 'foobar')
 
         expect(app).to receive(:create_operation!)
-          .with(type: 'deploy', env: { 'APTIBLE_DOCKER_IMAGE' => 'foobar' })
+          .with(type: 'deploy',
+                env: { 'APTIBLE_DOCKER_IMAGE' => 'foobar' },
+                settings: { 'APTIBLE_DOCKER_IMAGE' => 'foobar' })
           .and_return(operation)
         expect(subject).to receive(:attach_to_operation_logs)
           .with(operation)
 
         subject.deploy('APTIBLE_DOCKER_IMAGE=foobar')
       end
 
+      context 'dasherized option for image' do
+        it 'deploys via operation.settings' do
+          stub_options(docker_image: 'foobar')
+
+          expect(app).to receive(:create_operation!)
+            .with(type: 'deploy',
+                  settings: { 'APTIBLE_DOCKER_IMAGE' => 'foobar' })
+            .and_return(operation)
+          expect(subject).to receive(:attach_to_operation_logs)
+            .with(operation)
+
+          subject.deploy
+        end
+      end
+
+      context '(deprecated) environment variable style for image' do
+        it 'deploys via operation.env' do
+          stub_options
+
+          expect(app).to receive(:create_operation!)
+            .with(type: 'deploy', env: { 'APTIBLE_DOCKER_IMAGE' => 'foobar' })
+            .and_return(operation)
+          expect(subject).to receive(:attach_to_operation_logs)
+            .with(operation)
+
+          subject.deploy('APTIBLE_DOCKER_IMAGE=foobar')
+        end
+      end
+
       it 'reject contradictory command line argumnts' do
         stub_options(docker_image: 'foobar')