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 3363088aed2e0c1f4b46e295d12a499e87a1ec5e..9d4ba4fef2a51dcb91a49de7d2d83275c06815e9 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); diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb index bef7fd7060f2a828afa1887bc8a194a669dc3937..216c1b615cfe38f22d41b981dc362d03311ca425 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 c859677363934dfa450a56936dbf9f8de3e9fda2..71460a17a7540ca7bc08e0bb889e0fd2ad9cee5a 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 e73fb42d49c902c9a701f2d54998efeb04247de8..fc80e70260d93493728c019564754d1de4581b49 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 0810f4b7ab66532b335fe4dd81e42ec5dc2482cd..feb817969251f4da11fda2269c06b33caed736d9 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 40521f82489a58024f9cb180791d9ad92c94f67e..f83f84b985ae331f7ddc503620e4365526eeae69 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? && Feature.enabled?(:ci_hidden_variables, @group)) || (@project.present? && Feature.enabled?(:ci_hidden_variables, @project.group)) - %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/config/feature_flags/development/ci_hidden_variables.yml b/config/feature_flags/development/ci_hidden_variables.yml deleted file mode 100644 index 35b82f6d00e946d44938a2feed856a0530229f88..0000000000000000000000000000000000000000 --- 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/doc/ci/variables/index.md b/doc/ci/variables/index.md index 238a89d21ead65b8ca6ed5840b4c7c89dbb70b31..d5a52399a18c46c4d733e2c9e1917f9f0f155c6b 100644 --- a/doc/ci/variables/index.md +++ b/doc/ci/variables/index.md @@ -330,10 +330,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. +> - [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, diff --git a/spec/models/ci/variable_value_spec.rb b/spec/models/ci/variable_value_spec.rb index d80ac305f6bb5385010a868d54b25cdbb49c1a2b..29b9135c26958bc7cfa1c7ab5b455e93abdf91c5 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 6e39714fa01b3f8df76e92f1f74b24d683e2ac86..a431e6b5e723165f96461d4a4fa865d83e036f26 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 d4d4b7085885ad2622f02b10669dc63482d89dfe..8d64b7321c0f515d5e023f60cc0fdfabd687ee57 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 edc182dbfb4a9c12a76f3a4b1c17497a90b2576a..5390311bf7dbfa9462b80f62fac493ca0a2ff79e 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 ab664d79b43e6ee785810456525e1cecd3e6a8b8..3f54eb86204a8e705f97c51f571ecf4027eb9c6d 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 3805e0b5d06baeb8f547d6d9cf490c38f2c79232..9bfb549bea6117e44227115aca0a886398655b63 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