From d8c836046c71b162dd7907347546a971a9b7c96f Mon Sep 17 00:00:00 2001 From: Amir Hasanbasic <43892661+hamir-suspect@users.noreply.github.com> Date: Fri, 19 Sep 2025 12:54:48 +0200 Subject: [PATCH 1/4] feat(front): Support tags for tasks and task just run (#582) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description - Small improvement: search dropdown on `run_now` page if parameter has more than 10 possible values - Support for tags on schedulers/tasks - protobuf refresh - DB stub update fixes flaky browser tests in promotions: - [test when deployment targets are enabled when target has deployment target that blocks promotion ](https://semaphore.semaphoreci.com/projects/semaphore/flaky_tests/028249c9-90ad-38c3-9fa3-55ddf43195c1) - [test when deployment targets are enabled when target has deployment target that allows promotion](https://semaphore.semaphoreci.com/projects/semaphore/flaky_tests/3c1225ae-7037-3a06-be84-47fe841887fc) ## ✅ Checklist - [x] I have tested this change - [ ] This change requires documentation update --- front/assets/css/app-semaphore.css | 127 +++++++++++ front/assets/js/tasks/components/target.js | 83 +++++++- front/assets/js/tasks/index.js | 3 +- front/assets/js/tasks/just_run_form.js | 55 ++++- front/assets/js/workflow_view/switch.js | 2 +- .../assets/js/workflow_view/target_params.js | 6 +- front/lib/front/models/scheduler.ex | 95 ++++++++- .../controllers/schedulers_controller.ex | 52 ++++- .../templates/schedulers/_trigger.html.eex | 6 + .../schedulers/forms/_target.html.eex | 49 ++++- .../templates/schedulers/run.html.eex | 43 +++- .../schedulers/tasks/details.html.eex | 2 +- .../schedulers/tasks/header.html.eex | 2 +- front/lib/front_web/views/schedulers_view.ex | 11 +- .../lib/internal_api/periodic_scheduler.pb.ex | 24 +-- front/test/front/models/scheduler_test.exs | 201 ++++++++++++++++-- .../schedulers_controller_test.exs | 43 ++-- front/test/support/stubs/db.ex | 83 +++++--- front/test/support/stubs/scheduler.ex | 27 ++- 19 files changed, 752 insertions(+), 162 deletions(-) diff --git a/front/assets/css/app-semaphore.css b/front/assets/css/app-semaphore.css index 49988f443..f3aa69350 100644 --- a/front/assets/css/app-semaphore.css +++ b/front/assets/css/app-semaphore.css @@ -135,6 +135,133 @@ input[type="radio"][disabled] { opacity: 1; color: #96A0A6; } +/* TomSelect styling to match form-control exactly */ +.ts-control { + -moz-appearance: none; + -webkit-appearance: none; + outline: none; + font-size: 16px; + font-size: 1rem; + line-height: 1.5; + padding: 4px 10px; + padding-right: 24px; + border: 0; + border-radius: 6px; + color: #202b30; + background-color: #fff; + font-family: "Fakt Pro", -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, + Oxygen, Ubuntu, Cantarell, "Open Sans", "Helvetica Neue", sans-serif; + transition: background 0.1s ease, border-color 0.1s ease; + position: relative; + box-shadow: 0 0 0 1px rgba(0, 0, 0, 0.2), inset 0 -1px 1px 0 #e5e8ea; + background-position: right 8px center; + background-repeat: no-repeat; + background-image: url('data:image/svg+xml;utf8,'); +} + +.ts-control:focus, +.ts-control.focus { + outline: none; + box-shadow: 0 0 0 2px #00359f !important; + z-index: 4; +} + +.ts-control .ts-placeholder { + opacity: 1; + color: #96a0a6; +} + +/* Match disabled state */ +.ts-control.disabled, +.ts-control[disabled] { + color: #96a0a6; + background-color: #f7fafb; + cursor: not-allowed; + box-shadow: 0 0 0 1px rgba(0, 0, 0, 0.2); +} + +/* Hide TomSelect's default dropdown arrow since we're using the background image */ +.ts-control .ts-dropdown-trigger { + display: none; +} + +/* Ensure TomSelect dropdown matches form styling */ +.ts-dropdown { + border-radius: 6px; + border: 0; + box-shadow: rgba(0, 0, 0, 0.35) 0px 6px 30px 3px, + rgba(0, 0, 0, 0.08) 0px 0px 0px 1px; + font-family: "Fakt Pro", -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, + Oxygen, Ubuntu, Cantarell, "Open Sans", "Helvetica Neue", sans-serif; +} + +/* Style TomSelect options to match */ +.ts-dropdown .ts-option { + padding: 4px 10px; + font-size: 16px; + font-size: 1rem; +} + +.ts-dropdown .ts-option.selected { + background-color: #00359f; + color: white; +} + +.ts-dropdown .ts-option.active { + background-color: #cedcff; + color: #00359f; +} + +/* Ensure single value display matches */ +.ts-control.single .ts-control-input { + padding: 0; + margin: 0; + background: none; + border: none; + box-shadow: none; + font-size: 16px; + font-size: 1rem; + color: #202b30; + font-family: "Fakt Pro", -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, + Oxygen, Ubuntu, Cantarell, "Open Sans", "Helvetica Neue", sans-serif; +} + +/* Ensure input in editing mode maintains proper styling */ +.ts-control.single .ts-control-input input { + border: 1px solid #E5E8EA !important; + outline: none !important; + box-shadow: none !important; + background: none !important; + font-size: 16px; + font-size: 1rem; + color: #202B30; + font-family: 'Fakt Pro', -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, Cantarell, 'Open Sans', 'Helvetica Neue', sans-serif; +} + +/* When TomSelect wrapper is in active states, ensure the .ts-control maintains its border */ +.ts-wrapper.dropdown-active .ts-control, +.ts-wrapper.input-active .ts-control, +.ts-wrapper.focus .ts-control { + box-shadow: 0 0 0 1px rgba(0, 0, 0, .2), inset 0 -1px 1px 0 #E5E8EA !important; + border: 0 !important; + background-color: #FFF !important; + border-radius: 6px !important; +} + +/* Override focus state when dropdown is active to maintain blue focus border */ +.ts-wrapper.focus .ts-control { + box-shadow: 0 0 0 2px #00359f !important; +} + +/* Ensure TomSelect control always maintains form-control styling in all states */ +.ts-control, +.ts-wrapper.dropdown-active .ts-control, +.ts-wrapper.input-active .ts-control { + box-shadow: 0 0 0 1px rgba(0, 0, 0, .2), inset 0 -1px 1px 0 #E5E8EA !important; + background-color: #FFF !important; + border: 0 !important; + border-radius: 6px !important; +} textarea { display: block; min-height: 50px; diff --git a/front/assets/js/tasks/components/target.js b/front/assets/js/tasks/components/target.js index bf70b6812..ed2566616 100644 --- a/front/assets/js/tasks/components/target.js +++ b/front/assets/js/tasks/components/target.js @@ -1,15 +1,17 @@ export default { init(params) { - return new TargetComponent(params.branch, params.pipeline_file) + return new TargetComponent(params.branch || params.reference_name, params.pipeline_file, params.reference_type || 'branch') } } import { Validator, Rule } from './validation' class TargetComponent { - constructor(branch, pipelineFile) { + constructor(referenceName, pipelineFile, referenceType = 'branch') { + this.currentReferenceType = referenceType + this.validators = { - branch: new Validator('branch', branch, [ + branch: new Validator('branch', referenceName, [ new Rule((n) => n.length < 1, 'cannot be empty') ]), pipelineFile: new Validator('pipelineFile', pipelineFile, [ @@ -19,6 +21,10 @@ class TargetComponent { this.handleFieldChange('branch') this.handleFieldChange('pipelineFile') + this.handleReferenceTypeChange() + this.updateReferenceLabel() + this.updatePlaceholder() + this.updateReferenceTypeText() } isValid() { @@ -35,10 +41,75 @@ class TargetComponent { } handleFieldChange(fieldName) { - document - .querySelector(`[data-validation-input="${fieldName}"]`) - .addEventListener('input', (event) => { + const element = document.querySelector(`[data-validation-input="${fieldName}"]`) + if (element) { + element.addEventListener('input', (event) => { this.changeFieldValue(fieldName, event.target.value) }) + } + } + + handleReferenceTypeChange() { + const referenceTypeInputs = document.querySelectorAll('[data-validation-input="referenceType"]') + referenceTypeInputs.forEach(input => { + input.addEventListener('change', (event) => { + this.changeReferenceType(event.target.value) + }) + }) + } + + changeReferenceType(referenceType) { + this.currentReferenceType = referenceType + this.updateReferenceLabel() + this.updatePlaceholder() + this.updateReferenceTypeText() + } + + updateReferenceLabel() { + const labelElement = document.querySelector('[data-reference-label]') + if (labelElement) { + switch (this.currentReferenceType) { + case 'tag': + labelElement.textContent = 'Tag' + break + case 'pr': + labelElement.textContent = 'Pull Request' + break + default: + labelElement.textContent = 'Branch' + } + } + } + + updateReferenceTypeText() { + const textElement = document.querySelector('[data-reference-type-text]') + if (textElement) { + switch (this.currentReferenceType) { + case 'tag': + textElement.textContent = 'tag' + break + case 'pr': + textElement.textContent = 'pull request' + break + default: + textElement.textContent = 'branch' + } + } + } + + updatePlaceholder() { + const inputElement = document.querySelector('[data-validation-input="branch"]') + if (inputElement) { + switch (this.currentReferenceType) { + case 'tag': + inputElement.placeholder = 'e.g. v1.0.0' + break + case 'pr': + inputElement.placeholder = 'e.g. 123' + break + default: + inputElement.placeholder = 'e.g. master' + } + } } } diff --git a/front/assets/js/tasks/index.js b/front/assets/js/tasks/index.js index 66c1e2102..7701423a5 100644 --- a/front/assets/js/tasks/index.js +++ b/front/assets/js/tasks/index.js @@ -56,7 +56,8 @@ export class Tasks { static run() { return JustRunForm.init({ - branch: window.InjectedDataByBackend.Tasks.Branch, + referenceType: window.InjectedDataByBackend.Tasks.ReferenceType, + referenceName: window.InjectedDataByBackend.Tasks.ReferenceName, pipelineFile: window.InjectedDataByBackend.Tasks.PipelineFile, parameters: window.InjectedDataByBackend.Tasks.Parameters, }) diff --git a/front/assets/js/tasks/just_run_form.js b/front/assets/js/tasks/just_run_form.js index 705046fa1..b8cd8decd 100644 --- a/front/assets/js/tasks/just_run_form.js +++ b/front/assets/js/tasks/just_run_form.js @@ -1,5 +1,6 @@ import { Validator, Rule } from "./components/validation" +import { TargetParams } from '../workflow_view/target_params' export default class JustRunForm { static init(params) { @@ -7,7 +8,10 @@ export default class JustRunForm { } constructor(params) { - this.branchValidator = new Validator('branch', params.branch, [ + const referenceName = params.referenceName || 'Enter a branch or tag name…'; + const referenceType = params.referenceType || 'branch'; + + this.referenceNameValidator = new Validator('referenceName', referenceName, [ new Rule((v) => v.length < 1, 'cannot be empty') ]) this.pipelineFileValidator = new Validator('pipelineFile', params.pipelineFile, [ @@ -17,30 +21,55 @@ export default class JustRunForm { new Rule((v) => parameter.required && (!v || v.length < 1), 'cannot be empty') ])])) - this.handleBranchChange() + this.currentReferenceType = referenceType + this.handleReferenceTypeChange() + this.handleReferenceNameChange() this.handlePipelineFileChange() this.handleParameterChanges() this.handleSubmitButton() + this.updateReferenceLabel() + this.initializeParameterSelects() } renderAll() { - this.branchValidator.render() + this.referenceNameValidator.render() this.pipelineFileValidator.render() this.parameterValidators.forEach( pV => pV.render() ) } - handleBranchChange() { - const inputField = document.querySelector('[data-validation-input="branch"]') + handleReferenceTypeChange() { + const referenceTypeInputs = document.querySelectorAll('[data-validation-input="referenceType"]') + referenceTypeInputs.forEach(input => { + input.addEventListener('change', (event) => { + this.changeReferenceType(event.target.value) + }) + }) + } + + changeReferenceType(referenceType) { + this.currentReferenceType = referenceType + this.updateReferenceLabel() + } + + updateReferenceLabel() { + const labelElement = document.querySelector('[data-reference-label]') + if (labelElement) { + labelElement.textContent = this.currentReferenceType === 'tag' ? 'Tag' : 'Branch' + } + } + + handleReferenceNameChange() { + const inputField = document.querySelector('[data-validation-input="referenceName"]') if (inputField) { - inputField.addEventListener('input', (event) => { this.changeBranchValue(event.target.value) }) + inputField.addEventListener('input', (event) => { this.changeReferenceNameValue(event.target.value) }) } } - changeBranchValue(branchValue) { - this.branchValidator.setValue(branchValue) - this.branchValidator.render() + changeReferenceNameValue(referenceNameValue) { + this.referenceNameValidator.setValue(referenceNameValue) + this.referenceNameValidator.render() } handlePipelineFileChange() { @@ -75,7 +104,7 @@ export default class JustRunForm { validateForm() { const parameterValidators = Array.from(this.parameterValidators.values()) - return this.branchValidator.isValid() && + return this.referenceNameValidator.isValid() && this.pipelineFileValidator.isValid() && parameterValidators.every(parameterValidator => parameterValidator.isValid()) } @@ -84,7 +113,7 @@ export default class JustRunForm { const submitButton = document.querySelector('[data-action="submit-form"]') if (!submitButton) { return; } - submitButton.addEventListener('click', (event) => { + submitButton.addEventListener('click', () => { if (this.validateForm()) { document.forms[0].submit() } else { @@ -92,4 +121,8 @@ export default class JustRunForm { } }) } + + initializeParameterSelects() { + TargetParams.init('[data-parameter-select]') + } } diff --git a/front/assets/js/workflow_view/switch.js b/front/assets/js/workflow_view/switch.js index a3b7724a3..9ce9d1d37 100644 --- a/front/assets/js/workflow_view/switch.js +++ b/front/assets/js/workflow_view/switch.js @@ -33,7 +33,7 @@ export var Switch = { $("body").on("click", "[promote-button]", function(event) { Pollman.stop(); - TargetParams.init(); + TargetParams.init('[data-promotion-param-name]'); let button = $(event.currentTarget); let switchId = Switch.parentSwitch(button); let promotionTarget = Switch.parentPromotionTarget(button); diff --git a/front/assets/js/workflow_view/target_params.js b/front/assets/js/workflow_view/target_params.js index d4c98e78a..ee77c9c65 100644 --- a/front/assets/js/workflow_view/target_params.js +++ b/front/assets/js/workflow_view/target_params.js @@ -1,12 +1,12 @@ import TomSelect from 'tom-select' export var TargetParams = { - init: function () { - document.querySelectorAll('[data-promotion-param-name]').forEach((element) => { + init: function (selector = null) { + document.querySelectorAll(selector).forEach((element) => { if (!element.tomselect) { new TomSelect(element, { hidePlaceholder: true, - plugins: ['no_backspace_delete', 'dropdown_input'], + plugins: ['no_backspace_delete'], }) } }) diff --git a/front/lib/front/models/scheduler.ex b/front/lib/front/models/scheduler.ex index 05cc31682..7a647168a 100644 --- a/front/lib/front/models/scheduler.ex +++ b/front/lib/front/models/scheduler.ex @@ -26,7 +26,7 @@ defmodule Front.Models.Scheduler do alias InternalApi.Status, as: Status require Logger - @required_fields [:name, :branch, :pipeline_file, :recurring] + @required_fields [:name, :reference_name, :pipeline_file, :recurring] @default_page_args [page: 1, page_size: 10, page_query: ""] embedded_schema do field(:name, :string) @@ -34,7 +34,9 @@ defmodule Front.Models.Scheduler do field(:updated_at, :string) field(:project_id, :string) field(:recurring, :boolean) - field(:branch, :string) + field(:reference, :string) + field(:reference_type, :string, virtual: true) + field(:reference_name, :string, virtual: true) field(:at, :string) field(:parameters, :map) field(:pipeline_file, :string) @@ -143,7 +145,9 @@ defmodule Front.Models.Scheduler do use TypedStruct typedstruct do - field(:branch, :string) + field(:reference, :string) + field(:reference_type, :string) + field(:reference_name, :string) field(:pipeline_file, :string) field(:workflow_id, :string) field(:status, :string) @@ -159,8 +163,13 @@ defmodule Front.Models.Scheduler do end def construct(trigger) do + {reference_type, reference_name} = + Front.Models.Scheduler.parse_git_reference(trigger.reference) + %__MODULE__{ - branch: trigger.branch, + reference: trigger.reference, + reference_type: reference_type, + reference_name: reference_name, pipeline_file: trigger.pipeline_file, workflow_id: trigger.scheduled_workflow_id, status: trigger.scheduling_status, @@ -302,10 +311,17 @@ defmodule Front.Models.Scheduler do end def run_now(id, user_id, just_run_params \\ %{}, metadata \\ nil) do - run_now_params = just_run_params |> Map.merge(%{id: id, requester: user_id}) + run_now_params = + just_run_params + |> Map.merge(%{id: id, requester: user_id}) + |> Map.put(:reference, build_reference(just_run_params)) + |> Map.delete(:reference_type) + |> Map.delete(:reference_name) with {:ok, channel} <- GRPC.Stub.connect(api_endpoint()), request <- Util.Proto.deep_new!(RunNowRequest, run_now_params), + :ok <- + Logger.info("run_now_params: #{inspect(run_now_params)}, request: #{inspect(request)}"), {:ok, response = %RunNowResponse{status: %Status{code: 0}}} <- Stub.run_now(channel, request, options(metadata)), triggers <- response.triggers ++ [empty_trigger()] do @@ -417,9 +433,13 @@ defmodule Front.Models.Scheduler do all_data = Map.merge(form_data, context_data) scheduler_id = context_data[:id] || "" + # Build the reference field from reference_type and reference_name for gRPC service + reference = build_reference(all_data) + grpc_data = Map.put(all_data, :reference, reference) + with true <- changeset.valid?, {:ok, channel} <- GRPC.Stub.connect(api_endpoint()), - {:ok, request} <- Util.Proto.deep_new(PersistRequest, all_data), + {:ok, request} <- Util.Proto.deep_new(PersistRequest, grpc_data), {:ok, %PersistResponse{status: %Status{code: 0}, periodic: periodic}} <- Stub.persist(channel, request, options(metadata)) do {:ok, periodic.id} @@ -498,7 +518,7 @@ defmodule Front.Models.Scheduler do defp empty_trigger do %{ - branch: "", + reference: "", pipeline_file: "", scheduling_status: "", scheduled_at: %{seconds: 0}, @@ -514,6 +534,8 @@ defmodule Front.Models.Scheduler do end defp construct({raw_scheduler, latest_trigger}) do + {reference_type, reference_name} = parse_git_reference(raw_scheduler.reference) + %__MODULE__{ id: raw_scheduler.id, name: raw_scheduler.name, @@ -522,7 +544,9 @@ defmodule Front.Models.Scheduler do project_id: raw_scheduler.project_id, recurring: raw_scheduler.recurring, next: "not-added-yet", - branch: raw_scheduler.branch, + reference: raw_scheduler.reference, + reference_type: reference_type, + reference_name: reference_name, at: raw_scheduler.at, parameters: construct_parameters(raw_scheduler.parameters), pipeline_file: raw_scheduler.pipeline_file, @@ -566,6 +590,27 @@ defmodule Front.Models.Scheduler do [timeout: 30_000, metadata: metadata] end + defp build_reference(params) do + reference_type = Map.get(params, :reference_type, "branch") + reference_name = Map.get(params, :reference_name) || Map.get(params, :reference, "") + + # Format as proper Git reference + case String.downcase(to_string(reference_type)) do + "tag" -> + "refs/tags/#{String.trim(reference_name)}" + + "pr" -> + "refs/pull/#{String.trim(reference_name)}/head" + + "pull_request" -> + "refs/pull/#{String.trim(reference_name)}/head" + + _ -> + # Default to branch + "refs/heads/#{String.trim(reference_name)}" + end + end + defp parse_error_response_message(msg) do cond do msg =~ "parameters" -> @@ -574,8 +619,8 @@ defmodule Front.Models.Scheduler do msg =~ "name" -> %{errors: %{name: String.replace(msg, "Periodic with name", "Scheduler")}} - msg =~ "branch" -> - %{errors: %{branch: msg}} + msg =~ "reference" -> + %{errors: %{reference: msg}} msg =~ "Invalid cron expression in 'at' field" -> %{ @@ -593,4 +638,34 @@ defmodule Front.Models.Scheduler do %{errors: %{other: msg}} end end + + def parse_git_reference(nil), do: {"branch", ""} + def parse_git_reference(""), do: {"branch", ""} + + def parse_git_reference(reference) when is_binary(reference) do + cond do + String.starts_with?(reference, "refs/heads/") -> + {"branch", String.replace_prefix(reference, "refs/heads/", "")} + + String.starts_with?(reference, "refs/tags/") -> + {"tag", String.replace_prefix(reference, "refs/tags/", "")} + + String.starts_with?(reference, "refs/pull/") -> + pr_number = + reference + |> String.replace_prefix("refs/pull/", "") + |> String.replace_suffix("/head", "") + + {"pr", "PR ##{pr_number}"} + + String.contains?(reference, "/") -> + # If it looks like a full reference but doesn't match known patterns, treat as branch + parts = String.split(reference, "/") + {"branch", List.last(parts)} + + true -> + # Plain name, assume it's a branch + {"branch", reference} + end + end end diff --git a/front/lib/front_web/controllers/schedulers_controller.ex b/front/lib/front_web/controllers/schedulers_controller.ex index f05ba9832..4e205771e 100644 --- a/front/lib/front_web/controllers/schedulers_controller.ex +++ b/front/lib/front_web/controllers/schedulers_controller.ex @@ -170,7 +170,8 @@ defmodule FrontWeb.SchedulersController do |> Audit.add(description: "Added a periodic scheduler") |> Audit.add(resource_name: scheduler_input.name) |> Audit.metadata( - branch: scheduler_input.branch, + reference_name: scheduler_input.reference_name, + reference_type: scheduler_input.reference_type, project_name: context.project_name, pipeline_file: scheduler_input.pipeline_file ) @@ -310,7 +311,8 @@ defmodule FrontWeb.SchedulersController do |> Audit.add(resource_name: scheduler_input.name) |> Audit.add(resource_id: scheduler_id) |> Audit.metadata( - branch: scheduler_input.branch, + reference_name: scheduler_input.reference_name, + reference_type: scheduler_input.reference_type, project_name: context.project_name, pipeline_file: scheduler_input.pipeline_file ) @@ -679,7 +681,7 @@ defmodule FrontWeb.SchedulersController do defp validate_run_now_parameters(scheduler, parameters) do errors = [] - |> validate_run_now_field(parameters, :branch) + |> validate_run_now_field(parameters, :reference_name) |> validate_run_now_field(parameters, :pipeline_file) |> validate_run_now_parameter_values(parameters) @@ -691,7 +693,8 @@ defmodule FrontWeb.SchedulersController do errors: [], scheduler: scheduler, parameters: %{ - branch: parameters.branch, + reference_type: parameters.reference_type, + reference_name: parameters.reference_name, pipeline_file: parameters.pipeline_file, parameter_values: parameter_values } @@ -745,10 +748,13 @@ defmodule FrontWeb.SchedulersController do defp parse_form_input(params) do recurring? = params["recurring"] != "false" + reference_type = params["reference_type"] || "branch" + reference_name = String.trim(params["reference_name"] || "") %{ - at: if(recurring?, do: String.trim(params["at"]), else: ""), - branch: params["branch"], + at: get_at_value(params, recurring?), + reference_type: reference_type, + reference_name: reference_name, name: params["name"], description: params["description"] || "", pipeline_file: params["pipeline_file"], @@ -757,6 +763,9 @@ defmodule FrontWeb.SchedulersController do } end + defp get_at_value(params, true), do: String.trim(params["at"] || "") + defp get_at_value(_params, false), do: "" + defp parse_form_parameters(parameters) do parameters |> Map.values() |> Enum.map(&parse_form_input_parameter/1) end @@ -772,23 +781,42 @@ defmodule FrontWeb.SchedulersController do end defp parse_just_run_form_params(params, scheduler) do - branch = params["branch"] || scheduler.branch + reference_type = params["reference_type"] || "branch" + + reference_name = + String.trim(params["reference_name"] || params["branch"] || scheduler.reference_name || "") + pipeline_file = params["pipeline_file"] || scheduler.pipeline_file parameter_values = params["parameters"] || %{} parameters = merge_form_values_with_default_values(scheduler, parameter_values) - %{branch: branch, pipeline_file: pipeline_file, parameters: parameters} + + %{ + reference_type: reference_type, + reference_name: reference_name, + pipeline_file: pipeline_file, + parameters: parameters + } end defp parse_just_run_trigger_params(params, scheduler) do - branch = params["branch"] || scheduler.branch + reference_type = params["reference_type"] || "branch" + + reference_name = String.trim(params["reference_name"] || scheduler.reference_name || "") + pipeline_file = params["pipeline_file"] || scheduler.pipeline_file parameter_values = (params["parameters"] || %{}) |> Map.values() |> Enum.into(%{}, &{&1["name"], &1["value"]}) parameters = merge_form_values_with_default_values(scheduler, parameter_values) - %{branch: branch, pipeline_file: pipeline_file, parameters: parameters} + + %{ + reference_type: reference_type, + reference_name: reference_name, + pipeline_file: pipeline_file, + parameters: parameters + } end defp merge_form_values_with_default_values(scheduler, values) do @@ -890,6 +918,7 @@ defmodule FrontWeb.SchedulersController do _ -> # Periodic Scheduler returned expected validation error # this error is communicated within the form + Logger.error("Failed to #{action} the scheduler: #{inspect(message)}") "Failed to #{action} the scheduler." end @@ -898,7 +927,8 @@ defmodule FrontWeb.SchedulersController do defp compose_default_form_values(project_name) do %{ at: "0 0 * * *", - branch: "master", + reference_type: "branch", + reference_name: "master", name: "", description: "", recurring: true, diff --git a/front/lib/front_web/templates/schedulers/_trigger.html.eex b/front/lib/front_web/templates/schedulers/_trigger.html.eex index 86592461d..12307b482 100644 --- a/front/lib/front_web/templates/schedulers/_trigger.html.eex +++ b/front/lib/front_web/templates/schedulers/_trigger.html.eex @@ -2,6 +2,12 @@ <%= render FrontWeb.SchedulersView, "_workflow.html", conn: @conn, workflow: @trigger.workflow %>
+ <%= if @trigger.reference_type && @trigger.reference_name do %> +
+ <%= git_ref_icon(@trigger.reference_type) %> + <%= @trigger.reference_name %> +
+ <% end %>
<%= time_ago(@trigger.triggered_at) %>
diff --git a/front/lib/front_web/templates/schedulers/forms/_target.html.eex b/front/lib/front_web/templates/schedulers/forms/_target.html.eex index 1b4c786ca..91a5e0fdb 100644 --- a/front/lib/front_web/templates/schedulers/forms/_target.html.eex +++ b/front/lib/front_web/templates/schedulers/forms/_target.html.eex @@ -1,6 +1,7 @@ @@ -14,16 +15,42 @@
<% end %> -
- <%= label @form, :branch, class: "db b mb2" do %> - Branch (overridable) - <% end %> - <%= hidden_input @form, :id %> - <%= hidden_input @form, :unique_token %> - <%= text_input @form, :branch, value: @scheduler.branch, class: "form-control w-100 w-50-m", - "data-validation-input": "branch", placeholder: "e.g. master" %> -

Please ensure that selected branch exists in the repository.

-
+
+
+
<%= label @form, :reference_type, class: "db b mb2" do %>Reference Type<% end %>
+
+ + +
+
+
+ <%= label @form, :reference_name, class: "db b mb2" do %> + Branch Name (overridable) + <% end %> + <%= hidden_input @form, :id %> + <%= hidden_input @form, :unique_token %> + <% placeholder_text = cond do + @scheduler.reference_type == "tag" -> "e.g. v1.0.0" + true -> "e.g. master" + end %> + <%= text_input @form, :reference_name, name: "reference_name", value: @scheduler.reference_name, + class: "form-control w-100 w-50-m", "data-validation-input": "branch", + placeholder: placeholder_text %> +

Please ensure that selected branch exists in the repository.

+
+
diff --git a/front/lib/front_web/templates/schedulers/run.html.eex b/front/lib/front_web/templates/schedulers/run.html.eex index f1845b2ee..a1f71937b 100644 --- a/front/lib/front_web/templates/schedulers/run.html.eex +++ b/front/lib/front_web/templates/schedulers/run.html.eex @@ -3,7 +3,8 @@ window.InjectedDataByBackend.Tasks.Page = "run"; window.InjectedDataByBackend.Tasks.CanLoad = <%= @permissions["project.scheduler.view"] %> - window.InjectedDataByBackend.Tasks.Branch = "<%= @form_params.branch %>"; + window.InjectedDataByBackend.Tasks.ReferenceType = "<%= @form_params.reference_type || "branch" %>"; + window.InjectedDataByBackend.Tasks.ReferenceName = "<%= @form_params.reference_name %>"; window.InjectedDataByBackend.Tasks.PipelineFile = "<%= @form_params.pipeline_file %>"; window.InjectedDataByBackend.Tasks.Parameters = <%= raw injectable(@form_params.parameters) %>; @@ -36,18 +37,37 @@

What will run?

-
<%= label f, :branch, class: "f6 gray" do %>Branch<% end %>
-
- <%= text_input f, :branch, name: "branch", value: @form_params.branch, - class: "form-control w-100 #{error_css_class(@validation_errors, :branch)}", autocomplete: "off", - 'data-validation-input': "branch", placeholder: "Enter a branch…" %> -
+
<%= label f, :reference_type, class: "f6 gray" do %>Reference Type<% end %>
+
+ + +
+
<%= label f, :reference_name, class: "f6 gray" do %>Branch Name<% end %>
+
+ <%= text_input f, :reference_name, name: "reference_name", value: @form_params.reference_name, + class: "form-control w-100 #{error_css_class(@validation_errors, :reference_name)}", autocomplete: "off", + "data-validation-input": "referenceName", placeholder: "Enter a branch or tag name…" %> +
<%= label f, :pipeline_file, class: "f6 gray" do %>Pipeline<% end %>
<%= text_input f, :pipeline_file, name: "pipeline_file", value: @form_params.pipeline_file, class: "form-control w-100 #{error_css_class(@validation_errors, :pipeline_file)}", - 'data-validation-input': "pipelineFile", placeholder: "e.g. .semaphore/semaphore.yml" %> + "data-validation-input": "pipelineFile", placeholder: "e.g. .semaphore/semaphore.yml" %>
@@ -60,12 +80,13 @@ <%= hidden_input ff, :name, value: ff.data.name %> <%= if not Enum.empty?(ff.data.options) do %> <%= select ff, :value, ff.data.options, selected: ff.data.value, - 'data-validation-input': ff.data.name, + "data-validation-input": ff.data.name, + "data-parameter-select": length(ff.data.options) > 10, class: "db form-control mb3 mb0-m ml3 w-100", prompt: "Choose #{ff.data.name} value" %> <% else %> <%= text_input ff, :value, value: ff.data.value, - 'data-validation-input': ff.data.name, + "data-validation-input": ff.data.name, class: "form-control w-100 ml3", placeholder: "Enter value" %> <% end %> @@ -85,7 +106,7 @@
<%= link "Cancel", to: schedulers_path(@conn, :index, @project.name), class: "btn btn-secondary" %> - <%= submit type: "button", class: "btn btn-primary flex items-center justify-between", 'data-action': "submit-form" do %> + <%= submit type: "button", class: "btn btn-primary flex items-center justify-between", "data-action": "submit-form" do %> Runplay_circle <% end %>
diff --git a/front/lib/front_web/templates/schedulers/tasks/details.html.eex b/front/lib/front_web/templates/schedulers/tasks/details.html.eex index 1ed73f424..880849551 100644 --- a/front/lib/front_web/templates/schedulers/tasks/details.html.eex +++ b/front/lib/front_web/templates/schedulers/tasks/details.html.eex @@ -10,7 +10,7 @@ What will run:
- fork_right + <%= git_ref_icon(@scheduler.reference_type || "branch") %> <%= target_link(@project, @scheduler) %>
diff --git a/front/lib/front_web/templates/schedulers/tasks/header.html.eex b/front/lib/front_web/templates/schedulers/tasks/header.html.eex index da07d352d..9c7bd5505 100644 --- a/front/lib/front_web/templates/schedulers/tasks/header.html.eex +++ b/front/lib/front_web/templates/schedulers/tasks/header.html.eex @@ -8,7 +8,7 @@
- fork_right + <%= git_ref_icon(@scheduler.reference_type || "branch") %> <%= target_link(@project, @scheduler) %>
diff --git a/front/lib/front_web/views/schedulers_view.ex b/front/lib/front_web/views/schedulers_view.ex index 1fb733162..5f5dc0843 100644 --- a/front/lib/front_web/views/schedulers_view.ex +++ b/front/lib/front_web/views/schedulers_view.ex @@ -15,11 +15,14 @@ defmodule FrontWeb.SchedulersView do end def target_link(project, scheduler) do - branch = %{type: "branch", name: scheduler.branch, display_name: scheduler.branch} - branch_url = human_accessible_repository_url(project, branch) + ref_type = scheduler.reference_type || "branch" + ref_name = scheduler.reference_name || scheduler.branch || "" - Phoenix.HTML.Link.link("#{scheduler.branch} > #{scheduler.pipeline_file}", - to: branch_url <> "/" <> scheduler.pipeline_file, + ref_info = %{type: ref_type, name: ref_name, display_name: ref_name} + ref_url = human_accessible_repository_url(project, ref_info) + + Phoenix.HTML.Link.link("#{ref_name} > #{scheduler.pipeline_file}", + to: ref_url <> "/" <> scheduler.pipeline_file, class: "ml1 db link dark-gray underline-hover" ) end diff --git a/front/lib/internal_api/periodic_scheduler.pb.ex b/front/lib/internal_api/periodic_scheduler.pb.ex index 2422e3081..2bfcbd345 100644 --- a/front/lib/internal_api/periodic_scheduler.pb.ex +++ b/front/lib/internal_api/periodic_scheduler.pb.ex @@ -41,7 +41,7 @@ defmodule InternalApi.PeriodicScheduler.PersistRequest do organization_id: String.t(), project_name: String.t(), requester_id: String.t(), - branch: String.t(), + reference: String.t(), pipeline_file: String.t(), at: String.t(), parameters: [InternalApi.PeriodicScheduler.Periodic.Parameter.t()], @@ -56,7 +56,7 @@ defmodule InternalApi.PeriodicScheduler.PersistRequest do :organization_id, :project_name, :requester_id, - :branch, + :reference, :pipeline_file, :at, :parameters, @@ -71,7 +71,7 @@ defmodule InternalApi.PeriodicScheduler.PersistRequest do field(:organization_id, 6, type: :string) field(:project_name, 7, type: :string) field(:requester_id, 8, type: :string) - field(:branch, 9, type: :string) + field(:reference, 9, type: :string) field(:pipeline_file, 10, type: :string) field(:at, 11, type: :string) field(:parameters, 12, repeated: true, type: InternalApi.PeriodicScheduler.Periodic.Parameter) @@ -160,15 +160,15 @@ defmodule InternalApi.PeriodicScheduler.RunNowRequest do @type t :: %__MODULE__{ id: String.t(), requester: String.t(), - branch: String.t(), + reference: String.t(), pipeline_file: String.t(), parameter_values: [InternalApi.PeriodicScheduler.ParameterValue.t()] } - defstruct [:id, :requester, :branch, :pipeline_file, :parameter_values] + defstruct [:id, :requester, :reference, :pipeline_file, :parameter_values] field(:id, 1, type: :string) field(:requester, 2, type: :string) - field(:branch, 3, type: :string) + field(:reference, 3, type: :string) field(:pipeline_file, 4, type: :string) field(:parameter_values, 5, repeated: true, type: InternalApi.PeriodicScheduler.ParameterValue) end @@ -227,7 +227,7 @@ defmodule InternalApi.PeriodicScheduler.Periodic do id: String.t(), name: String.t(), project_id: String.t(), - branch: String.t(), + reference: String.t(), at: String.t(), pipeline_file: String.t(), requester_id: String.t(), @@ -246,7 +246,7 @@ defmodule InternalApi.PeriodicScheduler.Periodic do :id, :name, :project_id, - :branch, + :reference, :at, :pipeline_file, :requester_id, @@ -265,7 +265,7 @@ defmodule InternalApi.PeriodicScheduler.Periodic do field(:id, 1, type: :string) field(:name, 2, type: :string) field(:project_id, 3, type: :string) - field(:branch, 4, type: :string) + field(:reference, 4, type: :string) field(:at, 5, type: :string) field(:pipeline_file, 6, type: :string) field(:requester_id, 7, type: :string) @@ -308,7 +308,7 @@ defmodule InternalApi.PeriodicScheduler.Trigger do @type t :: %__MODULE__{ triggered_at: Google.Protobuf.Timestamp.t(), project_id: String.t(), - branch: String.t(), + reference: String.t(), pipeline_file: String.t(), scheduling_status: String.t(), scheduled_workflow_id: String.t(), @@ -321,7 +321,7 @@ defmodule InternalApi.PeriodicScheduler.Trigger do defstruct [ :triggered_at, :project_id, - :branch, + :reference, :pipeline_file, :scheduling_status, :scheduled_workflow_id, @@ -334,7 +334,7 @@ defmodule InternalApi.PeriodicScheduler.Trigger do field(:triggered_at, 1, type: Google.Protobuf.Timestamp) field(:project_id, 2, type: :string) - field(:branch, 3, type: :string) + field(:reference, 3, type: :string) field(:pipeline_file, 4, type: :string) field(:scheduling_status, 5, type: :string) field(:scheduled_workflow_id, 6, type: :string) diff --git a/front/test/front/models/scheduler_test.exs b/front/test/front/models/scheduler_test.exs index faa94a7c7..2477699d5 100644 --- a/front/test/front/models/scheduler_test.exs +++ b/front/test/front/models/scheduler_test.exs @@ -4,6 +4,7 @@ defmodule Front.Models.SchedulerTest do import Mock alias Front.Models.Scheduler, as: Subject + alias Front.Clients alias Google.Protobuf.Timestamp alias InternalApi.PeriodicScheduler.{ @@ -37,7 +38,8 @@ defmodule Front.Models.SchedulerTest do name: "sch-name", recurring: true, at: "addme", - branch: "master", + reference_type: "branch", + reference_name: "master", pipeline_file: "addme", project_name: "test-project", parameters: [ @@ -123,7 +125,7 @@ defmodule Front.Models.SchedulerTest do scheduler_desc(1, recurring: false, at: "", - branch: "", + reference: "", pipeline_file: "", parameters: [ %{ @@ -149,16 +151,36 @@ defmodule Front.Models.SchedulerTest do ] ) - with_mock Stub, - list: fn _c, _r, _o -> {:ok, response_scheduler} end, - latest_triggers: fn _c, _r, _o -> {:ok, response_triggers} end do + workflow_response = + InternalApi.PlumberWF.DescribeManyResponse.new( + status: + InternalApi.Status.new( + code: Google.Rpc.Code.value(:OK), + message: "" + ), + workflows: [] + ) + + with_mocks([ + {Stub, [], + [ + list: fn _c, _r, _o -> {:ok, response_scheduler} end, + latest_triggers: fn _c, _r, _o -> {:ok, response_triggers} end + ]}, + {Clients.Workflow, [], + [ + describe_many: fn _r -> {:ok, workflow_response} end + ]} + ]) do assert {:ok, list} = Subject.list(@project_id) assert list.entries == [ scheduler_model(1, recurring: false, at: "", - branch: "", + reference: "", + reference_type: "branch", + reference_name: "", pipeline_file: "", parameters: [ %{ @@ -200,7 +222,9 @@ defmodule Front.Models.SchedulerTest do at: "* * * * *", recurring: true, blocked: suspended(ind), - branch: "master", + reference: "refs/heads/master", + reference_type: "branch", + reference_name: "master", created_at: "", id: "id-#{ind}", inactive: paused(ind), @@ -231,7 +255,9 @@ defmodule Front.Models.SchedulerTest do status: status(ind), workflow_id: wf_id(ind), pipeline_file: "tests.yaml", - branch: "master" + reference: "refs/heads/master", + reference_type: "branch", + reference_name: "master" } ) ] @@ -243,7 +269,7 @@ defmodule Front.Models.SchedulerTest do Trigger.new( triggered_at: Timestamp.new(seconds: 0), project_id: "Test", - branch: "master", + reference: "refs/heads/master", pipeline_file: "tests.yaml", scheduling_status: status(ind), periodic_id: "id-#{ind}", @@ -278,7 +304,7 @@ defmodule Front.Models.SchedulerTest do paused: paused(ind), pause_toggled_by: ptb(ind), pause_toggled_at: Timestamp.new(seconds: 0) |> Map.take([:seconds, :nanos]), - branch: "master", + reference: "refs/heads/master", updated_at: Timestamp.new(seconds: 0) |> Map.take([:seconds, :nanos]), inserted_at: Timestamp.new(seconds: 0) |> Map.take([:seconds, :nanos]), parameters: [] @@ -327,7 +353,7 @@ defmodule Front.Models.SchedulerTest do scheduler_desc(1, recurring: false, at: "", - branch: "", + reference: "refs/heads/master", pipeline_file: "", parameters: [ %{ @@ -348,7 +374,9 @@ defmodule Front.Models.SchedulerTest do scheduler_model(1, recurring: false, at: "", - branch: "", + reference: "refs/heads/master", + reference_type: "branch", + reference_name: "master", pipeline_file: "", parameters: [ %{ @@ -622,7 +650,7 @@ defmodule Front.Models.SchedulerTest do {:error, %{ errors: %{ - branch: + other: "At least one regular workflow run on targeted branch is needed before periodic can be created." } }} @@ -714,7 +742,8 @@ defmodule Front.Models.SchedulerTest do assert Enum.all?(page.triggers, &match?(%Trigger{}, &1)) assert Enum.all?(page.triggers, &UUID.info!(&1.workflow_id)) - assert Enum.all?(page.triggers, &(&1.branch == "master")) + assert Enum.all?(page.triggers, &(&1.reference_name == "master")) + assert Enum.all?(page.triggers, &(&1.reference_type == "branch")) assert Enum.all?(page.triggers, &(&1.pipeline_file == ".semaphore/semaphore.yml")) assert Enum.all?(page.triggers, &is_nil(&1.triggerer)) assert Enum.all?(page.triggers, &is_nil(&1.workflow)) @@ -740,4 +769,148 @@ defmodule Front.Models.SchedulerTest do assert Enum.all?(page.triggers, &(&1.triggerer_avatar_url == avatar_url)) end end + + describe "reference building" do + test "persist/3 builds Git reference from reference_type and reference_name for branches" do + form_data = %{ + name: "test-scheduler", + recurring: true, + reference_type: "branch", + reference_name: "develop", + pipeline_file: ".semaphore/semaphore.yml" + } + + context_data = %{ + organization_id: @org_id, + requester_id: @user_id, + project_name: "test-project", + project_id: @project_id + } + + with_mock Stub, [:passthrough], + persist: fn _channel, request, _options -> + # Verify that the request contains the properly built Git reference + assert request.reference == "refs/heads/develop" + assert request.name == "test-scheduler" + assert request.pipeline_file == ".semaphore/semaphore.yml" + + {:ok, + %PersistResponse{ + status: %InternalApi.Status{code: 0}, + periodic: %Periodic{id: "new-scheduler-id"} + }} + end do + assert {:ok, "new-scheduler-id"} = Subject.persist(form_data, context_data) + end + end + + test "persist/3 builds Git reference from reference_type and reference_name for tags" do + form_data = %{ + name: "test-scheduler", + recurring: true, + reference_type: "tag", + reference_name: "v1.0.0", + pipeline_file: ".semaphore/semaphore.yml" + } + + context_data = %{ + organization_id: @org_id, + requester_id: @user_id, + project_name: "test-project", + project_id: @project_id + } + + with_mock Stub, [:passthrough], + persist: fn _channel, request, _options -> + # Verify that the request contains the properly built Git reference + assert request.reference == "refs/tags/v1.0.0" + assert request.name == "test-scheduler" + + {:ok, + %PersistResponse{ + status: %InternalApi.Status{code: 0}, + periodic: %Periodic{id: "new-scheduler-id"} + }} + end do + assert {:ok, "new-scheduler-id"} = Subject.persist(form_data, context_data) + end + end + + test "persist/3 defaults to branch when reference_type is missing" do + form_data = %{ + name: "test-scheduler", + recurring: true, + reference_name: "main", + pipeline_file: ".semaphore/semaphore.yml" + } + + context_data = %{ + organization_id: @org_id, + requester_id: @user_id, + project_name: "test-project", + project_id: @project_id + } + + with_mock Stub, [:passthrough], + persist: fn _channel, request, _options -> + # Should default to branch reference format + assert request.reference == "refs/heads/main" + + {:ok, + %PersistResponse{ + status: %InternalApi.Status{code: 0}, + periodic: %Periodic{id: "new-scheduler-id"} + }} + end do + assert {:ok, "new-scheduler-id"} = Subject.persist(form_data, context_data) + end + end + + test "run_now/4 builds Git reference from reference_type and reference_name" do + just_run_params = %{ + reference_type: "tag", + reference_name: "v2.1.0", + pipeline_file: ".semaphore/custom.yml" + } + + with_mock Stub, [:passthrough], + run_now: fn _channel, request, _options -> + # Verify that run_now builds the reference and removes the separate fields + assert request.reference == "refs/tags/v2.1.0" + assert request.pipeline_file == ".semaphore/custom.yml" + assert request.id == @scheduler_id + assert request.requester == @user_id + + # Should not include the separate reference fields + refute Map.has_key?(request, :reference_type) + refute Map.has_key?(request, :reference_name) + + {:ok, + %RunNowResponse{ + status: %InternalApi.Status{code: 0}, + periodic: %Periodic{ + id: @scheduler_id, + name: "test-scheduler", + description: "", + project_id: @project_id, + recurring: true, + reference: "refs/tags/v2.1.0", + pipeline_file: ".semaphore/custom.yml", + at: "", + parameters: [], + requester_id: @user_id, + updated_at: %Timestamp{seconds: 1_640_995_200}, + inserted_at: %Timestamp{seconds: 1_640_995_200}, + pause_toggled_at: nil, + pause_toggled_by: "", + paused: false, + suspended: false + }, + triggers: [] + }} + end do + assert {:ok, _scheduler} = Subject.run_now(@scheduler_id, @user_id, just_run_params) + end + end + end end diff --git a/front/test/front_web/controllers/schedulers_controller_test.exs b/front/test/front_web/controllers/schedulers_controller_test.exs index b95844555..5cac0b1c2 100644 --- a/front/test/front_web/controllers/schedulers_controller_test.exs +++ b/front/test/front_web/controllers/schedulers_controller_test.exs @@ -7,7 +7,8 @@ defmodule FrontWeb.SchedulersControllerTest do @raw_scheduler_form_params %{ at: "1 12,00 * * *", - branch: "master", + reference_type: "branch", + reference_name: "master", id: "888ea187-ssss-4f41-879d-a30a96faa01e", name: "first-scheduler", project_name_or_id: "ee2e6241-f30b-4892-a0d5-bd900b713430", @@ -565,7 +566,7 @@ defmodule FrontWeb.SchedulersControllerTest do %{project_name: project_name} do changeset = %{ errors: [ - branch: "Required. Cannot be empty.", + reference: "Required. Cannot be empty.", pipeline_file: "Required. Cannot be empty.", at: "Required. Cannot be empty." ], @@ -598,7 +599,7 @@ defmodule FrontWeb.SchedulersControllerTest do { Front.Models.Scheduler, [:passthrough], - [persist: fn _, _ -> {:error, %{errors: %{branch: "Error about the branch"}}} end] + [persist: fn _, _ -> {:error, %{errors: %{reference: "Error about the reference"}}} end] } ]) do conn = @@ -785,7 +786,7 @@ defmodule FrontWeb.SchedulersControllerTest do %{project_name: project_name, scheduler_id: scheduler_id} do changeset = %{ errors: [ - branch: "Required. Cannot be empty.", + reference: "Required. Cannot be empty.", pipeline_file: "Required. Cannot be empty.", at: "Required. Cannot be empty." ], @@ -846,7 +847,7 @@ defmodule FrontWeb.SchedulersControllerTest do test "correctly renders form with default values", %{conn: conn} do scheduler = prepare_scheduler_for_just_run( - branch: "develop", + reference: "refs/heads/develop", pipeline_file: "pipeline.yml", parameters: [ %{name: "PARAM1", default_value: "VALUE11"}, @@ -880,7 +881,7 @@ defmodule FrontWeb.SchedulersControllerTest do # imports default values assert html_response(conn, 200) =~ - "placeholder=\"Enter a branch…\" type=\"text\" value=\"\"" + "placeholder=\"Enter a branch or tag name…\" type=\"text\" value=\"\"" assert html_response(conn, 200) =~ "placeholder=\"e.g. .semaphore/semaphore.yml\" type=\"text\" value=\"\"" @@ -889,7 +890,7 @@ defmodule FrontWeb.SchedulersControllerTest do test "overrides default values with query parameters", %{conn: conn} do scheduler = prepare_scheduler_for_just_run( - branch: "master", + reference: "refs/heads/master", parameters: [ %{name: "PARAM1", default_value: "VALUE11"}, %{name: "PARAM2", options: ["VALUE21", "VALUE22"]}, @@ -900,7 +901,7 @@ defmodule FrontWeb.SchedulersControllerTest do conn = form_just_run(conn, scheduler, %{ - "branch" => "develop", + "reference_name" => "develop", "pipeline_file" => ".semaphore/semaphore.yml", "parameters" => %{ "PARAM1" => "VALUE12", @@ -911,7 +912,7 @@ defmodule FrontWeb.SchedulersControllerTest do # imports default values assert html_response(conn, 200) =~ - "placeholder=\"Enter a branch…\" type=\"text\" value=\"develop\"" + "placeholder=\"Enter a branch or tag name…\" type=\"text\" value=\"develop\"" assert html_response(conn, 200) =~ "placeholder=\"e.g. .semaphore/semaphore.yml\" type=\"text\" value=\".semaphore/semaphore.yml\"" @@ -959,7 +960,11 @@ defmodule FrontWeb.SchedulersControllerTest do test "when request fails because pipeline queue limit is reached it redirects to index with proper error message", %{conn: conn, project_name: project_name} do - scheduler = prepare_scheduler_for_just_run(branch: "master", pipeline_file: "pipeline.yml") + scheduler = + prepare_scheduler_for_just_run( + reference: "refs/heads/master", + pipeline_file: "pipeline.yml" + ) with_mocks([ { @@ -987,7 +992,7 @@ defmodule FrontWeb.SchedulersControllerTest do %{conn: conn, project_name: project_name} do scheduler = prepare_scheduler_for_just_run( - branch: "master", + reference: "refs/heads/master", pipeline_file: "pipeline.yml", parameters: [ %{name: "PARAM1", default_value: "VALUE11"}, @@ -999,7 +1004,7 @@ defmodule FrontWeb.SchedulersControllerTest do conn = trigger_just_run(conn, scheduler, %{ - "branch" => "develop", + "reference_name" => "develop", "pipeline_file" => "initial.yml", "parameters" => %{ "0" => %{"name" => "PARAM1", "value" => "VALUE1"}, @@ -1013,7 +1018,7 @@ defmodule FrontWeb.SchedulersControllerTest do assert get_flash(conn, :notice) == "Workflow started successfully." assert [trigger] = DB.find_all_by(:triggers, :periodic_id, scheduler.id) - assert trigger.api_model.branch == "develop" + assert trigger.api_model.reference == "refs/heads/develop" assert trigger.api_model.pipeline_file == "initial.yml" assert parameter_values = @@ -1031,7 +1036,7 @@ defmodule FrontWeb.SchedulersControllerTest do %{conn: conn, project_name: project_name} do scheduler = prepare_scheduler_for_just_run( - branch: "master", + reference: "refs/heads/master", pipeline_file: "pipeline.yml", parameters: [ %{name: "PARAM1", default_value: "VALUE11"}, @@ -1047,7 +1052,7 @@ defmodule FrontWeb.SchedulersControllerTest do assert get_flash(conn, :notice) == "Workflow started successfully." assert [trigger] = DB.find_all_by(:triggers, :periodic_id, scheduler.id) - assert trigger.api_model.branch == "master" + assert trigger.api_model.reference == "refs/heads/master" assert trigger.api_model.pipeline_file == "pipeline.yml" assert parameter_values = @@ -1061,7 +1066,7 @@ defmodule FrontWeb.SchedulersControllerTest do } == parameter_values end - test "fails if branch is not given", + test "fails if reference is not given", %{conn: conn, project_name: _project_name} do scheduler = prepare_scheduler_for_just_run( @@ -1081,7 +1086,7 @@ defmodule FrontWeb.SchedulersControllerTest do %{conn: conn, project_name: _project_name} do scheduler = prepare_scheduler_for_just_run( - branch: "master", + reference: "refs/heads/master", parameters: [ %{name: "PARAM1", default_value: "VALUE11"} ] @@ -1097,7 +1102,7 @@ defmodule FrontWeb.SchedulersControllerTest do %{conn: conn, project_name: _project_name} do scheduler = prepare_scheduler_for_just_run( - branch: "master", + reference: "refs/heads/master", pipeline_file: "pipeline.yml", parameters: [ %{name: "PARAM1", required: true} @@ -1119,7 +1124,7 @@ defmodule FrontWeb.SchedulersControllerTest do name: "JustRun Scheduler", recurring: false, at: "", - branch: "", + reference: "", pipeline_file: "", parameters: [] ] diff --git a/front/test/support/stubs/db.ex b/front/test/support/stubs/db.ex index 33bc51c40..badf8ad8d 100644 --- a/front/test/support/stubs/db.ex +++ b/front/test/support/stubs/db.ex @@ -90,12 +90,7 @@ defmodule Support.Stubs.DB do def insert(table, entry) do verify_insert!(table, entry) - new_table = all(table) ++ [entry] - new_tables = Map.update!(tables(), table, fn _ -> new_table end) - - State.update_tables(new_tables) - - entry + State.insert_entry(table, entry) end def upsert(table, entry, field \\ :id) do @@ -116,32 +111,13 @@ defmodule Support.Stubs.DB do def update(table, new_entry, field \\ :id) do verify_insert!(table, new_entry) - new_table = - all(table) - |> Enum.map(fn old_entry -> - if Map.get(old_entry, field) == Map.get(new_entry, field) do - new_entry - else - old_entry - end - end) - - new_tables = Map.update!(tables(), table, fn _ -> new_table end) - - State.update_tables(new_tables) - - new_entry + State.update_entry(table, new_entry, field) end def delete(table, callback) when is_function(callback) do verify_table_exists!(table) - new_table = Enum.reject(all(table), callback) - new_tables = Map.update!(tables(), table, fn _ -> new_table end) - - State.update_tables(new_tables) - - :ok + State.delete_entries(table, callback) end def delete(table, entry_id) do @@ -151,11 +127,7 @@ defmodule Support.Stubs.DB do def clear(table) do verify_table_exists!(table) - new_tables = Map.update!(tables(), table, fn _ -> [] end) - - State.update_tables(new_tables) - - :ok + State.clear_table(table) end # @@ -236,5 +208,52 @@ defmodule Support.Stubs.DB do def update_schemas(new_schemas) do Agent.update(__MODULE__, fn db -> %{db | schemas: new_schemas} end) end + + def insert_entry(table, entry) do + Agent.get_and_update(__MODULE__, fn db -> + current_table = Map.get(db.tables, table, []) + new_table = current_table ++ [entry] + new_tables = Map.put(db.tables, table, new_table) + + {entry, %{db | tables: new_tables}} + end) + end + + def update_entry(table, entry, field) do + Agent.get_and_update(__MODULE__, fn db -> + current_table = Map.get(db.tables, table, []) + + new_table = + Enum.map(current_table, fn old_entry -> + if Map.get(old_entry, field) == Map.get(entry, field) do + entry + else + old_entry + end + end) + + new_tables = Map.put(db.tables, table, new_table) + + {entry, %{db | tables: new_tables}} + end) + end + + def delete_entries(table, filter_fn) do + Agent.get_and_update(__MODULE__, fn db -> + current_table = Map.get(db.tables, table, []) + new_table = Enum.reject(current_table, filter_fn) + new_tables = Map.put(db.tables, table, new_table) + + {:ok, %{db | tables: new_tables}} + end) + end + + def clear_table(table) do + Agent.get_and_update(__MODULE__, fn db -> + new_tables = Map.put(db.tables, table, []) + + {:ok, %{db | tables: new_tables}} + end) + end end end diff --git a/front/test/support/stubs/scheduler.ex b/front/test/support/stubs/scheduler.ex index 5e7c566dd..f752c3561 100644 --- a/front/test/support/stubs/scheduler.ex +++ b/front/test/support/stubs/scheduler.ex @@ -29,7 +29,7 @@ defmodule Support.Stubs.Scheduler do name: "Scheduler", project_id: project.id, recurring: true, - branch: "master", + reference: "master", at: "10 17 * * 1-5", pipeline_file: ".semaphore/semaphore.yml", requester_id: user.id, @@ -63,7 +63,7 @@ defmodule Support.Stubs.Scheduler do defaults = [ triggered_at: Time.now(), project_id: periodic.project_id, - branch: periodic.branch, + reference: periodic.reference, pipeline_file: periodic.pipeline_file, scheduling_status: "passed", scheduled_workflow_id: workflow.wf_id, @@ -165,18 +165,12 @@ defmodule Support.Stubs.Scheduler do wf_id = DB.first(:workflows) |> Map.get(:id) workflow = %{wf_id: wf_id} + pipeline_file = get_pipeline_file(req.pipeline_file, scheduler.api_model.pipeline_file) + trigger = Support.Stubs.Scheduler.create_trigger(scheduler.api_model, workflow, user, - branch: - if(req.branch == "", - do: scheduler.api_model.branch, - else: req.branch - ), - pipeline_file: - if(req.pipeline_file == "", - do: scheduler.api_model.pipeline_file, - else: req.pipeline_file - ), + reference: req.reference, + pipeline_file: pipeline_file, parameter_values: req.parameter_values || [] ) @@ -188,9 +182,11 @@ defmodule Support.Stubs.Scheduler do RunNowResponse.new(status: status(:NOT_FOUND)) end - @modifiable_fields ~w(name description recurring branch pipeline_file at parameters)a + @modifiable_fields ~w(name description recurring reference pipeline_file at parameters)a def persist(req, _) do + require Logger + Logger.info("persisting scheduler: #{inspect(req)}") model_from_req = periodic_from_req(req) if req.id != nil && req.id != "" do @@ -325,11 +321,14 @@ defmodule Support.Stubs.Scheduler do description: req.description, recurring: req.recurring, requester_id: req.requester_id, - branch: req.branch, + reference: req.reference, pipeline_file: req.pipeline_file, at: req.at, parameters: req.parameters ) end + + defp get_pipeline_file("", default_file), do: default_file + defp get_pipeline_file(file, _default), do: file end end From 7da2366490f7c07c65656ae38d58ed5317c11f02 Mon Sep 17 00:00:00 2001 From: Amir Hasanbasic <43892661+hamir-suspect@users.noreply.github.com> Date: Mon, 22 Sep 2025 15:29:55 +0200 Subject: [PATCH 2/4] feat(periodic_scheduler): support generic git ref (#581) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description - Adds support for scheduling periodic workflows on Git tags in addition to branches - Introduces a new git_reference utility module to handle both branch and tag references - Updates the periodic scheduler to accept and process tag-based schedules - **gRPC/Protocol buffer updates:** Regenerated protocol buffer definitions to rename `branch` field into `reference`, will still work if caller service uses `branch` field name ([since type and order in api is preserved](https://stackoverflow.com/questions/45431685/protocol-buffer-does-changing-field-name-break-the-message)) ## ✅ Checklist - [x] I have tested this change - [x] This change requires documentation update --- periodic_scheduler/scheduler/config/dev.exs | 3 + .../scheduler/docker-compose.yml | 4 +- .../lib/internal_api/organization.pb.ex | 100 +- .../lib/internal_api/periodic_scheduler.pb.ex | 9 +- .../internal_api/plumber_w_f.workflow.pb.ex | 3 + .../lib/internal_api/projecthub.pb.ex | 74 ++ .../lib/internal_api/repo_proxy.pb.ex | 9 + .../lib/internal_api/repository.pb.ex | 61 ++ .../internal_api/repository_integrator.pb.ex | 14 + .../lib/scheduler/actions/apply_impl.ex | 74 +- .../lib/scheduler/actions/history_impl.ex | 16 +- .../lib/scheduler/actions/persist_impl.ex | 8 +- .../lib/scheduler/actions/run_now_impl.ex | 19 +- .../lib/scheduler/actions/schedule_wf_impl.ex | 141 +-- .../scheduler/periodics/model/periodics.ex | 33 +- .../periodics/model/periodics_queries.ex | 39 +- .../periodics_triggers/model/history_page.ex | 12 +- .../model/periodics_triggers.ex | 6 +- .../model/periodics_triggers_queries.ex | 16 +- .../lib/scheduler/utils/git_reference.ex | 126 +++ periodic_scheduler/scheduler/mix.lock | 2 +- .../test/actions/apply_impl_test.exs | 75 +- .../test/actions/describe_impl_test.exs | 2 +- .../test/actions/history_impl_test.exs | 2 +- .../test/actions/persist_impl_test.exs | 14 +- .../test/actions/run_now_impl_test.exs | 14 +- .../test/actions/schedule_wf_impl_test.exs | 934 ++++++++---------- .../test/actions/unpause_impl_test.exs | 2 +- .../events_consumers/org_blocked_test.exs | 2 +- .../scheduler/test/grpc_server_test.exs | 38 +- .../test/periodics/initializer_test.exs | 2 +- .../periodics/model/periodic_queries_test.exs | 50 +- .../test/periodics/model/periodics_test.exs | 14 +- .../model/history_page_test.exs | 26 +- .../model/periodics_triggers_queries_test.exs | 49 +- .../scheduler/test/support/factory.ex | 4 +- .../scheduler/test/support/yaml.ex | 8 +- .../test/utils/git_reference_test.exs | 96 ++ periodic_scheduler/spec/priv/v1.2.yml | 51 + 39 files changed, 1314 insertions(+), 838 deletions(-) create mode 100644 periodic_scheduler/scheduler/lib/scheduler/utils/git_reference.ex create mode 100644 periodic_scheduler/scheduler/test/utils/git_reference_test.exs create mode 100644 periodic_scheduler/spec/priv/v1.2.yml diff --git a/periodic_scheduler/scheduler/config/dev.exs b/periodic_scheduler/scheduler/config/dev.exs index ced742128..dff4440b3 100644 --- a/periodic_scheduler/scheduler/config/dev.exs +++ b/periodic_scheduler/scheduler/config/dev.exs @@ -21,3 +21,6 @@ config :watchman, host: "localhost", port: 8125, prefix: "periodic-sch.dev" + +config :scheduler, + feature_provider: {Scheduler.FeatureHubProvider, []} diff --git a/periodic_scheduler/scheduler/docker-compose.yml b/periodic_scheduler/scheduler/docker-compose.yml index 7c7b78e76..bca2db703 100644 --- a/periodic_scheduler/scheduler/docker-compose.yml +++ b/periodic_scheduler/scheduler/docker-compose.yml @@ -25,8 +25,8 @@ services: stdin_open: true tty: true volumes: - - .:/app:delegated - working_dir: "/app" + - ../:/app:delegated + working_dir: "/app/scheduler" ciapp: container_name: ciapp diff --git a/periodic_scheduler/scheduler/lib/internal_api/organization.pb.ex b/periodic_scheduler/scheduler/lib/internal_api/organization.pb.ex index 054916701..a909b8e30 100644 --- a/periodic_scheduler/scheduler/lib/internal_api/organization.pb.ex +++ b/periodic_scheduler/scheduler/lib/internal_api/organization.pb.ex @@ -24,20 +24,6 @@ defmodule InternalApi.Organization.Member.Role do field :ADMIN, 2 end -defmodule InternalApi.Organization.Quota.Type do - @moduledoc false - use Protobuf, enum: true, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 - - field :MAX_PEOPLE_IN_ORG, 0 - field :MAX_PARALELLISM_IN_ORG, 1 - field :MAX_PROJECTS_IN_ORG, 7 - field :MAX_PARALLEL_E1_STANDARD_2, 2 - field :MAX_PARALLEL_E1_STANDARD_4, 3 - field :MAX_PARALLEL_E1_STANDARD_8, 4 - field :MAX_PARALLEL_A1_STANDARD_4, 5 - field :MAX_PARALLEL_A1_STANDARD_8, 6 -end - defmodule InternalApi.Organization.OrganizationContact.ContactType do @moduledoc false use Protobuf, enum: true, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 @@ -55,6 +41,7 @@ defmodule InternalApi.Organization.DescribeRequest do field :org_id, 1, type: :string, json_name: "orgId" field :org_username, 2, type: :string, json_name: "orgUsername" field :include_quotas, 3, type: :bool, json_name: "includeQuotas" + field :soft_deleted, 4, type: :bool, json_name: "softDeleted" end defmodule InternalApi.Organization.DescribeResponse do @@ -70,6 +57,7 @@ defmodule InternalApi.Organization.DescribeManyRequest do use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 field :org_ids, 1, repeated: true, type: :string, json_name: "orgIds" + field :soft_deleted, 2, type: :bool, json_name: "softDeleted" end defmodule InternalApi.Organization.DescribeManyResponse do @@ -88,6 +76,7 @@ defmodule InternalApi.Organization.ListRequest do field :order, 4, type: InternalApi.Organization.ListRequest.Order, enum: true field :page_size, 5, type: :int32, json_name: "pageSize" field :page_token, 6, type: :string, json_name: "pageToken" + field :soft_deleted, 7, type: :bool, json_name: "softDeleted" end defmodule InternalApi.Organization.ListResponse do @@ -116,21 +105,6 @@ defmodule InternalApi.Organization.CreateResponse do field :organization, 2, type: InternalApi.Organization.Organization end -defmodule InternalApi.Organization.CreateWithQuotasRequest do - @moduledoc false - use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 - - field :organization, 1, type: InternalApi.Organization.Organization - field :quotas, 2, repeated: true, type: InternalApi.Organization.Quota -end - -defmodule InternalApi.Organization.CreateWithQuotasResponse do - @moduledoc false - use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 - - field :organization, 1, type: InternalApi.Organization.Organization -end - defmodule InternalApi.Organization.UpdateRequest do @moduledoc false use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 @@ -357,6 +331,13 @@ defmodule InternalApi.Organization.DestroyRequest do field :org_id, 1, type: :string, json_name: "orgId" end +defmodule InternalApi.Organization.RestoreRequest do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :org_id, 1, type: :string, json_name: "orgId" +end + defmodule InternalApi.Organization.Organization do @moduledoc false use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 @@ -375,7 +356,6 @@ defmodule InternalApi.Organization.Organization do field :allowed_id_providers, 13, repeated: true, type: :string, json_name: "allowedIdProviders" field :deny_member_workflows, 14, type: :bool, json_name: "denyMemberWorkflows" field :deny_non_member_workflows, 15, type: :bool, json_name: "denyNonMemberWorkflows" - field :quotas, 8, repeated: true, type: InternalApi.Organization.Quota field :settings, 16, repeated: true, type: InternalApi.Organization.OrganizationSetting end @@ -403,14 +383,6 @@ defmodule InternalApi.Organization.Member do field :github_uid, 8, type: :string, json_name: "githubUid" end -defmodule InternalApi.Organization.Quota do - @moduledoc false - use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 - - field :type, 1, type: InternalApi.Organization.Quota.Type, enum: true - field :value, 2, type: :uint32 -end - defmodule InternalApi.Organization.OrganizationSetting do @moduledoc false use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 @@ -419,36 +391,6 @@ defmodule InternalApi.Organization.OrganizationSetting do field :value, 2, type: :string end -defmodule InternalApi.Organization.GetQuotasRequest do - @moduledoc false - use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 - - field :org_id, 1, type: :string, json_name: "orgId" - field :types, 2, repeated: true, type: InternalApi.Organization.Quota.Type, enum: true -end - -defmodule InternalApi.Organization.GetQuotaResponse do - @moduledoc false - use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 - - field :quotas, 1, repeated: true, type: InternalApi.Organization.Quota -end - -defmodule InternalApi.Organization.UpdateQuotasRequest do - @moduledoc false - use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 - - field :org_id, 1, type: :string, json_name: "orgId" - field :quotas, 2, repeated: true, type: InternalApi.Organization.Quota -end - -defmodule InternalApi.Organization.UpdateQuotasResponse do - @moduledoc false - use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 - - field :quotas, 1, repeated: true, type: InternalApi.Organization.Quota -end - defmodule InternalApi.Organization.RepositoryIntegratorsRequest do @moduledoc false use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 @@ -620,6 +562,14 @@ defmodule InternalApi.Organization.OrganizationDailyUpdate do field :timestamp, 11, type: Google.Protobuf.Timestamp end +defmodule InternalApi.Organization.OrganizationRestored do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :org_id, 1, type: :string, json_name: "orgId" + field :timestamp, 2, type: Google.Protobuf.Timestamp +end + defmodule InternalApi.Organization.OrganizationService.Service do @moduledoc false use GRPC.Service, @@ -638,10 +588,6 @@ defmodule InternalApi.Organization.OrganizationService.Service do rpc :Create, InternalApi.Organization.CreateRequest, InternalApi.Organization.CreateResponse - rpc :CreateWithQuotas, - InternalApi.Organization.CreateWithQuotasRequest, - InternalApi.Organization.CreateWithQuotasResponse - rpc :Update, InternalApi.Organization.UpdateRequest, InternalApi.Organization.UpdateResponse rpc :IsValid, InternalApi.Organization.Organization, InternalApi.Organization.IsValidResponse @@ -684,16 +630,10 @@ defmodule InternalApi.Organization.OrganizationService.Service do InternalApi.Organization.ListSuspensionsRequest, InternalApi.Organization.ListSuspensionsResponse - rpc :UpdateQuotas, - InternalApi.Organization.UpdateQuotasRequest, - InternalApi.Organization.UpdateQuotasResponse - - rpc :GetQuotas, - InternalApi.Organization.GetQuotasRequest, - InternalApi.Organization.GetQuotaResponse - rpc :Destroy, InternalApi.Organization.DestroyRequest, Google.Protobuf.Empty + rpc :Restore, InternalApi.Organization.RestoreRequest, Google.Protobuf.Empty + rpc :RepositoryIntegrators, InternalApi.Organization.RepositoryIntegratorsRequest, InternalApi.Organization.RepositoryIntegratorsResponse diff --git a/periodic_scheduler/scheduler/lib/internal_api/periodic_scheduler.pb.ex b/periodic_scheduler/scheduler/lib/internal_api/periodic_scheduler.pb.ex index 487bb9703..8442fa779 100644 --- a/periodic_scheduler/scheduler/lib/internal_api/periodic_scheduler.pb.ex +++ b/periodic_scheduler/scheduler/lib/internal_api/periodic_scheduler.pb.ex @@ -61,7 +61,7 @@ defmodule InternalApi.PeriodicScheduler.PersistRequest do field :organization_id, 6, type: :string, json_name: "organizationId" field :project_name, 7, type: :string, json_name: "projectName" field :requester_id, 8, type: :string, json_name: "requesterId" - field :branch, 9, type: :string + field :reference, 9, type: :string field :pipeline_file, 10, type: :string, json_name: "pipelineFile" field :at, 11, type: :string field :parameters, 12, repeated: true, type: InternalApi.PeriodicScheduler.Periodic.Parameter @@ -112,7 +112,7 @@ defmodule InternalApi.PeriodicScheduler.RunNowRequest do field :id, 1, type: :string field :requester, 2, type: :string - field :branch, 3, type: :string + field :reference, 3, type: :string field :pipeline_file, 4, type: :string, json_name: "pipelineFile" field :parameter_values, 5, @@ -165,7 +165,7 @@ defmodule InternalApi.PeriodicScheduler.Periodic do field :id, 1, type: :string field :name, 2, type: :string field :project_id, 3, type: :string, json_name: "projectId" - field :branch, 4, type: :string + field :reference, 4, type: :string field :at, 5, type: :string field :pipeline_file, 6, type: :string, json_name: "pipelineFile" field :requester_id, 7, type: :string, json_name: "requesterId" @@ -178,6 +178,7 @@ defmodule InternalApi.PeriodicScheduler.Periodic do field :recurring, 14, type: :bool field :parameters, 15, repeated: true, type: InternalApi.PeriodicScheduler.Periodic.Parameter field :description, 16, type: :string + field :organization_id, 17, type: :string, json_name: "organizationId" end defmodule InternalApi.PeriodicScheduler.Trigger do @@ -186,7 +187,7 @@ defmodule InternalApi.PeriodicScheduler.Trigger do field :triggered_at, 1, type: Google.Protobuf.Timestamp, json_name: "triggeredAt" field :project_id, 2, type: :string, json_name: "projectId" - field :branch, 3, type: :string + field :reference, 3, type: :string field :pipeline_file, 4, type: :string, json_name: "pipelineFile" field :scheduling_status, 5, type: :string, json_name: "schedulingStatus" field :scheduled_workflow_id, 6, type: :string, json_name: "scheduledWorkflowId" diff --git a/periodic_scheduler/scheduler/lib/internal_api/plumber_w_f.workflow.pb.ex b/periodic_scheduler/scheduler/lib/internal_api/plumber_w_f.workflow.pb.ex index 75588616c..0f95242b2 100644 --- a/periodic_scheduler/scheduler/lib/internal_api/plumber_w_f.workflow.pb.ex +++ b/periodic_scheduler/scheduler/lib/internal_api/plumber_w_f.workflow.pb.ex @@ -129,6 +129,9 @@ defmodule InternalApi.PlumberWF.ScheduleRequest do repeated: true, type: InternalApi.PlumberWF.ScheduleRequest.EnvVar, json_name: "envVars" + + field :start_in_conceived_state, 18, type: :bool, json_name: "startInConceivedState" + field :git_reference, 19, type: :string, json_name: "gitReference" end defmodule InternalApi.PlumberWF.ScheduleResponse do diff --git a/periodic_scheduler/scheduler/lib/internal_api/projecthub.pb.ex b/periodic_scheduler/scheduler/lib/internal_api/projecthub.pb.ex index 61b19ce4c..61df470e9 100644 --- a/periodic_scheduler/scheduler/lib/internal_api/projecthub.pb.ex +++ b/periodic_scheduler/scheduler/lib/internal_api/projecthub.pb.ex @@ -35,6 +35,7 @@ defmodule InternalApi.Projecthub.Project.Spec.Repository.RunType do field :TAGS, 1 field :PULL_REQUESTS, 2 field :FORKED_PULL_REQUESTS, 3 + field :DRAFT_PULL_REQUESTS, 4 end defmodule InternalApi.Projecthub.Project.Spec.Repository.Status.PipelineFile.Level do @@ -70,6 +71,7 @@ defmodule InternalApi.Projecthub.Project.Status.State do field :INITIALIZING, 0 field :READY, 1 field :ERROR, 2 + field :ONBOARDING, 3 end defmodule InternalApi.Projecthub.ListKeysetRequest.Direction do @@ -350,6 +352,7 @@ defmodule InternalApi.Projecthub.ListRequest do field :pagination, 2, type: InternalApi.Projecthub.PaginationRequest field :owner_id, 3, type: :string, json_name: "ownerId" field :repo_url, 4, type: :string, json_name: "repoUrl" + field :soft_deleted, 5, type: :bool, json_name: "softDeleted" end defmodule InternalApi.Projecthub.ListResponse do @@ -371,6 +374,7 @@ defmodule InternalApi.Projecthub.ListKeysetRequest do field :direction, 4, type: InternalApi.Projecthub.ListKeysetRequest.Direction, enum: true field :owner_id, 5, type: :string, json_name: "ownerId" field :repo_url, 6, type: :string, json_name: "repoUrl" + field :created_after, 7, type: Google.Protobuf.Timestamp, json_name: "createdAfter" end defmodule InternalApi.Projecthub.ListKeysetResponse do @@ -391,6 +395,7 @@ defmodule InternalApi.Projecthub.DescribeRequest do field :id, 2, type: :string field :name, 3, type: :string field :detailed, 4, type: :bool + field :soft_deleted, 5, type: :bool, json_name: "softDeleted" end defmodule InternalApi.Projecthub.DescribeResponse do @@ -407,6 +412,7 @@ defmodule InternalApi.Projecthub.DescribeManyRequest do field :metadata, 1, type: InternalApi.Projecthub.RequestMeta field :ids, 2, repeated: true, type: :string + field :soft_deleted, 3, type: :bool, json_name: "softDeleted" end defmodule InternalApi.Projecthub.DescribeManyResponse do @@ -423,6 +429,7 @@ defmodule InternalApi.Projecthub.CreateRequest do field :metadata, 1, type: InternalApi.Projecthub.RequestMeta field :project, 2, type: InternalApi.Projecthub.Project + field :skip_onboarding, 3, type: :bool, json_name: "skipOnboarding" end defmodule InternalApi.Projecthub.CreateResponse do @@ -466,6 +473,21 @@ defmodule InternalApi.Projecthub.DestroyResponse do field :metadata, 1, type: InternalApi.Projecthub.ResponseMeta end +defmodule InternalApi.Projecthub.RestoreRequest do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :metadata, 1, type: InternalApi.Projecthub.RequestMeta + field :id, 2, type: :string +end + +defmodule InternalApi.Projecthub.RestoreResponse do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :metadata, 1, type: InternalApi.Projecthub.ResponseMeta +end + defmodule InternalApi.Projecthub.UsersRequest do @moduledoc false use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 @@ -497,6 +519,7 @@ defmodule InternalApi.Projecthub.CheckDeployKeyResponse.DeployKey do field :title, 1, type: :string field :fingerprint, 2, type: :string field :created_at, 3, type: Google.Protobuf.Timestamp, json_name: "createdAt" + field :public_key, 4, type: :string, json_name: "publicKey" end defmodule InternalApi.Projecthub.CheckDeployKeyResponse do @@ -525,6 +548,7 @@ defmodule InternalApi.Projecthub.RegenerateDeployKeyResponse.DeployKey do field :title, 1, type: :string field :fingerprint, 2, type: :string field :created_at, 3, type: Google.Protobuf.Timestamp, json_name: "createdAt" + field :public_key, 4, type: :string, json_name: "publicKey" end defmodule InternalApi.Projecthub.RegenerateDeployKeyResponse do @@ -624,6 +648,37 @@ defmodule InternalApi.Projecthub.GithubAppSwitchResponse do field :metadata, 1, type: InternalApi.Projecthub.ResponseMeta end +defmodule InternalApi.Projecthub.FinishOnboardingRequest do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :metadata, 1, type: InternalApi.Projecthub.RequestMeta + field :id, 2, type: :string +end + +defmodule InternalApi.Projecthub.FinishOnboardingResponse do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :metadata, 1, type: InternalApi.Projecthub.ResponseMeta +end + +defmodule InternalApi.Projecthub.RegenerateWebhookSecretRequest do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :metadata, 1, type: InternalApi.Projecthub.RequestMeta + field :id, 2, type: :string +end + +defmodule InternalApi.Projecthub.RegenerateWebhookSecretResponse do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :metadata, 1, type: InternalApi.Projecthub.ResponseMeta + field :secret, 2, type: :string +end + defmodule InternalApi.Projecthub.ProjectCreated do @moduledoc false use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 @@ -642,6 +697,15 @@ defmodule InternalApi.Projecthub.ProjectDeleted do field :org_id, 3, type: :string, json_name: "orgId" end +defmodule InternalApi.Projecthub.ProjectRestored do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :project_id, 1, type: :string, json_name: "projectId" + field :timestamp, 2, type: Google.Protobuf.Timestamp + field :org_id, 3, type: :string, json_name: "orgId" +end + defmodule InternalApi.Projecthub.ProjectUpdated do @moduledoc false use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 @@ -683,6 +747,8 @@ defmodule InternalApi.Projecthub.ProjectService.Service do rpc :Destroy, InternalApi.Projecthub.DestroyRequest, InternalApi.Projecthub.DestroyResponse + rpc :Restore, InternalApi.Projecthub.RestoreRequest, InternalApi.Projecthub.RestoreResponse + rpc :Users, InternalApi.Projecthub.UsersRequest, InternalApi.Projecthub.UsersResponse rpc :CheckDeployKey, @@ -701,6 +767,10 @@ defmodule InternalApi.Projecthub.ProjectService.Service do InternalApi.Projecthub.RegenerateWebhookRequest, InternalApi.Projecthub.RegenerateWebhookResponse + rpc :RegenerateWebhookSecret, + InternalApi.Projecthub.RegenerateWebhookSecretRequest, + InternalApi.Projecthub.RegenerateWebhookSecretResponse + rpc :ChangeProjectOwner, InternalApi.Projecthub.ChangeProjectOwnerRequest, InternalApi.Projecthub.ChangeProjectOwnerResponse @@ -712,6 +782,10 @@ defmodule InternalApi.Projecthub.ProjectService.Service do rpc :GithubAppSwitch, InternalApi.Projecthub.GithubAppSwitchRequest, InternalApi.Projecthub.GithubAppSwitchResponse + + rpc :FinishOnboarding, + InternalApi.Projecthub.FinishOnboardingRequest, + InternalApi.Projecthub.FinishOnboardingResponse end defmodule InternalApi.Projecthub.ProjectService.Stub do diff --git a/periodic_scheduler/scheduler/lib/internal_api/repo_proxy.pb.ex b/periodic_scheduler/scheduler/lib/internal_api/repo_proxy.pb.ex index 294a41cba..feca9b365 100644 --- a/periodic_scheduler/scheduler/lib/internal_api/repo_proxy.pb.ex +++ b/periodic_scheduler/scheduler/lib/internal_api/repo_proxy.pb.ex @@ -187,6 +187,15 @@ defmodule InternalApi.RepoProxy.CreateBlankResponse do field :repo, 5, type: InternalApi.RepoProxy.CreateBlankResponse.Repo end +defmodule InternalApi.RepoProxy.PullRequestUnmergeable do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :project_id, 1, type: :string, json_name: "projectId" + field :branch_name, 2, type: :string, json_name: "branchName" + field :timestamp, 3, type: Google.Protobuf.Timestamp +end + defmodule InternalApi.RepoProxy.RepoProxyService.Service do @moduledoc false use GRPC.Service, diff --git a/periodic_scheduler/scheduler/lib/internal_api/repository.pb.ex b/periodic_scheduler/scheduler/lib/internal_api/repository.pb.ex index 3e6e720df..1b402e8df 100644 --- a/periodic_scheduler/scheduler/lib/internal_api/repository.pb.ex +++ b/periodic_scheduler/scheduler/lib/internal_api/repository.pb.ex @@ -79,6 +79,7 @@ defmodule InternalApi.Repository.DeployKey do field :title, 1, type: :string field :fingerprint, 2, type: :string field :created_at, 3, type: Google.Protobuf.Timestamp, json_name: "createdAt" + field :public_key, 4, type: :string, json_name: "publicKey" end defmodule InternalApi.Repository.DescribeRemoteRepositoryRequest do @@ -327,6 +328,7 @@ defmodule InternalApi.Repository.Repository do field :whitelist, 11, type: InternalApi.Projecthub.Project.Spec.Repository.Whitelist field :hook_id, 12, type: :string, json_name: "hookId" field :default_branch, 13, type: :string, json_name: "defaultBranch" + field :connected, 14, type: :bool end defmodule InternalApi.Repository.RemoteRepository do @@ -481,6 +483,7 @@ defmodule InternalApi.Repository.CreateRequest do json_name: "commitStatus" field :whitelist, 9, type: InternalApi.Projecthub.Project.Spec.Repository.Whitelist + field :default_branch, 10, type: :string, json_name: "defaultBranch" end defmodule InternalApi.Repository.CreateResponse do @@ -522,6 +525,7 @@ defmodule InternalApi.Repository.UpdateRequest do json_name: "commitStatus" field :whitelist, 6, type: InternalApi.Projecthub.Project.Spec.Repository.Whitelist + field :default_branch, 7, type: :string, json_name: "defaultBranch" end defmodule InternalApi.Repository.UpdateResponse do @@ -539,6 +543,51 @@ defmodule InternalApi.Repository.RemoteRepositoryChanged do field :timestamp, 3, type: Google.Protobuf.Timestamp end +defmodule InternalApi.Repository.VerifyWebhookSignatureRequest do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :organization_id, 1, type: :string, json_name: "organizationId" + field :repository_id, 2, type: :string, json_name: "repositoryId" + field :payload, 3, type: :string + field :signature, 4, type: :string +end + +defmodule InternalApi.Repository.VerifyWebhookSignatureResponse do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :valid, 1, type: :bool +end + +defmodule InternalApi.Repository.ClearExternalDataRequest do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :repository_id, 1, type: :string, json_name: "repositoryId" +end + +defmodule InternalApi.Repository.ClearExternalDataResponse do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :repository, 1, type: InternalApi.Repository.Repository +end + +defmodule InternalApi.Repository.RegenerateWebhookSecretRequest do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :repository_id, 1, type: :string, json_name: "repositoryId" +end + +defmodule InternalApi.Repository.RegenerateWebhookSecretResponse do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 + + field :secret, 1, type: :string +end + defmodule InternalApi.Repository.RepositoryService.Service do @moduledoc false use GRPC.Service, @@ -610,6 +659,18 @@ defmodule InternalApi.Repository.RepositoryService.Service do rpc :DescribeRevision, InternalApi.Repository.DescribeRevisionRequest, InternalApi.Repository.DescribeRevisionResponse + + rpc :VerifyWebhookSignature, + InternalApi.Repository.VerifyWebhookSignatureRequest, + InternalApi.Repository.VerifyWebhookSignatureResponse + + rpc :ClearExternalData, + InternalApi.Repository.ClearExternalDataRequest, + InternalApi.Repository.ClearExternalDataResponse + + rpc :RegenerateWebhookSecret, + InternalApi.Repository.RegenerateWebhookSecretRequest, + InternalApi.Repository.RegenerateWebhookSecretResponse end defmodule InternalApi.Repository.RepositoryService.Stub do diff --git a/periodic_scheduler/scheduler/lib/internal_api/repository_integrator.pb.ex b/periodic_scheduler/scheduler/lib/internal_api/repository_integrator.pb.ex index f2b7e57db..b92f3b821 100644 --- a/periodic_scheduler/scheduler/lib/internal_api/repository_integrator.pb.ex +++ b/periodic_scheduler/scheduler/lib/internal_api/repository_integrator.pb.ex @@ -101,6 +101,16 @@ defmodule InternalApi.RepositoryIntegrator.GithubInstallationInfoResponse do field :installation_url, 3, type: :string, json_name: "installationUrl" end +defmodule InternalApi.RepositoryIntegrator.InitGithubInstallationRequest do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 +end + +defmodule InternalApi.RepositoryIntegrator.InitGithubInstallationResponse do + @moduledoc false + use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 +end + defmodule InternalApi.RepositoryIntegrator.GetRepositoriesRequest do @moduledoc false use Protobuf, protoc_gen_elixir_version: "0.11.0", syntax: :proto3 @@ -157,6 +167,10 @@ defmodule InternalApi.RepositoryIntegrator.RepositoryIntegratorService.Service d InternalApi.RepositoryIntegrator.GithubInstallationInfoRequest, InternalApi.RepositoryIntegrator.GithubInstallationInfoResponse + rpc :InitGithubInstallation, + InternalApi.RepositoryIntegrator.InitGithubInstallationRequest, + InternalApi.RepositoryIntegrator.InitGithubInstallationResponse + rpc :GetRepositories, InternalApi.RepositoryIntegrator.GetRepositoriesRequest, InternalApi.RepositoryIntegrator.GetRepositoriesResponse diff --git a/periodic_scheduler/scheduler/lib/scheduler/actions/apply_impl.ex b/periodic_scheduler/scheduler/lib/scheduler/actions/apply_impl.ex index 416c274b5..b95c730db 100644 --- a/periodic_scheduler/scheduler/lib/scheduler/actions/apply_impl.ex +++ b/periodic_scheduler/scheduler/lib/scheduler/actions/apply_impl.ex @@ -9,6 +9,7 @@ defmodule Scheduler.Actions.ApplyImpl do alias Scheduler.FrontDB.Model.FrontDBQueries alias Crontab.CronExpression.Parser alias Scheduler.Workers.QuantumScheduler + alias Scheduler.Utils.GitReference def apply(request) do with {:ok, definition} <- DefinitionValidator.validate_yaml_string(request.yml_definition), @@ -52,6 +53,9 @@ defmodule Scheduler.Actions.ApplyImpl do else: {:cron, {:ok, %Crontab.CronExpression{}}} end + defp validate_cron_expression(definition, "v1.2"), + do: validate_cron_expression(definition, "v1.1") + defp validate_cron_expression(definition, _version_1_0) do cron_exp = definition |> Map.get("spec", %{}) |> Map.get("at", "") @@ -73,7 +77,7 @@ defmodule Scheduler.Actions.ApplyImpl do {:hook, {:ok, true}} else recurring = definition |> Map.get("spec", %{}) |> Map.get("recurring", true) - branch = definition |> Map.get("spec", %{}) |> Map.get("branch", "") + branch = extract_branch_name(definition) if recurring, do: {:hook, FrontDBQueries.hook_exists?(project_id, branch)}, @@ -138,10 +142,13 @@ defmodule Scheduler.Actions.ApplyImpl do end defp form_periodic_params(request, definition, project_id, original_periodic \\ %{}) do + api_version = definition |> Map.get("apiVersion", "") + definition |> transform_keys() |> extract_spec() |> extract_metadata() + |> handle_reference_field_mapping(api_version, definition) |> Map.merge(request) |> consolidate_paused(original_periodic) |> Map.merge(%{project_id: project_id}) @@ -193,4 +200,69 @@ defmodule Scheduler.Actions.ApplyImpl do end defp consolidate_paused(params, _), do: params + + # Handle mapping between API versions and database field + defp handle_reference_field_mapping(params, api_version, original_definition) do + cond do + # v1.1 and earlier use 'branch' field - map it to 'reference' for database + api_version in ["v1.0", "v1.1"] -> + branch_name = extract_branch_name(original_definition) + + params + |> Map.delete(:branch) + |> Map.put(:reference, branch_name) + + # v1.2 uses 'reference' object - extract the name + api_version == "v1.2" -> + reference_name = extract_reference_name(original_definition) + + params + |> Map.delete(:reference) + |> Map.put(:reference, reference_name) + + # Default case + true -> + params + end + end + + # Extract branch name for v1.1 and earlier, or from v1.2 reference object + defp extract_branch_name(definition) do + api_version = definition |> Map.get("apiVersion", "") + spec = definition |> Map.get("spec", %{}) + + case api_version do + "v1.2" -> + case spec |> Map.get("reference") do + reference when is_map(reference) -> + reference |> Map.get("name", "") + + _not_a_map -> + spec |> Map.get("branch", "") + end + + _other -> + spec |> Map.get("branch", "") + end + end + + # Extract and build full reference path from v1.2 reference object or branch field + defp extract_reference_name(definition) do + spec = definition |> Map.get("spec", %{}) + + case spec |> Map.get("reference") do + reference when is_map(reference) -> + type = reference |> Map.get("type", "") + name = reference |> Map.get("name", "") + build_full_reference(type, name) + + _not_a_map -> + # Fallback to branch field for backwards compatibility in v1.2 + branch_name = spec |> Map.get("branch", "") + build_full_reference("BRANCH", branch_name) + end + end + + # Build full reference path based on type + defp build_full_reference(type, name), do: GitReference.build_full_reference(type, name) end diff --git a/periodic_scheduler/scheduler/lib/scheduler/actions/history_impl.ex b/periodic_scheduler/scheduler/lib/scheduler/actions/history_impl.ex index b47be9bb7..b7da0b542 100644 --- a/periodic_scheduler/scheduler/lib/scheduler/actions/history_impl.ex +++ b/periodic_scheduler/scheduler/lib/scheduler/actions/history_impl.ex @@ -6,6 +6,7 @@ defmodule Scheduler.Actions.HistoryImpl do alias Scheduler.Periodics.Model.PeriodicsQueries alias Scheduler.Periodics.Model.Periodics alias Scheduler.PeriodicsTriggers.Model.HistoryPage + alias Scheduler.Utils.GitReference def history(params) do {:ok, handle_history(params)} @@ -51,11 +52,24 @@ defmodule Scheduler.Actions.HistoryImpl do defp parse_filters(filters) when is_map(filters) do filter_keys = ~w(branch_name pipeline_file triggered_by)a - filters |> Map.take(filter_keys) |> Enum.reject(&empty?(elem(&1, 1))) |> Map.new() + + filters + |> Map.take(filter_keys) + |> Enum.reject(&empty?(elem(&1, 1))) + |> Enum.map(&transform_filter/1) + |> Map.new() end defp parse_filters(_filters), do: %{} + defp transform_filter({:branch_name, value}) do + normalized_ref = GitReference.normalize(value) + short_name = GitReference.extract_name(value) + {:reference, %{normalized: normalized_ref, short: short_name, original: value}} + end + + defp transform_filter(filter), do: filter + defp from_model(periodics_trigger) do parameter_values = Enum.into(periodics_trigger.parameter_values, [], &Map.from_struct/1) periodics_trigger |> Map.from_struct() |> Map.put(:parameter_values, parameter_values) diff --git a/periodic_scheduler/scheduler/lib/scheduler/actions/persist_impl.ex b/periodic_scheduler/scheduler/lib/scheduler/actions/persist_impl.ex index f173816d9..9e2e87088 100644 --- a/periodic_scheduler/scheduler/lib/scheduler/actions/persist_impl.ex +++ b/periodic_scheduler/scheduler/lib/scheduler/actions/persist_impl.ex @@ -50,7 +50,7 @@ defmodule Scheduler.Actions.PersistImpl do defp create(request) do with {:ok, project_name} <- get_project_name(request.organization_id, request.project_id), {:ok, params} <- form_periodic_params(request, project_name), - {:ok, periodic} <- PeriodicsQueries.insert(params, "v1.1"), + {:ok, periodic} <- PeriodicsQueries.insert(params, "v1.2"), {:ok, _job} <- start_periodic_job(periodic) do {:ok, periodic.id} end @@ -58,7 +58,7 @@ defmodule Scheduler.Actions.PersistImpl do defp update(periodic, request) do with {:ok, params} <- form_periodic_params(request, periodic), - {:ok, periodic} <- PeriodicsQueries.update(periodic, params, "v1.1"), + {:ok, periodic} <- PeriodicsQueries.update(periodic, params, "v1.2"), {:ok, _job} <- start_or_stop_periodic_job(periodic) do {:ok, periodic.id} end @@ -83,7 +83,7 @@ defmodule Scheduler.Actions.PersistImpl do |> Map.take(~w( name description recurring organization_id project_id requester_id - branch pipeline_file at + reference pipeline_file at )a) |> Map.put(:parameters, parameters) |> inject_paused(request.state) @@ -97,7 +97,7 @@ defmodule Scheduler.Actions.PersistImpl do request |> Map.take(~w( name description recurring requester_id - branch pipeline_file at + reference pipeline_file at )a) |> inject_paused(request.state) |> Map.put(:parameters, parameters) diff --git a/periodic_scheduler/scheduler/lib/scheduler/actions/run_now_impl.ex b/periodic_scheduler/scheduler/lib/scheduler/actions/run_now_impl.ex index a1d092762..a390dda25 100644 --- a/periodic_scheduler/scheduler/lib/scheduler/actions/run_now_impl.ex +++ b/periodic_scheduler/scheduler/lib/scheduler/actions/run_now_impl.ex @@ -9,6 +9,7 @@ defmodule Scheduler.Actions.RunNowImpl do alias Scheduler.Periodics.Model.Periodics alias Scheduler.Clients.{ProjecthubClient, RepositoryClient} alias Scheduler.Actions + alias Scheduler.Utils.GitReference alias Util.ToTuple def run_now(params) do @@ -25,8 +26,8 @@ defmodule Scheduler.Actions.RunNowImpl do end end - defp validate_params(%Periodics{branch: nil}, %{branch: ""}), - do: "You have to provide branch for this task" |> ToTuple.error(:INVALID_ARGUMENT) + defp validate_params(%Periodics{reference: nil}, %{reference: ""}), + do: "You have to provide reference for this task" |> ToTuple.error(:INVALID_ARGUMENT) defp validate_params(%Periodics{pipeline_file: nil}, %{pipeline_file: ""}), do: "You have to provide pipeline file for this task" |> ToTuple.error(:INVALID_ARGUMENT) @@ -35,10 +36,10 @@ defmodule Scheduler.Actions.RunNowImpl do periodics = %Periodics{parameters: parameters}, params = %{parameter_values: values} ) do - branch = - if String.length(params.branch) > 1, - do: params.branch, - else: periodics.branch + reference = + if String.length(params.reference) > 1, + do: params.reference, + else: periodics.reference pipeline_file = if String.length(params.pipeline_file) > 1, @@ -49,7 +50,7 @@ defmodule Scheduler.Actions.RunNowImpl do {:ok, values} -> {:ok, params - |> Map.put(:branch, branch) + |> Map.put(:reference, reference) |> Map.put(:pipeline_file, pipeline_file) |> Map.put(:parameter_values, values)} @@ -101,7 +102,9 @@ defmodule Scheduler.Actions.RunNowImpl do do: "The 'requester' parameter can not be empty string." |> ToTuple.error(:INVALID_ARGUMENT) defp verify_revision_exists(periodic, params) do - revision_args = [reference: "refs/heads/" <> params.branch, commit_sha: ""] + git_reference = GitReference.normalize(params.reference) + + revision_args = [reference: git_reference, commit_sha: ""] with {:ok, repository_id} <- fetch_project_repository_id(periodic.project_id), {:ok, commit} <- fetch_branch_revision(repository_id, revision_args) do diff --git a/periodic_scheduler/scheduler/lib/scheduler/actions/schedule_wf_impl.ex b/periodic_scheduler/scheduler/lib/scheduler/actions/schedule_wf_impl.ex index fe8cd90fe..5d5696c14 100644 --- a/periodic_scheduler/scheduler/lib/scheduler/actions/schedule_wf_impl.ex +++ b/periodic_scheduler/scheduler/lib/scheduler/actions/schedule_wf_impl.ex @@ -9,6 +9,7 @@ defmodule Scheduler.Actions.ScheduleWfImpl do alias Scheduler.FrontDB.Model.FrontDBQueries alias Scheduler.Clients.{WorkflowClient, RepoProxyClient, ProjecthubClient, RepositoryClient} alias Scheduler.Workers.ScheduleTaskManager + alias Scheduler.Utils.GitReference alias Util.ToTuple alias LogTee, as: LT @@ -36,39 +37,7 @@ defmodule Scheduler.Actions.ScheduleWfImpl do end end - def schedule_wf(periodic = %{organization_id: org_id}, trigger) do - cond do - FeatureProvider.feature_enabled?(:just_run, param: org_id) -> - schedule_wf_just_run(periodic, trigger) - - FeatureProvider.feature_enabled?(:scheduler_hook, param: org_id) -> - schedule_wf_run_api(periodic, trigger) - - true -> - schedule_wf_db_query(periodic, trigger) - end - end - - defp schedule_wf_run_api(periodic, trigger) do - with {:ok, params} <- form_create_params(periodic, trigger), - {:ok, wf_id} <- RepoProxyClient.create(params) do - params = %{ - scheduled_workflow_id: wf_id, - scheduling_status: "passed", - error_description: nil, - attempts: (trigger.attempts || 0) + 1 - } - - PeriodicsTriggersQueries.update(trigger, params) - else - error -> - Watchman.increment({"PeriodicSch.schedule_wf_failure", ["run-wf-api"]}) - record_error(error, trigger) - error - end - end - - defp schedule_wf_just_run(periodic, trigger) do + def schedule_wf(periodic, trigger) do Watchman.benchmark("PeriodicSch.schedule_just_run", fn -> with {:ok, repository} <- fetch_project_repository(trigger.project_id), {:ok, params} <- form_just_run_schedule_params(periodic, trigger, repository), @@ -92,14 +61,14 @@ defmodule Scheduler.Actions.ScheduleWfImpl do end) end - defp fetch_project_repository(project_id) do + def fetch_project_repository(project_id) do case ProjecthubClient.describe(project_id) do {:ok, project} -> {:ok, project.spec.repository} {:error, _reason} -> {:error, {:missing_project, project_id}} end end - defp fetch_branch_revision(repository_id, revision_args) do + def fetch_branch_revision(repository_id, revision_args) do case RepositoryClient.describe_revision(repository_id, revision_args) do {:ok, commit} -> {:ok, commit} {:error, _reason} -> {:error, {:missing_revision, revision_args}} @@ -117,20 +86,30 @@ defmodule Scheduler.Actions.ScheduleWfImpl do do: :MANUAL_RUN, else: :SCHEDULE + # Handle backwards compatibility: normalize reference to full format + git_reference = GitReference.normalize(trigger.reference) + + # Use legacy format for branch_name field (Plumber compatibility) + branch_name = legacy_branch_name(trigger.reference) + + # Extract clean name for label field + label = GitReference.extract_name(trigger.reference) + %{ service: schedule_workflow_service_type(repository.integration_type), - repo: %{branch_name: trigger.branch}, + repo: %{branch_name: branch_name}, request_token: trigger.periodic_id <> "-#{trigger.id}", project_id: trigger.project_id, requester_id: requester_id, definition_file: trigger.pipeline_file, organization_id: periodic.organization_id, - label: trigger.branch, + label: label, scheduler_task_id: periodic.id, git: %{ - reference: "refs/heads/" <> trigger.branch, + reference: git_reference, commit_sha: "" }, + git_reference: git_reference, triggered_by: triggered_by, env_vars: parameter_values_to_env_vars(trigger.parameter_values) } @@ -151,32 +130,12 @@ defmodule Scheduler.Actions.ScheduleWfImpl do %{name: name, value: if(is_nil(value), do: "", else: value)} end - defp schedule_wf_db_query(periodic, trigger) do - with {:ok, hook} <- FrontDBQueries.get_hook(periodic.project_id, trigger.branch), - {:ok, params} <- form_schedule_params(periodic, trigger, hook), - {:ok, wf_id} <- WorkflowClient.schedule(params) do - params = %{ - scheduled_workflow_id: wf_id, - scheduling_status: "passed", - error_description: nil, - attempts: (trigger.attempts || 0) + 1 - } - - PeriodicsTriggersQueries.update(trigger, params) - else - error -> - Watchman.increment({"PeriodicSch.schedule_wf_failure", ["db-query"]}) - record_error(error, trigger) - error - end - end - # The error is saved in DB after each failed attempt, so we can debug months # later when logs are not available. If scheduling passes in the following # attempt, the error_description field will be cleared. - defp record_error({:error, error}, trigger), do: record_error(error, trigger) + def record_error({:error, error}, trigger), do: record_error(error, trigger) - defp record_error(error, trigger) do + def record_error(error, trigger) do with log_msg <- "Scheduling for periodic #{trigger.periodic_id} failed with error", str_error <- error |> to_str() |> LT.warn(log_msg), str_error <- str_error |> String.slice(0..253), @@ -185,59 +144,23 @@ defmodule Scheduler.Actions.ScheduleWfImpl do end end - defp to_str(val) when is_binary(val), do: val - defp to_str(val), do: "#{inspect(val)}" + def to_str(val) when is_binary(val), do: val + def to_str(val), do: "#{inspect(val)}" - defp form_create_params(periodic, trigger) do - %{ - request_token: trigger.periodic_id <> "-#{trigger.id}", - project_id: trigger.project_id, - requester_id: periodic.requester_id, - definition_file: trigger.pipeline_file, - git: %{ - reference: "refs/heads/" <> trigger.branch, - commit_sha: "" - }, - triggered_by: :SCHEDULE - } - |> ToTuple.ok() + # Legacy branch_name format required by Plumber + defp legacy_branch_name(reference) do + reference + |> GitReference.normalize() + |> legacy_branch_name_format() end - defp form_schedule_params(periodic, trigger, hook) do - hook - |> Map.put(:service, :GIT_HUB) - |> Map.put(:triggered_by, :SCHEDULE) - |> add_trigger_data(periodic, trigger) - |> extract_commit_sha() - end + defp legacy_branch_name_format(full_ref = "refs/tags/" <> _), do: full_ref - defp add_trigger_data(params, periodic, trigger) do - params - |> Map.put(:organization_id, periodic.organization_id) - |> Map.put(:requester_id, periodic.requester_id) - |> Map.put(:definition_file, trigger.pipeline_file) - |> Map.put(:request_token, trigger.periodic_id <> "-#{trigger.id}") + defp legacy_branch_name_format("refs/pull/" <> rest) do + pr_number = rest |> String.trim_trailing("/head") + "pull-request-#{pr_number}" end - defp extract_commit_sha(params) do - with {:ok, payload} <- params.repo.payload |> Jason.decode(), - repo <- params.repo |> Map.delete(:payload), - {:ok, commit_sha} <- find_commit_sha(payload), - repo <- repo |> Map.put(:commit_sha, commit_sha) do - params |> Map.put(:repo, repo) |> ToTuple.ok() - else - error = {:error, _e} -> error - error -> {:error, error} - end - end - - defp find_commit_sha(%{"head_commit" => %{"id" => commit_sha}}) - when is_binary(commit_sha) and commit_sha != "", - do: {:ok, commit_sha} - - defp find_commit_sha(%{"after" => commit_sha}) - when is_binary(commit_sha) and commit_sha != "", - do: {:ok, commit_sha} - - defp find_commit_sha(_params), do: {:error, "Hook is missing commit_sha data"} + defp legacy_branch_name_format("refs/heads/" <> branch_name), do: branch_name + defp legacy_branch_name_format(_), do: nil end diff --git a/periodic_scheduler/scheduler/lib/scheduler/periodics/model/periodics.ex b/periodic_scheduler/scheduler/lib/scheduler/periodics/model/periodics.ex index 7574119c9..382ddfd6f 100644 --- a/periodic_scheduler/scheduler/lib/scheduler/periodics/model/periodics.ex +++ b/periodic_scheduler/scheduler/lib/scheduler/periodics/model/periodics.ex @@ -19,7 +19,7 @@ defmodule Scheduler.Periodics.Model.Periodics do field :description, :string field :project_name, :string field :project_id, :string - field :branch, :string + field :reference, :string, source: :branch field :at, :string field :pipeline_file, :string field :recurring, :boolean, read_after_writes: true, default: true @@ -34,19 +34,19 @@ defmodule Scheduler.Periodics.Model.Periodics do end @required_fields_v1_0 ~w(id requester_id organization_id name project_name - project_id branch at pipeline_file)a + project_id reference at pipeline_file)a @optional_fields_v1_0 ~w(paused pause_toggled_by pause_toggled_at)a @required_fields_update_v1_0 ~w(requester_id organization_id)a - @optional_fields_update_v1_0 ~w(name project_name project_id branch at pipeline_file + @optional_fields_update_v1_0 ~w(name project_name project_id reference at pipeline_file suspended paused pause_toggled_by pause_toggled_at)a @required_fields ~w(id requester_id organization_id name project_name - project_id recurring branch pipeline_file)a + project_id recurring reference pipeline_file)a @optional_fields ~w(description at paused pause_toggled_by pause_toggled_at)a @required_fields_update ~w(requester_id organization_id)a - @optional_fields_update ~w(name project_name project_id branch at pipeline_file recurring + @optional_fields_update ~w(name project_name project_id reference at pipeline_file recurring description suspended paused pause_toggled_by pause_toggled_at)a @doc """ @@ -79,7 +79,7 @@ defmodule Scheduler.Periodics.Model.Periodics do iex> alias Scheduler.Periodics.Model.Periodics iex> params = %{requester_id: UUID.uuid1(), organization_id: UUID.uuid1(), - ...> name: "P1", project_name: "Pr1", branch: "master", project_id: "p1", + ...> name: "P1", project_name: "Pr1", reference: "master", project_id: "p1", ...> at: "* * * * *", id: UUID.uuid1(), pipeline_file: "deploy.yml"} iex> Periodics.changeset(%Periodics{}, "v1.0", params) |> Map.get(:valid?) true @@ -87,7 +87,7 @@ defmodule Scheduler.Periodics.Model.Periodics do iex> alias Scheduler.Periodics.Model.Periodics iex> params = %{requester_id: UUID.uuid1(), organization_id: UUID.uuid1(), id: UUID.uuid1(), ...> name: "P1", project_name: "Pr1", project_id: "p1", - ...> branch: "master", pipeline_file: "deploy.yml", recurring: false, + ...> reference: "master", pipeline_file: "deploy.yml", recurring: false, ...> parameters: [%{name: "foo", required: true, default_value: "bar"}]} iex> Periodics.changeset(%Periodics{}, "v1.1", params) |> Map.get(:valid?) true @@ -102,6 +102,10 @@ defmodule Scheduler.Periodics.Model.Periodics do do_changeset(periodic, params, api_version, @required_fields, @optional_fields) end + def changeset(periodic, api_version = "v1.2", params) do + do_changeset(periodic, params, api_version, @required_fields, @optional_fields) + end + @doc ~S""" ## Examples: @@ -111,7 +115,7 @@ defmodule Scheduler.Periodics.Model.Periodics do iex> alias Scheduler.Periodics.Model.Periodics iex> params = %{requester_id: UUID.uuid1(), organization_id: UUID.uuid1(), at: "* * * * *", - ...> name: "P1", branch: "master", pipeline_file: "deploy.yml"} + ...> name: "P1", reference: "master", pipeline_file: "deploy.yml"} iex> Periodics.changeset_update(%Periodics{}, "v1.0", params) |> Map.get(:valid?) true @@ -136,6 +140,10 @@ defmodule Scheduler.Periodics.Model.Periodics do do_changeset(periodic, params, api_version, @required_fields_update, @optional_fields_update) end + def changeset_update(periodic, api_version = "v1.2", params) do + do_changeset(periodic, params, api_version, @required_fields_update, @optional_fields_update) + end + defp do_changeset(periodic, params, api_version, required_fields, optional_fields) do periodic |> cast(params, required_fields ++ optional_fields) @@ -148,20 +156,27 @@ defmodule Scheduler.Periodics.Model.Periodics do defp maybe_cast_parameters(changeset, "v1.0"), do: changeset defp maybe_cast_parameters(changeset, "v1.1"), do: cast_embed(changeset, :parameters) + defp maybe_cast_parameters(changeset, "v1.2"), do: cast_embed(changeset, :parameters) defp validate_recurring(changeset, "v1.0") do changeset |> put_change(:recurring, true) |> validate_inclusion(:recurring, [true]) - |> validate_required(~w(at branch pipeline_file)a) + |> validate_required(~w(at reference pipeline_file)a) end defp validate_recurring(changeset, "v1.1"), do: validate_recurring(changeset, "v1.1", get_field(changeset, :recurring)) + defp validate_recurring(changeset, "v1.2"), + do: validate_recurring(changeset, "v1.2", get_field(changeset, :recurring)) + defp validate_recurring(changeset, "v1.1", true), do: validate_required(changeset, [:at]) defp validate_recurring(changeset, "v1.1", false), do: changeset + defp validate_recurring(changeset, "v1.2", true), do: validate_required(changeset, [:at]) + defp validate_recurring(changeset, "v1.2", false), do: changeset + defp validate_cron(_field_name, cron_expression) do case Crontab.CronExpression.Parser.parse(cron_expression) do {:ok, %Crontab.CronExpression{}} -> [] diff --git a/periodic_scheduler/scheduler/lib/scheduler/periodics/model/periodics_queries.ex b/periodic_scheduler/scheduler/lib/scheduler/periodics/model/periodics_queries.ex index df96c29cb..40a71ab73 100644 --- a/periodic_scheduler/scheduler/lib/scheduler/periodics/model/periodics_queries.ex +++ b/periodic_scheduler/scheduler/lib/scheduler/periodics/model/periodics_queries.ex @@ -8,6 +8,7 @@ defmodule Scheduler.Periodics.Model.PeriodicsQueries do alias Scheduler.PeriodicsRepo, as: Repo alias Scheduler.Periodics.Model.Periodics + alias Scheduler.Utils.GitReference alias LogTee, as: LT alias Util.ToTuple @@ -15,12 +16,15 @@ defmodule Scheduler.Periodics.Model.PeriodicsQueries do Inserts new Periodic into DB """ def insert(params, api_version \\ "v1.1") do - params = params |> Map.put(:id, UUID.uuid4()) + processed_params = + params + |> Map.put(:id, UUID.uuid4()) + |> preprocess_reference_field(api_version) %Periodics{} - |> Periodics.changeset(api_version, params) + |> Periodics.changeset(api_version, processed_params) |> Repo.insert() - |> process_response(params) + |> process_response(processed_params) rescue e -> {:error, e} catch @@ -44,8 +48,8 @@ defmodule Scheduler.Periodics.Model.PeriodicsQueries do {:error, "The 'at' parameter can not be empty string."} end - defp process_response({:error, %{errors: [branch: {"can't be blank", _msg}]}}, _p) do - {:error, "The 'branch' parameter can not be empty string."} + defp process_response({:error, %{errors: [reference: {"can't be blank", _msg}]}}, _p) do + {:error, "The 'reference' parameter can not be empty string."} end defp process_response({:error, %{errors: [pipeline_file: {"can't be blank", _msg}]}}, _p) do @@ -69,10 +73,12 @@ defmodule Scheduler.Periodics.Model.PeriodicsQueries do Updates Periodic record with given params """ def update(periodic, params, api_version \\ "v1.1") do + processed_params = preprocess_reference_field(params, api_version) + periodic - |> Periodics.changeset_update(api_version, params) + |> Periodics.changeset_update(api_version, processed_params) |> Repo.update() - |> process_response(params) + |> process_response(processed_params) rescue e -> {:error, e} catch @@ -283,7 +289,7 @@ defmodule Scheduler.Periodics.Model.PeriodicsQueries do name: per.name, recurring: per.recurring, project_id: per.project_id, - branch: per.branch, + reference: per.reference, at: per.at, pipeline_file: per.pipeline_file, requester_id: per.requester_id, @@ -314,6 +320,23 @@ defmodule Scheduler.Periodics.Model.PeriodicsQueries do parameter |> Map.take(~w(name required description default_value options)a) end + defp preprocess_reference_field(params, "v1.0") do + case Map.get(params, :reference) do + nil -> + params + + reference when is_binary(reference) -> + Map.put(params, :reference, GitReference.normalize(reference)) + + _ -> + params + end + end + + defp preprocess_reference_field(params, "v1.1"), do: preprocess_reference_field(params, "v1.0") + + defp preprocess_reference_field(params, _api_version), do: params + # Utility defp return_tuple(nil, nil_msg), do: ToTuple.error(nil_msg) diff --git a/periodic_scheduler/scheduler/lib/scheduler/periodics_triggers/model/history_page.ex b/periodic_scheduler/scheduler/lib/scheduler/periodics_triggers/model/history_page.ex index c1469f677..1c14e7723 100644 --- a/periodic_scheduler/scheduler/lib/scheduler/periodics_triggers/model/history_page.ex +++ b/periodic_scheduler/scheduler/lib/scheduler/periodics_triggers/model/history_page.ex @@ -190,8 +190,16 @@ defmodule Scheduler.PeriodicsTriggers.Model.HistoryPage do defp apply_filter(query, {:triggered_by, triggered_by}), do: Ecto.Query.where(query, [dt, s], dt.run_now_requester_id == ^triggered_by) - defp apply_filter(query, {:branch_name, branch_name}), - do: Ecto.Query.where(query, [dt, s], dt.branch == ^branch_name) + defp apply_filter( + query, + {:reference, %{normalized: normalized, short: short, original: original}} + ) do + possible_values = [normalized, short, original] |> Enum.uniq() + Ecto.Query.where(query, [dt, s], dt.reference in ^possible_values) + end + + defp apply_filter(query, {:reference, reference}) when is_binary(reference), + do: Ecto.Query.where(query, [dt, s], dt.reference == ^reference) defp apply_filter(query, {:pipeline_file, pipeline_file}), do: Ecto.Query.where(query, [dt, s], dt.pipeline_file == ^pipeline_file) diff --git a/periodic_scheduler/scheduler/lib/scheduler/periodics_triggers/model/periodics_triggers.ex b/periodic_scheduler/scheduler/lib/scheduler/periodics_triggers/model/periodics_triggers.ex index 50b9538fa..903631690 100644 --- a/periodic_scheduler/scheduler/lib/scheduler/periodics_triggers/model/periodics_triggers.ex +++ b/periodic_scheduler/scheduler/lib/scheduler/periodics_triggers/model/periodics_triggers.ex @@ -18,7 +18,7 @@ defmodule Scheduler.PeriodicsTriggers.Model.PeriodicsTriggers do belongs_to :periodics, Periodics, type: Ecto.UUID, foreign_key: :periodic_id field :triggered_at, :utc_datetime_usec field :project_id, :string - field :branch, :string + field :reference, :string, source: :branch field :pipeline_file, :string field :scheduling_status, :string field :recurring, :boolean @@ -32,7 +32,7 @@ defmodule Scheduler.PeriodicsTriggers.Model.PeriodicsTriggers do timestamps() end - @required_fields_insert ~w(periodic_id triggered_at project_id branch + @required_fields_insert ~w(periodic_id triggered_at project_id reference pipeline_file scheduling_status recurring)a @optional_fields_insert ~w(run_now_requester_id)a @@ -50,7 +50,7 @@ defmodule Scheduler.PeriodicsTriggers.Model.PeriodicsTriggers do iex> alias Scheduler.PeriodicsTriggers.Model.PeriodicsTriggers iex> params = %{periodic_id: UUID.uuid1(), triggered_at: DateTime.utc_now(), - ...> branch: "master", project_id: "p1", pipeline_file: "deploy.yml", + ...> reference: "master", project_id: "p1", pipeline_file: "deploy.yml", ...> scheduling_status: "running", recurring: true, ...> parameter_values: [%{name: "p1", value: "v1"}]} iex> PeriodicsTriggers.changeset_insert(%PeriodicsTriggers{}, params) |> Map.get(:valid?) diff --git a/periodic_scheduler/scheduler/lib/scheduler/periodics_triggers/model/periodics_triggers_queries.ex b/periodic_scheduler/scheduler/lib/scheduler/periodics_triggers/model/periodics_triggers_queries.ex index a015d2c4a..0ed062091 100644 --- a/periodic_scheduler/scheduler/lib/scheduler/periodics_triggers/model/periodics_triggers_queries.ex +++ b/periodic_scheduler/scheduler/lib/scheduler/periodics_triggers/model/periodics_triggers_queries.ex @@ -9,6 +9,7 @@ defmodule Scheduler.PeriodicsTriggers.Model.PeriodicsTriggersQueries do alias Scheduler.PeriodicsRepo, as: Repo alias Scheduler.Periodics.Model.Periodics alias Scheduler.PeriodicsTriggers.Model.PeriodicsTriggers + alias Scheduler.Utils.GitReference alias LogTee, as: LT alias Util.ToTuple @@ -36,14 +37,25 @@ defmodule Scheduler.PeriodicsTriggers.Model.PeriodicsTriggersQueries do triggered_at: DateTime.utc_now(), project_id: periodic.project_id, recurring: periodic.recurring, - branch: periodic.branch, + reference: periodic.reference, pipeline_file: periodic.pipeline_file, parameter_values: Periodics.default_parameter_values(periodic), scheduling_status: "running", run_now_requester_id: params[:requester] } - default_params |> Map.merge(params) |> insert_() + merged_params = default_params |> Map.merge(params) + + normalized_params = + case merged_params.reference do + ref when is_binary(ref) -> + Map.put(merged_params, :reference, GitReference.normalize(ref)) + + _ -> + merged_params + end + + insert_(normalized_params) end defp insert_(params) do diff --git a/periodic_scheduler/scheduler/lib/scheduler/utils/git_reference.ex b/periodic_scheduler/scheduler/lib/scheduler/utils/git_reference.ex new file mode 100644 index 000000000..df9f9ba9a --- /dev/null +++ b/periodic_scheduler/scheduler/lib/scheduler/utils/git_reference.ex @@ -0,0 +1,126 @@ +defmodule Scheduler.Utils.GitReference do + @moduledoc """ + Utilities for handling Git reference normalization and conversion. + + Git references can come in various formats: + - Branch names: "master", "develop", "feature-branch" + - Full branch refs: "refs/heads/master", "refs/heads/develop" + - Tag refs: "refs/tags/v1.0.0", "refs/tags/release" + - PR refs: "refs/pull/123/head" + """ + + @doc """ + Normalizes a git reference to its full form. + + ## Examples + + iex> Scheduler.Utils.GitReference.normalize("master") + "refs/heads/master" + + iex> Scheduler.Utils.GitReference.normalize("refs/heads/master") + "refs/heads/master" + + iex> Scheduler.Utils.GitReference.normalize("refs/tags/v1.0.0") + "refs/tags/v1.0.0" + + iex> Scheduler.Utils.GitReference.normalize("refs/pull/123/head") + "refs/pull/123/head" + """ + def normalize(reference) when is_binary(reference) do + if String.starts_with?(reference, "refs/") do + reference + else + "refs/heads/" <> reference + end + end + + def normalize(nil), do: nil + + @doc """ + Extracts the short name from a full git reference. + + ## Examples + + iex> Scheduler.Utils.GitReference.extract_name("refs/heads/master") + "master" + + iex> Scheduler.Utils.GitReference.extract_name("refs/tags/v1.0.0") + "v1.0.0" + + iex> Scheduler.Utils.GitReference.extract_name("refs/pull/123/head") + "123/head" + + iex> Scheduler.Utils.GitReference.extract_name("feature-branch") + "feature-branch" + """ + def extract_name(reference) when is_binary(reference) do + cond do + String.starts_with?(reference, "refs/heads/") -> + String.replace_prefix(reference, "refs/heads/", "") + + String.starts_with?(reference, "refs/tags/") -> + String.replace_prefix(reference, "refs/tags/", "") + + String.starts_with?(reference, "refs/pull/") -> + String.replace_prefix(reference, "refs/pull/", "") + + true -> + reference + end + end + + def extract_name(nil), do: nil + + @doc """ + Builds a full git reference from type and name. + + ## Examples + + iex> Scheduler.Utils.GitReference.build_full_reference("BRANCH", "master") + "refs/heads/master" + + iex> Scheduler.Utils.GitReference.build_full_reference("TAG", "v1.0.0") + "refs/tags/v1.0.0" + + iex> Scheduler.Utils.GitReference.build_full_reference("PR", "123") + "refs/pull/123/head" + + iex> Scheduler.Utils.GitReference.build_full_reference("UNKNOWN", "something") + "something" + """ + def build_full_reference("BRANCH", name), do: "refs/heads/" <> name + def build_full_reference("TAG", name), do: "refs/tags/" <> name + def build_full_reference("PR", name), do: "refs/pull/" <> name <> "/head" + def build_full_reference(_unknown_type, name), do: name + + @doc """ + Determines the type of git reference. + + ## Examples + + iex> Scheduler.Utils.GitReference.get_type("refs/heads/master") + :branch + + iex> Scheduler.Utils.GitReference.get_type("refs/tags/v1.0.0") + :tag + + iex> Scheduler.Utils.GitReference.get_type("refs/pull/123/head") + :pull_request + + iex> Scheduler.Utils.GitReference.get_type("master") + :branch + + iex> Scheduler.Utils.GitReference.get_type("refs/invalid") + :branch + """ + def get_type(reference) when is_binary(reference) do + cond do + String.starts_with?(reference, "refs/heads/") -> :branch + String.starts_with?(reference, "refs/tags/") -> :tag + String.starts_with?(reference, "refs/pull/") -> :pull_request + true -> :branch + end + end + + def get_type(nil), do: nil +end diff --git a/periodic_scheduler/scheduler/mix.lock b/periodic_scheduler/scheduler/mix.lock index df71982e2..eceed9f51 100644 --- a/periodic_scheduler/scheduler/mix.lock +++ b/periodic_scheduler/scheduler/mix.lock @@ -63,7 +63,7 @@ "tzdata": {:hex, :tzdata, "1.1.1", "20c8043476dfda8504952d00adac41c6eda23912278add38edc140ae0c5bcc46", [:mix], [{:hackney, "~> 1.17", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm", "a69cec8352eafcd2e198dea28a34113b60fdc6cb57eb5ad65c10292a6ba89787"}, "unicode_util_compat": {:hex, :unicode_util_compat, "0.7.0", "bc84380c9ab48177092f43ac89e4dfa2c6d62b40b8bd132b1059ecc7232f9a78", [:rebar3], [], "hexpm", "25eee6d67df61960cf6a794239566599b09e17e668d3700247bc498638152521"}, "unsafe": {:hex, :unsafe, "1.0.1", "a27e1874f72ee49312e0a9ec2e0b27924214a05e3ddac90e91727bc76f8613d8", [:mix], [], "hexpm", "6c7729a2d214806450d29766abc2afaa7a2cbecf415be64f36a6691afebb50e5"}, - "util": {:git, "https://github.com/renderedtext/elixir-util.git", "7a35f61a97ea37b8a5861a1e6aa00d39ffb54f12", []}, + "util": {:git, "https://github.com/renderedtext/elixir-util.git", "34548a46450465b2fc69642d81f3ff282b1aa097", []}, "uuid": {:hex, :uuid, "1.1.8", "e22fc04499de0de3ed1116b770c7737779f226ceefa0badb3592e64d5cfb4eb9", [:mix], [], "hexpm", "c790593b4c3b601f5dc2378baae7efaf5b3d73c4c6456ba85759905be792f2ac"}, "vmstats": {:hex, :vmstats, "2.4.0", "57763d3edc861da2de02b2f0df3adc37e5a140117147937e83908cac40a10c9b", [:rebar3], [], "hexpm", "266f464e64b459f614ed64e25bda270f8a77951868bbaf0e2209274efc3d9b6d"}, "watchman": {:git, "https://github.com/renderedtext/ex-watchman.git", "b7a205b47a34106a3c865f7dde624381d795a876", []}, diff --git a/periodic_scheduler/scheduler/test/actions/apply_impl_test.exs b/periodic_scheduler/scheduler/test/actions/apply_impl_test.exs index e69d8d1ce..fd605e1f1 100644 --- a/periodic_scheduler/scheduler/test/actions/apply_impl_test.exs +++ b/periodic_scheduler/scheduler/test/actions/apply_impl_test.exs @@ -11,6 +11,9 @@ defmodule Test.Actions.ApplyImpl.Test do start_supervised!(QuantumScheduler) + # Ensure v1.2 schema is cached for tests that use it + GenServer.call(DefinitionValidator.YamlMapValidator.Server, {:cache_schema, "v1.2"}) + {:ok, params} end @@ -154,6 +157,76 @@ defmodule Test.Actions.ApplyImpl.Test do refute periodic_id |> String.to_atom() |> QuantumScheduler.find_job() end + test "apply v1.2 doesn't start quantum job when periodic is non-recurring", ctx do + yml_definition = """ + apiVersion: v1.2 + kind: Schedule + metadata: + name: test periodic v1.2 + spec: + project: Project 1 + recurring: false + reference: + type: BRANCH + name: master + at: "" + pipeline_file: .semaphore/cron.yml + parameters: + - name: example + description: Exemplary parameter + required: true + default_value: option1 + options: + - option1 + - option2 + - option3 + """ + + assert {:ok, periodic_id} = + ApplyImpl.apply(%{ + organization_id: ctx.org_id, + requester_id: ctx.usr_id, + yml_definition: yml_definition + }) + + refute periodic_id |> String.to_atom() |> QuantumScheduler.find_job() + end + + test "apply v1.2 does start quantum job when periodic is recurring", ctx do + yml_definition = """ + apiVersion: v1.2 + kind: Schedule + metadata: + name: test periodic v1.2 + spec: + project: Project 1 + recurring: true + reference: + type: BRANCH + name: master + at: "0 0 * * *" + pipeline_file: .semaphore/cron.yml + parameters: + - name: example + description: Exemplary parameter + required: true + default_value: option1 + options: + - option1 + - option2 + - option3 + """ + + assert {:ok, periodic_id} = + ApplyImpl.apply(%{ + organization_id: ctx.org_id, + requester_id: ctx.usr_id, + yml_definition: yml_definition + }) + + assert periodic_id |> String.to_atom() |> QuantumScheduler.find_job() + end + defp insert_periodics(ids, extra \\ %{}) do %{ requester_id: ids.usr_id, @@ -162,7 +235,7 @@ defmodule Test.Actions.ApplyImpl.Test do project_name: "Project_1", recurring: if(is_nil(extra[:recurring]), do: true, else: extra[:recurring]), project_id: ids.pr_id, - branch: extra[:branch] || "master", + reference: extra[:reference] || "master", at: extra[:at] || "0 0 * * *", paused: if(is_nil(extra[:paused]), do: false, else: extra[:paused]), pipeline_file: extra[:pipeline_file] || "deploy.yml", diff --git a/periodic_scheduler/scheduler/test/actions/describe_impl_test.exs b/periodic_scheduler/scheduler/test/actions/describe_impl_test.exs index fe0b9f9cf..1109453b1 100644 --- a/periodic_scheduler/scheduler/test/actions/describe_impl_test.exs +++ b/periodic_scheduler/scheduler/test/actions/describe_impl_test.exs @@ -14,7 +14,7 @@ defmodule Test.Actions.DescribeImpl.Test do project_name: "Project_1", project_id: UUID.uuid4(), recurring: true, - branch: "master", + reference: "master", at: "* * * * *", pipeline_file: "deploy.yml", parameters: [ diff --git a/periodic_scheduler/scheduler/test/actions/history_impl_test.exs b/periodic_scheduler/scheduler/test/actions/history_impl_test.exs index d92b41952..1da8a4d60 100644 --- a/periodic_scheduler/scheduler/test/actions/history_impl_test.exs +++ b/periodic_scheduler/scheduler/test/actions/history_impl_test.exs @@ -81,7 +81,7 @@ defmodule Test.Actions.HistoryImpl.Test do test "periodic has many triggers and filters are passed => list page", ctx do insert_triggers_for_past(ctx, 60, :minutes) - insert_triggers_for_past(ctx, 5, :minutes, branch: "develop") + insert_triggers_for_past(ctx, 5, :minutes, reference: "develop") insert_triggers_for_past(ctx, 5, :minutes, pipeline_file: ".semaphore/semaphore.yml") insert_triggers_for_past(ctx, 5, :minutes, triggered_by: "scheduler") diff --git a/periodic_scheduler/scheduler/test/actions/persist_impl_test.exs b/periodic_scheduler/scheduler/test/actions/persist_impl_test.exs index 3edff5388..6a137f98f 100644 --- a/periodic_scheduler/scheduler/test/actions/persist_impl_test.exs +++ b/periodic_scheduler/scheduler/test/actions/persist_impl_test.exs @@ -24,7 +24,7 @@ defmodule Test.Actions.PersistImpl.Test do organization_id: ctx.org_id, project_id: ctx.pr_id, requester_id: ctx.usr_id, - branch: "master", + reference: "master", pipeline_file: ".semaphore/cron.yml", at: "", parameters: [ @@ -47,7 +47,7 @@ defmodule Test.Actions.PersistImpl.Test do assert periodic.project_name == "Project 1" assert periodic.project_id == ctx.pr_id assert periodic.requester_id == ctx.usr_id - assert periodic.branch == "master" + assert periodic.reference == "master" assert periodic.pipeline_file == ".semaphore/cron.yml" refute periodic.at @@ -72,7 +72,7 @@ defmodule Test.Actions.PersistImpl.Test do organization_id: ctx.org_id, project_id: ctx.pr_id, requester_id: ctx.usr_id, - branch: "master", + reference: "master", pipeline_file: ".semaphore/cron.yml", at: "0 0 * * *", parameters: [] @@ -87,7 +87,7 @@ defmodule Test.Actions.PersistImpl.Test do assert periodic.project_name == "Project 1" assert periodic.project_id == ctx.pr_id assert periodic.requester_id == ctx.usr_id - assert periodic.branch == "master" + assert periodic.reference == "master" assert periodic.pipeline_file == ".semaphore/cron.yml" assert periodic.at == "0 0 * * *" assert periodic.parameters == [] @@ -112,7 +112,7 @@ defmodule Test.Actions.PersistImpl.Test do name: "test periodic new", recurring: false, requester_id: UUID.uuid4(), - branch: "master", + reference: "master", pipeline_file: ".semaphore/cron.yml", at: "", parameters: [ @@ -136,7 +136,7 @@ defmodule Test.Actions.PersistImpl.Test do assert periodic.project_name == "Project" assert periodic.project_id == ctx.pr_id assert periodic.requester_id != ctx.usr_id - assert periodic.branch == "master" + assert periodic.reference == "master" assert periodic.pipeline_file == ".semaphore/cron.yml" refute periodic.at @@ -169,7 +169,7 @@ defmodule Test.Actions.PersistImpl.Test do name: "test periodic new", recurring: false, requester_id: UUID.uuid4(), - branch: "master", + reference: "master", pipeline_file: ".semaphore/cron.yml", at: "", parameters: [ diff --git a/periodic_scheduler/scheduler/test/actions/run_now_impl_test.exs b/periodic_scheduler/scheduler/test/actions/run_now_impl_test.exs index c55164a99..3008cb747 100644 --- a/periodic_scheduler/scheduler/test/actions/run_now_impl_test.exs +++ b/periodic_scheduler/scheduler/test/actions/run_now_impl_test.exs @@ -60,19 +60,21 @@ defmodule Scheduler.Actions.RunNowImpl.Test do assert {:ok, %{trigger: trigger}} = RunNowImpl.run_now(run_now_params(periodics)) - assert %{branch: "master", pipeline_file: "deploy.yml", parameter_values: []} = trigger + assert %{reference: "refs/heads/master", pipeline_file: "deploy.yml", parameter_values: []} = + trigger end test "when periodic has default branch and pipeline file then these are overriden", ctx do assert {:ok, periodics} = - insert_periodics(ctx.ids, %{branch: "develop", pipeline_file: "test.yml"}) + insert_periodics(ctx.ids, %{reference: "develop", pipeline_file: "test.yml"}) assert {:ok, %{trigger: trigger}} = RunNowImpl.run_now( - run_now_params(periodics, %{branch: "master", pipeline_file: "deploy.yml"}) + run_now_params(periodics, %{reference: "master", pipeline_file: "deploy.yml"}) ) - assert %{branch: "master", pipeline_file: "deploy.yml", parameter_values: []} = trigger + assert %{reference: "refs/heads/master", pipeline_file: "deploy.yml", parameter_values: []} = + trigger end test "when periodic has required parameters without defaults and value is not provided then returns error", @@ -284,7 +286,7 @@ defmodule Scheduler.Actions.RunNowImpl.Test do project_name: "Project_1", recurring: if(is_nil(extra[:recurring]), do: true, else: extra[:recurring]), project_id: ids.pr_id, - branch: extra[:branch] || "master", + reference: extra[:reference] || "master", at: extra[:at] || "* * * * *", pipeline_file: extra[:pipeline_file] || "deploy.yml", parameters: extra[:parameters] || [] @@ -296,7 +298,7 @@ defmodule Scheduler.Actions.RunNowImpl.Test do %{ id: periodic.id, requester: extra[:requester_id] || periodic.requester_id, - branch: extra[:branch] || "", + reference: extra[:reference] || "", pipeline_file: extra[:pipeline_file] || "", parameter_values: extra[:parameter_values] || [] } diff --git a/periodic_scheduler/scheduler/test/actions/schedule_wf_impl_test.exs b/periodic_scheduler/scheduler/test/actions/schedule_wf_impl_test.exs index e02688c9a..e033466f5 100644 --- a/periodic_scheduler/scheduler/test/actions/schedule_wf_impl_test.exs +++ b/periodic_scheduler/scheduler/test/actions/schedule_wf_impl_test.exs @@ -2,7 +2,6 @@ defmodule Scheduler.Actions.ScheduleWfImpl.Test do use ExUnit.Case alias Scheduler.Actions.ScheduleWfImpl - alias Scheduler.Workers.ScheduleTask alias Scheduler.Workers.ScheduleTaskManager alias Scheduler.Periodics.Model.Periodics alias Scheduler.Periodics.Model.PeriodicsQueries @@ -50,258 +49,26 @@ defmodule Scheduler.Actions.ScheduleWfImpl.Test do name: "Periodic_1", project_name: "Project_1", project_id: ids.pr_id, - branch: "master", + reference: "master", at: "* * * * *", pipeline_file: "deploy.yml" } end - test "schedule() - schedule params are correctly formed when there is only after in hook payload", - ctx do - use_mock_workflow_service() - mock_workflow_service_response("ok") + describe "just_run scheduling implementation" do + test "schedule() - schedule params are correctly formed for JustRun case", ctx do + use_mock_workflow_service() + mock_workflow_service_response("just_run") + reset_mock_feature_service() + mock_feature_response("just_run") + use_mock_project_service() + mock_projecthub_response("ok") + use_mock_repository_service() + mock_repositoryhub_response("ok") - ts_before = DateTime.utc_now() + ts_before = DateTime.utc_now() + timestamp = Timex.shift(ts_before, minutes: -1) - timestamp = Timex.shift(ts_before, minutes: -1) - - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - - :timer.sleep(2_000) - - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr.scheduling_status == "passed" - assert tr.scheduled_workflow_id == "wf_id" - assert tr.error_description == nil - assert tr.attempts == 1 - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - end - - test "schedule() - schedule params are correctly formed for JustRun case", ctx do - use_mock_workflow_service() - mock_workflow_service_response("just_run") - reset_mock_feature_service() - mock_feature_response("just_run") - use_mock_project_service() - mock_projecthub_response("ok") - use_mock_repository_service() - mock_repositoryhub_response("ok") - - ts_before = DateTime.utc_now() - timestamp = Timex.shift(ts_before, minutes: -1) - - ctx.periodic - |> Periodics.changeset("v1.1", %{ - parameters: [ - %{name: "param1", required: true, default_value: "value1"}, - %{name: "param2", required: false, default_value: "value2"}, - %{name: "param3", required: false} - ] - }) - |> Scheduler.PeriodicsRepo.update!() - - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - - :timer.sleep(2_000) - - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - - assert tr.scheduling_status == "passed" - assert tr.scheduled_workflow_id == "wf_id" - assert tr.error_description == nil - assert tr.attempts == 1 - - assert Map.new(tr.parameter_values, &{&1.name, &1.value}) == - %{"param1" => "value1", "param2" => "value2"} - - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - end - - test "schedule() - schedule params are correctly formed for bitbucket in JustRun case", ctx do - alias Scheduler.Actions.ScheduleWfImpl - - use_mock_workflow_service() - mock_workflow_service_response("just_run") - reset_mock_feature_service() - mock_feature_response("just_run") - use_mock_project_service() - mock_projecthub_response("ok") - use_mock_repository_service() - mock_repositoryhub_response("ok") - - ts_before = DateTime.utc_now() - timestamp = Timex.shift(ts_before, minutes: -1) - - periodic = - ctx.periodic - |> Periodics.changeset("v1.1", %{ - parameters: [ - %{name: "param1", required: true, default_value: "value1"}, - %{name: "param2", required: false, default_value: "value2"}, - %{name: "param3", required: false} - ] - }) - |> Scheduler.PeriodicsRepo.update!() - - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - - :timer.sleep(2_000) - - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - - assert tr.scheduling_status == "passed" - assert tr.scheduled_workflow_id == "wf_id" - assert tr.error_description == nil - - assert Map.new(tr.parameter_values, &{&1.name, &1.value}) == - %{"param1" => "value1", "param2" => "value2"} - - assert tr.attempts == 1 - - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - - repository = %{id: UUID.uuid4(), integration_type: :BITBUCKET} - - assert {:ok, schedule_params} = - ScheduleWfImpl.form_just_run_schedule_params(periodic, tr, repository) - - assert schedule_params.service == :BITBUCKET - assert schedule_params.requester_id == periodic.requester_id - assert schedule_params.triggered_by == :SCHEDULE - end - - test "schedule() - schedule params are correctly formed for gitlab in JustRun case", ctx do - alias Scheduler.Actions.ScheduleWfImpl - - use_mock_workflow_service() - mock_workflow_service_response("just_run") - reset_mock_feature_service() - mock_feature_response("just_run") - use_mock_project_service() - mock_projecthub_response("ok") - use_mock_repository_service() - mock_repositoryhub_response("ok") - - ts_before = DateTime.utc_now() - timestamp = Timex.shift(ts_before, minutes: -1) - - periodic = - ctx.periodic - |> Periodics.changeset("v1.1", %{ - parameters: [ - %{name: "param_gitlab1", required: true, default_value: "value1"}, - %{name: "param_gitlab2", required: false, default_value: "value2"}, - %{name: "param_gitlab3", required: false, default_value: "value3"}, - %{name: "param_gitlab4", required: false} - ] - }) - |> Scheduler.PeriodicsRepo.update!() - - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - - :timer.sleep(2_000) - - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - - assert tr.scheduling_status == "passed" - assert tr.scheduled_workflow_id == "wf_id" - assert tr.error_description == nil - - assert Map.new(tr.parameter_values, &{&1.name, &1.value}) == - %{ - "param_gitlab1" => "value1", - "param_gitlab2" => "value2", - "param_gitlab3" => "value3" - } - - assert tr.attempts == 1 - - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - - repository = %{id: UUID.uuid4(), integration_type: :GITLAB} - - assert {:ok, schedule_params} = - ScheduleWfImpl.form_just_run_schedule_params(periodic, tr, repository) - - assert schedule_params.service == :GITLAB - assert schedule_params.requester_id == periodic.requester_id - assert schedule_params.triggered_by == :SCHEDULE - end - - test "schedule() - schedule params are correctly formed for git agnostic in JustRun case", - ctx do - alias Scheduler.Actions.ScheduleWfImpl - - use_mock_workflow_service() - mock_workflow_service_response("just_run") - reset_mock_feature_service() - mock_feature_response("just_run") - use_mock_project_service() - mock_projecthub_response("ok") - use_mock_repository_service() - mock_repositoryhub_response("ok") - - ts_before = DateTime.utc_now() - timestamp = Timex.shift(ts_before, minutes: -1) - - periodic = - ctx.periodic - |> Periodics.changeset("v1.1", %{ - parameters: [ - %{name: "param_git1", required: true, default_value: "value1"}, - %{name: "param_git2", required: false, default_value: "value2"}, - %{name: "param_git3", required: false, default_value: "value3"}, - %{name: "param_git4", required: false} - ] - }) - |> Scheduler.PeriodicsRepo.update!() - - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - - :timer.sleep(2_000) - - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - - assert tr.scheduling_status == "passed" - assert tr.scheduled_workflow_id == "wf_id" - assert tr.error_description == nil - - assert Map.new(tr.parameter_values, &{&1.name, &1.value}) == - %{ - "param_git1" => "value1", - "param_git2" => "value2", - "param_git3" => "value3" - } - - assert tr.attempts == 1 - - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - - repository = %{id: UUID.uuid4(), integration_type: :GIT} - - assert {:ok, schedule_params} = - ScheduleWfImpl.form_just_run_schedule_params(periodic, tr, repository) - - assert schedule_params.service == :GIT - assert schedule_params.requester_id == periodic.requester_id - assert schedule_params.triggered_by == :SCHEDULE - end - - test "schedule() - when project service fails for JustRun case then workflow is not scheduled", - ctx do - use_mock_workflow_service() - mock_workflow_service_response("just_run") - reset_mock_feature_service() - mock_feature_response("just_run") - use_mock_project_service() - mock_projecthub_response("failed_precondition") - use_mock_repository_service() - mock_repositoryhub_response("ok") - - ts_before = DateTime.utc_now() - timestamp = Timex.shift(ts_before, minutes: -1) - - periodic = ctx.periodic |> Periodics.changeset("v1.1", %{ parameters: [ @@ -312,371 +79,498 @@ defmodule Scheduler.Actions.ScheduleWfImpl.Test do }) |> Scheduler.PeriodicsRepo.update!() - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) + assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - :timer.sleep(2_000) + :timer.sleep(2_000) - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) + assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr.scheduling_status == "failed" - assert tr.scheduled_workflow_id == "" - assert tr.error_description == "{:missing_project, \"#{periodic.project_id}\"}" + assert tr.scheduling_status == "passed" + assert tr.scheduled_workflow_id == "wf_id" + assert tr.error_description == nil + assert tr.attempts == 1 - assert Map.new(tr.parameter_values, &{&1.name, &1.value}) == - %{"param1" => "value1", "param2" => "value2"} + assert Map.new(tr.parameter_values, &{&1.name, &1.value}) == + %{"param1" => "value1", "param2" => "value2"} - assert tr.attempts >= 1 + assert DateTime.compare(tr.scheduled_at, ts_before) == :gt + end - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - end + test "schedule() - schedule params are correctly formed for bitbucket in JustRun case", ctx do + alias Scheduler.Actions.ScheduleWfImpl - test "schedule() - when repository service fails for JustRun case then workflow is not scheduled", - ctx do - use_mock_workflow_service() - mock_workflow_service_response("just_run") - reset_mock_feature_service() - mock_feature_response("just_run") - use_mock_project_service() - mock_projecthub_response("ok") - use_mock_repository_service() - mock_repositoryhub_response("failed_precondition") + use_mock_workflow_service() + mock_workflow_service_response("just_run") + reset_mock_feature_service() + mock_feature_response("just_run") + use_mock_project_service() + mock_projecthub_response("ok") + use_mock_repository_service() + mock_repositoryhub_response("ok") - ts_before = DateTime.utc_now() - timestamp = Timex.shift(ts_before, minutes: -1) + ts_before = DateTime.utc_now() + timestamp = Timex.shift(ts_before, minutes: -1) - ctx.periodic - |> Periodics.changeset("v1.1", %{ - parameters: [ - %{name: "param1", required: true, default_value: "value1"}, - %{name: "param2", required: false, default_value: "value2"}, - %{name: "param3", required: false} - ] - }) - |> Scheduler.PeriodicsRepo.update!() + periodic = + ctx.periodic + |> Periodics.changeset("v1.1", %{ + parameters: [ + %{name: "param1", required: true, default_value: "value1"}, + %{name: "param2", required: false, default_value: "value2"}, + %{name: "param3", required: false} + ] + }) + |> Scheduler.PeriodicsRepo.update!() - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) + assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - :timer.sleep(2_000) + :timer.sleep(2_000) - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) + assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr.scheduling_status == "failed" - assert tr.scheduled_workflow_id == "" + assert tr.scheduling_status == "passed" + assert tr.scheduled_workflow_id == "wf_id" + assert tr.error_description == nil - assert tr.error_description == - "{:missing_revision, [commit_sha: \"\", reference: \"refs/heads/master\"]}" + assert Map.new(tr.parameter_values, &{&1.name, &1.value}) == + %{"param1" => "value1", "param2" => "value2"} - assert Map.new(tr.parameter_values, &{&1.name, &1.value}) == - %{"param1" => "value1", "param2" => "value2"} + assert tr.attempts == 1 - assert tr.attempts >= 1 + assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - end + repository = %{id: UUID.uuid4(), integration_type: :BITBUCKET} - test "schedule() - when using Create API schedule params are correctly formed and proper API is called", - %{project_id: project_id, org_id: org_id} do - use_mock_repo_proxy_service(project_id) - mock_repo_proxy_service_response("ok") - reset_mock_feature_service() - mock_feature_response("scheduler_hook") + assert {:ok, schedule_params} = + ScheduleWfImpl.form_just_run_schedule_params(periodic, tr, repository) - ids = %{ - usr_id: UUID.uuid4(), - org_id: org_id, - pr_id: project_id - } + assert schedule_params.service == :BITBUCKET + assert schedule_params.requester_id == periodic.requester_id + assert schedule_params.triggered_by == :SCHEDULE + end - assert {:ok, periodic} = periodic_params(ids) |> PeriodicsQueries.insert() + test "schedule() - schedule params are correctly formed for gitlab in JustRun case", ctx do + alias Scheduler.Actions.ScheduleWfImpl - ts_before = DateTime.utc_now() + use_mock_workflow_service() + mock_workflow_service_response("just_run") + reset_mock_feature_service() + mock_feature_response("just_run") + use_mock_project_service() + mock_projecthub_response("ok") + use_mock_repository_service() + mock_repositoryhub_response("ok") - timestamp = Timex.shift(ts_before, minutes: -1) + ts_before = DateTime.utc_now() + timestamp = Timex.shift(ts_before, minutes: -1) - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(periodic.id, timestamp) + periodic = + ctx.periodic + |> Periodics.changeset("v1.1", %{ + parameters: [ + %{name: "param_gitlab1", required: true, default_value: "value1"}, + %{name: "param_gitlab2", required: false, default_value: "value2"}, + %{name: "param_gitlab3", required: false, default_value: "value3"}, + %{name: "param_gitlab4", required: false} + ] + }) + |> Scheduler.PeriodicsRepo.update!() - :timer.sleep(5_000) + assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(periodic.id, 1) - assert tr.scheduling_status == "passed" - assert tr.scheduled_workflow_id == "repo_proxy_wf_id" - assert tr.error_description == nil - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - assert tr.attempts >= 1 - reset_mock_feature_service() - end + :timer.sleep(2_000) - test "schedule() - schedule params are correctly formed when there is only head_commit in hook payload", - ctx do - use_mock_workflow_service() - mock_workflow_service_response("ok") + assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - payload = %{head_commit: %{id: ctx.ids.commit_sha}, after: ""} - request = '{"payload": #{inspect(Jason.encode!(payload))}}' + assert tr.scheduling_status == "passed" + assert tr.scheduled_workflow_id == "wf_id" + assert tr.error_description == nil - assert {:ok, _resp} = - "UPDATE workflows SET request = '#{request}' WHERE true" - |> Scheduler.FrontRepo.query([]) + assert Map.new(tr.parameter_values, &{&1.name, &1.value}) == + %{ + "param_gitlab1" => "value1", + "param_gitlab2" => "value2", + "param_gitlab3" => "value3" + } - ts_before = DateTime.utc_now() + assert tr.attempts == 1 - timestamp = Timex.shift(ts_before, minutes: -1) + assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) + repository = %{id: UUID.uuid4(), integration_type: :GITLAB} - :timer.sleep(2_000) + assert {:ok, schedule_params} = + ScheduleWfImpl.form_just_run_schedule_params(periodic, tr, repository) - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr.scheduling_status == "passed" - assert tr.scheduled_workflow_id == "wf_id" - assert tr.error_description == nil - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - assert tr.attempts >= 1 - end + assert schedule_params.service == :GITLAB + assert schedule_params.requester_id == periodic.requester_id + assert schedule_params.triggered_by == :SCHEDULE + end - test "schedule() - scheduling fails if commit_sha can not be found", ctx do - use_mock_workflow_service() - mock_workflow_service_response("ok") + test "schedule() - schedule params are correctly formed for git agnostic in JustRun case", + ctx do + alias Scheduler.Actions.ScheduleWfImpl - payload = %{head_commit: %{id: ""}, after: ""} - request = '{"payload": #{inspect(Jason.encode!(payload))}}' + use_mock_workflow_service() + mock_workflow_service_response("just_run") + reset_mock_feature_service() + mock_feature_response("just_run") + use_mock_project_service() + mock_projecthub_response("ok") + use_mock_repository_service() + mock_repositoryhub_response("ok") - assert {:ok, _resp} = - "UPDATE workflows SET request = '#{request}' WHERE true" - |> Scheduler.FrontRepo.query([]) + ts_before = DateTime.utc_now() + timestamp = Timex.shift(ts_before, minutes: -1) - ts_before = DateTime.utc_now() + periodic = + ctx.periodic + |> Periodics.changeset("v1.1", %{ + parameters: [ + %{name: "param_git1", required: true, default_value: "value1"}, + %{name: "param_git2", required: false, default_value: "value2"}, + %{name: "param_git3", required: false, default_value: "value3"}, + %{name: "param_git4", required: false} + ] + }) + |> Scheduler.PeriodicsRepo.update!() - timestamp = Timex.shift(ts_before, minutes: -1) + assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) + :timer.sleep(2_000) - :timer.sleep(4_000) + assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr.scheduling_status == "failed" - assert tr.scheduled_workflow_id == "" - assert tr.error_description == "Hook is missing commit_sha data" - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - assert tr.attempts >= 1 - end + assert tr.scheduling_status == "passed" + assert tr.scheduled_workflow_id == "wf_id" + assert tr.error_description == nil - test "schedule() - scheduling fails if pipeline limit is exhausted", ctx do - use_mock_workflow_service() - mock_workflow_service_response("resource_exhausted") + assert Map.new(tr.parameter_values, &{&1.name, &1.value}) == + %{ + "param_git1" => "value1", + "param_git2" => "value2", + "param_git3" => "value3" + } - ts_before = DateTime.utc_now() - Timex.shift(ts_before, minutes: -1) - {:ok, trigger} = PTQueries.insert(ctx.periodic) - state = %{periodic: ctx.periodic, trigger: trigger} + assert tr.attempts == 1 - assert {:stop, :restart, _state} = ScheduleTask.handle_info(:schedule_workflow, state) + assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr.scheduling_status == "running" - assert tr.scheduled_workflow_id == "" - assert tr.error_description == "%{code: :RESOURCE_EXHAUSTED, message: \"Error\"}" - assert tr.attempts == 1 - end + repository = %{id: UUID.uuid4(), integration_type: :GIT} - test "schedule() - error response from workflow service is stored in trigger", ctx do - use_mock_workflow_service() - mock_workflow_service_response("invalid_argument") + assert {:ok, schedule_params} = + ScheduleWfImpl.form_just_run_schedule_params(periodic, tr, repository) - ts_before = DateTime.utc_now() + assert schedule_params.service == :GIT + assert schedule_params.requester_id == periodic.requester_id + assert schedule_params.triggered_by == :SCHEDULE + end - timestamp = Timex.shift(ts_before, minutes: -1) + test "schedule() - when project service fails for JustRun case then workflow is not scheduled", + ctx do + use_mock_workflow_service() + mock_workflow_service_response("just_run") + reset_mock_feature_service() + mock_feature_response("just_run") + use_mock_project_service() + mock_projecthub_response("failed_precondition") + use_mock_repository_service() + mock_repositoryhub_response("ok") - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) + ts_before = DateTime.utc_now() + timestamp = Timex.shift(ts_before, minutes: -1) - :timer.sleep(4_000) + periodic = + ctx.periodic + |> Periodics.changeset("v1.1", %{ + parameters: [ + %{name: "param1", required: true, default_value: "value1"}, + %{name: "param2", required: false, default_value: "value2"}, + %{name: "param3", required: false} + ] + }) + |> Scheduler.PeriodicsRepo.update!() - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr.scheduling_status == "failed" - assert tr.scheduled_workflow_id == "" - assert tr.error_description == "%{code: :INVALID_ARGUMENT, message: \"Error\"}" - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - assert tr.attempts >= 1 - end + assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - test "schedule() - error response when calling Create API is stored in trigger", %{ - project_id: project_id, - org_id: org_id - } do - use_mock_repo_proxy_service(project_id) - mock_repo_proxy_service_response("invalid_argument") - reset_mock_feature_service() - mock_feature_response("scheduler_hook") + :timer.sleep(2_000) - ids = %{ - usr_id: UUID.uuid4(), - org_id: org_id, - pr_id: project_id - } + assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert {:ok, periodic} = periodic_params(ids) |> PeriodicsQueries.insert() + assert tr.scheduling_status == "failed" + assert tr.scheduled_workflow_id == "" + assert tr.error_description == "{:missing_project, \"#{periodic.project_id}\"}" - ts_before = DateTime.utc_now() + assert Map.new(tr.parameter_values, &{&1.name, &1.value}) == + %{"param1" => "value1", "param2" => "value2"} - timestamp = Timex.shift(ts_before, minutes: -1) + assert tr.attempts >= 1 - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(periodic.id, timestamp) + assert DateTime.compare(tr.scheduled_at, ts_before) == :gt + end - :timer.sleep(4_000) + test "schedule() - when repository service fails for JustRun case then workflow is not scheduled", + ctx do + use_mock_workflow_service() + mock_workflow_service_response("just_run") + reset_mock_feature_service() + mock_feature_response("just_run") + use_mock_project_service() + mock_projecthub_response("ok") + use_mock_repository_service() + mock_repositoryhub_response("failed_precondition") - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(periodic.id, 1) - assert tr.scheduling_status == "failed" - assert tr.scheduled_workflow_id == "" - assert String.contains?(tr.error_description, "message: \"Invalid argument\"") - assert String.contains?(tr.error_description, "status: 3") - assert String.contains?(tr.error_description, "GRPC.RPCError") - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - assert tr.attempts >= 1 - reset_mock_feature_service() - end + ts_before = DateTime.utc_now() + timestamp = Timex.shift(ts_before, minutes: -1) - test "schedule() - too long error message from wf service is stored truncated to max length", - ctx do - use_mock_workflow_service() - mock_workflow_service_response("too_long_error_msg") + ctx.periodic + |> Periodics.changeset("v1.1", %{ + parameters: [ + %{name: "param1", required: true, default_value: "value1"}, + %{name: "param2", required: false, default_value: "value2"}, + %{name: "param3", required: false} + ] + }) + |> Scheduler.PeriodicsRepo.update!() - ts_before = DateTime.utc_now() + assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - timestamp = Timex.shift(ts_before, minutes: -1) + :timer.sleep(2_000) - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) + assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - :timer.sleep(4_000) + assert tr.scheduling_status == "failed" + assert tr.scheduled_workflow_id == "" - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr.scheduling_status == "failed" - assert tr.scheduled_workflow_id == "" + assert tr.error_description == + "{:missing_revision, [commit_sha: \"\", reference: \"refs/heads/master\"]}" - assert "%{code: :INVALID_ARGUMENT, message: \"aaaaaaa" <> _rest = tr.error_description + assert Map.new(tr.parameter_values, &{&1.name, &1.value}) == + %{"param1" => "value1", "param2" => "value2"} - assert String.length(tr.error_description) == 254 - assert tr.attempts >= 1 - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - end + assert tr.attempts >= 1 - test "schedule() - error message is removed if next scheduling attempt passes", ctx do - use_mock_workflow_service() - mock_workflow_service_response("invalid_argument") + assert DateTime.compare(tr.scheduled_at, ts_before) == :gt + end - ts_before = DateTime.utc_now() + test "schedule() - error response from workflow service is stored in trigger", ctx do + use_mock_workflow_service() + mock_workflow_service_response("invalid_argument") + reset_mock_feature_service() + mock_feature_response("just_run") + use_mock_project_service() + mock_projecthub_response("ok") + use_mock_repository_service() + mock_repositoryhub_response("ok") + + ts_before = DateTime.utc_now() + timestamp = Timex.shift(ts_before, minutes: -1) + + assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) + + :timer.sleep(4_000) + + assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) + assert tr.scheduling_status == "failed" + assert tr.scheduled_workflow_id == "" + assert tr.error_description == "%{code: :INVALID_ARGUMENT, message: \"Error\"}" + assert DateTime.compare(tr.scheduled_at, ts_before) == :gt + assert tr.attempts >= 1 + end - timestamp = Timex.shift(ts_before, minutes: -1) + test "schedule() - scheduling fails if pipeline limit is exhausted", ctx do + use_mock_workflow_service() + mock_workflow_service_response("resource_exhausted") + reset_mock_feature_service() + mock_feature_response("just_run") + use_mock_project_service() + mock_projecthub_response("ok") + use_mock_repository_service() + mock_repositoryhub_response("ok") + + ts_before = DateTime.utc_now() + timestamp = Timex.shift(ts_before, minutes: -1) + + assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) + + :timer.sleep(4_000) + + assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) + assert tr.scheduling_status == "running" + assert tr.scheduled_workflow_id == "" + assert tr.error_description == "%{code: :RESOURCE_EXHAUSTED, message: \"Error\"}" + assert DateTime.compare(tr.scheduled_at, ts_before) == :gt + assert tr.attempts >= 1 + end - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) + test "schedule() - too long error message from workflow service is stored truncated to max length", + ctx do + use_mock_workflow_service() + mock_workflow_service_response("too_long_error_msg") + reset_mock_feature_service() + mock_feature_response("just_run") + use_mock_project_service() + mock_projecthub_response("ok") + use_mock_repository_service() + mock_repositoryhub_response("ok") - :timer.sleep(1_000) + ts_before = DateTime.utc_now() + timestamp = Timex.shift(ts_before, minutes: -1) - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr.scheduling_status == "running" - assert tr.scheduled_workflow_id == "" - assert tr.error_description == "%{code: :INVALID_ARGUMENT, message: \"Error\"}" - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - assert tr.attempts >= 1 + assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - mock_workflow_service_response("ok") - :timer.sleep(3_000) + :timer.sleep(4_000) - assert {:ok, [tr2]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr2.scheduling_status == "passed" - assert tr2.scheduled_workflow_id == "wf_id" - assert tr2.error_description == nil - assert DateTime.compare(tr2.scheduled_at, ts_before) == :gt - assert tr2.attempts >= tr.attempts - end + assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) + assert tr.scheduling_status == "failed" + assert tr.scheduled_workflow_id == "" - test "schedule() - scheduling fails if periodic is supended", ctx do - use_mock_workflow_service() - mock_workflow_service_response("invalid_argument") + assert "%{code: :INVALID_ARGUMENT, message: \"aaaaaaa" <> _rest = tr.error_description - ts_before = DateTime.utc_now() + assert String.length(tr.error_description) == 254 + assert tr.attempts >= 1 + assert DateTime.compare(tr.scheduled_at, ts_before) == :gt + end - timestamp = Timex.shift(ts_before, minutes: -1) + test "schedule() - error message is removed if next scheduling attempt passes", ctx do + use_mock_workflow_service() + mock_workflow_service_response("invalid_argument") + reset_mock_feature_service() + mock_feature_response("just_run") + use_mock_project_service() + mock_projecthub_response("ok") + use_mock_repository_service() + mock_repositoryhub_response("ok") + + ts_before = DateTime.utc_now() + timestamp = Timex.shift(ts_before, minutes: -1) + + assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) + + :timer.sleep(1_000) + + assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) + assert tr.scheduling_status == "running" + assert tr.scheduled_workflow_id == "" + assert tr.error_description == "%{code: :INVALID_ARGUMENT, message: \"Error\"}" + assert DateTime.compare(tr.scheduled_at, ts_before) == :gt + assert tr.attempts >= 1 + + mock_workflow_service_response("just_run") + :timer.sleep(3_000) + + assert {:ok, [tr2]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) + assert tr2.scheduling_status == "passed" + assert tr2.scheduled_workflow_id == "wf_id" + assert tr2.error_description == nil + assert DateTime.compare(tr2.scheduled_at, ts_before) == :gt + assert tr2.attempts >= tr.attempts + end - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) + test "schedule() - scheduling fails if periodic is suspended", ctx do + use_mock_workflow_service() + mock_workflow_service_response("invalid_argument") + reset_mock_feature_service() + mock_feature_response("just_run") + use_mock_project_service() + mock_projecthub_response("ok") + use_mock_repository_service() + mock_repositoryhub_response("ok") - :timer.sleep(1_000) + ts_before = DateTime.utc_now() + timestamp = Timex.shift(ts_before, minutes: -1) - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr.scheduling_status == "running" - assert tr.scheduled_workflow_id == "" - assert tr.error_description == "%{code: :INVALID_ARGUMENT, message: \"Error\"}" - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - assert tr.attempts >= 1 + assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - assert {:ok, _periodic} = PeriodicsQueries.suspend(ctx.periodic) + :timer.sleep(1_000) - :timer.sleep(2_000) + assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) + assert tr.scheduling_status == "running" + assert tr.scheduled_workflow_id == "" + assert tr.error_description == "%{code: :INVALID_ARGUMENT, message: \"Error\"}" + assert DateTime.compare(tr.scheduled_at, ts_before) == :gt + assert tr.attempts >= 1 - assert {:ok, [tr2]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr2.scheduling_status == "failed" - assert tr2.scheduled_workflow_id == "" - assert tr2.error_description == "Scheduler with id '#{ctx.periodic.id}' is suspended." - assert DateTime.compare(tr2.scheduled_at, ts_before) == :gt - assert tr2.attempts >= tr.attempts - end + assert {:ok, _periodic} = PeriodicsQueries.suspend(ctx.periodic) - test "schedule() - scheduling fails if periodic is paused", ctx do - use_mock_workflow_service() - mock_workflow_service_response("invalid_argument") + :timer.sleep(2_000) - ts_before = DateTime.utc_now() + assert {:ok, [tr2]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) + assert tr2.scheduling_status == "failed" + assert tr2.scheduled_workflow_id == "" + assert tr2.error_description == "Scheduler with id '#{ctx.periodic.id}' is suspended." + assert DateTime.compare(tr2.scheduled_at, ts_before) == :gt + assert tr2.attempts >= tr.attempts + end - timestamp = Timex.shift(ts_before, minutes: -1) + test "schedule() - scheduling fails if periodic is paused", ctx do + use_mock_workflow_service() + mock_workflow_service_response("invalid_argument") + reset_mock_feature_service() + mock_feature_response("just_run") + use_mock_project_service() + mock_projecthub_response("ok") + use_mock_repository_service() + mock_repositoryhub_response("ok") - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) + ts_before = DateTime.utc_now() + timestamp = Timex.shift(ts_before, minutes: -1) - :timer.sleep(1_000) + assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr.scheduling_status == "running" - assert tr.scheduled_workflow_id == "" - assert tr.error_description == "%{code: :INVALID_ARGUMENT, message: \"Error\"}" - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - assert tr.attempts >= 1 + :timer.sleep(1_000) - assert {:ok, _periodic} = PeriodicsQueries.pause(ctx.periodic, "user_1") + assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) + assert tr.scheduling_status == "running" + assert tr.scheduled_workflow_id == "" + assert tr.error_description == "%{code: :INVALID_ARGUMENT, message: \"Error\"}" + assert DateTime.compare(tr.scheduled_at, ts_before) == :gt + assert tr.attempts >= 1 - :timer.sleep(2_000) + assert {:ok, _periodic} = PeriodicsQueries.pause(ctx.periodic, "user_1") - assert {:ok, [tr2]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr2.scheduling_status == "failed" - assert tr2.scheduled_workflow_id == "" - assert tr2.error_description == "Scheduler with id '#{ctx.periodic.id}' is paused." - assert tr2.attempts >= tr.attempts - assert DateTime.compare(tr2.scheduled_at, ts_before) == :gt - end + :timer.sleep(2_000) - test "schedule() - restarting schedulr task is terminated if periodic is deleted", ctx do - use_mock_workflow_service() - mock_workflow_service_response("invalid_argument") + assert {:ok, [tr2]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) + assert tr2.scheduling_status == "failed" + assert tr2.scheduled_workflow_id == "" + assert tr2.error_description == "Scheduler with id '#{ctx.periodic.id}' is paused." + assert tr2.attempts >= tr.attempts + assert DateTime.compare(tr2.scheduled_at, ts_before) == :gt + end - ts_before = DateTime.utc_now() + test "schedule() - restarting scheduler task is terminated if periodic is deleted", ctx do + use_mock_workflow_service() + mock_workflow_service_response("invalid_argument") + reset_mock_feature_service() + mock_feature_response("just_run") + use_mock_project_service() + mock_projecthub_response("ok") + use_mock_repository_service() + mock_repositoryhub_response("ok") - timestamp = Timex.shift(ts_before, minutes: -1) + ts_before = DateTime.utc_now() + timestamp = Timex.shift(ts_before, minutes: -1) - assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) + assert {:ok, _pid} = ScheduleWfImpl.start_schedule_task(ctx.periodic.id, timestamp) - :timer.sleep(1_000) + :timer.sleep(1_000) - assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) - assert tr.scheduling_status == "running" - assert tr.scheduled_workflow_id == "" - assert tr.error_description == "%{code: :INVALID_ARGUMENT, message: \"Error\"}" - assert DateTime.compare(tr.scheduled_at, ts_before) == :gt - assert tr.attempts >= 1 + assert {:ok, [tr]} = PTQueries.get_n_by_periodic_id(ctx.periodic.id, 1) + assert tr.scheduling_status == "running" + assert tr.scheduled_workflow_id == "" + assert tr.error_description == "%{code: :INVALID_ARGUMENT, message: \"Error\"}" + assert DateTime.compare(tr.scheduled_at, ts_before) == :gt + assert tr.attempts >= 1 - assert %{workers: 1} = ScheduleTaskManager.count_children() + assert %{workers: 1} = ScheduleTaskManager.count_children() - assert {:ok, _message} = Scheduler.Actions.delete(%{id: ctx.periodic.id, requester: "asdf"}) + assert {:ok, _message} = Scheduler.Actions.delete(%{id: ctx.periodic.id, requester: "asdf"}) - :timer.sleep(2_000) + :timer.sleep(2_000) - assert %{workers: 0} = ScheduleTaskManager.count_children() + assert %{workers: 0} = ScheduleTaskManager.count_children() + end end defp use_mock_project_service(), @@ -706,16 +600,6 @@ defmodule Scheduler.Actions.ScheduleWfImpl.Test do def mock_workflow_service_response(value), do: Application.put_env(:scheduler, :mock_workflow_service_response, value) - defp use_mock_repo_proxy_service(project_id) do - Application.put_env( - :scheduler, - :repo_proxy_api_grpc_endpoint, - "localhost:#{inspect(@grpc_port)}" - ) - - Application.put_env(:scheduler, :repo_proxy_service, {Test.MockRepoProxy, project_id}) - end - defp reset_mock_feature_service() do Cachex.clear(Elixir.Scheduler.FeatureHubProvider) @@ -750,7 +634,11 @@ defmodule AssertPramsWorkflowService do assert :GIT_HUB = request.service assert {:ok, _} = request.project_id |> UUID.info() - if Application.get_env(:scheduler, :mock_workflow_service_response) != "just_run" do + feature_response = Application.get_env(:scheduler, :mock_feature_service_response) + workflow_response = Application.get_env(:scheduler, :mock_workflow_service_response) + + # Check if we're using just_run feature (not the workflow response type) + if feature_response != "just_run" do assert {:ok, _} = request.branch_id |> UUID.info() assert {:ok, _} = request.hook_id |> UUID.info() assert {:ok, _} = request.repo.commit_sha |> UUID.info() @@ -763,16 +651,16 @@ defmodule AssertPramsWorkflowService do assert request.snapshot_id == "" assert request.definition_file == "deploy.yml" - response_type = Application.get_env(:scheduler, :mock_workflow_service_response) - - if response_type == "just_run" do + # For just_run feature, check different repo structure + if feature_response == "just_run" do assert request.repo.owner == "" assert request.repo.repo_name == "" assert request.repo.commit_sha == "" assert request.repo.branch_name == "master" end - response_type = if response_type == "just_run", do: "ok", else: response_type + # Convert just_run workflow response to ok for mock + response_type = if workflow_response == "just_run", do: "ok", else: workflow_response respond(response_type) end diff --git a/periodic_scheduler/scheduler/test/actions/unpause_impl_test.exs b/periodic_scheduler/scheduler/test/actions/unpause_impl_test.exs index a35af5fbd..c0531e7ad 100644 --- a/periodic_scheduler/scheduler/test/actions/unpause_impl_test.exs +++ b/periodic_scheduler/scheduler/test/actions/unpause_impl_test.exs @@ -42,7 +42,7 @@ defmodule Test.Actions.UnpauseImpl.Test do project_name: "Project_1", recurring: if(is_nil(extra[:recurring]), do: true, else: extra[:recurring]), project_id: ids.pr_id, - branch: extra[:branch] || "master", + reference: extra[:reference] || "master", at: extra[:at] || "* * * * *", paused: if(is_nil(extra[:paused]), do: false, else: extra[:paused]), pipeline_file: extra[:pipeline_file] || "deploy.yml", diff --git a/periodic_scheduler/scheduler/test/events_consumers/org_blocked_test.exs b/periodic_scheduler/scheduler/test/events_consumers/org_blocked_test.exs index e1db43e40..b67b9ff84 100644 --- a/periodic_scheduler/scheduler/test/events_consumers/org_blocked_test.exs +++ b/periodic_scheduler/scheduler/test/events_consumers/org_blocked_test.exs @@ -64,7 +64,7 @@ defmodule Scheduler.EventsConsumers.OrgBlocked.Test do defp valid_yml_definition(params) do %{ - branch: "master", + reference: "master", at: "0 0 * * * *", project: "Project 1", name: "P1", diff --git a/periodic_scheduler/scheduler/test/grpc_server_test.exs b/periodic_scheduler/scheduler/test/grpc_server_test.exs index 252e5d920..83d4405ad 100644 --- a/periodic_scheduler/scheduler/test/grpc_server_test.exs +++ b/periodic_scheduler/scheduler/test/grpc_server_test.exs @@ -80,7 +80,7 @@ defmodule Scheduler.GrpcServer.Test do assert periodic.name == "P1" assert periodic.project_name == "Project 1" assert periodic.project_id == ctx.ids.pr_id - assert periodic.branch == "master" + assert periodic.reference == "refs/heads/master" assert periodic.at == "0 0 * * * *" assert periodic.pipeline_file == ".semaphore/cron.yml" assert periodic.recurring @@ -127,7 +127,7 @@ defmodule Scheduler.GrpcServer.Test do assert periodic.project_id == ctx.ids.pr_id refute periodic.recurring refute periodic.at - assert periodic.branch == "master" + assert periodic.reference == "refs/heads/master" assert periodic.pipeline_file == ".semaphore/cron.yml" assert periodic.paused == false assert periodic.pause_toggled_by == "" @@ -147,7 +147,7 @@ defmodule Scheduler.GrpcServer.Test do defp default_params(ctx) do %{ - branch: "master", + reference: "master", at: "0 0 * * * *", project: "Project 1", name: "P1", @@ -182,7 +182,7 @@ defmodule Scheduler.GrpcServer.Test do spec: project: #{params.project} recurring: true - branch: #{params.branch} + branch: #{params.reference} at: #{params.at} pipeline_file: #{params.pipeline_file} #{if is_nil(params[:paused]), do: "", else: "paused: #{params.paused}"} @@ -242,7 +242,7 @@ defmodule Scheduler.GrpcServer.Test do assert periodic.name == "P1" assert periodic.project_name == "Project 1" assert periodic.project_id == ctx.ids.pr_id - assert periodic.branch == "master" + assert periodic.reference == "refs/heads/master" assert periodic.at == "0 0 * * * *" assert periodic.pipeline_file == ".semaphore/cron.yml" assert periodic.paused == false @@ -278,7 +278,7 @@ defmodule Scheduler.GrpcServer.Test do assert periodic.name == "P1" assert periodic.project_name == "Project 1" assert periodic.project_id == ctx.ids.pr_id - assert periodic.branch == "master" + assert periodic.reference == "refs/heads/master" assert periodic.at == "0 0 * * * *" assert periodic.pipeline_file == ".semaphore/cron.yml" assert periodic.paused == true @@ -310,7 +310,7 @@ defmodule Scheduler.GrpcServer.Test do id: #{id} spec: project: #{params.project} - branch: #{params.branch} + branch: #{params.reference} at: #{params.at} pipeline_file: #{params.pipeline_file} #{if params.paused != nil, do: "paused: #{params.paused}", else: ""} @@ -325,7 +325,7 @@ defmodule Scheduler.GrpcServer.Test do name: #{params.name} spec: project: #{params.project} - branch: #{params.branch} + branch: #{params.reference} at: #{params.at} pipeline_file: #{params.pipeline_file} #{if params.paused != nil, do: "paused: #{params.paused}", else: ""} @@ -341,7 +341,7 @@ defmodule Scheduler.GrpcServer.Test do spec: project: #{params.project} recurring: #{params.recurring} - branch: #{params.branch}] + branch: #{params.reference}] at: #{params.at} pipeline_file: #{params.pipeline_file} paused: #{params.paused == true} @@ -582,7 +582,7 @@ defmodule Scheduler.GrpcServer.Test do %{ requester_id: ctx.ids.usr_id, organization_id: ctx.ids.org_id, - yml_definition: valid_yml_definition(ctx, %{branch: "dev"}) + yml_definition: valid_yml_definition(ctx, %{reference: "dev"}) } |> Proto.deep_new!(ApplyRequest) @@ -675,7 +675,7 @@ defmodule Scheduler.GrpcServer.Test do assert periodic.name == "P1" assert periodic.project_name == "Project 1" assert periodic.project_id == ctx.ids.pr_id - assert periodic.branch == "master" + assert periodic.reference == "master" assert periodic.at == "0 0 * * * *" assert periodic.pipeline_file == ".semaphore/cron.yml" assert periodic.recurring @@ -719,7 +719,7 @@ defmodule Scheduler.GrpcServer.Test do assert periodic.project_id == ctx.ids.pr_id refute periodic.recurring refute periodic.at - assert periodic.branch == "master" + assert periodic.reference == "master" assert periodic.pipeline_file == ".semaphore/cron.yml" assert periodic.paused == false assert periodic.pause_toggled_by == "" @@ -755,7 +755,7 @@ defmodule Scheduler.GrpcServer.Test do assert periodic.project_id == ctx.ids.pr_id assert periodic.recurring assert periodic.at == "0 0 * * * *" - assert periodic.branch == "master" + assert periodic.reference == "master" assert periodic.pipeline_file == ".semaphore/cron.yml" assert periodic.paused == true assert periodic.pause_toggled_by == ctx.ids.usr_id @@ -971,9 +971,11 @@ defmodule Scheduler.GrpcServer.Test do assert String.starts_with?(msg, "Invalid cron expression in 'at' field:") end - test "gRPC persist() returns INVALID_ARGUMENT when one of parameters is missing", ctx do - for arg <- ~w(name branch pipeline_file organization_id project_id)a do - request = default_params(ctx) |> Map.put(arg, "") |> Proto.deep_new!(PersistRequest) + for arg <- ~w(name reference pipeline_file organization_id project_id)a do + test "gRPC persist() returns INVALID_ARGUMENT when #{arg} is missing", ctx do + request = + default_params(ctx) |> Map.put(unquote(arg), "") |> Proto.deep_new!(PersistRequest) + assert {nil, msg} = persist_grpc(request, :INVALID_ARGUMENT) assert String.contains?(msg, "empty") end @@ -1390,6 +1392,7 @@ defmodule Scheduler.GrpcServer.Test do :inserted_at, :description, :updated_at, + :organization_id, :pause_toggled_at, :__struct__, :__unknown_fields__ @@ -1446,7 +1449,6 @@ defmodule Scheduler.GrpcServer.Test do :description, :inserted_at, :updated_at, - :organization_id, :pause_toggled_at, :__meta__ ] @@ -1539,6 +1541,7 @@ defmodule Scheduler.GrpcServer.Test do :inserted_at, :description, :updated_at, + :organization_id, :pause_toggled_at, :__struct__, :__unknown_fields__ @@ -1613,6 +1616,7 @@ defmodule Scheduler.GrpcServer.Test do :description, :updated_at, :pause_toggled_at, + :organization_id, :__struct__, :__unknown_fields__ ]) == diff --git a/periodic_scheduler/scheduler/test/periodics/initializer_test.exs b/periodic_scheduler/scheduler/test/periodics/initializer_test.exs index 66bde4c50..6b5ab1cb2 100644 --- a/periodic_scheduler/scheduler/test/periodics/initializer_test.exs +++ b/periodic_scheduler/scheduler/test/periodics/initializer_test.exs @@ -36,7 +36,7 @@ defmodule Scheduler.Workers.Initializer.Test do name: "Periodic_#{ind}", project_name: "Project_1", project_id: ids.pr_id, - branch: "master", + reference: "master", at: "* * * * *", pipeline_file: "deploy.yml" } diff --git a/periodic_scheduler/scheduler/test/periodics/model/periodic_queries_test.exs b/periodic_scheduler/scheduler/test/periodics/model/periodic_queries_test.exs index 0cf8eabcf..0d9b135e5 100644 --- a/periodic_scheduler/scheduler/test/periodics/model/periodic_queries_test.exs +++ b/periodic_scheduler/scheduler/test/periodics/model/periodic_queries_test.exs @@ -246,7 +246,7 @@ defmodule Scheduler.Periodics.Model.PeriodicsQueries.Test do assert periodic.name == params.name assert periodic.project_name == params.project_name assert periodic.project_id == params.project_id - assert periodic.branch == params.branch + assert periodic.reference == "refs/heads/#{params.reference}" assert periodic.at == params.at assert periodic.pipeline_file == params.pipeline_file assert NaiveDateTime.compare(ts_before, periodic.inserted_at) == :lt @@ -264,7 +264,7 @@ defmodule Scheduler.Periodics.Model.PeriodicsQueries.Test do assert periodic.name == params.name assert periodic.project_name == params.project_name assert periodic.project_id == params.project_id - assert periodic.branch == params.branch + assert periodic.reference == "refs/heads/#{params.reference}" assert periodic.pipeline_file == params.pipeline_file refute periodic.at @@ -288,7 +288,7 @@ defmodule Scheduler.Periodics.Model.PeriodicsQueries.Test do name: "Periodic_1", project_name: "Project_1", project_id: "pr1", - branch: "master", + reference: "master", at: "* * * * *", pipeline_file: "deploy.yml" } @@ -302,7 +302,7 @@ defmodule Scheduler.Periodics.Model.PeriodicsQueries.Test do project_name: "Project_1", project_id: "pr1", recurring: false, - branch: "master", + reference: "master", at: "", pipeline_file: "deploy.yml", parameters: [ @@ -329,28 +329,28 @@ defmodule Scheduler.Periodics.Model.PeriodicsQueries.Test do end) end - test "can insert periodic without at" do - params = insert_params("v1.1") + describe "insert/2 validation" do + for field <- + ~w(requester_id organization_id name project_name project_id reference pipeline_file)a do + test "returns error when #{field} is missing" do + params = insert_params("v1.1") + params_ = params |> Map.delete(unquote(field)) - ~w(requester_id organization_id name project_name project_id branch pipeline_file)a - |> Enum.map(fn field_name -> - params_ = params |> Map.delete(field_name) + assert {:error, msg} = PeriodicsQueries.insert(params_, "v1.1") + error_msg_1 = "errors: [#{unquote(field)}: {\"can't be blank\", [validation: :required]}]" + error_msg_2 = "The '#{unquote(field)}' parameter can not be empty string." - assert {:error, msg} = PeriodicsQueries.insert(params_, "v1.1") - error_msg_1 = "errors: [#{field_name}: {\"can't be blank\", [validation: :required]}]" - error_msg_2 = "The '#{field_name}' parameter can not be empty string." - - assert String.contains?("#{inspect(msg)}", error_msg_1) or - String.contains?("#{inspect(msg)}", error_msg_2) - end) + assert String.contains?("#{inspect(msg)}", error_msg_1) or + String.contains?("#{inspect(msg)}", error_msg_2) + end + end - ~w(at)a - |> Enum.map(fn field_name -> - params_ = - params |> Map.delete(field_name) |> Map.put(:name, "Periodic without #{field_name}") + test "can insert periodic without at field" do + params = insert_params("v1.1") + params_ = params |> Map.delete(:at) |> Map.put(:name, "Periodic without at") assert {:ok, _periodic} = PeriodicsQueries.insert(params_, "v1.1") - end) + end end test "can not insert two periodics with same name for same project" do @@ -368,14 +368,14 @@ defmodule Scheduler.Periodics.Model.PeriodicsQueries.Test do params = insert_params("v1.0") assert {:ok, periodic_1} = PeriodicsQueries.insert(params, "v1.0") - params_2 = params |> Map.merge(%{branch: "dev", at: "@yearly"}) + params_2 = params |> Map.merge(%{reference: "dev", at: "@yearly"}) assert {:ok, periodic_2} = PeriodicsQueries.update(periodic_1, params_2, "v1.0") - assert periodic_2.branch == "dev" + assert periodic_2.reference == "refs/heads/dev" assert periodic_2.at == "@yearly" - assert periodic_1 |> Map.drop([:updated_at, :branch, :at]) == - periodic_2 |> Map.drop([:updated_at, :branch, :at]) + assert periodic_1 |> Map.drop([:updated_at, :reference, :at]) == + periodic_2 |> Map.drop([:updated_at, :reference, :at]) end test "cannot update periodics to the existing name for same project" do diff --git a/periodic_scheduler/scheduler/test/periodics/model/periodics_test.exs b/periodic_scheduler/scheduler/test/periodics/model/periodics_test.exs index d17a0f84f..9d71ce0e5 100644 --- a/periodic_scheduler/scheduler/test/periodics/model/periodics_test.exs +++ b/periodic_scheduler/scheduler/test/periodics/model/periodics_test.exs @@ -13,21 +13,21 @@ defmodule Scheduler.Periodics.Model.Periodics.Test do assert [at: {"can't be blank", _}] = errors full_params = - Map.merge(ctx.params, %{at: "* * * * *", branch: "master", pipeline_file: "deploy.yml"}) + Map.merge(ctx.params, %{at: "* * * * *", reference: "master", pipeline_file: "deploy.yml"}) assert %Ecto.Changeset{valid?: true} = Periodics.changeset(%Periodics{}, "v1.1", full_params) end - test "when recurring is true then validates if periodics has cron, branch and pipeline file", + test "when recurring is true then validates if periodics has cron, reference and pipeline file", ctx do partial_params = - Map.merge(ctx.params, %{recurring: true}) |> Map.drop(~w(at branch pipeline_file)a) + Map.merge(ctx.params, %{recurring: true}) |> Map.drop(~w(at reference pipeline_file)a) assert %Ecto.Changeset{valid?: false, errors: errors} = Periodics.changeset(%Periodics{}, "v1.1", partial_params) - assert [:at, :branch, :pipeline_file] = errors |> Keyword.keys() + assert [:at, :reference, :pipeline_file] = errors |> Keyword.keys() assert ["can't be blank"] = errors |> Keyword.values() |> Enum.map(&elem(&1, 0)) |> Enum.uniq() @@ -35,7 +35,7 @@ defmodule Scheduler.Periodics.Model.Periodics.Test do full_params = Map.merge(partial_params, %{ at: "* * * * *", - branch: "master", + reference: "master", pipeline_file: "deploy.yml" }) @@ -49,7 +49,7 @@ defmodule Scheduler.Periodics.Model.Periodics.Test do end test "when cron expression is invalid then invalid", ctx do - params = Map.merge(ctx.params, %{branch: "master", pipeline_file: "deploy.yml"}) + params = Map.merge(ctx.params, %{reference: "master", pipeline_file: "deploy.yml"}) invalid_params = Map.put(params, :at, "0 0 * * 12") @@ -71,7 +71,7 @@ defmodule Scheduler.Periodics.Model.Periodics.Test do name: "P1", project_name: "Pr1", project_id: "p1", - branch: "master", + reference: "master", pipeline_file: "deploy.yml", id: UUID.uuid1() }} diff --git a/periodic_scheduler/scheduler/test/periodics_triggers/model/history_page_test.exs b/periodic_scheduler/scheduler/test/periodics_triggers/model/history_page_test.exs index 78a15d6b8..8ac03ef5e 100644 --- a/periodic_scheduler/scheduler/test/periodics_triggers/model/history_page_test.exs +++ b/periodic_scheduler/scheduler/test/periodics_triggers/model/history_page_test.exs @@ -415,14 +415,34 @@ defmodule Scheduler.PeriodicsTriggers.Model.HistoryPage.Test do end test "filters for particular branch", ctx do - insert_triggers_for_past(ctx, 15..22, :days, branch: "develop") + insert_triggers_for_past(ctx, 15..22, :days, reference: "develop") assert %HistoryPage{results: results} = load_page_with_cursor(ctx, {:BEFORE, cursor_ago(ctx, 2, :days)}, - branch_name: "develop" + reference: %{ + normalized: "refs/heads/develop", + short: "develop", + original: "develop" + } ) - assert Enum.all?(results, &(&1.branch == "develop")) + assert Enum.all?(results, &(&1.reference == "develop")) + assert Enum.count(results) == 7 + end + + test "filters for particular branch with new reference format", ctx do + insert_triggers_for_past(ctx, 15..22, :days, reference: "refs/heads/develop") + + assert %HistoryPage{results: results} = + load_page_with_cursor(ctx, {:BEFORE, cursor_ago(ctx, 2, :days)}, + reference: %{ + normalized: "refs/heads/develop", + short: "develop", + original: "develop" + } + ) + + assert Enum.all?(results, &(&1.reference == "refs/heads/develop")) assert Enum.count(results) == 7 end diff --git a/periodic_scheduler/scheduler/test/periodics_triggers/model/periodics_triggers_queries_test.exs b/periodic_scheduler/scheduler/test/periodics_triggers/model/periodics_triggers_queries_test.exs index 8db50ccfc..8a9a37a1a 100644 --- a/periodic_scheduler/scheduler/test/periodics_triggers/model/periodics_triggers_queries_test.exs +++ b/periodic_scheduler/scheduler/test/periodics_triggers/model/periodics_triggers_queries_test.exs @@ -18,7 +18,7 @@ defmodule Scheduler.PeriodicsTriggers.Model.PeriodicsTriggersQueries.Test do name: "Periodic_1", project_name: "Project_1", project_id: "pr1", - branch: "master", + reference: "master", at: "* * * * *", pipeline_file: "deploy.yml", parameters: [ @@ -35,12 +35,12 @@ defmodule Scheduler.PeriodicsTriggers.Model.PeriodicsTriggersQueries.Test do assert {:ok, ptr} = PeriodicsTriggersQueries.insert(ctx.periodic) assert ptr.project_id == ctx.periodic.project_id - assert ptr.branch == ctx.periodic.branch + assert ptr.reference == ctx.periodic.reference assert ptr.pipeline_file == ctx.periodic.pipeline_file assert DateTime.compare(ts_before, ptr.triggered_at) == :lt assert ptr.scheduling_status == "running" - assert ptr.branch == "master" + assert ptr.reference == "refs/heads/master" assert ptr.pipeline_file == "deploy.yml" assert ptr.recurring refute ptr.run_now_requester_id @@ -69,14 +69,14 @@ defmodule Scheduler.PeriodicsTriggers.Model.PeriodicsTriggersQueries.Test do ], requester: "some_requester", pipeline_file: "cicd.yml", - branch: "develop" + reference: "refs/heads/develop" }) assert ptr.project_id == ctx.periodic.project_id assert DateTime.compare(ts_before, ptr.triggered_at) == :lt assert ptr.scheduling_status == "running" - assert ptr.branch == "develop" + assert ptr.reference == "refs/heads/develop" assert ptr.pipeline_file == "cicd.yml" assert ptr.run_now_requester_id == "some_requester" refute ptr.recurring @@ -94,7 +94,7 @@ defmodule Scheduler.PeriodicsTriggers.Model.PeriodicsTriggersQueries.Test do assert {:ok, ptr_u} = PeriodicsTriggersQueries.update(ptr, params) assert ptr_u.project_id == ptr.project_id - assert ptr_u.branch == ptr.branch + assert ptr_u.reference == ptr.reference assert ptr_u.pipeline_file == ptr.pipeline_file assert ptr_u.triggered_at == ptr.triggered_at assert ptr_u.scheduling_status == "passed" @@ -164,4 +164,41 @@ defmodule Scheduler.PeriodicsTriggers.Model.PeriodicsTriggersQueries.Test do assert {:ok, resp} = PeriodicsTriggersQueries.get_all_by_periodic_id(ctx.periodic.id) assert resp == [ptr_2, ptr_1] end + + test "insert new periodics_trigger with tag reference", ctx do + ts_before = DateTime.utc_now() + + assert {:ok, ptr} = + PeriodicsTriggersQueries.insert(ctx.periodic, %{ + reference: "refs/tags/v1.0.0" + }) + + assert ptr.project_id == ctx.periodic.project_id + assert ptr.reference == "refs/tags/v1.0.0" + assert ptr.pipeline_file == ctx.periodic.pipeline_file + assert DateTime.compare(ts_before, ptr.triggered_at) == :lt + assert ptr.scheduling_status == "running" + assert ptr.recurring + refute ptr.run_now_requester_id + + assert %{"p1" => "v1", "p2" => "v2"} == + Map.new(ptr.parameter_values, &{&1.name, &1.value}) + end + + test "insert new periodics_trigger with short tag name gets normalized", ctx do + ts_before = DateTime.utc_now() + + # Test that a short tag name without refs/tags/ prefix gets normalized + assert {:ok, ptr} = + PeriodicsTriggersQueries.insert(ctx.periodic, %{ + reference: "v2.0.0" + }) + + assert ptr.project_id == ctx.periodic.project_id + # Short names are assumed to be branches + assert ptr.reference == "refs/heads/v2.0.0" + assert ptr.pipeline_file == ctx.periodic.pipeline_file + assert DateTime.compare(ts_before, ptr.triggered_at) == :lt + assert ptr.scheduling_status == "running" + end end diff --git a/periodic_scheduler/scheduler/test/support/factory.ex b/periodic_scheduler/scheduler/test/support/factory.ex index 4cbaf4123..bc5909ce2 100644 --- a/periodic_scheduler/scheduler/test/support/factory.ex +++ b/periodic_scheduler/scheduler/test/support/factory.ex @@ -21,7 +21,7 @@ defmodule Test.Support.Factory do project_name: "Project", recurring: true, pipeline_file: "deploy.yml", - branch: "master", + reference: "master", at: "0 0 * * *", name: "Periodic", id: UUID.uuid4() @@ -42,7 +42,7 @@ defmodule Test.Support.Factory do periodic_id: context.periodic.id, project_id: context.periodic.project_id, recurring: context.periodic.recurring, - branch: extra[:branch] || context.periodic.branch, + reference: extra[:reference] || context.periodic.reference, pipeline_file: extra[:pipeline_file] || context.periodic.pipeline_file, scheduling_status: extra[:scheduling_status] || "passed", run_now_requester_id: extra[:triggered_by] || UUID.uuid4(), diff --git a/periodic_scheduler/scheduler/test/support/yaml.ex b/periodic_scheduler/scheduler/test/support/yaml.ex index c479e8e69..8376d5316 100644 --- a/periodic_scheduler/scheduler/test/support/yaml.ex +++ b/periodic_scheduler/scheduler/test/support/yaml.ex @@ -4,14 +4,18 @@ defmodule Support.Yaml do """ def valid_definition(params) do + version = Map.get(params, :version, "v1.0") + branch_or_reference_field = if version == "v1.2", do: "reference", else: "branch" + branch_or_reference_value = Map.get(params, :reference, Map.get(params, :branch, "master")) + """ - apiVersion: v1.0 + apiVersion: #{version} kind: Schedule metadata: name: #{params.name} spec: project: #{params.project} - branch: #{params.branch} + #{branch_or_reference_field}: #{branch_or_reference_value} at: #{params.at} pipeline_file: #{params.pipeline_file} """ diff --git a/periodic_scheduler/scheduler/test/utils/git_reference_test.exs b/periodic_scheduler/scheduler/test/utils/git_reference_test.exs new file mode 100644 index 000000000..d5e328dd4 --- /dev/null +++ b/periodic_scheduler/scheduler/test/utils/git_reference_test.exs @@ -0,0 +1,96 @@ +defmodule Scheduler.Utils.GitReference.Test do + use ExUnit.Case + doctest Scheduler.Utils.GitReference + + alias Scheduler.Utils.GitReference + + describe "normalize/1" do + test "converts branch name to full reference" do + assert GitReference.normalize("master") == "refs/heads/master" + assert GitReference.normalize("develop") == "refs/heads/develop" + assert GitReference.normalize("feature-branch") == "refs/heads/feature-branch" + end + + test "leaves full references unchanged" do + assert GitReference.normalize("refs/heads/master") == "refs/heads/master" + assert GitReference.normalize("refs/tags/v1.0.0") == "refs/tags/v1.0.0" + assert GitReference.normalize("refs/pull/123/head") == "refs/pull/123/head" + end + + test "handles nil input" do + assert GitReference.normalize(nil) == nil + end + end + + describe "extract_name/1" do + test "extracts branch name from full reference" do + assert GitReference.extract_name("refs/heads/master") == "master" + assert GitReference.extract_name("refs/heads/develop") == "develop" + assert GitReference.extract_name("refs/heads/feature-branch") == "feature-branch" + end + + test "extracts tag name from full reference" do + assert GitReference.extract_name("refs/tags/v1.0.0") == "v1.0.0" + assert GitReference.extract_name("refs/tags/release") == "release" + end + + test "extracts PR reference from full reference" do + assert GitReference.extract_name("refs/pull/123/head") == "123/head" + end + + test "leaves short names unchanged" do + assert GitReference.extract_name("master") == "master" + assert GitReference.extract_name("develop") == "develop" + end + + test "handles nil input" do + assert GitReference.extract_name(nil) == nil + end + end + + describe "build_full_reference/2" do + test "builds branch references" do + assert GitReference.build_full_reference("BRANCH", "master") == "refs/heads/master" + assert GitReference.build_full_reference("BRANCH", "develop") == "refs/heads/develop" + end + + test "builds tag references" do + assert GitReference.build_full_reference("TAG", "v1.0.0") == "refs/tags/v1.0.0" + assert GitReference.build_full_reference("TAG", "release") == "refs/tags/release" + end + + test "builds PR references" do + assert GitReference.build_full_reference("PR", "123") == "refs/pull/123/head" + end + + test "returns name unchanged for unknown types" do + assert GitReference.build_full_reference("UNKNOWN", "something") == "something" + assert GitReference.build_full_reference("", "test") == "test" + end + end + + describe "get_type/1" do + test "identifies branch references" do + assert GitReference.get_type("refs/heads/master") == :branch + assert GitReference.get_type("refs/heads/develop") == :branch + assert GitReference.get_type("master") == :branch + end + + test "identifies tag references" do + assert GitReference.get_type("refs/tags/v1.0.0") == :tag + assert GitReference.get_type("refs/tags/release") == :tag + end + + test "identifies PR references" do + assert GitReference.get_type("refs/pull/123/head") == :pull_request + end + + test "handles unknown ref types" do + assert GitReference.get_type("refs/unknown/something") == :branch + end + + test "handles nil input" do + assert GitReference.get_type(nil) == nil + end + end +end diff --git a/periodic_scheduler/spec/priv/v1.2.yml b/periodic_scheduler/spec/priv/v1.2.yml new file mode 100644 index 000000000..d8931acec --- /dev/null +++ b/periodic_scheduler/spec/priv/v1.2.yml @@ -0,0 +1,51 @@ +$schema: http://json-schema.org/draft-04/schema# +version: v1.2 +title: Semaphore pipeline definition file specification +type: object +properties: + project: + type: string + reference: + type: object + properties: + type: + type: string + enum: [BRANCH, TAG, PR] + description: "The type of Git reference" + name: + type: string + description: "The name of the reference (branch name, tag name, or PR number)" + additionalProperties: false + required: [type, name] + at: + type: string + pipeline_file: + type: string + paused: + type: boolean + recurring: + type: boolean + parameters: + type: array + items: + type: object + properties: + name: + type: string + options: + type: array + items: + type: string + required: + type: boolean + default_value: + type: string + description: + type: string + additionalProperties: false + required: [name, required] +additionalProperties: false +required: [project, recurring] +anyOf: + - required: [reference] + - required: [branch] From 219ce3360584e1b1141c464194b67caf8da571b1 Mon Sep 17 00:00:00 2001 From: Amir Hasanbasic <43892661+hamir-suspect@users.noreply.github.com> Date: Tue, 23 Sep 2025 10:40:31 +0200 Subject: [PATCH 3/4] feat(projecthub): Support tags for periodic schedulers and tasks (#583) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description - Use persist instead of apply for periodic schedulers and tasks - refresh proto: update field name to reference instead of branch - support in `projecthub-rest-api` for reference object (if present, if not fallback to branch field) ## ✅ Checklist - [x] I have tested this change - [ ] This change requires documentation update --- .../lib/projecthub/http_api.ex | 68 +++++-- projecthub/docker-compose.yml | 1 - projecthub/lib/internal_api/artifacthub.pb.ex | 1 + .../lib/internal_api/periodic_scheduler.pb.ex | 8 +- projecthub/lib/internal_api/rbac.pb.ex | 1 + projecthub/lib/internal_api/user.pb.ex | 1 + .../lib/projecthub/models/periodic_task.ex | 30 +++ .../projecthub/models/periodic_task/grpc.ex | 3 + .../projecthub/models/periodic_task/yaml.ex | 35 +++- projecthub/lib/projecthub/models/scheduler.ex | 75 ++++---- .../test/projecthub/api/grpc_server_test.exs | 4 +- .../models/periodic_task/grpc_test.exs | 8 +- .../models/periodic_task/yaml_test.exs | 100 ++++++++-- .../projecthub/models/periodic_task_test.exs | 10 +- .../test/projecthub/models/scheduler_test.exs | 171 ++++++++++++------ .../test/projecthub/schedulers_test.exs | 2 +- .../periodic_scheduler_service.ex | 4 + 17 files changed, 383 insertions(+), 139 deletions(-) diff --git a/projecthub-rest-api/lib/projecthub/http_api.ex b/projecthub-rest-api/lib/projecthub/http_api.ex index 2c771c1e7..f9c14ad3a 100644 --- a/projecthub-rest-api/lib/projecthub/http_api.ex +++ b/projecthub-rest-api/lib/projecthub/http_api.ex @@ -507,16 +507,32 @@ defmodule Projecthub.HttpApi do schedulers |> Enum.map(fn scheduler -> - case scheduler.status do - @unspecified_status -> - Map.delete(scheduler, :status) - - _ -> - Map.put(scheduler, :status, encode_scheduler_status(scheduler.status)) - end + scheduler + |> encode_scheduler_status_field() + |> encode_reference_field() end) end + defp encode_scheduler_status_field(scheduler) do + case scheduler.status do + @unspecified_status -> + Map.delete(scheduler, :status) + + status -> + Map.put(scheduler, :status, encode_scheduler_status(status)) + end + end + + defp encode_reference_field(%{branch: "refs/tags/" <> tag_name} = scheduler) do + Map.put(scheduler, :reference, %{"type" => "tag", "name" => tag_name}) + end + + defp encode_reference_field(%{branch: "refs/heads/" <> branch_name} = scheduler) do + Map.put(scheduler, :reference, %{"type" => "branch", "name" => branch_name}) + end + + defp encode_reference_field(scheduler), do: scheduler + defp encode_scheduler_status(@status_inactive), do: "INACTIVE" defp encode_scheduler_status(@status_active), do: "ACTIVE" @@ -528,18 +544,24 @@ defmodule Projecthub.HttpApi do defp encode_tasks(tasks) do tasks |> Stream.map(fn task -> - case task.status do - @task_unspecified_status -> - Map.delete(task, :status) - - _ -> - Map.put(task, :status, encode_task_status(task.status)) - end + task + |> encode_task_status_field() + |> encode_reference_field() end) |> Stream.map(&Map.put(&1, :scheduled, &1.recurring)) |> Enum.map(&Map.delete(&1, :recurring)) end + defp encode_task_status_field(task) do + case task.status do + @task_unspecified_status -> + Map.delete(task, :status) + + status -> + Map.put(task, :status, encode_task_status(status)) + end + end + defp encode_task_status(@task_status_inactive), do: "INACTIVE" defp encode_task_status(@task_status_active), do: "ACTIVE" @@ -596,7 +618,9 @@ defmodule Projecthub.HttpApi do Scheduler.new( id: scheduler["id"] || "", name: scheduler["name"], - branch: scheduler["branch"], + branch: + scheduler["branch"] || + construct_reference(scheduler["reference_type"], scheduler["reference_name"]), at: scheduler["at"], pipeline_file: scheduler["pipeline_file"], status: scheduler_status(scheduler["status"]) @@ -618,7 +642,9 @@ defmodule Projecthub.HttpApi do name: task["name"], description: task["description"] || "", recurring: if(is_nil(task["scheduled"]), do: true, else: task["scheduled"]), - branch: task["branch"] || "", + branch: + task["branch"] || construct_reference(task["reference_type"], task["reference_name"]) || + "", at: task["at"] || "", pipeline_file: task["pipeline_file"] || "", parameters: construct_task_parameters(task["parameters"]), @@ -630,6 +656,16 @@ defmodule Projecthub.HttpApi do end end + defp construct_reference("branch", reference_name) do + "refs/heads/#{reference_name}" + end + + defp construct_reference("tag", reference_name) do + "refs/tags/#{reference_name}" + end + + defp construct_reference(_, _), do: nil + defp construct_task_parameters(raw_task_parameters) do alias InternalApi.Projecthub.Project.Spec.Task.Parameter, as: SpecTaskParameter diff --git a/projecthub/docker-compose.yml b/projecthub/docker-compose.yml index a29143c10..e7a519588 100644 --- a/projecthub/docker-compose.yml +++ b/projecthub/docker-compose.yml @@ -2,7 +2,6 @@ version: '3.6' services: app: - platform: linux/amd64 container_name: projecthub image: "${IMAGE:-projecthub-app}:${IMAGE_TAG:-latest}" build: diff --git a/projecthub/lib/internal_api/artifacthub.pb.ex b/projecthub/lib/internal_api/artifacthub.pb.ex index 88b515a62..d35d9896f 100644 --- a/projecthub/lib/internal_api/artifacthub.pb.ex +++ b/projecthub/lib/internal_api/artifacthub.pb.ex @@ -258,6 +258,7 @@ defmodule InternalApi.Artifacthub.ListItem do field(:name, 1, type: :string) field(:is_directory, 2, type: :bool, json_name: "isDirectory") + field(:size, 3, type: :int64) end defmodule InternalApi.Artifacthub.Artifact do diff --git a/projecthub/lib/internal_api/periodic_scheduler.pb.ex b/projecthub/lib/internal_api/periodic_scheduler.pb.ex index 239f8f06c..1a1661eef 100644 --- a/projecthub/lib/internal_api/periodic_scheduler.pb.ex +++ b/projecthub/lib/internal_api/periodic_scheduler.pb.ex @@ -61,7 +61,7 @@ defmodule InternalApi.PeriodicScheduler.PersistRequest do field(:organization_id, 6, type: :string, json_name: "organizationId") field(:project_name, 7, type: :string, json_name: "projectName") field(:requester_id, 8, type: :string, json_name: "requesterId") - field(:branch, 9, type: :string) + field(:reference, 9, type: :string) field(:pipeline_file, 10, type: :string, json_name: "pipelineFile") field(:at, 11, type: :string) field(:parameters, 12, repeated: true, type: InternalApi.PeriodicScheduler.Periodic.Parameter) @@ -112,7 +112,7 @@ defmodule InternalApi.PeriodicScheduler.RunNowRequest do field(:id, 1, type: :string) field(:requester, 2, type: :string) - field(:branch, 3, type: :string) + field(:reference, 3, type: :string) field(:pipeline_file, 4, type: :string, json_name: "pipelineFile") field(:parameter_values, 5, @@ -166,7 +166,7 @@ defmodule InternalApi.PeriodicScheduler.Periodic do field(:id, 1, type: :string) field(:name, 2, type: :string) field(:project_id, 3, type: :string, json_name: "projectId") - field(:branch, 4, type: :string) + field(:reference, 4, type: :string) field(:at, 5, type: :string) field(:pipeline_file, 6, type: :string, json_name: "pipelineFile") field(:requester_id, 7, type: :string, json_name: "requesterId") @@ -188,7 +188,7 @@ defmodule InternalApi.PeriodicScheduler.Trigger do field(:triggered_at, 1, type: Google.Protobuf.Timestamp, json_name: "triggeredAt") field(:project_id, 2, type: :string, json_name: "projectId") - field(:branch, 3, type: :string) + field(:reference, 3, type: :string) field(:pipeline_file, 4, type: :string, json_name: "pipelineFile") field(:scheduling_status, 5, type: :string, json_name: "schedulingStatus") field(:scheduled_workflow_id, 6, type: :string, json_name: "scheduledWorkflowId") diff --git a/projecthub/lib/internal_api/rbac.pb.ex b/projecthub/lib/internal_api/rbac.pb.ex index 2b68009be..1202a9b71 100644 --- a/projecthub/lib/internal_api/rbac.pb.ex +++ b/projecthub/lib/internal_api/rbac.pb.ex @@ -4,6 +4,7 @@ defmodule InternalApi.RBAC.SubjectType do field(:USER, 0) field(:GROUP, 1) + field(:SERVICE_ACCOUNT, 2) end defmodule InternalApi.RBAC.Scope do diff --git a/projecthub/lib/internal_api/user.pb.ex b/projecthub/lib/internal_api/user.pb.ex index e2137ba60..9bbd3fc95 100644 --- a/projecthub/lib/internal_api/user.pb.ex +++ b/projecthub/lib/internal_api/user.pb.ex @@ -50,6 +50,7 @@ defmodule InternalApi.User.User.CreationSource do field(:NOT_SET, 0) field(:OKTA, 1) + field(:SERVICE_ACCOUNT, 2) end defmodule InternalApi.User.ListFavoritesRequest do diff --git a/projecthub/lib/projecthub/models/periodic_task.ex b/projecthub/lib/projecthub/models/periodic_task.ex index 2015d0ccc..eac4a8a85 100644 --- a/projecthub/lib/projecthub/models/periodic_task.ex +++ b/projecthub/lib/projecthub/models/periodic_task.ex @@ -15,12 +15,16 @@ defmodule Projecthub.Models.PeriodicTask do parameters = Enum.into(periodic_or_task.parameters, [], &construct_parameter/1) status = construct_status(periodic_or_task) + # Handle reference-to-branch mapping for gRPC responses + branch = extract_branch_from_reference_or_branch(periodic_or_task) + params = periodic_or_task |> Map.take(@fields) |> Map.put(:project_name, project_name) |> Map.put(:status, status) |> Map.put(:parameters, parameters) + |> Map.put(:branch, branch) |> Map.merge(Map.new()) struct!(__MODULE__, params) @@ -114,4 +118,30 @@ defmodule Projecthub.Models.PeriodicTask do end end) end + + # Helper function to extract branch name from reference or fall back to branch field + # This handles the transition from gRPC "branch" field to "reference" field + defp extract_branch_from_reference_or_branch(%{reference: reference}) when is_binary(reference) do + extract_branch_name(reference) + end + + defp extract_branch_from_reference_or_branch(%{branch: branch}) when is_binary(branch) do + branch + end + + defp extract_branch_from_reference_or_branch(_), do: nil + + # Helper function to extract branch name from Git reference format + # "refs/heads/main" -> "main" + # "refs/tags/v1.0" -> "refs/tags/v1.0" + # "main" -> "main" (fallback for plain strings) + defp extract_branch_name(reference) when is_binary(reference) do + if String.starts_with?(reference, "refs/heads/") do + String.replace_prefix(reference, "refs/heads/", "") + else + reference + end + end + + defp extract_branch_name(_), do: nil end diff --git a/projecthub/lib/projecthub/models/periodic_task/grpc.ex b/projecthub/lib/projecthub/models/periodic_task/grpc.ex index d9325555c..a3f6334cc 100644 --- a/projecthub/lib/projecthub/models/periodic_task/grpc.ex +++ b/projecthub/lib/projecthub/models/periodic_task/grpc.ex @@ -22,6 +22,7 @@ defmodule Projecthub.Models.PeriodicTask.GRPC do @doc """ Creates or updates a task + Note: This still uses ApplyRequest for YAML-based task creation """ @spec upsert(String.t(), String.t(), String.t()) :: {:ok, String.t()} | {:error, any()} def upsert(yml_definition, organization_id, requester_id) do @@ -69,10 +70,12 @@ defmodule Projecthub.Models.PeriodicTask.GRPC do defp stub_func(%API.ListRequest{}), do: &Stub.list/3 defp stub_func(%API.ApplyRequest{}), do: &Stub.apply/3 + defp stub_func(%API.PersistRequest{}), do: &Stub.persist/3 defp stub_func(%API.DeleteRequest{}), do: &Stub.delete/3 defp parse_response(%{code: :OK}, _request, %API.ListResponse{} = response), do: {:ok, response.periodics} defp parse_response(%{code: :OK}, _request, %API.ApplyResponse{} = response), do: {:ok, response.id} + defp parse_response(%{code: :OK}, _request, %API.PersistResponse{} = response), do: {:ok, response.periodic.id} defp parse_response(%{code: :OK}, %API.DeleteRequest{} = request, _response), do: {:ok, request.id} defp parse_response(%{code: code, message: message}, _request, _response) diff --git a/projecthub/lib/projecthub/models/periodic_task/yaml.ex b/projecthub/lib/projecthub/models/periodic_task/yaml.ex index 834afa312..4406f95fd 100644 --- a/projecthub/lib/projecthub/models/periodic_task/yaml.ex +++ b/projecthub/lib/projecthub/models/periodic_task/yaml.ex @@ -21,8 +21,10 @@ defmodule Projecthub.Models.PeriodicTask.YAML do end defp base_yml_definition(task, project) do + reference_yml = compose_reference_yaml(task.branch, 2) + """ - apiVersion: v1.1 + apiVersion: v1.2 kind: Schedule metadata: name: \"#{task.name}\" @@ -33,8 +35,7 @@ defmodule Projecthub.Models.PeriodicTask.YAML do recurring: #{task.recurring} paused: #{task.status == :STATUS_INACTIVE} at: \"#{task.at}\" - branch: \"#{task.branch}\" - pipeline_file: \"#{task.pipeline_file}\" + #{reference_yml} pipeline_file: \"#{task.pipeline_file}\" """ end @@ -78,6 +79,34 @@ defmodule Projecthub.Models.PeriodicTask.YAML do defp indentation(number), do: 0..(number - 1) |> Enum.map_join(fn _ -> " " end) + # Composes reference YAML according to v1.2 spec + defp compose_reference_yaml(branch, indent) when is_binary(branch) do + {ref_type, ref_name} = parse_reference(branch) + + "#{indentation(indent)}reference:\n" <> + "#{indentation(indent)} type: #{ref_type}\n" <> + "#{indentation(indent)} name: \"#{ref_name}\"\n" + end + + defp compose_reference_yaml(_, indent), + do: + "#{indentation(indent)}reference:\n#{indentation(indent)} type: BRANCH\n#{indentation(indent)} name: \"master\"\n" + + # Parse different reference formats to type and name + defp parse_reference("refs/heads/" <> branch_name), do: {"BRANCH", branch_name} + defp parse_reference("refs/tags/" <> tag_name), do: {"TAG", tag_name} + defp parse_reference("refs/pull/" <> pr_ref), do: {"PR", extract_pr_number(pr_ref)} + defp parse_reference(branch_name) when is_binary(branch_name) and branch_name != "", do: {"BRANCH", branch_name} + defp parse_reference(_), do: {"BRANCH", "master"} + + # Extract PR number from refs/pull/123/head format + defp extract_pr_number(pr_ref) do + case String.split(pr_ref, "/") do + [pr_number | _] -> pr_number + _ -> pr_ref + end + end + defp empty?(nil), do: true defp empty?(""), do: true defp empty?([]), do: true diff --git a/projecthub/lib/projecthub/models/scheduler.ex b/projecthub/lib/projecthub/models/scheduler.ex index 3e68f4ece..9168d88d6 100644 --- a/projecthub/lib/projecthub/models/scheduler.ex +++ b/projecthub/lib/projecthub/models/scheduler.ex @@ -41,18 +41,26 @@ defmodule Projecthub.Models.Scheduler do end def apply(scheduler, project, requester_id, metadata \\ nil) do - scheduler_yml_definition = to_yaml(scheduler, project) - - Logger.info("Scheduler yml definition: #{inspect(scheduler_yml_definition)}") + Logger.info("Creating/updating scheduler #{scheduler.name} for project #{project.id}") req = - InternalApi.PeriodicScheduler.ApplyRequest.new( + InternalApi.PeriodicScheduler.PersistRequest.new( + id: scheduler.id, + name: scheduler.name, + description: "", + recurring: true, + state: :UNCHANGED, organization_id: project.organization_id, + project_name: project.name, requester_id: requester_id, - yml_definition: scheduler_yml_definition + reference: format_branch_as_reference(scheduler.branch), + pipeline_file: scheduler.pipeline_file, + at: scheduler.at, + parameters: [], + project_id: project.id ) - {:ok, res} = InternalApi.PeriodicScheduler.PeriodicService.Stub.apply(channel(), req, options(metadata)) + {:ok, res} = InternalApi.PeriodicScheduler.PeriodicService.Stub.persist(channel(), req, options(metadata)) if res.status.code == status_ok() do {:ok, nil} @@ -65,33 +73,6 @@ defmodule Projecthub.Models.Scheduler do end end - defp to_yaml(scheduler = %{status: status}, project) when status != :STATUS_UNSPECIFIED and status != nil do - yaml = - scheduler - |> Map.put(:status, :STATUS_UNSPECIFIED) - |> to_yaml(project) - - yaml <> - """ - paused: #{scheduler.status == :STATUS_INACTIVE} - """ - end - - defp to_yaml(scheduler, project) do - """ - apiVersion: v1.0 - kind: Schedule - metadata: - name: \"#{scheduler.name}\" - id: \"#{scheduler.id}\" - spec: - project: \"#{project.name}\" - branch: \"#{scheduler.branch}\" - at: \"#{scheduler.at}\" - pipeline_file: \"#{scheduler.pipeline_file}\" - """ - end - def construct_list(raw_schedulers) do raw_schedulers |> Enum.map(fn s -> construct(s) end) @@ -101,7 +82,7 @@ defmodule Projecthub.Models.Scheduler do %__MODULE__{ :id => raw_scheduler.id, :name => raw_scheduler.name, - :branch => raw_scheduler.branch, + :branch => extract_branch_name(raw_scheduler.reference), :at => raw_scheduler.at, :pipeline_file => raw_scheduler.pipeline_file, :status => construct_status(raw_scheduler.paused) @@ -146,4 +127,30 @@ defmodule Projecthub.Models.Scheduler do end defp status_ok, do: :OK + + # Helper function to extract branch name from Git reference format + # "refs/heads/main" -> "main" + # "refs/tags/v1.0" -> "refs/tags/v1.0" + # "main" -> "main" (fallback for plain strings) + defp extract_branch_name(reference) when is_binary(reference) do + if String.starts_with?(reference, "refs/heads/") do + String.replace_prefix(reference, "refs/heads/", "") + else + reference + end + end + + defp extract_branch_name(_), do: "" + + # Helper function to format branch name as Git reference + # "main" -> "refs/heads/main" + # "refs/tags/v1.0" -> "refs/tags/v1.0" (default to branch format) + defp format_branch_as_reference("refs/tags/" <> _ = tag), do: tag + defp format_branch_as_reference("refs/pull/" <> _ = pr), do: pr + + defp format_branch_as_reference(branch_name) when is_binary(branch_name) do + "refs/heads/#{branch_name}" + end + + defp format_branch_as_reference(_), do: "refs/heads/main" end diff --git a/projecthub/test/projecthub/api/grpc_server_test.exs b/projecthub/test/projecthub/api/grpc_server_test.exs index 8b2d98a13..6c1ae611e 100644 --- a/projecthub/test/projecthub/api/grpc_server_test.exs +++ b/projecthub/test/projecthub/api/grpc_server_test.exs @@ -250,7 +250,7 @@ defmodule Projecthub.Api.GrpcServerTest do id: "12345678-1234-5678-1234-567812345678", name: "test", project_id: "12345678-1234-5678-1234-567812345678", - branch: "master", + reference: "refs/heads/master", at: "0 0 * * *", pipeline_file: ".semaphore/semaphore.yml", requester_id: "12345678-1234-5678-1234-567812345678", @@ -418,7 +418,7 @@ defmodule Projecthub.Api.GrpcServerTest do name: "test", description: "test description", project_id: "12345678-1234-5678-1234-567812345678", - branch: "master", + reference: "refs/heads/master", at: "", pipeline_file: ".semaphore/semaphore.yml", requester_id: "12345678-1234-5678-1234-567812345678", diff --git a/projecthub/test/projecthub/models/periodic_task/grpc_test.exs b/projecthub/test/projecthub/models/periodic_task/grpc_test.exs index 999d253db..580ba52b4 100644 --- a/projecthub/test/projecthub/models/periodic_task/grpc_test.exs +++ b/projecthub/test/projecthub/models/periodic_task/grpc_test.exs @@ -79,7 +79,7 @@ defmodule Projecthub.Models.PeriodicTask.GrpcTest do name: "cron", recurring: true, project_id: "project_id", - branch: "master", + reference: "refs/heads/master", at: "0 0 * * *", pipeline_file: ".semaphore/cron.yml", parameters: [ @@ -96,7 +96,7 @@ defmodule Projecthub.Models.PeriodicTask.GrpcTest do defp yml_definition do """ - apiVersion: v1.1 + apiVersion: v1.2 kind: Schedule metadata: name: \"cron\" @@ -105,7 +105,9 @@ defmodule Projecthub.Models.PeriodicTask.GrpcTest do project: \"project_name\" recurring: true at: \"0 0 * * *\" - branch: \"master\" + reference: + type: BRANCH + name: \"master\" pipeline_file: \".semaphore/cron.yml\" parameters: - name: \"foo\" diff --git a/projecthub/test/projecthub/models/periodic_task/yaml_test.exs b/projecthub/test/projecthub/models/periodic_task/yaml_test.exs index 0a026c57b..c7f2f698e 100644 --- a/projecthub/test/projecthub/models/periodic_task/yaml_test.exs +++ b/projecthub/test/projecthub/models/periodic_task/yaml_test.exs @@ -7,7 +7,7 @@ defmodule Projecthub.Models.PeriodicTask.YamlTest do describe "compose/2" do test "paused" do expected = """ - apiVersion: v1.1 + apiVersion: v1.2 kind: Schedule metadata: name: \"name\" @@ -18,7 +18,9 @@ defmodule Projecthub.Models.PeriodicTask.YamlTest do recurring: true paused: true at: \"* * * * *\" - branch: \"master\" + reference: + type: BRANCH + name: \"master\" pipeline_file: \"semaphore.yml\" """ @@ -41,7 +43,7 @@ defmodule Projecthub.Models.PeriodicTask.YamlTest do test "without description" do expected = """ - apiVersion: v1.1 + apiVersion: v1.2 kind: Schedule metadata: name: \"name\" @@ -52,7 +54,9 @@ defmodule Projecthub.Models.PeriodicTask.YamlTest do recurring: true paused: true at: \"* * * * *\" - branch: \"master\" + reference: + type: BRANCH + name: \"master\" pipeline_file: \"semaphore.yml\" """ @@ -75,7 +79,7 @@ defmodule Projecthub.Models.PeriodicTask.YamlTest do test "without parameters" do expected = """ - apiVersion: v1.1 + apiVersion: v1.2 kind: Schedule metadata: name: \"name\" @@ -86,7 +90,9 @@ defmodule Projecthub.Models.PeriodicTask.YamlTest do recurring: true paused: false at: \"* * * * *\" - branch: \"master\" + reference: + type: BRANCH + name: \"master\" pipeline_file: \"semaphore.yml\" """ @@ -108,7 +114,7 @@ defmodule Projecthub.Models.PeriodicTask.YamlTest do test "without cron expression" do expected = """ - apiVersion: v1.1 + apiVersion: v1.2 kind: Schedule metadata: name: \"name\" @@ -119,7 +125,9 @@ defmodule Projecthub.Models.PeriodicTask.YamlTest do recurring: false paused: false at: \"\" - branch: \"master\" + reference: + type: BRANCH + name: \"master\" pipeline_file: \"semaphore.yml\" """ @@ -138,9 +146,9 @@ defmodule Projecthub.Models.PeriodicTask.YamlTest do ) end - test "with branch, pipeline file and parameters" do + test "with reference, pipeline file and parameters" do expected = """ - apiVersion: v1.1 + apiVersion: v1.2 kind: Schedule metadata: name: \"name\" @@ -151,7 +159,9 @@ defmodule Projecthub.Models.PeriodicTask.YamlTest do recurring: true paused: false at: \"* * * * *\" - branch: \"master\" + reference: + type: BRANCH + name: \"master\" pipeline_file: \"semaphore.yml\" parameters: - name: \"parameter1\" @@ -193,5 +203,73 @@ defmodule Projecthub.Models.PeriodicTask.YamlTest do %Project{name: "project_name"} ) end + + test "with tag reference" do + expected = """ + apiVersion: v1.2 + kind: Schedule + metadata: + name: \"release-task\" + id: \"tag-id\" + description: "tag release task" + spec: + project: \"project_name\" + recurring: false + paused: false + at: \"\" + reference: + type: TAG + name: \"v1.0.0\" + pipeline_file: \"semaphore.yml\" + """ + + assert ^expected = + Projecthub.Models.PeriodicTask.YAML.compose( + %PeriodicTask{ + id: "tag-id", + name: "release-task", + description: "tag release task", + project_name: "project_name", + recurring: false, + branch: "refs/tags/v1.0.0", + pipeline_file: "semaphore.yml" + }, + %Project{name: "project_name"} + ) + end + + test "with pull request reference" do + expected = """ + apiVersion: v1.2 + kind: Schedule + metadata: + name: \"pr-task\" + id: \"pr-id\" + description: "PR task" + spec: + project: \"project_name\" + recurring: false + paused: false + at: \"\" + reference: + type: PR + name: \"123\" + pipeline_file: \"semaphore.yml\" + """ + + assert ^expected = + Projecthub.Models.PeriodicTask.YAML.compose( + %PeriodicTask{ + id: "pr-id", + name: "pr-task", + description: "PR task", + project_name: "project_name", + recurring: false, + branch: "refs/pull/123/head", + pipeline_file: "semaphore.yml" + }, + %Project{name: "project_name"} + ) + end end end diff --git a/projecthub/test/projecthub/models/periodic_task_test.exs b/projecthub/test/projecthub/models/periodic_task_test.exs index 10c15a4b3..e2e131485 100644 --- a/projecthub/test/projecthub/models/periodic_task_test.exs +++ b/projecthub/test/projecthub/models/periodic_task_test.exs @@ -20,7 +20,7 @@ defmodule Projecthub.Models.PeriodicTaskTest do name: "task1", recurring: false, at: "", - branch: "", + reference: "", pipeline_file: "" }), periodic2: @@ -39,7 +39,7 @@ defmodule Projecthub.Models.PeriodicTaskTest do name: "task3", recurring: false, at: "", - branch: "develop", + reference: "refs/heads/develop", pipeline_file: ".semaphore/semaphore.yml", paused: true })} @@ -157,7 +157,7 @@ defmodule Projecthub.Models.PeriodicTaskTest do FunRegistry.set!(PeriodicService, :apply, fn request, _stream -> assert request.yml_definition == """ - apiVersion: v1.1 + apiVersion: v1.2 kind: Schedule metadata: name: "task" @@ -168,7 +168,9 @@ defmodule Projecthub.Models.PeriodicTaskTest do recurring: true paused: false at: "0 0 * * *" - branch: "master" + reference: + type: BRANCH + name: "master" pipeline_file: "pipeline.yml" """ diff --git a/projecthub/test/projecthub/models/scheduler_test.exs b/projecthub/test/projecthub/models/scheduler_test.exs index 46277e80e..b87d643e5 100644 --- a/projecthub/test/projecthub/models/scheduler_test.exs +++ b/projecthub/test/projecthub/models/scheduler_test.exs @@ -33,7 +33,7 @@ defmodule Projecthub.Models.SchedulerTest do id: "12345678-1234-5678-1234-567812345678", name: "cron", project_id: "12345678-1234-5678-1234-567812345678", - branch: "master", + reference: "refs/heads/master", at: "*", pipeline_file: ".semaphore/cron.yml", paused: false @@ -130,39 +130,37 @@ defmodule Projecthub.Models.SchedulerTest do {:ok, project} = Support.Factories.Project.create() - apply_response = - InternalApi.PeriodicScheduler.ApplyResponse.new( + persist_response = + InternalApi.PeriodicScheduler.PersistResponse.new( status: InternalApi.Status.new(code: Google.Rpc.Code.value(:OK)) ) - FunRegistry.set!(Support.FakeServices.PeriodicSchedulerService, :apply, fn req, _s -> + FunRegistry.set!(Support.FakeServices.PeriodicSchedulerService, :persist, fn req, _s -> assert req.organization_id == project.organization_id assert req.requester_id == "requester_id" + assert req.id == scheduler.id + assert req.name == scheduler.name + assert req.description == "" + assert req.recurring == true + assert req.state == :UNCHANGED + assert req.project_name == project.name + assert req.reference == "refs/heads/master" + assert req.pipeline_file == scheduler.pipeline_file + assert req.at == scheduler.at + assert req.parameters == [] + assert req.project_id == project.id - assert req.yml_definition == """ - apiVersion: v1.0 - kind: Schedule - metadata: - name: \"cron\" - id: \"12345678-1234-5678-1234-567812345678\" - spec: - project: \"#{project.name}\" - branch: \"master\" - at: \"*\" - pipeline_file: \".semaphore/cron.yml\" - """ - - apply_response + persist_response end) {:ok, nil} = Scheduler.apply(scheduler, project, "requester_id") end - test "it applies scheduler paused with correct params and returns ok" do + test "it applies scheduler with tag reference correctly" do scheduler = %Scheduler{ id: "12345678-1234-5678-1234-567812345678", name: "cron", - branch: "master", + branch: "refs/tags/v1.0.0", at: "*", pipeline_file: ".semaphore/cron.yml", status: :STATUS_ACTIVE @@ -170,36 +168,23 @@ defmodule Projecthub.Models.SchedulerTest do {:ok, project} = Support.Factories.Project.create() - apply_response = - InternalApi.PeriodicScheduler.ApplyResponse.new( + persist_response = + InternalApi.PeriodicScheduler.PersistResponse.new( status: InternalApi.Status.new(code: Google.Rpc.Code.value(:OK)) ) - FunRegistry.set!(Support.FakeServices.PeriodicSchedulerService, :apply, fn req, _s -> + FunRegistry.set!(Support.FakeServices.PeriodicSchedulerService, :persist, fn req, _s -> assert req.organization_id == project.organization_id assert req.requester_id == "requester_id" + assert req.reference == "refs/tags/v1.0.0" - assert req.yml_definition == """ - apiVersion: v1.0 - kind: Schedule - metadata: - name: \"cron\" - id: \"12345678-1234-5678-1234-567812345678\" - spec: - project: \"#{project.name}\" - branch: \"master\" - at: \"*\" - pipeline_file: \".semaphore/cron.yml\" - paused: false - """ - - apply_response + persist_response end) {:ok, nil} = Scheduler.apply(scheduler, project, "requester_id") end - test "when scheduler is new => send yml with empty ID" do + test "when scheduler is new => send request with empty ID" do scheduler = %Scheduler{ id: "", name: "cron", @@ -211,30 +196,21 @@ defmodule Projecthub.Models.SchedulerTest do {:ok, project} = Support.Factories.Project.create() - apply_response = - InternalApi.PeriodicScheduler.ApplyResponse.new( + persist_response = + InternalApi.PeriodicScheduler.PersistResponse.new( status: InternalApi.Status.new(code: Google.Rpc.Code.value(:OK)) ) - yml = """ - apiVersion: v1.0 - kind: Schedule - metadata: - name: \"#{scheduler.name}\" - id: \"\" - spec: - project: \"#{project.name}\" - branch: \"#{scheduler.branch}\" - at: \"#{scheduler.at}\" - pipeline_file: \"#{scheduler.pipeline_file}\" - """ - - FunRegistry.set!(Support.FakeServices.PeriodicSchedulerService, :apply, fn req, _s -> + FunRegistry.set!(Support.FakeServices.PeriodicSchedulerService, :persist, fn req, _s -> assert req.organization_id == project.organization_id assert req.requester_id == "requester_id" - assert req.yml_definition == yml + assert req.id == "" + assert req.name == scheduler.name + assert req.reference == "refs/heads/master" + assert req.at == scheduler.at + assert req.pipeline_file == scheduler.pipeline_file - apply_response + persist_response end) {:ok, nil} = Scheduler.apply(scheduler, project, "requester_id") @@ -244,8 +220,8 @@ defmodule Projecthub.Models.SchedulerTest do scheduler = %Scheduler{id: ""} {:ok, project} = Support.Factories.Project.create() - apply_response = - InternalApi.PeriodicScheduler.ApplyResponse.new( + persist_response = + InternalApi.PeriodicScheduler.PersistResponse.new( status: InternalApi.Status.new( code: Google.Rpc.Code.value(:FAILED_PRECONDITION), @@ -253,10 +229,85 @@ defmodule Projecthub.Models.SchedulerTest do ) ) - FunRegistry.set!(Support.FakeServices.PeriodicSchedulerService, :apply, apply_response) + FunRegistry.set!(Support.FakeServices.PeriodicSchedulerService, :persist, persist_response) {:error, error} = Scheduler.apply(scheduler, project, "requester_id") assert error == "Failed precondition" end end + + describe "reference/tag support" do + test "constructs scheduler with branch reference correctly" do + raw_scheduler = + InternalApi.PeriodicScheduler.Periodic.new( + id: "123", + name: "test", + reference: "refs/heads/main", + at: "*", + pipeline_file: "test.yml", + paused: false + ) + + scheduler = Projecthub.Models.Scheduler.construct_list([raw_scheduler]) |> List.first() + + assert scheduler.branch == "main" + assert scheduler.status == :STATUS_ACTIVE + end + + test "constructs scheduler with tag reference correctly" do + raw_scheduler = + InternalApi.PeriodicScheduler.Periodic.new( + id: "123", + name: "test", + reference: "refs/tags/v1.0.0", + at: "*", + pipeline_file: "test.yml", + paused: false + ) + + scheduler = Projecthub.Models.Scheduler.construct_list([raw_scheduler]) |> List.first() + + assert scheduler.branch == "refs/tags/v1.0.0" + end + + test "constructs scheduler with pull request reference correctly" do + raw_scheduler = + InternalApi.PeriodicScheduler.Periodic.new( + id: "123", + name: "test", + reference: "refs/pull/42/head", + at: "*", + pipeline_file: "test.yml", + paused: false + ) + + scheduler = Projecthub.Models.Scheduler.construct_list([raw_scheduler]) |> List.first() + + assert scheduler.branch == "refs/pull/42/head" + end + + test "applies scheduler with pull request reference correctly" do + scheduler = %Scheduler{ + id: "12345678-1234-5678-1234-567812345678", + name: "pr-check", + branch: "refs/pull/42/head", + at: "*", + pipeline_file: ".semaphore/pr.yml" + } + + {:ok, project} = Support.Factories.Project.create() + + persist_response = + InternalApi.PeriodicScheduler.PersistResponse.new( + status: InternalApi.Status.new(code: Google.Rpc.Code.value(:OK)) + ) + + FunRegistry.set!(Support.FakeServices.PeriodicSchedulerService, :persist, fn req, _s -> + assert req.reference == "refs/pull/42/head" + persist_response + end) + + {:ok, nil} = Scheduler.apply(scheduler, project, "requester_id") + end + end end diff --git a/projecthub/test/projecthub/schedulers_test.exs b/projecthub/test/projecthub/schedulers_test.exs index 4b209b1d2..160d74fc7 100644 --- a/projecthub/test/projecthub/schedulers_test.exs +++ b/projecthub/test/projecthub/schedulers_test.exs @@ -12,7 +12,7 @@ defmodule Projecthub.SchedulersTest do id: "12345678-1234-5678-1234-567812345678", name: "cron", project_id: "12345678-1234-5678-1234-567812345678", - branch: "master", + reference: "refs/heads/master", at: "*", pipeline_file: ".semaphore/cron.yml" ) diff --git a/projecthub/test/support/fake_services/periodic_scheduler_service.ex b/projecthub/test/support/fake_services/periodic_scheduler_service.ex index 80cc565d2..565036cdb 100644 --- a/projecthub/test/support/fake_services/periodic_scheduler_service.ex +++ b/projecthub/test/support/fake_services/periodic_scheduler_service.ex @@ -15,4 +15,8 @@ defmodule Support.FakeServices.PeriodicSchedulerService do def apply(req, stream) do FunRegistry.run!(__MODULE__, :apply, [req, stream]) end + + def persist(req, stream) do + FunRegistry.run!(__MODULE__, :persist, [req, stream]) + end end From f25352732971b1e286bd4fe023c49f6ce2ba6b9a Mon Sep 17 00:00:00 2001 From: Amir Hasanbasic <43892661+hamir-suspect@users.noreply.github.com> Date: Tue, 23 Sep 2025 10:44:30 +0200 Subject: [PATCH 4/4] feat(public-api): Support tags for tasks/periodics (#584) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📝 Description - Added tag support to task/periodic scheduler endpoints: Create, update, replace, and trigger operations now accept an optional tags parameter - Extended internal gRPC clients: Updated request/response formatters to handle tags when communicating with the periodic scheduler service - Updated OpenAPI schemas: Added tag field definitions to task specifications and trigger specifications ## ✅ Checklist - [x] I have tested this change - [ ] This change requires documentation update --- public-api/v1alpha/Makefile | 3 +- .../lib/internal_api/artifacthub.pb.ex | 6 +- .../lib/internal_api/periodic_scheduler.pb.ex | 24 +-- .../v1alpha/lib/internal_api/rbac.pb.ex | 1 + .../v1alpha/lib/internal_api/user.pb.ex | 1 + .../request_fromatter.ex | 39 +++-- .../response_formatter.ex | 22 ++- .../lib/pipelines_api/schedules/run_now.ex | 101 ++++++++++++- .../request_formatter_test.exs | 54 ++++++- .../test/router/schedules/run_now_test.exs | 94 +++++++++++- .../v1alpha/test/support/stubs/scheduler.ex | 12 +- public-api/v2/docker-compose.yml | 2 +- .../v2/lib/internal_api/artifacthub.pb.ex | 1 + public-api/v2/lib/internal_api/audit.pb.ex | 1 + .../v2/lib/internal_api/notifications.pb.ex | 1 + .../lib/internal_api/periodic_scheduler.pb.ex | 8 +- .../internal_api/plumber_w_f.workflow.pb.ex | 3 + public-api/v2/lib/internal_api/rbac.pb.ex | 1 + public-api/v2/lib/internal_api/user.pb.ex | 1 + .../schedulers_client/request_formatter.ex | 22 ++- .../schedulers_client/response_formatter.ex | 21 ++- .../lib/public_api/handlers/tasks/create.ex | 1 + .../lib/public_api/handlers/tasks/replace.ex | 1 + .../lib/public_api/handlers/tasks/trigger.ex | 1 + .../v2/lib/public_api/schemas/tasks/spec.ex | 21 ++- .../public_api/schemas/tasks/triggers/spec.ex | 25 +++- .../request_formatter_test.exs | 139 ++++++++++++++++++ .../schedulers_client_test.exs | 55 +++++-- .../v2/test/router/tasks/create_test.exs | 99 ++++++++++++- .../v2/test/router/tasks/replace_test.exs | 104 ++++++++++++- .../v2/test/router/tasks/trigger_test.exs | 108 +++++++++++++- .../v2/test/router/tasks/update_test.exs | 105 ++++++++++++- public-api/v2/test/support/stubs/scheduler.ex | 12 +- 33 files changed, 1008 insertions(+), 81 deletions(-) create mode 100644 public-api/v2/test/internal_clients/schedulers_client/request_formatter_test.exs diff --git a/public-api/v1alpha/Makefile b/public-api/v1alpha/Makefile index 3047dccf9..7bb68f536 100644 --- a/public-api/v1alpha/Makefile +++ b/public-api/v1alpha/Makefile @@ -4,7 +4,7 @@ include ../../Makefile DOCKER_BUILD_PATH=../.. INTERNAL_API_BRANCH?=master -TMP_INTERNAL_REPO_DIR?=/tmp/internal_api +TMP_REPO_DIR ?= /tmp/internal_api RELATIVE_INTERNAL_PB_OUTPUT_DIR=lib/internal_api RT_PROTOC_IMG_VSN=1.15.4-3.20.1-0.12.0 APP_NAME=plumber-public @@ -32,7 +32,6 @@ PROJECTHUB_API_GRPC_URL?=127.0.0.1:50052 LOG_LEVEL?=debug API_VERSION?=v1alpha ON_PREM?="false" -TMP_REPO_DIR ?= /tmp/internal_api CONTAINER_ENV_VARS= \ -e IN_DOCKER=$(IN_DOCKER) \ diff --git a/public-api/v1alpha/lib/internal_api/artifacthub.pb.ex b/public-api/v1alpha/lib/internal_api/artifacthub.pb.ex index d952ea7c3..46c16957b 100644 --- a/public-api/v1alpha/lib/internal_api/artifacthub.pb.ex +++ b/public-api/v1alpha/lib/internal_api/artifacthub.pb.ex @@ -388,12 +388,14 @@ defmodule InternalApi.Artifacthub.ListItem do @type t :: %__MODULE__{ name: String.t(), - is_directory: boolean + is_directory: boolean, + size: integer } - defstruct [:name, :is_directory] + defstruct [:name, :is_directory, :size] field(:name, 1, type: :string) field(:is_directory, 2, type: :bool) + field(:size, 3, type: :int64) end defmodule InternalApi.Artifacthub.Artifact do diff --git a/public-api/v1alpha/lib/internal_api/periodic_scheduler.pb.ex b/public-api/v1alpha/lib/internal_api/periodic_scheduler.pb.ex index 2422e3081..2bfcbd345 100644 --- a/public-api/v1alpha/lib/internal_api/periodic_scheduler.pb.ex +++ b/public-api/v1alpha/lib/internal_api/periodic_scheduler.pb.ex @@ -41,7 +41,7 @@ defmodule InternalApi.PeriodicScheduler.PersistRequest do organization_id: String.t(), project_name: String.t(), requester_id: String.t(), - branch: String.t(), + reference: String.t(), pipeline_file: String.t(), at: String.t(), parameters: [InternalApi.PeriodicScheduler.Periodic.Parameter.t()], @@ -56,7 +56,7 @@ defmodule InternalApi.PeriodicScheduler.PersistRequest do :organization_id, :project_name, :requester_id, - :branch, + :reference, :pipeline_file, :at, :parameters, @@ -71,7 +71,7 @@ defmodule InternalApi.PeriodicScheduler.PersistRequest do field(:organization_id, 6, type: :string) field(:project_name, 7, type: :string) field(:requester_id, 8, type: :string) - field(:branch, 9, type: :string) + field(:reference, 9, type: :string) field(:pipeline_file, 10, type: :string) field(:at, 11, type: :string) field(:parameters, 12, repeated: true, type: InternalApi.PeriodicScheduler.Periodic.Parameter) @@ -160,15 +160,15 @@ defmodule InternalApi.PeriodicScheduler.RunNowRequest do @type t :: %__MODULE__{ id: String.t(), requester: String.t(), - branch: String.t(), + reference: String.t(), pipeline_file: String.t(), parameter_values: [InternalApi.PeriodicScheduler.ParameterValue.t()] } - defstruct [:id, :requester, :branch, :pipeline_file, :parameter_values] + defstruct [:id, :requester, :reference, :pipeline_file, :parameter_values] field(:id, 1, type: :string) field(:requester, 2, type: :string) - field(:branch, 3, type: :string) + field(:reference, 3, type: :string) field(:pipeline_file, 4, type: :string) field(:parameter_values, 5, repeated: true, type: InternalApi.PeriodicScheduler.ParameterValue) end @@ -227,7 +227,7 @@ defmodule InternalApi.PeriodicScheduler.Periodic do id: String.t(), name: String.t(), project_id: String.t(), - branch: String.t(), + reference: String.t(), at: String.t(), pipeline_file: String.t(), requester_id: String.t(), @@ -246,7 +246,7 @@ defmodule InternalApi.PeriodicScheduler.Periodic do :id, :name, :project_id, - :branch, + :reference, :at, :pipeline_file, :requester_id, @@ -265,7 +265,7 @@ defmodule InternalApi.PeriodicScheduler.Periodic do field(:id, 1, type: :string) field(:name, 2, type: :string) field(:project_id, 3, type: :string) - field(:branch, 4, type: :string) + field(:reference, 4, type: :string) field(:at, 5, type: :string) field(:pipeline_file, 6, type: :string) field(:requester_id, 7, type: :string) @@ -308,7 +308,7 @@ defmodule InternalApi.PeriodicScheduler.Trigger do @type t :: %__MODULE__{ triggered_at: Google.Protobuf.Timestamp.t(), project_id: String.t(), - branch: String.t(), + reference: String.t(), pipeline_file: String.t(), scheduling_status: String.t(), scheduled_workflow_id: String.t(), @@ -321,7 +321,7 @@ defmodule InternalApi.PeriodicScheduler.Trigger do defstruct [ :triggered_at, :project_id, - :branch, + :reference, :pipeline_file, :scheduling_status, :scheduled_workflow_id, @@ -334,7 +334,7 @@ defmodule InternalApi.PeriodicScheduler.Trigger do field(:triggered_at, 1, type: Google.Protobuf.Timestamp) field(:project_id, 2, type: :string) - field(:branch, 3, type: :string) + field(:reference, 3, type: :string) field(:pipeline_file, 4, type: :string) field(:scheduling_status, 5, type: :string) field(:scheduled_workflow_id, 6, type: :string) diff --git a/public-api/v1alpha/lib/internal_api/rbac.pb.ex b/public-api/v1alpha/lib/internal_api/rbac.pb.ex index 29fb21c76..5e1c7a6bf 100644 --- a/public-api/v1alpha/lib/internal_api/rbac.pb.ex +++ b/public-api/v1alpha/lib/internal_api/rbac.pb.ex @@ -514,6 +514,7 @@ defmodule InternalApi.RBAC.SubjectType do field(:USER, 0) field(:GROUP, 1) + field(:SERVICE_ACCOUNT, 2) end defmodule InternalApi.RBAC.Scope do diff --git a/public-api/v1alpha/lib/internal_api/user.pb.ex b/public-api/v1alpha/lib/internal_api/user.pb.ex index 5fc2ba9a3..0389664df 100644 --- a/public-api/v1alpha/lib/internal_api/user.pb.ex +++ b/public-api/v1alpha/lib/internal_api/user.pb.ex @@ -534,6 +534,7 @@ defmodule InternalApi.User.User.CreationSource do field(:NOT_SET, 0) field(:OKTA, 1) + field(:SERVICE_ACCOUNT, 2) end defmodule InternalApi.User.UserCreated do diff --git a/public-api/v1alpha/lib/pipelines_api/periodic_scheduler_client/request_fromatter.ex b/public-api/v1alpha/lib/pipelines_api/periodic_scheduler_client/request_fromatter.ex index 7766784f7..d9f3cd566 100644 --- a/public-api/v1alpha/lib/pipelines_api/periodic_scheduler_client/request_fromatter.ex +++ b/public-api/v1alpha/lib/pipelines_api/periodic_scheduler_client/request_fromatter.ex @@ -98,15 +98,18 @@ defmodule PipelinesAPI.PeriodicSchedulerClient.RequestFormatter do # RunNow def form_run_now_request(params, conn) when is_map(params) do - %{ - id: params |> Map.get("periodic_id", ""), - requester: Conn.get_req_header(conn, "x-semaphore-user-id") |> Enum.at(0, ""), - branch: params |> Map.get("branch", ""), - pipeline_file: params |> Map.get("pipeline_file", ""), - parameter_values: params |> Map.get("parameters", %{}) |> to_param_values() - } - |> RunNowRequest.new() - |> ToTuple.ok() + reference = build_reference(params) + + req = + %{ + id: params |> Map.get("periodic_id", ""), + requester: Conn.get_req_header(conn, "x-semaphore-user-id") |> Enum.at(0, ""), + reference: reference, + pipeline_file: params |> Map.get("pipeline_file", ""), + parameter_values: params |> Map.get("parameters", %{}) |> to_param_values() + } + |> RunNowRequest.new() + |> ToTuple.ok() catch error -> error end @@ -121,6 +124,24 @@ defmodule PipelinesAPI.PeriodicSchedulerClient.RequestFormatter do |> throw() end + defp build_reference(params) do + case params |> Map.get("reference") do + reference_map when is_map(reference_map) -> + reference_type = Map.get(reference_map, "type", "BRANCH") + reference_name = Map.get(reference_map, "name", "") + + case String.upcase(reference_type) do + "TAG" -> "refs/tags/#{reference_name}" + _ -> "refs/heads/#{reference_name}" + end + + _ -> + # Fall back to legacy branch parameter for backward compatibility + branch_name = params |> Map.get("branch", "") + "refs/heads/#{branch_name}" + end + end + defp to_param_values(parameters) do Enum.into(parameters, [], &ParameterValue.new(name: elem(&1, 0), value: elem(&1, 1))) end diff --git a/public-api/v1alpha/lib/pipelines_api/periodic_scheduler_client/response_formatter.ex b/public-api/v1alpha/lib/pipelines_api/periodic_scheduler_client/response_formatter.ex index b5589bf1a..dd6c199c6 100644 --- a/public-api/v1alpha/lib/pipelines_api/periodic_scheduler_client/response_formatter.ex +++ b/public-api/v1alpha/lib/pipelines_api/periodic_scheduler_client/response_formatter.ex @@ -56,7 +56,11 @@ defmodule PipelinesAPI.PeriodicSchedulerClient.ResponseFormatter do with tf_map <- %{Timestamp => {__MODULE__, :timestamp_to_datetime_string}}, {:ok, response} <- Proto.to_map(proto_response, transformations: tf_map), :OK <- response.status.code do - {:ok, %{schedule: response.periodic, triggers: response.triggers}} + {:ok, + %{ + schedule: rename_reference_to_branch(response.periodic), + triggers: rename_reference_to_branch(response.triggers) + }} else :INVALID_ARGUMENT -> proto_response.status |> Map.get(:message) |> ToTuple.user_error() @@ -98,7 +102,7 @@ defmodule PipelinesAPI.PeriodicSchedulerClient.ResponseFormatter do {:ok, response} <- Proto.to_map(proto_response, transformations: tf_map), :OK <- response.status.code do response - |> Map.put(:entries, response.periodics) + |> Map.put(:entries, rename_reference_to_branch(response.periodics)) |> Map.drop([:periodics, :status]) |> to_page() |> ToTuple.ok() @@ -152,6 +156,20 @@ defmodule PipelinesAPI.PeriodicSchedulerClient.ResponseFormatter do DateTime.to_string(ts_date_time) end + defp rename_reference_to_branch(periodics) when is_list(periodics) do + Enum.map(periodics, &rename_reference_to_branch/1) + end + + defp rename_reference_to_branch(periodic) do + case reference_to_branch_field(periodic.reference) do + nil -> periodic + branch_name -> Map.put(periodic, :branch, branch_name) + end + end + + def reference_to_branch_field("refs/heads/" <> branch_name), do: branch_name + def reference_to_branch_field(_), do: nil + defp log_invalid_response(response, rpc_method) do response |> LT.error("PeriodicScheduler service responded to #{rpc_method} with :ok and invalid data:") diff --git a/public-api/v1alpha/lib/pipelines_api/schedules/run_now.ex b/public-api/v1alpha/lib/pipelines_api/schedules/run_now.ex index 997494490..8e1755ca5 100644 --- a/public-api/v1alpha/lib/pipelines_api/schedules/run_now.ex +++ b/public-api/v1alpha/lib/pipelines_api/schedules/run_now.ex @@ -1,6 +1,7 @@ defmodule PipelinesAPI.Schedules.RunNow do @moduledoc """ - Plug which serves for running given schedule with schedule definition + Plug which serves for running given schedule with schedule definition. + Supports both new reference format (reference.type/reference.name) and legacy branch parameter. """ use Plug.Builder @@ -15,12 +16,26 @@ defmodule PipelinesAPI.Schedules.RunNow do plug(:identify_path_param) plug(:get_project_id) plug(:authorize_run_now) + plug(:validate_and_normalize_params) plug(:run_now) + def validate_and_normalize_params(conn, _opts) do + case normalize_and_validate_reference_params(conn.params) do + {:ok, normalized_params} -> + %{conn | params: Map.merge(conn.params, normalized_params)} + + {:error, reason} -> + {:error, {:user, reason}} + |> RespCommon.respond(conn) + |> halt() + end + end + def run_now(conn, _opts) do Metrics.benchmark("PipelinesAPI.router", ["periodic_run_now"], fn -> conn.params |> PeriodicSchedulerClient.run_now(conn) + |> handle_run_now_response() |> RespCommon.respond(conn) end) end @@ -41,4 +56,88 @@ defmodule PipelinesAPI.Schedules.RunNow do params = conn.params |> Map.put(key, value) Map.put(conn, :params, params) end + + # Request validation and normalization + + defp normalize_and_validate_reference_params(params) do + case normalize_reference_params(params) do + {:ok, normalized_params} -> + validate_reference_params(normalized_params) + + {:error, reason} -> + {:error, reason} + end + end + + defp normalize_reference_params(params) do + cond do + # New reference format + params["reference"] -> + {:ok, params} + + # Legacy branch format - convert to reference + params["branch"] -> + reference = %{ + "type" => "BRANCH", + "name" => params["branch"] + } + + updated_params = + params + |> Map.put("reference", reference) + |> Map.delete("branch") + + {:ok, updated_params} + + # No reference information provided + true -> + {:error, "Either 'reference' or 'branch' parameter is required"} + end + end + + defp validate_reference_params(params = %{"reference" => reference}) do + case reference do + %{"type" => type, "name" => name} when type in ["BRANCH", "TAG"] and is_binary(name) -> + if String.trim(name) != "" do + {:ok, params} + else + {:error, "Reference name cannot be empty"} + end + + %{"type" => type} when type not in ["BRANCH", "TAG"] -> + {:error, "Reference type must be 'BRANCH' or 'TAG'"} + + _ -> + {:error, "Reference must contain 'type' and 'name' fields"} + end + end + + # Enhanced error handling + + defp handle_run_now_response({:error, {:user, message}}) when is_binary(message) do + cond do + String.contains?(message, "refs/heads/") -> + branch_name = extract_reference_name(message, "refs/heads/") + {:error, {:user, "Branch '#{branch_name}' does not exist in the repository"}} + + String.contains?(message, "refs/tags/") -> + tag_name = extract_reference_name(message, "refs/tags/") + {:error, {:user, "Tag '#{tag_name}' does not exist in the repository"}} + + String.contains?(message, "Project assigned to periodic was not found") -> + {:error, {:user, "Project not found or access denied"}} + + true -> + {:error, {:user, message}} + end + end + + defp handle_run_now_response(response), do: response + + defp extract_reference_name(message, prefix) do + message + |> String.split(prefix) + |> List.last() + |> String.trim_trailing(".") + end end diff --git a/public-api/v1alpha/test/periodic_scheduler_client/request_formatter_test.exs b/public-api/v1alpha/test/periodic_scheduler_client/request_formatter_test.exs index c870e2c3f..56b9f00f1 100644 --- a/public-api/v1alpha/test/periodic_scheduler_client/request_formatter_test.exs +++ b/public-api/v1alpha/test/periodic_scheduler_client/request_formatter_test.exs @@ -128,7 +128,7 @@ defmodule PipelinesAPI.PeriodicSchedulerClient.RequestFormatter.Test do RequestFormatter.form_run_now_request(nil, conn) end - test "form_run_now_request() returns {:ok, request} when called with map with all params" do + test "form_run_now_request() returns {:ok, request} when called with legacy branch parameter" do conn = create_conn(:run_now) params = %{ @@ -142,7 +142,7 @@ defmodule PipelinesAPI.PeriodicSchedulerClient.RequestFormatter.Test do assert request.id == params["periodic_id"] assert request.requester == "test_user" - assert request.branch == "master" + assert request.reference == "refs/heads/master" assert request.pipeline_file == ".semaphore/semaphore.yml" assert request.parameter_values == [ @@ -151,6 +151,54 @@ defmodule PipelinesAPI.PeriodicSchedulerClient.RequestFormatter.Test do ] end + test "form_run_now_request() returns {:ok, request} when called with new reference format - BRANCH" do + conn = create_conn(:run_now) + + params = %{ + "reference" => %{ + "type" => "BRANCH", + "name" => "feature/new-feature" + }, + "pipeline_file" => ".semaphore/deploy.yml", + "periodic_id" => UUID.uuid4(), + "parameters" => %{"ENV" => "staging"} + } + + assert {:ok, request = %RunNowRequest{}} = RequestFormatter.form_run_now_request(params, conn) + + assert request.id == params["periodic_id"] + assert request.requester == "test_user" + # BRANCH + assert request.reference == "refs/heads/feature/new-feature" + assert request.pipeline_file == ".semaphore/deploy.yml" + + assert request.parameter_values == [ + %ParameterValue{name: "ENV", value: "staging"} + ] + end + + test "form_run_now_request() returns {:ok, request} when called with new reference format - TAG" do + conn = create_conn(:run_now) + + params = %{ + "reference" => %{ + "type" => "TAG", + "name" => "v1.2.0" + }, + "pipeline_file" => ".semaphore/release.yml", + "periodic_id" => UUID.uuid4() + } + + assert {:ok, request = %RunNowRequest{}} = RequestFormatter.form_run_now_request(params, conn) + + assert request.id == params["periodic_id"] + assert request.requester == "test_user" + # TAG + assert request.reference == "refs/tags/v1.2.0" + assert request.pipeline_file == ".semaphore/release.yml" + assert request.parameter_values == [] + end + test "form_run_now_request() returns {:ok, request} when called with map with missing params" do conn = create_conn(:run_now) params = %{"periodic_id" => UUID.uuid4()} @@ -158,7 +206,7 @@ defmodule PipelinesAPI.PeriodicSchedulerClient.RequestFormatter.Test do assert request.id == params["periodic_id"] assert request.requester == "test_user" - assert request.branch == "" + assert request.reference == "refs/heads/" assert request.pipeline_file == "" assert request.parameter_values == [] end diff --git a/public-api/v1alpha/test/router/schedules/run_now_test.exs b/public-api/v1alpha/test/router/schedules/run_now_test.exs index 3a402cc91..bdd12d351 100644 --- a/public-api/v1alpha/test/router/schedules/run_now_test.exs +++ b/public-api/v1alpha/test/router/schedules/run_now_test.exs @@ -67,7 +67,7 @@ defmodule PipelinesAPI.Schedules.RunNow.Test do post_run_now(params, scheduler.id, 400, false) end - test "POST /schedules/:id/run_now - success when running a schedule and PeriodicSch returns :OK" do + test "POST /schedules/:id/run_now - success with legacy branch parameter" do org = Support.Stubs.Organization.create_default() user = Support.Stubs.User.create_default() project = Support.Stubs.Project.create(org, user) @@ -88,6 +88,98 @@ defmodule PipelinesAPI.Schedules.RunNow.Test do assert Support.Stubs.DB.find_all_by(:triggers, :periodic_id, scheduler.id) |> Enum.count() > 0 end + test "POST /schedules/:id/run_now - success with new reference format - BRANCH" do + org = Support.Stubs.Organization.create_default() + user = Support.Stubs.User.create_default() + project = Support.Stubs.Project.create(org, user) + scheduler = Support.Stubs.Scheduler.create(project.id, user.id) + + params = %{ + "reference" => %{ + "type" => "BRANCH", + "name" => "feature/deployment" + }, + "pipeline_file" => ".semaphore/deploy.yml", + "parameters" => %{ + "ENV" => "staging" + } + } + + assert %{"workflow_id" => workflow_id} = post_run_now(params, scheduler.id, 200) + assert {:ok, _} = UUID.info(workflow_id) + + assert Support.Stubs.DB.find_all_by(:triggers, :periodic_id, scheduler.id) |> Enum.count() > 0 + end + + test "POST /schedules/:id/run_now - success with new reference format - TAG" do + org = Support.Stubs.Organization.create_default() + user = Support.Stubs.User.create_default() + project = Support.Stubs.Project.create(org, user) + scheduler = Support.Stubs.Scheduler.create(project.id, user.id) + + params = %{ + "reference" => %{ + "type" => "TAG", + "name" => "v2.1.0" + }, + "pipeline_file" => ".semaphore/release.yml" + } + + assert %{"workflow_id" => workflow_id} = post_run_now(params, scheduler.id, 200) + assert {:ok, _} = UUID.info(workflow_id) + + assert Support.Stubs.DB.find_all_by(:triggers, :periodic_id, scheduler.id) |> Enum.count() > 0 + end + + test "POST /schedules/:id/run_now - fails with invalid reference type" do + org = Support.Stubs.Organization.create_default() + user = Support.Stubs.User.create_default() + project = Support.Stubs.Project.create(org, user) + scheduler = Support.Stubs.Scheduler.create(project.id, user.id) + + params = %{ + "reference" => %{ + "type" => "INVALID", + "name" => "main" + }, + "pipeline_file" => ".semaphore/semaphore.yml" + } + + assert "\"Reference type must be 'BRANCH' or 'TAG'\"" = + post_run_now(params, scheduler.id, 400, false) + end + + test "POST /schedules/:id/run_now - fails when both reference and branch are missing" do + org = Support.Stubs.Organization.create_default() + user = Support.Stubs.User.create_default() + project = Support.Stubs.Project.create(org, user) + scheduler = Support.Stubs.Scheduler.create(project.id, user.id) + + params = %{ + "pipeline_file" => ".semaphore/semaphore.yml" + } + + assert "\"Either 'reference' or 'branch' parameter is required\"" = + post_run_now(params, scheduler.id, 400, false) + end + + test "POST /schedules/:id/run_now - fails with empty reference name" do + org = Support.Stubs.Organization.create_default() + user = Support.Stubs.User.create_default() + project = Support.Stubs.Project.create(org, user) + scheduler = Support.Stubs.Scheduler.create(project.id, user.id) + + params = %{ + "reference" => %{ + "type" => "BRANCH", + "name" => " " + }, + "pipeline_file" => ".semaphore/semaphore.yml" + } + + assert "\"Reference name cannot be empty\"" = post_run_now(params, scheduler.id, 400, false) + end + def post_run_now(args, id, expected_status_code, decode \\ true) when is_map(args) do {:ok, response} = args |> Poison.encode!() |> post_schedules_request(id) diff --git a/public-api/v1alpha/test/support/stubs/scheduler.ex b/public-api/v1alpha/test/support/stubs/scheduler.ex index b69ac691d..12bf5b702 100644 --- a/public-api/v1alpha/test/support/stubs/scheduler.ex +++ b/public-api/v1alpha/test/support/stubs/scheduler.ex @@ -28,7 +28,7 @@ defmodule Support.Stubs.Scheduler do id: UUID.uuid4(), name: "Scheduler", project_id: project_id, - branch: "master", + reference: "refs/heads/master", at: "* * * * *", pipeline_file: ".semaphore/semaphore.yml", requester_id: user_id, @@ -61,7 +61,7 @@ defmodule Support.Stubs.Scheduler do defaults = [ triggered_at: Time.now(), project_id: periodic.project_id, - branch: params[:branch] || periodic.branch, + reference: params[:reference] || periodic.reference, pipeline_file: params[:pipeline_file] || periodic.pipeline_file, parameter_values: params[:parameter_values] || [], scheduling_status: "passed", @@ -123,7 +123,7 @@ defmodule Support.Stubs.Scheduler do end def run_now(req, _) do - if req.branch == "RESOURCE_EXHAUSTED" do + if req.reference == "refs/heads/RESOURCE_EXHAUSTED" do InternalApi.PeriodicScheduler.RunNowResponse.new( status: InternalApi.Status.new( @@ -149,7 +149,7 @@ defmodule Support.Stubs.Scheduler do scheduler.api_model, UUID.uuid4(), req.requester, - branch: req.branch, + reference: req.reference, pipeline_file: req.pipeline_file, parameter_values: req.parameter_values ) @@ -240,11 +240,13 @@ defmodule Support.Stubs.Scheduler do metadata = data["metadata"] spec = data["spec"] + reference = if data["apiVersion"] == "v1.2", do: spec["reference"], else: spec["branch"] + InternalApi.PeriodicScheduler.Periodic.new( id: metadata["id"], name: metadata["name"], project_id: "", - branch: spec["branch"], + reference: reference, at: spec["at"], pipeline_file: spec["pipeline_file"], requester_id: req.requester_id, diff --git a/public-api/v2/docker-compose.yml b/public-api/v2/docker-compose.yml index ce23374a8..a8df72e9c 100644 --- a/public-api/v2/docker-compose.yml +++ b/public-api/v2/docker-compose.yml @@ -37,7 +37,7 @@ services: LOG_LEVEL: "debug" volumes: - - .:/app + - .:/app/api - /.elixir_ls depends_on: diff --git a/public-api/v2/lib/internal_api/artifacthub.pb.ex b/public-api/v2/lib/internal_api/artifacthub.pb.ex index 340b6241b..d47c49fd4 100644 --- a/public-api/v2/lib/internal_api/artifacthub.pb.ex +++ b/public-api/v2/lib/internal_api/artifacthub.pb.ex @@ -289,6 +289,7 @@ defmodule InternalApi.Artifacthub.ListItem do field(:name, 1, type: :string) field(:is_directory, 2, type: :bool, json_name: "isDirectory") + field(:size, 3, type: :int64) end defmodule InternalApi.Artifacthub.Artifact do diff --git a/public-api/v2/lib/internal_api/audit.pb.ex b/public-api/v2/lib/internal_api/audit.pb.ex index be6266fca..1cc4aa618 100644 --- a/public-api/v2/lib/internal_api/audit.pb.ex +++ b/public-api/v2/lib/internal_api/audit.pb.ex @@ -67,6 +67,7 @@ defmodule InternalApi.Audit.Event.Resource do field(:Okta, 17) field(:FlakyTests, 18) field(:RBACRole, 19) + field(:ServiceAccount, 20) end defmodule InternalApi.Audit.Event.Operation do diff --git a/public-api/v2/lib/internal_api/notifications.pb.ex b/public-api/v2/lib/internal_api/notifications.pb.ex index 0b429f989..8e1ff2e7a 100644 --- a/public-api/v2/lib/internal_api/notifications.pb.ex +++ b/public-api/v2/lib/internal_api/notifications.pb.ex @@ -152,6 +152,7 @@ defmodule InternalApi.Notifications.Notification do field(:rules, 5, repeated: true, type: InternalApi.Notifications.Notification.Rule) field(:status, 6, type: InternalApi.Notifications.Notification.Status) field(:org_id, 7, type: :string, json_name: "orgId") + field(:creator_id, 8, type: :string, json_name: "creatorId") end defmodule InternalApi.Notifications.ListRequest do diff --git a/public-api/v2/lib/internal_api/periodic_scheduler.pb.ex b/public-api/v2/lib/internal_api/periodic_scheduler.pb.ex index fba9a63f6..a117bd9f3 100644 --- a/public-api/v2/lib/internal_api/periodic_scheduler.pb.ex +++ b/public-api/v2/lib/internal_api/periodic_scheduler.pb.ex @@ -68,7 +68,7 @@ defmodule InternalApi.PeriodicScheduler.PersistRequest do field(:organization_id, 6, type: :string, json_name: "organizationId") field(:project_name, 7, type: :string, json_name: "projectName") field(:requester_id, 8, type: :string, json_name: "requesterId") - field(:branch, 9, type: :string) + field(:reference, 9, type: :string) field(:pipeline_file, 10, type: :string, json_name: "pipelineFile") field(:at, 11, type: :string) field(:parameters, 12, repeated: true, type: InternalApi.PeriodicScheduler.Periodic.Parameter) @@ -125,7 +125,7 @@ defmodule InternalApi.PeriodicScheduler.RunNowRequest do field(:id, 1, type: :string) field(:requester, 2, type: :string) - field(:branch, 3, type: :string) + field(:reference, 3, type: :string) field(:pipeline_file, 4, type: :string, json_name: "pipelineFile") field(:parameter_values, 5, @@ -184,7 +184,7 @@ defmodule InternalApi.PeriodicScheduler.Periodic do field(:id, 1, type: :string) field(:name, 2, type: :string) field(:project_id, 3, type: :string, json_name: "projectId") - field(:branch, 4, type: :string) + field(:reference, 4, type: :string) field(:at, 5, type: :string) field(:pipeline_file, 6, type: :string, json_name: "pipelineFile") field(:requester_id, 7, type: :string, json_name: "requesterId") @@ -207,7 +207,7 @@ defmodule InternalApi.PeriodicScheduler.Trigger do field(:triggered_at, 1, type: Google.Protobuf.Timestamp, json_name: "triggeredAt") field(:project_id, 2, type: :string, json_name: "projectId") - field(:branch, 3, type: :string) + field(:reference, 3, type: :string) field(:pipeline_file, 4, type: :string, json_name: "pipelineFile") field(:scheduling_status, 5, type: :string, json_name: "schedulingStatus") field(:scheduled_workflow_id, 6, type: :string, json_name: "scheduledWorkflowId") diff --git a/public-api/v2/lib/internal_api/plumber_w_f.workflow.pb.ex b/public-api/v2/lib/internal_api/plumber_w_f.workflow.pb.ex index 6bb59b4ae..b735a1d1a 100644 --- a/public-api/v2/lib/internal_api/plumber_w_f.workflow.pb.ex +++ b/public-api/v2/lib/internal_api/plumber_w_f.workflow.pb.ex @@ -144,6 +144,9 @@ defmodule InternalApi.PlumberWF.ScheduleRequest do type: InternalApi.PlumberWF.ScheduleRequest.EnvVar, json_name: "envVars" ) + + field(:start_in_conceived_state, 18, type: :bool, json_name: "startInConceivedState") + field(:git_reference, 19, type: :string, json_name: "gitReference") end defmodule InternalApi.PlumberWF.ScheduleResponse do diff --git a/public-api/v2/lib/internal_api/rbac.pb.ex b/public-api/v2/lib/internal_api/rbac.pb.ex index fed11bb6f..9724c1456 100644 --- a/public-api/v2/lib/internal_api/rbac.pb.ex +++ b/public-api/v2/lib/internal_api/rbac.pb.ex @@ -5,6 +5,7 @@ defmodule InternalApi.RBAC.SubjectType do field(:USER, 0) field(:GROUP, 1) + field(:SERVICE_ACCOUNT, 2) end defmodule InternalApi.RBAC.Scope do diff --git a/public-api/v2/lib/internal_api/user.pb.ex b/public-api/v2/lib/internal_api/user.pb.ex index fdc19bcbc..c0574fcb8 100644 --- a/public-api/v2/lib/internal_api/user.pb.ex +++ b/public-api/v2/lib/internal_api/user.pb.ex @@ -56,6 +56,7 @@ defmodule InternalApi.User.User.CreationSource do field(:NOT_SET, 0) field(:OKTA, 1) + field(:SERVICE_ACCOUNT, 2) end defmodule InternalApi.User.ListFavoritesRequest do diff --git a/public-api/v2/lib/internal_clients/schedulers_client/request_formatter.ex b/public-api/v2/lib/internal_clients/schedulers_client/request_formatter.ex index 241649167..c1a751662 100644 --- a/public-api/v2/lib/internal_clients/schedulers_client/request_formatter.ex +++ b/public-api/v2/lib/internal_clients/schedulers_client/request_formatter.ex @@ -56,7 +56,7 @@ defmodule InternalClients.Schedulers.RequestFormatter do state: state_from_params(params), organization_id: from_params(params, :organization_id), project_id: from_params(params, :project_id), - branch: from_params!(params, :branch), + reference: build_reference(params), pipeline_file: from_params!(params, :pipeline_file), requester_id: from_params!(params, :requester_id), at: from_params(params, :cron_schedule, ""), @@ -79,11 +79,13 @@ defmodule InternalClients.Schedulers.RequestFormatter do end def form_request({API.RunNowRequest, params}) do + reference = build_reference(params) + {:ok, %API.RunNowRequest{ id: from_params!(params, :task_id), requester: from_params!(params, :requester_id), - branch: from_params(params, :branch), + reference: reference, pipeline_file: from_params(params, :pipeline_file), parameter_values: from_params(params, :parameters) }} @@ -109,6 +111,22 @@ defmodule InternalClients.Schedulers.RequestFormatter do end end + defp build_reference(params) do + case from_params(params, :reference) do + reference_map when is_map(reference_map) -> + reference_type = Map.get(reference_map, :type) || Map.get(reference_map, "type", "branch") + reference_name = Map.get(reference_map, :name) || Map.get(reference_map, "name", "") + + case reference_type do + "tag" -> "refs/tags/#{reference_name}" + _ -> "refs/heads/#{reference_name}" + end + + _ -> + "" + end + end + defp recurring_from_params(params) do from_params(params, :cron_schedule, "") != "" end diff --git a/public-api/v2/lib/internal_clients/schedulers_client/response_formatter.ex b/public-api/v2/lib/internal_clients/schedulers_client/response_formatter.ex index 5e34ea65f..396dec461 100644 --- a/public-api/v2/lib/internal_clients/schedulers_client/response_formatter.ex +++ b/public-api/v2/lib/internal_clients/schedulers_client/response_formatter.ex @@ -93,7 +93,7 @@ defmodule InternalClients.Schedulers.ResponseFormatter do %{ name: periodic.name, description: periodic.description, - branch: periodic.branch, + reference: reference_from_pb(periodic.reference), pipeline_file: periodic.pipeline_file, cron_schedule: periodic.at, parameters: Enum.into(periodic.parameters, [], ¶meter_from_pb/1) @@ -112,7 +112,7 @@ defmodule InternalClients.Schedulers.ResponseFormatter do status: trigger.scheduling_status |> String.upcase() }, spec: %{ - branch: trigger.branch, + reference: reference_from_pb(trigger.reference), pipeline_file: trigger.pipeline_file, parameters: Enum.into(trigger.parameter_values, [], ¶meter_value_from_pb/1) } @@ -138,6 +138,23 @@ defmodule InternalClients.Schedulers.ResponseFormatter do } end + defp reference_from_pb(reference) when is_binary(reference) do + cond do + String.starts_with?(reference, "refs/heads/") -> + name = String.replace_prefix(reference, "refs/heads/", "") + %{"type" => "branch", "name" => name} + + String.starts_with?(reference, "refs/tags/") -> + name = String.replace_prefix(reference, "refs/tags/", "") + %{"type" => "tag", "name" => name} + + true -> + %{"type" => "branch", "name" => reference} + end + end + + defp reference_from_pb(_), do: %{"type" => "branch", "name" => ""} + defp user_from_id(nil), do: nil defp user_from_id(""), do: nil defp user_from_id(uuid), do: User.from_id(uuid) diff --git a/public-api/v2/lib/public_api/handlers/tasks/create.ex b/public-api/v2/lib/public_api/handlers/tasks/create.ex index fb5af8782..29797f0f8 100644 --- a/public-api/v2/lib/public_api/handlers/tasks/create.ex +++ b/public-api/v2/lib/public_api/handlers/tasks/create.ex @@ -71,6 +71,7 @@ defmodule PublicAPI.Handlers.Tasks.Create do conn = super(conn, opts) conn.body_params[:spec] + |> Map.from_struct() |> Map.put(:organization_id, conn.assigns[:organization_id]) |> Map.put(:requester_id, conn.assigns[:user_id]) |> Map.put(:project_id, conn.assigns[:project_id]) diff --git a/public-api/v2/lib/public_api/handlers/tasks/replace.ex b/public-api/v2/lib/public_api/handlers/tasks/replace.ex index 47c10e52a..088c8a0b1 100644 --- a/public-api/v2/lib/public_api/handlers/tasks/replace.ex +++ b/public-api/v2/lib/public_api/handlers/tasks/replace.ex @@ -86,6 +86,7 @@ defmodule PublicAPI.Handlers.Tasks.Replace do def replace(conn, _opts) do conn.body_params[:spec] + |> Map.from_struct() |> Map.put(:requester_id, conn.assigns[:user_id]) |> Map.put(:task_id, conn.params[:task_id]) |> Client.persist() diff --git a/public-api/v2/lib/public_api/handlers/tasks/trigger.ex b/public-api/v2/lib/public_api/handlers/tasks/trigger.ex index dcd700940..8eac981a3 100644 --- a/public-api/v2/lib/public_api/handlers/tasks/trigger.ex +++ b/public-api/v2/lib/public_api/handlers/tasks/trigger.ex @@ -85,6 +85,7 @@ defmodule PublicAPI.Handlers.Tasks.Trigger do def trigger(conn, _opts) do conn.body_params[:spec] + |> Map.from_struct() |> Map.put(:requester_id, conn.assigns[:user_id]) |> Map.put(:task_id, conn.params[:task_id]) |> Client.run_now() diff --git a/public-api/v2/lib/public_api/schemas/tasks/spec.ex b/public-api/v2/lib/public_api/schemas/tasks/spec.ex index ac145ed5a..7c49e1fb3 100644 --- a/public-api/v2/lib/public_api/schemas/tasks/spec.ex +++ b/public-api/v2/lib/public_api/schemas/tasks/spec.ex @@ -11,12 +11,29 @@ defmodule PublicAPI.Schemas.Tasks.Spec do properties: %{ name: %Schema{type: :string, example: "Periodic task"}, description: %Schema{type: :string, example: "Periodic task description"}, - branch: %Schema{type: :string, example: "master"}, + reference: %Schema{ + type: :object, + description: "Git reference for the task", + properties: %{ + type: %Schema{ + type: :string, + enum: ["branch", "tag"], + description: "Type of git reference", + example: "branch" + }, + name: %Schema{ + type: :string, + description: "Name of the branch or tag", + example: "master" + } + }, + required: [:type, :name] + }, pipeline_file: %Schema{type: :string, example: "pipeline.yml"}, cron_schedule: %Schema{type: :string, example: "0 0 * * *"}, paused: %Schema{type: :boolean, example: false}, parameters: %Schema{type: :array, items: PublicAPI.Schemas.Tasks.Parameter.schema()} }, - required: [:name, :branch, :pipeline_file] + required: [:name, :reference, :pipeline_file] }) end diff --git a/public-api/v2/lib/public_api/schemas/tasks/triggers/spec.ex b/public-api/v2/lib/public_api/schemas/tasks/triggers/spec.ex index 022ae3c6c..70f578bd0 100644 --- a/public-api/v2/lib/public_api/schemas/tasks/triggers/spec.ex +++ b/public-api/v2/lib/public_api/schemas/tasks/triggers/spec.ex @@ -9,7 +9,30 @@ defmodule PublicAPI.Schemas.Tasks.Triggers.Spec do description: "Task Trigger specification", type: :object, properties: %{ - branch: %Schema{type: :string, example: "master"}, + reference: %Schema{ + type: :object, + description: "Git reference to trigger the task with", + properties: %{ + type: %Schema{ + type: :string, + enum: ["branch", "tag"], + description: "Type of git reference", + example: "branch" + }, + name: %Schema{ + type: :string, + description: "Name of the branch or tag", + example: "master" + } + }, + required: [:type, :name] + }, + branch: %Schema{ + type: :string, + example: "master", + description: "Legacy branch parameter - use reference.name instead", + deprecated: true + }, pipeline_file: %Schema{type: :string, example: ".semaphore/semaphore.yml"}, parameters: %Schema{ type: :array, diff --git a/public-api/v2/test/internal_clients/schedulers_client/request_formatter_test.exs b/public-api/v2/test/internal_clients/schedulers_client/request_formatter_test.exs new file mode 100644 index 000000000..17174a0c7 --- /dev/null +++ b/public-api/v2/test/internal_clients/schedulers_client/request_formatter_test.exs @@ -0,0 +1,139 @@ +defmodule InternalClients.Schedulers.RequestFormatterTest do + use ExUnit.Case, async: true + alias InternalClients.Schedulers.RequestFormatter + alias InternalApi.PeriodicScheduler, as: API + + describe "form_request/1 with RunNowRequest" do + test "formats request with missing reference defaults to empty" do + params = %{ + task_id: "task-123", + requester_id: "user-1", + pipeline_file: "semaphore.yml" + } + + {:ok, request} = RequestFormatter.form_request({API.RunNowRequest, params}) + + assert request.id == "task-123" + assert request.requester == "user-1" + assert request.pipeline_file == "semaphore.yml" + assert request.reference == "" + end + + test "formats request with new reference structure for branch" do + params = %{ + task_id: "task-123", + requester_id: "user-1", + reference: %{"type" => "branch", "name" => "feature-branch"}, + pipeline_file: "semaphore.yml" + } + + {:ok, request} = RequestFormatter.form_request({API.RunNowRequest, params}) + + assert request.id == "task-123" + assert request.requester == "user-1" + assert request.pipeline_file == "semaphore.yml" + assert request.reference == "refs/heads/feature-branch" + end + + test "formats request with new reference structure for tag" do + params = %{ + task_id: "task-123", + requester_id: "user-1", + reference: %{"type" => "tag", "name" => "v1.0.0"}, + pipeline_file: "semaphore.yml" + } + + {:ok, request} = RequestFormatter.form_request({API.RunNowRequest, params}) + + assert request.id == "task-123" + assert request.requester == "user-1" + assert request.pipeline_file == "semaphore.yml" + assert request.reference == "refs/tags/v1.0.0" + end + + test "defaults reference type to branch when type is unknown" do + params = %{ + task_id: "task-123", + requester_id: "user-1", + reference: %{"type" => "unknown-type", "name" => "some-ref"}, + pipeline_file: "semaphore.yml" + } + + {:ok, request} = RequestFormatter.form_request({API.RunNowRequest, params}) + + # defaults to branch for unknown types + assert request.reference == "refs/heads/some-ref" + end + end + + describe "form_request/1 with PersistRequest" do + test "formats request with new reference structure for branch" do + params = %{ + name: "My Task", + reference: %{"type" => "branch", "name" => "feature-branch"}, + pipeline_file: "semaphore.yml", + organization_id: "org-1", + project_id: "proj-1", + requester_id: "user-1" + } + + {:ok, request} = RequestFormatter.form_request({API.PersistRequest, params}) + + assert request.name == "My Task" + assert request.reference == "refs/heads/feature-branch" + assert request.pipeline_file == "semaphore.yml" + end + + test "formats request with new reference structure for tag" do + params = %{ + name: "My Task", + reference: %{"type" => "tag", "name" => "v1.0.0"}, + pipeline_file: "semaphore.yml", + organization_id: "org-1", + project_id: "proj-1", + requester_id: "user-1" + } + + {:ok, request} = RequestFormatter.form_request({API.PersistRequest, params}) + + assert request.name == "My Task" + assert request.reference == "refs/tags/v1.0.0" + assert request.pipeline_file == "semaphore.yml" + end + + test "defaults to empty reference when no reference is provided" do + params = %{ + name: "My Task", + pipeline_file: "semaphore.yml", + organization_id: "org-1", + project_id: "proj-1", + requester_id: "user-1" + } + + {:ok, request} = RequestFormatter.form_request({API.PersistRequest, params}) + + assert request.name == "My Task" + # empty reference default + assert request.reference == "" + assert request.pipeline_file == "semaphore.yml" + end + + test "defaults reference type to branch when type is unknown" do + params = %{ + name: "My Task", + reference: %{"type" => "unknown-type", "name" => "some-ref"}, + pipeline_file: "semaphore.yml", + organization_id: "org-1", + project_id: "proj-1", + requester_id: "user-1" + } + + {:ok, request} = RequestFormatter.form_request({API.PersistRequest, params}) + + assert request.name == "My Task" + # defaults to branch for unknown types + assert request.reference == "refs/heads/some-ref" + assert request.pipeline_file == "semaphore.yml" + end + end +end diff --git a/public-api/v2/test/internal_clients/schedulers_client_test.exs b/public-api/v2/test/internal_clients/schedulers_client_test.exs index f346db223..d3365d82e 100644 --- a/public-api/v2/test/internal_clients/schedulers_client_test.exs +++ b/public-api/v2/test/internal_clients/schedulers_client_test.exs @@ -185,18 +185,11 @@ defmodule InternalClients.SchedulersClientTest do assert {:error, {:user, "missing :name"}} = Client.persist(%{}) end - test "fails without branch" do - assert {:error, {:user, "missing :branch"}} = - Client.persist(%{ - name: "Scheduler" - }) - end - test "fails without pipeline_file" do assert {:error, {:user, "missing :pipeline_file"}} = Client.persist(%{ name: "Scheduler", - branch: "master" + reference: %{type: "branch", name: "master"} }) end @@ -220,7 +213,7 @@ defmodule InternalClients.SchedulersClientTest do }) assert response.spec.name == "Scheduler" - assert response.spec.branch == "master" + assert response.spec.reference == %{"name" => "", "type" => "branch"} assert response.spec.pipeline_file == "pipeline.yml" assert response.metadata.updated_by.id == "user-1" end @@ -245,7 +238,7 @@ defmodule InternalClients.SchedulersClientTest do }) assert response.spec.name == "Scheduler" - assert response.spec.branch == "master" + assert response.spec.reference == %{"name" => "", "type" => "branch"} assert response.spec.pipeline_file == "pipeline.yml" assert parameter = List.first(response.spec.parameters) @@ -362,7 +355,7 @@ defmodule InternalClients.SchedulersClientTest do assert {:ok, response} = Client.run_now(%{task_id: scheduler.id, requester_id: "user-1"}) assert {:ok, _} = UUID.info(response.metadata.workflow_id) assert response.metadata.status == "PASSED" - assert response.spec.branch == scheduler.branch + assert response.spec.reference == %{"name" => "master", "type" => "branch"} assert response.spec.pipeline_file == scheduler.pipeline_file assert response.metadata.triggered_by.id == "user-1" end @@ -374,7 +367,7 @@ defmodule InternalClients.SchedulersClientTest do Client.run_now(%{ task_id: scheduler.id, requester_id: "user-1", - branch: "develop", + reference: %{"name" => "develop", "type" => "branch"}, pipeline_file: "semaphore.yml", parameters: [ %{ @@ -386,7 +379,7 @@ defmodule InternalClients.SchedulersClientTest do assert {:ok, _} = UUID.info(response.metadata.workflow_id) assert response.metadata.status == "PASSED" - assert response.spec.branch == "develop" + assert response.spec.reference == %{"name" => "develop", "type" => "branch"} assert response.spec.pipeline_file == "semaphore.yml" assert response.metadata.triggered_by.id == "user-1" @@ -394,5 +387,41 @@ defmodule InternalClients.SchedulersClientTest do assert parameter_value.name == "param1" assert parameter_value.value == "value1" end + + test "runs existing task with new reference structure for branch" do + scheduler = Support.Stubs.Scheduler.create(UUID.uuid4(), UUID.uuid4(), name: "Scheduler") + + assert {:ok, response} = + Client.run_now(%{ + task_id: scheduler.id, + requester_id: "user-1", + reference: %{"type" => "branch", "name" => "feature-branch"}, + pipeline_file: "semaphore.yml" + }) + + assert {:ok, _} = UUID.info(response.metadata.workflow_id) + assert response.metadata.status == "PASSED" + assert response.spec.reference == %{"name" => "feature-branch", "type" => "branch"} + assert response.spec.pipeline_file == "semaphore.yml" + assert response.metadata.triggered_by.id == "user-1" + end + + test "runs existing task with new reference structure for tag" do + scheduler = Support.Stubs.Scheduler.create(UUID.uuid4(), UUID.uuid4(), name: "Scheduler") + + assert {:ok, response} = + Client.run_now(%{ + task_id: scheduler.id, + requester_id: "user-1", + reference: %{"type" => "tag", "name" => "v1.0.0"}, + pipeline_file: "semaphore.yml" + }) + + assert {:ok, _} = UUID.info(response.metadata.workflow_id) + assert response.metadata.status == "PASSED" + assert response.spec.reference == %{"name" => "v1.0.0", "type" => "tag"} + assert response.spec.pipeline_file == "semaphore.yml" + assert response.metadata.triggered_by.id == "user-1" + end end end diff --git a/public-api/v2/test/router/tasks/create_test.exs b/public-api/v2/test/router/tasks/create_test.exs index e80877312..7d8a8cfe3 100644 --- a/public-api/v2/test/router/tasks/create_test.exs +++ b/public-api/v2/test/router/tasks/create_test.exs @@ -18,7 +18,10 @@ defmodule Router.Tasks.CreateTest do spec: %{ name: "Task", description: "Task description", - branch: "master", + reference: %{ + type: "branch", + name: "master" + }, pipeline_file: "pipeline.yml", cron_schedule: "0 0 * * *", parameters: [ @@ -59,7 +62,10 @@ defmodule Router.Tasks.CreateTest do spec: %{ name: "Task", description: "Task description", - branch: "master", + reference: %{ + type: "branch", + name: "master" + }, pipeline_file: "pipeline.yml", cron_schedule: "0 0 * * *", parameters: [ @@ -81,13 +87,100 @@ defmodule Router.Tasks.CreateTest do assert {:ok, _} = UUID.info(task_id) end + test "POST /tasks - endpoint returns 200 when task is created with reference structure for branch", + ctx do + params = %{ + apiVersion: "v2", + kind: "Task", + spec: %{ + name: "Task with Reference", + description: "Task description", + reference: %{ + type: "branch", + name: "feature-branch" + }, + pipeline_file: "pipeline.yml", + cron_schedule: "0 0 * * *", + parameters: [ + %{ + name: "PARAM_NAME", + description: "Parameter description", + required: true, + default_value: "Default value", + options: ["Option 1", "Option 2"] + } + ] + } + } + + assert {:ok, %Tesla.Env{status: 200, body: %{"metadata" => %{"id" => task_id}}}} = + Tesla.post(http_client(ctx), "/projects/#{ctx.project_id}/tasks", params) + + assert Support.Stubs.DB.find(:schedulers, task_id) + assert {:ok, _} = UUID.info(task_id) + end + + test "POST /tasks - endpoint returns 200 when task is created with reference structure for tag", + ctx do + params = %{ + apiVersion: "v2", + kind: "Task", + spec: %{ + name: "Task with Tag", + description: "Task description", + reference: %{ + type: "tag", + name: "v1.0.0" + }, + pipeline_file: "pipeline.yml", + cron_schedule: "0 0 * * *", + parameters: [] + } + } + + assert {:ok, %Tesla.Env{status: 200, body: %{"metadata" => %{"id" => task_id}}}} = + Tesla.post(http_client(ctx), "/projects/#{ctx.project_id}/tasks", params) + + assert Support.Stubs.DB.find(:schedulers, task_id) + assert {:ok, _} = UUID.info(task_id) + end + + test "POST /tasks - endpoint returns 200 when task is created with reference structure", + ctx do + params = %{ + apiVersion: "v2", + kind: "Task", + spec: %{ + name: "Task with Reference", + description: "Task description", + reference: %{ + type: "branch", + name: "main" + }, + pipeline_file: "pipeline.yml", + cron_schedule: "0 0 * * *", + parameters: [] + } + } + + assert {:ok, %Tesla.Env{status: 200, body: %{"metadata" => %{"id" => task_id}}}} = + Tesla.post(http_client(ctx), "/projects/#{ctx.project_id}/tasks", params) + + task = Support.Stubs.DB.find(:schedulers, task_id) + assert task + assert task.api_model.reference == "refs/heads/main" + end + test "POST /tasks - endpoint returns 422 when request is invalid", ctx do params = %{ apiVersion: "v2", kind: "Task", spec: %{ name: "Test", - branch: "master", + reference: %{ + type: "branch", + name: "master" + }, pipeline_file: "pipeline.yml", cron_schedule: "0 0 * * *", parameters: [ diff --git a/public-api/v2/test/router/tasks/replace_test.exs b/public-api/v2/test/router/tasks/replace_test.exs index c25609e33..34aa6a64c 100644 --- a/public-api/v2/test/router/tasks/replace_test.exs +++ b/public-api/v2/test/router/tasks/replace_test.exs @@ -25,7 +25,10 @@ defmodule Router.Tasks.ReplaceTest do kind: "Task", spec: %{ name: "Task", - branch: "master", + reference: %{ + type: "branch", + name: "master" + }, pipeline_file: "pipeline.yml" } } @@ -51,7 +54,10 @@ defmodule Router.Tasks.ReplaceTest do spec: %{ name: "Task", description: "Task description", - branch: "master", + reference: %{ + type: "branch", + name: "master" + }, pipeline_file: "pipeline.yml", cron_schedule: "0 0 * * *", parameters: [ @@ -92,7 +98,10 @@ defmodule Router.Tasks.ReplaceTest do apiVersion: "v2", kind: "Task", spec: %{ - branch: "master", + reference: %{ + type: "branch", + name: "master" + }, pipeline_file: "pipeline.yml", cron_schedule: "0 0 * * *", parameters: [ @@ -119,6 +128,90 @@ defmodule Router.Tasks.ReplaceTest do assert %{"errors" => [%{"detail" => "Missing field: name"}]} = env.body end + test "PUT /projects/:project_id_or_name/tasks/:id - endpoint replaces with reference structure for branch", + ctx do + params = %{ + apiVersion: "v2", + kind: "Task", + spec: %{ + name: "Task with Reference", + reference: %{ + type: "branch", + name: "feature-branch" + }, + pipeline_file: "pipeline.yml" + } + } + + scheduler = Support.Stubs.Scheduler.create(ctx.project_id, ctx.user_id, name: "Scheduler") + + assert {:ok, %Tesla.Env{status: 200}} = + Tesla.put( + http_client(ctx), + "/projects/#{ctx.project_id}/tasks/" <> scheduler.id, + params + ) + + assert %{api_model: %{name: "Task with Reference", reference: "refs/heads/feature-branch"}} = + Support.Stubs.DB.find(:schedulers, scheduler.id) + end + + test "PUT /projects/:project_id_or_name/tasks/:id - endpoint replaces with reference structure for tag", + ctx do + params = %{ + apiVersion: "v2", + kind: "Task", + spec: %{ + name: "Task with Tag", + reference: %{ + type: "tag", + name: "v1.0.0" + }, + pipeline_file: "pipeline.yml" + } + } + + scheduler = Support.Stubs.Scheduler.create(ctx.project_id, ctx.user_id, name: "Scheduler") + + assert {:ok, %Tesla.Env{status: 200}} = + Tesla.put( + http_client(ctx), + "/projects/#{ctx.project_id}/tasks/" <> scheduler.id, + params + ) + + assert %{api_model: %{name: "Task with Tag", reference: "refs/tags/v1.0.0"}} = + Support.Stubs.DB.find(:schedulers, scheduler.id) + end + + test "PUT /projects/:project_id_or_name/tasks/:id - endpoint replaces task with reference structure", + ctx do + params = %{ + apiVersion: "v2", + kind: "Task", + spec: %{ + name: "Task with Reference", + reference: %{ + type: "tag", + name: "v1.0.0" + }, + pipeline_file: "pipeline.yml" + } + } + + scheduler = Support.Stubs.Scheduler.create(ctx.project_id, ctx.user_id, name: "Scheduler") + + assert {:ok, %Tesla.Env{status: 200}} = + Tesla.put( + http_client(ctx), + "/projects/#{ctx.project_id}/tasks/" <> scheduler.id, + params + ) + + assert %{api_model: %{name: "Task with Reference", reference: "refs/tags/v1.0.0"}} = + Support.Stubs.DB.find(:schedulers, scheduler.id) + end + test "PUT /projects/:project_id_or_name/tasks/:id - endpoint returns 404 when task is not owned by requester org", ctx do wrong_owner = UUID.uuid4() @@ -140,7 +233,10 @@ defmodule Router.Tasks.ReplaceTest do kind: "Task", spec: %{ name: "Task", - branch: "master", + reference: %{ + type: "branch", + name: "master" + }, pipeline_file: "pipeline.yml" } } diff --git a/public-api/v2/test/router/tasks/trigger_test.exs b/public-api/v2/test/router/tasks/trigger_test.exs index 609a175d4..892f505fd 100644 --- a/public-api/v2/test/router/tasks/trigger_test.exs +++ b/public-api/v2/test/router/tasks/trigger_test.exs @@ -39,7 +39,10 @@ defmodule Router.Tasks.TriggerTest do apiVersion: "v2", kind: "TaskTrigger", spec: %{ - branch: "master", + reference: %{ + type: "branch", + name: "master" + }, pipeline_file: ".semaphore/semaphore.yml", parameters: [ %{name: "FIRST_PARAM", value: "first_value"} @@ -79,13 +82,109 @@ defmodule Router.Tasks.TriggerTest do assert [_] = Support.Stubs.DB.find_all_by(:triggers, :periodic_id, scheduler.id) end + test "POST /projects/:project_id_or_name/tasks/:id/triggers - endpoint returns 200 with new reference structure for branch", + ctx do + params = %{ + apiVersion: "v2", + kind: "TaskTrigger", + spec: %{ + reference: %{ + type: "branch", + name: "feature-branch" + }, + pipeline_file: ".semaphore/semaphore.yml", + parameters: [ + %{name: "FIRST_PARAM", value: "first_value"} + ] + } + } + + scheduler = Support.Stubs.Scheduler.create(ctx.project_id, ctx.user_id) + + assert {:ok, %Tesla.Env{status: 200} = env} = + Tesla.post( + http_client(ctx), + "/projects/#{ctx.project_id}/tasks/#{scheduler.id}/triggers", + params + ) + + assert %{"metadata" => %{"workflow_id" => workflow_id}} = env.body + assert {:ok, _} = UUID.info(workflow_id) + + assert [_] = Support.Stubs.DB.find_all_by(:triggers, :periodic_id, scheduler.id) + end + + test "POST /projects/:project_id_or_name/tasks/:id/triggers - endpoint returns 200 with new reference structure for tag", + ctx do + params = %{ + apiVersion: "v2", + kind: "TaskTrigger", + spec: %{ + reference: %{ + type: "tag", + name: "v1.0.0" + }, + pipeline_file: ".semaphore/semaphore.yml", + parameters: [ + %{name: "FIRST_PARAM", value: "first_value"} + ] + } + } + + scheduler = Support.Stubs.Scheduler.create(ctx.project_id, ctx.user_id) + + assert {:ok, %Tesla.Env{status: 200} = env} = + Tesla.post( + http_client(ctx), + "/projects/#{ctx.project_id}/tasks/#{scheduler.id}/triggers", + params + ) + + assert %{"metadata" => %{"workflow_id" => workflow_id}} = env.body + assert {:ok, _} = UUID.info(workflow_id) + + assert [_] = Support.Stubs.DB.find_all_by(:triggers, :periodic_id, scheduler.id) + end + + test "POST /projects/:project_id_or_name/tasks/:id/triggers - endpoint returns 200 with reference structure", + ctx do + params = %{ + apiVersion: "v2", + kind: "TaskTrigger", + spec: %{ + reference: %{ + type: "branch", + name: "master" + }, + pipeline_file: ".semaphore/semaphore.yml" + } + } + + scheduler = Support.Stubs.Scheduler.create(ctx.project_id, ctx.user_id) + + assert {:ok, %Tesla.Env{status: 200} = env} = + Tesla.post( + http_client(ctx), + "/projects/#{ctx.project_id}/tasks/#{scheduler.id}/triggers", + params + ) + + assert %{"metadata" => %{"workflow_id" => workflow_id}} = env.body + assert {:ok, _} = UUID.info(workflow_id) + + assert [_] = Support.Stubs.DB.find_all_by(:triggers, :periodic_id, scheduler.id) + end + test "POST /projects/:project_id_or_name/tasks/:id/triggers - endpoint returns 404 when periodic does not exist", ctx do params = %{ apiVersion: "v2", kind: "TaskTrigger", spec: %{ - branch: "master", + reference: %{ + type: "branch", + name: "master" + }, pipeline_file: ".semaphore/semaphore.yml", parameters: [ %{name: "FIRST_PARAM", value: "first_value"} @@ -123,7 +222,10 @@ defmodule Router.Tasks.TriggerTest do apiVersion: "v2", kind: "TaskTrigger", spec: %{ - branch: "master", + reference: %{ + type: "branch", + name: "master" + }, pipeline_file: ".semaphore/semaphore.yml" } } diff --git a/public-api/v2/test/router/tasks/update_test.exs b/public-api/v2/test/router/tasks/update_test.exs index 5197a3dd5..1fd4b12b0 100644 --- a/public-api/v2/test/router/tasks/update_test.exs +++ b/public-api/v2/test/router/tasks/update_test.exs @@ -39,7 +39,10 @@ defmodule Router.Tasks.UpdateTest do apiVersion: "v2", kind: "Task", spec: %{ - branch: "develop", + reference: %{ + type: "branch", + name: "develop" + }, pipeline_file: "pipeline.yml", cron_schedule: "" } @@ -66,7 +69,7 @@ defmodule Router.Tasks.UpdateTest do assert %{ api_model: %{ id: ^scheduler_id, - branch: "develop", + reference: "refs/heads/develop", pipeline_file: "pipeline.yml", at: "", recurring: false @@ -117,6 +120,99 @@ defmodule Router.Tasks.UpdateTest do } = Support.Stubs.DB.find(:schedulers, scheduler.id) end + test "PATCH /projects/:project_id_or_name/tasks/:id - endpoint updates reference structure for branch", + ctx do + params = %{ + apiVersion: "v2", + kind: "Task", + spec: %{ + reference: %{ + type: "branch", + name: "feature-branch" + } + } + } + + scheduler = Support.Stubs.Scheduler.create(ctx.project_id, ctx.user_id, name: "Scheduler") + scheduler_id = scheduler.id + + assert {:ok, %Tesla.Env{status: 200}} = + Tesla.patch( + http_client(ctx), + "/projects/#{ctx.project_id}/tasks/" <> scheduler.id, + params + ) + + assert %{ + api_model: %{ + id: ^scheduler_id, + reference: "refs/heads/feature-branch" + } + } = Support.Stubs.DB.find(:schedulers, scheduler.id) + end + + test "PATCH /projects/:project_id_or_name/tasks/:id - endpoint updates reference structure for tag", + ctx do + params = %{ + apiVersion: "v2", + kind: "Task", + spec: %{ + reference: %{ + type: "tag", + name: "v1.0.0" + } + } + } + + scheduler = Support.Stubs.Scheduler.create(ctx.project_id, ctx.user_id, name: "Scheduler") + scheduler_id = scheduler.id + + assert {:ok, %Tesla.Env{status: 200}} = + Tesla.patch( + http_client(ctx), + "/projects/#{ctx.project_id}/tasks/" <> scheduler.id, + params + ) + + assert %{ + api_model: %{ + id: ^scheduler_id, + reference: "refs/tags/v1.0.0" + } + } = Support.Stubs.DB.find(:schedulers, scheduler.id) + end + + test "PATCH /projects/:project_id_or_name/tasks/:id - endpoint updates reference structure with partial updates", + ctx do + params = %{ + apiVersion: "v2", + kind: "Task", + spec: %{ + reference: %{ + type: "tag", + name: "v1.0.0" + } + } + } + + scheduler = Support.Stubs.Scheduler.create(ctx.project_id, ctx.user_id, name: "Scheduler") + scheduler_id = scheduler.id + + assert {:ok, %Tesla.Env{status: 200}} = + Tesla.patch( + http_client(ctx), + "/projects/#{ctx.project_id}/tasks/" <> scheduler.id, + params + ) + + assert %{ + api_model: %{ + id: ^scheduler_id, + reference: "refs/tags/v1.0.0" + } + } = Support.Stubs.DB.find(:schedulers, scheduler.id) + end + test "PATCH /projects/:project_id_or_name/tasks/:id - endpoint returns 404 when task is not owned by requester", ctx do wrong_owner = UUID.uuid4() @@ -138,7 +234,10 @@ defmodule Router.Tasks.UpdateTest do kind: "Task", spec: %{ name: "Task", - branch: "master", + reference: %{ + type: "branch", + name: "master" + }, pipeline_file: "pipeline.yml" } } diff --git a/public-api/v2/test/support/stubs/scheduler.ex b/public-api/v2/test/support/stubs/scheduler.ex index c1a6fc975..0afe78ba5 100644 --- a/public-api/v2/test/support/stubs/scheduler.ex +++ b/public-api/v2/test/support/stubs/scheduler.ex @@ -29,7 +29,7 @@ defmodule Support.Stubs.Scheduler do id: UUID.uuid4(), name: "Scheduler", project_id: project_id, - branch: "master", + reference: "refs/heads/master", at: "* * * * *", pipeline_file: ".semaphore/semaphore.yml", requester_id: user_id, @@ -64,7 +64,7 @@ defmodule Support.Stubs.Scheduler do [ triggered_at: Time.now(), project_id: periodic.project_id, - branch: periodic.branch, + reference: periodic.reference, pipeline_file: periodic.pipeline_file, scheduling_status: "passed", scheduled_workflow_id: workflow_id, @@ -159,7 +159,7 @@ defmodule Support.Stubs.Scheduler do end end - @modifiable_fields ~w(name description recurring branch pipeline_file at parameters)a + @modifiable_fields ~w(name description recurring reference pipeline_file at parameters)a def persist(req, _) do model_from_req = periodic_from_req(req) @@ -324,7 +324,7 @@ defmodule Support.Stubs.Scheduler do params = Enum.filter( [ - branch: if(req.branch != "", do: req.branch), + reference: if(req.reference != "", do: req.reference), pipeline_file: if(req.pipeline_file != "", do: req.pipeline_file), parameter_values: req.parameter_values ], @@ -363,7 +363,7 @@ defmodule Support.Stubs.Scheduler do id: metadata["id"], name: metadata["name"], project_id: "", - branch: spec["branch"], + reference: "refs/heads/#{spec["branch"]}", at: spec["at"], pipeline_file: spec["pipeline_file"], requester_id: req.requester_id, @@ -378,7 +378,7 @@ defmodule Support.Stubs.Scheduler do description: req.description, recurring: req.recurring, requester_id: req.requester_id, - branch: req.branch, + reference: req.reference, pipeline_file: req.pipeline_file, at: req.at, parameters: req.parameters