From be9aadb8d000376c6bd142baaddf1753e1d349eb Mon Sep 17 00:00:00 2001
From: Dmytro Biryukov <dbiryukov@gitlab.com>
Date: Wed, 11 Sep 2024 17:51:57 +0200
Subject: [PATCH 1/4] Remove ci_hidden_variables ff

Changelog: changed
---
 .../groups/settings/ci_cd_controller.rb       |  1 -
 .../projects/settings/ci_cd_controller.rb     |  1 -
 app/models/ci/variable_value.rb               | 15 -------
 app/models/concerns/ci/hidable_variable.rb    | 15 -------
 app/views/ci/variables/_attributes.html.haml  |  2 +-
 .../development/ci_hidden_variables.yml       |  8 ----
 spec/models/ci/variable_value_spec.rb         | 36 ---------------
 .../concerns/ci/hidable_variable_spec.rb      | 44 -------------------
 spec/requests/api/ci/variables_spec.rb        | 38 ----------------
 .../api/graphql/ci/group_variables_spec.rb    | 33 --------------
 .../api/graphql/ci/project_variables_spec.rb  | 35 ---------------
 spec/requests/api/group_variables_spec.rb     | 36 ---------------
 12 files changed, 1 insertion(+), 263 deletions(-)
 delete mode 100644 config/feature_flags/development/ci_hidden_variables.yml

diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb
index bef7fd7060f2a8..216c1b615cfe38 100644
--- a/app/controllers/groups/settings/ci_cd_controller.rb
+++ b/app/controllers/groups/settings/ci_cd_controller.rb
@@ -16,7 +16,6 @@ class CiCdController < Groups::ApplicationController
 
       before_action do
         push_frontend_feature_flag(:ci_variables_pages, current_user)
-        push_frontend_feature_flag(:ci_hidden_variables, group)
       end
 
       urgency :low
diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb
index c859677363934d..71460a17a7540c 100644
--- a/app/controllers/projects/settings/ci_cd_controller.rb
+++ b/app/controllers/projects/settings/ci_cd_controller.rb
@@ -17,7 +17,6 @@ class CiCdController < Projects::ApplicationController
       before_action do
         push_frontend_feature_flag(:ci_variables_pages, current_user)
         push_frontend_feature_flag(:allow_push_repository_for_job_token, @project)
-        push_frontend_feature_flag(:ci_hidden_variables, @project.root_ancestor)
 
         push_frontend_ability(ability: :admin_project, resource: @project, user: current_user)
       end
diff --git a/app/models/ci/variable_value.rb b/app/models/ci/variable_value.rb
index e73fb42d49c902..fc80e70260d934 100644
--- a/app/models/ci/variable_value.rb
+++ b/app/models/ci/variable_value.rb
@@ -9,26 +9,11 @@ def initialize(variable)
     end
 
     def evaluate
-      return variable.value if hidden_variables_feature_flag_is_disabled?
-
       variable.hidden? ? nil : variable.value
     end
 
     private
 
-    # This logic will go away on the ff `ci_hidden_variables` deprecation
-    def hidden_variables_feature_flag_is_disabled?
-      parent = if variable.is_a?(Ci::Variable)
-                 variable.project&.root_ancestor
-               elsif variable.is_a?(Ci::GroupVariable)
-                 variable.group
-               end
-
-      return true unless parent
-
-      ::Feature.disabled?(:ci_hidden_variables, parent)
-    end
-
     attr_reader :variable
   end
 end
diff --git a/app/models/concerns/ci/hidable_variable.rb b/app/models/concerns/ci/hidable_variable.rb
index 0810f4b7ab6653..feb817969251f4 100644
--- a/app/models/concerns/ci/hidable_variable.rb
+++ b/app/models/concerns/ci/hidable_variable.rb
@@ -13,14 +13,12 @@ module HidableVariable
     private
 
     def validate_masked_and_hidden_on_create
-      return if hidden_variables_feature_flag_is_disabled?
       return unless masked == false && hidden == true
 
       errors.add(:masked, 'should be true when variable is hidden')
     end
 
     def validate_masked_and_hidden_on_update
-      return if hidden_variables_feature_flag_is_disabled?
       return if !masked_changed? && !hidden_changed?
       return if hidden == false && !hidden_changed?
 
@@ -30,18 +28,5 @@ def validate_masked_and_hidden_on_update
         errors.add(:base, 'Updating masked attribute is not allowed on updates for hidden variables.')
       end
     end
-
-    # This logic will go away on the ff `ci_hidden_variables` deprecation
-    def hidden_variables_feature_flag_is_disabled?
-      parent = if is_a?(Ci::Variable)
-                 project&.root_ancestor
-               elsif is_a?(Ci::GroupVariable)
-                 group
-               end
-
-      return true unless parent
-
-      ::Feature.disabled?(:ci_hidden_variables, parent)
-    end
   end
 end
diff --git a/app/views/ci/variables/_attributes.html.haml b/app/views/ci/variables/_attributes.html.haml
index 40521f82489a58..a418f58217209a 100644
--- a/app/views/ci/variables/_attributes.html.haml
+++ b/app/views/ci/variables/_attributes.html.haml
@@ -1,4 +1,4 @@
-- hidden_variables_enabled = (@group.present? && Feature.enabled?(:ci_hidden_variables, @group)) || (@project.present? && Feature.enabled?(:ci_hidden_variables, @project.group))
+- hidden_variables_enabled = @group.present? || @project.present?
 
 %p
   = s_('CiVariables|Variables can be accidentally exposed in a job log, or maliciously sent to a third party server. The masked variable feature can help reduce the risk of accidentally exposing variable values, but is not a guaranteed method to prevent malicious users from accessing variables.')
diff --git a/config/feature_flags/development/ci_hidden_variables.yml b/config/feature_flags/development/ci_hidden_variables.yml
deleted file mode 100644
index 35b82f6d00e946..00000000000000
--- a/config/feature_flags/development/ci_hidden_variables.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: ci_hidden_variables
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/141926
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/451326
-milestone: '16.11'
-type: development
-group: group::pipeline security
-default_enabled: true
diff --git a/spec/models/ci/variable_value_spec.rb b/spec/models/ci/variable_value_spec.rb
index d80ac305f6bb53..29b9135c26958b 100644
--- a/spec/models/ci/variable_value_spec.rb
+++ b/spec/models/ci/variable_value_spec.rb
@@ -35,24 +35,6 @@
 
         it_behaves_like 'hidden variable'
       end
-
-      context 'when feature flag `ci_hidden_variables` is disabled' do
-        before do
-          stub_feature_flags(ci_hidden_variables: false)
-        end
-
-        context 'and it is not hidden' do
-          let(:is_hidden) { false }
-
-          it_behaves_like 'not hidden variable'
-        end
-
-        context 'and it is hidden' do
-          let(:is_hidden) { true }
-
-          it_behaves_like 'not hidden variable'
-        end
-      end
     end
 
     context 'when variable is a group variable' do
@@ -70,24 +52,6 @@
 
         it_behaves_like 'hidden variable'
       end
-
-      context 'when feature flag `ci_hidden_variables` is disabled' do
-        before do
-          stub_feature_flags(ci_hidden_variables: false)
-        end
-
-        context 'and it is not hidden' do
-          let(:is_hidden) { false }
-
-          it_behaves_like 'not hidden variable'
-        end
-
-        context 'and it is hidden' do
-          let(:is_hidden) { true }
-
-          it_behaves_like 'not hidden variable'
-        end
-      end
     end
   end
 end
diff --git a/spec/models/concerns/ci/hidable_variable_spec.rb b/spec/models/concerns/ci/hidable_variable_spec.rb
index 6e39714fa01b3f..a431e6b5e72316 100644
--- a/spec/models/concerns/ci/hidable_variable_spec.rb
+++ b/spec/models/concerns/ci/hidable_variable_spec.rb
@@ -18,25 +18,6 @@
     describe '#validate_masked_and_hidden_on_create' do
       subject(:validate_create) { create_variable }
 
-      context 'when feature flag `ci_hidden_variables` is disabled' do
-        before do
-          stub_feature_flags(ci_hidden_variables: false)
-        end
-
-        where(:pending_masked, :pending_hidden) do
-          true | true
-          false | false
-          true | false
-          false | true
-        end
-
-        with_them do
-          it 'passes the validation' do
-            expect { validate_create }.not_to raise_error
-          end
-        end
-      end
-
       context 'when masked and hidden attribute are allowed' do
         where(:pending_masked, :pending_hidden) do
           true | true
@@ -79,31 +60,6 @@
         test_variable.update!(masked: pending_masked, hidden: pending_hidden)
       end
 
-      context 'when feature flag `ci_hidden_variables` is disabled' do
-        before do
-          stub_feature_flags(ci_hidden_variables: false)
-        end
-
-        where(:stored_masked, :stored_hidden, :pending_masked,
-          :pending_hidden) do
-          true | false | true | false
-          true | false | false | false
-          true | true | true | true
-          false | false | true | false
-          true | true | true | false
-          true | true | false | false
-          false | false | true | true
-        end
-
-        with_them do
-          it 'passed the validation' do
-            expect do
-              validate_update
-            end.not_to raise_error
-          end
-        end
-      end
-
       context 'when update is allowed' do
         where(:stored_masked, :stored_hidden, :pending_masked,
           :pending_hidden) do
diff --git a/spec/requests/api/ci/variables_spec.rb b/spec/requests/api/ci/variables_spec.rb
index d4d4b7085885ad..8d64b7321c0f51 100644
--- a/spec/requests/api/ci/variables_spec.rb
+++ b/spec/requests/api/ci/variables_spec.rb
@@ -56,25 +56,6 @@
           expect(json_response['description']).to be_nil
           expect(json_response['hidden']).to eq(true)
         end
-
-        context 'and feature flag `ci_hidden_variables is disabled`' do
-          before do
-            stub_feature_flags(ci_hidden_variables: false)
-          end
-
-          it 'returns project variable details without changes' do
-            get api("/projects/#{project.id}/variables/#{variable.key}", user)
-
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(json_response['value']).to eq(variable.value)
-            expect(json_response['protected']).to eq(variable.protected?)
-            expect(json_response['masked']).to eq(variable.masked?)
-            expect(json_response['raw']).to eq(variable.raw?)
-            expect(json_response['variable_type']).to eq('env_var')
-            expect(json_response['description']).to be_nil
-            expect(json_response['hidden']).to eq(true)
-          end
-        end
       end
 
       context 'when variable is not hidden' do
@@ -93,25 +74,6 @@
           expect(json_response['description']).to be_nil
           expect(json_response['hidden']).to eq(false)
         end
-
-        context 'and feature flag `ci_hidden_variables is disabled`' do
-          before do
-            stub_feature_flags(ci_hidden_variables: false)
-          end
-
-          it 'returns project variable details' do
-            get api("/projects/#{project.id}/variables/#{variable.key}", user)
-
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(json_response['value']).to eq(variable.value)
-            expect(json_response['protected']).to eq(variable.protected?)
-            expect(json_response['masked']).to eq(variable.masked?)
-            expect(json_response['raw']).to eq(variable.raw?)
-            expect(json_response['variable_type']).to eq('env_var')
-            expect(json_response['description']).to be_nil
-            expect(json_response['hidden']).to eq(false)
-          end
-        end
       end
 
       it 'responds with 404 Not Found if requesting non-existing variable' do
diff --git a/spec/requests/api/graphql/ci/group_variables_spec.rb b/spec/requests/api/graphql/ci/group_variables_spec.rb
index edc182dbfb4a9c..5390311bf7dbfa 100644
--- a/spec/requests/api/graphql/ci/group_variables_spec.rb
+++ b/spec/requests/api/graphql/ci/group_variables_spec.rb
@@ -88,39 +88,6 @@
         'environmentScope' => 'staging'
       })
     end
-
-    context 'when feature flag `ci_hidden_variables is disabled`' do
-      before do
-        stub_feature_flags(ci_hidden_variables: false)
-      end
-
-      it "returns the value even if it is hidden" do
-        variable = create(:ci_group_variable,
-          group: group,
-          key: 'TEST_VAR',
-          value: 'TestValue',
-          masked: true,
-          hidden: true,
-          protected: true,
-          raw: false,
-          environment_scope: 'staging')
-
-        post_graphql(query, current_user: user)
-
-        expect(graphql_data.dig('group', 'ciVariables', 'limit')).to be(30000)
-        expect(graphql_data.dig('group', 'ciVariables', 'nodes')).to contain_exactly({
-          'id' => variable.to_global_id.to_s,
-          'key' => 'TEST_VAR',
-          'value' => 'TestValue',
-          'variableType' => 'ENV_VAR',
-          'masked' => true,
-          'protected' => true,
-          'hidden' => true,
-          'raw' => false,
-          'environmentScope' => 'staging'
-        })
-      end
-    end
   end
 
   context 'when the user cannot administer the group' do
diff --git a/spec/requests/api/graphql/ci/project_variables_spec.rb b/spec/requests/api/graphql/ci/project_variables_spec.rb
index ab664d79b43e6e..3f54eb86204a8e 100644
--- a/spec/requests/api/graphql/ci/project_variables_spec.rb
+++ b/spec/requests/api/graphql/ci/project_variables_spec.rb
@@ -92,41 +92,6 @@
         'environmentScope' => 'production'
       })
     end
-
-    context 'when feature flag `ci_hidden_variables is disabled`' do
-      before do
-        stub_feature_flags(ci_hidden_variables: false)
-      end
-
-      it "returns the value even it it is hidden" do
-        variable = create(
-          :ci_variable,
-          project: project,
-          key: 'TEST_VAR',
-          value: 'TestVariable',
-          masked: true,
-          hidden: true,
-          protected: true,
-          raw: false,
-          environment_scope: 'production'
-        )
-
-        post_graphql(query, current_user: user)
-
-        expect(graphql_data.dig('project', 'ciVariables', 'limit')).to be(8000)
-        expect(graphql_data.dig('project', 'ciVariables', 'nodes')).to contain_exactly({
-          'id' => variable.to_global_id.to_s,
-          'key' => 'TEST_VAR',
-          'value' => 'TestVariable',
-          'variableType' => 'ENV_VAR',
-          'masked' => true,
-          'protected' => true,
-          'hidden' => true,
-          'raw' => false,
-          'environmentScope' => 'production'
-        })
-      end
-    end
   end
 
   context 'when the user cannot administer builds' do
diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb
index 3805e0b5d06bae..9bfb549bea6117 100644
--- a/spec/requests/api/group_variables_spec.rb
+++ b/spec/requests/api/group_variables_spec.rb
@@ -61,24 +61,6 @@
           expect(json_response['environment_scope']).to eq(variable.environment_scope)
           expect(json_response['description']).to be_nil
         end
-
-        context 'and feature flag `ci_hidden_variables is disabled`' do
-          before do
-            stub_feature_flags(ci_hidden_variables: false)
-          end
-
-          it 'returns group variable details without changes' do
-            get api("/groups/#{group.id}/variables/#{variable.key}", user)
-
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(json_response['value']).to eq(variable.value)
-            expect(json_response['protected']).to eq(variable.protected?)
-            expect(json_response['hidden']).to eq(true)
-            expect(json_response['variable_type']).to eq(variable.variable_type)
-            expect(json_response['environment_scope']).to eq(variable.environment_scope)
-            expect(json_response['description']).to be_nil
-          end
-        end
       end
 
       context 'when variable is not hidden' do
@@ -95,24 +77,6 @@
           expect(json_response['environment_scope']).to eq(variable.environment_scope)
           expect(json_response['description']).to be_nil
         end
-
-        context 'and feature flag `ci_hidden_variables is disabled`' do
-          before do
-            stub_feature_flags(ci_hidden_variables: false)
-          end
-
-          it 'returns group variable details' do
-            get api("/groups/#{group.id}/variables/#{variable.key}", user)
-
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(json_response['value']).to eq(variable.value)
-            expect(json_response['protected']).to eq(variable.protected?)
-            expect(json_response['hidden']).to eq(false)
-            expect(json_response['variable_type']).to eq(variable.variable_type)
-            expect(json_response['environment_scope']).to eq(variable.environment_scope)
-            expect(json_response['description']).to be_nil
-          end
-        end
       end
 
       it 'responds with 404 Not Found if requesting non-existing variable' do
-- 
GitLab


From 90793dcec6a47095a8212f433d4df4b4985f54a5 Mon Sep 17 00:00:00 2001
From: Miranda Fluharty <mfluharty@gitlab.com>
Date: Wed, 2 Oct 2024 15:56:42 -0600
Subject: [PATCH 2/4] Remove frontend feature flag check

Keep areHiddenVariablesAvailable check because
hidden variables are only supported at group/project level
Add explanatory comment
---
 .../ci/ci_variable_list/components/ci_variable_shared.vue      | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_shared.vue b/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_shared.vue
index 3363088aed2e0c..9d4ba4fef2a51d 100644
--- a/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_shared.vue
+++ b/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_shared.vue
@@ -186,7 +186,8 @@ export default {
       return this.$apollo.queries.environments.loading;
     },
     areHiddenVariablesAvailable() {
-      return Boolean(this.entity && this.glFeatures?.ciHiddenVariables);
+      // group and project variables can be hidden, instance variables cannot
+      return Boolean(this.entity);
     },
     hasEnvScopeQuery() {
       return Boolean(this.queryData?.environments?.query);
-- 
GitLab


From 42af52c08d5210851e4390ae36e61f79a332c17b Mon Sep 17 00:00:00 2001
From: Dmytro Biryukov <dbiryukov@gitlab.com>
Date: Tue, 8 Oct 2024 19:06:11 +0200
Subject: [PATCH 3/4] Replace hidden_variables_enabled with group and project
 presence validation, update docs

---
 app/views/ci/variables/_attributes.html.haml | 4 +---
 doc/ci/variables/index.md                    | 6 +-----
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/app/views/ci/variables/_attributes.html.haml b/app/views/ci/variables/_attributes.html.haml
index a418f58217209a..f83f84b985ae33 100644
--- a/app/views/ci/variables/_attributes.html.haml
+++ b/app/views/ci/variables/_attributes.html.haml
@@ -1,12 +1,10 @@
-- hidden_variables_enabled = @group.present? || @project.present?
-
 %p
   = s_('CiVariables|Variables can be accidentally exposed in a job log, or maliciously sent to a third party server. The masked variable feature can help reduce the risk of accidentally exposing variable values, but is not a guaranteed method to prevent malicious users from accessing variables.')
   = link_to _('How can I make my variables more secure?'), help_page_path('ci/variables/index.md', anchor: 'cicd-variable-security'), target: '_blank', rel: 'noopener noreferrer'
 %p
   = s_('CiVariables|Variables can have several attributes.')
   = link_to _('Learn more.'), help_page_path('ci/variables/index.md', anchor: 'define-a-cicd-variable-in-the-ui'), target: '_blank', rel: 'noopener noreferrer'
-- if hidden_variables_enabled
+- if @group.present? || @project.present?
   %ul
     %li
       = safe_format(s_('CiVariables|%{strong_start}Visibility:%{strong_end} Set the visibility level for the value. Can be visible, masked, or masked and hidden.'), tag_pair(tag.strong, :strong_start, :strong_end))
diff --git a/doc/ci/variables/index.md b/doc/ci/variables/index.md
index 238a89d21ead65..83b33e7bd37cc7 100644
--- a/doc/ci/variables/index.md
+++ b/doc/ci/variables/index.md
@@ -329,11 +329,7 @@ Different versions of [GitLab Runner](../runners/index.md) have different maskin
 
 ### Hide a CI/CD variable
 
-> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/29674) in GitLab 17.4 [with a flag](../../administration/feature_flags.md) named `ci_hidden_variables`. Enabled by default.
-
-FLAG:
-The availability of this feature is controlled by a feature flag.
-For more information, see the history.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/29674) in GitLab 17.4
 
 In addition to masking, you can also prevent the value of CI/CD variables from being revealed
 in the **CI/CD** settings page. Hiding a variable is only possible when creating a new variable,
-- 
GitLab


From 922a1a91c3d6ed0db9bd418c8e48e673c0786407 Mon Sep 17 00:00:00 2001
From: Dmytro Biryukov <dbiryukov@gitlab.com>
Date: Mon, 14 Oct 2024 06:58:44 +0000
Subject: [PATCH 4/4] Apply 1 suggestion(s) to 1 file(s)

Co-authored-by: Pam Artiaga <partiaga@gitlab.com>
---
 doc/ci/variables/index.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/doc/ci/variables/index.md b/doc/ci/variables/index.md
index 83b33e7bd37cc7..d5a52399a18c46 100644
--- a/doc/ci/variables/index.md
+++ b/doc/ci/variables/index.md
@@ -329,7 +329,8 @@ Different versions of [GitLab Runner](../runners/index.md) have different maskin
 
 ### Hide a CI/CD variable
 
-> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/29674) in GitLab 17.4
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/29674) in GitLab 17.4 [with a flag](../../administration/feature_flags.md) named `ci_hidden_variables`. Enabled by default.
+> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165843) in GitLab 17.6. Feature flag `ci_hidden_variables` removed.
 
 In addition to masking, you can also prevent the value of CI/CD variables from being revealed
 in the **CI/CD** settings page. Hiding a variable is only possible when creating a new variable,
-- 
GitLab