Fix bug where custom helm values for services were being wiped when d…

…eployed from the frontend (#3579)
porter-dev · Sep 15, 2023 · a0e1b3d · a0e1b3d
1 parent 29f4853
commit a0e1b3d
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 39 deletions.
diff --git a/api/server/handlers/porter_app/create.go b/api/server/handlers/porter_app/create.go
@@ -106,30 +106,29 @@ func (c *CreatePorterAppHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques
 
 	var releaseValues map[string]interface{}
 	var releaseDependencies []*chart.Dependency
-	if shouldCreate || request.OverrideRelease {
-		releaseValues = nil
-		releaseDependencies = nil
-
-		// this is required because when the front-end sends an update request with overrideRelease=true, it is unable to
-		// get the image info from the release. unless it is explicitly provided in the request, we avoid overwriting it
-		// by attempting to get the image info from the release or the provided helm values
-		if helmRelease != nil && (imageInfo.Repository == "" || imageInfo.Tag == "") {
-			if request.FullHelmValues != "" {
-				imageInfo, err = attemptToGetImageInfoFromFullHelmValues(request.FullHelmValues)
-				if err != nil {
-					err = telemetry.Error(ctx, span, err, "error getting image info from full helm values")
-					telemetry.WithAttributes(span, telemetry.AttributeKV{Key: "porter-yaml-base64", Value: porterYamlBase64})
-					c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(err, http.StatusBadRequest))
-					return
-				}
-			} else {
-				imageInfo = attemptToGetImageInfoFromRelease(helmRelease.Config)
+	// unless it is explicitly provided in the request, we avoid overwriting the image info
+	// by attempting to get it from the release or the provided helm values
+	if helmRelease != nil && (imageInfo.Repository == "" || imageInfo.Tag == "") {
+		if request.FullHelmValues != "" {
+			imageInfo, err = attemptToGetImageInfoFromFullHelmValues(request.FullHelmValues)
+			if err != nil {
+				err = telemetry.Error(ctx, span, err, "error getting image info from full helm values")
+				telemetry.WithAttributes(span, telemetry.AttributeKV{Key: "porter-yaml-base64", Value: porterYamlBase64})
+				c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(err, http.StatusBadRequest))
+				return
 			}
+		} else {
+			imageInfo = attemptToGetImageInfoFromRelease(helmRelease.Config)
 		}
+	}
+	if shouldCreate {
+		releaseValues = nil
+		releaseDependencies = nil
 	} else {
 		releaseValues = helmRelease.Config
 		releaseDependencies = helmRelease.Chart.Metadata.Dependencies
 	}
+
 	telemetry.WithAttributes(span, telemetry.AttributeKV{Key: "image-repo", Value: imageInfo.Repository}, telemetry.AttributeKV{Key: "image-tag", Value: imageInfo.Tag})
 
 	if request.Builder == "" {
@@ -192,6 +191,7 @@ func (c *CreatePorterAppHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques
 			ShouldValidateHelmValues:     shouldCreate,
 			FullHelmValues:               request.FullHelmValues,
 			AddCustomNodeSelector:        addCustomNodeSelector,
+			RemoveDeletedServices:        request.OverrideRelease,
 		},
 	)
 	if err != nil {

diff --git a/api/server/handlers/porter_app/parse.go b/api/server/handlers/porter_app/parse.go
@@ -111,6 +111,8 @@ type ParseConf struct {
 	FullHelmValues string
 	// AddCustomNodeSelector is a flag to determine whether to add porter.run/workload-kind: application to the nodeselector attribute of the helm values
 	AddCustomNodeSelector bool
+	// RemoveDeletedServices is a flag to determine whether to remove values and dependencies for services that are not defined in the porter.yaml
+	RemoveDeletedServices bool
 }
 
 func parse(ctx context.Context, conf ParseConf) (*chart.Chart, map[string]interface{}, map[string]interface{}, error) {
@@ -199,7 +201,7 @@ func parse(ctx context.Context, conf ParseConf) (*chart.Chart, map[string]interf
 		Release:  parsed.Release,
 	}
 
-	values, err := buildUmbrellaChartValues(ctx, application, synced_env, conf.ImageInfo, conf.ExistingHelmValues, conf.SubdomainCreateOpts, conf.InjectLauncherToStartCommand, conf.ShouldValidateHelmValues, conf.UserUpdate, conf.Namespace, conf.AddCustomNodeSelector)
+	values, err := buildUmbrellaChartValues(ctx, application, synced_env, conf.ImageInfo, conf.ExistingHelmValues, conf.SubdomainCreateOpts, conf.InjectLauncherToStartCommand, conf.ShouldValidateHelmValues, conf.UserUpdate, conf.Namespace, conf.AddCustomNodeSelector, conf.RemoveDeletedServices)
 	if err != nil {
 		err = telemetry.Error(ctx, span, err, "error building values")
 		return nil, nil, nil, err
@@ -210,7 +212,7 @@ func parse(ctx context.Context, conf ParseConf) (*chart.Chart, map[string]interf
 		return nil, nil, nil, err
 	}
 
-	umbrellaChart, err := buildUmbrellaChart(application, conf.ServerConfig, conf.ProjectID, conf.ExistingChartDependencies)
+	umbrellaChart, err := buildUmbrellaChart(application, conf.ServerConfig, conf.ProjectID, conf.ExistingChartDependencies, conf.RemoveDeletedServices)
 	if err != nil {
 		err = telemetry.Error(ctx, span, err, "error building umbrella chart")
 		return nil, nil, nil, err
@@ -238,6 +240,7 @@ func buildUmbrellaChartValues(
 	userUpdate bool,
 	namespace string,
 	addCustomNodeSelector bool,
+	removeDeletedValues bool,
 ) (map[string]interface{}, error) {
 	values := make(map[string]interface{})
 
@@ -290,10 +293,12 @@ func buildUmbrellaChartValues(
 		values[helmName] = helm_values
 	}
 
-	// add back in the existing services that were not overwritten
-	for k, v := range existingValues {
-		if values[k] == nil {
-			values[k] = v
+	if !removeDeletedValues {
+		// add back in the existing services that were not overwritten
+		for k, v := range existingValues {
+			if values[k] == nil {
+				values[k] = v
+			}
 		}
 	}
 
@@ -505,7 +510,7 @@ func deconstructSyncedEnvs(synced_env []*SyncedEnvSection, env map[string]string
 	return synced
 }
 
-func buildUmbrellaChart(application *Application, config *config.Config, projectID uint, existingDependencies []*chart.Dependency) (*chart.Chart, error) {
+func buildUmbrellaChart(application *Application, config *config.Config, projectID uint, existingDependencies []*chart.Dependency, removeDeletedDependencies bool) (*chart.Chart, error) {
 	deps := make([]*chart.Dependency, 0)
 	for alias, service := range application.Services {
 		var serviceType string
@@ -540,22 +545,24 @@ func buildUmbrellaChart(application *Application, config *config.Config, project
 		})
 	}
 
-	// add in the existing dependencies that were not overwritten
-	for _, dep := range existingDependencies {
-		if !dependencyExists(deps, dep) {
-			// have to repair the dependency name because of https://github.com/helm/helm/issues/9214
-			if strings.HasSuffix(dep.Name, "-web") || strings.HasSuffix(dep.Name, "-wkr") || strings.HasSuffix(dep.Name, "-job") {
-				dep.Name = getChartTypeFromHelmName(dep.Name)
-				if dep.Name == "" {
-					return nil, fmt.Errorf("unable to determine type of existing dependency")
-				}
-				version, err := getLatestTemplateVersion(dep.Name, config, projectID)
-				if err != nil {
-					return nil, err
+	if !removeDeletedDependencies {
+		// add in the existing dependencies that were not overwritten
+		for _, dep := range existingDependencies {
+			if !dependencyExists(deps, dep) {
+				// have to repair the dependency name because of https://github.com/helm/helm/issues/9214
+				if strings.HasSuffix(dep.Name, "-web") || strings.HasSuffix(dep.Name, "-wkr") || strings.HasSuffix(dep.Name, "-job") {
+					dep.Name = getChartTypeFromHelmName(dep.Name)
+					if dep.Name == "" {
+						return nil, fmt.Errorf("unable to determine type of existing dependency")
+					}
+					version, err := getLatestTemplateVersion(dep.Name, config, projectID)
+					if err != nil {
+						return nil, err
+					}
+					dep.Version = version
 				}
-				dep.Version = version
+				deps = append(deps, dep)
 			}
-			deps = append(deps, dep)
 		}
 	}