Skip to content

Commit

Permalink
feat: only open pr if yaml is found (#64)
Browse files Browse the repository at this point in the history
* only open pr if yaml is found

Signed-off-by: Zach Aller <[email protected]>

* review changes

Signed-off-by: Zach Aller <[email protected]>

* change logic

Signed-off-by: Zach Aller <[email protected]>

* short circut

Signed-off-by: Zach Aller <[email protected]>

* log message

Signed-off-by: Zach Aller <[email protected]>

* remove log

Signed-off-by: Zach Aller <[email protected]>

---------

Signed-off-by: Zach Aller <[email protected]>
  • Loading branch information
zachaller authored Oct 21, 2024
1 parent 18d60c7 commit d5c25e7
Show file tree
Hide file tree
Showing 3 changed files with 197 additions and 68 deletions.
151 changes: 88 additions & 63 deletions internal/controller/proposedcommit_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (r *ProposedCommitReconciler) Reconcile(ctx context.Context, req ctrl.Reque
return ctrl.Result{}, err
}

err = r.creatOrUpdatePullRequest(ctx, &pc)
err = r.mergeOrPullRequestPromote(ctx, gitOperations, &pc)
if err != nil {
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -295,81 +295,106 @@ func (r *ProposedCommitReconciler) calculateStatus(ctx context.Context, pc *prom
return nil
}

func (r *ProposedCommitReconciler) creatOrUpdatePullRequest(ctx context.Context, pc *promoterv1alpha1.ProposedCommit) error {
logger := log.FromContext(ctx)
func (r *ProposedCommitReconciler) mergeOrPullRequestPromote(ctx context.Context, gitOperations *git.GitOperations, pc *promoterv1alpha1.ProposedCommit) error {
if pc.Status.Proposed.Dry.Sha == pc.Status.Active.Dry.Sha {
return nil
}

if pc.Status.Proposed.Dry.Sha != pc.Status.Active.Dry.Sha {
// If the proposed dry sha is different from the active dry sha, create a pull request
prRequired, err := gitOperations.IsPullRequestRequired(ctx, pc.Spec.ActiveBranch, pc.Spec.ProposedBranch)
if err != nil {
return err
}

logger.V(4).Info("Proposed dry sha, does not match active", "proposedDrySha", pc.Status.Proposed.Dry.Sha, "activeDrySha", pc.Status.Active.Dry.Sha)
gitRepo, err := utils.GetGitRepositorytFromRepositoryReference(ctx, r.Client, pc.Spec.RepositoryReference)
if prRequired {
err = r.creatOrUpdatePullRequest(ctx, pc)
if err != nil {
return fmt.Errorf("failed to get GitRepository: %w", err)
return err
}
prName := utils.KubeSafeUniqueName(ctx, utils.GetPullRequestName(ctx, gitRepo.Spec.Owner, gitRepo.Spec.Name, pc.Spec.ProposedBranch, pc.Spec.ActiveBranch))

var pr promoterv1alpha1.PullRequest
err = r.Get(ctx, client.ObjectKey{
Namespace: pc.Namespace,
Name: prName,
}, &pr)
} else {
err = gitOperations.PromoteEnvironmentWithMerge(ctx, pc.Spec.ActiveBranch, pc.Spec.ProposedBranch)
if err != nil {
if errors.IsNotFound(err) {
// The code below sets the ownership for the PullRequest Object
kind := reflect.TypeOf(promoterv1alpha1.ProposedCommit{}).Name()
gvk := promoterv1alpha1.GroupVersion.WithKind(kind)
controllerRef := metav1.NewControllerRef(pc, gvk)

pr = promoterv1alpha1.PullRequest{
ObjectMeta: metav1.ObjectMeta{
Name: prName,
Namespace: pc.Namespace,
OwnerReferences: []metav1.OwnerReference{*controllerRef},
Labels: map[string]string{
promoterv1alpha1.PromotionStrategyLabel: utils.KubeSafeLabel(ctx, pc.Labels["promoter.argoproj.io/promotion-strategy"]),
promoterv1alpha1.ProposedCommitLabel: utils.KubeSafeLabel(ctx, pc.Name),
promoterv1alpha1.EnvironmentLabel: utils.KubeSafeLabel(ctx, pc.Spec.ActiveBranch),
},
},
Spec: promoterv1alpha1.PullRequestSpec{
RepositoryReference: pc.Spec.RepositoryReference,
Title: fmt.Sprintf("Promote %s to `%s`", pc.Status.Proposed.DryShaShort(), pc.Spec.ActiveBranch),
TargetBranch: pc.Spec.ActiveBranch,
SourceBranch: pc.Spec.ProposedBranch,
Description: fmt.Sprintf("This PR is promoting the environment branch `%s` which is currently on dry sha %s to dry sha %s.", pc.Spec.ActiveBranch, pc.Status.Active.Dry.Sha, pc.Status.Proposed.Dry.Sha),
State: "open",
return err
}
}

return nil
}

func (r *ProposedCommitReconciler) creatOrUpdatePullRequest(ctx context.Context, pc *promoterv1alpha1.ProposedCommit) error {
logger := log.FromContext(ctx)
if pc.Status.Proposed.Dry.Sha == pc.Status.Active.Dry.Sha {
// If the proposed dry sha is different from the active dry sha, create a pull request
return nil
}

logger.V(4).Info("Proposed dry sha, does not match active", "proposedDrySha", pc.Status.Proposed.Dry.Sha, "activeDrySha", pc.Status.Active.Dry.Sha)
gitRepo, err := utils.GetGitRepositorytFromRepositoryReference(ctx, r.Client, pc.Spec.RepositoryReference)
if err != nil {
return fmt.Errorf("failed to get GitRepository: %w", err)
}
prName := utils.KubeSafeUniqueName(ctx, utils.GetPullRequestName(ctx, gitRepo.Spec.Owner, gitRepo.Spec.Name, pc.Spec.ProposedBranch, pc.Spec.ActiveBranch))

var pr promoterv1alpha1.PullRequest
err = r.Get(ctx, client.ObjectKey{
Namespace: pc.Namespace,
Name: prName,
}, &pr)
if err != nil {
if errors.IsNotFound(err) {
// The code below sets the ownership for the PullRequest Object
kind := reflect.TypeOf(promoterv1alpha1.ProposedCommit{}).Name()
gvk := promoterv1alpha1.GroupVersion.WithKind(kind)
controllerRef := metav1.NewControllerRef(pc, gvk)

pr = promoterv1alpha1.PullRequest{
ObjectMeta: metav1.ObjectMeta{
Name: prName,
Namespace: pc.Namespace,
OwnerReferences: []metav1.OwnerReference{*controllerRef},
Labels: map[string]string{
promoterv1alpha1.PromotionStrategyLabel: utils.KubeSafeLabel(ctx, pc.Labels["promoter.argoproj.io/promotion-strategy"]),
promoterv1alpha1.ProposedCommitLabel: utils.KubeSafeLabel(ctx, pc.Name),
promoterv1alpha1.EnvironmentLabel: utils.KubeSafeLabel(ctx, pc.Spec.ActiveBranch),
},
}
err = r.Create(ctx, &pr)
if err != nil {
return err
}
r.Recorder.Event(pc, "Normal", "PullRequestCreated", fmt.Sprintf("Pull Request %s created", pr.Name))
logger.V(4).Info("Created pull request")
} else {
},
Spec: promoterv1alpha1.PullRequestSpec{
RepositoryReference: pc.Spec.RepositoryReference,
Title: fmt.Sprintf("Promote %s to `%s`", pc.Status.Proposed.DryShaShort(), pc.Spec.ActiveBranch),
TargetBranch: pc.Spec.ActiveBranch,
SourceBranch: pc.Spec.ProposedBranch,
Description: fmt.Sprintf("This PR is promoting the environment branch `%s` which is currently on dry sha %s to dry sha %s.", pc.Spec.ActiveBranch, pc.Status.Active.Dry.Sha, pc.Status.Proposed.Dry.Sha),
State: "open",
},
}
err = r.Create(ctx, &pr)
if err != nil {
return err
}
r.Recorder.Event(pc, "Normal", "PullRequestCreated", fmt.Sprintf("Pull Request %s created", pr.Name))
logger.V(4).Info("Created pull request")
} else {
// Pull request already exists, update it.
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
prUpdated := promoterv1alpha1.PullRequest{}
err := r.Get(ctx, client.ObjectKey{Namespace: pr.Namespace, Name: pr.Name}, &prUpdated)
if err != nil {
return err
}
prUpdated.Spec.RepositoryReference = pc.Spec.RepositoryReference
prUpdated.Spec.Title = fmt.Sprintf("Promote %s to `%s`", pc.Status.Proposed.DryShaShort(), pc.Spec.ActiveBranch)
prUpdated.Spec.TargetBranch = pc.Spec.ActiveBranch
prUpdated.Spec.SourceBranch = pc.Spec.ProposedBranch
prUpdated.Spec.Description = fmt.Sprintf("This PR is promoting the environment branch `%s` which is currently on dry sha %s to dry sha %s.", pc.Spec.ActiveBranch, pc.Status.Active.Dry.Sha, pc.Status.Proposed.Dry.Sha)
return r.Update(ctx, &prUpdated)
})
return err
}
} else {
// Pull request already exists, update it.
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
prUpdated := promoterv1alpha1.PullRequest{}
err := r.Get(ctx, client.ObjectKey{Namespace: pr.Namespace, Name: pr.Name}, &prUpdated)
if err != nil {
return err
}
//r.Recorder.Event(pc, "Normal", "PullRequestUpdated", fmt.Sprintf("Pull Request %s updated", pr.Name))
logger.V(4).Info("Updated pull request resource")
prUpdated.Spec.RepositoryReference = pc.Spec.RepositoryReference
prUpdated.Spec.Title = fmt.Sprintf("Promote %s to `%s`", pc.Status.Proposed.DryShaShort(), pc.Spec.ActiveBranch)
prUpdated.Spec.TargetBranch = pc.Spec.ActiveBranch
prUpdated.Spec.SourceBranch = pc.Spec.ProposedBranch
prUpdated.Spec.Description = fmt.Sprintf("This PR is promoting the environment branch `%s` which is currently on dry sha %s to dry sha %s.", pc.Spec.ActiveBranch, pc.Status.Active.Dry.Sha, pc.Status.Proposed.Dry.Sha)
return r.Update(ctx, &prUpdated)
})
if err != nil {
return err
}
//r.Recorder.Event(pc, "Normal", "PullRequestUpdated", fmt.Sprintf("Pull Request %s updated", pr.Name))
logger.V(4).Info("Updated pull request resource")
}

return nil
Expand Down
8 changes: 4 additions & 4 deletions internal/controller/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,14 +362,14 @@ func makeChangeAndHydrateRepo(gitPath string, repoOwner string, repoName string)
_, err = runGitCmd(gitPath, "checkout", defaultBranch)
Expect(err).NotTo(HaveOccurred())

f, err := os.Create(path.Join(gitPath, "manifests-fake.timestamp"))
f, err := os.Create(path.Join(gitPath, "manifests-fake.yaml"))
Expect(err).NotTo(HaveOccurred())
str := fmt.Sprintf("{\"time\": \"%s\"}", time.Now().Format(time.RFC3339))
_, err = f.WriteString(str)
Expect(err).NotTo(HaveOccurred())
err = f.Close()
Expect(err).NotTo(HaveOccurred())
_, err = runGitCmd(gitPath, "add", "manifests-fake.timestamp")
_, err = runGitCmd(gitPath, "add", "manifests-fake.yaml")
Expect(err).NotTo(HaveOccurred())
_, err = runGitCmd(gitPath, "commit", "-m", "added fake manifests commit with timestamp")
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -397,14 +397,14 @@ func makeChangeAndHydrateRepo(gitPath string, repoOwner string, repoName string)
_, err = runGitCmd(gitPath, "add", "hydrator.metadata")
Expect(err).NotTo(HaveOccurred())

f, err = os.Create(path.Join(gitPath, "manifests-fake.timestamp"))
f, err = os.Create(path.Join(gitPath, "manifests-fake.yaml"))
Expect(err).NotTo(HaveOccurred())
str := fmt.Sprintf("{\"time\": \"%s\"}", time.Now().Format(time.RFC3339))
_, err = f.WriteString(str)
Expect(err).NotTo(HaveOccurred())
err = f.Close()
Expect(err).NotTo(HaveOccurred())
_, err = runGitCmd(gitPath, "add", "manifests-fake.timestamp")
_, err = runGitCmd(gitPath, "add", "manifests-fake.yaml")
Expect(err).NotTo(HaveOccurred())

_, err = runGitCmd(gitPath, "commit", "-m", "added pending commit from dry sha, "+sha+" from environment "+strings.TrimRight(environment, "-next"))
Expand Down
106 changes: 105 additions & 1 deletion internal/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,21 @@ func (g *GitOperations) CloneRepo(ctx context.Context) error {

stdout, stderr, err = g.runCmd(ctx, path, "config", "pull.rebase", "false")
if err != nil {
logger.Error(err, "could set git config", "stdout", stdout, "stderr", stderr)
logger.Error(err, "could not set git config", "stdout", stdout, "stderr", stderr)
return err
}
stdout, stderr, err = g.runCmd(ctx, path, "config", "user.name", "GitOps Promoter")
if err != nil {
logger.Error(err, "could not set git config", "stdout", stdout, "stderr", stderr)
return err
}

stdout, stderr, err = g.runCmd(ctx, path, "config", "user.email", "[email protected]")
if err != nil {
logger.Error(err, "could not set git config", "stdout", stdout, "stderr", stderr)
return err
}

logger.V(4).Info("Cloned repo successful", "repo", g.gap.GetGitHttpsRepoUrl(*g.gitRepo))

g.pathLookup.Set(g.gap.GetGitHttpsRepoUrl(*g.gitRepo)+g.pathContext, path)
Expand Down Expand Up @@ -178,6 +190,98 @@ func (g *GitOperations) GetShaTime(ctx context.Context, sha string) (v1.Time, er
return v1.Time{Time: cTime}, nil
}

func (g *GitOperations) PromoteEnvironmentWithMerge(ctx context.Context, environmentBranch, environmentNextBranch string) error {
logger := log.FromContext(ctx)

if g.pathLookup.Get(g.gap.GetGitHttpsRepoUrl(*g.gitRepo)+g.pathContext) == "" {
return fmt.Errorf("no repo path found")
}

_, stderr, err := g.runCmd(ctx, g.pathLookup.Get(g.gap.GetGitHttpsRepoUrl(*g.gitRepo)+g.pathContext), "fetch", "origin")
if err != nil {
logger.Error(err, "could not fetch origin", "gitError", stderr)
return err
}

_, stderr, err = g.runCmd(ctx, g.pathLookup.Get(g.gap.GetGitHttpsRepoUrl(*g.gitRepo)+g.pathContext), "checkout", "--progress", "-B", environmentBranch, fmt.Sprintf("origin/%s", environmentBranch))
if err != nil {
logger.Error(err, "could not git checkout", "gitError", stderr)
return err
}
logger.V(4).Info("Checked out branch", "branch", environmentBranch)

// Don't think this is needed
//_, stderr, err = g.runCmd(ctx, g.pathLookup.Get(g.gap.GetGitHttpsRepoUrl(*g.repoRef)+g.pathContext), "pull", "--progress")
//if err != nil {
// logger.Error(err, "could not git pull", "gitError", stderr)
// return err
//}
//logger.V(4).Info("Pulled branch", "branch", environmentBranch)

_, stderr, err = g.runCmd(ctx, g.pathLookup.Get(g.gap.GetGitHttpsRepoUrl(*g.gitRepo)+g.pathContext), "pull", "--progress", "origin", environmentNextBranch)
if err != nil {
logger.Error(err, "could not git pull", "gitError", stderr)
return err
}
logger.V(4).Info("Pulled branch", "branch", environmentNextBranch)

_, stderr, err = g.runCmd(ctx, g.pathLookup.Get(g.gap.GetGitHttpsRepoUrl(*g.gitRepo)+g.pathContext), "push", "--progress", "origin", environmentBranch)
if err != nil {
logger.Error(err, "could not git pull", "gitError", stderr)
return err
}
logger.V(4).Info("Pushed branch", "branch", environmentBranch)

return nil
}

// IsPullRequestRequired will compare the environment branch with the next environment branch and return true if a PR is required.
// The PR is required if the diff between the two branches contain edits to yaml files.
func (g *GitOperations) IsPullRequestRequired(ctx context.Context, environmentBranch, environmentNextBranch string) (bool, error) {
logger := log.FromContext(ctx)

//environmentNextBranch := environmentBranch + "-next"

if g.pathLookup.Get(g.gap.GetGitHttpsRepoUrl(*g.gitRepo)+g.pathContext) == "" {
return false, fmt.Errorf("no repo path found")
}

// Checkout the environment branch
_, stderr, err := g.runCmd(ctx, g.pathLookup.Get(g.gap.GetGitHttpsRepoUrl(*g.gitRepo)+g.pathContext), "checkout", "--progress", "-B", environmentBranch, fmt.Sprintf("origin/%s", environmentBranch))
if err != nil {
logger.Error(err, "could not git checkout", "gitError", stderr)
return false, err
}
logger.V(4).Info("Checked out branch", "branch", environmentBranch)

// Fetch the next environment branch
_, stderr, err = g.runCmd(ctx, g.pathLookup.Get(g.gap.GetGitHttpsRepoUrl(*g.gitRepo)+g.pathContext), "fetch", "origin", environmentNextBranch)
if err != nil {
logger.Error(err, "could not fetch branch", "gitError", stderr)
return false, err
}
logger.V(4).Info("Fetched branch", "branch", environmentNextBranch)

// Get the diff between the two branches
stdout, stderr, err := g.runCmd(ctx, g.pathLookup.Get(g.gap.GetGitHttpsRepoUrl(*g.gitRepo)+g.pathContext), "diff", fmt.Sprintf("origin/%s", environmentBranch), fmt.Sprintf("origin/%s", environmentNextBranch), "--name-only", "--diff-filter=ACMRT")
if err != nil {
logger.Error(err, "could not get diff", "gitError", stderr)
return false, err
}
logger.V(4).Info("Got diff", "diff", stdout)

// Check if the diff contains any YAML files if so we expect a manifest to have changed
// TODO: This is temporary check we should add some path globbing support to the specs
for _, file := range strings.Split(stdout, "\n") {
if strings.HasSuffix(file, ".yaml") || strings.HasSuffix(file, ".yml") {
logger.V(4).Info("YAML file changed", "file", file)
return true, nil
}
}

return false, nil
}

func (g *GitOperations) runCmd(ctx context.Context, directory string, args ...string) (string, string, error) {
user, err := g.gap.GetUser(ctx)
if err != nil {
Expand Down

0 comments on commit d5c25e7

Please sign in to comment.