From 6bf8de7f5cc0a1ebac3732b7081070560ad8f8e8 Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Wed, 13 Mar 2024 12:39:24 +0100 Subject: [PATCH] Align SCMWebhook payload data Gitlab payload now also contains the source/target repository name with the same key names at Github/Gitea. --- src/api/app/models/gitlab_payload/merge_request.rb | 4 +++- src/api/app/models/gitlab_payload/push.rb | 4 ++++ src/api/app/models/gitlab_payload/tag_push.rb | 2 ++ src/api/app/models/workflow/step.rb | 6 +----- .../app/models/workflow/step/branch_package_step.rb | 7 ++++--- src/api/app/models/workflow/step/set_flags.rb | 6 +----- .../services/workflows/yaml_to_workflows_service.rb | 3 +-- src/api/spec/models/workflow/step_spec.rb | 2 +- .../trigger_controller_service/scm_extractor_spec.rb | 12 +++++++++++- .../workflows/yaml_to_workflows_service_spec.rb | 1 + 10 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/api/app/models/gitlab_payload/merge_request.rb b/src/api/app/models/gitlab_payload/merge_request.rb index 73248d80c94..883ea16669d 100644 --- a/src/api/app/models/gitlab_payload/merge_request.rb +++ b/src/api/app/models/gitlab_payload/merge_request.rb @@ -8,6 +8,8 @@ def payload target_branch: webhook_payload.dig(:object_attributes, :target_branch), action: webhook_payload.dig(:object_attributes, :action), project_id: webhook_payload.dig(:object_attributes, :source_project_id), - path_with_namespace: webhook_payload.dig(:project, :path_with_namespace)) + path_with_namespace: webhook_payload.dig(:project, :path_with_namespace), + target_repository_full_name: webhook_payload.dig(:object_attributes, :target, :path_with_namespace), + source_repository_full_name: webhook_payload.dig(:object_attributes, :source, :path_with_namespace)) end end diff --git a/src/api/app/models/gitlab_payload/push.rb b/src/api/app/models/gitlab_payload/push.rb index 34433be84e0..7ea3c25b2ae 100644 --- a/src/api/app/models/gitlab_payload/push.rb +++ b/src/api/app/models/gitlab_payload/push.rb @@ -7,6 +7,10 @@ def payload target_branch: webhook_payload[:ref].sub('refs/heads/', ''), # We need this for Workflows::YAMLDownloader#download_url path_with_namespace: webhook_payload.dig(:project, :path_with_namespace), + # We need this for Workflow::Step#branch_request_content_github + source_repository_full_name: webhook_payload.dig(:project, :path_with_namespace), + # We need this for SCMStatusReporter#call + target_repository_full_name: webhook_payload.dig(:project, :path_with_namespace), # We need this for SCMStatusReporter#call project_id: webhook_payload[:project_id], # We need this for SCMWebhookEventValidator#valid_push_event diff --git a/src/api/app/models/gitlab_payload/tag_push.rb b/src/api/app/models/gitlab_payload/tag_push.rb index 0b51990f187..4ce8b67a077 100644 --- a/src/api/app/models/gitlab_payload/tag_push.rb +++ b/src/api/app/models/gitlab_payload/tag_push.rb @@ -9,6 +9,8 @@ def payload target_branch: webhook_payload[:after], # We need this for Workflows::YAMLDownloader#download_url path_with_namespace: webhook_payload.dig(:project, :path_with_namespace), + source_repository_full_name: webhook_payload.dig(:project, :path_with_namespace), + target_repository_full_name: webhook_payload.dig(:project, :path_with_namespace), # We need this for SCMWebhookEventValidator#valid_push_event ref: webhook_payload[:ref], # We need this for Workflow::Step#branch_request_content_{github,gitlab} diff --git a/src/api/app/models/workflow/step.rb b/src/api/app/models/workflow/step.rb index f777a4f9064..2537bbbbf8b 100644 --- a/src/api/app/models/workflow/step.rb +++ b/src/api/app/models/workflow/step.rb @@ -26,11 +26,7 @@ def target_project_name return nil unless scm_webhook.pull_request_event? - pr_subproject_name = if %w[github gitea].include?(scm_webhook.payload[:scm]) - scm_webhook.payload[:target_repository_full_name]&.tr('/', ':') - else - scm_webhook.payload[:path_with_namespace]&.tr('/', ':') - end + pr_subproject_name = scm_webhook.payload[:target_repository_full_name]&.tr('/', ':') "#{target_project_base_name}:#{pr_subproject_name}:PR-#{scm_webhook.payload[:pr_number]}" end diff --git a/src/api/app/models/workflow/step/branch_package_step.rb b/src/api/app/models/workflow/step/branch_package_step.rb index dcba9ecb2ea..aa0b1567c0e 100644 --- a/src/api/app/models/workflow/step/branch_package_step.rb +++ b/src/api/app/models/workflow/step/branch_package_step.rb @@ -105,6 +105,10 @@ def create_target_project project.store end + # FIXME: Just because the tar_scm service accepts different formats for the _branch_request file, we don't need to have code + # to generate those different formats. We can just generate one format, should be the gitlab format because it provides more + # flexibility regarding the URL. + # https://github.com/openSUSE/obs-service-tar_scm/blob/2319f50e741e058ad599a6890ac5c710112d5e48/TarSCM/tasks.py#L145 def branch_request_content case scm_webhook.payload[:scm] when 'github' @@ -118,9 +122,6 @@ def branch_request_content def branch_request_content_github { - # TODO: change to scm_webhook.payload[:action] - # when check_for_branch_request method in obs-service-tar_scm accepts other actions than 'opened' - # https://github.com/openSUSE/obs-service-tar_scm/blob/2319f50e741e058ad599a6890ac5c710112d5e48/TarSCM/tasks.py#L145 action: 'opened', pull_request: { head: { diff --git a/src/api/app/models/workflow/step/set_flags.rb b/src/api/app/models/workflow/step/set_flags.rb index 6a5486979ce..c4afdf4d121 100644 --- a/src/api/app/models/workflow/step/set_flags.rb +++ b/src/api/app/models/workflow/step/set_flags.rb @@ -59,11 +59,7 @@ def target_project_name(project_name:) return nil unless scm_webhook.pull_request_event? - pr_subproject_name = if scm_webhook.payload[:scm] == 'github' - scm_webhook.payload[:target_repository_full_name]&.tr('/', ':') - else - scm_webhook.payload[:path_with_namespace]&.tr('/', ':') - end + pr_subproject_name = scm_webhook.payload[:target_repository_full_name]&.tr('/', ':') "#{project_name}:#{pr_subproject_name}:PR-#{scm_webhook.payload[:pr_number]}" end diff --git a/src/api/app/services/workflows/yaml_to_workflows_service.rb b/src/api/app/services/workflows/yaml_to_workflows_service.rb index 76741fd21b9..b014ce7d762 100644 --- a/src/api/app/services/workflows/yaml_to_workflows_service.rb +++ b/src/api/app/services/workflows/yaml_to_workflows_service.rb @@ -35,8 +35,7 @@ def create_workflows end def parse_workflow_configuration(workflow_configuration) - target_repository_full_name = @scm_webhook.payload.values_at(:target_repository_full_name, :path_with_namespace).compact.first - scm_organization_name, scm_repository_name = target_repository_full_name.split('/') + scm_organization_name, scm_repository_name = @scm_webhook.payload.fetch(:target_repository_full_name).split('/') # The PR number is only present in webhook events for pull requests, so we have a default value in case someone doesn't use # this correctly. Here, we cannot inform users about this since we're processing the whole workflows file diff --git a/src/api/spec/models/workflow/step_spec.rb b/src/api/spec/models/workflow/step_spec.rb index 29286073f07..2500502dafb 100644 --- a/src/api/spec/models/workflow/step_spec.rb +++ b/src/api/spec/models/workflow/step_spec.rb @@ -124,7 +124,7 @@ def target_project_base_name scm: 'gitlab', event: 'Merge Request Hook', pr_number: 1, - path_with_namespace: 'openSUSE/repo123' + target_repository_full_name: 'openSUSE/repo123' } end diff --git a/src/api/spec/services/trigger_controller_service/scm_extractor_spec.rb b/src/api/spec/services/trigger_controller_service/scm_extractor_spec.rb index 91ea24bd8d7..316ea7e1b26 100644 --- a/src/api/spec/services/trigger_controller_service/scm_extractor_spec.rb +++ b/src/api/spec/services/trigger_controller_service/scm_extractor_spec.rb @@ -164,7 +164,9 @@ source_project_id: 26_212_710, source_branch: 'nuevo', target_branch: 'master', - action: 'open' + action: 'open', + source: { path_with_namespace: 'hans/test' }, + target: { path_with_namespace: 'eduardoj2/test' } }, action: 'opened' } @@ -177,7 +179,9 @@ commit_sha: '4b486afefa44177f23b4388d2147ae42407e7f64', pr_number: 3, source_branch: 'nuevo', + source_repository_full_name: 'hans/test', target_branch: 'master', + target_repository_full_name: 'eduardoj2/test', action: 'open', project_id: 26_212_710, path_with_namespace: 'eduardoj2/test', @@ -212,7 +216,9 @@ object_kind: 'push', http_url: 'https://gitlab.com/eduardoj2/test.git', commit_sha: '9e0ea1fd99c9000cbb8b8c9d28763d0ddace0b65', + source_repository_full_name: 'eduardoj2/test', target_branch: 'main/fix-bug', + target_repository_full_name: 'eduardoj2/test', project_id: 3, path_with_namespace: 'eduardoj2/test', event: 'Push Hook', @@ -248,8 +254,10 @@ http_url: 'https://gitlab.com/jane/doe.git', event: 'Tag Push Hook', api_endpoint: 'https://gitlab.com', + source_repository_full_name: 'jane/doe', tag_name: 'release_abc', target_branch: '82b3d5ae55f7080f1e6022629cdb57bfae7cccc7', + target_repository_full_name: 'jane/doe', path_with_namespace: 'jane/doe', ref: 'refs/tags/release_abc', commit_sha: '82b3d5ae55f7080f1e6022629cdb57bfae7cccc7', @@ -281,7 +289,9 @@ commit_sha: nil, pr_number: nil, source_branch: nil, + source_repository_full_name: nil, target_branch: nil, + target_repository_full_name: nil, action: nil, project_id: nil, path_with_namespace: nil, diff --git a/src/api/spec/services/workflows/yaml_to_workflows_service_spec.rb b/src/api/spec/services/workflows/yaml_to_workflows_service_spec.rb index 900047d18db..cdd98caa413 100644 --- a/src/api/spec/services/workflows/yaml_to_workflows_service_spec.rb +++ b/src/api/spec/services/workflows/yaml_to_workflows_service_spec.rb @@ -24,6 +24,7 @@ action: 'open', project_id: 1, path_with_namespace: 'gitlabhq/gitlab-test', + target_repository_full_name: 'openSUSE/open-build-service', event: 'Merge Request Hook' } end