Rename flag to "current-sha" and remove fallback to environment variable

Shopify · Nov 8, 2019 · 96250f0 · 96250f0
1 parent d4778c7
commit 96250f0
Show file tree

Hide file tree

Showing 7 changed files with 16 additions and 59 deletions.
diff --git a/exe/kubernetes-deploy b/exe/kubernetes-deploy
@@ -17,7 +17,7 @@ bindings = {}
 verbose_log_prefix = false
 max_watch_seconds = nil
 selector = nil
-revision = ENV["REVISION"]
+current_sha = ENV["REVISION"]
 
 ARGV.options do |opts|
   parser = Krane::BindingsParser.new
@@ -53,7 +53,7 @@ ARGV.options do |opts|
     puts "v#{Krane::VERSION}"
     exit
   end
-  opts.on("--revision=REVISION", "Expose SHA `current_sha` in ERB bindings") { |r| revision = r }
+  opts.on("--current-sha=CURRENT_SHA", "Expose SHA `current_sha` in ERB bindings") { |r| current_sha = r }
   opts.parse!
   bindings = parser.parse
 end
@@ -74,7 +74,7 @@ begin
     runner = KubernetesDeploy::DeployTask.new(
       namespace: namespace,
       context: context,
-      current_sha: revision,
+      current_sha: current_sha,
       template_paths: paths,
       bindings: bindings,
       logger: logger,

diff --git a/exe/kubernetes-render b/exe/kubernetes-render
@@ -9,7 +9,7 @@ require 'optparse'
 
 template_dir = []
 bindings = {}
-revision = ENV["REVISION"]
+current_sha = ENV["REVISION"]
 
 ARGV.options do |opts|
   parser = Krane::BindingsParser.new
@@ -18,7 +18,7 @@ ARGV.options do |opts|
   opts.on("--template-dir=DIR", "Set the template dir (default: config/deploy/$ENVIRONMENT).") do |d|
     template_dir = [d]
   end
-  opts.on("--revision=REVISION", "Expose SHA `current_sha` in ERB bindings") { |r| revision = r }
+  opts.on("--current-sha=CURRENT_SHA", "Expose SHA `current_sha` in ERB bindings") { |r| current_sha = r }
   opts.parse!
   bindings = parser.parse
 end
@@ -29,7 +29,7 @@ logger = Krane::FormattedLogger.build(verbose_prefix: false)
 begin
   Krane::OptionsHelper.with_processed_template_paths(template_dir) do |dir|
     runner = KubernetesDeploy::RenderTask.new(
-      current_sha: revision,
+      current_sha: current_sha,
       template_dir: dir.first,
       bindings: bindings,
     )

diff --git a/lib/krane/cli/deploy_command.rb b/lib/krane/cli/deploy_command.rb
@@ -31,7 +31,7 @@ class DeployCommand
                                   default: true },
         "verify-result" => { type: :boolean, default: true,
                              desc: "Verify workloads correctly deployed" },
-        "revision" => { type: :string, banner: "SHA", desc: "Expose SHA `current_sha` in ERB bindings" },
+        "current-sha" => { type: :string, banner: "SHA", desc: "Expose SHA `current_sha` in ERB bindings" },
 
       }
 
@@ -59,7 +59,7 @@ def self.from_options(namespace, context, options)
           deploy = ::Krane::DeployTask.new(
             namespace: namespace,
             context: context,
-            current_sha: options.fetch(:revision, ENV["REVISION"]),
+            current_sha: options['current-sha'],
             template_paths: paths,
             bindings: bindings_parser.parse,
             logger: logger,

diff --git a/lib/krane/cli/render_command.rb b/lib/krane/cli/render_command.rb
@@ -7,7 +7,7 @@ class RenderCommand
         bindings: { type: :array, banner: "foo=bar abc=def", desc: 'Bindings for erb' },
         filenames: { type: :array, banner: 'config/deploy/production config/deploy/my-extra-resource.yml',
                      required: true, aliases: 'f', desc: 'Directories and files to render' },
-        revision: { type: :string, banner: "SHA", desc: "Expose SHA `current_sha` in ERB bindings" },
+        'current-sha': { type: :string, banner: "SHA", desc: "Expose SHA `current_sha` in ERB bindings" },
       }
 
       def self.from_options(options)
@@ -20,7 +20,7 @@ def self.from_options(options)
 
         ::Krane::OptionsHelper.with_processed_template_paths(options[:filenames]) do |paths|
           runner = ::Krane::RenderTask.new(
-            current_sha: options.fetch(:revision, ENV["REVISION"]),
+            current_sha: options['current-sha'],
             template_paths: paths,
             bindings: bindings_parser.parse,
           )

diff --git a/test/exe/deploy_test.rb b/test/exe/deploy_test.rb
@@ -4,8 +4,6 @@
 require 'krane/bindings_parser'
 
 class DeployTest < Krane::TestCase
-  include EnvTestHelper
-
   def test_deploy_with_default_options
     set_krane_deploy_expectations
     krane_deploy!
@@ -81,27 +79,10 @@ def test_deploy_fails_without_filename
     end
   end
 
-  def test_deploy_uses_revision_env_var
-    test_sha = "TEST"
-    with_env("REVISION", test_sha) do
-      set_krane_deploy_expectations(new_args: { current_sha: test_sha })
-      krane_deploy!
-    end
-  end
-
-  def test_deploy_uses_revision
+  def test_deploy_uses_current_sha
     test_sha = "TEST"
     set_krane_deploy_expectations(new_args: { current_sha: test_sha })
-    krane_deploy!(flags: "--revision #{test_sha}")
-  end
-
-  def test_render_uses_revision_flag_over_env_var
-    env_test_sha = "NOT_TEST_VALUE_TEST"
-    test_sha = "TEST"
-    with_env("REVISION", env_test_sha) do
-      set_krane_deploy_expectations(new_args: { current_sha: test_sha })
-      krane_deploy!(flags: "--revision #{test_sha}")
-    end
+    krane_deploy!(flags: "--current-sha #{test_sha}")
   end
 
   private

diff --git a/test/exe/render_test.rb b/test/exe/render_test.rb
@@ -3,7 +3,6 @@
 require 'krane/cli/krane'
 
 class RendertTest < Krane::TestCase
-  include EnvTestHelper
   def test_render_with_default_options
     install_krane_render_expectations
     krane_render!
@@ -24,26 +23,9 @@ def test_render_parses_bindings
   end
 
   def test_render_uses_current_sha
-    test_sha = "TEST"
-    with_env("REVISION", test_sha) do
-      install_krane_render_expectations
-      krane_render!
-    end
-  end
-
-  def test_render_uses_revision
     test_sha = "TEST"
     install_krane_render_expectations(current_sha: test_sha)
-    krane_render!("--revision #{test_sha}")
-  end
-
-  def test_render_uses_revision_flag_over_env_current_sha
-    env_test_sha = "NOT_TEST_VALUE_TEST"
-    test_sha = "TEST"
-    with_env("REVISION", env_test_sha) do
-      install_krane_render_expectations(current_sha: test_sha)
-      krane_render!("--revision #{test_sha}")
-    end
+    krane_render!("--current-sha #{test_sha}")
   end
 
   private

diff --git a/test/integration/krane_test.rb b/test/integration/krane_test.rb
@@ -2,8 +2,6 @@
 require 'integration_test_helper'
 
 class KraneTest < Krane::IntegrationTest
-  include EnvTestHelper
-
   def test_restart_black_box
     assert_deploy_success(deploy_fixtures("hello-cloud", subset: ["configmap-data.yml", "web.yml.erb", "redis.yml"]))
     refute(fetch_restarted_at("web"), "RESTARTED_AT env on fresh deployment")
@@ -38,19 +36,15 @@ def test_render_black_box
     bindings = "data=#{data_value}"
     test_sha = rand(10_000).to_s
 
-    out, err, status = nil
-    with_env("REVISION", test_sha) do
-      out, err, status = krane_black_box("render", "-f #{paths.join(' ')} --bindings #{bindings}")
-    end
+    out, err, status = krane_black_box("render", "-f #{paths.join(' ')} --bindings #{bindings} "\
+      "--current-sha #{test_sha}")
 
     assert_predicate(status, :success?)
     assert_match("Success", err)
     assert_match(test_sha, out)
     assert_match(data_value, out)
 
-    with_env("REVISION", test_sha) do
-      out, err, status = krane_black_box("render", "-f #{paths.join(' ')}")
-    end
+    out, err, status = krane_black_box("render", "-f #{paths.join(' ')} --current-sha #{test_sha}")
 
     refute_predicate(status, :success?)
     assert_match("FAILURE", err)