Merge branch '415855-remove-legacy-component-fetching' into 'master'

Remove legacy fetching for CI components

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138343



Merged-by: Laura Montemayor <lmontemayor@gitlab.com>
Approved-by: Leaminn Ma <lma@gitlab.com>
Approved-by: Laura Montemayor <lmontemayor@gitlab.com>
Reviewed-by: Leaminn Ma <lma@gitlab.com>
Co-authored-by: Avielle Wolfe <awolfe@gitlab.com>

app/models/ci/catalog/components_project.rb

@@ -45,6 +45,8 @@ def extract_inputs(blob)
       end
       def fetch_component(component_name)
+        return ComponentData.new unless component_name.index('/').nil?
+
         path = simple_template_path(component_name)
         content = fetch_content(path)
@@ -53,11 +55,6 @@ def fetch_component(component_name)
           content = fetch_content(path)
         end
-        if content.nil?
-          path = legacy_template_path(component_name)
-          content = fetch_content(path)
-        end
-
         ComponentData.new(content: content, path: path)
       end
@@ -71,9 +68,6 @@ def fetch_content(component_path)
       # A simple template consists of a single file
       def simple_template_path(component_name)
-        # TODO: Extract this line and move to fetch_content once we remove legacy fetching
-        return unless component_name.index('/').nil?
-
         File.join(TEMPLATES_DIR, "#{component_name}.yml")
       end
@@ -81,15 +75,8 @@ def simple_template_path(component_name)
       # Given a path like "my-org/sub-group/the-project/templates/component"
       # returns the entry point path: "templates/component/template.yml".
       def complex_template_path(component_name)
-        # TODO: Extract this line and move to fetch_content once we remove legacy fetching
-        return unless component_name.index('/').nil?
-
         File.join(TEMPLATES_DIR, component_name, TEMPLATE_FILE)
       end
-
-      def legacy_template_path(component_name)
-        File.join(component_name, TEMPLATE_FILE).delete_prefix('/')
-      end
     end
   end
 end

app/services/ci/components/fetch_service.rb

@@ -24,7 +24,7 @@ def execute
         component_path = component_path_class.new(address: address)
         result = component_path.fetch_content!(current_user: current_user)
-        if result
+        if result&.content
           ServiceResponse.success(payload: {
             content: result.content,
             path: result.path,

doc/ci/components/index.md

@@ -92,65 +92,6 @@ In this example:
 - The `all-scans` component configuration is defined in a single file.
 - The `secret-detection` component configuration contains multiple files in a directory.
-#### Component configurations saved in any directory (deprecated)
-
-WARNING:
-Saving components through the following directory structure is [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/415855) and should be avoided.
-
-Components configurations can be saved through the following directory structure, containing:
-
-- `template.yml`: The component configuration, one file per component. If there is
-  only one component, this file can be in the root of the project. If there are multiple
-  components, each file must be in a separate subdirectory.
-- `README.md`: A documentation file explaining the details of all the components in the repository.
-
-For example, if the project is on GitLab.com, named `my-project`, and in a personal
-namespace named `my-namespace`:
-
-- Containing a single component and a simple pipeline to test the component, then
-  the file structure might be:
-
-  ```plaintext
-  ├── template.yml
-  ├── README.md
-  └── .gitlab-ci.yml
-  ```
-
-  This component is referenced with the path `gitlab.com/my-namespace/my-project@<version>`.
-
-- Containing one default component and multiple sub-components, then the file structure
-  might be:
-
-  ```plaintext
-  ├── template.yml
-  ├── README.md
-  ├── .gitlab-ci.yml
-  ├── unit/
-  │   └── template.yml
-  └── integration/
-      └── template.yml
-  ```
-
-  These components are identified by these paths:
-
-  - `gitlab.com/my-namespace/my-project`
-  - `gitlab.com/my-namespace/my-project/unit`
-  - `gitlab.com/my-namespace/my-project/integration`
-
-It is possible to have a components repository with no default component, by having
-no `template.yml` in the root directory.
-
-**Additional notes:**
-
-Nesting of components is not possible. For example:
-
-```plaintext
-├── unit/
-│   └── template.yml
-│   └── another_folder/
-│       └── nested_template.yml
-```
-
 ## Use a component
 You can use a component in a CI/CD configuration with the `include: component` keyword.

spec/lib/gitlab/ci/components/instance_path_spec.rb

@@ -181,88 +181,5 @@
         end
       end
     end
-
-    # All the following tests are for deprecated code and will be removed
-    # in https://gitlab.com/gitlab-org/gitlab/-/issues/415855
-    context 'when the project does not contain a templates directory' do
-      let(:project_path) { project.full_path }
-      let(:address) { "acme.com/#{project_path}/component@#{version}" }
-
-      let_it_be(:project) do
-        create(
-          :project, :custom_repo,
-          files: {
-            'component/template.yml' => 'image: alpine'
-          }
-        )
-      end
-
-      before do
-        project.add_developer(user)
-      end
-
-      it 'fetches the component content', :aggregate_failures do
-        result = path.fetch_content!(current_user: user)
-        expect(result.content).to eq('image: alpine')
-        expect(result.path).to eq('component/template.yml')
-        expect(path.host).to eq(current_host)
-        expect(path.project).to eq(project)
-        expect(path.sha).to eq(project.commit('master').id)
-      end
-
-      context 'when project path is nested under a subgroup' do
-        let_it_be(:group) { create(:group, :nested) }
-        let_it_be(:project) do
-          create(
-            :project, :custom_repo,
-            files: {
-              'component/template.yml' => 'image: alpine'
-            },
-            group: group
-          )
-        end
-
-        it 'fetches the component content', :aggregate_failures do
-          result = path.fetch_content!(current_user: user)
-          expect(result.content).to eq('image: alpine')
-          expect(result.path).to eq('component/template.yml')
-          expect(path.host).to eq(current_host)
-          expect(path.project).to eq(project)
-          expect(path.sha).to eq(project.commit('master').id)
-        end
-      end
-
-      context 'when current GitLab instance is installed on a relative URL' do
-        let(:address) { "acme.com/gitlab/#{project_path}/component@#{version}" }
-        let(:current_host) { 'acme.com/gitlab/' }
-
-        it 'fetches the component content', :aggregate_failures do
-          result = path.fetch_content!(current_user: user)
-          expect(result.content).to eq('image: alpine')
-          expect(result.path).to eq('component/template.yml')
-          expect(path.host).to eq(current_host)
-          expect(path.project).to eq(project)
-          expect(path.sha).to eq(project.commit('master').id)
-        end
-      end
-
-      context 'when version does not exist' do
-        let(:version) { 'non-existent' }
-
-        it 'returns nil', :aggregate_failures do
-          expect(path.fetch_content!(current_user: user)).to be_nil
-          expect(path.host).to eq(current_host)
-          expect(path.project).to eq(project)
-          expect(path.sha).to be_nil
-        end
-      end
-
-      context 'when user does not have permissions' do
-        it 'raises an error when fetching the content' do
-          expect { path.fetch_content!(current_user: build(:user)) }
-            .to raise_error(Gitlab::Access::AccessDeniedError)
-        end
-      end
-    end
   end
 end

spec/lib/gitlab/ci/config/external/processor_spec.rb

@@ -410,7 +410,7 @@
       let(:other_project_files) do
         {
-          '/component-x/template.yml' => <<~YAML
+          '/templates/component-x/template.yml' => <<~YAML
           component_x_job:
             script: echo Component X
           YAML

spec/models/ci/catalog/components_project_spec.rb

@@ -97,6 +97,7 @@
       'dast'             | 'image: alpine_2'                                   | 'templates/dast/template.yml'
       'template'         | 'image: alpine_3'                                   | 'templates/template.yml'
       'blank-yaml'       | ''                                                  | 'templates/blank-yaml.yml'
+      'non/exist'        | nil                                                 | nil
     end
     with_them do

spec/services/ci/components/fetch_service_spec.rb

@@ -21,14 +21,15 @@
     project = create(
       :project, :custom_repo,
       files: {
-        'template.yml' => content,
-        'my-component/template.yml' => content,
-        'my-dir/my-component/template.yml' => content
+        'templates/first-component.yml' => content,
+        'templates/second-component/template.yml' => content
       }
     )
     project.repository.add_tag(project.creator, 'v0.1', project.repository.commit.sha)
+    create(:release, project: project, tag: 'v0.1', sha: project.repository.commit.sha)
+
     project
   end
@@ -119,32 +120,27 @@
     context 'when address points to an external component' do
       let(:address) { "#{current_host}/#{component_path}@#{version}" }
-      context 'when component path is the full path to a project' do
-        let(:component_path) { project.full_path }
-        let(:component_yaml_path) { 'template.yml' }
+      context 'when component path points to a template file in a project' do
+        let(:component_path) { "#{project.full_path}/first-component" }
         it_behaves_like 'an external component'
       end
-      context 'when component path points to a directory in a project' do
-        let(:component_path) { "#{project.full_path}/my-component" }
-        let(:component_yaml_path) { 'my-component/template.yml' }
+      context 'when component path points to a template directory in a project' do
+        let(:component_path) { "#{project.full_path}/second-component" }
         it_behaves_like 'an external component'
       end
-      context 'when component path points to a nested directory in a project' do
-        let(:component_path) { "#{project.full_path}/my-dir/my-component" }
-        let(:component_yaml_path) { 'my-dir/my-component/template.yml' }
-        it_behaves_like 'an external component'
+      context 'when the project exists but the component does not' do
+        let(:component_path) { "#{project.full_path}/unknown-component" }
+        let(:version) { '~latest' }
+        it 'returns a content not found error' do
+          expect(result).to be_error
+          expect(result.reason).to eq(:content_not_found)
+        end
       end
     end
   end
-
-  def stub_project_blob(ref, path, content)
-    allow_next_instance_of(Repository) do |instance|
-      allow(instance).to receive(:blob_data_at).with(ref, path).and_return(content)
-    end
-  end
 end

spec/services/ci/create_pipeline_service_spec.rb

@@ -1757,7 +1757,7 @@ def previous_commit_sha_from_ref(ref)
       let(:sha) do
         components_project.repository.create_file(
           user,
-          'my-component/template.yml',
+          'templates/my-component/template.yml',
           template,
           message: 'Add my first CI component',
           branch_name: 'master'
@@ -1894,7 +1894,7 @@ def previous_commit_sha_from_ref(ref)
       let(:sha) do
         components_project.repository.create_file(
           user,
-          'my-component/template.yml',
+          'templates/my-component/template.yml',
           template,
           message: 'Add my first CI component',
           branch_name: 'master'