diff --git a/app/models/concerns/integrations/has_issue_tracker_fields.rb b/app/models/concerns/integrations/has_issue_tracker_fields.rb
new file mode 100644
index 0000000000000000000000000000000000000000..b1def38d0193ac1617a1c0fccd453d98a561483c
--- /dev/null
+++ b/app/models/concerns/integrations/has_issue_tracker_fields.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module Integrations
+ module HasIssueTrackerFields
+ extend ActiveSupport::Concern
+
+ included do
+ field :project_url,
+ required: true,
+ storage: :data_fields,
+ title: -> { _('Project URL') },
+ help: -> { s_('IssueTracker|The URL to the project in the external issue tracker.') }
+
+ field :issues_url,
+ required: true,
+ storage: :data_fields,
+ title: -> { s_('IssueTracker|Issue URL') },
+ help: -> do
+ format s_('IssueTracker|The URL to view an issue in the external issue tracker. Must contain %{colon_id}.'),
+ colon_id: ':id'.html_safe
+ end
+
+ field :new_issue_url,
+ required: true,
+ storage: :data_fields,
+ title: -> { s_('IssueTracker|New issue URL') },
+ help: -> { s_('IssueTracker|The URL to create an issue in the external issue tracker.') }
+ end
+ end
+end
diff --git a/app/models/integration.rb b/app/models/integration.rb
index fb44d90139ae9966d7990a3596a787a7cb59e8e4..3cf71f46420f6ea19d3b8141f3dc5c59c74627ff 100644
--- a/app/models/integration.rb
+++ b/app/models/integration.rb
@@ -122,6 +122,39 @@ class Integration < ApplicationRecord
scope :alert_hooks, -> { where(alert_events: true, active: true) }
scope :deployment, -> { where(category: 'deployment') }
+ class << self
+ private
+
+ attr_writer :field_storage
+
+ def field_storage
+ @field_storage || :properties
+ end
+ end
+
+ # :nocov: Tested on subclasses.
+ def self.field(name, storage: field_storage, **attrs)
+ fields << ::Integrations::Field.new(name: name, **attrs)
+
+ case storage
+ when :properties
+ prop_accessor(name)
+ when :data_fields
+ data_field(name)
+ else
+ raise ArgumentError, "Unknown field storage: #{storage}"
+ end
+ end
+ # :nocov:
+
+ def self.fields
+ @fields ||= []
+ end
+
+ def fields
+ self.class.fields
+ end
+
# Provide convenient accessor methods for each serialized property.
# Also keep track of updated properties in a similar way as ActiveModel::Dirty
def self.prop_accessor(*args)
@@ -395,11 +428,6 @@ def to_param
self.class.to_param
end
- def fields
- # implement inside child
- []
- end
-
def sections
[]
end
diff --git a/app/models/integrations/base_issue_tracker.rb b/app/models/integrations/base_issue_tracker.rb
index 71329f34dc1d58b02ef2b6a3b737a048b7c6bb45..458d0199e7a40a06db90aa7c6fcf85779aabcece 100644
--- a/app/models/integrations/base_issue_tracker.rb
+++ b/app/models/integrations/base_issue_tracker.rb
@@ -4,10 +4,6 @@ module Integrations
class BaseIssueTracker < Integration
validate :one_issue_tracker, if: :activated?, on: :manual_change
- # TODO: we can probably just delegate as part of
- # https://gitlab.com/gitlab-org/gitlab/issues/29404
- data_field :project_url, :issues_url, :new_issue_url
-
default_value_for :category, 'issue_tracker'
before_validation :handle_properties
@@ -72,14 +68,6 @@ def issue_path(iid)
issue_url(iid)
end
- def fields
- [
- { type: 'text', name: 'project_url', title: _('Project URL'), help: s_('IssueTracker|The URL to the project in the external issue tracker.'), required: true },
- { type: 'text', name: 'issues_url', title: s_('IssueTracker|Issue URL'), help: s_('IssueTracker|The URL to view an issue in the external issue tracker. Must contain %{colon_id}.') % { colon_id: ':id'.html_safe }, required: true },
- { type: 'text', name: 'new_issue_url', title: s_('IssueTracker|New issue URL'), help: s_('IssueTracker|The URL to create an issue in the external issue tracker.'), required: true }
- ]
- end
-
def initialize_properties
{}
end
diff --git a/app/models/integrations/bugzilla.rb b/app/models/integrations/bugzilla.rb
index 1b711264e1084617c0861e4daa62b68ed051703b..74e282f6848aaaa379349a00874b3248bca22fb9 100644
--- a/app/models/integrations/bugzilla.rb
+++ b/app/models/integrations/bugzilla.rb
@@ -2,6 +2,8 @@
module Integrations
class Bugzilla < BaseIssueTracker
+ include Integrations::HasIssueTrackerFields
+
validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated?
def title
diff --git a/app/models/integrations/custom_issue_tracker.rb b/app/models/integrations/custom_issue_tracker.rb
index 0299a50605f32c9f7d2cb1a3aea2d73f534783fe..3770e813eaaaeb3ae112add424fbf186f7137b26 100644
--- a/app/models/integrations/custom_issue_tracker.rb
+++ b/app/models/integrations/custom_issue_tracker.rb
@@ -2,6 +2,8 @@
module Integrations
class CustomIssueTracker < BaseIssueTracker
+ include HasIssueTrackerFields
+
validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated?
def title
diff --git a/app/models/integrations/ewm.rb b/app/models/integrations/ewm.rb
index c6ea989a03f351155d4326a0a62521a2906ceb47..1b86ef73c85199074c089e8761b2342345123524 100644
--- a/app/models/integrations/ewm.rb
+++ b/app/models/integrations/ewm.rb
@@ -2,6 +2,8 @@
module Integrations
class Ewm < BaseIssueTracker
+ include HasIssueTrackerFields
+
validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated?
def self.reference_pattern(only_long: true)
diff --git a/app/models/integrations/field.rb b/app/models/integrations/field.rb
new file mode 100644
index 0000000000000000000000000000000000000000..49ab97677db4e024d0a4c8e67bb62e1643cac5c6
--- /dev/null
+++ b/app/models/integrations/field.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+module Integrations
+ class Field
+ SENSITIVE_NAME = %r/token|key|password|passphrase|secret/.freeze
+
+ ATTRIBUTES = %i[
+ section type placeholder required choices value checkbox_label
+ title help
+ non_empty_password_help
+ non_empty_password_title
+ api_only
+ ].freeze
+
+ attr_reader :name
+
+ def initialize(name:, type: 'text', api_only: false, **attributes)
+ @name = name.to_s.freeze
+
+ attributes[:type] = SENSITIVE_NAME.match?(@name) ? 'password' : type
+ attributes[:api_only] = api_only
+ @attributes = attributes.freeze
+ end
+
+ def [](key)
+ return name if key == :name
+
+ value = @attributes[key]
+ return value.call if value.respond_to?(:call)
+
+ value
+ end
+
+ def sensitive?
+ @attributes[:type] == 'password'
+ end
+
+ ATTRIBUTES.each do |name|
+ define_method(name) { self[name] }
+ end
+ end
+end
diff --git a/app/models/integrations/jira.rb b/app/models/integrations/jira.rb
index 8fc0a3461f763e78db3a25239cd2830e6b225d1e..7308c731bb09bb0c8fb39d793c8cccefcd929957 100644
--- a/app/models/integrations/jira.rb
+++ b/app/models/integrations/jira.rb
@@ -28,11 +28,6 @@ class Jira < BaseIssueTracker
# We should use username/password for Jira Server and email/api_token for Jira Cloud,
# for more information check: https://gitlab.com/gitlab-org/gitlab-foss/issues/49936.
- # TODO: we can probably just delegate as part of
- # https://gitlab.com/gitlab-org/gitlab/issues/29404
- data_field :username, :password, :url, :api_url, :jira_issue_transition_automatic, :jira_issue_transition_id, :project_key, :issues_enabled,
- :vulnerabilities_enabled, :vulnerabilities_issuetype
-
before_validation :reset_password
after_commit :update_deployment_type, on: [:create, :update], if: :update_deployment_type?
@@ -41,6 +36,44 @@ class Jira < BaseIssueTracker
all_details: 2
}
+ self.field_storage = :data_fields
+
+ field :url,
+ section: SECTION_TYPE_CONNECTION,
+ required: true,
+ title: -> { s_('JiraService|Web URL') },
+ help: -> { s_('JiraService|Base URL of the Jira instance.') },
+ placeholder: 'https://jira.example.com'
+
+ field :api_url,
+ section: SECTION_TYPE_CONNECTION,
+ title: -> { s_('JiraService|Jira API URL') },
+ help: -> { s_('JiraService|If different from Web URL.') }
+
+ field :username,
+ section: SECTION_TYPE_CONNECTION,
+ required: true,
+ title: -> { s_('JiraService|Username or Email') },
+ help: -> { s_('JiraService|Use a username for server version and an email for cloud version.') }
+
+ field :password,
+ section: SECTION_TYPE_CONNECTION,
+ required: true,
+ title: -> { s_('JiraService|Password or API token') },
+ non_empty_password_title: -> { s_('JiraService|Enter new password or API token') },
+ non_empty_password_help: -> { s_('JiraService|Leave blank to use your current password or API token.') },
+ help: -> { s_('JiraService|Use a password for server version and an API token for cloud version.') }
+
+ # TODO: we can probably just delegate as part of
+ # https://gitlab.com/gitlab-org/gitlab/issues/29404
+ # These fields are API only, so no field definition is required.
+ data_field :jira_issue_transition_automatic
+ data_field :jira_issue_transition_id
+ data_field :project_key
+ data_field :issues_enabled
+ data_field :vulnerabilities_enabled
+ data_field :vulnerabilities_issuetype
+
# When these are false GitLab does not create cross reference
# comments on Jira except when an issue gets transitioned.
def self.supported_events
@@ -127,45 +160,6 @@ def self.to_param
'jira'
end
- def fields
- [
- {
- section: SECTION_TYPE_CONNECTION,
- type: 'text',
- name: 'url',
- title: s_('JiraService|Web URL'),
- placeholder: 'https://jira.example.com',
- help: s_('JiraService|Base URL of the Jira instance.'),
- required: true
- },
- {
- section: SECTION_TYPE_CONNECTION,
- type: 'text',
- name: 'api_url',
- title: s_('JiraService|Jira API URL'),
- help: s_('JiraService|If different from Web URL.')
- },
- {
- section: SECTION_TYPE_CONNECTION,
- type: 'text',
- name: 'username',
- title: s_('JiraService|Username or Email'),
- help: s_('JiraService|Use a username for server version and an email for cloud version.'),
- required: true
- },
- {
- section: SECTION_TYPE_CONNECTION,
- type: 'password',
- name: 'password',
- title: s_('JiraService|Password or API token'),
- non_empty_password_title: s_('JiraService|Enter new password or API token'),
- non_empty_password_help: s_('JiraService|Leave blank to use your current password or API token.'),
- help: s_('JiraService|Use a password for server version and an API token for cloud version.'),
- required: true
- }
- ]
- end
-
def sections
[
{
@@ -194,17 +188,12 @@ def web_url(path = nil, **params)
url.to_s
end
- override :project_url
- def project_url
- web_url
- end
+ alias_method :project_url, :web_url
- override :issues_url
def issues_url
web_url('browse/:id')
end
- override :new_issue_url
def new_issue_url
web_url('secure/CreateIssue!default.jspa')
end
diff --git a/app/models/integrations/redmine.rb b/app/models/integrations/redmine.rb
index 3812033c3880eab2175cb07e884e07ddd9bbdded..bc2a64b084891c9130a7f82e6c3b2c2a4486b990 100644
--- a/app/models/integrations/redmine.rb
+++ b/app/models/integrations/redmine.rb
@@ -2,6 +2,8 @@
module Integrations
class Redmine < BaseIssueTracker
+ include Integrations::HasIssueTrackerFields
+
validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated?
def title
diff --git a/app/models/integrations/youtrack.rb b/app/models/integrations/youtrack.rb
index 254f0c6c69a14384516eaff9b0ad75813efac7d4..ab6e1da27f8404dc25c6f056b41480a5b836c9fa 100644
--- a/app/models/integrations/youtrack.rb
+++ b/app/models/integrations/youtrack.rb
@@ -2,6 +2,8 @@
module Integrations
class Youtrack < BaseIssueTracker
+ include Integrations::HasIssueTrackerFields
+
validates :project_url, :issues_url, presence: true, public_url: true, if: :activated?
# {PROJECT-KEY}-{NUMBER} Examples: YT-1, PRJ-1, gl-030
diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb
index 3d85bd245d901f669176278149d740268f7a9c17..48d8ba975b68aec48010e1fcedadfc5a8f78ff21 100644
--- a/spec/models/integration_spec.rb
+++ b/spec/models/integration_spec.rb
@@ -719,34 +719,63 @@
end
describe '#api_field_names' do
- let(:fake_integration) do
- Class.new(Integration) do
- def fields
- [
- { name: 'token' },
- { name: 'api_token' },
- { name: 'token_api' },
- { name: 'safe_token' },
- { name: 'key' },
- { name: 'api_key' },
- { name: 'password' },
- { name: 'password_field' },
- { name: 'some_safe_field' },
- { name: 'safe_field' },
- { name: 'url' },
- { name: 'trojan_horse', type: 'password' },
- { name: 'trojan_gift', type: 'gift' }
- ].shuffle
- end
+ shared_examples 'api field names' do
+ it 'filters out sensitive fields' do
+ safe_fields = %w[some_safe_field safe_field url trojan_gift]
+
+ expect(fake_integration.new).to have_attributes(
+ api_field_names: match_array(safe_fields)
+ )
end
end
- it 'filters out sensitive fields' do
- safe_fields = %w[some_safe_field safe_field url trojan_gift]
+ context 'when the class overrides #fields' do
+ let(:fake_integration) do
+ Class.new(Integration) do
+ def fields
+ [
+ { name: 'token' },
+ { name: 'api_token' },
+ { name: 'token_api' },
+ { name: 'safe_token' },
+ { name: 'key' },
+ { name: 'api_key' },
+ { name: 'password' },
+ { name: 'password_field' },
+ { name: 'some_safe_field' },
+ { name: 'safe_field' },
+ { name: 'url' },
+ { name: 'trojan_horse', type: 'password' },
+ { name: 'trojan_gift', type: 'gift' }
+ ].shuffle
+ end
+ end
+ end
- expect(fake_integration.new).to have_attributes(
- api_field_names: match_array(safe_fields)
- )
+ it_behaves_like 'api field names'
+ end
+
+ context 'when the class uses the field DSL' do
+ let(:fake_integration) do
+ Class.new(described_class) do
+ field :token
+ field :token
+ field :api_token
+ field :token_api
+ field :safe_token
+ field :key
+ field :api_key
+ field :password
+ field :password_field
+ field :some_safe_field
+ field :safe_field
+ field :url
+ field :trojan_horse, type: 'password'
+ field :trojan_gift, type: 'gift'
+ end
+ end
+
+ it_behaves_like 'api field names'
end
end
@@ -921,4 +950,75 @@ def fields
end
end
end
+
+ describe 'field DSL' do
+ let(:integration_type) do
+ Class.new(described_class) do
+ field :foo
+ field :foo_p, storage: :properties
+ field :foo_dt, storage: :data_fields
+
+ field :bar, type: 'password'
+ field :password
+
+ field :with_help,
+ help: -> { 'help' }
+
+ field :a_number,
+ type: 'number'
+ end
+ end
+
+ before do
+ allow(integration).to receive(:data_fields).and_return(data_fields)
+ end
+
+ let(:integration) { integration_type.new }
+ let(:data_fields) { Struct.new(:foo_dt).new }
+
+ it 'checks the value of storage' do
+ expect do
+ Class.new(described_class) { field(:foo, storage: 'bar') }
+ end.to raise_error(ArgumentError, /Unknown field storage/)
+ end
+
+ it 'provides prop_accessors' do
+ integration.foo = 1
+ expect(integration.foo).to eq 1
+ expect(integration.properties['foo']).to eq 1
+ expect(integration).to be_foo_changed
+
+ integration.foo_p = 2
+ expect(integration.foo_p).to eq 2
+ expect(integration.properties['foo_p']).to eq 2
+ expect(integration).to be_foo_p_changed
+ end
+
+ it 'provides data fields' do
+ integration.foo_dt = 3
+ expect(integration.foo_dt).to eq 3
+ expect(data_fields.foo_dt).to eq 3
+ expect(integration).to be_foo_dt_changed
+ end
+
+ it 'registers fields in the fields list' do
+ expect(integration.fields.pluck(:name)).to match_array %w[
+ foo foo_p foo_dt bar password with_help a_number
+ ]
+
+ expect(integration.api_field_names).to match_array %w[
+ foo foo_p foo_dt with_help a_number
+ ]
+ end
+
+ specify 'fields have expected attributes' do
+ expect(integration.fields).to include(
+ have_attributes(name: 'foo', type: 'text'),
+ have_attributes(name: 'bar', type: 'password'),
+ have_attributes(name: 'password', type: 'password'),
+ have_attributes(name: 'a_number', type: 'number'),
+ have_attributes(name: 'with_help', help: 'help')
+ )
+ end
+ end
end
diff --git a/spec/models/integrations/field_spec.rb b/spec/models/integrations/field_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..0d660c4a3ab32ef78cb27afdcbd8f576b5051608
--- /dev/null
+++ b/spec/models/integrations/field_spec.rb
@@ -0,0 +1,118 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Integrations::Field do
+ subject(:field) { described_class.new(**attrs) }
+
+ let(:attrs) { { name: nil } }
+
+ describe '#name' do
+ before do
+ attrs[:name] = :foo
+ end
+
+ it 'is stringified' do
+ expect(field.name).to eq 'foo'
+ expect(field[:name]).to eq 'foo'
+ end
+
+ context 'when not set' do
+ before do
+ attrs.delete(:name)
+ end
+
+ it 'complains' do
+ expect { field }.to raise_error(ArgumentError)
+ end
+ end
+ end
+
+ described_class::ATTRIBUTES.each do |name|
+ describe "##{name}" do
+ it "responds to #{name}" do
+ expect(field).to be_respond_to(name)
+ end
+
+ context 'when not set' do
+ before do
+ attrs.delete(name)
+ end
+
+ let(:have_correct_default) do
+ case name
+ when :api_only
+ be false
+ when :type
+ eq 'text'
+ else
+ be_nil
+ end
+ end
+
+ it 'has the correct default' do
+ expect(field[name]).to have_correct_default
+ expect(field.send(name)).to have_correct_default
+ end
+ end
+
+ context 'when set to a static value' do
+ before do
+ attrs[name] = :known
+ end
+
+ it 'is known' do
+ expect(field[name]).to eq(:known)
+ expect(field.send(name)).to eq(:known)
+ end
+ end
+
+ context 'when set to a dynamic value' do
+ before do
+ attrs[name] = -> { Time.current }
+ end
+
+ it 'is computed' do
+ start = Time.current
+
+ travel_to(start + 1.minute) do
+ expect(field[name]).to be_after(start)
+ expect(field.send(name)).to be_after(start)
+ end
+ end
+ end
+ end
+ end
+
+ describe '#sensitive' do
+ context 'when empty' do
+ it { is_expected.not_to be_sensitive }
+ end
+
+ context 'when a password field' do
+ before do
+ attrs[:type] = 'password'
+ end
+
+ it { is_expected.to be_sensitive }
+ end
+
+ %w[token api_token api_key secret_key secret_sauce password passphrase].each do |name|
+ context "when named #{name}" do
+ before do
+ attrs[:name] = name
+ end
+
+ it { is_expected.to be_sensitive }
+ end
+ end
+
+ context "when named url" do
+ before do
+ attrs[:name] = :url
+ end
+
+ it { is_expected.not_to be_sensitive }
+ end
+ end
+end