Merge branch 'master' into 38464-k8s-apps

12 files changed, 168 insertions, 61 deletions

diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 41ecdb604f1..5ed2e1ca99a 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1271,6 +1271,7 @@ describe Ci::Build do
         { key: 'CI_PROJECT_PATH_SLUG', value: project.full_path_slug, public: true },
         { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true },
         { key: 'CI_PROJECT_URL', value: project.web_url, public: true },
+        { key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true },
         { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true },
         { key: 'CI_CONFIG_PATH', value: pipeline.ci_yaml_file_path, public: true },
         { key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true },
diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb
index e6ebe079ceb..ed76be703a5 100644
--- a/spec/models/clusters/platforms/kubernetes_spec.rb
+++ b/spec/models/clusters/platforms/kubernetes_spec.rb
@@ -9,7 +9,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
 
   describe 'before_validation' do
     context 'when namespace includes upper case' do
-      let(:kubernetes) { create(:platform_kubernetes, :configured, namespace: namespace) }
+      let(:kubernetes) { create(:cluster_platform_kubernetes, :configured, namespace: namespace) }
       let(:namespace) { 'ABC' }
 
       it 'converts to lower case' do
@@ -22,7 +22,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
     subject { kubernetes.valid? }
 
     context 'when validates namespace' do
-      let(:kubernetes) { build(:platform_kubernetes, :configured, namespace: namespace) }
+      let(:kubernetes) { build(:cluster_platform_kubernetes, :configured, namespace: namespace) }
 
       context 'when namespace is blank' do
         let(:namespace) { '' }
@@ -50,7 +50,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
     end
 
     context 'when validates api_url' do
-      let(:kubernetes) { build(:platform_kubernetes, :configured) }
+      let(:kubernetes) { build(:cluster_platform_kubernetes, :configured) }
 
       before do
         kubernetes.api_url = api_url
@@ -76,7 +76,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
     end
 
     context 'when validates token' do
-      let(:kubernetes) { build(:platform_kubernetes, :configured) }
+      let(:kubernetes) { build(:cluster_platform_kubernetes, :configured) }
 
       before do
         kubernetes.token = token
@@ -95,8 +95,8 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
       let(:enabled) { true }
       let(:project) { create(:project) }
       let(:cluster) { build(:cluster, provider_type: :gcp, platform_type: :kubernetes, platform_kubernetes: platform, provider_gcp: provider, enabled: enabled, projects: [project]) }
-      let(:platform) { build(:platform_kubernetes, :configured) }
-      let(:provider) { build(:provider_gcp) }
+      let(:platform) { build(:cluster_platform_kubernetes, :configured) }
+      let(:provider) { build(:cluster_provider_gcp) }
       let(:kubernetes_service) { project.kubernetes_service }
 
       it 'updates KubernetesService' do
@@ -126,8 +126,8 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
     context 'when kubernetes_service has been configured without cluster integration' do
       let!(:project) { create(:project) }
       let(:cluster) { build(:cluster, provider_type: :gcp, platform_type: :kubernetes, platform_kubernetes: platform, provider_gcp: provider, projects: [project]) }
-      let(:platform) { build(:platform_kubernetes, :configured, api_url: 'https://111.111.111.111') }
-      let(:provider) { build(:provider_gcp) }
+      let(:platform) { build(:cluster_platform_kubernetes, :configured, api_url: 'https://111.111.111.111') }
+      let(:provider) { build(:cluster_provider_gcp) }
 
       before do
         create(:kubernetes_service, project: project)
@@ -144,7 +144,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
 
     let!(:cluster) { create(:cluster, :project, platform_kubernetes: kubernetes) }
     let(:project) { cluster.project }
-    let(:kubernetes) { create(:platform_kubernetes, :configured, namespace: namespace) }
+    let(:kubernetes) { create(:cluster_platform_kubernetes, :configured, namespace: namespace) }
 
     context 'when namespace is present' do
       let(:namespace) { 'namespace-123' }
@@ -170,7 +170,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
   describe '#default_namespace' do
     subject { kubernetes.default_namespace }
 
-    let(:kubernetes) { create(:platform_kubernetes, :configured) }
+    let(:kubernetes) { create(:cluster_platform_kubernetes, :configured) }
 
     context 'when cluster belongs to a project' do
       let!(:cluster) { create(:cluster, :project, platform_kubernetes: kubernetes) }
diff --git a/spec/models/clusters/providers/gcp_spec.rb b/spec/models/clusters/providers/gcp_spec.rb
index 99eb8c46e9a..ecd0a08c953 100644
--- a/spec/models/clusters/providers/gcp_spec.rb
+++ b/spec/models/clusters/providers/gcp_spec.rb
@@ -5,7 +5,7 @@ describe Clusters::Providers::Gcp do
   it { is_expected.to validate_presence_of(:zone) }
 
   describe 'default_value_for' do
-    let(:gcp) { build(:provider_gcp) }
+    let(:gcp) { build(:cluster_provider_gcp) }
 
     it "has default value" do
       expect(gcp.zone).to eq('us-central1-a')
@@ -18,7 +18,7 @@ describe Clusters::Providers::Gcp do
     subject { gcp.valid? }
 
     context 'when validates gcp_project_id' do
-      let(:gcp) { build(:provider_gcp, gcp_project_id: gcp_project_id) }
+      let(:gcp) { build(:cluster_provider_gcp, gcp_project_id: gcp_project_id) }
 
       context 'when gcp_project_id is shorter than 1' do
         let(:gcp_project_id) { '' }
@@ -46,7 +46,7 @@ describe Clusters::Providers::Gcp do
     end
 
     context 'when validates num_nodes' do
-      let(:gcp) { build(:provider_gcp, num_nodes: num_nodes) }
+      let(:gcp) { build(:cluster_provider_gcp, num_nodes: num_nodes) }
 
       context 'when num_nodes is string' do
         let(:num_nodes) { 'A3' }
@@ -76,7 +76,7 @@ describe Clusters::Providers::Gcp do
 
   describe '#state_machine' do
     context 'when any => [:created]' do
-      let(:gcp) { build(:provider_gcp, :creating) }
+      let(:gcp) { build(:cluster_provider_gcp, :creating) }
 
       before do
         gcp.make_created
@@ -90,7 +90,7 @@ describe Clusters::Providers::Gcp do
     end
 
     context 'when any => [:creating]' do
-      let(:gcp) { build(:provider_gcp) }
+      let(:gcp) { build(:cluster_provider_gcp) }
 
       context 'when operation_id is present' do
         let(:operation_id) { 'operation-xxx' }
@@ -116,7 +116,7 @@ describe Clusters::Providers::Gcp do
     end
 
     context 'when any => [:errored]' do
-      let(:gcp) { build(:provider_gcp, :creating) }
+      let(:gcp) { build(:cluster_provider_gcp, :creating) }
       let(:status_reason) { 'err msg' }
 
       it 'nullify access_token and operation_id' do
@@ -129,7 +129,7 @@ describe Clusters::Providers::Gcp do
       end
 
       context 'when status_reason is nil' do
-        let(:gcp) { build(:provider_gcp, :errored) }
+        let(:gcp) { build(:cluster_provider_gcp, :errored) }
 
         it 'does not set status_reason' do
           gcp.make_errored(nil)
@@ -144,13 +144,13 @@ describe Clusters::Providers::Gcp do
     subject { gcp.on_creation? }
 
     context 'when status is creating' do
-      let(:gcp) { create(:provider_gcp, :creating) }
+      let(:gcp) { create(:cluster_provider_gcp, :creating) }
 
       it { is_expected.to be_truthy }
     end
 
     context 'when status is created' do
-      let(:gcp) { create(:provider_gcp, :created) }
+      let(:gcp) { create(:cluster_provider_gcp, :created) }
 
       it { is_expected.to be_falsey }
     end
@@ -160,7 +160,7 @@ describe Clusters::Providers::Gcp do
     subject { gcp.api_client }
 
     context 'when status is creating' do
-      let(:gcp) { build(:provider_gcp, :creating) }
+      let(:gcp) { build(:cluster_provider_gcp, :creating) }
 
       it 'returns Cloud Platform API clinet' do
         expect(subject).to be_an_instance_of(GoogleApi::CloudPlatform::Client)
@@ -169,13 +169,13 @@ describe Clusters::Providers::Gcp do
     end
 
     context 'when status is created' do
-      let(:gcp) { build(:provider_gcp, :created) }
+      let(:gcp) { build(:cluster_provider_gcp, :created) }
 
       it { is_expected.to be_nil }
     end
 
     context 'when status is errored' do
-      let(:gcp) { build(:provider_gcp, :errored) }
+      let(:gcp) { build(:cluster_provider_gcp, :errored) }
 
       it { is_expected.to be_nil }
     end
diff --git a/spec/models/concerns/ignorable_column_spec.rb b/spec/models/concerns/ignorable_column_spec.rb
index dba9fe43327..b70f2331a0e 100644
--- a/spec/models/concerns/ignorable_column_spec.rb
+++ b/spec/models/concerns/ignorable_column_spec.rb
@@ -5,7 +5,11 @@ describe IgnorableColumn do
     Class.new do
       def self.columns
         # This method does not have access to "double"
-        [Struct.new(:name).new('id'), Struct.new(:name).new('title')]
+        [
+          Struct.new(:name).new('id'),
+          Struct.new(:name).new('title'),
+          Struct.new(:name).new('date')
+        ]
       end
     end
   end
@@ -18,7 +22,7 @@ describe IgnorableColumn do
 
   describe '.columns' do
     it 'returns the columns, excluding the ignored ones' do
-      model.ignore_column(:title)
+      model.ignore_column(:title, :date)
 
       expect(model.columns.map(&:name)).to eq(%w(id))
     end
@@ -30,9 +34,9 @@ describe IgnorableColumn do
     end
 
     it 'returns the names of the ignored columns' do
-      model.ignore_column(:title)
+      model.ignore_column(:title, :date)
 
-      expect(model.ignored_columns).to eq(Set.new(%w(title)))
+      expect(model.ignored_columns).to eq(Set.new(%w(title date)))
     end
   end
 end
diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb
index ab8773b7ede..3106207811a 100644
--- a/spec/models/concerns/routable_spec.rb
+++ b/spec/models/concerns/routable_spec.rb
@@ -134,6 +134,7 @@ describe Group, 'Routable' do
 
     context 'with RequestStore active', :request_store do
       it 'does not load the route table more than once' do
+        group.expires_full_path_cache
         expect(group).to receive(:uncached_full_path).once.and_call_original
 
         3.times { group.full_path }
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index f75de0a0d88..1ce1d595c60 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -18,7 +18,6 @@ describe Environment do
   it { is_expected.to validate_length_of(:slug).is_at_most(24) }
 
   it { is_expected.to validate_length_of(:external_url).is_at_most(255) }
-  it { is_expected.to validate_uniqueness_of(:external_url).scoped_to(:project_id) }
 
   describe '.order_by_last_deployed_at' do
     let(:project) { create(:project, :repository) }
diff --git a/spec/models/fork_network_spec.rb b/spec/models/fork_network_spec.rb
index 605ccd6db06..a43baf1820a 100644
--- a/spec/models/fork_network_spec.rb
+++ b/spec/models/fork_network_spec.rb
@@ -24,6 +24,16 @@ describe ForkNetwork do
     end
   end
 
+  describe '#merge_requests' do
+    it 'finds merge requests within the fork network' do
+      project = create(:project)
+      forked_project = fork_project(project)
+      merge_request = create(:merge_request, source_project: forked_project, target_project: project)
+
+      expect(project.fork_network.merge_requests).to include(merge_request)
+    end
+  end
+
   context 'for a deleted project' do
     it 'keeps the fork network' do
       project = create(:project, :public)
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index f36d6eeb327..0e1a7fdce0b 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -488,6 +488,47 @@ describe Group do
     end
   end
 
+  describe '#path_changed_hook' do
+    let(:system_hook_service) { SystemHooksService.new }
+
+    context 'for a new group' do
+      let(:group) { build(:group) }
+
+      before do
+        expect(group).to receive(:system_hook_service).and_return(system_hook_service)
+      end
+
+      it 'does not trigger system hook' do
+        expect(system_hook_service).to receive(:execute_hooks_for).with(group, :create)
+
+        group.save!
+      end
+    end
+
+    context 'for an existing group' do
+      let(:group) { create(:group, path: 'old-path') }
+
+      context 'when the path is changed' do
+        let(:new_path) { 'very-new-path' }
+
+        it 'triggers the rename system hook' do
+          expect(group).to receive(:system_hook_service).and_return(system_hook_service)
+          expect(system_hook_service).to receive(:execute_hooks_for).with(group, :rename)
+
+          group.update_attributes!(path: new_path)
+        end
+      end
+
+      context 'when the path is not changed' do
+        it 'does not trigger system hook' do
+          expect(group).not_to receive(:system_hook_service)
+
+          group.update_attributes!(name: 'new name')
+        end
+      end
+    end
+  end
+
   describe '#secret_variables_for' do
     let(:project) { create(:project, group: group) }
 
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 476a2697605..d022dae3476 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1755,39 +1755,12 @@ describe MergeRequest do
     end
   end
 
-  describe '#fetch_ref' do
-    it 'sets "ref_fetched" flag to true' do
-      subject.update!(ref_fetched: nil)
+  describe '#fetch_ref!' do
+    it 'fetches the ref correctly' do
+      expect { subject.target_project.repository.delete_refs(subject.ref_path) }.not_to raise_error
 
-      subject.fetch_ref
-
-      expect(subject.reload.ref_fetched).to be_truthy
-    end
-  end
-
-  describe '#ref_fetched?' do
-    it 'does not perform git operation when value is cached' do
-      subject.ref_fetched = true
-
-      expect_any_instance_of(Repository).not_to receive(:ref_exists?)
-      expect(subject.ref_fetched?).to be_truthy
-    end
-
-    it 'caches the value when ref exists but value is not cached' do
-      subject.update!(ref_fetched: nil)
-      allow_any_instance_of(Repository).to receive(:ref_exists?)
-        .and_return(true)
-
-      expect(subject.ref_fetched?).to be_truthy
-      expect(subject.reload.ref_fetched).to be_truthy
-    end
-
-    it 'returns false when ref does not exist' do
-      subject.update!(ref_fetched: nil)
-      allow_any_instance_of(Repository).to receive(:ref_exists?)
-        .and_return(false)
-
-      expect(subject.ref_fetched?).to be_falsey
+      subject.fetch_ref!
+      expect(subject.target_project.repository.ref_exists?(subject.ref_path)).to be_truthy
     end
   end
 
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index ed6e42d476e..0e50909988b 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -276,6 +276,12 @@ describe Project do
 
         expect(project).to be_valid
       end
+
+      it 'allows a path ending in a period' do
+        project = build(:project, path: 'foo.')
+
+        expect(project).to be_valid
+      end
     end
   end
 
@@ -1923,6 +1929,20 @@ describe Project do
         expect(forked_project.in_fork_network_of?(other_project)).to be_falsy
       end
     end
+
+    describe '#fork_source' do
+      let!(:second_fork) { fork_project(forked_project) }
+
+      it 'returns the direct source if it exists' do
+        expect(second_fork.fork_source).to eq(forked_project)
+      end
+
+      it 'returns the root of the fork network when the directs source was deleted' do
+        forked_project.destroy
+
+        expect(second_fork.fork_source).to eq(project)
+      end
+    end
   end
 
   describe '#pushes_since_gc' do
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index d7c07676911..8a6aa767ce6 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -2298,4 +2298,24 @@ describe Repository do
       project.commit_by(oid: '1' * 40)
     end
   end
+
+  describe '#raw_repository' do
+    subject { repository.raw_repository }
+
+    it 'returns a Gitlab::Git::Repository representation of the repository' do
+      expect(subject).to be_a(Gitlab::Git::Repository)
+      expect(subject.relative_path).to eq(project.disk_path + '.git')
+      expect(subject.gl_repository).to eq("project-#{project.id}")
+    end
+
+    context 'with a wiki repository' do
+      let(:repository) { project.wiki.repository }
+
+      it 'creates a Gitlab::Git::Repository with the proper attributes' do
+        expect(subject).to be_a(Gitlab::Git::Repository)
+        expect(subject.relative_path).to eq(project.disk_path + '.wiki.git')
+        expect(subject.gl_repository).to eq("wiki-#{project.id}")
+      end
+    end
+  end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index fb03e320734..d2f97009ad9 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -788,14 +788,16 @@ describe User do
       end
 
       it "creates external user by default" do
-        user = build(:user)
+        user = create(:user)
 
         expect(user.external).to be_truthy
+        expect(user.can_create_group).to be_falsey
+        expect(user.projects_limit).to be 0
       end
 
       describe 'with default overrides' do
         it "creates a non-external user" do
-          user = build(:user, external: false)
+          user = create(:user, external: false)
 
           expect(user.external).to be_falsey
         end
@@ -2217,6 +2219,42 @@ describe User do
     end
   end
 
+  describe '#username_changed_hook' do
+    context 'for a new user' do
+      let(:user) { build(:user) }
+
+      it 'does not trigger system hook' do
+        expect(user).not_to receive(:system_hook_service)
+
+        user.save!
+      end
+    end
+
+    context 'for an existing user' do
+      let(:user) { create(:user, username: 'old-username') }
+
+      context 'when the username is changed' do
+        let(:new_username) { 'very-new-name' }
+
+        it 'triggers the rename system hook' do
+          system_hook_service = SystemHooksService.new
+          expect(system_hook_service).to receive(:execute_hooks_for).with(user, :rename)
+          expect(user).to receive(:system_hook_service).and_return(system_hook_service)
+
+          user.update_attributes!(username: new_username)
+        end
+      end
+
+      context 'when the username is not changed' do
+        it 'does not trigger system hook' do
+          expect(user).not_to receive(:system_hook_service)
+
+          user.update_attributes!(email: 'asdf@asdf.com')
+        end
+      end
+    end
+  end
+
   describe '#sync_attribute?' do
     let(:user) { described_class.new }