Remove workhorse_extract_filename_base feature flag

This feature flag was enabled by default in GitLab 13.11 and no longer
needs to be present. We leave the field in the Workhorse structure to
avoid potential deserialization issues.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/326379

Changelog: changed

app/uploaders/object_storage.rb

@@ -187,7 +187,6 @@ module ObjectStorage
             hash[:TempPath] = workhorse_local_upload_path
           end
 
-          hash[:FeatureFlagExtractBase] = Feature.enabled?(:workhorse_extract_filename_base, default_enabled: :yaml)
           hash[:MaximumSize] = maximum_size if maximum_size.present?
         end
       end

changelogs/unreleased/sh-remove-workhorse-extract-filename-base-ff.yml

+---
+title: Remove workhorse_extract_filename_base feature flag
+merge_request: 60070
+author:
+type: changed

config/feature_flags/development/workhorse_extract_filename_base.yml

----
-name: workhorse_extract_filename_base
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57889
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/326379
-milestone: '13.11'
-type: development
-group: group::source code
-default_enabled: true

spec/uploaders/object_storage_spec.rb

@@ -441,22 +441,6 @@ RSpec.describe ObjectStorage do
       end
     end
 
-    shared_examples 'extracts base filename' do
-      it "returns true for ExtractsBase" do
-        expect(subject[:FeatureFlagExtractBase]).to be true
-      end
-
-      context 'when workhorse_extract_filename_base is disabled' do
-        before do
-          stub_feature_flags(workhorse_extract_filename_base: false)
-        end
-
-        it "returns false for ExtractsBase" do
-          expect(subject[:FeatureFlagExtractBase]).to be false
-        end
-      end
-    end
-
     shared_examples 'uses local storage' do
       it_behaves_like 'returns the maximum size given' do
         it "returns temporary path" do
@@ -518,7 +502,6 @@ RSpec.describe ObjectStorage do
       end
 
       it_behaves_like 'uses local storage'
-      it_behaves_like 'extracts base filename'
     end
 
     context 'when object storage is enabled' do
@@ -526,8 +509,6 @@ RSpec.describe ObjectStorage do
         allow(Gitlab.config.uploads.object_store).to receive(:enabled) { true }
       end
 
-      it_behaves_like 'extracts base filename'
-
       context 'when direct upload is enabled' do
         before do
           allow(Gitlab.config.uploads.object_store).to receive(:direct_upload) { true }

workhorse/internal/api/api.go

@@ -149,7 +149,7 @@ type Response struct {
 	ProcessLsifReferences bool
 	// The maximum accepted size in bytes of the upload
 	MaximumSize int64
-	// Feature flag used to determine whether to strip the multipart filename of any directories
+	// DEPRECATED: Feature flag used to determine whether to strip the multipart filename of any directories
 	FeatureFlagExtractBase bool
 }
 

workhorse/internal/filestore/save_file_opts.go

@@ -63,8 +63,6 @@ type SaveFileOpts struct {
 	PresignedCompleteMultipart string
 	// PresignedAbortMultipart is a presigned URL for AbortMultipartUpload
 	PresignedAbortMultipart string
-	// FeatureFlagExtractBase uses the base of the filename and strips directories
-	FeatureFlagExtractBase bool
 }
 
 // UseWorkhorseClientEnabled checks if the options require direct access to object storage
@@ -90,7 +88,6 @@ func GetOpts(apiResponse *api.Response) (*SaveFileOpts, error) {
 	}
 
 	opts := SaveFileOpts{
-		FeatureFlagExtractBase: apiResponse.FeatureFlagExtractBase,
 		LocalTempPath:      apiResponse.TempPath,
 		RemoteID:           apiResponse.RemoteObject.ID,
 		RemoteURL:          apiResponse.RemoteObject.GetURL,

workhorse/internal/filestore/save_file_opts_test.go

@@ -61,15 +61,11 @@ func TestGetOpts(t *testing.T) {
 		multipart        *api.MultipartUploadParams
 		customPutHeaders bool
 		putHeaders       map[string]string
-		FeatureFlagExtractBase bool
 	}{
 		{
 			name: "Single upload",
 		},
 		{
-			name:                   "Single upload w/ FeatureFlagExtractBase enabled",
-			FeatureFlagExtractBase: true,
-		}, {
 			name: "Multipart upload",
 			multipart: &api.MultipartUploadParams{
 				PartSize:    10,
@@ -98,7 +94,6 @@ func TestGetOpts(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			apiResponse := &api.Response{
-				FeatureFlagExtractBase: test.FeatureFlagExtractBase,
 				RemoteObject: api.RemoteObject{
 					Timeout:          10,
 					ID:               "id",
@@ -114,7 +109,6 @@ func TestGetOpts(t *testing.T) {
 			opts, err := filestore.GetOpts(apiResponse)
 			require.NoError(t, err)
 
-			require.Equal(t, apiResponse.FeatureFlagExtractBase, opts.FeatureFlagExtractBase)
 			require.Equal(t, apiResponse.TempPath, opts.LocalTempPath)
 			require.WithinDuration(t, deadline, opts.Deadline, time.Second)
 			require.Equal(t, apiResponse.RemoteObject.ID, opts.RemoteID)

workhorse/internal/upload/rewrite.go

@@ -116,11 +116,7 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, pr
 func (rew *rewriter) handleFilePart(ctx context.Context, name string, p *multipart.Part, opts *filestore.SaveFileOpts) error {
 	multipartFiles.WithLabelValues(rew.filter.Name()).Inc()
 
-	filename := p.FileName()
-
-	if opts.FeatureFlagExtractBase {
-		filename = filepath.Base(filename)
-	}
+	filename := filepath.Base(p.FileName())
 
 	if strings.Contains(filename, "/") || filename == "." || filename == ".." {
 		return fmt.Errorf("illegal filename: %q", filename)

workhorse/internal/upload/uploads_test.go

@@ -327,18 +327,15 @@ func TestInvalidFileNames(t *testing.T) {
 	for _, testCase := range []struct {
 		filename       string
 		code           int
-		FeatureFlagExtractBase bool
 		expectedPrefix string
 	}{
-		{"foobar", 200, false, "foobar"}, // sanity check for test setup below
-		{"foo/bar", 500, false, ""},
-		{"foo/bar", 200, true, "bar"},
-		{"foo/bar/baz", 200, true, "baz"},
-		{"/../../foobar", 500, false, ""},
-		{"/../../foobar", 200, true, "foobar"},
-		{".", 500, false, ""},
-		{"..", 500, false, ""},
-		{"./", 500, false, ""},
+		{"foobar", 200, "foobar"}, // sanity check for test setup below
+		{"foo/bar", 200, "bar"},
+		{"foo/bar/baz", 200, "baz"},
+		{"/../../foobar", 200, "foobar"},
+		{".", 500, ""},
+		{"..", 500, ""},
+		{"./", 500, ""},
 	} {
 		buffer := &bytes.Buffer{}
 
@@ -356,7 +353,6 @@ func TestInvalidFileNames(t *testing.T) {
 		apiResponse := &api.Response{TempPath: tempPath}
 		preparer := &DefaultPreparer{}
 		opts, _, err := preparer.Prepare(apiResponse)
-		opts.FeatureFlagExtractBase = testCase.FeatureFlagExtractBase
 		require.NoError(t, err)
 
 		HandleFileUploads(response, httpRequest, nilHandler, apiResponse, &SavedFileTracker{Request: httpRequest}, opts)