Merge branch 'qa-ml-improve-git-repo-auth' into 'master'

Add git credentials to .netrc when needed

Closes gitlab-org/quality/nightly#57 and #56857

See merge request gitlab-org/gitlab-ce!24691

3 files changed, 124 insertions, 58 deletions

diff --git a/qa/qa/git/repository.rb b/qa/qa/git/repository.rb
index ac8dcbf0d83..0aa94101098 100644
--- a/qa/qa/git/repository.rb
+++ b/qa/qa/git/repository.rb
@@ -5,15 +5,19 @@ require 'uri'
 require 'open3'
 require 'fileutils'
 require 'tmpdir'
+require 'tempfile'
+require 'securerandom'
 
 module QA
   module Git
     class Repository
       include Scenario::Actable
 
-      attr_writer :password, :use_lfs
+      attr_writer :use_lfs
       attr_accessor :env_vars
 
+      InvalidCredentialsError = Class.new(RuntimeError)
+
       def initialize
         # We set HOME to the current working directory (which is a
         # temporary directory created in .perform()) so the temporarily dropped
@@ -28,6 +32,14 @@ module QA
         end
       end
 
+      def password=(password)
+        @password = password
+
+        raise InvalidCredentialsError, "Please provide a username when setting a password" unless username
+
+        try_add_credentials_to_netrc
+      end
+
       def uri=(address)
         @uri = URI(address)
       end
@@ -148,16 +160,7 @@ module QA
         return unless add_credentials?
         return if netrc_already_contains_content?
 
-        # Despite libcurl supporting a custom .netrc location through the
-        # CURLOPT_NETRC_FILE environment variable, git does not support it :(
-        # Info: https://curl.haxx.se/libcurl/c/CURLOPT_NETRC_FILE.html
-        #
-        # This will create a .netrc in the correct working directory, which is
-        # a temporary directory created in .perform()
-        #
-        FileUtils.mkdir_p(tmp_home_dir)
-        File.open(netrc_file_path, 'a') { |file| file.puts(netrc_content) }
-        File.chmod(0600, netrc_file_path)
+        save_netrc_content
       end
 
       private
@@ -175,7 +178,6 @@ module QA
       def add_credentials?
         return false if !username || !password
         return true unless ssh_key_set?
-        return true if ssh_key_set? && use_lfs?
 
         false
       end
@@ -214,6 +216,23 @@ module QA
         end
       end
 
+      def read_netrc_content
+        File.exist?(netrc_file_path) ? File.readlines(netrc_file_path) : []
+      end
+
+      def save_netrc_content
+        # Despite libcurl supporting a custom .netrc location through the
+        # CURLOPT_NETRC_FILE environment variable, git does not support it :(
+        # Info: https://curl.haxx.se/libcurl/c/CURLOPT_NETRC_FILE.html
+        #
+        # This will create a .netrc in the correct working directory, which is
+        # a temporary directory created in .perform()
+        #
+        FileUtils.mkdir_p(tmp_home_dir)
+        File.open(netrc_file_path, 'a') { |file| file.puts(netrc_content) }
+        File.chmod(0600, netrc_file_path)
+      end
+
       def tmp_home_dir
         @tmp_home_dir ||= File.join(Dir.tmpdir, "qa-netrc-credentials", $$.to_s)
       end
@@ -227,8 +246,7 @@ module QA
       end
 
       def netrc_already_contains_content?
-        File.exist?(netrc_file_path) &&
-          File.readlines(netrc_file_path).grep(/^#{netrc_content}$/).any?
+        read_netrc_content.grep(/^#{netrc_content}$/).any?
       end
     end
   end
diff --git a/qa/qa/resource/repository/push.rb b/qa/qa/resource/repository/push.rb
index 32f15547da2..a5827fb6e73 100644
--- a/qa/qa/resource/repository/push.rb
+++ b/qa/qa/resource/repository/push.rb
@@ -67,8 +67,6 @@ module QA
               email = user.email
             end
 
-            repository.try_add_credentials_to_netrc
-
             @output += repository.clone
             repository.configure_identity(username, email)
 
diff --git a/qa/spec/git/repository_spec.rb b/qa/spec/git/repository_spec.rb
index faa154c78da..4a350cd6c42 100644
--- a/qa/spec/git/repository_spec.rb
+++ b/qa/spec/git/repository_spec.rb
@@ -1,69 +1,119 @@
 describe QA::Git::Repository do
   include Support::StubENV
 
-  let(:repository) { described_class.new }
+  shared_context 'git directory' do
+    let(:repository) { described_class.new }
+    let(:tmp_git_dir) { Dir.mktmpdir }
+    let(:tmp_netrc_dir) { Dir.mktmpdir }
 
-  before do
-    stub_env('GITLAB_USERNAME', 'root')
-    cd_empty_temp_directory
-    set_bad_uri
-    repository.use_default_credentials
-  end
+    before do
+      stub_env('GITLAB_USERNAME', 'root')
+      cd_empty_temp_directory
+      set_bad_uri
 
-  describe '#clone' do
-    it 'is unable to resolve host' do
-      expect(repository.clone).to include("fatal: unable to access 'http://root@foo/bar.git/'")
+      allow(repository).to receive(:tmp_home_dir).and_return(tmp_netrc_dir)
     end
-  end
 
-  describe '#push_changes' do
-    before do
-      `git init` # need a repo to push from
+    after do
+      # Switch to a safe dir before deleting tmp dirs to avoid dir access errors
+      FileUtils.cd __dir__
+      FileUtils.remove_entry_secure(tmp_git_dir, true)
+      FileUtils.remove_entry_secure(tmp_netrc_dir, true)
     end
 
-    it 'fails to push changes' do
-      expect(repository.push_changes).to include("error: failed to push some refs to 'http://root@foo/bar.git'")
+    def cd_empty_temp_directory
+      FileUtils.cd tmp_git_dir
+    end
+
+    def set_bad_uri
+      repository.uri = 'http://foo/bar.git'
     end
   end
 
-  describe '#git_protocol=' do
-    [0, 1, 2].each do |version|
-      it "configures git to use protocol version #{version}" do
-        expect(repository).to receive(:run).with("git config protocol.version #{version}")
-        repository.git_protocol = version
+  context 'with default credentials' do
+    include_context 'git directory' do
+      before do
+        repository.use_default_credentials
       end
     end
 
-    it 'raises an error if the version is unsupported' do
-      expect { repository.git_protocol = 'foo' }.to raise_error(ArgumentError, "Please specify the protocol you would like to use: 0, 1, or 2")
+    describe '#clone' do
+      it 'is unable to resolve host' do
+        expect(repository.clone).to include("fatal: unable to access 'http://root@foo/bar.git/'")
+      end
     end
-  end
 
-  describe '#fetch_supported_git_protocol' do
-    it "reports the detected version" do
-      expect(repository).to receive(:run).and_return("packet: git< version 2")
-      expect(repository.fetch_supported_git_protocol).to eq('2')
+    describe '#push_changes' do
+      before do
+        `git init` # need a repo to push from
+      end
+
+      it 'fails to push changes' do
+        expect(repository.push_changes).to include("error: failed to push some refs to 'http://root@foo/bar.git'")
+      end
     end
 
-    it 'reports unknown if version is unknown' do
-      expect(repository).to receive(:run).and_return("packet: git< version -1")
-      expect(repository.fetch_supported_git_protocol).to eq('unknown')
+    describe '#git_protocol=' do
+      [0, 1, 2].each do |version|
+        it "configures git to use protocol version #{version}" do
+          expect(repository).to receive(:run).with("git config protocol.version #{version}")
+          repository.git_protocol = version
+        end
+      end
+
+      it 'raises an error if the version is unsupported' do
+        expect { repository.git_protocol = 'foo' }.to raise_error(ArgumentError, "Please specify the protocol you would like to use: 0, 1, or 2")
+      end
     end
 
-    it 'reports unknown if content does not identify a version' do
-      expect(repository).to receive(:run).and_return("foo")
-      expect(repository.fetch_supported_git_protocol).to eq('unknown')
+    describe '#fetch_supported_git_protocol' do
+      it "reports the detected version" do
+        expect(repository).to receive(:run).and_return("packet: git< version 2")
+        expect(repository.fetch_supported_git_protocol).to eq('2')
+      end
+
+      it 'reports unknown if version is unknown' do
+        expect(repository).to receive(:run).and_return("packet: git< version -1")
+        expect(repository.fetch_supported_git_protocol).to eq('unknown')
+      end
+
+      it 'reports unknown if content does not identify a version' do
+        expect(repository).to receive(:run).and_return("foo")
+        expect(repository.fetch_supported_git_protocol).to eq('unknown')
+      end
     end
-  end
 
-  def cd_empty_temp_directory
-    tmp_dir = 'tmp/git-repository-spec/'
-    FileUtils.rm_rf(tmp_dir) if ::File.exist?(tmp_dir)
-    FileUtils.mkdir_p tmp_dir
-    FileUtils.cd tmp_dir
+    describe '#use_default_credentials' do
+      it 'adds credentials to .netrc' do
+        expect(File.read(File.join(tmp_netrc_dir, '.netrc')))
+          .to eq("machine foo login #{QA::Runtime::User.default_username} password #{QA::Runtime::User.default_password}\n")
+      end
+    end
   end
 
-  def set_bad_uri
-    repository.uri = 'http://foo/bar.git'
+  context 'with specific credentials' do
+    include_context 'git directory'
+
+    context 'before setting credentials' do
+      it 'does not add credentials to .netrc' do
+        expect(repository).not_to receive(:save_netrc_content)
+      end
+    end
+
+    describe '#password=' do
+      it 'raises an error if no username was given' do
+        expect { repository.password = 'foo' }
+          .to raise_error(QA::Git::Repository::InvalidCredentialsError,
+            "Please provide a username when setting a password")
+      end
+
+      it 'adds credentials to .netrc' do
+        repository.username = 'user'
+        repository.password = 'foo'
+
+        expect(File.read(File.join(tmp_netrc_dir, '.netrc')))
+          .to eq("machine foo login user password foo\n")
+      end
+    end
   end
 end