feat: [CDE-753]: Fix backend handling of gitspace deletion requests (#3745)

* feat: [CDE-753]: Fix lint
* feat: [CDE-753]: Fix backend handling of gitspace deletion requests

app/gitspace/infrastructure/trigger_infra_event.go

@@ -100,7 +100,7 @@ func (i InfraProvisioner) TriggerInfraEventWithOpts(
 		return err
 	}
 
-	_, configMetadata, err := i.getAllParamsFromDB(ctx, gitspaceConfig.InfraProviderResource, infraProvider)
+	allParams, configMetadata, err := i.getAllParamsFromDB(ctx, gitspaceConfig.InfraProviderResource, infraProvider)
 	if err != nil {
 		return fmt.Errorf("could not get all params from DB while provisioning: %w", err)
 	}
@@ -126,6 +126,8 @@ func (i InfraProvisioner) TriggerInfraEventWithOpts(
 				gitspaceConfig,
 				opts.RequiredGitspacePorts,
 				stoppedInfra,
+				configMetadata,
+				allParams,
 			)
 		}
 		return i.provisionExistingInfrastructure(
@@ -145,9 +147,10 @@ func (i InfraProvisioner) TriggerInfraEventWithOpts(
 				*infra,
 				opts.CanDeleteUserData,
 				configMetadata,
+				allParams,
 			)
 		}
-		return infraProvider.Deprovision(ctx, *infra, opts.CanDeleteUserData, configMetadata)
+		return infraProvider.Deprovision(ctx, *infra, opts.CanDeleteUserData, configMetadata, allParams)
 
 	case enum.InfraEventCleanup:
 		return infraProvider.CleanupInstanceResources(ctx, *infra)
@@ -167,6 +170,8 @@ func (i InfraProvisioner) provisionNewInfrastructure(
 	gitspaceConfig types.GitspaceConfig,
 	requiredGitspacePorts []types.GitspacePort,
 	stoppedInfra types.Infrastructure,
+	configMetadata map[string]any,
+	allParams []types.InfraProviderParameter,
 ) error {
 	// Logic for new provisioning...
 	infraProvisionedLatest, _ := i.infraProvisionedStore.FindLatestByGitspaceInstanceID(
@@ -185,12 +190,8 @@ func (i InfraProvisioner) provisionNewInfrastructure(
 	}
 
 	infraProviderResource := gitspaceConfig.InfraProviderResource
-	allParams, configMetadata, err := i.getAllParamsFromDB(ctx, infraProviderResource, infraProvider)
-	if err != nil {
-		return fmt.Errorf("could not get all params from DB while provisioning: %w", err)
-	}
 
-	err = infraProvider.ValidateParams(allParams)
+	err := infraProvider.ValidateParams(allParams)
 	if err != nil {
 		return fmt.Errorf("invalid provisioning params %v: %w", infraProviderResource.Metadata, err)
 	}
@@ -210,6 +211,7 @@ func (i InfraProvisioner) provisionNewInfrastructure(
 		InputParameters:            allParams,
 		ConfigMetadata:             configMetadata,
 		InstanceInfo:               stoppedInfra.InstanceInfo,
+		Status:                     enum.InfraStatusPending,
 	}
 	responseMetadata, err := json.Marshal(infrastructure)
 	if err != nil {
@@ -315,6 +317,7 @@ func (i InfraProvisioner) deprovisionNewInfrastructure(
 	infra types.Infrastructure,
 	canDeleteUserData bool,
 	configMetadata map[string]any,
+	params []types.InfraProviderParameter,
 ) error {
 	infraProvisionedLatest, err := i.infraProvisionedStore.FindLatestByGitspaceInstanceID(
 		ctx, gitspaceConfig.GitspaceInstance.ID)
@@ -328,7 +331,7 @@ func (i InfraProvisioner) deprovisionNewInfrastructure(
 		return nil
 	}
 
-	err = infraProvider.Deprovision(ctx, infra, canDeleteUserData, configMetadata)
+	err = infraProvider.Deprovision(ctx, infra, canDeleteUserData, configMetadata, params)
 	if err != nil {
 		return fmt.Errorf("unable to trigger deprovision infra %+v: %w", infra, err)
 	}

app/gitspace/orchestrator/orchestrator_trigger.go

@@ -344,17 +344,18 @@ func (o Orchestrator) TriggerCleanupInstanceResources(ctx context.Context, gitsp
 	return nil
 }
 
-// TriggerDeleteGitspace removes the Gitspace container and triggers infra deprovisioning to deprovision
+// TriggerStopAndDeleteGitspace removes the Gitspace container and triggers infra deprovisioning to deprovision
 // the infra resources.
 // canDeleteUserData = false -> trigger deprovision of all resources except storage associated to user data.
 // canDeleteUserData = true -> trigger deprovision of all resources.
-func (o Orchestrator) TriggerDeleteGitspace(
+func (o Orchestrator) TriggerStopAndDeleteGitspace(
 	ctx context.Context,
 	gitspaceConfig types.GitspaceConfig,
 	canDeleteUserData bool,
 ) error {
 	infra, err := o.getProvisionedInfra(ctx, gitspaceConfig,
 		[]enum.InfraStatus{
+			enum.InfraStatusPending,
 			enum.InfraStatusProvisioned,
 			enum.InfraStatusStopped,
 			enum.InfraStatusDestroyed,
@@ -368,6 +369,8 @@ func (o Orchestrator) TriggerDeleteGitspace(
 	}
 	if err = o.stopAndRemoveGitspaceContainer(ctx, gitspaceConfig, *infra, canDeleteUserData); err != nil {
 		log.Warn().Msgf("error stopping and removing gitspace container: %v", err)
+		// If stop fails, delete the gitspace anyway
+		return o.triggerDeleteGitspace(ctx, gitspaceConfig, infra, canDeleteUserData)
 	}
 
 	// TODO: Add a job for cleanup of infra if stop fails
@@ -375,22 +378,56 @@ func (o Orchestrator) TriggerDeleteGitspace(
 		"Checking and force deleting the infra if required for gitspace instance %s",
 		gitspaceConfig.GitspaceInstance.Identifier,
 	)
-	ticker := time.NewTicker(60 * time.Second)
-	timeout := time.After(15 * time.Minute)
+	if err = o.waitForGitspaceCleanupOrTimeout(ctx, gitspaceConfig, 15*time.Minute, 60*time.Second); err != nil {
+		return o.triggerDeleteGitspace(ctx, gitspaceConfig, infra, canDeleteUserData)
+	}
+	return nil
+}
+
+func (o Orchestrator) triggerDeleteGitspace(
+	ctx context.Context,
+	gitspaceConfig types.GitspaceConfig,
+	infra *types.Infrastructure,
+	canDeleteUserData bool,
+) error {
+	opts := infrastructure.InfraEventOpts{CanDeleteUserData: true}
+	err := o.infraProvisioner.TriggerInfraEventWithOpts(
+		ctx,
+		enum.InfraEventDeprovision,
+		gitspaceConfig,
+		infra,
+		opts,
+	)
+	if err != nil {
+		if canDeleteUserData {
+			o.emitGitspaceEvent(ctx, gitspaceConfig, enum.GitspaceEventTypeInfraDeprovisioningFailed)
+		} else {
+			o.emitGitspaceEvent(ctx, gitspaceConfig, enum.GitspaceEventTypeInfraResetFailed)
+		}
+		return fmt.Errorf(
+			"cannot trigger deprovision infrastructure with gitspace identifier %s: %w",
+			gitspaceConfig.GitspaceInstance.Identifier,
+			err,
+		)
+	}
+	return nil
+}
+
+func (o Orchestrator) waitForGitspaceCleanupOrTimeout(
+	ctx context.Context,
+	gitspaceConfig types.GitspaceConfig,
+	timeoutDuration time.Duration,
+	tickInterval time.Duration,
+) error {
+	ticker := time.NewTicker(tickInterval)
 	defer ticker.Stop()
-	ch := make(chan error)
+
+	timeout := time.After(timeoutDuration)
 
 	for {
 		select {
-		case msg := <-ch:
-			if msg == nil {
-				return msg
-			}
 		case <-ticker.C:
-			instance, err := o.gitspaceInstanceStore.Find(
-				ctx,
-				gitspaceConfig.GitspaceInstance.ID,
-			)
+			instance, err := o.gitspaceInstanceStore.Find(ctx, gitspaceConfig.GitspaceInstance.ID)
 			if err != nil {
 				return fmt.Errorf(
 					"failed to find gitspace instance %s: %w",
@@ -403,27 +440,10 @@ func (o Orchestrator) TriggerDeleteGitspace(
 				return nil
 			}
 		case <-timeout:
-			opts := infrastructure.InfraEventOpts{CanDeleteUserData: true}
-			err := o.infraProvisioner.TriggerInfraEventWithOpts(
-				ctx,
-				enum.InfraEventDeprovision,
-				gitspaceConfig,
-				infra,
-				opts,
+			return fmt.Errorf(
+				"timeout waiting for gitspace cleanup for instance %s",
+				gitspaceConfig.GitspaceInstance.Identifier,
 			)
-			if err != nil {
-				if canDeleteUserData {
-					o.emitGitspaceEvent(ctx, gitspaceConfig, enum.GitspaceEventTypeInfraDeprovisioningFailed)
-				} else {
-					o.emitGitspaceEvent(ctx, gitspaceConfig, enum.GitspaceEventTypeInfraResetFailed)
-				}
-				return fmt.Errorf(
-					"cannot trigger deprovision infrastructure with gitspace identifier %s: %w",
-					gitspaceConfig.GitspaceInstance.Identifier,
-					err,
-				)
-			}
-			return nil
 		}
 	}
 }

app/services/gitspace/action_delete.go

@@ -89,7 +89,7 @@ func (c *Service) RemoveGitspace(ctx context.Context, config types.GitspaceConfi
 		)
 	}
 
-	if err := c.orchestrator.TriggerDeleteGitspace(ctx, config, canDeleteUserData); err != nil {
+	if err := c.orchestrator.TriggerStopAndDeleteGitspace(ctx, config, canDeleteUserData); err != nil {
 		log.Ctx(ctx).Err(err).Msgf("error during triggering delete for gitspace instance %s",
 			config.GitspaceInstance.Identifier)
 		config.GitspaceInstance.State = enum.GitspaceInstanceStateError

infraprovider/docker_provider.go

@@ -208,6 +208,7 @@ func (d DockerProvider) Deprovision(
 	infra types.Infrastructure,
 	canDeleteUserData bool,
 	_ map[string]any,
+	_ []types.InfraProviderParameter,
 ) error {
 	if canDeleteUserData {
 		err := d.deleteVolume(ctx, infra)

infraprovider/infra_provider.go

@@ -67,6 +67,7 @@ type InfraProvider interface {
 		infra types.Infrastructure,
 		canDeleteUserData bool,
 		configMetadata map[string]any,
+		params []types.InfraProviderParameter,
 	) error
 
 	// AvailableParams provides a schema to define the infrastructure.

registry/app/api/handler/npm/download.go

@@ -57,7 +57,7 @@ func (h *handler) DownloadPackageFile(w http.ResponseWriter, r *http.Request) {
 
 	w.Header().Set("Content-Disposition", "attachment; filename="+info.Filename)
 	if response.RedirectURL != "" {
-		http.Redirect(w, r, response.RedirectURL, http.StatusTemporaryRedirect)
+		http.Redirect(w, r, response.RedirectURL, http.StatusFound)
 		return
 	}
 	err := commons.ServeContent(w, r, response.Body, info.Filename, response.ReadCloser)

registry/app/api/handler/npm/download_file.go

@@ -50,7 +50,7 @@ func (h *handler) DownloadPackageFileByName(w http.ResponseWriter, r *http.Reque
 
 	w.Header().Set("Content-Disposition", "attachment; filename="+info.Filename)
 	if response.RedirectURL != "" {
-		http.Redirect(w, r, response.RedirectURL, http.StatusTemporaryRedirect)
+		http.Redirect(w, r, response.RedirectURL, http.StatusFound)
 		return
 	}
 	h.ServeContent(w, r, response.Body, info.Filename)