Merge branch 'revert-allow-dots-ff' into 'master'

Revert "Merge branch 'pw-update-ff-removal-language' into 'master'" for 15.11

See merge request !118239



Merged-by: Steve Abrams <sabrams@gitlab.com>
Approved-by: Mayra Cabrera <mcabrera@gitlab.com>
Approved-by: Steve Abrams <sabrams@gitlab.com>
Co-authored-by: Timo Furrer <tfurrer@gitlab.com>

config/feature_flags/development/allow_dots_on_tf_state_names.yml

+---
+name: allow_dots_on_tf_state_names
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106861
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/385597
+milestone: '15.7'
+type: development
+group: group::configure
+default_enabled: true

doc/user/infrastructure/iac/terraform_state.md

@@ -7,8 +7,10 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 # GitLab-managed Terraform state **(FREE)**
 
 > - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/2673) in GitLab 13.0.
-> - Support for state names that contain periods introduced in GitLab 15.7 [with a flag](../../../administration/feature_flags.md) named `allow_dots_on_tf_state_names`. Disabled by default.
-> - Support for state names that contain periods [generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/385597) in GitLab 16.0. Feature flag `allow_dots_on_tf_state_names` removed.
+> - Support for state names that contain periods introduced in GitLab 15.7 [with a flag](../../../administration/feature_flags.md) named `allow_dots_on_tf_state_names`. Disabled by default. [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106861) in GitLab 15.7.
+
+FLAG:
+On self-managed GitLab, by default support for state names that contain periods is not available. To make it available, ask an administrator to [enable the feature flag](../../../administration/feature_flags.md) named `allow_dots_on_tf_state_names`. On GitLab.com, support for state names that contain periods is available. Requests for state files might generate HTTP 404 errors after enabling this feature. For more information, see [Troubleshooting the Terraform integration with GitLab](troubleshooting.md#state-not-found-if-the-state-name-contains-a-period).
 
 Terraform uses state files to store details about your infrastructure configuration.
 With Terraform remote [backends](https://www.terraform.io/language/settings/backends/configuration),

doc/user/infrastructure/iac/troubleshooting.md

@@ -160,3 +160,12 @@ If your `TF_HTTP_ADDRESS`, `TF_HTTP_LOCK_ADDRESS` and `TF_HTTP_UNLOCK_ADDRESS` a
 to update the state names there.
 
 Alternatively, you can [migrate your terraform state](terraform_state.md#migrate-to-a-gitlab-managed-terraform-state).
+
+#### Self-managed GitLab instances
+
+By default, support for state names with periods is not enabled on self-managed GitLab.
+You can enable it from the Rails console:
+
+```ruby
+Feature.enable(:allow_dots_on_tf_state_names)
+```

lib/api/terraform/state.rb

@@ -55,6 +55,17 @@ class State < ::API::Base
             def remote_state_handler
               ::Terraform::RemoteStateHandler.new(user_project, current_user, name: params[:name], lock_id: params[:ID])
             end
+
+            def not_found_for_dots?
+              Feature.disabled?(:allow_dots_on_tf_state_names) && params[:name].include?(".")
+            end
+
+            # Change the state name to behave like before, https://gitlab.com/gitlab-org/gitlab/-/merge_requests/105674
+            # has been introduced. This behavior can be controlled via `allow_dots_on_tf_state_names` FF.
+            # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106861
+            def legacy_state_name!
+              params[:name] = params[:name].split('.').first
+            end
           end
 
           desc 'Get a Terraform state by its name' do
@@ -72,6 +83,8 @@ def remote_state_handler
           end
           route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth
           get do
+            legacy_state_name! if not_found_for_dots?
+
             remote_state_handler.find_with_lock do |state|
               no_content! unless state.latest_file && state.latest_file.exists?
 
@@ -96,6 +109,7 @@ def remote_state_handler
           route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth
           post do
             authorize! :admin_terraform_state, user_project
+            legacy_state_name! if not_found_for_dots?
 
             data = request.body.read
             no_content! if data.empty?
@@ -124,6 +138,7 @@ def remote_state_handler
           route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth
           delete do
             authorize! :admin_terraform_state, user_project
+            legacy_state_name! if not_found_for_dots?
 
             remote_state_handler.find_with_lock do |state|
               ::Terraform::States::TriggerDestroyService.new(state, current_user: current_user).execute
@@ -155,6 +170,8 @@ def remote_state_handler
             requires :Path, type: String, desc: 'Terraform path'
           end
           post '/lock' do
+            not_found! if not_found_for_dots?
+
             authorize! :admin_terraform_state, user_project
 
             status_code = :ok
@@ -198,6 +215,8 @@ def remote_state_handler
             optional :ID, type: String, limit: 255, desc: 'Terraform state lock ID'
           end
           delete '/lock' do
+            not_found! if not_found_for_dots?
+
             authorize! :admin_terraform_state, user_project
 
             remote_state_handler.unlock!

spec/requests/api/terraform/state_spec.rb

@@ -114,6 +114,17 @@
           end
         end
 
+        context 'allow_dots_on_tf_state_names is disabled, and the state name contains a dot' do
+          let(:state_name) { 'state-name-with-dot' }
+          let(:state_path) { "/projects/#{project_id}/terraform/state/#{state_name}.tfstate" }
+
+          before do
+            stub_feature_flags(allow_dots_on_tf_state_names: false)
+          end
+
+          it_behaves_like 'can access terraform state'
+        end
+
         context 'for a project that does not exist' do
           let(:project_id) { '0000' }
 
@@ -266,6 +277,21 @@
             expect(Gitlab::Json.parse(response.body)).to be_empty
           end
         end
+
+        context 'allow_dots_on_tf_state_names is disabled, and the state name contains a dot' do
+          let(:non_existing_state_name) { 'state-name-with-dot.tfstate' }
+
+          before do
+            stub_feature_flags(allow_dots_on_tf_state_names: false)
+          end
+
+          it 'strips characters after the dot' do
+            expect { request }.to change { Terraform::State.count }.by(1)
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(Terraform::State.last.name).to eq('state-name-with-dot')
+          end
+        end
       end
 
       context 'without body' do
@@ -373,6 +399,18 @@
         it_behaves_like 'schedules the state for deletion'
       end
 
+      context 'allow_dots_on_tf_state_names is disabled, and the state name contains a dot' do
+        let(:state_name) { 'state-name-with-dot' }
+        let(:state_name_with_dot) { "#{state_name}.tfstate" }
+        let(:state_path) { "/projects/#{project_id}/terraform/state/#{state_name_with_dot}" }
+
+        before do
+          stub_feature_flags(allow_dots_on_tf_state_names: false)
+        end
+
+        it_behaves_like 'schedules the state for deletion'
+      end
+
       context 'with invalid state name' do
         let(:state_name) { 'foo/bar' }
 
@@ -462,10 +500,30 @@
     context 'with a dot in the state name' do
       let(:state_name) { 'test.state' }
 
-      it 'locks the terraform state' do
-        request
+      context 'with allow_dots_on_tf_state_names ff enabled' do
+        before do
+          stub_feature_flags(allow_dots_on_tf_state_names: true)
+        end
 
-        expect(response).to have_gitlab_http_status(:ok)
+        let(:state_name) { 'test.state' }
+
+        it 'locks the terraform state' do
+          request
+
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+      end
+
+      context 'with allow_dots_on_tf_state_names ff disabled' do
+        before do
+          stub_feature_flags(allow_dots_on_tf_state_names: false)
+        end
+
+        it 'returns 404' do
+          request
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
       end
     end
   end
@@ -486,6 +544,7 @@
     before do
       state.lock_xid = '123.456'
       state.save!
+      stub_feature_flags(allow_dots_on_tf_state_names: true)
     end
 
     subject(:request) { delete api("#{state_path}/lock"), headers: auth_header, params: params }
@@ -516,6 +575,23 @@
         end
       end
 
+      context 'with allow_dots_on_tf_state_names ff disabled' do
+        before do
+          stub_feature_flags(allow_dots_on_tf_state_names: false)
+        end
+
+        context 'with dots in the state name' do
+          let(:lock_id) { '123.456' }
+          let(:state_name) { 'test.state' }
+
+          it 'returns 404' do
+            request
+
+            expect(response).to have_gitlab_http_status(:not_found)
+          end
+        end
+      end
+
       context 'with no lock id (force-unlock)' do
         let(:params) { {} }