From ca520489ffe196b194843851148a3d0a17064957 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 15 Jun 2022 03:09:07 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/models/integration.rb | 2 +- app/models/integrations/field.rb | 7 +- .../import_release_authors_from_github.yml | 8 +++ db/docs/group_deploy_keys.yml | 4 +- db/docs/group_deploy_tokens.yml | 4 +- .../Security/Coverage-Fuzzing.gitlab-ci.yml | 4 +- .../importer/releases_importer.rb | 20 +++++- lib/gitlab/legacy_github_import/importer.rb | 4 ++ .../importer/releases_importer_spec.rb | 72 +++++++++++++++++-- .../legacy_github_import/importer_spec.rb | 22 ------ spec/models/integrations/field_spec.rb | 21 ++++-- 11 files changed, 124 insertions(+), 44 deletions(-) create mode 100644 config/feature_flags/development/import_release_authors_from_github.yml diff --git a/app/models/integration.rb b/app/models/integration.rb index c35713b4fc8..726e95b7cbf 100644 --- a/app/models/integration.rb +++ b/app/models/integration.rb @@ -144,7 +144,7 @@ class Integration < ApplicationRecord # :nocov: Tested on subclasses. def self.field(name, storage: field_storage, **attrs) - fields << ::Integrations::Field.new(name: name, **attrs) + fields << ::Integrations::Field.new(name: name, integration_class: self, **attrs) case storage when :properties diff --git a/app/models/integrations/field.rb b/app/models/integrations/field.rb index ca7833c1a56..cbda418755b 100644 --- a/app/models/integrations/field.rb +++ b/app/models/integrations/field.rb @@ -13,10 +13,11 @@ module Integrations exposes_secrets ].freeze - attr_reader :name + attr_reader :name, :integration_class - def initialize(name:, type: 'text', api_only: false, **attributes) + def initialize(name:, integration_class:, type: 'text', api_only: false, **attributes) @name = name.to_s.freeze + @integration_class = integration_class attributes[:type] = SECRET_NAME.match?(@name) ? 'password' : type attributes[:api_only] = api_only @@ -27,7 +28,7 @@ module Integrations return name if key == :name value = @attributes[key] - return value.call if value.respond_to?(:call) + return integration_class.class_exec(&value) if value.respond_to?(:call) value end diff --git a/config/feature_flags/development/import_release_authors_from_github.yml b/config/feature_flags/development/import_release_authors_from_github.yml new file mode 100644 index 00000000000..b0ddca12d87 --- /dev/null +++ b/config/feature_flags/development/import_release_authors_from_github.yml @@ -0,0 +1,8 @@ +--- +name: import_release_authors_from_github +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89692 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/343448 +milestone: '15.1' +type: development +group: group::release +default_enabled: false diff --git a/db/docs/group_deploy_keys.yml b/db/docs/group_deploy_keys.yml index c96b6fd0470..0e85102dbb9 100644 --- a/db/docs/group_deploy_keys.yml +++ b/db/docs/group_deploy_keys.yml @@ -3,7 +3,7 @@ table_name: group_deploy_keys classes: - GroupDeployKey feature_categories: -- secrets_management -description: TODO +- continuous_delivery +description: https://docs.gitlab.com/ee/user/project/deploy_keys/ introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/30886 milestone: '13.1' diff --git a/db/docs/group_deploy_tokens.yml b/db/docs/group_deploy_tokens.yml index 4b8a4d4598f..6b497f59285 100644 --- a/db/docs/group_deploy_tokens.yml +++ b/db/docs/group_deploy_tokens.yml @@ -3,7 +3,7 @@ table_name: group_deploy_tokens classes: - GroupDeployToken feature_categories: -- secrets_management -description: TODO +- continuous_delivery +description: https://docs.gitlab.com/ee/user/project/deploy_tokens/ introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23460 milestone: '12.8' diff --git a/lib/gitlab/ci/templates/Security/Coverage-Fuzzing.gitlab-ci.yml b/lib/gitlab/ci/templates/Security/Coverage-Fuzzing.gitlab-ci.yml index f7f016b5e57..d4b6a252b25 100644 --- a/lib/gitlab/ci/templates/Security/Coverage-Fuzzing.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Security/Coverage-Fuzzing.gitlab-ci.yml @@ -12,8 +12,8 @@ variables: # Which branch we want to run full fledged long running fuzzing jobs. # All others will run fuzzing regression COVFUZZ_BRANCH: "$CI_DEFAULT_BRANCH" - # This is using semantic version and will always download latest v2 gitlab-cov-fuzz release - COVFUZZ_VERSION: v2 + # This is using semantic version and will always download latest v3 gitlab-cov-fuzz release + COVFUZZ_VERSION: v3 # This is for users who have an offline environment and will have to replicate gitlab-cov-fuzz release binaries # to their own servers COVFUZZ_URL_PREFIX: "https://gitlab.com/gitlab-org/security-products/analyzers/gitlab-cov-fuzz/-/raw" diff --git a/lib/gitlab/github_import/importer/releases_importer.rb b/lib/gitlab/github_import/importer/releases_importer.rb index 64ec0251e54..7241e1ef703 100644 --- a/lib/gitlab/github_import/importer/releases_importer.rb +++ b/lib/gitlab/github_import/importer/releases_importer.rb @@ -27,7 +27,7 @@ module Gitlab def build(release) existing_tags.add(release.tag_name) - { + build_hash = { name: release.name, tag: release.tag_name, description: description_for(release), @@ -37,6 +37,12 @@ module Gitlab released_at: release.published_at || Time.current, project_id: project.id } + + if Feature.enabled?(:import_release_authors_from_github, project) + build_hash[:author_id] = fetch_author_id(release) + end + + build_hash end def each_release @@ -50,6 +56,18 @@ module Gitlab def object_type :release end + + private + + def fetch_author_id(release) + author_id, _author_found = user_finder.author_id_for(release) + + author_id + end + + def user_finder + @user_finder ||= GithubImport::UserFinder.new(project, client) + end end end end diff --git a/lib/gitlab/legacy_github_import/importer.rb b/lib/gitlab/legacy_github_import/importer.rb index fc5834613fd..4ddafbac4c6 100644 --- a/lib/gitlab/legacy_github_import/importer.rb +++ b/lib/gitlab/legacy_github_import/importer.rb @@ -63,6 +63,10 @@ module Gitlab # Gitea doesn't have a Release API yet # See https://github.com/go-gitea/gitea/issues/330 + # On re-enabling care should be taken to include releases `author_id` field and enable corresponding tests. + # See: + # 1) https://gitlab.com/gitlab-org/gitlab/-/issues/343448#note_985979730 + # 2) https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89694/diffs#dfc4a8141aa296465ea3c50b095a30292fb6ebc4_180_182 unless project.gitea_import? import_releases end diff --git a/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb b/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb index 6b3d18f20e9..b0f553dbef7 100644 --- a/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb @@ -9,6 +9,12 @@ RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter do let(:github_release_name) { 'Initial Release' } let(:created_at) { Time.new(2017, 1, 1, 12, 00) } let(:released_at) { Time.new(2017, 1, 1, 12, 00) } + let(:author) do + double( + login: 'User A', + id: 1 + ) + end let(:github_release) do double( @@ -17,11 +23,23 @@ RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter do name: github_release_name, body: 'This is my release', created_at: created_at, - published_at: released_at + published_at: released_at, + author: author ) end + def stub_email_for_github_username(user_name = 'User A', user_email = 'user@example.com') + allow_next_instance_of(Gitlab::GithubImport::UserFinder) do |instance| + allow(instance).to receive(:email_for_github_username) + .with(user_name).and_return(user_email) + end + end + describe '#execute' do + before do + stub_email_for_github_username + end + it 'imports the releases in bulk' do release_hash = { tag_name: '1.0', @@ -45,7 +63,8 @@ RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter do description: 'This is my release', created_at: created_at, updated_at: created_at, - published_at: nil + published_at: nil, + author: author ) expect(importer).to receive(:each_release).and_return([release_double]) @@ -61,6 +80,10 @@ RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter do end describe '#build_releases' do + before do + stub_email_for_github_username + end + it 'returns an Array containing release rows' do expect(importer).to receive(:each_release).and_return([github_release]) @@ -108,11 +131,15 @@ RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter do describe '#build' do let(:release_hash) { importer.build(github_release) } - it 'returns the attributes of the release as a Hash' do - expect(release_hash).to be_an_instance_of(Hash) - end - context 'the returned Hash' do + before do + stub_email_for_github_username + end + + it 'returns the attributes of the release as a Hash' do + expect(release_hash).to be_an_instance_of(Hash) + end + it 'includes the tag name' do expect(release_hash[:tag]).to eq('1.0') end @@ -137,6 +164,39 @@ RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter do expect(release_hash[:name]).to eq(github_release_name) end end + + context 'author_id attribute' do + it 'returns the Gitlab user_id when Github release author is found' do + # Stub user email which matches a Gitlab user. + stub_email_for_github_username('User A', project.users.first.email) + + # Disable cache read as the redis cache key can be set by other specs. + # https://gitlab.com/gitlab-org/gitlab/-/blob/88bffda004e0aca9c4b9f2de86bdbcc0b49f2bc7/lib/gitlab/github_import/user_finder.rb#L75 + # Above line can return different user when read from cache. + allow(Gitlab::Cache::Import::Caching).to receive(:read).and_return(nil) + + expect(release_hash[:author_id]).to eq(project.users.first.id) + end + + it 'returns ghost user when author is empty in Github release' do + allow(github_release).to receive(:author).and_return(nil) + + expect(release_hash[:author_id]).to eq(Gitlab::GithubImport.ghost_user_id) + end + + context 'when Github author is not found in Gitlab' do + let(:author) { double(login: 'octocat', id: 1 ) } + + before do + # Stub user email which does not match a Gitlab user. + stub_email_for_github_username('octocat', 'octocat@example.com') + end + + it 'returns project creator as author' do + expect(release_hash[:author_id]).to eq(project.creator_id) + end + end + end end describe '#each_release' do diff --git a/spec/lib/gitlab/legacy_github_import/importer_spec.rb b/spec/lib/gitlab/legacy_github_import/importer_spec.rb index e69edbe6dc0..1800b42160d 100644 --- a/spec/lib/gitlab/legacy_github_import/importer_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/importer_spec.rb @@ -260,28 +260,6 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do ) end - context 'when importing a GitHub project' do - let(:api_root) { 'https://api.github.com' } - let(:repo_root) { 'https://github.com' } - - subject { described_class.new(project) } - - it_behaves_like 'Gitlab::LegacyGithubImport::Importer#execute' - it_behaves_like 'Gitlab::LegacyGithubImport::Importer#execute an error occurs' - it_behaves_like 'Gitlab::LegacyGithubImport unit-testing' - - describe '#client' do - it 'instantiates a Client' do - allow(project).to receive(:import_data).and_return(double(credentials: credentials)) - expect(Gitlab::LegacyGithubImport::Client).to receive(:new).with( - credentials[:user] - ) - - subject.client - end - end - end - context 'when importing a Gitea project' do let(:api_root) { 'https://try.gitea.io/api/v1' } let(:repo_root) { 'https://try.gitea.io' } diff --git a/spec/models/integrations/field_spec.rb b/spec/models/integrations/field_spec.rb index c8caf831191..6b1ce7fcbde 100644 --- a/spec/models/integrations/field_spec.rb +++ b/spec/models/integrations/field_spec.rb @@ -5,7 +5,14 @@ require 'spec_helper' RSpec.describe ::Integrations::Field do subject(:field) { described_class.new(**attrs) } - let(:attrs) { { name: nil } } + let(:attrs) { { name: nil, integration_class: test_integration } } + let(:test_integration) do + Class.new(Integration) do + def self.default_placeholder + 'my placeholder' + end + end + end describe '#name' do before do @@ -68,11 +75,8 @@ RSpec.describe ::Integrations::Field do end context 'when set to a dynamic value' do - before do - attrs[name] = -> { Time.current } - end - it 'is computed' do + attrs[name] = -> { Time.current } start = Time.current travel_to(start + 1.minute) do @@ -80,6 +84,13 @@ RSpec.describe ::Integrations::Field do expect(field.send(name)).to be_after(start) end end + + it 'is executed in the class scope' do + attrs[name] = -> { default_placeholder } + + expect(field[name]).to eq('my placeholder') + expect(field.send(name)).to eq('my placeholder') + end end end end