Rollback uses explicit rollback variables if declared

Author: timothysmith0609

app/models/shipit/deploy_spec.rb

@@ -140,7 +140,7 @@ def rollback_steps!
     end
 
     def rollback_variables
-      if config.dig('rollback', 'variables').present?
+      if config('rollback', 'variables').present?
         Array.wrap(config('rollback', 'variables')).map(&VariableDefinition.method(:new))
       else
         # For backwards compatibility, fallback to using deploy_variables if no explicit rollback variables are set

test/dummy/config/secrets.test.json

@@ -17,5 +17,5 @@
       "teams": null
     }
   },
-  "redis_url": "redis://127.0.0.1:6379/7"
-}
+  "redis_url": "redis://shipit-engine.root.shopify.dev.internal:6379/7"
+}

test/dummy/config/secrets.yml

@@ -44,4 +44,4 @@ test:
       id: Iv1.bf2c2c45b449bfd9
       secret: ef694cd6e45223075d78d138ef014049052665f1
       teams:
-  redis_url: "redis://127.0.0.1:6379/7"
+  redis_url: "redis://shipit-engine.root.shopify.dev.internal:6379/7"

test/models/deploy_spec_test.rb

@@ -243,6 +243,31 @@ class DeploySpecTest < ActiveSupport::TestCase
       assert_equal ["kubernetes-deploy --max-watch-seconds 900 foo bar"], @spec.rollback_steps
     end
 
+    test "#rollback_variables returns an empty array by default" do
+      assert_equal [], @spec.deploy_variables
+    end
+
+    test "#rollback_variables returns an array of VariableDefinition instances" do
+      @spec.stubs(:load_config).returns('rollback' => { 'variables' => [{
+                                          'name' => 'SAFETY_DISABLED',
+                                          'title' => 'Set to 1 to do dangerous things',
+                                          'default' => 0
+                                        }] })
+
+      assert_equal 1, @spec.rollback_variables.size
+      variable_definition = @spec.rollback_variables.first
+      assert_equal 'SAFETY_DISABLED', variable_definition.name
+    end
+
+    test "#rollback_variables falls back to deploy_variables if rollback_variables itself is empty" do
+      @spec.stubs(:load_config).returns('deploy' => { 'variables' => [{
+                                          'name' => 'SAFETY_DISABLED',
+                                          'title' => 'Set to 1 to do dangerous things',
+                                          'default' => 0
+                                        }] })
+      assert_equal @spec.rollback_variables.map(&:to_h), @spec.deploy_variables.map(&:to_h)
+    end
+
     test "#discover_task_definitions include a kubernetes restart command if `kubernetes` is present" do
       @spec.stubs(:load_config).returns(
         'kubernetes' => {
@@ -381,7 +406,8 @@ class DeploySpecTest < ActiveSupport::TestCase
         },
         'rollback' => {
           'override' => nil,
-          'retries' => nil
+          'retries' => nil,
+          'variables' => []
         },
         'fetch' => nil,
         'tasks' => {}