aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKasper Timm Hansen <kaspth@gmail.com>2019-08-04 01:32:41 +0200
committerKasper Timm Hansen <kaspth@gmail.com>2019-08-04 01:32:41 +0200
commit03e44f93001db97953917e0a100c627e189e2be6 (patch)
tree644d82e33a8b7269bfeb22e7f363cd726a6c97be
parent6db2c426c0156dd3738673a676261693cfe92a8d (diff)
downloadrails-03e44f93001db97953917e0a100c627e189e2be6.tar.gz
rails-03e44f93001db97953917e0a100c627e189e2be6.tar.bz2
rails-03e44f93001db97953917e0a100c627e189e2be6.zip
Revise credentials diffing flow to use a separate diff command
Didn't like the complicated stuff that happened on credentials:edit. It would append to .gitattributes multiple times. Though I see why it was written that way. I'm cutting off for now, but since this new flow would require each developer to run --enable perhaps this should really be: 1. Developer enrolls Rails app by running `credentials:diff --enable` 2. credentials:edit checks .gitattributes for `diff=rails_credentials` and if the current file is covered by that. 3. If so, set up the "rails_credentials" driver automatically.
-rw-r--r--railties/lib/rails/commands/credentials/USAGE14
-rw-r--r--railties/lib/rails/commands/credentials/credentials_command.rb19
-rw-r--r--railties/lib/rails/commands/credentials/credentials_command/diffing.rb43
-rw-r--r--railties/test/commands/credentials_test.rb104
-rw-r--r--railties/test/isolation/abstract_unit.rb4
5 files changed, 79 insertions, 105 deletions
diff --git a/railties/lib/rails/commands/credentials/USAGE b/railties/lib/rails/commands/credentials/USAGE
index c8d3fb9eda..0396fcb403 100644
--- a/railties/lib/rails/commands/credentials/USAGE
+++ b/railties/lib/rails/commands/credentials/USAGE
@@ -30,6 +30,20 @@ You could prepend that to your server's start command like this:
RAILS_MASTER_KEY="very-secret-and-secure" server.start
+=== Set up Git to Diff Credentials
+
+Rails provides `rails credentials:diff --enable` to instruct Git to call `rails credentials:diff`
+when `git diff` is run on a credentials file.
+
+Any credentials files are set to use the "rails_credentials" diff driver in .gitattributes.
+Since Git requires the diff driver to be set up in a config file, the command uses
+the project local .git/config. Since that config isn't stored in Git each team member
+must enable separately.
+
+Or set up the "rails_credentials" diff driver globally with:
+
+ git config --global diff.rails_credentials.textconv "bin/rails credentials:diff"
+
=== Editing Credentials
This will open a temporary file in `$EDITOR` with the decrypted contents to edit
diff --git a/railties/lib/rails/commands/credentials/credentials_command.rb b/railties/lib/rails/commands/credentials/credentials_command.rb
index d1054f8b63..9b3a7dbb86 100644
--- a/railties/lib/rails/commands/credentials/credentials_command.rb
+++ b/railties/lib/rails/commands/credentials/credentials_command.rb
@@ -38,7 +38,6 @@ module Rails
end
say "File encrypted and saved."
- enable_credentials_diffing
rescue ActiveSupport::MessageEncryptor::InvalidMessage
say "Couldn't decrypt #{content_path}. Perhaps you passed the wrong key?"
end
@@ -50,14 +49,20 @@ module Rails
say credentials.read.presence || missing_credentials_message
end
- def diff(content_path)
- @content_path = content_path
+ option :enable, type: :boolean, default: false,
+ desc: "Pass `--enable` to make credential files diffable with `git diff`"
- extract_environment_option_from_argument(default_environment: extract_environment_from_path(content_path))
- require_application!
+ def diff(content_path = nil)
+ if @content_path = content_path
+ extract_environment_option_from_argument(default_environment: extract_environment_from_path(content_path))
+ require_application!
- say credentials.read.presence || credentials.content_path.read
- rescue
+ say credentials.read.presence || credentials.content_path.read
+ else
+ require_application!
+ enable_diffing if options[:enable]
+ end
+ rescue ActiveSupport::MessageEncryptor::InvalidMessage
say credentials.content_path.read
end
diff --git a/railties/lib/rails/commands/credentials/credentials_command/diffing.rb b/railties/lib/rails/commands/credentials/credentials_command/diffing.rb
index 1598ecaa8d..b7330178a3 100644
--- a/railties/lib/rails/commands/credentials/credentials_command/diffing.rb
+++ b/railties/lib/rails/commands/credentials/credentials_command/diffing.rb
@@ -3,45 +3,28 @@
module Rails::Command::CredentialsCommand::Diffing # :nodoc:
class Error < StandardError; end
- def enable_credentials_diffing
- unless already_answered? || enabled?
- answer = yes?("Would you like to make the credentials diff from git more readable in the future? [Y/n]")
+ def enable_diffing
+ if enabled?
+ say "Already enabled!"
+ else
+ enable
+ say "Diffing enabled! Editing a credentials file will display a diff of what actually changed."
end
-
- enable if answer
- FileUtils.touch(tracker) unless answer.nil?
rescue Error
- say "Couldn't setup git to enable credentials diffing"
+ say "Couldn't setup Git to enable credentials diffing."
end
private
- def already_answered?
- tracker.exist?
- end
-
def enabled?
- system_call("git config --get 'diff.rails_credentials.textconv'", accepted_codes: [0, 1])
+ system "git config --get diff.rails_credentials.textconv", out: File::NULL
end
def enable
- system_call("git config diff.rails_credentials.textconv 'bin/rails credentials:diff'", accepted_codes: [0])
-
- git_attributes = Rails.root.join(".gitattributes")
- File.open(git_attributes, "a+") do |file|
- file.write(<<~EOM)
- config/credentials/*.yml.enc diff=rails_credentials
- config/credentials.yml.enc diff=rails_credentials
- EOM
- end
- end
-
- def tracker
- Rails.root.join("tmp", "rails_pretty_credentials")
- end
+ raise Error unless system("git config diff.rails_credentials.textconv 'bin/rails credentials:diff'")
- def system_call(command_line, accepted_codes:)
- result = system(command_line)
- raise(Error) if accepted_codes.exclude?($?.exitstatus)
- result
+ Rails.root.join(".gitattributes").write(<<~end_of_template, mode: "a")
+ config/credentials/*.yml.enc diff=rails_credentials
+ config/credentials.yml.enc diff=rails_credentials
+ end_of_template
end
end
diff --git a/railties/test/commands/credentials_test.rb b/railties/test/commands/credentials_test.rb
index ec8fa4365e..e5397523e1 100644
--- a/railties/test/commands/credentials_test.rb
+++ b/railties/test/commands/credentials_test.rb
@@ -89,62 +89,60 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase
assert_match(/secret_key_base/, output)
end
- test "edit ask the user to opt in to pretty credentials" do
- assert_match(/Would you like to make the credentials diff from git/, run_edit_command)
+
+ test "show credentials" do
+ assert_match(/access_key_id: 123/, run_show_command)
end
- test "edit doesn't ask the user to opt in to pretty credentials when alreasy asked" do
- app_file("tmp/rails_pretty_credentials", "")
+ test "show command raises error when require_master_key is specified and key does not exist" do
+ remove_file "config/master.key"
+ add_to_config "config.require_master_key = true"
- assert_no_match(/Would you like to make the credentials diff from git/, run_edit_command)
+ assert_match(/Missing encryption key to decrypt file with/, run_show_command(allow_failure: true))
end
- test "edit doesn't ask the user to opt in when user already opted in" do
- content = <<~EOM
- [diff "rails_credentials"]
- textconv = bin/rails credentials:show
- EOM
- app_file(".git/config", content)
+ test "show command does not raise error when require_master_key is false and master key does not exist" do
+ remove_file "config/master.key"
+ add_to_config "config.require_master_key = false"
- assert_no_match(/Would you like to make the credentials diff from git/, run_edit_command)
+ assert_match(/Missing 'config\/master\.key' to decrypt credentials/, run_show_command)
end
- test "edit ask the user to opt in to pretty credentials, user accepts" do
- file = Tempfile.open("credentials_test")
- file.write("y")
- file.rewind
+ test "show command displays content specified by environment option" do
+ run_edit_command(environment: "production")
- run_edit_command(stdin: file.path)
+ assert_match(/access_key_id: 123/, run_show_command(environment: "production"))
+ end
+
+ test "show command properly expands environment option" do
+ run_edit_command(environment: "production")
- git_attributes = app_path(".gitattributes")
- expected = <<~EOM
+ output = run_show_command(environment: "prod")
+ assert_match(/access_key_id: 123/, output)
+ assert_no_match(/secret_key_base/, output)
+ end
+
+
+ test "diff enable diffing" do
+ run_diff_command(enable: true)
+
+ assert_equal <<~EOM, File.read(app_path(".gitattributes"))
config/credentials/*.yml.enc diff=rails_credentials
config/credentials.yml.enc diff=rails_credentials
EOM
- assert(File.exist?(git_attributes))
- assert_equal(expected, File.read(git_attributes))
+
Dir.chdir(app_path) do
assert_equal "bin/rails credentials:diff", `git config --get 'diff.rails_credentials.textconv'`.strip
end
- ensure
- file.close!
end
- test "edit ask the user to opt in to pretty credentials, user refuses" do
- file = Tempfile.open("credentials_test")
- file.write("n")
- file.rewind
-
- run_edit_command(stdin: file.path)
-
- git_attributes = app_path(".gitattributes")
- assert_not(File.exist?(git_attributes))
- ensure
- file.close!
- end
+ test "diff won't enable again if already enabled" do
+ app_file(".git/config", <<~EOM)
+ [diff "rails_credentials"]
+ textconv = bin/rails credentials:diff
+ EOM
- test "show credentials" do
- assert_match(/access_key_id: 123/, run_show_command)
+ assert_no_match(/Diffing enabled/, run_diff_command(enable: true))
end
test "diff from git diff left file" do
@@ -184,33 +182,6 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase
assert_match(encrypted_content, run_diff_command(content_path))
end
- test "show command raises error when require_master_key is specified and key does not exist" do
- remove_file "config/master.key"
- add_to_config "config.require_master_key = true"
-
- assert_match(/Missing encryption key to decrypt file with/, run_show_command(allow_failure: true))
- end
-
- test "show command does not raise error when require_master_key is false and master key does not exist" do
- remove_file "config/master.key"
- add_to_config "config.require_master_key = false"
-
- assert_match(/Missing 'config\/master\.key' to decrypt credentials/, run_show_command)
- end
-
- test "show command displays content specified by environment option" do
- run_edit_command(environment: "production")
-
- assert_match(/access_key_id: 123/, run_show_command(environment: "production"))
- end
-
- test "show command properly expands environment option" do
- run_edit_command(environment: "production")
-
- output = run_show_command(environment: "prod")
- assert_match(/access_key_id: 123/, output)
- assert_no_match(/secret_key_base/, output)
- end
private
def run_edit_command(editor: "cat", environment: nil, **options)
@@ -225,7 +196,8 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase
rails "credentials:show", args, **options
end
- def run_diff_command(path, **options)
- rails "credentials:diff", path, **options
+ def run_diff_command(path = nil, enable: nil, **options)
+ args = enable ? ["--enable"] : [path]
+ rails "credentials:diff", args, **options
end
end
diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb
index 6077ba3ee7..0fe62df8ba 100644
--- a/railties/test/isolation/abstract_unit.rb
+++ b/railties/test/isolation/abstract_unit.rb
@@ -301,7 +301,7 @@ module TestHelpers
# stderr:: true to pass STDERR output straight to the "real" STDERR.
# By default, the STDERR and STDOUT of the process will be
# combined in the returned string.
- def rails(*args, allow_failure: false, stderr: false, stdin: File::NULL)
+ def rails(*args, allow_failure: false, stderr: false)
args = args.flatten
fork = true
@@ -328,7 +328,7 @@ module TestHelpers
out_read.close
err_read.close if err_read
- $stdin.reopen(stdin, "r")
+ $stdin.reopen(File::NULL, "r")
$stdout.reopen(out_write)
$stderr.reopen(err_write)