Merge pull request #2185 from porter-dev/nafees/preview-env-improvements

[POR-581] [POR-588] Preview Environments improvements

api/server/handlers/environment/create.go

@@ -63,7 +63,7 @@ func (c *CreateEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 		return
 	}
 
-	env, err := c.Repo().Environment().CreateEnvironment(&models.Environment{
+	env := &models.Environment{
 		ProjectID:           project.ID,
 		ClusterID:           cluster.ID,
 		GitInstallationID:   uint(ga.InstallationID),
@@ -73,25 +73,20 @@ func (c *CreateEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 		Mode:                request.Mode,
 		WebhookID:           string(webhookUID),
 		NewCommentsDisabled: false,
-	})
-
-	if err != nil {
-		c.deleteEnvAndReportError(w, r, env, err)
-		return
 	}
 
 	// write Github actions files to the repo
 	client, err := getGithubClientFromEnvironment(c.Config(), env)
 
 	if err != nil {
-		c.deleteEnvAndReportError(w, r, env, err)
+		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
 
-	webhookURL := fmt.Sprintf("%s/api/github/incoming_webhook/%s", c.Config().ServerConf.ServerURL, string(webhookUID))
+	webhookURL := getGithubWebhookURLFromUID(c.Config().ServerConf.ServerURL, string(webhookUID))
 
 	// create incoming webhook
-	_, _, err = client.Repositories.CreateHook(
+	hook, _, err := client.Repositories.CreateHook(
 		r.Context(), owner, name, &github.Hook{
 			Config: map[string]interface{}{
 				"url":          webhookURL,
@@ -104,7 +99,18 @@ func (c *CreateEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 	)
 
 	if err != nil && !strings.Contains(err.Error(), "already exists on this repository") {
-		c.deleteEnvAndReportError(w, r, env, err)
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
+			fmt.Errorf("error trying to create a new github repository webhook: %w", err), http.StatusConflict),
+		)
+		return
+	}
+
+	env.GithubWebhookID = hook.GetID()
+
+	env, err = c.Repo().Environment().CreateEnvironment(env)
+
+	if err != nil {
+		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
 
@@ -188,3 +194,7 @@ func getGithubClientFromEnvironment(config *config.Config, env *models.Environme
 
 	return github.NewClient(&http.Client{Transport: itr}), nil
 }
+
+func getGithubWebhookURLFromUID(serverURL, webhookUID string) string {
+	return fmt.Sprintf("%s/api/github/incoming_webhook/%s", serverURL, string(webhookUID))
+}

api/server/handlers/environment/create_deployment.go

@@ -66,6 +66,22 @@ func (c *CreateDeploymentHandler) ServeHTTP(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
+	// add a check for Github PR status
+	prClosed, err := isGithubPRClosed(client, owner, name, int(request.PullRequestID))
+
+	if err != nil {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
+			fmt.Errorf("error fetching details of github PR. Error: %w", err), http.StatusConflict,
+		))
+		return
+	}
+
+	if prClosed {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("Github PR has been closed"),
+			http.StatusConflict))
+		return
+	}
+
 	ghDeployment, err := createDeployment(client, env, request.PRBranchFrom, request.ActionID)
 
 	if err != nil {

api/server/handlers/environment/delete.go

@@ -1,10 +1,12 @@
 package environment
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"net/http"
 
+	"github.com/google/go-github/v41/github"
 	"github.com/porter-dev/porter/api/server/authz"
 	"github.com/porter-dev/porter/api/server/handlers"
 	"github.com/porter-dev/porter/api/server/shared"
@@ -79,6 +81,9 @@ func (c *DeleteEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 		agent.DeleteNamespace(depl.Namespace)
 	}
 
+	ghWebhookID := env.GithubWebhookID
+	webhookUID := env.WebhookID
+
 	// delete the environment
 	env, err = c.Repo().Environment().DeleteEnvironment(env)
 
@@ -87,6 +92,29 @@ func (c *DeleteEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 		return
 	}
 
+	// FIXME: ignore the return status codes for now, should be fixed when we start returning all non-fatal errors
+	if ghWebhookID != 0 {
+		client.Repositories.DeleteHook(context.Background(), owner, name, ghWebhookID)
+	} else {
+		webhookURL := getGithubWebhookURLFromUID(c.Config().ServerConf.ServerURL, string(webhookUID))
+
+		// FIXME: should be cycling through all webhooks if pagination is needed
+		hooks, _, err := client.Repositories.ListHooks(context.Background(), owner, name, &github.ListOptions{})
+
+		if err == nil {
+			for _, hook := range hooks {
+				if hookURL, ok := hook.Config["url"]; ok {
+					if hookURLStr, ok := hookURL.(string); ok {
+						if hookURLStr == webhookURL {
+							client.Repositories.DeleteHook(context.Background(), owner, name, hook.GetID())
+							break
+						}
+					}
+				}
+			}
+		}
+	}
+
 	err = actions.DeleteEnv(&actions.EnvOpts{
 		Client:            client,
 		ServerURL:         c.Config().ServerConf.ServerURL,

api/server/handlers/environment/enable_pull_request.go

@@ -56,14 +56,20 @@ func (c *EnablePullRequestHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 	}
 
 	// add an extra check that the installation has permission to read this pull request
-	pr, ghResp, err := client.PullRequests.Get(r.Context(), env.GitRepoOwner, env.GitRepoName, int(request.Number))
+	pr, _, err := client.PullRequests.Get(r.Context(), env.GitRepoOwner, env.GitRepoName, int(request.Number))
 
 	if err != nil {
 		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
 
-	ghResp, err = client.Actions.CreateWorkflowDispatchEventByFileName(
+	if pr.GetState() == "closed" {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("Github PR has been closed"),
+			http.StatusConflict))
+		return
+	}
+
+	ghResp, err := client.Actions.CreateWorkflowDispatchEventByFileName(
 		r.Context(), env.GitRepoOwner, env.GitRepoName, fmt.Sprintf("porter_%s_env.yml", env.Name),
 		github.CreateWorkflowDispatchEventRequest{
 			Ref: request.BranchFrom,

api/server/handlers/environment/finalize_deployment.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"net/url"
 	"strings"
 
 	"github.com/google/go-github/v41/github"
@@ -106,6 +107,23 @@ func (c *FinalizeDeploymentHandler) ServeHTTP(w http.ResponseWriter, r *http.Req
 		return
 	}
 
+	// add a check for the PR to be open before creating a comment
+	prClosed, err := isGithubPRClosed(client, owner, name, int(depl.PullRequestID))
+
+	if err != nil {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
+			fmt.Errorf("error fetching details of github PR for deployment ID: %d. Error: %w",
+				depl.ID, err), http.StatusConflict,
+		))
+		return
+	}
+
+	if prClosed {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("Github PR has been closed"),
+			http.StatusConflict))
+		return
+	}
+
 	workflowRun, err := commonutils.GetLatestWorkflowRun(client, depl.RepoOwner, depl.RepoName,
 		fmt.Sprintf("porter_%s_env.yml", env.Name), depl.PRBranchFrom)
 
@@ -127,9 +145,9 @@ func (c *FinalizeDeploymentHandler) ServeHTTP(w http.ResponseWriter, r *http.Req
 			"| Latest SHA | [`%s`](https://github.com/%s/%s/commit/%s) |\n"+
 			"| Live URL | %s |\n"+
 			"| Build Logs | %s |\n"+
-			"| Porter Deployments URL | %s/preview-environments/details/%s?environment_id=%d |",
+			"| Porter Deployments URL | %s/preview-environments/details/%s?environment_id=%d&project_id=%d&cluster=%s |",
 		depl.CommitSHA, depl.RepoOwner, depl.RepoName, depl.CommitSHA, depl.Subdomain, workflowRun.GetHTMLURL(),
-		c.Config().ServerConf.ServerURL, depl.Namespace, depl.EnvironmentID,
+		c.Config().ServerConf.ServerURL, depl.Namespace, depl.EnvironmentID, project.ID, url.QueryEscape(cluster.Name),
 	)
 
 	if len(request.SuccessfulResources) > 0 {
@@ -264,3 +282,19 @@ func updateGithubComment(
 
 	return err
 }
+
+func isGithubPRClosed(
+	client *github.Client,
+	owner, name string,
+	prNumber int,
+) (bool, error) {
+	ghPR, _, err := client.PullRequests.Get(
+		context.Background(), owner, name, prNumber,
+	)
+
+	if err != nil {
+		return false, err
+	}
+
+	return ghPR.GetState() == "closed", nil
+}

api/server/handlers/environment/finalize_deployment_with_errors.go

@@ -82,12 +82,6 @@ func (c *FinalizeDeploymentWithErrorsHandler) ServeHTTP(w http.ResponseWriter, r
 		return
 	}
 
-	depl.Status = types.DeploymentStatusFailed
-
-	// we do not care of the error in this case because the list deployments endpoint
-	// talks to the github API to fetch the deployment status correctly
-	c.Repo().Environment().UpdateDeployment(depl)
-
 	client, err := getGithubClientFromEnvironment(c.Config(), env)
 
 	if err != nil {
@@ -97,7 +91,30 @@ func (c *FinalizeDeploymentWithErrorsHandler) ServeHTTP(w http.ResponseWriter, r
 		return
 	}
 
-	// FIXME: ignore the status of thie API call for now
+	// add a check for the PR to be open before creating a comment
+	prClosed, err := isGithubPRClosed(client, owner, name, int(depl.PullRequestID))
+
+	if err != nil {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
+			fmt.Errorf("error fetching details of github PR for deployment ID: %d. Error: %w",
+				depl.ID, err), http.StatusConflict,
+		))
+		return
+	}
+
+	if prClosed {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("Github PR has been closed"),
+			http.StatusConflict))
+		return
+	}
+
+	depl.Status = types.DeploymentStatusFailed
+
+	// we do not care of the error in this case because the list deployments endpoint
+	// talks to the github API to fetch the deployment status correctly
+	c.Repo().Environment().UpdateDeployment(depl)
+
+	// FIXME: ignore the status of this API call for now
 	client.Repositories.CreateDeploymentStatus(
 		context.Background(), owner, name, depl.GHDeploymentID, &github.DeploymentStatusRequest{
 			State:       github.String("failure"),

api/server/handlers/environment/reenable_deployment.go

@@ -51,41 +51,61 @@ func (c *ReenableDeploymentHandler) ServeHTTP(w http.ResponseWriter, r *http.Req
 	}
 
 	if depl.Status != types.DeploymentStatusInactive {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
+			fmt.Errorf("trying to re-enable deployment which is not marked \"inactive\""), http.StatusPreconditionFailed,
+		))
 		return
 	}
 
-	depl.Status = types.DeploymentStatusCreating
-
-	depl, err = c.Repo().Environment().UpdateDeployment(depl)
+	env, err := c.Repo().Environment().ReadEnvironmentByID(project.ID, cluster.ID, depl.EnvironmentID)
 
 	if err != nil {
 		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
 
-	// create the backing namespace
-	agent, err := c.GetAgent(r, cluster, "")
+	client, err := getGithubClientFromEnvironment(c.Config(), env)
 
 	if err != nil {
 		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
 
-	_, err = agent.CreateNamespace(depl.Namespace)
+	// add a check for the PR to be open before creating a comment
+	prClosed, err := isGithubPRClosed(client, depl.RepoOwner, depl.RepoName, int(depl.PullRequestID))
+
+	if err != nil {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
+			fmt.Errorf("error fetching details of github PR for deployment ID: %d. Error: %w",
+				depl.ID, err), http.StatusConflict,
+		))
+		return
+	}
+
+	if prClosed {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("Github PR has been closed"),
+			http.StatusConflict))
+		return
+	}
+
+	depl.Status = types.DeploymentStatusCreating
+
+	depl, err = c.Repo().Environment().UpdateDeployment(depl)
 
 	if err != nil {
 		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
 
-	env, err := c.Repo().Environment().ReadEnvironmentByID(project.ID, cluster.ID, depl.EnvironmentID)
+	// create the backing namespace
+	agent, err := c.GetAgent(r, cluster, "")
 
 	if err != nil {
 		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
 
-	client, err := getGithubClientFromEnvironment(c.Config(), env)
+	_, err = agent.CreateNamespace(depl.Namespace)
 
 	if err != nil {
 		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))

api/server/handlers/environment/trigger_deployment_workflow.go

@@ -52,6 +52,9 @@ func (c *TriggerDeploymentWorkflowHandler) ServeHTTP(w http.ResponseWriter, r *h
 	}
 
 	if depl.Status == types.DeploymentStatusInactive {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
+			fmt.Errorf("trying to trigger workflow run for inactive deployment"), http.StatusConflict,
+		))
 		return
 	}
 
@@ -69,6 +72,23 @@ func (c *TriggerDeploymentWorkflowHandler) ServeHTTP(w http.ResponseWriter, r *h
 		return
 	}
 
+	// add a check for the PR to be open before creating a comment
+	prClosed, err := isGithubPRClosed(client, depl.RepoOwner, depl.RepoName, int(depl.PullRequestID))
+
+	if err != nil {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
+			fmt.Errorf("error fetching details of github PR for deployment ID: %d. Error: %w",
+				depl.ID, err), http.StatusConflict,
+		))
+		return
+	}
+
+	if prClosed {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("Github PR has been closed"),
+			http.StatusConflict))
+		return
+	}
+
 	latestWorkflowRun, err := commonutils.GetLatestWorkflowRun(client, env.GitRepoOwner, env.GitRepoName,
 		fmt.Sprintf("porter_%s_env.yml", env.Name), depl.PRBranchFrom)
 

api/server/handlers/environment/update_deployment.go

@@ -1,6 +1,7 @@
 package environment
 
 import (
+	"fmt"
 	"net/http"
 
 	"github.com/porter-dev/porter/api/server/authz"
@@ -71,6 +72,23 @@ func (c *UpdateDeploymentHandler) ServeHTTP(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
+	// add a check for the PR to be open before creating a comment
+	prClosed, err := isGithubPRClosed(client, owner, name, int(depl.PullRequestID))
+
+	if err != nil {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
+			fmt.Errorf("error fetching details of github PR for deployment ID: %d. Error: %w",
+				depl.ID, err), http.StatusConflict,
+		))
+		return
+	}
+
+	if prClosed {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("Github PR has been closed"),
+			http.StatusConflict))
+		return
+	}
+
 	ghDeployment, err := createDeployment(client, env, request.PRBranchFrom, request.ActionID)
 
 	if err != nil {

api/server/handlers/environment/update_deployment_status.go

@@ -1,6 +1,7 @@
 package environment
 
 import (
+	"fmt"
 	"net/http"
 
 	"github.com/porter-dev/porter/api/server/authz"
@@ -63,6 +64,32 @@ func (c *UpdateDeploymentStatusHandler) ServeHTTP(w http.ResponseWriter, r *http
 		return
 	}
 
+	client, err := getGithubClientFromEnvironment(c.Config(), env)
+
+	if err != nil {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
+			fmt.Errorf("unable to get github client: %w", err), http.StatusConflict,
+		))
+		return
+	}
+
+	// add a check for the PR to be open before creating a comment
+	prClosed, err := isGithubPRClosed(client, depl.RepoOwner, depl.RepoName, int(depl.PullRequestID))
+
+	if err != nil {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
+			fmt.Errorf("error fetching details of github PR for deployment ID: %d. Error: %w",
+				depl.ID, err), http.StatusConflict,
+		))
+		return
+	}
+
+	if prClosed {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("Github PR has been closed"),
+			http.StatusConflict))
+		return
+	}
+
 	if depl.Status == types.DeploymentStatusInactive && request.Status != string(types.DeploymentStatusCreating) {
 		// a deployment from "inactive" state can only transition to "creating"
 		c.WriteResult(w, r, depl.ToDeploymentType())

api/server/handlers/webhook/github_incoming.go

@@ -2,6 +2,7 @@ package webhook
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"strconv"
@@ -18,6 +19,7 @@ import (
 	"github.com/porter-dev/porter/api/server/shared/requestutils"
 	"github.com/porter-dev/porter/api/types"
 	"github.com/porter-dev/porter/internal/models"
+	"gorm.io/gorm"
 )
 
 type GithubIncomingWebhookHandler struct {
@@ -74,7 +76,7 @@ func (c *GithubIncomingWebhookHandler) processPullRequestEvent(event *github.Pul
 
 	env, err := c.Repo().Environment().ReadEnvironmentByWebhookIDOwnerRepoName(webhookID, owner, repo)
 
-	if err != nil {
+	if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
 		return fmt.Errorf("[webhookID: %s, owner: %s, repo: %s] error reading environment: %w", webhookID, owner, repo, err)
 	}
 

internal/models/environment.go

@@ -24,6 +24,8 @@ type Environment struct {
 	// WebhookID uniquely identifies the environment when other fields (project, cluster)
 	// aren't present
 	WebhookID string `gorm:"unique"`
+
+	GithubWebhookID int64
 }
 
 func (e *Environment) ToEnvironmentType() *types.Environment {