fix: update same-name Helm images by image path

pkg/microservice/aslan/core/common/repository/models/service.go

@@ -166,6 +166,14 @@ type ImagePathSpec struct {
 	Tag       string `bson:"tag,omitempty"            json:"tag,omitempty"`
 }
 
+// GetKey returns a stable identity for one image location in Helm values.
+func (i *ImagePathSpec) GetKey() string {
+	if i == nil {
+		return ""
+	}
+	return fmt.Sprintf("%s\x00%s\x00%s\x00%s", i.Repo, i.Namespace, i.Image, i.Tag)
+}
+
 // Container ...
 type Container struct {
 	Name      string                `bson:"name"                          json:"name"`
@@ -175,6 +183,19 @@ type Container struct {
 	ImagePath *ImagePathSpec        `bson:"image_path,omitempty"          json:"image_path,omitempty"`
 }
 
+// GetKey distinguishes containers with the same name at different Helm
+// values paths. For non-Helm containers, the nil image path keeps the legacy
+// name-only identity.
+func (c *Container) GetKey() string {
+	if c == nil {
+		return ""
+	}
+	if c.ImagePath == nil {
+		return c.Name
+	}
+	return fmt.Sprintf("%s\x00%s", c.Name, c.ImagePath.GetKey())
+}
+
 // ServiceTmplPipeResp ...router
 type ServiceTmplPipeResp struct {
 	ID               ServiceTmplRevision `bson:"_id"                            json:"_id"`

pkg/microservice/aslan/core/common/repository/models/service_module.go

@@ -36,8 +36,9 @@ import (
 //     Manual records carry RevisionBound = 0 — they are version-agnostic and
 //     load for every revision of the service.
 //
-// Read-merge rule (see ResolveServiceModules): records are unioned by Name with
-// time-of-creation precedence ("first-come-first-served" by CreateTime).
+// Read-merge rule (see ResolveServiceModules): records are unioned by Name and
+// ImagePath with time-of-creation precedence ("first-come-first-served" by
+// CreateTime).
 // Conflicts are recorded and surfaced to the caller so the UI can warn users.
 type ServiceModule struct {
 	ID          primitive.ObjectID `bson:"_id,omitempty"             json:"id,omitempty"`

pkg/microservice/aslan/core/common/service/helm/helm.go

@@ -375,6 +375,46 @@ func NewHelmDeployService() *HelmDeployService {
 	return &HelmDeployService{}
 }
 
+// MergeHelmContainers keeps existing environment images while adding or
+// refreshing the containers parsed from the selected service revision.
+func MergeHelmContainers(current, templates []*commonmodels.Container) []*commonmodels.Container {
+	currentByKey := make(map[string]*commonmodels.Container, len(current))
+	currentByName := make(map[string]*commonmodels.Container, len(current))
+	for _, container := range current {
+		if container == nil {
+			continue
+		}
+		currentByKey[container.GetKey()] = container
+		if currentByName[container.Name] == nil {
+			currentByName[container.Name] = container
+		}
+	}
+
+	for _, template := range templates {
+		if template == nil {
+			continue
+		}
+
+		if existing := currentByKey[template.GetKey()]; existing != nil {
+			existing.ImagePath = template.ImagePath
+			existing.Type = template.Type
+			if existing.ImageName == "" {
+				existing.ImageName = template.ImageName
+			}
+			continue
+		}
+		if existing := currentByName[template.Name]; existing != nil {
+			template.Image = existing.Image
+			if template.ImageName == "" {
+				template.ImageName = existing.ImageName
+			}
+		}
+		currentByKey[template.GetKey()] = template
+		current = append(current, template)
+	}
+	return current
+}
+
 // GeneMergedValues generate values.yaml used to install or upgrade helm chart, like param in after option -f
 // defaultValues: global values yaml
 // productSvc: environment service, contains service's values yaml, override kvs and zadig recorded containers. And productSvc will be updated with correct image and values yaml in this function
@@ -421,21 +461,19 @@ func (s *HelmDeployService) GenMergedValues(productSvc *commonmodels.ProductServ
 		}
 
 		name := commonutil.ExtractImageName(imageUrl)
-
-		if container.ImageName == name {
-			// find corresponding image in values
-			if imageMap[name] != "" {
-				// if found image in images, and the images are from build job, we should override it
-				container.Image = imageMap[name]
-				mergedContainers = append(mergedContainers, container)
-			}
-		} else {
-			// not found corresponding image in values
-			// add container image into values
-			if imageMap[container.ImageName] != "" {
-				// if found image in images, and the images are from build job, we should override it
-				container.Image = imageMap[container.ImageName]
+		deployImage := imageMap[container.ImageName]
+
+		// Write back when the values path is newly introduced, its image differs
+		// from the recorded environment image, or a deploy image is supplied.
+		shouldWriteBack := container.ImageName != name || container.Image != imageUrl || deployImage != ""
+		if shouldWriteBack {
+			// Deploy images are matched by image name, intentionally updating every
+			// same-name container even when they use different values paths.
+			if deployImage != "" {
+				container.Image = deployImage
 			}
+			// Step 2 writes the selected image back through this container's own
+			// ImagePath, including paths newly introduced by a service revision.
 			mergedContainers = append(mergedContainers, container)
 		}
 	}
@@ -540,41 +578,30 @@ func (s *HelmDeployService) GenNewEnvService(prod *commonmodels.Product, service
 		if err != nil {
 			return nil, nil, errors.Wrapf(err, "failed to find service %s/%d in product %s", serviceName, svcFindOption.Revision, prod.ProductName)
 		}
+	}
 
-		// Service.Containers no longer persisted — pull modules for the
-		// loaded template revision from the service_module table.
-		tmplContainers, _, rerr := repository.ResolveServiceModules(context.Background(), tmplSvc.ProductName, tmplSvc.ServiceName, prod.Production, tmplSvc.Revision)
-		if rerr != nil {
-			return nil, nil, errors.Wrapf(rerr, "failed to resolve modules for %s/%s rev %d", tmplSvc.ProductName, tmplSvc.ServiceName, tmplSvc.Revision)
+	// Service.Containers is no longer persisted in the template. Resolve the
+	// selected service revision from service_module; when updateServiceRevision
+	// is false, tmplSvc is pinned to the environment's current revision.
+	tmplContainers, _, rerr := repository.ResolveServiceModules(context.Background(), tmplSvc.ProductName, tmplSvc.ServiceName, prod.Production, tmplSvc.Revision)
+	if rerr != nil {
+		return nil, nil, errors.Wrapf(rerr, "failed to resolve modules for %s/%s rev %d", tmplSvc.ProductName, tmplSvc.ServiceName, tmplSvc.Revision)
+	}
+	if prodSvc == nil {
+		prodSvc = &commonmodels.ProductService{
+			ServiceName: serviceName,
+			ReleaseName: serviceName,
+			ProductName: prod.ProductName,
+			Type:        tmplSvc.Type,
+			Revision:    tmplSvc.Revision,
+			Containers:  tmplContainers,
 		}
-		if prodSvc == nil {
-			prodSvc = &commonmodels.ProductService{
-				ServiceName: serviceName,
-				ReleaseName: serviceName,
-				ProductName: prod.ProductName,
-				Type:        tmplSvc.Type,
-				Revision:    tmplSvc.Revision,
-				Containers:  tmplContainers,
-			}
-		} else {
+	} else {
+		if updateServiceRevision {
 			prodSvc.Revision = tmplSvc.Revision
-
-			containerMap := make(map[string]*commonmodels.Container)
-			for _, container := range prodSvc.Containers {
-				containerMap[container.Name] = container
-			}
-
-			for _, templateContainer := range tmplContainers {
-				if containerMap[templateContainer.Name] == nil {
-					prodSvc.Containers = append(prodSvc.Containers, templateContainer)
-				} else {
-					if templateContainer.ImagePath != nil {
-						containerMap[templateContainer.Name].ImagePath = templateContainer.ImagePath
-					}
-					containerMap[templateContainer.Name].Type = templateContainer.Type
-				}
-			}
 		}
+
+		prodSvc.Containers = MergeHelmContainers(prodSvc.Containers, tmplContainers)
 	}
 	return prodSvc, tmplSvc, nil
 }

pkg/microservice/aslan/core/common/service/kube/helm.go

@@ -973,23 +973,7 @@ func BuildInstallParam(defaultValues string, productInfo *commonmodels.Product,
 		if err != nil {
 			return ret, fmt.Errorf("failed to resolve modules for %s/%s rev %d: %w", templateSvc.ProductName, templateSvc.ServiceName, templateSvc.Revision, err)
 		}
-		containerMap := make(map[string]*commonmodels.Container)
-		for _, container := range productSvc.Containers {
-			containerMap[container.Name] = container
-		}
-		for _, tmplContainer := range tmplContainers {
-			if containerMap[tmplContainer.Name] == nil {
-				productSvc.Containers = append(productSvc.Containers, tmplContainer)
-				continue
-			}
-			if tmplContainer.ImagePath != nil {
-				containerMap[tmplContainer.Name].ImagePath = tmplContainer.ImagePath
-			}
-			containerMap[tmplContainer.Name].Type = tmplContainer.Type
-			if containerMap[tmplContainer.Name].ImageName == "" {
-				containerMap[tmplContainer.Name].ImageName = tmplContainer.ImageName
-			}
-		}
+		productSvc.Containers = helmservice.MergeHelmContainers(productSvc.Containers, tmplContainers)
 		ret.ServiceObj = templateSvc
 		ret.ReleaseName = util.GeneReleaseName(templateSvc.GetReleaseNaming(), templateSvc.ProductName, namespace, envName, templateSvc.ServiceName)
 	} else {

pkg/microservice/aslan/core/common/service/repository/service_module.go

@@ -25,8 +25,8 @@ import (
 	"github.com/koderover/zadig/v2/pkg/microservice/aslan/core/common/repository/mongodb"
 )
 
-// ModuleConflict reports that two records share a Name. The Winner is the
-// record currently in effect under the precedence rule (see
+// ModuleConflict reports that two records share a Name and ImagePath. The
+// Winner is the record currently in effect under the precedence rule (see
 // ResolveServiceModules); Shadowed lists the records that were displaced.
 // Callers (typically API handlers) should surface these to the UI so users
 // can disambiguate — e.g., "module 'api' has both a manual declaration and
@@ -42,9 +42,9 @@ type ModuleConflict struct {
 // underlying collection (service_module vs production_service_module).
 //
 // Merge rule: time-of-creation precedence ("first-come-first-served"). For
-// every Name in the union of (manual records, auto records bound to
-// `revision`), the record with the smallest CreateTime wins; later records
-// with the same Name are reported as shadowed. Ties (same CreateTime) are
+// every Name + ImagePath in the union of (manual records, auto records bound
+// to `revision`), the record with the smallest CreateTime wins; later records
+// with the same identity are reported as shadowed. Ties (same CreateTime) are
 // broken by ObjectID order — deterministic, but "shouldn't happen in
 // practice."
 //
@@ -252,10 +252,11 @@ func mergeServiceModules(records []*models.ServiceModule) []*models.Container {
 	winners := make([]*models.Container, 0, len(records))
 	seen := make(map[string]struct{}, len(records))
 	for _, r := range records {
-		if _, ok := seen[r.Name]; ok {
+		key := serviceModuleKey(r)
+		if _, ok := seen[key]; ok {
 			continue
 		}
-		seen[r.Name] = struct{}{}
+		seen[key] = struct{}{}
 		winners = append(winners, &models.Container{
 			Name:      r.Name,
 			Type:      r.Type,
@@ -267,30 +268,37 @@ func mergeServiceModules(records []*models.ServiceModule) []*models.Container {
 	return winners
 }
 
+func serviceModuleKey(module *models.ServiceModule) string {
+	if module == nil {
+		return ""
+	}
+	return (&models.Container{Name: module.Name, ImagePath: module.ImagePath}).GetKey()
+}
+
 // TODO: convenience wrapper ResolveServiceModulesFor(ctx, svc, production)
 // that infers project/service/revision from svc — deferred per discussion,
 // callers will pass the explicit args until usage volume justifies it.
 
-// conflictsFromMerge walks the same pre-sorted slice and groups shadowed
-// records under their winning entry. Returned slice is empty when there are
-// no name collisions.
+// conflictsFromMerge walks the same pre-sorted slice and groups records with
+// the same name and image path under their winning entry.
 func conflictsFromMerge(records []*models.ServiceModule) []ModuleConflict {
-	byName := make(map[string][]*models.ServiceModule, len(records))
+	byKey := make(map[string][]*models.ServiceModule, len(records))
 	order := make([]string, 0, len(records))
 	for _, r := range records {
-		if _, ok := byName[r.Name]; !ok {
-			order = append(order, r.Name)
+		key := serviceModuleKey(r)
+		if _, ok := byKey[key]; !ok {
+			order = append(order, key)
 		}
-		byName[r.Name] = append(byName[r.Name], r)
+		byKey[key] = append(byKey[key], r)
 	}
 	conflicts := make([]ModuleConflict, 0)
-	for _, name := range order {
-		group := byName[name]
+	for _, key := range order {
+		group := byKey[key]
 		if len(group) < 2 {
 			continue
 		}
 		conflicts = append(conflicts, ModuleConflict{
-			Name:     name,
+			Name:     group[0].Name,
 			Winner:   group[0],
 			Shadowed: group[1:],
 		})

pkg/microservice/aslan/core/environment/service/environment.go

@@ -1111,6 +1111,23 @@ func updateHelmProduct(productName, envName, username, requestID string, overrid
 				continue
 			}
 
+			curUsedSvc := serviceMap[svr.ServiceName]
+			if curUsedSvc != nil {
+				// Service.Containers is not persisted, so resolve the deployed revision's modules for the baseline.
+				curUsedMerged, _, rerr := repository.ResolveServiceModules(
+					context.Background(),
+					curUsedSvc.ProductName,
+					curUsedSvc.ServiceName,
+					productResp.Production,
+					curUsedSvc.Revision,
+				)
+				if rerr != nil {
+					log.Errorf("failed to resolve current service modules for %s/%s rev %d: %s", curUsedSvc.ProductName, curUsedSvc.ServiceName, curUsedSvc.Revision, rerr)
+				} else {
+					curUsedSvc.Containers = curUsedMerged
+				}
+			}
+
 			svr.Containers = kube.CalculateContainer(ps, serviceMap[svr.ServiceName], svr.Containers, productResp)
 		}
 		allServices = append(allServices, svcGroup)