Merge branch...

Merge branch '220789-10io-add-missing-attributes-to-graphql-container-expiration-policy-mutation' into 'master'

Add regex fields to the container expiration policy update mutation

See merge request gitlab-org/gitlab!34389

app/graphql/mutations/container_expiration_policies/update.rb

@@ -34,6 +34,16 @@ module Mutations
                required: false,
                description: copy_field_description(Types::ContainerExpirationPolicyType, :keep_n)
 
+      argument :name_regex,
+               Types::UntrustedRegexp,
+               required: false,
+               description: copy_field_description(Types::ContainerExpirationPolicyType, :name_regex)
+
+      argument :name_regex_keep,
+               Types::UntrustedRegexp,
+               required: false,
+               description: copy_field_description(Types::ContainerExpirationPolicyType, :name_regex_keep)
+
       field :container_expiration_policy,
             Types::ContainerExpirationPolicyType,
             null: true,

app/graphql/types/container_expiration_policy_type.rb

@@ -14,8 +14,8 @@ module Types
     field :older_than, Types::ContainerExpirationPolicyOlderThanEnum, null: true, description: 'Tags older that this will expire'
     field :cadence, Types::ContainerExpirationPolicyCadenceEnum, null: false, description: 'This container expiration policy schedule'
     field :keep_n, Types::ContainerExpirationPolicyKeepEnum, null: true, description: 'Number of tags to retain'
-    field :name_regex, GraphQL::STRING_TYPE, null: true, description: 'Tags with names matching this regex pattern will expire'
-    field :name_regex_keep, GraphQL::STRING_TYPE, null: true, description: 'Tags with names matching this regex pattern will be preserved'
+    field :name_regex, Types::UntrustedRegexp, null: true, description: 'Tags with names matching this regex pattern will expire'
+    field :name_regex_keep, Types::UntrustedRegexp, null: true, description: 'Tags with names matching this regex pattern will be preserved'
     field :next_run_at, Types::TimeType, null: true, description: 'Next time that this container expiration policy will get executed'
   end
 end

app/graphql/types/untrusted_regexp.rb

+# frozen_string_literal: true
+
+module Types
+  class UntrustedRegexp < Types::BaseScalar
+    description 'A regexp containing patterns sourced from user input'
+
+    def self.coerce_input(input_value, _)
+      return unless input_value
+
+      Gitlab::UntrustedRegexp.new(input_value)
+
+      input_value
+    rescue RegexpError => e
+      message = "#{input_value} is an invalid regexp: #{e.message}"
+      raise GraphQL::CoercionError, message
+    end
+
+    def self.coerce_result(ruby_value, _)
+      ruby_value.to_s
+    end
+  end
+end

changelogs/unreleased/220789-10io-add-missing-attributes-to-graphql-container-expiration-policy.yml

+---
+title: Add regex fields to the container expiration policy update mutation
+merge_request: 34389
+author:
+type: added

doc/api/graphql/reference/gitlab_schema.graphql

@@ -1248,12 +1248,12 @@ type ContainerExpirationPolicy {
   """
   Tags with names matching this regex pattern will expire
   """
-  nameRegex: String
+  nameRegex: UntrustedRegexp
 
   """
   Tags with names matching this regex pattern will be preserved
   """
-  nameRegexKeep: String
+  nameRegexKeep: UntrustedRegexp
 
   """
   Next time that this container expiration policy will get executed
@@ -12507,6 +12507,11 @@ enum TypeEnum {
   project
 }
 
+"""
+A regexp containing patterns sourced from user input
+"""
+scalar UntrustedRegexp
+
 """
 Autogenerated input type of UpdateAlertStatus
 """
@@ -12581,6 +12586,16 @@ input UpdateContainerExpirationPolicyInput {
   """
   keepN: ContainerExpirationPolicyKeepEnum
 
+  """
+  Tags with names matching this regex pattern will expire
+  """
+  nameRegex: UntrustedRegexp
+
+  """
+  Tags with names matching this regex pattern will be preserved
+  """
+  nameRegexKeep: UntrustedRegexp
+
   """
   Tags older that this will expire
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -3322,7 +3322,7 @@
               ],
               "type": {
                 "kind": "SCALAR",
-                "name": "String",
+                "name": "UntrustedRegexp",
                 "ofType": null
               },
               "isDeprecated": false,
@@ -3336,7 +3336,7 @@
               ],
               "type": {
                 "kind": "SCALAR",
-                "name": "String",
+                "name": "UntrustedRegexp",
                 "ofType": null
               },
               "isDeprecated": false,
@@ -37028,6 +37028,16 @@
           ],
           "possibleTypes": null
         },
+        {
+          "kind": "SCALAR",
+          "name": "UntrustedRegexp",
+          "description": "A regexp containing patterns sourced from user input",
+          "fields": null,
+          "inputFields": null,
+          "interfaces": null,
+          "enumValues": null,
+          "possibleTypes": null
+        },
         {
           "kind": "INPUT_OBJECT",
           "name": "UpdateAlertStatusInput",
@@ -37232,6 +37242,26 @@
               },
               "defaultValue": null
             },
+            {
+              "name": "nameRegex",
+              "description": "Tags with names matching this regex pattern will expire",
+              "type": {
+                "kind": "SCALAR",
+                "name": "UntrustedRegexp",
+                "ofType": null
+              },
+              "defaultValue": null
+            },
+            {
+              "name": "nameRegexKeep",
+              "description": "Tags with names matching this regex pattern will be preserved",
+              "type": {
+                "kind": "SCALAR",
+                "name": "UntrustedRegexp",
+                "ofType": null
+              },
+              "defaultValue": null
+            },
             {
               "name": "clientMutationId",
               "description": "A unique identifier for the client performing the mutation.",

doc/api/graphql/reference/index.md

@@ -217,8 +217,8 @@ A tag expiration policy designed to keep only the images that matter most
 | `createdAt` | Time! | Timestamp of when the container expiration policy was created |
 | `enabled` | Boolean! | Indicates whether this container expiration policy is enabled |
 | `keepN` | ContainerExpirationPolicyKeepEnum | Number of tags to retain |
-| `nameRegex` | String | Tags with names matching this regex pattern will expire |
-| `nameRegexKeep` | String | Tags with names matching this regex pattern will be preserved |
+| `nameRegex` | UntrustedRegexp | Tags with names matching this regex pattern will expire |
+| `nameRegexKeep` | UntrustedRegexp | Tags with names matching this regex pattern will be preserved |
 | `nextRunAt` | Time | Next time that this container expiration policy will get executed |
 | `olderThan` | ContainerExpirationPolicyOlderThanEnum | Tags older that this will expire |
 | `updatedAt` | Time! | Timestamp of when the container expiration policy was updated |

spec/graphql/types/container_expiration_policy_type_spec.rb

@@ -24,4 +24,20 @@ describe GitlabSchema.types['ContainerExpirationPolicy'] do
       is_expected.to have_graphql_type(Types::ContainerExpirationPolicyKeepEnum)
     end
   end
+
+  describe 'name_regex field' do
+    subject { described_class.fields['nameRegex'] }
+
+    it 'returns untrusted regexp type' do
+      is_expected.to have_graphql_type(Types::UntrustedRegexp)
+    end
+  end
+
+  describe 'name_regex_keep field' do
+    subject { described_class.fields['nameRegexKeep'] }
+
+    it 'returns untrusted regexp type' do
+      is_expected.to have_graphql_type(Types::UntrustedRegexp)
+    end
+  end
 end

spec/graphql/types/untrusted_regexp_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe GitlabSchema.types['UntrustedRegexp'] do
+  using RSpec::Parameterized::TableSyntax
+
+  specify { expect(described_class.graphql_name).to eq('UntrustedRegexp') }
+
+  specify { expect(described_class.description).to eq('A regexp containing patterns sourced from user input') }
+
+  describe '.coerce_input' do
+    subject { described_class.coerce_input(input, nil) }
+
+    where(:input, :expected_result) do
+      '.*'       | '.*'
+      '(.*)'     | '(.*)'
+      '[test*]+' | '[test*]+'
+      '*v1'      | :raise_error
+      '[test*'   | :raise_error
+      'test*+'   | :raise_error
+    end
+
+    with_them do
+      context "with input #{params[:input]}" do
+        if params[:expected_result] == :raise_error
+          it 'raises a coercion error' do
+            expect { subject }.to raise_error(GraphQL::CoercionError, /#{Regexp.quote(input)} is an invalid regexp/)
+          end
+        else
+          it { expect(subject).to eq(expected_result) }
+        end
+      end
+    end
+  end
+
+  describe '.coerce_result' do
+    subject { described_class.coerce_result(input, nil) }
+
+    where(:input, :expected_result) do
+      '1'  | '1'
+      1    | '1'
+      true | 'true'
+    end
+
+    with_them do
+      context "with input #{params[:input]}" do
+        it { expect(subject).to eq(expected_result) }
+      end
+    end
+  end
+end

spec/requests/api/graphql/mutations/container_expiration_policy/update_spec.rb

@@ -48,13 +48,48 @@ describe 'Updating the container expiration policy' do
     end
   end
 
-  RSpec.shared_examples 'updating the container expiration policy' do
+  RSpec.shared_examples 'rejecting invalid regex for' do |field_name|
+    context "for field #{field_name}" do
+      let_it_be(:invalid_regex) { '*production' }
+      let(:params) do
+        {
+          :project_path => project.full_path,
+          field_name => invalid_regex
+        }
+      end
+
+      it_behaves_like 'returning response status', :success
+
+      it_behaves_like 'not creating the container expiration policy'
+
+      it 'returns an error' do
+        subject
+
+        expect(graphql_errors.size).to eq(1)
+        expect(graphql_errors.first['message']).to include("#{invalid_regex} is an invalid regexp")
+      end
+    end
+  end
+
+  RSpec.shared_examples 'accepting the mutation request updating the container expiration policy' do
     it_behaves_like 'updating the container expiration policy attributes', mode: :update, from: { cadence: '1d', keep_n: 10, older_than: '90d' }, to: { cadence: '3month', keep_n: 100, older_than: '14d' }
 
     it_behaves_like 'returning a success'
+
+    it_behaves_like 'rejecting invalid regex for', :name_regex
+    it_behaves_like 'rejecting invalid regex for', :name_regex_keep
+  end
+
+  RSpec.shared_examples 'accepting the mutation request creating the container expiration policy' do
+    it_behaves_like 'creating the container expiration policy'
+
+    it_behaves_like 'returning a success'
+
+    it_behaves_like 'rejecting invalid regex for', :name_regex
+    it_behaves_like 'rejecting invalid regex for', :name_regex_keep
   end
 
-  RSpec.shared_examples 'denying access to container expiration policy' do
+  RSpec.shared_examples 'denying the mutation request' do
     it_behaves_like 'not creating the container expiration policy'
 
     it_behaves_like 'returning response status', :success
@@ -71,11 +106,11 @@ describe 'Updating the container expiration policy' do
 
     context 'with existing container expiration policy' do
       where(:user_role, :shared_examples_name) do
-        :maintainer | 'updating the container expiration policy'
-        :developer  | 'updating the container expiration policy'
-        :reporter   | 'denying access to container expiration policy'
-        :guest      | 'denying access to container expiration policy'
-        :anonymous  | 'denying access to container expiration policy'
+        :maintainer | 'accepting the mutation request updating the container expiration policy'
+        :developer  | 'accepting the mutation request updating the container expiration policy'
+        :reporter   | 'denying the mutation request'
+        :guest      | 'denying the mutation request'
+        :anonymous  | 'denying the mutation request'
       end
 
       with_them do
@@ -91,11 +126,11 @@ describe 'Updating the container expiration policy' do
       let_it_be(:project, reload: true) { create(:project, :without_container_expiration_policy) }
 
       where(:user_role, :shared_examples_name) do
-        :maintainer | 'creating the container expiration policy'
-        :developer  | 'creating the container expiration policy'
-        :reporter   | 'denying access to container expiration policy'
-        :guest      | 'denying access to container expiration policy'
-        :anonymous  | 'denying access to container expiration policy'
+        :maintainer | 'accepting the mutation request creating the container expiration policy'
+        :developer  | 'accepting the mutation request creating the container expiration policy'
+        :reporter   | 'denying the mutation request'
+        :guest      | 'denying the mutation request'
+        :anonymous  | 'denying the mutation request'
       end
 
       with_them do