Merge branch 'ci/update-buildkite' into 'master'

CI: Update Buildkite service

See merge request gitlab-org/gitlab!33697

app/models/project_services/buildkite_service.rb

@@ -8,13 +8,32 @@ class BuildkiteService < CiService
   ENDPOINT = "https://buildkite.com"
 
   prop_accessor :project_url, :token
-  boolean_accessor :enable_ssl_verification
 
   validates :project_url, presence: true, public_url: true, if: :activated?
   validates :token, presence: true, if: :activated?
 
   after_save :compose_service_hook, if: :activated?
 
+  def self.supported_events
+    %w(push merge_request tag_push)
+  end
+
+  # This is a stub method to work with deprecated API response
+  # TODO: remove enable_ssl_verification after 14.0
+  # https://gitlab.com/gitlab-org/gitlab/-/issues/222808
+  def enable_ssl_verification
+    true
+  end
+
+  # Since SSL verification will always be enabled for Buildkite,
+  # we no longer needs to store the boolean.
+  # This is a stub method to work with deprecated API param.
+  # TODO: remove enable_ssl_verification after 14.0
+  # https://gitlab.com/gitlab-org/gitlab/-/issues/222808
+  def enable_ssl_verification=(_value)
+    self.properties.delete('enable_ssl_verification') # Remove unused key
+  end
+
   def webhook_url
     "#{buildkite_endpoint('webhook')}/deliver/#{webhook_token}"
   end
@@ -22,7 +41,7 @@ class BuildkiteService < CiService
   def compose_service_hook
     hook = service_hook || build_service_hook
     hook.url = webhook_url
-    hook.enable_ssl_verification = !!enable_ssl_verification
+    hook.enable_ssl_verification = true
     hook.save
   end
 
@@ -49,7 +68,7 @@ class BuildkiteService < CiService
   end
 
   def description
-    'Continuous integration and deployments'
+    'Buildkite is a platform for running fast, secure, and scalable continuous integration pipelines on your own infrastructure'
   end
 
   def self.to_param
@@ -60,15 +79,15 @@ class BuildkiteService < CiService
     [
       { type: 'text',
         name: 'token',
-        placeholder: 'Buildkite project GitLab token', required: true },
+        title: 'Integration Token',
+        help: 'This token will be provided when you create a Buildkite pipeline with a GitLab repository',
+        required: true },
 
       { type: 'text',
         name: 'project_url',
-        placeholder: "#{ENDPOINT}/example/project", required: true },
-
-      { type: 'checkbox',
-        name: 'enable_ssl_verification',
-        title: "Enable SSL verification" }
+        title: 'Pipeline URL',
+        placeholder: "#{ENDPOINT}/acme-inc/test-pipeline",
+        required: true }
     ]
   end
 

changelogs/unreleased/ci-update-buildkite.yml

+---
+title: "Update Buildkite Service for supported events, fields, and always verify SSL"
+merge_request: 33697
+author: "Juanito Fatas"
+type: changed

doc/api/services.md

@@ -229,8 +229,8 @@ Parameters:
 | Parameter | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
 | `token` | string | true | Buildkite project GitLab token |
-| `project_url` | string | true | `https://buildkite.com/example/project` |
-| `enable_ssl_verification` | boolean | false | Enable SSL verification |
+| `project_url` | string | true | Pipeline URL. For example, `https://buildkite.com/example/pipeline` |
+| `enable_ssl_verification` | boolean | false | DEPRECATED: This parameter has no effect since SSL verification will always be enabled |
 | `push_events` | boolean | false | Enable notifications for push events |
 
 ### Delete Buildkite service

lib/api/helpers/services_helpers.rb

@@ -247,13 +247,13 @@ module API
               required: true,
               name: :project_url,
               type: String,
-              desc: 'The buildkite project URL'
+              desc: 'The Buildkite pipeline URL'
             },
             {
               required: false,
               name: :enable_ssl_verification,
               type: Boolean,
-              desc: 'Enable SSL verification for communication'
+              desc: 'DEPRECATED: This parameter has no effect since SSL verification will always be enabled'
             }
         ],
           'campfire' => [

spec/controllers/projects/services_controller_spec.rb

@@ -82,7 +82,7 @@ RSpec.describe Projects::ServicesController do
             'active' => '1',
             'push_events' => '1',
             'token' => 'token',
-            'project_url' => 'http://test.com'
+            'project_url' => 'https://buildkite.com/organization/pipeline'
           }
         end
 

spec/models/project_services/buildkite_service_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe BuildkiteService, :use_clean_rails_memory_store_caching do
       project: project,
       properties: {
         service_hook: true,
-        project_url: 'https://buildkite.com/account-name/example-project',
+        project_url: 'https://buildkite.com/organization-name/example-pipeline',
         token: 'secret-sauce-webhook-token:secret-sauce-status-token'
       }
     )
@@ -45,11 +45,27 @@ RSpec.describe BuildkiteService, :use_clean_rails_memory_store_caching do
     end
   end
 
+  describe '.supported_events' do
+    it 'supports push, merge_request, and tag_push events' do
+      expect(service.supported_events).to eq %w(push merge_request tag_push)
+    end
+  end
+
   describe 'commits methods' do
     before do
       allow(project).to receive(:default_branch).and_return('default-brancho')
     end
 
+    it 'always activates SSL verification after saved' do
+      service.create_service_hook(enable_ssl_verification: false)
+
+      service.enable_ssl_verification = false
+      service.active = true
+
+      expect { service.save! }
+        .to change { service.service_hook.enable_ssl_verification }.from(false).to(true)
+    end
+
     describe '#webhook_url' do
       it 'returns the webhook url' do
         expect(service.webhook_url).to eq(
@@ -69,7 +85,7 @@ RSpec.describe BuildkiteService, :use_clean_rails_memory_store_caching do
     describe '#build_page' do
       it 'returns the correct build page' do
         expect(service.build_page('2ab7834c', nil)).to eq(
-          'https://buildkite.com/account-name/example-project/builds?commit=2ab7834c'
+          'https://buildkite.com/organization-name/example-pipeline/builds?commit=2ab7834c'
         )
       end
     end