Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pull request comment for multiple sources #1022

Merged
merged 2 commits into from
Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 12 additions & 17 deletions internal/notification/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ package notification

import (
"context"
"errors"
"fmt"

argocdv1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/go-logr/logr"
"github.com/int128/argocd-commenter/internal/argocd"
"github.com/int128/argocd-commenter/internal/github"
"k8s.io/apimachinery/pkg/util/errors"
)

type Client interface {
Expand All @@ -30,7 +31,7 @@ func IsNotFoundError(err error) bool {

type Comment struct {
GitHubRepository github.Repository
Revision string
SourceRevision argocd.SourceRevision
Body string
}

Expand All @@ -40,35 +41,29 @@ type client struct {

func (c client) createComment(ctx context.Context, comment Comment, app argocdv1alpha1.Application) error {
logger := logr.FromContextOrDiscard(ctx).WithValues(
"revision", comment.Revision,
"revision", comment.SourceRevision.Revision,
"repository", comment.GitHubRepository,
)
pulls, err := c.ghc.ListPullRequests(ctx, comment.GitHubRepository, comment.Revision)
pulls, err := c.ghc.ListPullRequests(ctx, comment.GitHubRepository, comment.SourceRevision.Revision)
if err != nil {
return fmt.Errorf("unable to list pull requests of revision %s: %w", comment.Revision, err)
return fmt.Errorf("unable to list pull requests of revision %s: %w", comment.SourceRevision.Revision, err)
}
relatedPullNumbers := filterPullRequestsRelatedToEvent(pulls, app)
if len(relatedPullNumbers) == 0 {
relatedPulls := filterPullRequestsRelatedToEvent(pulls, comment.SourceRevision, app)
if len(relatedPulls) == 0 {
logger.Info("no pull request related to the revision")
return nil
}
if err := c.createPullRequestComment(ctx, comment.GitHubRepository, relatedPullNumbers, comment.Body); err != nil {
return fmt.Errorf("unable to create comment(s) on revision %s: %w", comment.Revision, err)
}
logger.Info("created comment(s)", "pulls", relatedPullNumbers)
return nil
}

func (c client) createPullRequestComment(ctx context.Context, repository github.Repository, pullNumbers []int, body string) error {
var errs []error
for _, pullNumber := range pullNumbers {
if err := c.ghc.CreateComment(ctx, repository, pullNumber, body); err != nil {
for _, pull := range relatedPulls {
if err := c.ghc.CreateComment(ctx, comment.GitHubRepository, pull.Number, comment.Body); err != nil {
errs = append(errs, err)
continue
}
logger.Info("created a comment", "pullNumber", pull.Number)
}
if len(errs) > 0 {
return errors.NewAggregate(errs)
return fmt.Errorf("unable to create comment(s) on revision %s: %w", comment.SourceRevision.Revision, errors.Join(errs...))
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion internal/notification/healthcomment.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func generateCommentOnHealthChanged(app argocdv1alpha1.Application, argocdURL st
}
return &Comment{
GitHubRepository: *repository,
Revision: sourceRevision.Revision,
SourceRevision: sourceRevision,
Body: body,
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/notification/phasecomment.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func generateCommentOnPhaseChanged(app argocdv1alpha1.Application, argocdURL str
}
return &Comment{
GitHubRepository: *repository,
Revision: sourceRevision.Revision,
SourceRevision: sourceRevision,
Body: body,
}
}
Expand Down
64 changes: 28 additions & 36 deletions internal/notification/pull.go
Original file line number Diff line number Diff line change
@@ -1,39 +1,36 @@
package notification

import (
"path/filepath"
"path"
"slices"
"strings"

argocdv1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/int128/argocd-commenter/internal/argocd"
"github.com/int128/argocd-commenter/internal/github"
)

func filterPullRequestsRelatedToEvent(pulls []github.PullRequest, app argocdv1alpha1.Application) []int {
var numbers []int
func filterPullRequestsRelatedToEvent(pulls []github.PullRequest, sourceRevision argocd.SourceRevision, app argocdv1alpha1.Application) []github.PullRequest {
manifestGeneratePaths := getManifestGeneratePaths(app)

var relatedPulls []github.PullRequest
for _, pull := range pulls {
if isPullRequestRelatedToEvent(pull, app) {
numbers = append(numbers, pull.Number)
if isPullRequestRelatedToEvent(pull, sourceRevision, manifestGeneratePaths) {
relatedPulls = append(relatedPulls, pull)
}
}
return numbers
return relatedPulls
}

func isPullRequestRelatedToEvent(pull github.PullRequest, app argocdv1alpha1.Application) bool {
if app.Spec.Source == nil {
return false
}

// support manifest path annotation
// see https://argo-cd.readthedocs.io/en/stable/operator-manual/high_availability/#webhook-and-manifest-paths-annotation
// https://github.com/int128/argocd-commenter/pull/656
manifestGeneratePaths := getManifestGeneratePaths(app)

func isPullRequestRelatedToEvent(pull github.PullRequest, sourceRevision argocd.SourceRevision, manifestGeneratePaths []string) bool {
absSourcePath := path.Join("/", sourceRevision.Source.Path)
for _, file := range pull.Files {
if strings.HasPrefix(file, app.Spec.Source.Path) {
absPullFile := path.Join("/", file)
if strings.HasPrefix(absPullFile, absSourcePath) {
return true
}
for _, path := range manifestGeneratePaths {
if strings.HasPrefix(file, path) {
for _, manifestGeneratePath := range manifestGeneratePaths {
if strings.HasPrefix(absPullFile, manifestGeneratePath) {
return true
}
}
Expand All @@ -44,30 +41,25 @@ func isPullRequestRelatedToEvent(pull github.PullRequest, app argocdv1alpha1.App
// getManifestGeneratePaths returns canonical paths of "argocd.argoproj.io/manifest-generate-paths" annotation.
// It returns nil if the field is nil or empty.
// https://argo-cd.readthedocs.io/en/stable/operator-manual/high_availability/#webhook-and-manifest-paths-annotation
// https://github.com/int128/argocd-commenter/pull/656
func getManifestGeneratePaths(app argocdv1alpha1.Application) []string {
if app.Annotations == nil {
return nil
}
if app.Spec.Source == nil {
return nil
}
var canonicalPaths []string
annotatedPaths := strings.Split(app.Annotations["argocd.argoproj.io/manifest-generate-paths"], ";")
for _, path := range annotatedPaths {
if path == "" {
var absPaths []string
manifestGeneratePaths := strings.Split(app.Annotations["argocd.argoproj.io/manifest-generate-paths"], ";")
for _, manifestGeneratePath := range manifestGeneratePaths {
if manifestGeneratePath == "" {
return nil
}
// convert to absolute path
absolutePath := path
if !filepath.IsAbs(path) {
absolutePath = filepath.Join(app.Spec.Source.Path, path)
if path.IsAbs(manifestGeneratePath) {
absPaths = append(absPaths, path.Clean(manifestGeneratePath))
continue
}
// remove leading slash
if absolutePath[0:1] == "/" {
absolutePath = absolutePath[1:]

for _, source := range app.Spec.GetSources() {
absPaths = append(absPaths, path.Join("/", source.Path, manifestGeneratePath))
}
// add to list of manifest paths
canonicalPaths = append(canonicalPaths, absolutePath)
}
return canonicalPaths
return slices.Compact(absPaths)
}
Loading