Merge branch 'ee-5320-fix-gitlab-features-env-var' into 'master'

Get GITLAB_FEATURES from Project#licensed_features instead of Namespace#features (#5320)

See merge request gitlab-org/gitlab-ee!5242

app/models/ci/build.rb

@@ -615,7 +615,7 @@ module Ci
       Gitlab::Ci::Variables::Collection.new.tap do |variables|
         variables.append(key: 'CI', value: 'true')
         variables.append(key: 'GITLAB_CI', value: 'true')
-        variables.append(key: 'GITLAB_FEATURES', value: project.namespace.features.join(','))
+        variables.append(key: 'GITLAB_FEATURES', value: project.licensed_features.join(','))
         variables.append(key: 'CI_SERVER_NAME', value: 'GitLab')
         variables.append(key: 'CI_SERVER_VERSION', value: Gitlab::VERSION)
         variables.append(key: 'CI_SERVER_REVISION', value: Gitlab::REVISION)

app/models/namespace.rb

@@ -258,10 +258,6 @@ class Namespace < ActiveRecord::Base
     all_projects.with_storage_feature(:repository).find_each(&:remove_exports)
   end
 
-  def features
-    []
-  end
-
   def refresh_project_authorizations
     owner.refresh_authorized_projects
   end

app/models/project.rb

@@ -1885,6 +1885,10 @@ class Project < ActiveRecord::Base
     memoized_results[cache_key]
   end
 
+  def licensed_features
+    []
+  end
+
   private
 
   def storage

ee/app/models/ee/namespace.rb

@@ -64,15 +64,6 @@ module EE
       succeeded
     end
 
-    override :features
-    def features
-      return super unless License.current
-
-      License.current.features.select do |feature|
-        License.global_feature?(feature) || feature_available?(feature)
-      end
-    end
-
     # Checks features (i.e. https://about.gitlab.com/products/) availabily
     # for a given Namespace plan. This method should consider ancestor groups
     # being licensed.

ee/app/models/ee/project.rb

@@ -495,6 +495,15 @@ module EE
                  .external_authorization_service_default_label
     end
 
+    override :licensed_features
+    def licensed_features
+      return super unless License.current
+
+      License.current.features.select do |feature|
+        License.global_feature?(feature) || licensed_feature_available?(feature)
+      end
+    end
+
     private
 
     def set_override_pull_mirror_available

ee/changelogs/unreleased/ee-5320-fix-gitlab-features-env-var.yml

+---
+title: Fix GITLAB_FEATURES CI/CD env var for public projects
+merge_request: 5242
+author:
+type: fixed

ee/spec/models/namespace_spec.rb

@@ -122,76 +122,6 @@ describe Namespace do
     end
   end
 
-  describe '#features' do
-    let(:plan_license) { :free_plan }
-    let(:group) { create(:group, plan: plan_license) }
-    let(:global_license) { create(:license) }
-
-    before do
-      allow(License).to receive(:current).and_return(global_license)
-      allow(global_license).to receive(:features).and_return([
-        :epics, # Gold only
-        :service_desk, # Silver and up
-        :audit_events, # Bronze and up
-        :geo, # Global feature, should not be checked at namespace level
-      ])
-    end
-
-    subject { group.features }
-
-    context 'when the namespace should be checked' do
-      before do
-        enable_namespace_license_check!
-      end
-
-      context 'when bronze' do
-        let(:plan_license) { :bronze_plan }
-
-        it 'filters for bronze features' do
-          is_expected.to contain_exactly(:audit_events, :geo)
-        end
-      end
-
-      context 'when silver' do
-        let(:plan_license) { :silver_plan }
-
-        it 'filters for silver features' do
-          is_expected.to contain_exactly(:service_desk, :audit_events, :geo)
-        end
-      end
-
-      context 'when gold' do
-        let(:plan_license) { :gold_plan }
-
-        it 'filters for gold features' do
-          is_expected.to contain_exactly(:epics, :service_desk, :audit_events, :geo)
-        end
-      end
-
-      context 'when free plan' do
-        let(:plan_license) { :free_plan }
-
-        it 'filters out paid features' do
-          is_expected.to contain_exactly(:geo)
-        end
-      end
-    end
-
-    context 'when namespace should not be checked' do
-      it 'includes all features in global license' do
-        is_expected.to contain_exactly(:epics, :service_desk, :audit_events, :geo)
-      end
-    end
-
-    context 'when there is no license' do
-      before do
-        allow(License).to receive(:current).and_return(nil)
-      end
-
-      it { is_expected.to be_empty }
-    end
-  end
-
   describe '#feature_available?' do
     let(:plan_license) { :bronze_plan }
     let(:group) { create(:group, plan: plan_license) }

ee/spec/models/project_spec.rb

@@ -1364,4 +1364,84 @@ describe Project do
       end
     end
   end
+
+  describe '#licensed_features' do
+    let(:plan_license) { :free_plan }
+    let(:global_license) { create(:license) }
+    let(:group) { create(:group, plan: plan_license) }
+    let(:project) { create(:project, group: group) }
+
+    before do
+      allow(License).to receive(:current).and_return(global_license)
+      allow(global_license).to receive(:features).and_return([
+        :epics, # Gold only
+        :service_desk, # Silver and up
+        :audit_events, # Bronze and up
+        :geo, # Global feature, should not be checked at namespace level
+      ])
+    end
+
+    subject { project.licensed_features }
+
+    context 'when the namespace should be checked' do
+      before do
+        enable_namespace_license_check!
+      end
+
+      context 'when bronze' do
+        let(:plan_license) { :bronze_plan }
+
+        it 'filters for bronze features' do
+          is_expected.to contain_exactly(:audit_events, :geo)
+        end
+      end
+
+      context 'when silver' do
+        let(:plan_license) { :silver_plan }
+
+        it 'filters for silver features' do
+          is_expected.to contain_exactly(:service_desk, :audit_events, :geo)
+        end
+      end
+
+      context 'when gold' do
+        let(:plan_license) { :gold_plan }
+
+        it 'filters for gold features' do
+          is_expected.to contain_exactly(:epics, :service_desk, :audit_events, :geo)
+        end
+      end
+
+      context 'when free plan' do
+        let(:plan_license) { :free_plan }
+
+        it 'filters out paid features' do
+          is_expected.to contain_exactly(:geo)
+        end
+
+        context 'when public project and namespace' do
+          let(:group) { create(:group, :public, plan: plan_license) }
+          let(:project) { create(:project, :public, group: group) }
+
+          it 'includes all features in global license' do
+            is_expected.to contain_exactly(:epics, :service_desk, :audit_events, :geo)
+          end
+        end
+      end
+    end
+
+    context 'when namespace should not be checked' do
+      it 'includes all features in global license' do
+        is_expected.to contain_exactly(:epics, :service_desk, :audit_events, :geo)
+      end
+    end
+
+    context 'when there is no license' do
+      before do
+        allow(License).to receive(:current).and_return(nil)
+      end
+
+      it { is_expected.to be_empty }
+    end
+  end
 end

spec/models/ci/build_spec.rb

@@ -1492,7 +1492,7 @@ describe Ci::Build do
         { key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false },
         { key: 'CI', value: 'true', public: true },
         { key: 'GITLAB_CI', value: 'true', public: true },
-        { key: 'GITLAB_FEATURES', value: project.namespace.features.join(','), public: true },
+        { key: 'GITLAB_FEATURES', value: project.licensed_features.join(','), public: true },
         { key: 'CI_SERVER_NAME', value: 'GitLab', public: true },
         { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true },
         { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true },