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

[hotfix] Preview environments improvements

api/client/deploy.go

@@ -107,3 +107,20 @@ func (c *Client) UpgradeRelease(
 		},
 	)
 }
+
+// DeleteRelease deletes a Porter release
+func (c *Client) DeleteRelease(
+	ctx context.Context,
+	projID, clusterID uint,
+	namespace, name string,
+) error {
+	return c.deleteRequest(
+		fmt.Sprintf(
+			"/projects/%d/clusters/%d/namespaces/%s/releases/%s/0",
+			projID, clusterID,
+			namespace, name,
+		),
+		nil,
+		nil,
+	)
+}

api/server/handlers/environment/delete_deployment.go

@@ -10,7 +10,6 @@ import (
 	"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/handlers/gitinstallation"
 	"github.com/porter-dev/porter/api/server/shared"
 	"github.com/porter-dev/porter/api/server/shared/apierrors"
 	"github.com/porter-dev/porter/api/server/shared/config"
@@ -40,33 +39,7 @@ func (c *DeleteDeploymentHandler) ServeHTTP(w http.ResponseWriter, r *http.Reque
 	project, _ := r.Context().Value(types.ProjectScope).(*models.Project)
 	cluster, _ := r.Context().Value(types.ClusterScope).(*models.Cluster)
 
-	envID, reqErr := requestutils.GetURLParamUint(r, "environment_id")
-
-	if reqErr != nil {
-		c.HandleAPIError(w, r, apierrors.NewErrInternal(reqErr))
-		return
-	}
-
-	// check that the environment belongs to the project and cluster IDs
-	env, err := c.Repo().Environment().ReadEnvironmentByID(project.ID, cluster.ID, envID)
-
-	if err != nil {
-		if errors.Is(err, gorm.ErrRecordNotFound) {
-			c.HandleAPIError(w, r, apierrors.NewErrForbidden(fmt.Errorf("environment id not found in cluster and project")))
-			return
-		}
-
-		c.HandleAPIError(w, r, apierrors.NewErrInternal(reqErr))
-		return
-	}
-
-	owner, name, ok := gitinstallation.GetOwnerAndNameParams(c, w, r)
-
-	if !ok {
-		return
-	}
-
-	prNumber, reqErr := requestutils.GetURLParamUint(r, "pr_number")
+	deplID, reqErr := requestutils.GetURLParamUint(r, "deployment_id")
 
 	if reqErr != nil {
 		c.HandleAPIError(w, r, apierrors.NewErrInternal(reqErr))
@@ -74,7 +47,7 @@ func (c *DeleteDeploymentHandler) ServeHTTP(w http.ResponseWriter, r *http.Reque
 	}
 
 	// read the deployment
-	depl, err := c.Repo().Environment().ReadDeploymentByGitDetails(envID, owner, name, prNumber)
+	depl, err := c.Repo().Environment().ReadDeploymentByID(project.ID, cluster.ID, deplID)
 
 	if err != nil {
 		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
@@ -99,6 +72,19 @@ func (c *DeleteDeploymentHandler) ServeHTTP(w http.ResponseWriter, r *http.Reque
 		}
 	}
 
+	// check that the environment belongs to the project and cluster IDs
+	env, err := c.Repo().Environment().ReadEnvironmentByID(project.ID, cluster.ID, depl.EnvironmentID)
+
+	if err != nil {
+		if errors.Is(err, gorm.ErrRecordNotFound) {
+			c.HandleAPIError(w, r, apierrors.NewErrForbidden(fmt.Errorf("environment id not found in cluster and project")))
+			return
+		}
+
+		c.HandleAPIError(w, r, apierrors.NewErrInternal(reqErr))
+		return
+	}
+
 	client, err := getGithubClientFromEnvironment(c.Config(), env)
 
 	if err != nil {

api/server/handlers/environment/list_deployments_by_cluster.go

@@ -140,6 +140,10 @@ func updateDeploymentWithGithubWorkflowRunStatus(
 	env *models.Environment,
 	deployment *types.Deployment,
 ) {
+	if deployment.Status == types.DeploymentStatusInactive {
+		return
+	}
+
 	client, err := getGithubClientFromEnvironment(config, env)
 
 	if err == nil {

api/server/handlers/environment/trigger_deployment_workflow.go

@@ -70,7 +70,7 @@ func (c *TriggerDeploymentWorkflowHandler) ServeHTTP(w http.ResponseWriter, r *h
 	}
 
 	latestWorkflowRun, err := getLatestWorkflowRun(client, env.GitRepoOwner, env.GitRepoName,
-		fmt.Sprintf("porter_%s_env.yml", env.Name))
+		fmt.Sprintf("porter_%s_env.yml", env.Name), depl.PRBranchFrom)
 
 	if err != nil && errors.Is(err, ErrNoWorkflowRuns) {
 		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(err, 400))
@@ -107,11 +107,22 @@ func (c *TriggerDeploymentWorkflowHandler) ServeHTTP(w http.ResponseWriter, r *h
 		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
+
+	// set the status to updating manually here for the frontend to case on
+	depl.Status = types.DeploymentStatusUpdating
+
+	_, err = c.Repo().Environment().UpdateDeployment(depl)
+
+	if err != nil {
+		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
+		return
+	}
 }
 
-func getLatestWorkflowRun(client *github.Client, owner, repo, filename string) (*github.WorkflowRun, error) {
+func getLatestWorkflowRun(client *github.Client, owner, repo, filename, branch string) (*github.WorkflowRun, error) {
 	workflowRuns, _, err := client.Actions.ListWorkflowRunsByFileName(
 		context.Background(), owner, repo, filename, &github.ListWorkflowRunsOptions{
+			Branch: branch,
 			ListOptions: github.ListOptions{
 				Page:    1,
 				PerPage: 1,

api/server/router/cluster.go

@@ -463,20 +463,15 @@ func getClusterRoutes(
 			Router:   r,
 		})
 
-		// DELETE /api/projects/{project_id}/clusters/{cluster_id}/deployments/{environment_id}/{owner}/{name}/{pr_number} ->
+		// DELETE /api/projects/{project_id}/clusters/{cluster_id}/deployments/{deployment_id} ->
 		// environment.NewDeleteDeploymentHandler
 		deleteDeploymentEndpoint := factory.NewAPIEndpoint(
 			&types.APIRequestMetadata{
 				Verb:   types.APIVerbDelete,
 				Method: types.HTTPVerbDelete,
 				Path: &types.Path{
-					Parent: basePath,
-					RelativePath: fmt.Sprintf(
-						"%s/deployments/{environment_id}/{%s}/{%s}/{pr_number}",
-						relPath,
-						types.URLParamGitRepoOwner,
-						types.URLParamGitRepoName,
-					),
+					Parent:       basePath,
+					RelativePath: relPath + "/deployments/{deployment_id}",
 				},
 				Scopes: []types.PermissionScope{
 					types.UserScope,

cli/cmd/apply.go

@@ -398,26 +398,31 @@ func (d *Driver) applyApplication(resource *models.Resource, client *api.Client,
 	if d.source.Name == "job" && appConfig.WaitForJob && (shouldCreate || !appConfig.OnlyCreate) {
 		color.New(color.FgYellow).Printf("Waiting for job '%s' to finish\n", resource.Name)
 
-		prevProject := cliConf.Project
-		prevCluster := cliConf.Cluster
-		name = resource.Name
-		namespace = d.target.Namespace
-		cliConf.Project = d.target.Project
-		cliConf.Cluster = d.target.Cluster
-
 		err = wait.WaitForJob(client, &wait.WaitOpts{
-			ProjectID: cliConf.Project,
-			ClusterID: cliConf.Cluster,
-			Namespace: namespace,
-			Name:      name,
+			ProjectID: d.target.Project,
+			ClusterID: d.target.Cluster,
+			Namespace: d.target.Namespace,
+			Name:      resource.Name,
 		})
 
 		if err != nil {
-			return nil, err
-		}
+			if appConfig.OnlyCreate {
+				err = client.DeleteRelease(
+					context.Background(),
+					d.target.Project,
+					d.target.Cluster,
+					d.target.Namespace,
+					resource.Name,
+				)
 
-		cliConf.Project = prevProject
-		cliConf.Cluster = prevCluster
+				if err != nil {
+					return nil, fmt.Errorf("error deleting job %s with waitForJob and onlyCreate set to true: %w",
+						resource.Name, err)
+				}
+			}
+
+			return nil, fmt.Errorf("error waiting for job %s: %w", resource.Name, err)
+		}
 	}
 
 	return resource, err

dashboard/src/main/home/cluster-dashboard/preview-environments/deployments/DeploymentCard.tsx

@@ -41,10 +41,7 @@ const DeploymentCard: React.FC<{
         {
           cluster_id: currentCluster.id,
           project_id: currentProject.id,
-          environment_id: deployment.environment_id,
-          repo_owner: deployment.gh_repo_owner,
-          repo_name: deployment.gh_repo_name,
-          pr_number: deployment.pull_request_id,
+          deployment_id: deployment.id,
         }
       )
       .then(() => {

dashboard/src/shared/api.tsx

@@ -367,21 +367,15 @@ const deletePRDeployment = baseApi<
   {
     cluster_id: number;
     project_id: number;
-    environment_id: number;
-    repo_owner: string;
-    repo_name: string;
-    pr_number: number;
+    deployment_id: number;
   }
 >("DELETE", (pathParams) => {
   const {
     cluster_id,
     project_id,
-    environment_id,
-    repo_owner,
-    repo_name,
-    pr_number,
+    deployment_id,
   } = pathParams;
-  return `/api/projects/${project_id}/clusters/${cluster_id}/deployments/${environment_id}/${repo_owner}/${repo_name}/${pr_number}`;
+  return `/api/projects/${project_id}/clusters/${cluster_id}/deployments/${deployment_id}`;
 });
 
 const getNotificationConfig = baseApi<