Add latest changes from gitlab-org/gitlab@master

app/controllers/admin/application_settings_controller.rb

@@ -208,7 +208,10 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
 
     params[:application_setting][:import_sources]&.delete("")
     params[:application_setting][:restricted_visibility_levels]&.delete("")
-    params[:application_setting][:required_instance_ci_template] = nil if params[:application_setting][:required_instance_ci_template].blank?
+
+    if params[:application_setting].key?(:required_instance_ci_template)
+      params[:application_setting][:required_instance_ci_template] = nil if params[:application_setting][:required_instance_ci_template].empty?
+    end
 
     remove_blank_params_for!(:elasticsearch_aws_secret_access_key, :eks_secret_access_key)
 
@@ -217,9 +220,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
     params.delete(:domain_denylist_raw) if params[:domain_denylist]
     params.delete(:domain_allowlist_raw) if params[:domain_allowlist]
 
-    params.require(:application_setting).permit(
-      visible_application_setting_attributes
-    )
+    params[:application_setting].permit(visible_application_setting_attributes)
   end
 
   def recheck_user_consent?

app/controllers/confirmations_controller.rb

@@ -13,7 +13,7 @@ class ConfirmationsController < Devise::ConfirmationsController
   protected
 
   def after_resending_confirmation_instructions_path_for(resource)
-    return users_almost_there_path(email: resource.email) unless Feature.enabled?(:soft_email_confirmation)
+    return users_almost_there_path unless Feature.enabled?(:soft_email_confirmation)
 
     stored_location_for(resource) || dashboard_projects_path
   end

app/models/packages/package.rb

@@ -158,7 +158,7 @@ class Packages::Package < ApplicationRecord
     joins(:project).reorder(keyset_order)
   end
 
-  after_commit :update_composer_cache, on: :destroy, if: -> { composer? }
+  after_commit :update_composer_cache, on: :destroy, if: -> { composer? && Feature.disabled?(:disable_composer_callback) }
 
   def self.only_maven_packages_with_path(path, use_cte: false)
     if use_cte

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

config/feature_flags/development/disable_composer_callback.yml

@@ -0,0 +1,8 @@
+---
+name: disable_composer_callback
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/64016
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/333587
+milestone: '14.0'
+type: development
+group: group::package
+default_enabled: false

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

db/schema_migrations/20210614143954

@@ -0,0 +1 @@
+b958d65f1b3b43d7bcd2a703489132ba9a2ba1e0374d45533399355ce6be9365

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);

doc/.vale/gitlab/Admin.yml

@@ -8,6 +8,6 @@ extends: substitution
 message: 'Verify this use of the word "admin". Can it be updated to "administration", "administrator", "administer", or "Admin Area"?'
 link: https://docs.gitlab.com/ee/development/documentation/styleguide/index.html
 level: suggestion
-ignorecase: true
+ignorecase: false
 swap:
-  'admin ?\w*': '(?:Admin (Area|Mode)|[Aa]dminist(ration|rator|rators|er|rative))'
+  '[Aa]dmin ?\w*': '(?:Admin( Area| Mode)?|[Aa]dminist(ration|rator|rators|er|rative))'

doc/administration/troubleshooting/kubernetes_cheat_sheet.md

@@ -147,7 +147,7 @@ and they will assist you with any issues you are having.
 
   You can also use `gitlab-rake`, instead of `/usr/local/bin/gitlab-rake`.
 
-- Troubleshooting **Infrastructure > Kubernetes** integration:
+- Troubleshooting **Infrastructure > Kubernetes clusters** integration:
 
   - Check the output of `kubectl get events -w --all-namespaces`.
   - Check the logs of pods within `gitlab-managed-apps` namespace.

doc/user/project/clusters/index.md

@@ -31,7 +31,7 @@ Besides integration at the project level, Kubernetes clusters can also be
 integrated at the [group level](../../group/clusters/index.md) or
 [GitLab instance level](../../instance/clusters/index.md).
 
-To view your project level Kubernetes clusters, navigate to **Infrastructure > Kubernetes**
+To view your project level Kubernetes clusters, navigate to **Infrastructure > Kubernetes clusters**
 from your project. On this page, you can [add a new cluster](#adding-and-removing-clusters)
 and view information about your existing clusters, such as:
 
@@ -187,7 +187,7 @@ your cluster. This can cause deployment jobs to fail.
 
 To clear the cache:
 
-1. Navigate to your project's **Infrastructure > Kubernetes** page, and select your cluster.
+1. Navigate to your project's **Infrastructure > Kubernetes clusters** page, and select your cluster.
 1. Expand the **Advanced settings** section.
 1. Click **Clear cluster cache**.
 

doc/user/shortcuts.md

@@ -81,7 +81,7 @@ relatively quickly to work, and they take you to another page in the project.
 | <kbd>g</kbd> + <kbd>j</kbd> | Go to the CI/CD jobs list (**CI/CD > Jobs**). |
 | <kbd>g</kbd> + <kbd>l</kbd> | Go to the project metrics (**Monitor > Metrics**). |
 | <kbd>g</kbd> + <kbd>e</kbd> | Go to the project environments (**Deployments > Environments**). |
-| <kbd>g</kbd> + <kbd>k</kbd> | Go to the project Kubernetes cluster integration page (**Infrastructure > Kubernetes**). Note that you must have at least [`maintainer` permissions](permissions.md) to access this page. |
+| <kbd>g</kbd> + <kbd>k</kbd> | Go to the project Kubernetes cluster integration page (**Infrastructure > Kubernetes clusters**). Note that you must have at least [`maintainer` permissions](permissions.md) to access this page. |
 | <kbd>g</kbd> + <kbd>s</kbd> | Go to the project snippets list (**Snippets**). |
 | <kbd>g</kbd> + <kbd>w</kbd> | Go to the project wiki (**Wiki**), if enabled. |
 

spec/factories/packages/package_file.rb

@@ -208,6 +208,8 @@ FactoryBot.define do
 
       transient do
         without_loaded_metadatum { false }
+        package_name { package&.name || 'foo' }
+        sequence(:package_version) { |n| package&.version || "v#{n}" }
         channel { 'stable' }
       end
 

spec/features/users/login_spec.rb

@@ -128,6 +128,20 @@ RSpec.describe 'Login' do
         end
       end
     end
+
+    context 'when resending the confirmation email' do
+      it 'redirects to the "almost there" page' do
+        stub_feature_flags(soft_email_confirmation: false)
+
+        user = create(:user)
+
+        visit new_user_confirmation_path
+        fill_in 'user_email', with: user.email
+        click_button 'Resend'
+
+        expect(current_path).to eq users_almost_there_path
+      end
+    end
   end
 
   describe 'with the ghost user' do

spec/models/packages/package_spec.rb

@@ -1019,10 +1019,24 @@ RSpec.describe Packages::Package, type: :model do
       package.composer_metadatum.reload
     end
 
-    it 'schedule the update job' do
-      expect(::Packages::Composer::CacheUpdateWorker).to receive(:perform_async).with(project.id, package_name, package.composer_metadatum.version_cache_sha)
+    context 'with feature flag disabled' do
+      before do
+        stub_feature_flags(disable_composer_callback: false)
+      end
 
-      package.destroy!
+      it 'schedule the update job' do
+        expect(::Packages::Composer::CacheUpdateWorker).to receive(:perform_async).with(project.id, package_name, package.composer_metadatum.version_cache_sha)
+
+        package.destroy!
+      end
+    end
+
+    context 'with feature flag enabled' do
+      it 'does nothing' do
+        expect(::Packages::Composer::CacheUpdateWorker).not_to receive(:perform_async)
+
+        package.destroy!
+      end
     end
   end
 end

spec/services/packages/helm/extract_file_metadata_service_spec.rb

@@ -38,9 +38,7 @@ RSpec.describe Packages::Helm::ExtractFileMetadataService do
   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
 

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