Merge branch '22761-ci-allow-never-in-job-artifacts-expire_in' into 'master'

Resolve "CI: allow "never" in job:artifacts:expire_in"

See merge request gitlab-org/gitlab!38578

app/models/ci/job_artifact.rb

@@ -284,7 +284,7 @@ module Ci
     def expire_in=(value)
       self.expire_at =
         if value
-          ChronicDuration.parse(value)&.seconds&.from_now
+          ::Gitlab::Ci::Build::Artifacts::ExpireInParser.new(value).seconds_from_now
         end
     end
 

changelogs/unreleased/feat-artifact-expire-never-keyword.yml

+---
+title: Add support for never keyword in expire_in job artifacts
+merge_request: 38578
+author: Fabio Huser
+type: added

doc/ci/yaml/README.md

@@ -3184,8 +3184,11 @@ stored on GitLab. If the expiry time is not defined, it defaults to the
 [instance wide setting](../../user/admin_area/settings/continuous_integration.md#default-artifacts-expiration-core-only)
 (30 days by default).
 
-You can use the **Keep** button on the job page to override expiration and
-keep artifacts forever.
+To override the expiration time and keep artifacts forever:
+
+- Use the **Keep** button on the job page.
+- Set the value of `expire_in` to `never`. [Available](https://gitlab.com/gitlab-org/gitlab/-/issues/22761)
+  in GitLab 13.3 and later.
 
 After their expiry, artifacts are deleted hourly by default (via a cron job),
 and are not accessible anymore.
@@ -3200,6 +3203,7 @@ provided. Examples of valid values:
 - `6 mos 1 day`
 - `47 yrs 6 mos and 4d`
 - `3 weeks and 2 days`
+- `never`
 
 To expire artifacts 1 week after being uploaded:
 

lib/gitlab/ci/build/artifacts/expire_in_parser.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Build
+      module Artifacts
+        class ExpireInParser
+          def self.validate_duration(value)
+            new(value).validate_duration
+          end
+
+          def initialize(value)
+            @value = value
+          end
+
+          def validate_duration
+            return true if never?
+
+            parse
+          rescue ChronicDuration::DurationParseError
+            false
+          end
+
+          def seconds_from_now
+            parse&.seconds&.from_now
+          end
+
+          private
+
+          attr_reader :value
+
+          def parse
+            return if never?
+
+            ChronicDuration.parse(value)
+          end
+
+          def never?
+            value.to_s.casecmp('never') == 0
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/config/entry/artifacts.rb

@@ -42,7 +42,7 @@ module Gitlab
                 inclusion: { in: %w[on_success on_failure always],
                              message: 'should be on_success, on_failure ' \
                                       'or always' }
-              validates :expire_in, duration: true
+              validates :expire_in, duration: { parser: ::Gitlab::Ci::Build::Artifacts::ExpireInParser }
             end
           end
 

lib/gitlab/config/entry/legacy_validation_helpers.rb

@@ -6,17 +6,27 @@ module Gitlab
       module LegacyValidationHelpers
         private
 
-        def validate_duration(value)
-          value.is_a?(String) && ChronicDuration.parse(value)
+        def validate_duration(value, parser = nil)
+          return false unless value.is_a?(String)
+
+          if parser && parser.respond_to?(:validate_duration)
+            parser.validate_duration(value)
+          else
+            ChronicDuration.parse(value)
+          end
         rescue ChronicDuration::DurationParseError
           false
         end
 
-        def validate_duration_limit(value, limit)
+        def validate_duration_limit(value, limit, parser = nil)
           return false unless value.is_a?(String)
 
+          if parser && parser.respond_to?(:validate_duration_limit)
+            parser.validate_duration_limit(value, limit)
+          else
             ChronicDuration.parse(value).second.from_now <
               ChronicDuration.parse(limit).second.from_now
+          end
         rescue ChronicDuration::DurationParseError
           false
         end

lib/gitlab/config/entry/validators.rb

@@ -106,12 +106,12 @@ module Gitlab
           include LegacyValidationHelpers
 
           def validate_each(record, attribute, value)
-            unless validate_duration(value)
+            unless validate_duration(value, options[:parser])
               record.errors.add(attribute, 'should be a duration')
             end
 
             if options[:limit]
-              unless validate_duration_limit(value, options[:limit])
+              unless validate_duration_limit(value, options[:limit], options[:parser])
                 record.errors.add(attribute, 'should not exceed the limit')
               end
             end

spec/lib/gitlab/ci/build/artifacts/expire_in_parser_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Build::Artifacts::ExpireInParser do
+  describe '.validate_duration' do
+    subject { described_class.validate_duration(value) }
+
+    context 'with never' do
+      let(:value) { 'never' }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'with never value camelized' do
+      let(:value) { 'Never' }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'with a duration' do
+      let(:value) { '1 Day' }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'without a duration' do
+      let(:value) { 'something' }
+
+      it { is_expected.to be_falsy }
+    end
+  end
+
+  describe '#seconds_from_now' do
+    subject { described_class.new(value).seconds_from_now }
+
+    context 'with never' do
+      let(:value) { 'never' }
+
+      it { is_expected.to be_nil }
+    end
+
+    context 'with an empty string' do
+      let(:value) { '' }
+
+      it { is_expected.to be_nil }
+    end
+
+    context 'with a duration' do
+      let(:value) { '1 day' }
+
+      it { is_expected.to be_like_time(1.day.from_now) }
+    end
+  end
+end

spec/lib/gitlab/ci/yaml_processor_spec.rb

@@ -1559,6 +1559,21 @@ module Gitlab
           })
         end
 
+        it "returns artifacts with expire_in never keyword" do
+          config = YAML.dump({
+                                rspec: {
+                                  script: "rspec",
+                                  artifacts: { paths: ["releases/"], expire_in: "never" }
+                                }
+                              })
+
+          config_processor = Gitlab::Ci::YamlProcessor.new(config)
+          builds = config_processor.stage_builds_attributes("test")
+
+          expect(builds.size).to eq(1)
+          expect(builds.first[:options][:artifacts][:expire_in]).to eq('never')
+        end
+
         %w[on_success on_failure always].each do |when_state|
           it "returns artifacts for when #{when_state}  defined" do
             config = YAML.dump({

spec/requests/api/ci/runner/jobs_artifacts_spec.rb

@@ -479,6 +479,16 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
                     expect(job.reload.artifacts_expire_at).to be_nil
                   end
                 end
+
+                context 'when value is never' do
+                  let(:expire_in) { 'never' }
+                  let(:default_artifacts_expire_in) { '5 days' }
+
+                  it 'does not set expire_in' do
+                    expect(response).to have_gitlab_http_status(:created)
+                    expect(job.reload.artifacts_expire_at).to be_nil
+                  end
+                end
               end
             end
           end

spec/services/ci/create_job_artifacts_service_spec.rb

@@ -73,7 +73,7 @@ RSpec.describe Ci::CreateJobArtifactsService do
           expect(metadata_artifact.expire_at).to be_within(1.minute).of(expected_expire_at)
         end
 
-        context 'when expire_in params is set' do
+        context 'when expire_in params is set to a specific value' do
           before do
             params.merge!('expire_in' => '2 hours')
           end
@@ -89,6 +89,23 @@ RSpec.describe Ci::CreateJobArtifactsService do
             expect(metadata_artifact.expire_at).to be_within(1.minute).of(expected_expire_at)
           end
         end
+
+        context 'when expire_in params is set to `never`' do
+          before do
+            params.merge!('expire_in' => 'never')
+          end
+
+          it 'sets expiration date according to the parameter' do
+            expected_expire_at = nil
+
+            expect(subject).to be_truthy
+            archive_artifact, metadata_artifact = job.job_artifacts.last(2)
+
+            expect(job.artifacts_expire_at).to eq(expected_expire_at)
+            expect(archive_artifact.expire_at).to eq(expected_expire_at)
+            expect(metadata_artifact.expire_at).to eq(expected_expire_at)
+          end
+        end
       end
     end