Merge branch 'remove-group-scoped-variables-feature-flag' into 'master'

Remove :scoped_group_variables feature flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!58200

app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue

@@ -130,9 +130,6 @@ export default {
 
       return true;
     },
-    scopedVariablesEnabled() {
-      return !this.isGroup || this.glFeatures.scopedGroupVariables;
-    },
     scopedVariablesAvailable() {
       return !this.isGroup || this.glFeatures.groupScopedCiVariables;
     },
@@ -230,17 +227,11 @@ export default {
       </gl-form-group>
 
       <div class="d-flex">
-        <gl-form-group
-          :label="__('Type')"
-          label-for="ci-variable-type"
-          class="w-50 gl-mr-5"
-          :class="{ 'w-100': !scopedVariablesEnabled }"
-        >
+        <gl-form-group :label="__('Type')" label-for="ci-variable-type" class="w-50 gl-mr-5">
           <gl-form-select id="ci-variable-type" v-model="variable_type" :options="typeOptions" />
         </gl-form-group>
 
         <gl-form-group
-          v-if="scopedVariablesEnabled"
           :label="__('Environment scope')"
           label-for="ci-variable-env"
           class="w-50"

app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue

@@ -62,7 +62,7 @@ export default {
   },
   mixins: [glFeatureFlagsMixin()],
   computed: {
-    ...mapState(['variables', 'valuesHidden', 'isGroup', 'isLoading', 'isDeleting']),
+    ...mapState(['variables', 'valuesHidden', 'isLoading', 'isDeleting']),
     valuesButtonText() {
       return this.valuesHidden ? __('Reveal values') : __('Hide values');
     },
@@ -70,9 +70,6 @@ export default {
       return this.variables && this.variables.length > 0;
     },
     fields() {
-      if (this.isGroup && !this.glFeatures.scopedGroupVariables) {
-        return this.$options.fields.filter((field) => field.key !== 'environment_scope');
-      }
       return this.$options.fields;
     },
   },

app/controllers/groups/settings/ci_cd_controller.rb

@@ -10,7 +10,6 @@ module Groups
       before_action :authorize_admin_group!
       before_action :authorize_update_max_artifacts_size!, only: [:update]
       before_action :define_variables, only: [:show]
-      before_action :push_feature_flags, only: [:show]
       before_action :push_licensed_features, only: [:show]
 
       feature_category :continuous_integration
@@ -94,10 +93,6 @@ module Groups
         params.require(:group).permit(:max_artifacts_size)
       end
 
-      def push_feature_flags
-        push_frontend_feature_flag(:scoped_group_variables, group)
-      end
-
       # Overridden in EE
       def push_licensed_features
       end

app/models/group.rb

@@ -824,13 +824,11 @@ class Group < Namespace
     variables = Ci::GroupVariable.where(group: list_of_ids)
     variables = variables.unprotected unless project.protected_for?(ref)
 
-    if Feature.enabled?(:scoped_group_variables, self, default_enabled: :yaml)
-      variables = if environment
-                    variables.on_environment(environment)
-                  else
-                    variables.where(environment_scope: '*')
-                  end
-    end
+    variables = if environment
+                  variables.on_environment(environment)
+                else
+                  variables.where(environment_scope: '*')
+                end
 
     variables = variables.group_by(&:group_id)
     list_of_ids.reverse.flat_map { |group| variables[group.id] }.compact

app/views/ci/group_variables/_index.html.haml

@@ -7,9 +7,8 @@
       %tr
         %td.gl-text-truncate
           = variable.key
-        - if Feature.enabled?(:scoped_group_variables, default_enabled: :yaml)
-          %td.gl-text-truncate
-            = variable.environment_scope
+        %td.gl-text-truncate
+          = variable.environment_scope
         %td.gl-text-truncate
           %a.group-origin-link{ href: group_settings_ci_cd_path(variable.group) }
             = variable.group.name

app/views/ci/group_variables/_variable_header.html.haml

 %tr
   %th
     = s_('Key')
-  - if Feature.enabled?(:scoped_group_variables, default_enabled: :yaml)
-    %th
-      = s_('Environments')
+  %th
+    = s_('Environments')
   %th
     = s_('Group')

config/feature_flags/development/scoped_group_variables.yml

----
-name: scoped_group_variables
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55256
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/323298
-milestone: '13.10'
-type: development
-group: group::configure
-default_enabled: false

doc/api/group_level_variables.md

@@ -31,14 +31,16 @@ curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/a
         "variable_type": "env_var",
         "value": "TEST_1",
         "protected": false,
-        "masked": false
+        "masked": false,
+        "environment_scope": "*"
     },
     {
         "key": "TEST_VARIABLE_2",
         "variable_type": "env_var",
         "value": "TEST_2",
         "protected": false,
-        "masked": false
+        "masked": false,
+        "environment_scope": "*"
     }
 ]
 ```
@@ -66,7 +68,8 @@ curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/a
     "variable_type": "env_var",
     "value": "TEST_1",
     "protected": false,
-    "masked": false
+    "masked": false,
+    "environment_scope": "*"
 }
 ```
 
@@ -86,6 +89,7 @@ POST /groups/:id/variables
 | `variable_type` | string  | no       | The type of a variable. Available types are: `env_var` (default) and `file` |
 | `protected`     | boolean | no       | Whether the variable is protected |
 | `masked`        | boolean | no       | Whether the variable is masked |
+| `environment_scope` **(PREMIUM)** | string | no | The [environment scope](../ci/variables/README.md#limit-the-environment-scope-of-a-cicd-variable) of a variable |
 
 ```shell
 curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/groups/1/variables" --form "key=NEW_VARIABLE" --form "value=new value"
@@ -97,7 +101,8 @@ curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" "https://gitla
     "value": "new value",
     "variable_type": "env_var",
     "protected": false,
-    "masked": false
+    "masked": false,
+    "environment_scope": "*"
 }
 ```
 
@@ -117,6 +122,7 @@ PUT /groups/:id/variables/:key
 | `variable_type` | string  | no       | The type of a variable. Available types are: `env_var` (default) and `file` |
 | `protected`     | boolean | no       | Whether the variable is protected |
 | `masked`        | boolean | no       | Whether the variable is masked |
+| `environment_scope` **(PREMIUM)** | string | no | The [environment scope](../ci/variables/README.md#limit-the-environment-scope-of-a-cicd-variable) of a variable |
 
 ```shell
 curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/groups/1/variables/NEW_VARIABLE" --form "value=updated value"
@@ -128,7 +134,8 @@ curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab
     "value": "updated value",
     "variable_type": "env_var",
     "protected": true,
-    "masked": true
+    "masked": true,
+    "environment_scope": "*"
 }
 ```
 

doc/ci/variables/README.md

@@ -163,6 +163,9 @@ The output is:
 
 ### Group CI/CD variables
 
+> - Introduced in GitLab 9.4.
+> - Support for [environment scopes](https://gitlab.com/gitlab-org/gitlab/-/issues/2874) added to GitLab Premium in 13.11
+
 To make a CI/CD variable available to all projects in a group, define a group CI/CD variable.
 
 Use group variables to store secrets like passwords, SSH keys, and credentials, if you:
@@ -178,6 +181,7 @@ To add a group variable:
    - **Key**: Must be one line, with no spaces, using only letters, numbers, or `_`.
    - **Value**: No limitations.
    - **Type**: [`File` or `Variable`](#cicd-variable-types).
+   - **Environment scope** (optional): `All`, or specific [environments](#limit-the-environment-scope-of-a-cicd-variable).
    - **Protect variable** (Optional): If selected, the variable is only available
      in pipelines that run on protected branches or tags.
    - **Mask variable** (Optional): If selected, the variable's **Value** is masked

ee/app/models/ee/group.rb

@@ -244,8 +244,7 @@ module EE
     end
 
     def scoped_variables_available?
-      ::Feature.enabled?(:scoped_group_variables, self, default_enabled: :yaml) &&
-        feature_available?(:group_scoped_ci_variables)
+      feature_available?(:group_scoped_ci_variables)
     end
 
     def actual_size_limit

ee/changelogs/unreleased/remove-group-scoped-variables-feature-flag.yml

+---
+title: Add environment scopes to group CI/CD variables
+merge_request: 58200
+author:
+type: added

ee/spec/models/ee/group_spec.rb

@@ -677,26 +677,24 @@ RSpec.describe Group do
   end
 
   describe '#scoped_variables_available?' do
-    using RSpec::Parameterized::TableSyntax
-
     let(:group) { create(:group) }
 
     subject { group.scoped_variables_available? }
 
-    where(:feature_enabled, :feature_available, :scoped_variables_available) do
-      true  | true  | true
-      false | true  | false
-      true  | false | false
-      false | false | false
+    before do
+      stub_licensed_features(group_scoped_ci_variables: feature_available)
     end
 
-    with_them do
-      before do
-        stub_feature_flags(scoped_group_variables: feature_enabled)
-        stub_licensed_features(group_scoped_ci_variables: feature_available)
-      end
+    context 'licensed feature is available' do
+      let(:feature_available) { true }
+
+      it { is_expected.to be true }
+    end
+
+    context 'licensed feature is not available' do
+      let(:feature_available) { false }
 
-      it { is_expected.to eq(scoped_variables_available) }
+      it { is_expected.to be false }
     end
   end
 

spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js

@@ -158,7 +158,6 @@ describe('Ci variable modal', () => {
           isGroup: true,
           provide: {
             glFeatures: {
-              scopedGroupVariables: true,
               groupScopedCiVariables: true,
             },
           },
@@ -168,29 +167,12 @@ describe('Ci variable modal', () => {
         expect(findCiEnvironmentsDropdown().isVisible()).toBe(true);
       });
 
-      describe('feature flag is disabled', () => {
-        it('hides the dropdown', () => {
-          createComponent(shallowMount, {
-            isGroup: true,
-            provide: {
-              glFeatures: {
-                scopedGroupVariables: false,
-                groupScopedCiVariables: true,
-              },
-            },
-          });
-
-          expect(findCiEnvironmentsDropdown().exists()).toBe(false);
-        });
-      });
-
       describe('licensed feature is not available', () => {
         it('disables the dropdown', () => {
           createComponent(mount, {
             isGroup: true,
             provide: {
               glFeatures: {
-                scopedGroupVariables: true,
                 groupScopedCiVariables: false,
               },
             },

spec/frontend/ci_variable_list/components/ci_variable_table_spec.js

@@ -11,18 +11,13 @@ describe('Ci variable table', () => {
   let wrapper;
   let store;
 
-  const createComponent = (isGroup = false, scopedGroupVariables = false) => {
-    store = createStore({ isGroup });
+  const createComponent = () => {
+    store = createStore();
     jest.spyOn(store, 'dispatch').mockImplementation();
     wrapper = mount(CiVariableTable, {
       attachTo: document.body,
       localVue,
       store,
-      provide: {
-        glFeatures: {
-          scopedGroupVariables,
-        },
-      },
     });
   };
 
@@ -42,19 +37,6 @@ describe('Ci variable table', () => {
     expect(store.dispatch).toHaveBeenCalledWith('fetchVariables');
   });
 
-  it('fields do not contain environment_scope if group level and feature is disabled', () => {
-    createComponent(true, false);
-
-    expect(wrapper.vm.fields).not.toEqual(
-      expect.arrayContaining([
-        expect.objectContaining({
-          key: 'environment_scope',
-          label: 'Environments',
-        }),
-      ]),
-    );
-  });
-
   describe('Renders correct data', () => {
     it('displays empty message when variables are not present', () => {
       expect(findEmptyVariablesPlaceholder().exists()).toBe(true);

spec/models/group_spec.rb

@@ -1755,30 +1755,6 @@ RSpec.describe Group do
              perfectly_matched_variable])
         end
       end
-
-      context 'when :scoped_group_variables feature flag is disabled' do
-        before do
-          stub_feature_flags(scoped_group_variables: false)
-        end
-
-        context 'when environment scope is exactly matched' do
-          let(:environment_scope) { 'review/name' }
-
-          it { is_expected.to contain_exactly(ci_variable) }
-        end
-
-        context 'when environment scope is partially matched' do
-          let(:environment_scope) { 'review/*' }
-
-          it { is_expected.to contain_exactly(ci_variable) }
-        end
-
-        context 'when environment scope does not match' do
-          let(:environment_scope) { 'review/*/special' }
-
-          it { is_expected.to contain_exactly(ci_variable) }
-        end
-      end
     end
 
     context 'when group has children' do