Commit 149d4428 authored by Kerri Miller's avatar Kerri Miller Committed by Bob Van Landuyt

Validate diff actor against CODEOWNERS entries

This addresses an issue where we were mistakenly not checking the user,
but instead checking to see if any file paths from the diff changeset
matched against _any_ rule found in CODEOWNERS.
parent fe97ef3f
---
title: Validate actor against CODEOWNERS entries
merge_request:
author:
type: fixed
......@@ -23,7 +23,10 @@ module EE
lambda do |paths|
loader = ::Gitlab::CodeOwners::Loader.new(project, branch_name, paths)
assemble_error_msg_for_codeowner_matches(loader) if loader.entries.any?
return if loader.entries.blank?
return if loader.members.include?(change_access.user_access.user)
assemble_error_msg_for_codeowner_matches(loader)
end
end
......
......@@ -8,8 +8,12 @@ module Gitlab
@extractor = extractor
end
# Generate a list of all project members who are mentioned in the
# CODEOWNERS file, and load them to the matching entry.
#
def load_to(entries)
members = project.members_among(users)
entries.each do |entry|
entry.add_matching_users_from(members)
end
......
......@@ -36,21 +36,35 @@ describe Gitlab::Checks::DiffCheck do
end
before do
project.add_developer(code_owner)
allow(project.repository).to receive(:code_owners_blob)
.with(ref: codeowner_lookup_ref)
.and_return(codeowner_blob)
end
context "the MR contains a matching file path" do
it "return an error message" do
expect(subject.send(:validate_code_owners)
.call(["docs/CODEOWNERS", "README"])).not_to be_nil
let(:validation_result) do
subject.send(:validate_code_owners).call(["docs/CODEOWNERS", "README"])
end
context "and the user is not listed as a code owner" do
it "returns an error message" do
expect(validation_result).to include("Pushes to protected branches")
end
end
context "and the user is listed as a code owner" do
# `user` is set as the owner of the incoming change by the shared
# context found in 'push rules checks context'
let(:codeowner_content) { "* @#{user.username}" }
it "returns nil" do
expect(validation_result).to be_nil
end
end
end
context "the MR doesn't contain a matching file path" do
it "doesn't raise an exception" do
it "returns nil" do
expect(subject.send(:validate_code_owners)
.call(["docs/SAFE_FILE_NAME", "README"])).to be_nil
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment