Merge branch 'jivanvl-keep-latest-artifact-project-level' into 'master'

Add "keep latest artifact" option for projects

See merge request gitlab-org/gitlab!49256

app/graphql/mutations/ci/ci_cd_settings_update.rb

+# frozen_string_literal: true
+
+module Mutations
+  module Ci
+    class CiCdSettingsUpdate < BaseMutation
+      include FindsProject
+
+      graphql_name 'CiCdSettingsUpdate'
+
+      authorize :admin_project
+
+      argument :full_path, GraphQL::ID_TYPE,
+        required: true,
+        description: 'Full Path of the project the settings belong to.'
+
+      argument :keep_latest_artifact, GraphQL::BOOLEAN_TYPE,
+        required: false,
+        description: 'Indicates if the latest artifact should be kept for this project.'
+
+      def resolve(full_path:, **args)
+        project = authorized_find!(full_path)
+        settings = project.ci_cd_settings
+        settings.update(args)
+
+        { errors: errors_on_object(settings) }
+      end
+    end
+  end
+end

app/graphql/types/ci/ci_cd_setting_type.rb

@@ -13,6 +13,8 @@ module Types
       field :merge_trains_enabled, GraphQL::BOOLEAN_TYPE, null: true,
         description: 'Whether merge trains are enabled.',
         method: :merge_trains_enabled?
+      field :keep_latest_artifact, GraphQL::BOOLEAN_TYPE, null: true,
+        description: 'Whether to keep the latest builds artifacts.'
       field :project, Types::ProjectType, null: true,
         description: 'Project the CI/CD settings belong to.'
     end

app/graphql/types/mutation_type.rb

@@ -91,6 +91,7 @@ module Types
     mount_mutation Mutations::Ci::Pipeline::Cancel
     mount_mutation Mutations::Ci::Pipeline::Destroy
     mount_mutation Mutations::Ci::Pipeline::Retry
+    mount_mutation Mutations::Ci::CiCdSettingsUpdate
     mount_mutation Mutations::Namespace::PackageSettings::Update
   end
 end

app/models/ci/ref.rb

@@ -33,6 +33,9 @@ module Ci
       state :still_failing, value: 5
 
       after_transition any => [:fixed, :success] do |ci_ref|
+        # Do not try to unlock if no artifacts are locked
+        next unless ci_ref.artifacts_locked?
+
         ci_ref.run_after_commit do
           Ci::PipelineSuccessUnlockArtifactsWorker.perform_async(ci_ref.last_finished_pipeline_id)
         end
@@ -54,6 +57,10 @@ module Ci
       Ci::Pipeline.last_finished_for_ref_id(self.id)&.id
     end
 
+    def artifacts_locked?
+      self.pipelines.where(locked: :artifacts_locked).exists?
+    end
+
     def update_status_by!(pipeline)
       retry_lock(self) do
         next unless last_finished_pipeline_id == pipeline.id

app/models/project.rb

@@ -410,6 +410,7 @@ class Project < ApplicationRecord
   delegate :dashboard_timezone, to: :metrics_setting, allow_nil: true, prefix: true
   delegate :default_git_depth, :default_git_depth=, to: :ci_cd_settings, prefix: :ci
   delegate :forward_deployment_enabled, :forward_deployment_enabled=, :forward_deployment_enabled?, to: :ci_cd_settings, prefix: :ci
+  delegate :keep_latest_artifact, :keep_latest_artifact=, :keep_latest_artifact?, to: :ci_cd_settings, prefix: :ci
   delegate :actual_limits, :actual_plan_name, to: :namespace, allow_nil: true
   delegate :allow_merge_on_skipped_pipeline, :allow_merge_on_skipped_pipeline?,
     :allow_merge_on_skipped_pipeline=, :has_confluence?, :allow_editing_commit_messages?,
@@ -833,6 +834,10 @@ class Project < ApplicationRecord
     webide_pipelines.running_or_pending.for_user(user)
   end
 
+  def latest_pipeline_locked
+    ci_keep_latest_artifact? ? :artifacts_locked : :unlocked
+  end
+
   def autoclose_referenced_issues
     return true if super.nil?
 

changelogs/unreleased/jivanvl-keep-latest-artifact-project-level.yml

+---
+title: Add keep latest artifact option for projects
+merge_request: 49256
+author:
+type: added

db/migrate/20201204193952_keep_latest_artifact_project_level.rb

+# frozen_string_literal: true
+
+class KeepLatestArtifactProjectLevel < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_column :project_ci_cd_settings, :keep_latest_artifact, :boolean, default: true, null: false
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :project_ci_cd_settings, :keep_latest_artifact
+    end
+  end
+end

db/schema_migrations/20201204193952

+39e5550b6ad6f718a51cf9838ac9148bcaa070aff60f6114bd96e4a76faf2ca1
\ No newline at end of file

db/structure.sql

@@ -15535,7 +15535,8 @@ CREATE TABLE project_ci_cd_settings (
     default_git_depth integer,
     forward_deployment_enabled boolean,
     merge_trains_enabled boolean DEFAULT false,
-    auto_rollback_enabled boolean DEFAULT false NOT NULL
+    auto_rollback_enabled boolean DEFAULT false NOT NULL,
+    keep_latest_artifact boolean DEFAULT true NOT NULL
 );
 
 CREATE SEQUENCE project_ci_cd_settings_id_seq

doc/api/graphql/reference/gitlab_schema.graphql

@@ -2345,6 +2345,41 @@ type CiBuildNeedEdge {
   node: CiBuildNeed
 }
 
+"""
+Autogenerated input type of CiCdSettingsUpdate
+"""
+input CiCdSettingsUpdateInput {
+  """
+  A unique identifier for the client performing the mutation.
+  """
+  clientMutationId: String
+
+  """
+  Full Path of the project the settings belong to.
+  """
+  fullPath: ID!
+
+  """
+  Indicates if the latest artifact should be kept for this project.
+  """
+  keepLatestArtifact: Boolean
+}
+
+"""
+Autogenerated return type of CiCdSettingsUpdate
+"""
+type CiCdSettingsUpdatePayload {
+  """
+  A unique identifier for the client performing the mutation.
+  """
+  clientMutationId: String
+
+  """
+  Errors encountered during execution of the mutation.
+  """
+  errors: [String!]!
+}
+
 type CiConfig {
   """
   Linting errors
@@ -15385,6 +15420,7 @@ type Mutation {
   awardEmojiToggle(input: AwardEmojiToggleInput!): AwardEmojiTogglePayload
   boardListCreate(input: BoardListCreateInput!): BoardListCreatePayload
   boardListUpdateLimitMetrics(input: BoardListUpdateLimitMetricsInput!): BoardListUpdateLimitMetricsPayload
+  ciCdSettingsUpdate(input: CiCdSettingsUpdateInput!): CiCdSettingsUpdatePayload
   clusterAgentDelete(input: ClusterAgentDeleteInput!): ClusterAgentDeletePayload
   clusterAgentTokenCreate(input: ClusterAgentTokenCreateInput!): ClusterAgentTokenCreatePayload
   clusterAgentTokenDelete(input: ClusterAgentTokenDeleteInput!): ClusterAgentTokenDeletePayload
@@ -18968,6 +19004,11 @@ type Project {
 }
 
 type ProjectCiCdSetting {
+  """
+  Whether to keep the latest builds artifacts.
+  """
+  keepLatestArtifact: Boolean
+
   """
   Whether merge pipelines are enabled.
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -6286,6 +6286,104 @@
           "enumValues": null,
           "possibleTypes": null
         },
+        {
+          "kind": "INPUT_OBJECT",
+          "name": "CiCdSettingsUpdateInput",
+          "description": "Autogenerated input type of CiCdSettingsUpdate",
+          "fields": null,
+          "inputFields": [
+            {
+              "name": "fullPath",
+              "description": "Full Path of the project the settings belong to.",
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "SCALAR",
+                  "name": "ID",
+                  "ofType": null
+                }
+              },
+              "defaultValue": null
+            },
+            {
+              "name": "keepLatestArtifact",
+              "description": "Indicates if the latest artifact should be kept for this project.",
+              "type": {
+                "kind": "SCALAR",
+                "name": "Boolean",
+                "ofType": null
+              },
+              "defaultValue": null
+            },
+            {
+              "name": "clientMutationId",
+              "description": "A unique identifier for the client performing the mutation.",
+              "type": {
+                "kind": "SCALAR",
+                "name": "String",
+                "ofType": null
+              },
+              "defaultValue": null
+            }
+          ],
+          "interfaces": null,
+          "enumValues": null,
+          "possibleTypes": null
+        },
+        {
+          "kind": "OBJECT",
+          "name": "CiCdSettingsUpdatePayload",
+          "description": "Autogenerated return type of CiCdSettingsUpdate",
+          "fields": [
+            {
+              "name": "clientMutationId",
+              "description": "A unique identifier for the client performing the mutation.",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "SCALAR",
+                "name": "String",
+                "ofType": null
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "errors",
+              "description": "Errors encountered during execution of the mutation.",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "LIST",
+                  "name": null,
+                  "ofType": {
+                    "kind": "NON_NULL",
+                    "name": null,
+                    "ofType": {
+                      "kind": "SCALAR",
+                      "name": "String",
+                      "ofType": null
+                    }
+                  }
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            }
+          ],
+          "inputFields": null,
+          "interfaces": [
+
+          ],
+          "enumValues": null,
+          "possibleTypes": null
+        },
         {
           "kind": "OBJECT",
           "name": "CiConfig",
@@ -42641,6 +42739,33 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "ciCdSettingsUpdate",
+              "description": null,
+              "args": [
+                {
+                  "name": "input",
+                  "description": null,
+                  "type": {
+                    "kind": "NON_NULL",
+                    "name": null,
+                    "ofType": {
+                      "kind": "INPUT_OBJECT",
+                      "name": "CiCdSettingsUpdateInput",
+                      "ofType": null
+                    }
+                  },
+                  "defaultValue": null
+                }
+              ],
+              "type": {
+                "kind": "OBJECT",
+                "name": "CiCdSettingsUpdatePayload",
+                "ofType": null
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "clusterAgentDelete",
               "description": null,
@@ -55174,6 +55299,20 @@
           "name": "ProjectCiCdSetting",
           "description": null,
           "fields": [
+            {
+              "name": "keepLatestArtifact",
+              "description": "Whether to keep the latest builds artifacts.",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "SCALAR",
+                "name": "Boolean",
+                "ofType": null
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "mergePipelinesEnabled",
               "description": "Whether merge pipelines are enabled.",

doc/api/graphql/reference/index.md

@@ -383,6 +383,15 @@ Represents the total number of issues and their weights for a particular day.
 | ----- | ---- | ----------- |
 | `name` | String | Name of the job we need to complete. |
 
+### CiCdSettingsUpdatePayload
+
+Autogenerated return type of CiCdSettingsUpdate.
+
+| Field | Type | Description |
+| ----- | ---- | ----------- |
+| `clientMutationId` | String | A unique identifier for the client performing the mutation. |
+| `errors` | String! => Array | Errors encountered during execution of the mutation. |
+
 ### CiConfig
 
 | Field | Type | Description |
@@ -2714,6 +2723,7 @@ Autogenerated return type of PipelineRetry.
 
 | Field | Type | Description |
 | ----- | ---- | ----------- |
+| `keepLatestArtifact` | Boolean | Whether to keep the latest builds artifacts. |
 | `mergePipelinesEnabled` | Boolean | Whether merge pipelines are enabled. |
 | `mergeTrainsEnabled` | Boolean | Whether merge trains are enabled. |
 | `project` | Project | Project the CI/CD settings belong to. |

lib/gitlab/ci/pipeline/chain/build.rb

@@ -20,7 +20,8 @@ module Gitlab
               pipeline_schedule: @command.schedule,
               merge_request: @command.merge_request,
               external_pull_request: @command.external_pull_request,
-              variables_attributes: Array(@command.variables_attributes)
+              variables_attributes: Array(@command.variables_attributes),
+              locked: @command.project.latest_pipeline_locked
             )
           end
 

spec/factories/projects.rb

@@ -39,6 +39,7 @@ FactoryBot.define do
       group_runners_enabled { nil }
       merge_pipelines_enabled { nil }
       merge_trains_enabled { nil }
+      ci_keep_latest_artifact { nil }
       import_status { nil }
       import_jid { nil }
       import_correlation_id { nil }
@@ -82,6 +83,7 @@ FactoryBot.define do
       project.group_runners_enabled = evaluator.group_runners_enabled unless evaluator.group_runners_enabled.nil?
       project.merge_pipelines_enabled = evaluator.merge_pipelines_enabled unless evaluator.merge_pipelines_enabled.nil?
       project.merge_trains_enabled = evaluator.merge_trains_enabled unless evaluator.merge_trains_enabled.nil?
+      project.ci_keep_latest_artifact = evaluator.ci_keep_latest_artifact unless evaluator.ci_keep_latest_artifact.nil?
 
       if evaluator.import_status
         import_state = project.import_state || project.build_import_state

spec/lib/gitlab/ci/pipeline/chain/build_spec.rb

@@ -157,4 +157,25 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build do
       expect(pipeline.target_sha).to eq(external_pull_request.target_sha)
     end
   end
+
+  context 'when keep_latest_artifact is set' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:keep_latest_artifact, :locking_result) do
+      true          | 'artifacts_locked'
+      false         | 'unlocked'
+    end
+
+    with_them do
+      before do
+        project.update!(ci_keep_latest_artifact: keep_latest_artifact)
+      end
+
+      it 'builds a pipeline with appropriate locked value' do
+        step.perform!
+
+        expect(pipeline.locked).to eq(locking_result)
+      end
+    end
+  end
 end

spec/models/ci/ref_spec.rb

@@ -16,6 +16,9 @@ RSpec.describe Ci::Ref do
         stub_const('Ci::PipelineSuccessUnlockArtifactsWorker', unlock_artifacts_worker_spy)
       end
 
+      context 'pipline is locked' do
+        let!(:pipeline) { create(:ci_pipeline, ci_ref_id: ci_ref.id, locked: :artifacts_locked) }
+
         where(:initial_state, :action, :count) do
           :unknown | :succeed! | 1
           :unknown | :do_fail! | 0
@@ -46,6 +49,17 @@ RSpec.describe Ci::Ref do
           end
         end
       end
+
+      context 'pipeline is unlocked' do
+        let!(:pipeline) { create(:ci_pipeline, ci_ref_id: ci_ref.id, locked: :unlocked) }
+
+        it 'does not call unlock artifacts service' do
+          ci_ref.succeed!
+
+          expect(unlock_artifacts_worker_spy).not_to have_received(:perform_async)
+        end
+      end
+    end
   end
 
   describe '.ensure_for' do

spec/requests/api/graphql/ci/ci_cd_setting_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe 'Getting Ci Cd Setting' do
 
   let(:fields) do
     <<~QUERY
-      #{all_graphql_fields_for('ProjectCiCdSetting')}
+      #{all_graphql_fields_for('ProjectCiCdSetting', max_depth: 1)}
     QUERY
   end
 
@@ -43,8 +43,10 @@ RSpec.describe 'Getting Ci Cd Setting' do
 
     it_behaves_like 'a working graphql query'
 
-    specify { expect(settings_data['mergePipelinesEnabled']).to eql project.ci_cd_settings.merge_pipelines_enabled? }
-    specify { expect(settings_data['mergeTrainsEnabled']).to eql project.ci_cd_settings.merge_trains_enabled? }
-    specify { expect(settings_data['project']['id']).to eql "gid://gitlab/Project/#{project.id}" }
+    it 'fetches the settings data' do
+      expect(settings_data['mergePipelinesEnabled']).to eql project.ci_cd_settings.merge_pipelines_enabled?
+      expect(settings_data['mergeTrainsEnabled']).to eql project.ci_cd_settings.merge_trains_enabled?
+      expect(settings_data['keepLatestArtifact']).to eql project.ci_keep_latest_artifact?
+    end
   end
 end

spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'CiCdSettingsUpdate' do
+  include GraphqlHelpers
+
+  let_it_be(:project) { create(:project, ci_keep_latest_artifact: true) }
+  let(:variables) { { full_path: project.full_path, keep_latest_artifact: false } }
+  let(:mutation) { graphql_mutation(:ci_cd_settings_update, variables) }
+
+  context 'when unauthorized' do
+    let(:user) { create(:user) }
+
+    shared_examples 'unauthorized' do
+      it 'returns an error' do
+        post_graphql_mutation(mutation, current_user: user)
+
+        expect(graphql_errors).not_to be_empty
+      end
+    end
+
+    context 'when not a project member' do
+      it_behaves_like 'unauthorized'
+    end
+
+    context 'when a non-admin project member' do
+      before do
+        project.add_developer(user)
+      end
+
+      it_behaves_like 'unauthorized'
+    end
+  end
+
+  context 'when authorized' do
+    let_it_be(:user) { project.owner }
+
+    it 'updates ci cd settings' do
+      post_graphql_mutation(mutation, current_user: user)
+
+      project.reload
+
+      expect(response).to have_gitlab_http_status(:success)
+      expect(project.ci_keep_latest_artifact).to eq(false)
+    end
+
+    context 'when bad arguments are provided' do
+      let(:variables) { { full_path: '', keep_latest_artifact: false } }
+
+      it 'returns the errors' do
+        post_graphql_mutation(mutation, current_user: user)
+
+        expect(graphql_errors).not_to be_empty
+      end
+    end
+  end
+end