diff --git a/engine/api/v2_entities.go b/engine/api/v2_entities.go index 7e328f06fa..06c89b4d79 100644 --- a/engine/api/v2_entities.go +++ b/engine/api/v2_entities.go @@ -70,7 +70,7 @@ func (api *API) postEntityCheckHandler() ([]service.RbacChecker, service.Handler response.Messages = append(response.Messages, fmt.Sprintf("%q", err)) } if err == nil { - errs := wm.Lint() + errs := wm.LintYamlDefinition() for _, err := range errs { response.Messages = append(response.Messages, err.Error()) } @@ -82,7 +82,7 @@ func (api *API) postEntityCheckHandler() ([]service.RbacChecker, service.Handler response.Messages = append(response.Messages, fmt.Sprintf("%q", err)) } if err == nil { - errs := a.Lint() + errs := a.LintYamlDefinition() for _, err := range errs { response.Messages = append(response.Messages, err.Error()) } @@ -94,7 +94,7 @@ func (api *API) postEntityCheckHandler() ([]service.RbacChecker, service.Handler response.Messages = append(response.Messages, fmt.Sprintf("%q", err)) } if err == nil { - errs := w.Lint() + errs := w.LintYamlDefinition() for _, err := range errs { response.Messages = append(response.Messages, err.Error()) } @@ -106,7 +106,7 @@ func (api *API) postEntityCheckHandler() ([]service.RbacChecker, service.Handler response.Messages = append(response.Messages, fmt.Sprintf("%q", err)) } if err == nil { - errs := wt.Lint() + errs := wt.LintYamlDefinition() for _, err := range errs { response.Messages = append(response.Messages, err.Error()) } diff --git a/engine/api/v2_repository_analyze.go b/engine/api/v2_repository_analyze.go index 358be5d310..c3c99aa747 100644 --- a/engine/api/v2_repository_analyze.go +++ b/engine/api/v2_repository_analyze.go @@ -1656,7 +1656,7 @@ func (api *API) handleEntitiesFiles(ctx context.Context, ef *EntityFinder, files // and concurrencies belong to the run's project. func Lint[T sdk.Lintable](ctx context.Context, db *gorp.DbMap, store cache.Store, o T, ef *EntityFinder, ownerProjectKey string, wmDockerImageWhiteList []regexp.Regexp) []error { // 1. Static lint - if err := o.Lint(); err != nil { + if err := o.LintYamlDefinition(); err != nil { return err } diff --git a/engine/api/v2_workflow_run_craft.go b/engine/api/v2_workflow_run_craft.go index dc87344847..12d9e8f4f8 100644 --- a/engine/api/v2_workflow_run_craft.go +++ b/engine/api/v2_workflow_run_craft.go @@ -316,7 +316,7 @@ func (api *API) craftWorkflowRunV2(ctx context.Context, id string) error { // Check workflow lint in case of modification through job template msgs := make([]sdk.V2WorkflowRunInfo, 0) - errs := run.WorkflowData.Workflow.Lint() + errs := run.WorkflowData.Workflow.LintYamlDefinition() for _, e := range errs { msgs = append(msgs, sdk.V2WorkflowRunInfo{ WorkflowRunID: run.ID, @@ -558,7 +558,7 @@ func (api *API) craftWorkflowRunV2(ctx context.Context, id string) error { } func retrieveAndUpdateAllJobDependencies(ctx context.Context, db *gorp.DbMap, store cache.Store, run *sdk.V2WorkflowRun, jobID string, j sdk.V2Job, wref *WorkflowRunEntityFinder, integrations map[string]sdk.ProjectIntegration, allVariableSets []sdk.ProjectVariableSet, defaultRegion string) *sdk.V2WorkflowRunInfo { - if len(j.Steps) == 0 && j.From == "" { + if len(j.Steps) == 0 && !j.NeedsTemplateResolution() { return nil } @@ -598,7 +598,7 @@ func retrieveAndUpdateAllJobDependencies(ctx context.Context, db *gorp.DbMap, st } // Check worker model - if !strings.Contains(j.RunsOn.Model, "${{") && j.From == "" && !strings.Contains(j.Region, "${{") { + if !strings.Contains(j.RunsOn.Model, "${{") && !j.NeedsTemplateResolution() && !strings.Contains(j.Region, "${{") { completeName, msg, err := wref.checkWorkerModel(ctx, db, store, jobID, j.RunsOn.Model, j.Region, defaultRegion) if err != nil { log.ErrorWithStackTrace(ctx, err) diff --git a/engine/api/v2_workflow_run_craft_test.go b/engine/api/v2_workflow_run_craft_test.go index 6865225baf..2648c5d178 100644 --- a/engine/api/v2_workflow_run_craft_test.go +++ b/engine/api/v2_workflow_run_craft_test.go @@ -3555,3 +3555,152 @@ spec: |- require.Equal(t, 0, len(wrInfos), "Error found: %v", wrInfos) require.Equal(t, sdk.V2WorkflowRunStatusBuilding, wrDB.Status) } + +// A workflow holding a job template reference: crafting must leave the reference +// untouched (no worker model resolution on it) while resolving the dependencies of +// the sibling concrete job. +func TestCraftWorkflowRunWithJobTemplateReference(t *testing.T) { + api, db, _ := newTestAPI(t) + ctx := context.TODO() + + db.Exec("DELETE FROM rbac") + db.Exec("DELETE FROM region") + + reg := sdk.Region{Name: "build"} + require.NoError(t, region.Insert(ctx, db, ®)) + api.Config.Workflow.JobDefaultRegion = reg.Name + + proj := assets.InsertTestProject(t, db, api.Cache, sdk.RandomString(10), sdk.RandomString(10)) + admin, _ := assets.InsertAdminUser(t, db) + + vcsProject := assets.InsertTestVCSProject(t, db, proj.ID, "github", "github") + repo := assets.InsertTestProjectRepository(t, db, proj.Key, vcsProject.ID, sdk.RandomString(10)) + + s, _ := assets.InsertService(t, db, t.Name()+"_VCS", sdk.TypeVCS) + ctrl := gomock.NewController(t) + defer ctrl.Finish() + servicesClients := mock_services.NewMockClient(ctrl) + services.NewClient = func(_ []sdk.Service) services.Client { + return servicesClients + } + t.Cleanup(func() { + _ = services.Delete(db, s) + services.NewClient = services.NewDefaultClient + }) + servicesClients.EXPECT(). + DoJSONRequest(gomock.Any(), "GET", "/vcs/github/repos/"+repo.Name, gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn( + func(ctx context.Context, method, path string, in interface{}, out interface{}, _ interface{}) (http.Header, int, error) { + b := &sdk.VCSRepo{} + *(out.(*sdk.VCSRepo)) = *b + return nil, 200, nil + }, + ).Times(1) + + entityTmpl := sdk.Entity{ + ProjectKey: proj.Key, + ProjectRepositoryID: repo.ID, + Type: sdk.EntityTypeWorkflowTemplate, + FilePath: ".cds/workflow-templates/mytmpl.yml", + Name: "myTemplate", + Ref: "refs/heads/master", + Commit: "123456789", + Data: `name: mytemplate +spec: |- + jobs: + fromTemplate: + runs-on: .cds/worker-models/myworker-model.yml + steps: + - run: echo "from template"`, + } + require.NoError(t, entity.Insert(ctx, db, &entityTmpl)) + + myWMEnt := sdk.Entity{ + ProjectKey: proj.Key, + ProjectRepositoryID: repo.ID, + Type: sdk.EntityTypeWorkerModel, + FilePath: ".cds/worker-models/myworker-model.yml", + Name: "myworker-model", + Ref: "refs/heads/master", + Commit: "123456789", + Data: "name: myworkermodel", + } + require.NoError(t, entity.Insert(ctx, db, &myWMEnt)) + + hatch := sdk.Hatchery{Name: sdk.RandomString(10), ModelType: ""} + require.NoError(t, hatchery.Insert(ctx, db, &hatch)) + perm := sdk.RBAC{ + Name: sdk.RandomString(10), + Hatcheries: []sdk.RBACHatchery{ + { + RegionID: reg.ID, + HatcheryID: hatch.ID, + Role: sdk.HatcheryRoleSpawn, + }, + }, + } + require.NoError(t, rbac.Insert(ctx, db, &perm)) + + wkName := sdk.RandomString(10) + wr := sdk.V2WorkflowRun{ + DeprecatedUserID: admin.ID, + ProjectKey: proj.Key, + Status: sdk.V2WorkflowRunStatusCrafting, + VCSServerID: vcsProject.ID, + RepositoryID: repo.ID, + RunNumber: 0, + RunAttempt: 0, + WorkflowRef: "refs/heads/master", + WorkflowSha: "123456789", + WorkflowName: wkName, + WorkflowData: sdk.V2WorkflowRunData{ + Workflow: sdk.V2Workflow{ + Name: wkName, + Jobs: map[string]sdk.V2Job{ + "tmplJob": { + From: ".cds/workflow-templates/mytmpl.yml", + }, + "normalJob": { + RunsOn: sdk.V2JobRunsOn{ + Model: "myworker-model", + }, + Steps: []sdk.ActionStep{ + {ID: "step1", Run: "echo hello"}, + }, + }, + }, + }, + }, + Initiator: &sdk.V2Initiator{ + UserID: admin.ID, + IsAdminWithMFA: true, + }, + RunEvent: sdk.V2WorkflowRunEvent{ + HookType: sdk.WorkflowHookTypeRepository, + Payload: nil, + Ref: "refs/heads/master", + Sha: "123456789", + EventName: sdk.WorkflowHookEventNamePush, + }, + } + require.NoError(t, workflow_v2.InsertRun(ctx, db, &wr)) + + require.NoError(t, api.craftWorkflowRunV2(ctx, wr.ID)) + + wrDB, err := workflow_v2.LoadRunByID(ctx, db, wr.ID) + require.NoError(t, err) + wrInfos, err := workflow_v2.LoadRunInfosByRunID(ctx, db, wr.ID) + require.NoError(t, err) + require.Equal(t, 0, len(wrInfos), "Error found: %v", wrInfos) + require.Equal(t, sdk.V2WorkflowRunStatusBuilding, wrDB.Status) + + // The job template reference is kept as is: no expansion, no model resolution + tmplJob := wrDB.WorkflowData.Workflow.Jobs["tmplJob"] + require.Equal(t, ".cds/workflow-templates/mytmpl.yml", tmplJob.From) + require.Empty(t, tmplJob.RunsOn.Model) + require.Empty(t, tmplJob.Steps) + + // The concrete job got its worker model resolved + normalJob := wrDB.WorkflowData.Workflow.Jobs["normalJob"] + require.Contains(t, normalJob.RunsOn.Model, "myworker-model@refs/heads/master") +} diff --git a/engine/api/v2_workflow_run_engine.go b/engine/api/v2_workflow_run_engine.go index b9faf1de5f..325df9ca6a 100644 --- a/engine/api/v2_workflow_run_engine.go +++ b/engine/api/v2_workflow_run_engine.go @@ -325,7 +325,7 @@ func (api *API) workflowRunV2Trigger(ctx context.Context, wrEnqueue sdk.V2Workfl // Enqueue JOB hasTemplatedJob := false for _, j := range jobsToQueue { - if j.Job.From != "" { + if j.Job.NeedsTemplateResolution() { hasTemplatedJob = true } } @@ -1536,7 +1536,7 @@ func prepareRunJobs(ctx context.Context, db *gorp.DbMap, store cache.Store, proj } // No matrixed jobs or templated job skipped - if len(matrixPermutation) == 0 || (jobDef.From != "" && jobToTrigger.Status.IsTerminated()) { + if len(matrixPermutation) == 0 || (jobDef.NeedsTemplateResolution() && jobToTrigger.Status.IsTerminated()) { runJob := sdk.V2WorkflowRunJob{ ID: sdk.UUID(), WorkflowRunID: run.ID, @@ -1555,7 +1555,7 @@ func prepareRunJobs(ctx context.Context, db *gorp.DbMap, store cache.Store, proj RunAttempt: run.RunAttempt, Initiator: wrEnqueue.Initiator, } - if jobDef.From == "" && len(jobDef.Steps) == 0 && !jobToTrigger.Status.IsTerminated() { + if !jobDef.NeedsTemplateResolution() && len(jobDef.Steps) == 0 && !jobToTrigger.Status.IsTerminated() { runJob.Status = sdk.V2WorkflowRunJobStatusSuccess } // If the current job was a matrix, skip it @@ -1572,7 +1572,7 @@ func prepareRunJobs(ctx context.Context, db *gorp.DbMap, store cache.Store, proj // If job has to be run if !runJob.Status.IsTerminated() { // If from template, retrieve template and apply it - if jobDef.From != "" { + if jobDef.NeedsTemplateResolution() { hasToUpdateRun = true // For templated job, we only create new jobs on the parent worklow // With hasToUpdateRun the workflow run will be saved to update his definition @@ -1660,7 +1660,7 @@ func prepareRunJobs(ctx context.Context, db *gorp.DbMap, store cache.Store, proj allVariableSets: allVariableSets, } - if jobDef.From == "" { + if !jobDef.NeedsTemplateResolution() { jobs, runUpdated, err := createMatrixedRunJobs(ctx, db, store, wref, matrixPermutation, runJobsInfo, run, jobData, concurrenciesDef, concurrencyUnlockedCount, runObjectsToCancelled) if err != nil { return nil, nil, nil, nil, false, err @@ -1791,7 +1791,7 @@ func computeJobFromTemplate(ctx context.Context, db *gorp.DbMap, store cache.Sto } msgsLint := make([]sdk.V2WorkflowRunInfo, 0) - errs := run.WorkflowData.Workflow.Lint() + errs := run.WorkflowData.Workflow.LintWorkflowRunData() for _, e := range errs { msgsLint = append(msgsLint, sdk.V2WorkflowRunInfo{ WorkflowRunID: run.ID, @@ -1892,6 +1892,12 @@ loop: // Set job on workflow for k, v := range newJobs { + // Jobs resolved from the template keep the template's complete name as + // provenance. Empty jobs are skipped: `from` without content would read as + // an unresolved reference. Nested references keep their own `from`. + if v.From == "" && (len(v.Steps) > 0 || v.RunsOn.Model != "") { + v.From = templateEntity.CompleteName + } run.WorkflowData.Workflow.Jobs[k] = v msg := retrieveAndUpdateAllJobDependencies(ctx, db, store, run, k, v, wrefTemplate, integrations, allVariableSets, defaultRegion) if msg != nil { @@ -2102,7 +2108,7 @@ func createTemplatedMatrixedJobs(ctx context.Context, db *gorp.DbMap, store cach } msgsLint := make([]sdk.V2WorkflowRunInfo, 0) - errs := run.WorkflowData.Workflow.Lint() + errs := run.WorkflowData.Workflow.LintWorkflowRunData() for _, e := range errs { msgsLint = append(msgsLint, sdk.V2WorkflowRunInfo{ WorkflowRunID: run.ID, @@ -2373,7 +2379,7 @@ func retrieveJobToQueue(ctx context.Context, db *gorp.DbMap, wrEnqueue sdk.V2Wor if runJobMapItem.Job.Strategy != nil && len(runJobMapItem.Job.Strategy.Matrix) > 0 { // If runjob has a status && a template, ignore it. A matrix job can be run if template has been resolved - if runJobMapItem.Job.From != "" { + if runJobMapItem.Job.NeedsTemplateResolution() { continue } diff --git a/engine/api/v2_workflow_run_engine_job_template_test.go b/engine/api/v2_workflow_run_engine_job_template_test.go index be1590b3fa..ecaf8c0b4c 100644 --- a/engine/api/v2_workflow_run_engine_job_template_test.go +++ b/engine/api/v2_workflow_run_engine_job_template_test.go @@ -2,10 +2,12 @@ package api import ( "context" + "fmt" "testing" "time" "github.com/ovh/cds/engine/api/entity" + "github.com/ovh/cds/engine/api/hatchery" "github.com/ovh/cds/engine/api/organization" "github.com/ovh/cds/engine/api/project" "github.com/ovh/cds/engine/api/rbac" @@ -164,6 +166,11 @@ spec: |- _, has = wrAfter1.WorkflowData.Workflow.Jobs["deploy"] require.True(t, has) + // Empty jobs carry no provenance, the nested reference keeps its own from + require.Empty(t, wrAfter1.WorkflowData.Workflow.Jobs["build"].From) + require.Empty(t, wrAfter1.WorkflowData.Workflow.Jobs["test"].From) + require.Equal(t, ".cds/workflow-templates/mytmpl2.yml", wrAfter1.WorkflowData.Workflow.Jobs["deploy"].From) + rjs, err := workflow_v2.LoadRunJobsByRunID(context.TODO(), db, wr.ID, wr.RunAttempt) require.NoError(t, err) require.Equal(t, 0, len(rjs)) // No run jobs @@ -203,6 +210,10 @@ spec: |- _, has = wrAfter2.WorkflowData.Workflow.Jobs["it4"] require.True(t, has) + // Empty jobs injected from the nested template carry no provenance + for _, jobID := range []string{"it", "it2", "it3", "it4"} { + require.Empty(t, wrAfter2.WorkflowData.Workflow.Jobs[jobID].From) + } } func TestWorkflowTrigger_JobTemplateDuplicateJob(t *testing.T) { @@ -638,3 +649,513 @@ spec: |- } require.Equal(t, sdk.V2WorkflowRunStatusBuilding, wrAfter1.Status) } + +// A job template whose content declares a job combining `from` with runs-on: +// resolving the template must fail the run with a lint error. +func TestWorkflowTrigger_JobTemplateWithFromAndRunsOnFails(t *testing.T) { + ctx := context.TODO() + api, db, _ := newTestAPI(t) + + _, err := db.Exec("DELETE FROM rbac") + require.NoError(t, err) + _, err = db.Exec("DELETE FROM region") + require.NoError(t, err) + + admin, _ := assets.InsertAdminUser(t, db) + + org, err := organization.LoadOrganizationByName(context.TODO(), db, "default") + require.NoError(t, err) + + reg := sdk.Region{ + Name: "build", + } + require.NoError(t, region.Insert(context.TODO(), db, ®)) + api.Config.Workflow.JobDefaultRegion = reg.Name + + proj := assets.InsertTestProject(t, db, api.Cache, sdk.RandomString(10), sdk.RandomString(10)) + + rb := sdk.RBAC{ + Name: sdk.RandomString(10), + Regions: []sdk.RBACRegion{ + { + RegionID: reg.ID, + AllUsers: true, + RBACOrganizationIDs: []string{org.ID}, + Role: sdk.RegionRoleExecute, + }, + }, + RegionProjects: []sdk.RBACRegionProject{ + { + Role: sdk.RegionRoleExecute, + AllProjects: true, + RegionID: reg.ID, + }, + }, + } + require.NoError(t, rbac.Insert(context.TODO(), db, &rb)) + + vcsServer := assets.InsertTestVCSProject(t, db, proj.ID, "github", "github") + repo := assets.InsertTestProjectRepository(t, db, proj.Key, vcsServer.ID, sdk.RandomString(10)) + + // Template whose spec declares an invalid job: from combined with runs-on + entityTmpl := sdk.Entity{ + ProjectKey: proj.Key, + Type: sdk.EntityTypeWorkflowTemplate, + FilePath: ".cds/workflow-templates/mytmpl.yml", + Name: "myTemplate", + Commit: "123456789", + Ref: "refs/heads/master", + ProjectRepositoryID: repo.ID, + UserID: &admin.ID, + Data: `name: mytemplate +spec: |- + jobs: + bad: + from: .cds/workflow-templates/other.yml + runs-on: mymodel`, + } + require.NoError(t, entity.Insert(ctx, db, &entityTmpl)) + + wr := sdk.V2WorkflowRun{ + ProjectKey: proj.Key, + VCSServerID: vcsServer.ID, + VCSServer: vcsServer.Name, + RepositoryID: repo.ID, + Repository: repo.Name, + WorkflowName: sdk.RandomString(10), + WorkflowSha: "123456789", + WorkflowRef: "refs/heads/master", + RunAttempt: 1, + RunNumber: 1, + Started: time.Now(), + LastModified: time.Now(), + Status: sdk.V2WorkflowRunStatusBuilding, + RunEvent: sdk.V2WorkflowRunEvent{}, + WorkflowData: sdk.V2WorkflowRunData{Workflow: sdk.V2Workflow{ + Name: "myworkflow", + Jobs: map[string]sdk.V2Job{ + "root": { + From: ".cds/workflow-templates/mytmpl.yml", + }, + }, + }}, + Initiator: &sdk.V2Initiator{ + UserID: admin.ID, + User: admin.Initiator(), + }, + } + require.NoError(t, workflow_v2.InsertRun(context.Background(), db, &wr)) + + require.NoError(t, api.workflowRunV2Trigger(context.Background(), sdk.V2WorkflowRunEnqueue{ + RunID: wr.ID, + Initiator: sdk.V2Initiator{ + UserID: admin.ID, + User: admin.Initiator(), + IsAdminWithMFA: true, + }, + })) + + wrAfter, err := workflow_v2.LoadRunByID(context.TODO(), db, wr.ID) + require.NoError(t, err) + require.Equal(t, sdk.V2WorkflowRunStatusFail, wrAfter.Status) + + runInfos, err := workflow_v2.LoadRunInfosByRunID(context.TODO(), db, wr.ID) + require.NoError(t, err) + require.Equal(t, 1, len(runInfos)) + require.Contains(t, runInfos[0].Message, "from cannot be combined with steps or runs-on") +} + +// A job template whose content declares a concrete job with a matrix strategy: +// the first trigger replaces the templated job by the matrix job in the workflow +// definition; the second trigger enqueues one run job per matrix permutation. +func TestWorkflowTrigger_JobTemplateContainingMatrixJob(t *testing.T) { + ctx := context.TODO() + api, db, _ := newTestAPI(t) + + _, err := db.Exec("DELETE FROM rbac") + require.NoError(t, err) + _, err = db.Exec("DELETE FROM region") + require.NoError(t, err) + + admin, _ := assets.InsertAdminUser(t, db) + + org, err := organization.LoadOrganizationByName(context.TODO(), db, "default") + require.NoError(t, err) + + reg := sdk.Region{ + Name: "build", + } + require.NoError(t, region.Insert(context.TODO(), db, ®)) + api.Config.Workflow.JobDefaultRegion = reg.Name + + proj := assets.InsertTestProject(t, db, api.Cache, sdk.RandomString(10), sdk.RandomString(10)) + + rb := sdk.RBAC{ + Name: sdk.RandomString(10), + Regions: []sdk.RBACRegion{ + { + RegionID: reg.ID, + AllUsers: true, + RBACOrganizationIDs: []string{org.ID}, + Role: sdk.RegionRoleExecute, + }, + }, + RegionProjects: []sdk.RBACRegionProject{ + { + Role: sdk.RegionRoleExecute, + AllProjects: true, + RegionID: reg.ID, + }, + }, + } + require.NoError(t, rbac.Insert(context.TODO(), db, &rb)) + + // Hatchery allowed to spawn docker jobs on the region + hatch := sdk.Hatchery{Name: sdk.RandomString(10), ModelType: "docker"} + require.NoError(t, hatchery.Insert(context.TODO(), db, &hatch)) + rbHatch := sdk.RBAC{ + Name: sdk.RandomString(10), + Hatcheries: []sdk.RBACHatchery{ + { + RegionID: reg.ID, + HatcheryID: hatch.ID, + Role: sdk.HatcheryRoleSpawn, + }, + }, + } + require.NoError(t, rbac.Insert(context.TODO(), db, &rbHatch)) + + vcsServer := assets.InsertTestVCSProject(t, db, proj.ID, "github", "github") + repo := assets.InsertTestProjectRepository(t, db, proj.Key, vcsServer.ID, sdk.RandomString(10)) + + entityModel := sdk.Entity{ + ProjectKey: proj.Key, + Type: sdk.EntityTypeWorkerModel, + FilePath: ".cds/worker-models/mymodel.yml", + Name: "mymodel", + Commit: "123456789", + Ref: "refs/heads/master", + ProjectRepositoryID: repo.ID, + UserID: &admin.ID, + Data: `name: mymodel +type: docker +osarch: linux-amd64 +spec: + image: debian:12`, + } + require.NoError(t, entity.Insert(ctx, db, &entityModel)) + + // Template containing a concrete job with a matrix strategy + entityTmpl := sdk.Entity{ + ProjectKey: proj.Key, + Type: sdk.EntityTypeWorkflowTemplate, + FilePath: ".cds/workflow-templates/mytmpl.yml", + Name: "myTemplate", + Commit: "123456789", + Ref: "refs/heads/master", + ProjectRepositoryID: repo.ID, + UserID: &admin.ID, + Data: `name: mytemplate +spec: |- + jobs: + deploy: + runs-on: .cds/worker-models/mymodel.yml + strategy: + matrix: + region: [region1, region2] + steps: + - run: echo "Deploy ${{ matrix.region }}"`, + } + require.NoError(t, entity.Insert(ctx, db, &entityTmpl)) + + wr := sdk.V2WorkflowRun{ + ProjectKey: proj.Key, + VCSServerID: vcsServer.ID, + VCSServer: vcsServer.Name, + RepositoryID: repo.ID, + Repository: repo.Name, + WorkflowName: sdk.RandomString(10), + WorkflowSha: "123456789", + WorkflowRef: "refs/heads/master", + RunAttempt: 1, + RunNumber: 1, + Started: time.Now(), + LastModified: time.Now(), + Status: sdk.V2WorkflowRunStatusBuilding, + RunEvent: sdk.V2WorkflowRunEvent{}, + WorkflowData: sdk.V2WorkflowRunData{Workflow: sdk.V2Workflow{ + Name: "myworkflow", + Jobs: map[string]sdk.V2Job{ + "root": { + From: ".cds/workflow-templates/mytmpl.yml", + }, + }, + }}, + Initiator: &sdk.V2Initiator{ + UserID: admin.ID, + User: admin.Initiator(), + }, + } + require.NoError(t, workflow_v2.InsertRun(context.Background(), db, &wr)) + + // First trigger: the templated job is expanded into the workflow definition, no run job yet + require.NoError(t, api.workflowRunV2Trigger(context.Background(), sdk.V2WorkflowRunEnqueue{ + RunID: wr.ID, + Initiator: sdk.V2Initiator{ + UserID: admin.ID, + User: admin.Initiator(), + IsAdminWithMFA: true, + }, + })) + + runInfos, err := workflow_v2.LoadRunInfosByRunID(context.TODO(), db, wr.ID) + require.NoError(t, err) + for _, ri := range runInfos { + t.Logf("RunInfo: %s", ri.Message) + } + require.Equal(t, 0, len(runInfos)) + + wrAfter1, err := workflow_v2.LoadRunByID(context.TODO(), db, wr.ID) + require.NoError(t, err) + require.Equal(t, 1, len(wrAfter1.WorkflowData.Workflow.Jobs)) + deployJob, has := wrAfter1.WorkflowData.Workflow.Jobs["deploy"] + require.True(t, has) + require.NotNil(t, deployJob.Strategy) + // The injected concrete job carries the template's complete name as provenance + require.Equal(t, fmt.Sprintf("%s/%s/%s/mytemplate@refs/heads/master", proj.Key, vcsServer.Name, repo.Name), deployJob.From) + + rjs, err := workflow_v2.LoadRunJobsByRunID(context.TODO(), db, wr.ID, wr.RunAttempt) + require.NoError(t, err) + require.Equal(t, 0, len(rjs)) + + // Second trigger: all matrix permutations must be enqueued + require.NoError(t, api.workflowRunV2Trigger(context.Background(), sdk.V2WorkflowRunEnqueue{ + RunID: wr.ID, + Initiator: sdk.V2Initiator{ + UserID: admin.ID, + User: admin.Initiator(), + IsAdminWithMFA: true, + }, + })) + + runInfos, err = workflow_v2.LoadRunInfosByRunID(context.TODO(), db, wr.ID) + require.NoError(t, err) + for _, ri := range runInfos { + t.Logf("RunInfo: %s", ri.Message) + } + require.Equal(t, 0, len(runInfos)) + + rjs, err = workflow_v2.LoadRunJobsByRunID(context.TODO(), db, wr.ID, wr.RunAttempt) + require.NoError(t, err) + require.Equal(t, 2, len(rjs)) + permutations := map[string]bool{} + for _, rj := range rjs { + require.Equal(t, "deploy", rj.JobID) + require.Equal(t, sdk.V2WorkflowRunJobStatusWaiting, rj.Status) + require.Equal(t, fmt.Sprintf("%s/%s/%s/mytemplate@refs/heads/master", proj.Key, vcsServer.Name, repo.Name), rj.Job.From) + permutations[rj.Matrix["region"]] = true + } + require.True(t, permutations["region1"]) + require.True(t, permutations["region2"]) +} + +// A matrix job coming from a job template with some permutations already run: +// triggering the workflow must enqueue only the missing permutations, leaving +// the already-run ones untouched. +func TestWorkflowTrigger_JobTemplateContainingMatrixJobPartialPermutations(t *testing.T) { + ctx := context.TODO() + api, db, _ := newTestAPI(t) + + _, err := db.Exec("DELETE FROM rbac") + require.NoError(t, err) + _, err = db.Exec("DELETE FROM region") + require.NoError(t, err) + + admin, _ := assets.InsertAdminUser(t, db) + + org, err := organization.LoadOrganizationByName(context.TODO(), db, "default") + require.NoError(t, err) + + reg := sdk.Region{ + Name: "build", + } + require.NoError(t, region.Insert(context.TODO(), db, ®)) + api.Config.Workflow.JobDefaultRegion = reg.Name + + proj := assets.InsertTestProject(t, db, api.Cache, sdk.RandomString(10), sdk.RandomString(10)) + + rb := sdk.RBAC{ + Name: sdk.RandomString(10), + Regions: []sdk.RBACRegion{ + { + RegionID: reg.ID, + AllUsers: true, + RBACOrganizationIDs: []string{org.ID}, + Role: sdk.RegionRoleExecute, + }, + }, + RegionProjects: []sdk.RBACRegionProject{ + { + Role: sdk.RegionRoleExecute, + AllProjects: true, + RegionID: reg.ID, + }, + }, + } + require.NoError(t, rbac.Insert(context.TODO(), db, &rb)) + + // Hatchery allowed to spawn docker jobs on the region + hatch := sdk.Hatchery{Name: sdk.RandomString(10), ModelType: "docker"} + require.NoError(t, hatchery.Insert(context.TODO(), db, &hatch)) + rbHatch := sdk.RBAC{ + Name: sdk.RandomString(10), + Hatcheries: []sdk.RBACHatchery{ + { + RegionID: reg.ID, + HatcheryID: hatch.ID, + Role: sdk.HatcheryRoleSpawn, + }, + }, + } + require.NoError(t, rbac.Insert(context.TODO(), db, &rbHatch)) + + vcsServer := assets.InsertTestVCSProject(t, db, proj.ID, "github", "github") + repo := assets.InsertTestProjectRepository(t, db, proj.Key, vcsServer.ID, sdk.RandomString(10)) + + entityModel := sdk.Entity{ + ProjectKey: proj.Key, + Type: sdk.EntityTypeWorkerModel, + FilePath: ".cds/worker-models/mymodel.yml", + Name: "mymodel", + Commit: "123456789", + Ref: "refs/heads/master", + ProjectRepositoryID: repo.ID, + UserID: &admin.ID, + Data: `name: mymodel +type: docker +osarch: linux-amd64 +spec: + image: debian:12`, + } + require.NoError(t, entity.Insert(ctx, db, &entityModel)) + + // Template containing a concrete job with a matrix strategy + entityTmpl := sdk.Entity{ + ProjectKey: proj.Key, + Type: sdk.EntityTypeWorkflowTemplate, + FilePath: ".cds/workflow-templates/mytmpl.yml", + Name: "myTemplate", + Commit: "123456789", + Ref: "refs/heads/master", + ProjectRepositoryID: repo.ID, + UserID: &admin.ID, + Data: `name: mytemplate +spec: |- + jobs: + deploy: + runs-on: .cds/worker-models/mymodel.yml + strategy: + matrix: + region: [region1, region2] + steps: + - run: echo "Deploy ${{ matrix.region }}"`, + } + require.NoError(t, entity.Insert(ctx, db, &entityTmpl)) + + wr := sdk.V2WorkflowRun{ + ProjectKey: proj.Key, + VCSServerID: vcsServer.ID, + VCSServer: vcsServer.Name, + RepositoryID: repo.ID, + Repository: repo.Name, + WorkflowName: sdk.RandomString(10), + WorkflowSha: "123456789", + WorkflowRef: "refs/heads/master", + RunAttempt: 1, + RunNumber: 1, + Started: time.Now(), + LastModified: time.Now(), + Status: sdk.V2WorkflowRunStatusBuilding, + RunEvent: sdk.V2WorkflowRunEvent{}, + WorkflowData: sdk.V2WorkflowRunData{Workflow: sdk.V2Workflow{ + Name: "myworkflow", + Jobs: map[string]sdk.V2Job{ + "root": { + From: ".cds/workflow-templates/mytmpl.yml", + }, + }, + }}, + Initiator: &sdk.V2Initiator{ + UserID: admin.ID, + User: admin.Initiator(), + }, + } + require.NoError(t, workflow_v2.InsertRun(context.Background(), db, &wr)) + + // First trigger: the templated job is expanded into the workflow definition, no run job yet + require.NoError(t, api.workflowRunV2Trigger(context.Background(), sdk.V2WorkflowRunEnqueue{ + RunID: wr.ID, + Initiator: sdk.V2Initiator{ + UserID: admin.ID, + User: admin.Initiator(), + IsAdminWithMFA: true, + }, + })) + + wrAfter1, err := workflow_v2.LoadRunByID(context.TODO(), db, wr.ID) + require.NoError(t, err) + deployJob, has := wrAfter1.WorkflowData.Workflow.Jobs["deploy"] + require.True(t, has) + + // Simulate a permutation that already ran + now := time.Now() + rjDone := sdk.V2WorkflowRunJob{ + JobID: "deploy", + WorkflowRunID: wr.ID, + ProjectKey: proj.Key, + WorkflowName: wr.WorkflowName, + RunNumber: wr.RunNumber, + RunAttempt: wr.RunAttempt, + Status: sdk.V2WorkflowRunJobStatusSuccess, + Queued: time.Now(), + Scheduled: &now, + Started: &now, + Ended: &now, + Job: deployJob, + Matrix: map[string]string{"region": "region1"}, + Initiator: *wr.Initiator, + } + require.NoError(t, workflow_v2.InsertRunJob(context.TODO(), db, &rjDone)) + + // Second trigger: only the missing permutation must be enqueued + require.NoError(t, api.workflowRunV2Trigger(context.Background(), sdk.V2WorkflowRunEnqueue{ + RunID: wr.ID, + Initiator: sdk.V2Initiator{ + UserID: admin.ID, + User: admin.Initiator(), + IsAdminWithMFA: true, + }, + })) + + runInfos, err := workflow_v2.LoadRunInfosByRunID(context.TODO(), db, wr.ID) + require.NoError(t, err) + for _, ri := range runInfos { + t.Logf("RunInfo: %s", ri.Message) + } + require.Equal(t, 0, len(runInfos)) + + rjs, err := workflow_v2.LoadRunJobsByRunID(context.TODO(), db, wr.ID, wr.RunAttempt) + require.NoError(t, err) + require.Equal(t, 2, len(rjs)) + for _, rj := range rjs { + require.Equal(t, "deploy", rj.JobID) + switch rj.Matrix["region"] { + case "region1": + require.Equal(t, sdk.V2WorkflowRunJobStatusSuccess, rj.Status) + case "region2": + require.Equal(t, sdk.V2WorkflowRunJobStatusWaiting, rj.Status) + default: + t.Fatalf("unexpected matrix permutation %v", rj.Matrix) + } + } +} diff --git a/engine/api/v2_workflow_run_engine_test.go b/engine/api/v2_workflow_run_engine_test.go index da0ea85507..0273735490 100644 --- a/engine/api/v2_workflow_run_engine_test.go +++ b/engine/api/v2_workflow_run_engine_test.go @@ -2069,6 +2069,11 @@ spec: require.True(t, sm2) require.Len(t, wrDB.WorkflowData.Workflow.Concurrencies, 1) require.Equal(t, "mymatrixconcu", wrDB.WorkflowData.Workflow.Concurrencies[0].Name) + + // All jobs injected from the matrix permutations share the template's complete name as provenance + for _, j := range []sdk.V2Job{deploy1, smoke1, deploy2, smoke2} { + require.Equal(t, fmt.Sprintf("%s/%s/%s/jobtmpl@refs/heads/master", proj.Key, vcsServer.Name, repo.Name), j.From) + } } func TestCreateJobsFromTemplatedMatrix_WithStage(t *testing.T) { diff --git a/engine/api/v2_workflow_template.go b/engine/api/v2_workflow_template.go index 29fdaa44fa..fbaab9cf9a 100644 --- a/engine/api/v2_workflow_template.go +++ b/engine/api/v2_workflow_template.go @@ -18,7 +18,7 @@ func (api *API) postGenerateWorkflowFromTemplateHandler() ([]service.RbacChecker return err } - errs := tmplGen.Template.Lint() + errs := tmplGen.Template.LintYamlDefinition() if len(errs) > 0 { errorsS := "" for _, e := range errs { diff --git a/sdk/entity.go b/sdk/entity.go index e341b5dba0..dbf40ed18b 100644 --- a/sdk/entity.go +++ b/sdk/entity.go @@ -141,7 +141,8 @@ func GetManageRoleByEntity(entityType string) (string, error) { } type Lintable interface { - Lint() []error + // LintYamlDefinition validates a user-authored yaml definition file. + LintYamlDefinition() []error GetName() string } diff --git a/sdk/v2_action.go b/sdk/v2_action.go index 78b7838718..a4c066aa87 100644 --- a/sdk/v2_action.go +++ b/sdk/v2_action.go @@ -65,7 +65,7 @@ func (a *V2Action) Clean() { } } -func (a V2Action) Lint() []error { +func (a V2Action) LintYamlDefinition() []error { actionSchema := GetActionJsonSchema(nil) actionSchemaS, err := actionSchema.MarshalJSON() if err != nil { diff --git a/sdk/v2_worker_model.go b/sdk/v2_worker_model.go index fedbbfb8b6..4285a40bfa 100644 --- a/sdk/v2_worker_model.go +++ b/sdk/v2_worker_model.go @@ -43,7 +43,7 @@ func (wm V2WorkerModel) GetName() string { return wm.Name } -func (wm V2WorkerModel) Lint() []error { +func (wm V2WorkerModel) LintYamlDefinition() []error { workerModelSchema := GetWorkerModelJsonSchema() workerModelSchemaS, err := workerModelSchema.MarshalJSON() if err != nil { diff --git a/sdk/v2_worker_model_test.go b/sdk/v2_worker_model_test.go index decb7c6c6e..9546a4c8fb 100644 --- a/sdk/v2_worker_model_test.go +++ b/sdk/v2_worker_model_test.go @@ -24,7 +24,7 @@ func TestWorkerDockerModelWithoutImage(t *testing.T) { var dockerModel V2WorkerModel require.NoError(t, yaml.Unmarshal([]byte(dockerWM), &dockerModel)) - err := dockerModel.Lint() + err := dockerModel.LintYamlDefinition() require.NotEqual(t, 0, len(err)) require.Contains(t, fmt.Sprintf("%v", err), "image is required") } @@ -45,7 +45,7 @@ func TestWorkerDockerModelWrongType(t *testing.T) { var dockerModel V2WorkerModel require.NoError(t, yaml.Unmarshal([]byte(dockerWM), &dockerModel)) - err := dockerModel.Lint() + err := dockerModel.LintYamlDefinition() require.NotEqual(t, 0, len(err)) require.Contains(t, fmt.Sprintf("%v", err), "type must be one of the following") } @@ -63,5 +63,5 @@ func TestWorkerDockerModelOK(t *testing.T) { var dockerModel V2WorkerModel require.NoError(t, yaml.Unmarshal([]byte(dockerWM), &dockerModel)) - require.Nil(t, dockerModel.Lint()) + require.Nil(t, dockerModel.LintYamlDefinition()) } diff --git a/sdk/v2_workflow.go b/sdk/v2_workflow.go index fb138f221e..f883037631 100644 --- a/sdk/v2_workflow.go +++ b/sdk/v2_workflow.go @@ -357,6 +357,13 @@ func (j V2Job) Copy() V2Job { return new } +// NeedsTemplateResolution returns true when the job references a job template +// (`from`) that has not been resolved yet. A job resolved from a template keeps +// `from` as provenance alongside its concrete content (steps or runs-on). +func (j V2Job) NeedsTemplateResolution() bool { + return j.From != "" && len(j.Steps) == 0 && j.RunsOn.Model == "" +} + type V2JobRunsOn struct { Model string `json:"model" jsonschema_description:"Worker model name to use for the job"` Memory string `json:"memory" jsonschema_description:"Amount of memory to use for the job"` @@ -574,13 +581,46 @@ func (w V2Workflow) GetName() string { return w.Name } -func (w V2Workflow) Lint() []error { - // Before anything, check if workflow inherits from a workflow template. - // Skip other checks if it is the case. +// LintYamlDefinition validates a user-authored workflow definition, as written in a +// repository file. It enforces rules that only make sense on user input, such as: +// a job referencing a template (`from`) cannot declare steps or runs-on. +func (w V2Workflow) LintYamlDefinition() []error { + if w.From != "" { + return nil + } + errs := w.lintStructure() + errs = append(errs, w.checkJobsTemplateReference()...) + if len(errs) > 0 { + return errs + } + return nil +} + +// LintWorkflowRunData validates the structural consistency of the workflow held by a +// workflow run. Such a workflow has been mutated by the run engine: jobs resolved from +// a job template keep `from` as provenance alongside their concrete content, which is +// invalid in a user file but expected here. +func (w V2Workflow) LintWorkflowRunData() []error { if w.From != "" { return nil } + return w.lintStructure() +} + +// checkJobsTemplateReference returns an error for each job combining `from` with +// concrete content (steps or runs-on). +func (w V2Workflow) checkJobsTemplateReference() []error { + var errs []error + for jobID, j := range w.Jobs { + if j.From != "" && !j.NeedsTemplateResolution() { + errs = append(errs, NewErrorFrom(ErrInvalidData, "workflow %s job %s: from cannot be combined with steps or runs-on", w.Name, jobID)) + } + } + return errs +} +// lintStructure holds the checks shared by LintYamlDefinition and LintWorkflowRunData. +func (w V2Workflow) lintStructure() []error { errs := w.CheckStageAndJobNeeds() for _, j := range w.Jobs { diff --git a/sdk/v2_workflow_template.go b/sdk/v2_workflow_template.go index 9a0c92bc7a..14c556d31e 100644 --- a/sdk/v2_workflow_template.go +++ b/sdk/v2_workflow_template.go @@ -45,7 +45,7 @@ type V2WorkflowTemplateGenerateResponse struct { Workflow string `json:"workflow" cli:"workflow"` } -func (wt V2WorkflowTemplate) Lint() (errs []error) { +func (wt V2WorkflowTemplate) LintYamlDefinition() (errs []error) { schema := GetWorkflowTemplateJsonSchema() rawSchema, err := schema.MarshalJSON() if err != nil { diff --git a/sdk/v2_workflow_test.go b/sdk/v2_workflow_test.go index 05dd679d85..8e6b2ba062 100644 --- a/sdk/v2_workflow_test.go +++ b/sdk/v2_workflow_test.go @@ -8,6 +8,85 @@ import ( "github.com/stretchr/testify/require" ) +func TestV2JobNeedsTemplateResolution(t *testing.T) { + tests := []struct { + name string + job V2Job + want bool + }{ + { + name: "from only is an unresolved reference", + job: V2Job{From: ".cds/workflow-templates/build.yml"}, + want: true, + }, + { + name: "from with matrix but no content is an unresolved reference", + job: V2Job{ + From: "proj/vcs/my/repo/tmpl@refs/heads/main", + Strategy: &V2JobStrategy{Matrix: map[string]interface{}{"region": []string{"r1", "r2"}}}, + }, + want: true, + }, + { + name: "from with steps is already resolved", + job: V2Job{From: "tmpl", Steps: []ActionStep{{Run: "echo"}}}, + want: false, + }, + { + name: "from with runs-on is already resolved", + job: V2Job{From: "tmpl", RunsOn: V2JobRunsOn{Model: "docker-debian"}}, + want: false, + }, + { + name: "job without from has nothing to resolve", + job: V2Job{Steps: []ActionStep{{Run: "echo"}}}, + want: false, + }, + { + name: "empty job without from has nothing to resolve", + job: V2Job{}, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, tt.job.NeedsTemplateResolution()) + }) + } +} + +func TestV2WorkflowLintJobTemplateReference(t *testing.T) { + jobFromOnly := V2Job{From: ".cds/workflow-templates/build.yml"} + jobFromAndRunsOn := V2Job{From: ".cds/workflow-templates/build.yml", RunsOn: V2JobRunsOn{Model: "docker-debian"}} + jobFromAndSteps := V2Job{From: ".cds/workflow-templates/build.yml", Steps: []ActionStep{{Run: "echo"}}} + + t.Run("yaml definition accepts a job template reference", func(t *testing.T) { + w := V2Workflow{Name: "w", Jobs: map[string]V2Job{"myjob": jobFromOnly}} + require.Empty(t, w.LintYamlDefinition()) + }) + t.Run("yaml definition rejects from combined with runs-on", func(t *testing.T) { + w := V2Workflow{Name: "w", Jobs: map[string]V2Job{"myjob": jobFromAndRunsOn}} + errs := w.LintYamlDefinition() + require.Len(t, errs, 1) + require.Contains(t, errs[0].Error(), "from cannot be combined with steps or runs-on") + }) + t.Run("yaml definition rejects from combined with steps", func(t *testing.T) { + w := V2Workflow{Name: "w", Jobs: map[string]V2Job{"myjob": jobFromAndSteps}} + errs := w.LintYamlDefinition() + require.Len(t, errs, 1) + require.Contains(t, errs[0].Error(), "from cannot be combined with steps or runs-on") + }) + t.Run("workflow run data accepts from as provenance on resolved jobs", func(t *testing.T) { + w := V2Workflow{Name: "w", Jobs: map[string]V2Job{"myjob": jobFromAndRunsOn}} + require.Empty(t, w.LintWorkflowRunData()) + }) + t.Run("workflow from a template skips both lints", func(t *testing.T) { + w := V2Workflow{Name: "w", From: "proj/vcs/my/repo/tmpl", Jobs: map[string]V2Job{"myjob": jobFromAndRunsOn}} + require.Empty(t, w.LintYamlDefinition()) + require.Empty(t, w.LintWorkflowRunData()) + }) +} + func TestUnmarshalV2Job(t *testing.T) { src := `jobs: myFirstJob: