Merge branch 'feature/gb/configurable-github-status-name' into 'master'

Configurable GitHub static context for statuses integration

Closes #6696

See merge request gitlab-org/gitlab-ee!8235

doc/user/project/integrations/github.md

@@ -30,6 +30,19 @@ with `repo:status` access granted:
 1. Select the "Active" checkbox.
 1. Paste the token you've generated on GitHub
 1. Enter the path to your project on GitHub, such as `https://github.com/username/repository`
+1. Optionally check "Static status check names" checkbox to enable static status check names.
 1. Save or optionally click "Test Settings".
 
+#### Static / dynamic status check names
+
+Since GitLab 11.5 it is possible to opt-in to using static status check names.
+
+This makes it possible to mark these status checks as _Required_ on GitHub.
+If you check "Static status check names" checkbox on the integration page, your
+GitLab instance host name is going to be appended to a status check name,
+whereas in case of dynamic status check names, a branch name is going to be
+appended.
+
+Dynamic status check name is a default behavior.
+
 ![Configure GitHub Project Integration](img/github_configuration.png)

ee/app/controllers/concerns/ee/service_params.rb

@@ -5,7 +5,8 @@ module EE
       :multiproject_enabled,
       :pass_unstable,
       :project_name,
-      :repository_url
+      :repository_url,
+      :static_context
     ].freeze
 
     def allowed_service_params

ee/app/models/project_services/github_service.rb

@@ -3,6 +3,7 @@ class GithubService < Service
   include ActionView::Helpers::UrlHelper
 
   prop_accessor :token, :repository_url
+  boolean_accessor :static_context
 
   delegate :api_url, :owner, :repository_name, to: :remote_project
 
@@ -31,8 +32,20 @@ class GithubService < Service
 
   def fields
     [
-      { type: 'text', name: "token", required: true, placeholder: "e.g. 8d3f016698e...", help: 'Create a <a href="https://github.com/settings/tokens">personal access token</a> with  <code>repo:status</code> access granted and paste it here.'.html_safe },
-      { type: 'text', name: "repository_url", title: 'Repository URL', required: true, placeholder: 'e.g. https://github.com/owner/repository' }
+      { type: 'text',
+        name: "token",
+        required: true,
+        placeholder: "e.g. 8d3f016698e...",
+        help: 'Create a <a href="https://github.com/settings/tokens">personal access token</a> with  <code>repo:status</code> access granted and paste it here.'.html_safe },
+      { type: 'text',
+        name: "repository_url",
+        title: 'Repository URL',
+        required: true,
+        placeholder: 'e.g. https://github.com/owner/repository' },
+      { type: 'checkbox',
+        name: "static_context",
+        title: 'Static status check names',
+        help: 'GitHub status checks need static name in order to be marked as "required".' }
     ]
   end
 
@@ -51,7 +64,7 @@ class GithubService < Service
   def execute(data)
     return if disabled?
 
-    status_message = StatusMessage.from_pipeline_data(project, data)
+    status_message = StatusMessage.from_pipeline_data(project, self, data)
 
     update_status(status_message)
   end

ee/app/models/project_services/github_service/status_message.rb

@@ -4,8 +4,9 @@ class GithubService
 
     attr_reader :sha
 
-    def initialize(project, params)
+    def initialize(project, service, params)
       @project = project
+      @service = service
       @gitlab_status = params[:status]
       @detailed_status = params[:detailed_status]
       @pipeline_id = params[:id]
@@ -14,7 +15,7 @@ class GithubService
     end
 
     def context
-      "ci/gitlab/#{@ref_name}".truncate(255)
+      context_name.truncate(255)
     end
 
     def description
@@ -50,8 +51,18 @@ class GithubService
       }
     end
 
-    def self.from_pipeline_data(project, data)
-      new(project, data[:object_attributes])
+    def self.from_pipeline_data(project, service, data)
+      new(project, service, data[:object_attributes])
+    end
+
+    private
+
+    def context_name
+      if @service.static_context?
+        "ci/gitlab/#{::Gitlab.config.gitlab.host}"
+      else
+        "ci/gitlab/#{@ref_name}"
+      end
     end
   end
 end

ee/changelogs/unreleased/feature-gb-configurable-github-status-name.yml

+---
+title: Configurable GitHub static context for statuses integration
+merge_request: 8235
+author:
+type: added

ee/spec/models/project_services/github_service/status_message_spec.rb

@@ -4,17 +4,22 @@ describe GithubService::StatusMessage do
   include Rails.application.routes.url_helpers
 
   let(:project) { double(:project, namespace: "me", to_s: 'example_project') }
+  let(:service) { double(:service, static_context?: false) }
+
+  before do
+    stub_config_setting(host: 'instance-host')
+  end
 
   describe '#description' do
     it 'includes human readable gitlab status' do
-      subject = described_class.new(project, detailed_status: 'passed')
+      subject = described_class.new(project, service, detailed_status: 'passed')
 
       expect(subject.description).to eq "Pipeline passed on GitLab"
     end
 
     it 'gets truncated to 140 chars' do
       dummy_text = 'a' * 500
-      subject = described_class.new(project, detailed_status: dummy_text)
+      subject = described_class.new(project, service, detailed_status: dummy_text)
 
       expect(subject.description.length).to eq 140
     end
@@ -36,7 +41,7 @@ describe GithubService::StatusMessage do
 
     with_them do
       it 'transforms status' do
-        subject = described_class.new(project, status: gitlab_status)
+        subject = described_class.new(project, service, status: gitlab_status)
 
         expect(subject.status).to eq github_status
       end
@@ -44,7 +49,7 @@ describe GithubService::StatusMessage do
   end
 
   describe '#status_options' do
-    let(:subject) { described_class.new(project, id: 1) }
+    let(:subject) { described_class.new(project, service, id: 1) }
 
     it 'includes context' do
       expect(subject.status_options[:context]).to be_a String
@@ -59,11 +64,40 @@ describe GithubService::StatusMessage do
     end
   end
 
+  describe '#context' do
+    subject do
+      described_class.new(project, service, ref: 'some-ref')
+    end
+
+    context 'when status context is supposed to be dynamic' do
+      before do
+        allow(service).to receive(:static_context?).and_return(false)
+      end
+
+      it 'appends pipeline reference to the status context' do
+        expect(subject.context).to eq 'ci/gitlab/some-ref'
+      end
+    end
+
+    context 'when status context is supposed to be static' do
+      before do
+        allow(service).to receive(:static_context?).and_return(true)
+      end
+
+      it 'appends instance hostname to the status context' do
+        expect(subject.context).to eq 'ci/gitlab/instance-host'
+      end
+    end
+  end
+
   describe '.from_pipeline_data' do
-    let(:pipeline) { create(:ci_pipeline) }
-    let(:project) { pipeline.project }
+    let(:project) { create(:project) }
+    let(:pipeline) { create(:ci_pipeline, ref: 'some-ref', project: project) }
     let(:sample_data) { Gitlab::DataBuilder::Pipeline.build(pipeline) }
-    let(:subject) { described_class.from_pipeline_data(project, sample_data) }
+
+    subject do
+      described_class.from_pipeline_data(project, service, sample_data)
+    end
 
     it 'builds an instance of GithubService::StatusMessage' do
       expect(subject).to be_a described_class
@@ -87,16 +121,30 @@ describe GithubService::StatusMessage do
       end
 
       specify 'context' do
-        expect(subject.context).to eq "ci/gitlab/#{pipeline.ref}"
+        expect(subject.context).to eq "ci/gitlab/some-ref"
       end
 
-      context 'blocked pipeline' do
+      context 'when pipeline is blocked' do
         let(:pipeline) { create(:ci_pipeline, :blocked) }
 
         it 'uses human readable status which can be used in a sentence' do
           expect(subject.description). to eq 'Pipeline waiting for manual action on GitLab'
         end
       end
+
+      context 'when static context has been configured' do
+        before do
+          allow(service).to receive(:static_context?).and_return(true)
+        end
+
+        subject do
+          described_class.from_pipeline_data(project, service, sample_data)
+        end
+
+        it 'appends instance name to the context name' do
+          expect(subject.context).to eq 'ci/gitlab/instance-host'
+        end
+      end
     end
   end
 end

ee/spec/models/project_services/github_service_spec.rb

@@ -80,7 +80,10 @@ describe GithubService do
     it 'uses StatusMessage to build message' do
       allow(subject).to receive(:update_status)
 
-      expect(GithubService::StatusMessage).to receive(:from_pipeline_data).with(project, pipeline_sample_data).and_return(status_message)
+      expect(GithubService::StatusMessage)
+        .to receive(:from_pipeline_data)
+        .with(project, subject, pipeline_sample_data)
+        .and_return(status_message)
 
       subject.execute(pipeline_sample_data)
     end