diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 5e59c059c..c4437c856 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -31,7 +31,8 @@ jobs: if: > github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' || - (github.event_name == 'pull_request_target' && contains(fromJSON('["zakisk", "infernus01", "savitaashture", "chmouel", "vdemeester", "PuneetPunamiya", "enarha", "aThorp96", "sm43", "waveywaves", "dependabot[bot]"]'), github.event.pull_request.user.login)) + (github.event_name == 'pull_request_target' && + contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR", "CONTRIBUTOR"]'), github.event.pull_request.author_association)) concurrency: group: ${{ github.workflow }}-${{ matrix.provider }}-${{ github.event.pull_request.number || github.ref_name }} cancel-in-progress: true diff --git a/config/302-pac-configmap.yaml b/config/302-pac-configmap.yaml index 6fd0c1bea..2416a3923 100644 --- a/config/302-pac-configmap.yaml +++ b/config/302-pac-configmap.yaml @@ -139,12 +139,18 @@ data: # namespace and repository CR, supported only with GitHub App auto-configure-new-github-repo: "false" - # add a template to generate name for namespace for your auto configured - # github repo supported fields are repo_owner, repo_name eg. if defined as - # `{{repo_owner}}-{{repo_name}}-ci`, then namespace generated for repository - # https://github.com/owner/repo will be `owner-repo-ci` + # Add a template to generate the name for a Repository CR for an auto-configured + # GitHub repository. Supported fields are `repo_owner` and `repo_name`. e.g., + # if defined as `{{repo_owner}}-{{repo_name}}-repo-cr`, the generated CR name for + # https://github.com/owner/test will be `owner-test-repo-cr` auto-configure-repo-namespace-template: "" + # add a template to generate name for repository for your auto configured + # github repo. supported fields are repo_owner, repo_name eg. if defined as + # `{{repo_owner}}-{{repo_name}}-repo-cr`, then repository CR generated for git repository + # https://github.com/owner/test will be `owner-test-repo-cr` + auto-configure-repo-repository-template: "" + # Enable or disable the feature to rerun the CI if push event happens on # a pull request # diff --git a/docs/content/docs/install/settings.md b/docs/content/docs/install/settings.md index 71afba243..57f35d42d 100644 --- a/docs/content/docs/install/settings.md +++ b/docs/content/docs/install/settings.md @@ -124,6 +124,19 @@ There is a few things you can configure through the config map `https://github.com/owner/repo` will be `owner-repo-ci` +* `auto-configure-repo-repository-template` + + If `auto-configure-new-github-repo` is enabled then you can provide a template + for generating the name for your new repository custom resource. By default, the repository custom resource name will be generated using this format `{{repo_name}}-repo-cr`. + + You can override the default using the following variables + + * `{{repo_owner}}`: The repository owner. + * `{{repo_name}}`: The repository name. + For example, if the template is defined as `{{repo_owner}}-{{repo_name}}-repo-cr`, + then the Repository CR name generated for the repository + `https://github.com/owner/test` will be `owner-test-repo-cr` + * `remember-ok-to-test` If `remember-ok-to-test` is true then if `ok-to-test` is done on pull request then in diff --git a/pkg/apis/pipelinesascode/keys/keys.go b/pkg/apis/pipelinesascode/keys/keys.go index f61b729bf..10febd501 100644 --- a/pkg/apis/pipelinesascode/keys/keys.go +++ b/pkg/apis/pipelinesascode/keys/keys.go @@ -23,43 +23,44 @@ import ( ) const ( - ControllerInfo = pipelinesascode.GroupName + "/controller-info" - Task = pipelinesascode.GroupName + "/task" - Pipeline = pipelinesascode.GroupName + "/pipeline" - URLOrg = pipelinesascode.GroupName + "/url-org" - URLRepository = pipelinesascode.GroupName + "/url-repository" - SHA = pipelinesascode.GroupName + "/sha" - Sender = pipelinesascode.GroupName + "/sender" - EventType = pipelinesascode.GroupName + "/event-type" - Branch = pipelinesascode.GroupName + "/branch" - SourceBranch = pipelinesascode.GroupName + "/source-branch" - Repository = pipelinesascode.GroupName + "/repository" - GitProvider = pipelinesascode.GroupName + "/git-provider" - State = pipelinesascode.GroupName + "/state" - ShaTitle = pipelinesascode.GroupName + "/sha-title" - ShaURL = pipelinesascode.GroupName + "/sha-url" - RepoURL = pipelinesascode.GroupName + "/repo-url" - SourceRepoURL = pipelinesascode.GroupName + "/source-repo-url" - PullRequest = pipelinesascode.GroupName + "/pull-request" - InstallationID = pipelinesascode.GroupName + "/installation-id" - GHEURL = pipelinesascode.GroupName + "/ghe-url" - SourceProjectID = pipelinesascode.GroupName + "/source-project-id" - TargetProjectID = pipelinesascode.GroupName + "/target-project-id" - OriginalPRName = pipelinesascode.GroupName + "/original-prname" - GitAuthSecret = pipelinesascode.GroupName + "/git-auth-secret" - CheckRunID = pipelinesascode.GroupName + "/check-run-id" - OnEvent = pipelinesascode.GroupName + "/on-event" - OnComment = pipelinesascode.GroupName + "/on-comment" - OnTargetBranch = pipelinesascode.GroupName + "/on-target-branch" - OnPathChange = pipelinesascode.GroupName + "/on-path-change" - OnLabel = pipelinesascode.GroupName + "/on-label" - OnPathChangeIgnore = pipelinesascode.GroupName + "/on-path-change-ignore" - OnCelExpression = pipelinesascode.GroupName + "/on-cel-expression" - TargetNamespace = pipelinesascode.GroupName + "/target-namespace" - MaxKeepRuns = pipelinesascode.GroupName + "/max-keep-runs" - CancelInProgress = pipelinesascode.GroupName + "/cancel-in-progress" - LogURL = pipelinesascode.GroupName + "/log-url" - ExecutionOrder = pipelinesascode.GroupName + "/execution-order" + ControllerInfo = pipelinesascode.GroupName + "/controller-info" + Task = pipelinesascode.GroupName + "/task" + Pipeline = pipelinesascode.GroupName + "/pipeline" + URLOrg = pipelinesascode.GroupName + "/url-org" + URLRepository = pipelinesascode.GroupName + "/url-repository" + SHA = pipelinesascode.GroupName + "/sha" + Sender = pipelinesascode.GroupName + "/sender" + EventType = pipelinesascode.GroupName + "/event-type" + Branch = pipelinesascode.GroupName + "/branch" + SourceBranch = pipelinesascode.GroupName + "/source-branch" + Repository = pipelinesascode.GroupName + "/repository" + GitProvider = pipelinesascode.GroupName + "/git-provider" + State = pipelinesascode.GroupName + "/state" + ShaTitle = pipelinesascode.GroupName + "/sha-title" + ShaURL = pipelinesascode.GroupName + "/sha-url" + RepoURL = pipelinesascode.GroupName + "/repo-url" + SourceRepoURL = pipelinesascode.GroupName + "/source-repo-url" + PullRequest = pipelinesascode.GroupName + "/pull-request" + InstallationID = pipelinesascode.GroupName + "/installation-id" + GHEURL = pipelinesascode.GroupName + "/ghe-url" + SourceProjectID = pipelinesascode.GroupName + "/source-project-id" + TargetProjectID = pipelinesascode.GroupName + "/target-project-id" + OriginalPRName = pipelinesascode.GroupName + "/original-prname" + GitAuthSecret = pipelinesascode.GroupName + "/git-auth-secret" + CheckRunID = pipelinesascode.GroupName + "/check-run-id" + OnEvent = pipelinesascode.GroupName + "/on-event" + OnComment = pipelinesascode.GroupName + "/on-comment" + OnTargetBranch = pipelinesascode.GroupName + "/on-target-branch" + OnPathChange = pipelinesascode.GroupName + "/on-path-change" + OnLabel = pipelinesascode.GroupName + "/on-label" + OnPathChangeIgnore = pipelinesascode.GroupName + "/on-path-change-ignore" + OnCelExpression = pipelinesascode.GroupName + "/on-cel-expression" + TargetNamespace = pipelinesascode.GroupName + "/target-namespace" + MaxKeepRuns = pipelinesascode.GroupName + "/max-keep-runs" + CancelInProgress = pipelinesascode.GroupName + "/cancel-in-progress" + LogURL = pipelinesascode.GroupName + "/log-url" + ExecutionOrder = pipelinesascode.GroupName + "/execution-order" + SCMReportingPLRStarted = pipelinesascode.GroupName + "/scm-reporting-plr-started" // PublicGithubAPIURL default is "https://api.github.com" but it can be overridden by X-GitHub-Enterprise-Host header. PublicGithubAPIURL = "https://api.github.com" GithubApplicationID = "github-application-id" diff --git a/pkg/kubeinteraction/labels.go b/pkg/kubeinteraction/labels.go index 846d6a2e0..50fcf2f85 100644 --- a/pkg/kubeinteraction/labels.go +++ b/pkg/kubeinteraction/labels.go @@ -55,7 +55,6 @@ func AddLabelsAndAnnotations(event *info.Event, pipelineRun *tektonv1.PipelineRu keys.SourceBranch: event.HeadBranch, keys.Repository: repo.GetName(), keys.GitProvider: providerConfig.Name, - keys.State: StateStarted, keys.ControllerInfo: fmt.Sprintf(`{"name":"%s","configmap":"%s","secret":"%s", "gRepo": "%s"}`, paramsinfo.Controller.Name, paramsinfo.Controller.Configmap, paramsinfo.Controller.Secret, paramsinfo.Controller.GlobalRepository), } diff --git a/pkg/params/settings/config.go b/pkg/params/settings/config.go index 86aa1659d..6c853fd56 100644 --- a/pkg/params/settings/config.go +++ b/pkg/params/settings/config.go @@ -48,16 +48,17 @@ type HubCatalog struct { // if there is a change performed on the default value, // update the same on "config/302-pac-configmap.yaml". type Settings struct { - ApplicationName string `default:"Pipelines as Code CI" json:"application-name"` - HubCatalogs *sync.Map - RemoteTasks bool `default:"true" json:"remote-tasks"` - MaxKeepRunsUpperLimit int `json:"max-keep-run-upper-limit"` - DefaultMaxKeepRuns int `json:"default-max-keep-runs"` - BitbucketCloudCheckSourceIP bool `default:"true" json:"bitbucket-cloud-check-source-ip"` - BitbucketCloudAdditionalSourceIP string `json:"bitbucket-cloud-additional-source-ip"` - TektonDashboardURL string `json:"tekton-dashboard-url"` - AutoConfigureNewGitHubRepo bool `default:"false" json:"auto-configure-new-github-repo"` - AutoConfigureRepoNamespaceTemplate string `json:"auto-configure-repo-namespace-template"` + ApplicationName string `default:"Pipelines as Code CI" json:"application-name"` + HubCatalogs *sync.Map + RemoteTasks bool `default:"true" json:"remote-tasks"` + MaxKeepRunsUpperLimit int `json:"max-keep-run-upper-limit"` + DefaultMaxKeepRuns int `json:"default-max-keep-runs"` + BitbucketCloudCheckSourceIP bool `default:"true" json:"bitbucket-cloud-check-source-ip"` + BitbucketCloudAdditionalSourceIP string `json:"bitbucket-cloud-additional-source-ip"` + TektonDashboardURL string `json:"tekton-dashboard-url"` + AutoConfigureNewGitHubRepo bool `default:"false" json:"auto-configure-new-github-repo"` + AutoConfigureRepoNamespaceTemplate string `json:"auto-configure-repo-namespace-template"` + AutoConfigureRepoRepositoryTemplate string `json:"auto-configure-repo-repository-template"` SecretAutoCreation bool `default:"true" json:"secret-auto-create"` SecretGHAppRepoScoped bool `default:"true" json:"secret-github-app-token-scoped"` diff --git a/pkg/params/settings/config_test.go b/pkg/params/settings/config_test.go index 64ae79c0c..707fdb646 100644 --- a/pkg/params/settings/config_test.go +++ b/pkg/params/settings/config_test.go @@ -52,55 +52,57 @@ func TestSyncConfig(t *testing.T) { { name: "override values", configMap: map[string]string{ - "application-name": "pac-pac", - "remote-tasks": "false", - "max-keep-run-upper-limit": "10", - "default-max-keep-runs": "5", - "bitbucket-cloud-check-source-ip": "false", - "bitbucket-cloud-additional-source-ip": "some-ip", - "tekton-dashboard-url": "https://tekton-dashboard", - "auto-configure-new-github-repo": "true", - "auto-configure-repo-namespace-template": "template", - "secret-auto-create": "false", - "secret-github-app-token-scoped": "false", - "secret-github-app-scope-extra-repos": "extra-repos", - "error-log-snippet": "false", - "error-detection-from-container-logs": "false", - "error-detection-max-number-of-lines": "100", - "error-detection-simple-regexp": "^(?P[^:]*):(?P[0-9]+):(?P[0-9]+)?([ ]*)?(?P.*)", - "custom-console-name": "custom-console", - "custom-console-url": "https://custom-console", - "custom-console-url-pr-details": "https://custom-console-pr-details", - "custom-console-url-pr-tasklog": "https://custom-console-pr-tasklog", - "custom-console-url-namespace": "https://custom-console-namespace", - "remember-ok-to-test": "false", - "skip-push-event-for-pr-commits": "true", + "application-name": "pac-pac", + "remote-tasks": "false", + "max-keep-run-upper-limit": "10", + "default-max-keep-runs": "5", + "bitbucket-cloud-check-source-ip": "false", + "bitbucket-cloud-additional-source-ip": "some-ip", + "tekton-dashboard-url": "https://tekton-dashboard", + "auto-configure-new-github-repo": "true", + "auto-configure-repo-namespace-template": "template", + "auto-configure-repo-repository-template": "template", + "secret-auto-create": "false", + "secret-github-app-token-scoped": "false", + "secret-github-app-scope-extra-repos": "extra-repos", + "error-log-snippet": "false", + "error-detection-from-container-logs": "false", + "error-detection-max-number-of-lines": "100", + "error-detection-simple-regexp": "^(?P[^:]*):(?P[0-9]+):(?P[0-9]+)?([ ]*)?(?P.*)", + "custom-console-name": "custom-console", + "custom-console-url": "https://custom-console", + "custom-console-url-pr-details": "https://custom-console-pr-details", + "custom-console-url-pr-tasklog": "https://custom-console-pr-tasklog", + "custom-console-url-namespace": "https://custom-console-namespace", + "remember-ok-to-test": "false", + "skip-push-event-for-pr-commits": "true", }, expectedStruct: Settings{ - ApplicationName: "pac-pac", - HubCatalogs: nil, - RemoteTasks: false, - MaxKeepRunsUpperLimit: 10, - DefaultMaxKeepRuns: 5, - BitbucketCloudCheckSourceIP: false, - BitbucketCloudAdditionalSourceIP: "some-ip", - TektonDashboardURL: "https://tekton-dashboard", - AutoConfigureNewGitHubRepo: true, - AutoConfigureRepoNamespaceTemplate: "template", - SecretAutoCreation: false, - SecretGHAppRepoScoped: false, - SecretGhAppTokenScopedExtraRepos: "extra-repos", - ErrorLogSnippet: false, - ErrorDetection: false, - ErrorDetectionNumberOfLines: 100, - ErrorDetectionSimpleRegexp: "^(?P[^:]*):(?P[0-9]+):(?P[0-9]+)?([ ]*)?(?P.*)", - CustomConsoleName: "custom-console", - CustomConsoleURL: "https://custom-console", - CustomConsolePRdetail: "https://custom-console-pr-details", - CustomConsolePRTaskLog: "https://custom-console-pr-tasklog", - CustomConsoleNamespaceURL: "https://custom-console-namespace", - RememberOKToTest: false, - SkipPushEventForPRCommits: true, + ApplicationName: "pac-pac", + HubCatalogs: nil, + RemoteTasks: false, + MaxKeepRunsUpperLimit: 10, + DefaultMaxKeepRuns: 5, + BitbucketCloudCheckSourceIP: false, + BitbucketCloudAdditionalSourceIP: "some-ip", + TektonDashboardURL: "https://tekton-dashboard", + AutoConfigureNewGitHubRepo: true, + AutoConfigureRepoNamespaceTemplate: "template", + AutoConfigureRepoRepositoryTemplate: "template", + SecretAutoCreation: false, + SecretGHAppRepoScoped: false, + SecretGhAppTokenScopedExtraRepos: "extra-repos", + ErrorLogSnippet: false, + ErrorDetection: false, + ErrorDetectionNumberOfLines: 100, + ErrorDetectionSimpleRegexp: "^(?P[^:]*):(?P[0-9]+):(?P[0-9]+)?([ ]*)?(?P.*)", + CustomConsoleName: "custom-console", + CustomConsoleURL: "https://custom-console", + CustomConsolePRdetail: "https://custom-console-pr-details", + CustomConsolePRTaskLog: "https://custom-console-pr-tasklog", + CustomConsoleNamespaceURL: "https://custom-console-namespace", + RememberOKToTest: false, + SkipPushEventForPRCommits: true, }, }, { diff --git a/pkg/pipelineascode/pipelineascode.go b/pkg/pipelineascode/pipelineascode.go index 4a1bdcbe4..c0384b9f7 100644 --- a/pkg/pipelineascode/pipelineascode.go +++ b/pkg/pipelineascode/pipelineascode.go @@ -176,14 +176,10 @@ func (p *PacRun) startPR(ctx context.Context, match matcher.Match) (*tektonv1.Pi p.logger.Errorf("Error adding labels/annotations to PipelineRun '%s' in namespace '%s': %v", match.PipelineRun.GetName(), match.Repo.GetNamespace(), err) } - // if concurrency is defined then start the pipelineRun in pending state and - // state as queued + // if concurrency is defined then start the pipelineRun in pending state if match.Repo.Spec.ConcurrencyLimit != nil && *match.Repo.Spec.ConcurrencyLimit != 0 { // pending status match.PipelineRun.Spec.Status = tektonv1.PipelineRunSpecStatusPending - // pac state as queued - match.PipelineRun.Labels[keys.State] = kubeinteraction.StateQueued - match.PipelineRun.Annotations[keys.State] = kubeinteraction.StateQueued } // Create the actual pipelineRun @@ -240,23 +236,31 @@ func (p *PacRun) startPR(ctx context.Context, match matcher.Match) (*tektonv1.Pi OriginalPipelineRunName: pr.GetAnnotations()[keys.OriginalPRName], } + // Patch the pipelineRun with the appropriate annotations and labels. + // Set the state so the watcher will continue with reconciling the pipelineRun + // The watcher reconciles only pipelineRuns that has the state annotation. patchAnnotations := map[string]string{} patchLabels := map[string]string{} whatPatching := "" // if pipelineRun is in pending state then report status as queued + // The pipelineRun can be pending because of PAC's concurrency limit or because of an external mutatingwebhook if pr.Spec.Status == tektonv1.PipelineRunSpecStatusPending { status.Status = queuedStatus if status.Text, err = mt.MakeTemplate(p.vcx.GetTemplate(provider.QueueingPipelineType)); err != nil { return nil, fmt.Errorf("cannot create message template: %w", err) } - // If the PipelineRun is in the "queued" state, add the appropriate label and annotation. - // These are later used by the watcher to determine whether the PipelineRun status - // should be reported back to the Git provider. We do add the `state` annotations and label when - // concurrency is enabled but this would happen when PipelineRun's status has been changed by - // the other controller and PaC is not aware of that change. whatPatching = "annotations.state and labels.state" patchAnnotations[keys.State] = kubeinteraction.StateQueued patchLabels[keys.State] = kubeinteraction.StateQueued + } else { + // Mark that the start will be reported to the Git provider + patchAnnotations[keys.SCMReportingPLRStarted] = "true" + patchAnnotations[keys.State] = kubeinteraction.StateStarted + patchLabels[keys.State] = kubeinteraction.StateStarted + whatPatching = fmt.Sprintf( + "annotation.%s and annotations.state and labels.state", + keys.SCMReportingPLRStarted, + ) } if err := p.vcx.CreateStatus(ctx, p.event, status); err != nil { @@ -278,6 +282,20 @@ func (p *PacRun) startPR(ctx context.Context, match matcher.Match) (*tektonv1.Pi // unneeded SIGSEGV's return pr, fmt.Errorf("cannot patch pipelinerun %s: %w", pr.GetGenerateName(), err) } + currentReason := "" + if len(pr.Status.GetConditions()) > 0 { + currentReason = pr.Status.GetConditions()[0].GetReason() + } + + p.logger.Infof("PipelineRun %s/%s patched successfully - Spec.Status: %s, State annotation: '%s', SCMReportingPLRStarted annotation: '%s', Status reason: '%s', Git provider status: '%s', Patched: %s", + pr.GetNamespace(), + pr.GetName(), + pr.Spec.Status, + pr.GetAnnotations()[keys.State], + pr.GetAnnotations()[keys.SCMReportingPLRStarted], + currentReason, + status.Status, + whatPatching) } return pr, nil diff --git a/pkg/pipelineascode/pipelineascode_test.go b/pkg/pipelineascode/pipelineascode_test.go index 9b9f23e5c..17ed9ba2c 100644 --- a/pkg/pipelineascode/pipelineascode_test.go +++ b/pkg/pipelineascode/pipelineascode_test.go @@ -718,10 +718,19 @@ func TestRun(t *testing.T) { secretName, ok := pr.GetAnnotations()[keys.GitAuthSecret] assert.Assert(t, ok, "Cannot find secret %s on annotations", secretName) } - if tt.concurrencyLimit > 0 || pr.Spec.Status == pipelinev1.PipelineRunSpecStatusPending { + if pr.Spec.Status == pipelinev1.PipelineRunSpecStatusPending { state, ok := pr.GetAnnotations()[keys.State] assert.Assert(t, ok, "State hasn't been set on PR", state) assert.Equal(t, state, kubeinteraction.StateQueued) + + // When PipelineRun is queued, SCMReportingPLRStarted should not be set + _, scmStartedExists := pr.GetAnnotations()[keys.SCMReportingPLRStarted] + assert.Assert(t, !scmStartedExists, "SCMReportingPLRStarted should not be set for queued PipelineRuns") + } else { + // When PipelineRun is not queued, SCMReportingPLRStarted should be set to "true" + scmStarted, scmStartedExists := pr.GetAnnotations()[keys.SCMReportingPLRStarted] + assert.Assert(t, scmStartedExists, "SCMReportingPLRStarted should be set for non-queued PipelineRuns") + assert.Equal(t, scmStarted, "true", "SCMReportingPLRStarted should be 'true'") } } } diff --git a/pkg/provider/github/repository.go b/pkg/provider/github/repository.go index de320bffc..0f82685f1 100644 --- a/pkg/provider/github/repository.go +++ b/pkg/provider/github/repository.go @@ -19,7 +19,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -const defaultNsTemplate = "%v-pipelines" +const ( + defaultNsTemplate = "%v-pipelines" + defaultRepoTemplate = "%v-repo-cr" +) func ConfigureRepository(ctx context.Context, run *params.Run, req *http.Request, payload string, pacInfo *info.PacOpts, logger *zap.SugaredLogger) (bool, bool, error) { // check if repo auto configuration is enabled @@ -48,7 +51,9 @@ func ConfigureRepository(ctx context.Context, run *params.Run, req *http.Request } logger.Infof("github: configuring repository cr for repo: %v", repoEvent.Repo.GetHTMLURL()) - if err := createRepository(ctx, pacInfo.AutoConfigureRepoNamespaceTemplate, run.Clients, repoEvent, logger); err != nil { + nsTemplate := pacInfo.AutoConfigureRepoNamespaceTemplate + repoTemplate := pacInfo.AutoConfigureRepoRepositoryTemplate + if err := createRepository(ctx, nsTemplate, repoTemplate, run.Clients, repoEvent, logger); err != nil { logger.Errorf("failed repository creation: %v", err) return true, true, err } @@ -56,8 +61,8 @@ func ConfigureRepository(ctx context.Context, run *params.Run, req *http.Request return true, true, nil } -func createRepository(ctx context.Context, nsTemplate string, clients clients.Clients, gitEvent *github.RepositoryEvent, logger *zap.SugaredLogger) error { - repoNsName, err := generateNamespaceName(nsTemplate, gitEvent) +func createRepository(ctx context.Context, nsTemplate, repoTemplate string, clients clients.Clients, gitEvent *github.RepositoryEvent, logger *zap.SugaredLogger) error { + repoNsName, repoCRName, err := generateNamespaceAndRepositoryName(nsTemplate, repoTemplate, gitEvent) if err != nil { return fmt.Errorf("failed to generate namespace for repo: %w", err) } @@ -84,7 +89,7 @@ func createRepository(ctx context.Context, nsTemplate string, clients clients.Cl // create repository repo := &v1alpha1.Repository{ ObjectMeta: metav1.ObjectMeta{ - Name: repoNsName, + Name: repoCRName, Namespace: repoNsName, }, Spec: v1alpha1.RepositorySpec{ @@ -100,19 +105,29 @@ func createRepository(ctx context.Context, nsTemplate string, clients clients.Cl return nil } -func generateNamespaceName(nsTemplate string, gitEvent *github.RepositoryEvent) (string, error) { +func generateNamespaceAndRepositoryName(nsTemplate, repoTemplate string, gitEvent *github.RepositoryEvent) (string, string, error) { repoOwner, repoName, err := formatting.GetRepoOwnerSplitted(gitEvent.Repo.GetHTMLURL()) if err != nil { - return "", fmt.Errorf("failed to parse git repo url: %w", err) + return "", "", fmt.Errorf("failed to parse git repo url: %w", err) + } + + nsName := "" + repoCRName := "" + placeholders := map[string]string{ + "repo_owner": repoOwner, + "repo_name": repoName, } if nsTemplate == "" { - return fmt.Sprintf(defaultNsTemplate, repoName), nil + nsName = fmt.Sprintf(defaultNsTemplate, repoName) + } else { + nsName = templates.ReplacePlaceHoldersVariables(nsTemplate, placeholders, nil, http.Header{}, map[string]any{}) } - maptemplate := map[string]string{ - "repo_owner": repoOwner, - "repo_name": repoName, + if repoTemplate == "" { + repoCRName = fmt.Sprintf(defaultRepoTemplate, repoName) + } else { + repoCRName = templates.ReplacePlaceHoldersVariables(repoTemplate, placeholders, nil, http.Header{}, map[string]any{}) } - return templates.ReplacePlaceHoldersVariables(nsTemplate, maptemplate, nil, http.Header{}, map[string]any{}), nil + return nsName, repoCRName, nil } diff --git a/pkg/provider/github/repository_test.go b/pkg/provider/github/repository_test.go index 76a501a99..eca3b7ba5 100644 --- a/pkg/provider/github/repository_test.go +++ b/pkg/provider/github/repository_test.go @@ -39,16 +39,18 @@ func TestConfigureRepository(t *testing.T) { assert.NilError(t, err) tests := []struct { - name string - request *http.Request - eventType string - event []byte - detected bool - configuring bool - wantErr string - expectedNs string - nsTemplate string - testData testclient.Data + name string + request *http.Request + eventType string + event []byte + detected bool + configuring bool + wantErr string + expectedNs string + expectedRepo string + nsTemplate string + repoTemplate string + testData testclient.Data }{ { name: "non supported event", @@ -70,34 +72,49 @@ func TestConfigureRepository(t *testing.T) { testData: testclient.Data{}, }, { - name: "repo create event with no ns template", - event: repoCreateEvent, - eventType: "repository", - detected: true, - configuring: true, - wantErr: "", - expectedNs: "test-repo-pipelines", - testData: testclient.Data{}, + name: "repo create event with no ns template", + event: repoCreateEvent, + eventType: "repository", + detected: true, + configuring: true, + wantErr: "", + expectedNs: "test-repo-pipelines", + expectedRepo: "test-repo-repo-cr", + testData: testclient.Data{}, }, { - name: "repo create event with ns template", - event: repoCreateEvent, - eventType: "repository", - detected: true, - configuring: true, - wantErr: "", - expectedNs: "pac-test-repo-ci", - nsTemplate: "{{repo_owner}}-{{repo_name}}-ci", - testData: testclient.Data{}, + name: "repo create event with ns template", + event: repoCreateEvent, + eventType: "repository", + detected: true, + configuring: true, + wantErr: "", + expectedNs: "pac-test-repo-ci", + expectedRepo: "test-repo-repo-cr", + nsTemplate: "{{repo_owner}}-{{repo_name}}-ci", + testData: testclient.Data{}, }, { - name: "repo create event with ns already exist", - event: repoCreateEvent, - eventType: "repository", - detected: true, - configuring: true, - wantErr: "", - expectedNs: "test-repo-pipelines", + name: "repo create event with repo template", + event: repoCreateEvent, + eventType: "repository", + detected: true, + configuring: true, + wantErr: "", + expectedNs: "test-repo-pipelines", + expectedRepo: "pac-test-repo-cr", + repoTemplate: "{{repo_owner}}-{{repo_name}}-cr", + testData: testclient.Data{}, + }, + { + name: "repo create event with ns already exist", + event: repoCreateEvent, + eventType: "repository", + detected: true, + configuring: true, + wantErr: "", + expectedNs: "test-repo-pipelines", + expectedRepo: "test-repo-repo-cr", testData: testclient.Data{ Namespaces: []*v12.Namespace{ { @@ -127,8 +144,9 @@ func TestConfigureRepository(t *testing.T) { infoPac := &info.PacOpts{ Settings: settings.Settings{ - AutoConfigureNewGitHubRepo: true, - AutoConfigureRepoNamespaceTemplate: tt.nsTemplate, + AutoConfigureNewGitHubRepo: true, + AutoConfigureRepoNamespaceTemplate: tt.nsTemplate, + AutoConfigureRepoRepositoryTemplate: tt.repoTemplate, }, } detected, configuring, err := ConfigureRepository(ctx, run, req, string(tt.event), infoPac, logger) @@ -146,47 +164,76 @@ func TestConfigureRepository(t *testing.T) { assert.NilError(t, err) assert.Equal(t, ns.Name, tt.expectedNs) - repo, err := run.Clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(tt.expectedNs).Get(ctx, tt.expectedNs, v1.GetOptions{}) + repo, err := run.Clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(tt.expectedNs).Get(ctx, tt.expectedRepo, v1.GetOptions{}) assert.NilError(t, err) - assert.Equal(t, repo.Name, tt.expectedNs) + assert.Equal(t, repo.Name, tt.expectedRepo) } }) } } -func TestGetNamespace(t *testing.T) { +func TestGenerateNamespaceAndRepositoryName(t *testing.T) { tests := []struct { - name string - nsTemplate string - gitEvent *github.RepositoryEvent - want string + name string + nsTemplate string + repoTemplate string + gitEvent *github.RepositoryEvent + want string + wantRepo string }{ { - name: "no template", - nsTemplate: "", + name: "no template", + nsTemplate: "", + repoTemplate: "", gitEvent: &github.RepositoryEvent{ Repo: &github.Repository{ HTMLURL: github.Ptr("https://github.com/user/pac"), }, }, - want: "pac-pipelines", + want: "pac-pipelines", + wantRepo: "pac-repo-cr", }, { - name: "template", + name: "template", + nsTemplate: "{{repo_owner}}-{{repo_name}}-ci", + repoTemplate: "{{repo_owner}}-{{repo_name}}-repo-cr", + gitEvent: &github.RepositoryEvent{ + Repo: &github.Repository{ + HTMLURL: github.Ptr("https://github.com/user/pac"), + }, + }, + want: "user-pac-ci", + wantRepo: "user-pac-repo-cr", + }, + { + name: "empty repo template", nsTemplate: "{{repo_owner}}-{{repo_name}}-ci", gitEvent: &github.RepositoryEvent{ Repo: &github.Repository{ HTMLURL: github.Ptr("https://github.com/user/pac"), }, }, - want: "user-pac-ci", + want: "user-pac-ci", + wantRepo: "pac-repo-cr", + }, + { + name: "empty ns template", + repoTemplate: "{{repo_owner}}-{{repo_name}}-repo-cr", + gitEvent: &github.RepositoryEvent{ + Repo: &github.Repository{ + HTMLURL: github.Ptr("https://github.com/user/pac"), + }, + }, + want: "pac-pipelines", + wantRepo: "user-pac-repo-cr", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := generateNamespaceName(tt.nsTemplate, tt.gitEvent) + got, gotRepo, err := generateNamespaceAndRepositoryName(tt.nsTemplate, tt.repoTemplate, tt.gitEvent) assert.NilError(t, err) assert.Equal(t, got, tt.want) + assert.Equal(t, gotRepo, tt.wantRepo) }) } } diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 95aba5dc0..58493f289 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -54,6 +54,20 @@ var ( func (r *Reconciler) ReconcileKind(ctx context.Context, pr *tektonv1.PipelineRun) pkgreconciler.Event { ctx = info.StoreNS(ctx, system.Namespace()) logger := logging.FromContext(ctx).With("namespace", pr.GetNamespace()) + + logger.Debugf("reconciling pipelineRun %s/%s", pr.GetNamespace(), pr.GetName()) + + // make sure we have the latest pipelinerun to reconcile, since there is something updating at the same time + lpr, err := r.run.Clients.Tekton.TektonV1().PipelineRuns(pr.GetNamespace()).Get(ctx, pr.GetName(), metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("cannot get pipelineRun: %w", err) + } + + if lpr.GetResourceVersion() != pr.GetResourceVersion() { + logger.Debugf("Skipping reconciliation, pipelineRun was updated (cached version %s vs fresh version %s)", pr.GetResourceVersion(), lpr.GetResourceVersion()) + return nil + } + // if pipelineRun is in completed or failed state then return state, exist := pr.GetAnnotations()[keys.State] if exist && (state == kubeinteraction.StateCompleted || state == kubeinteraction.StateFailed) { @@ -69,7 +83,12 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, pr *tektonv1.PipelineRun // another controller outside PaC). To maintain consistency between the PipelineRun // status and the Git provider status, we update both the PipelineRun resource and // the corresponding status on the Git provider here. - if reason == string(tektonv1.PipelineRunReasonRunning) && state == kubeinteraction.StateQueued { + scmReportingPLRStarted, exist := pr.GetAnnotations()[keys.SCMReportingPLRStarted] + startReported := exist && scmReportingPLRStarted == "true" + logger.Debugf("pipelineRun %s/%s scmReportingPLRStarted=%v, exist=%v", pr.GetNamespace(), pr.GetName(), startReported, exist) + + if reason == string(tektonv1.PipelineRunReasonRunning) && !startReported { + logger.Infof("pipelineRun %s/%s is running but not yet reported to provider, updating status", pr.GetNamespace(), pr.GetName()) repoName := pr.GetAnnotations()[keys.Repository] repo, err := r.repoLister.Repositories(pr.Namespace).Get(repoName) if err != nil { @@ -77,6 +96,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, pr *tektonv1.PipelineRun } return r.updatePipelineRunToInProgress(ctx, logger, repo, pr) } + logger.Debugf("pipelineRun %s/%s condition not met: reason='%s', startReported=%v", pr.GetNamespace(), pr.GetName(), reason, startReported) // if its a GitHub App pipelineRun PR then process only if check run id is added otherwise wait if _, ok := pr.Annotations[keys.InstallationID]; ok { @@ -95,16 +115,6 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, pr *tektonv1.PipelineRun return nil } - // make sure we have the latest pipelinerun to reconcile, since there is something updating at the same time - lpr, err := r.run.Clients.Tekton.TektonV1().PipelineRuns(pr.GetNamespace()).Get(ctx, pr.GetName(), metav1.GetOptions{}) - if err != nil { - return fmt.Errorf("cannot get pipelineRun: %w", err) - } - - if lpr.GetResourceVersion() != pr.GetResourceVersion() { - return nil - } - // If we have a controllerInfo annotation, then we need to get the // configmap configuration for it // @@ -343,16 +353,24 @@ func (r *Reconciler) updatePipelineRunToInProgress(ctx context.Context, logger * } func (r *Reconciler) updatePipelineRunState(ctx context.Context, logger *zap.SugaredLogger, pr *tektonv1.PipelineRun, state string) (*tektonv1.PipelineRun, error) { + currentState := pr.GetAnnotations()[keys.State] + logger.Infof("updating pipelineRun %v/%v state from %s to %s", pr.GetNamespace(), pr.GetName(), currentState, state) + annotations := map[string]string{ + keys.State: state, + } + if state == kubeinteraction.StateStarted { + annotations[keys.SCMReportingPLRStarted] = "true" + } + mergePatch := map[string]any{ "metadata": map[string]any{ "labels": map[string]string{ keys.State: state, }, - "annotations": map[string]string{ - keys.State: state, - }, + "annotations": annotations, }, } + // if state is started then remove pipelineRun pending status if state == kubeinteraction.StateStarted { mergePatch["spec"] = map[string]any{ diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index e3c4001a1..b001de831 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -299,6 +299,189 @@ func TestUpdatePipelineRunState(t *testing.T) { assert.Equal(t, updatedPR.Annotations[keys.State], tt.state) assert.Equal(t, updatedPR.Spec.Status, tektonv1.PipelineRunSpecStatus("")) + + // Test SCMReportingPLRStarted annotation for started state + if tt.state == kubeinteraction.StateStarted { + scmStarted, exists := updatedPR.GetAnnotations()[keys.SCMReportingPLRStarted] + assert.Assert(t, exists, "SCMReportingPLRStarted annotation should exist when state is started") + assert.Equal(t, scmStarted, "true", "SCMReportingPLRStarted should be 'true'") + } else { + _, exists := updatedPR.GetAnnotations()[keys.SCMReportingPLRStarted] + assert.Assert(t, !exists, "SCMReportingPLRStarted annotation should not exist for non-started states") + } + }) + } +} + +func TestReconcileKind_SCMReportingLogic(t *testing.T) { + observer, _ := zapobserver.New(zap.InfoLevel) + _ = zap.New(observer).Sugar() + + tests := []struct { + name string + pipelineRun *tektonv1.PipelineRun + shouldCallUpdateToInProgress bool + description string + }{ + { + name: "Running reason without SCMReportingPLRStarted - should call updatePipelineRunToInProgress", + pipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "test-pr", + Annotations: map[string]string{ + keys.State: kubeinteraction.StateQueued, + keys.Repository: "test-repo", + }, + }, + Spec: tektonv1.PipelineRunSpec{}, + Status: tektonv1.PipelineRunStatus{ + Status: knativeduckv1.Status{ + Conditions: knativeduckv1.Conditions{ + { + Type: knativeapi.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: string(tektonv1.PipelineRunReasonRunning), + }, + }, + }, + }, + }, + shouldCallUpdateToInProgress: true, + description: "PipelineRun is Running but no SCM reporting done yet", + }, + { + name: "Running reason with SCMReportingPLRStarted=true - should NOT call updatePipelineRunToInProgress", + pipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "test-pr", + Annotations: map[string]string{ + keys.State: kubeinteraction.StateStarted, + keys.Repository: "test-repo", + keys.SCMReportingPLRStarted: "true", + }, + }, + Spec: tektonv1.PipelineRunSpec{}, + Status: tektonv1.PipelineRunStatus{ + Status: knativeduckv1.Status{ + Conditions: knativeduckv1.Conditions{ + { + Type: knativeapi.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: string(tektonv1.PipelineRunReasonRunning), + }, + }, + }, + }, + }, + shouldCallUpdateToInProgress: false, + description: "PipelineRun is Running and SCM reporting already done", + }, + { + name: "Non-Running reason - should NOT call updatePipelineRunToInProgress", + pipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "test-pr", + Annotations: map[string]string{ + keys.State: kubeinteraction.StateQueued, + keys.Repository: "test-repo", + }, + }, + Spec: tektonv1.PipelineRunSpec{ + Status: tektonv1.PipelineRunSpecStatusPending, + }, + Status: tektonv1.PipelineRunStatus{ + Status: knativeduckv1.Status{ + Conditions: knativeduckv1.Conditions{ + { + Type: knativeapi.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: string(tektonv1.PipelineRunReasonPending), + }, + }, + }, + }, + }, + shouldCallUpdateToInProgress: false, + description: "PipelineRun is not in Running state", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, _ := rtesting.SetupFakeContext(t) + + testRepo := &v1alpha1.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: tt.pipelineRun.GetNamespace(), + }, + Spec: v1alpha1.RepositorySpec{ + URL: randomURL, + }, + } + + testData := testclient.Data{ + Repositories: []*v1alpha1.Repository{testRepo}, + PipelineRuns: []*tektonv1.PipelineRun{tt.pipelineRun}, + } + stdata, informers := testclient.SeedTestData(t, ctx, testData) + + // Track if updatePipelineRunToInProgress was called by checking state changes + originalState := tt.pipelineRun.GetAnnotations()[keys.State] + + r := &Reconciler{ + repoLister: informers.Repository.Lister(), + run: ¶ms.Run{ + Clients: clients.Clients{ + Tekton: stdata.Pipeline, + }, + Info: info.Info{ + Pac: &info.PacOpts{ + Settings: settings.Settings{}, + }, + }, + }, + } + + err := r.ReconcileKind(ctx, tt.pipelineRun) + + // For test cases that should call updatePipelineRunToInProgress, + // we expect no error and the state should be updated + if tt.shouldCallUpdateToInProgress { + assert.NilError(t, err, tt.description) + + // Get the updated PipelineRun to check if state was changed + updatedPR, getErr := stdata.Pipeline.TektonV1().PipelineRuns(tt.pipelineRun.GetNamespace()).Get(ctx, tt.pipelineRun.GetName(), metav1.GetOptions{}) + assert.NilError(t, getErr) + + // Should have been updated to started state with SCMReportingPLRStarted annotation + assert.Equal(t, updatedPR.GetAnnotations()[keys.State], kubeinteraction.StateStarted) + scmStarted, exists := updatedPR.GetAnnotations()[keys.SCMReportingPLRStarted] + assert.Assert(t, exists, "SCMReportingPLRStarted should be set") + assert.Equal(t, scmStarted, "true") + } else { + // For cases that should NOT call updatePipelineRunToInProgress, + // the state should remain unchanged + updatedPR, getErr := stdata.Pipeline.TektonV1().PipelineRuns(tt.pipelineRun.GetNamespace()).Get(ctx, tt.pipelineRun.GetName(), metav1.GetOptions{}) + assert.NilError(t, getErr) + + // State should be unchanged + assert.Equal(t, updatedPR.GetAnnotations()[keys.State], originalState, tt.description) + + // Check SCMReportingPLRStarted annotation based on original state + scmStarted, exists := updatedPR.GetAnnotations()[keys.SCMReportingPLRStarted] + if originalState == kubeinteraction.StateStarted { + // If original state was already 'started', SCMReportingPLRStarted should exist and be "true" + assert.Assert(t, exists, "SCMReportingPLRStarted should exist when original state is started") + assert.Equal(t, scmStarted, "true", "SCMReportingPLRStarted should be 'true'") + } else { + // If original state was not 'started', SCMReportingPLRStarted should not exist + assert.Assert(t, !exists, "SCMReportingPLRStarted should not exist when original state is not started") + } + } }) } }