diff --git a/app/services/packages/helm/process_file_service.rb b/app/services/packages/helm/process_file_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..31b357c1616e85ac928dcad6d82eb86df7f56f1b --- /dev/null +++ b/app/services/packages/helm/process_file_service.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +module Packages + module Helm + class ProcessFileService + include Gitlab::Utils::StrongMemoize + include ExclusiveLeaseGuard + + ExtractionError = Class.new(StandardError) + DEFAULT_LEASE_TIMEOUT = 1.hour.to_i + + def initialize(channel, package_file) + @channel = channel + @package_file = package_file + end + + def execute + raise ExtractionError, 'Helm chart was not processed - package_file is not set' unless package_file + + try_obtain_lease do + temp_package.transaction do + rename_package_and_set_version + rename_package_file_and_set_metadata + cleanup_temp_package + end + end + end + + private + + attr_reader :channel, :package_file + + def rename_package_and_set_version + package.update!( + name: metadata['name'], + version: metadata['version'], + status: :default + ) + end + + def rename_package_file_and_set_metadata + # Updating file_name updates the path where the file is stored. + # We must pass the file again so that CarrierWave can handle the update + package_file.update!( + file_name: file_name, + file: package_file.file, + package_id: package.id, + helm_file_metadatum_attributes: { + channel: channel, + metadata: metadata + } + ) + end + + def cleanup_temp_package + temp_package.destroy if package.id != temp_package.id + end + + def temp_package + strong_memoize(:temp_package) do + package_file.package + end + end + + def package + strong_memoize(:package) do + project_packages = package_file.package.project.packages + package = project_packages.with_package_type(:helm) + .with_name(metadata['name']) + .with_version(metadata['version']) + .last + package || temp_package + end + end + + def metadata + strong_memoize(:metadata) do + ::Packages::Helm::ExtractFileMetadataService.new(package_file).execute + end + end + + def file_name + "#{metadata['name']}-#{metadata['version']}.tgz" + end + + # used by ExclusiveLeaseGuard + def lease_key + "packages:helm:process_file_service:package_file:#{package_file.id}" + end + + # used by ExclusiveLeaseGuard + def lease_timeout + DEFAULT_LEASE_TIMEOUT + end + end + end +end diff --git a/db/migrate/20210614143954_add_unique_index_for_helm_packages.rb b/db/migrate/20210614143954_add_unique_index_for_helm_packages.rb new file mode 100644 index 0000000000000000000000000000000000000000..e6b7ba7616d7a95e7e71e40a86f8fe7026eb0705 --- /dev/null +++ b/db/migrate/20210614143954_add_unique_index_for_helm_packages.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddUniqueIndexForHelmPackages < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + INDEX_NAME = 'index_packages_on_project_id_name_version_unique_when_helm' + PACKAGE_TYPE_HELM = 11 + + disable_ddl_transaction! + + def up + add_concurrent_index :packages_packages, [:project_id, :name, :version], unique: true, where: "package_type = #{PACKAGE_TYPE_HELM}", name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :packages_packages, INDEX_NAME + end +end diff --git a/db/schema_migrations/20210614143954 b/db/schema_migrations/20210614143954 new file mode 100644 index 0000000000000000000000000000000000000000..7fd3ce9b49eeeabe2fdae1eede47177450727615 --- /dev/null +++ b/db/schema_migrations/20210614143954 @@ -0,0 +1 @@ +b958d65f1b3b43d7bcd2a703489132ba9a2ba1e0374d45533399355ce6be9365 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index fa72d72cb9da5177879b25694fabd5040a71c741..6de556b602f7bdb8bc4af3bd17ebaaca2c18588e 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -24117,6 +24117,8 @@ CREATE UNIQUE INDEX index_packages_on_project_id_name_version_unique_when_generi CREATE UNIQUE INDEX index_packages_on_project_id_name_version_unique_when_golang ON packages_packages USING btree (project_id, name, version) WHERE (package_type = 8); +CREATE UNIQUE INDEX index_packages_on_project_id_name_version_unique_when_helm ON packages_packages USING btree (project_id, name, version) WHERE (package_type = 11); + CREATE INDEX index_packages_package_file_build_infos_on_package_file_id ON packages_package_file_build_infos USING btree (package_file_id); CREATE INDEX index_packages_package_file_build_infos_on_pipeline_id ON packages_package_file_build_infos USING btree (pipeline_id); diff --git a/spec/factories/packages/package_file.rb b/spec/factories/packages/package_file.rb index b02af85dbebc4c28aa000a78fed6e8b1523b2374..d82fbe02311d523c10cd12fccb8a687caccc1811 100644 --- a/spec/factories/packages/package_file.rb +++ b/spec/factories/packages/package_file.rb @@ -208,6 +208,8 @@ transient do without_loaded_metadatum { false } + package_name { package&.name || 'foo' } + sequence(:package_version) { |n| package&.version || "v#{n}" } channel { 'stable' } end diff --git a/spec/services/packages/helm/extract_file_metadata_service_spec.rb b/spec/services/packages/helm/extract_file_metadata_service_spec.rb index ea196190e24e22c5c996035cf73cb3c62879bd97..273f679b7360b7d58fe05d782111951846a45b4e 100644 --- a/spec/services/packages/helm/extract_file_metadata_service_spec.rb +++ b/spec/services/packages/helm/extract_file_metadata_service_spec.rb @@ -38,9 +38,7 @@ context 'with Chart.yaml at root' do before do expect_next_instances_of(Gem::Package::TarReader::Entry, 14) do |entry| - expect(entry).to receive(:full_name).exactly(:once) do - 'Chart.yaml' - end + expect(entry).to receive(:full_name).exactly(:once).and_return('Chart.yaml') end end diff --git a/spec/services/packages/helm/process_file_service_spec.rb b/spec/services/packages/helm/process_file_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..2e98590a4f4ed69ce8750095a055c2fd95301336 --- /dev/null +++ b/spec/services/packages/helm/process_file_service_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Packages::Helm::ProcessFileService do + let(:package) { create(:helm_package, without_package_files: true, status: 'processing')} + let!(:package_file) { create(:helm_package_file, without_loaded_metadatum: true, package: package) } + let(:channel) { 'stable' } + let(:service) { described_class.new(channel, package_file) } + + let(:expected) do + { + 'apiVersion' => 'v2', + 'description' => 'File, Block, and Object Storage Services for your Cloud-Native Environment', + 'icon' => 'https://rook.io/images/rook-logo.svg', + 'name' => 'rook-ceph', + 'sources' => ['https://github.com/rook/rook'], + 'version' => 'v1.5.8' + } + end + + describe '#execute' do + subject(:execute) { service.execute } + + context 'without a file' do + let(:package_file) { nil } + + it 'returns error', :aggregate_failures do + expect { execute } + .to not_change { Packages::Package.count } + .and not_change { Packages::PackageFile.count } + .and not_change { Packages::Helm::FileMetadatum.count } + .and raise_error(Packages::Helm::ProcessFileService::ExtractionError, 'Helm chart was not processed - package_file is not set') + end + end + + context 'with existing package' do + let!(:existing_package) { create(:helm_package, project: package.project, name: 'rook-ceph', version: 'v1.5.8') } + + it 'reuses existing package', :aggregate_failures do + expect { execute } + .to change { Packages::Package.count }.from(2).to(1) + .and not_change { package.name } + .and not_change { package.version } + .and not_change { package.status } + .and not_change { Packages::PackageFile.count } + .and change { package_file.file_name }.from(package_file.file_name).to("#{expected['name']}-#{expected['version']}.tgz") + .and change { Packages::Helm::FileMetadatum.count }.from(1).to(2) + .and change { package_file.helm_file_metadatum }.from(nil) + + expect { package.reload } + .to raise_error(ActiveRecord::RecordNotFound) + + expect(package_file.helm_file_metadatum.channel).to eq(channel) + expect(package_file.helm_file_metadatum.metadata).to eq(expected) + end + end + + context 'with a valid file' do + it 'processes file', :aggregate_failures do + expect { execute } + .to not_change { Packages::Package.count } + .and change { package.name }.from(package.name).to(expected['name']) + .and change { package.version }.from(package.version).to(expected['version']) + .and change { package.status }.from('processing').to('default') + .and not_change { Packages::PackageFile.count } + .and change { package_file.file_name }.from(package_file.file_name).to("#{expected['name']}-#{expected['version']}.tgz") + .and change { Packages::Helm::FileMetadatum.count }.by(1) + .and change { package_file.helm_file_metadatum }.from(nil) + + expect(package_file.helm_file_metadatum.channel).to eq(channel) + expect(package_file.helm_file_metadatum.metadata).to eq(expected) + end + end + + context 'without Chart.yaml' do + before do + expect_next_instances_of(Gem::Package::TarReader::Entry, 14) do |entry| + expect(entry).to receive(:full_name).exactly(:once).and_wrap_original do |m, *args| + m.call(*args) + '_suffix' + end + end + end + + it { expect { execute }.to raise_error(Packages::Helm::ExtractFileMetadataService::ExtractionError, 'Chart.yaml not found within a directory') } + end + + context 'with Chart.yaml at root' do + before do + expect_next_instances_of(Gem::Package::TarReader::Entry, 14) do |entry| + expect(entry).to receive(:full_name).exactly(:once).and_return('Chart.yaml') + end + end + + it { expect { execute }.to raise_error(Packages::Helm::ExtractFileMetadataService::ExtractionError, 'Chart.yaml not found within a directory') } + end + + context 'with an invalid YAML' do + before do + expect_next_instance_of(Gem::Package::TarReader::Entry) do |entry| + expect(entry).to receive(:read).and_return('{') + end + end + + it { expect { execute }.to raise_error(Packages::Helm::ExtractFileMetadataService::ExtractionError, 'Error while parsing Chart.yaml: (<unknown>): did not find expected node content while parsing a flow node at line 2 column 1') } + end + end +end