From f4d61b342b06d1eb7730bc750575b3aff01f3c79 Mon Sep 17 00:00:00 2001
From: Katrina Verey <katrina.verey@shopify.com>
Date: Thu, 28 Nov 2019 12:11:52 -0500
Subject: [PATCH] Exclude stderr from parsing on decryption success

---
 CHANGELOG.md                                  |  3 ++
 lib/krane/ejson_secret_provisioner.rb         | 11 ++++---
 .../krane/ejson_secret_provisioner_test.rb    | 31 +++++++++++++++++++
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b08a4ba97..98dd95457 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,8 @@
 ## next
 
+*Bug Fixes*
+- Fix a bug causing secret generation from ejson to fail when decryption succeeded but a warning was also emitted. [#647](https://github.com/Shopify/krane/pull/647)
+
 # 1.0.0
 
 We've renamed the gem and cli to Krane.
diff --git a/lib/krane/ejson_secret_provisioner.rb b/lib/krane/ejson_secret_provisioner.rb
index a4a6e22fa..6f09fcb09 100644
--- a/lib/krane/ejson_secret_provisioner.rb
+++ b/lib/krane/ejson_secret_provisioner.rb
@@ -134,10 +134,13 @@ def with_decrypted_ejson
     end
 
     def decrypt_ejson(key_dir)
-      # ejson seems to dump both errors and output to STDOUT
-      out_err, st = Open3.capture2e("EJSON_KEYDIR=#{key_dir} ejson decrypt #{@ejson_file}")
-      raise EjsonSecretError, out_err unless st.success?
-      JSON.parse(out_err)
+      out, err, st = Open3.capture3("EJSON_KEYDIR=#{key_dir} ejson decrypt #{@ejson_file}")
+      unless st.success?
+        # older ejson versions dump some errors to STDOUT
+        msg = err.presence || out
+        raise EjsonSecretError, msg
+      end
+      JSON.parse(out)
     rescue JSON::ParserError
       raise EjsonSecretError, "Failed to parse decrypted ejson"
     end
diff --git a/test/unit/krane/ejson_secret_provisioner_test.rb b/test/unit/krane/ejson_secret_provisioner_test.rb
index 6daab7262..e1a03732b 100644
--- a/test/unit/krane/ejson_secret_provisioner_test.rb
+++ b/test/unit/krane/ejson_secret_provisioner_test.rb
@@ -48,6 +48,37 @@ def test_run_with_bad_private_key_in_cloud_keys
     end
   end
 
+  def test_decryption_failure_with_error_on_stdout_reports_error
+    # ejson < 1.2 prints errors on stdout
+    Open3.expects(:capture3).with(regexp_matches(/ejson decrypt/))
+      .returns(["Some error from ejson", "", stub(success?: false)])
+    msg = "Generation of Kubernetes secrets from ejson failed: Some error from ejson"
+    assert_raises_message(Krane::EjsonSecretError, msg) do
+      build_provisioner(fixture_path('ejson-cloud')).resources
+    end
+  end
+
+  def test_decryption_successful_but_warning_on_stderr_does_not_confuse_us
+    valid_response = {
+      "_public_key" => fixture_public_key,
+      "kubernetes_secrets" =>
+        {
+          "test" => {
+            "_type" => "Opaque",
+            "data" => { "test" => "true" },
+          },
+        },
+    }.to_json
+
+    Open3.expects(:capture3).with(regexp_matches(/ejson decrypt/))
+      .returns([valid_response, "Permissions warning!", stub(success?: true)])
+    stub_server_dry_run_version_request
+    stub_server_dry_run_validation_request
+
+    resources = build_provisioner(fixture_path('ejson-cloud')).resources
+    refute_empty(resources)
+  end
+
   def test_no_ejson_keys_secret_provided
     assert_raises_message(Krane::EjsonSecretError,
       /Generation of Kubernetes secrets from ejson failed: Secret ejson-keys not provided, cannot decrypt secrets/) do