fix: handling side-effects for displaying external helm apps with same name across diff namespaces and clusters (#4951)

* store display name, along with unique appName(displayName-ns-clusterId) for external helm apps (in case same name ext helm app exists across diff ns or cluster)

* comments

* appName and display name handling at other places as well for external app

* handling display name in GetHelmAppMetaInfo in AppCrudOperationService.go

* FetchAppDetailsForInstalledAppV2 send display name if present

* DeleteDBLinkedHelmApplication deletion handling for ext app with uniq app identifier and if not found by unique app identifier, then find by display name

* applist method incorporated checks on display name if exists instead of appName

* query change in GetAppAndEnvDetailsForDeploymentAppTypeInstalledApps to include also display_name

* GenerateInstallAppVersionMinDTO if display name exists then send appname as display name

* FindAppDetailsForAppstoreApplication, considering display name as well while preparing helm package name deploymentContainer.HelmPackageName

* multiple fixes around corner cases around display names

* minor fix

* fixes for fetching resoirce trees for ext apps using display name instead of appName

* bug fixes

* linkHelmApplicationToChartStore, use displayName instead of appName for UpdateApplicationWithChartInfo

* LinkHelmApplicationToChartStore: server mode full from hyperion when going to link a chart

* revert:- LinkHelmApplicationToChartStore: server mode full from hyperion when going to link a chart

* on update proj, update app offering mode to full and, getAppNameForInstalledApp changed param to installed app id from appId which was wrong

* handled corner  cases in DeleteDBLinkedHelmApplication

* code refactoring

* remove unwanted error check

* removed logic to block update and deploy when not linked to devtron

* code review incorporations and code refactoring

* handling display name in migrate app type and trigger for migrated app type

* minor fix

* minor fix

* code review incorporation and refactoring

* wire fix

* fixes for multiple installed apps with same app pointer linked to chart store

* remove unused code

* fix for showing unassigned proj to ext app that's already linked to devtron with same name

* fix

* fix

* revert

* fix

Author: prakash100198

api/appStore/InstalledAppRestHandler.go

@@ -26,6 +26,7 @@ import (
 	"github.com/devtron-labs/devtron/pkg/appStore/installedApp/service/FullMode"
 	"github.com/devtron-labs/devtron/pkg/appStore/installedApp/service/FullMode/deploymentTypeChange"
 	"github.com/devtron-labs/devtron/pkg/appStore/installedApp/service/FullMode/resource"
+	util3 "github.com/devtron-labs/devtron/pkg/appStore/util"
 	"github.com/devtron-labs/devtron/pkg/bean"
 	"net/http"
 	"strconv"
@@ -603,6 +604,10 @@ func (handler *InstalledAppRestHandlerImpl) FetchAppDetailsForInstalledApp(w htt
 		common.WriteJsonResp(w, err, "App not found in database", http.StatusBadRequest)
 		return
 	}
+	if util3.IsExternalChartStoreApp(installedApp.App.DisplayName) {
+		//this is external app case where app_name is a unique identifier, and we want to fetch resource based on display_name
+		handler.installedAppService.ChangeAppNameToDisplayNameForInstalledApp(installedApp)
+	}
 
 	appDetail, err := handler.installedAppService.FindAppDetailsForAppstoreApplication(installedAppId, envId)
 	if err != nil {
@@ -724,6 +729,10 @@ func (handler *InstalledAppRestHandlerImpl) FetchResourceTree(w http.ResponseWri
 		common.WriteJsonResp(w, err, "App not found in database", http.StatusBadRequest)
 		return
 	}
+	if util3.IsExternalChartStoreApp(installedApp.App.DisplayName) {
+		//this is external app case where app_name is a unique identifier, and we want to fetch resource based on display_name
+		handler.installedAppService.ChangeAppNameToDisplayNameForInstalledApp(installedApp)
+	}
 	if installedApp.Environment.IsVirtualEnvironment {
 		common.WriteJsonResp(w, nil, nil, http.StatusOK)
 		return

api/appStore/deployment/CommonDeploymentRestHandler.go

@@ -93,7 +93,8 @@ func (handler *CommonDeploymentRestHandlerImpl) getAppOfferingMode(installedAppI
 			err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "invalid app id"}
 			return appOfferingMode, installedAppDto, err
 		}
-		installedAppDto, err = handler.installedAppService.GetInstalledAppByClusterNamespaceAndName(appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.ReleaseName)
+		uniqueAppName := appIdentifier.GetUniqueAppNameIdentifier()
+		installedAppDto, err = handler.installedAppService.GetInstalledAppByClusterNamespaceAndName(appIdentifier.ClusterId, appIdentifier.Namespace, uniqueAppName)
 		if err != nil {
 			err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "unable to find app in database"}
 			return appOfferingMode, installedAppDto, err

api/helm-app/HelmAppRestHandler.go

@@ -224,15 +224,15 @@ func (handler *HelmAppRestHandlerImpl) GetReleaseInfo(w http.ResponseWriter, r *
 		common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
 		return
 	}
-	installedApp, err := handler.installedAppService.GetInstalledAppByClusterNamespaceAndName(appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.ReleaseName)
+	installedAppVersionDto, err := handler.installedAppService.GetReleaseInfo(appIdentifier)
 	if err != nil {
 		common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
 		return
 	}
 
 	res := &bean.ReleaseAndInstalledAppInfo{
 		ReleaseInfo:      releaseInfo,
-		InstalledAppInfo: bean.ConvertToInstalledAppInfo(installedApp),
+		InstalledAppInfo: bean.ConvertToInstalledAppInfo(installedAppVersionDto),
 	}
 
 	common.WriteJsonResp(w, err, res, http.StatusOK)

api/helm-app/HelmAppRouter.go

@@ -29,6 +29,7 @@ func (impl *HelmAppRouterImpl) InitAppListRouter(helmRouter *mux.Router) {
 	helmRouter.Path("/hibernate").HandlerFunc(impl.helmAppRestHandler.Hibernate).Methods("POST")
 	helmRouter.Path("/unhibernate").HandlerFunc(impl.helmAppRestHandler.UnHibernate).Methods("POST")
 
+	// GetReleaseInfo used only for external apps
 	helmRouter.Path("/release-info").Queries("appId", "{appId}").
 		HandlerFunc(impl.helmAppRestHandler.GetReleaseInfo).Methods("GET")
 

api/helm-app/service/HelmAppService.go

@@ -14,7 +14,6 @@ import (
 	"net/http"
 	"reflect"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/caarlos0/env/v6"
@@ -56,7 +55,7 @@ type HelmAppService interface {
 	GetValuesYaml(ctx context.Context, app *AppIdentifier) (*gRPC.ReleaseInfo, error)
 	GetDesiredManifest(ctx context.Context, app *AppIdentifier, resource *openapi.ResourceIdentifier) (*openapi.DesiredManifestResponse, error)
 	DeleteApplication(ctx context.Context, app *AppIdentifier) (*openapi.UninstallReleaseResponse, error)
-	DeleteDBLinkedHelmApplication(ctx context.Context, app *AppIdentifier, useId int32) (*openapi.UninstallReleaseResponse, error)
+	DeleteDBLinkedHelmApplication(ctx context.Context, appIdentifier *AppIdentifier, useId int32) (*openapi.UninstallReleaseResponse, error)
 	// UpdateApplication is a wrapper over helmAppClient.UpdateApplication, sends update request to kubelink for external chart store apps
 	UpdateApplication(ctx context.Context, app *AppIdentifier, request *bean.UpdateApplicationRequestDto) (*openapi.UpdateReleaseResponse, error)
 	GetDeploymentDetail(ctx context.Context, app *AppIdentifier, version int32) (*openapi.HelmAppDeploymentManifestDetail, error)
@@ -448,7 +447,56 @@ func (impl *HelmAppServiceImpl) GetDesiredManifest(ctx context.Context, app *App
 	return response, nil
 }
 
-func (impl *HelmAppServiceImpl) DeleteDBLinkedHelmApplication(ctx context.Context, app *AppIdentifier, userId int32) (*openapi.UninstallReleaseResponse, error) {
+// getInstalledAppForAppIdentifier return installed_apps for app unique identifier or releaseName/displayName whichever exists else return pg.ErrNoRows
+func (impl *HelmAppServiceImpl) getInstalledAppForAppIdentifier(appIdentifier *AppIdentifier) (*repository.InstalledApps, error) {
+	model := &repository.InstalledApps{}
+	var err error
+	//for ext apps search app from unique identifier
+	appUniqueIdentifier := appIdentifier.GetUniqueAppNameIdentifier()
+	model, err = impl.installedAppRepository.GetInstalledAppByAppName(appUniqueIdentifier)
+	if err != nil {
+		if util.IsErrNoRows(err) {
+			//if error is pg no rows, then find installed app via app.DisplayName because this can also happen that
+			//an ext-app is already linked to devtron, and it's entry in app_name col in app table will not be a unique
+			//identifier but the display name.
+			displayName := appIdentifier.ReleaseName
+			model, err = impl.installedAppRepository.GetInstalledAppByAppName(displayName)
+			if err != nil {
+				impl.logger.Errorw("error in fetching installed app from display name", "appDisplayName", displayName, "err", err)
+				return model, err
+			}
+		} else {
+			impl.logger.Errorw("error in fetching installed app by app unique identifier", "appUniqueIdentifier", appUniqueIdentifier, "err", err)
+			return model, err
+		}
+	}
+	return model, nil
+}
+
+func (impl *HelmAppServiceImpl) getAppForAppIdentifier(appIdentifier *AppIdentifier) (*app.App, error) {
+	//for ext apps search app from unique identifier
+	appUniqueIdentifier := appIdentifier.GetUniqueAppNameIdentifier()
+	model, err := impl.appRepository.FindActiveByName(appUniqueIdentifier)
+	if err != nil {
+		if util.IsErrNoRows(err) {
+			//if error is pg no rows, then find app via release name because this can also happen that a project is
+			//already assigned a project, and it's entry in app_name col in app table will not be a unique
+			//identifier but the display name i.e. release name.
+			displayName := appIdentifier.ReleaseName
+			model, err = impl.appRepository.FindActiveByName(displayName)
+			if err != nil {
+				impl.logger.Errorw("error in fetching app from display name", "appDisplayName", displayName, "err", err)
+				return nil, err
+			}
+		} else {
+			impl.logger.Errorw("error in fetching app by app unique identifier", "appUniqueIdentifier", appUniqueIdentifier, "err", err)
+			return nil, err
+		}
+	}
+	return model, nil
+}
+
+func (impl *HelmAppServiceImpl) DeleteDBLinkedHelmApplication(ctx context.Context, appIdentifier *AppIdentifier, userId int32) (*openapi.UninstallReleaseResponse, error) {
 	dbConnection := impl.appRepository.GetConnection()
 	tx, err := dbConnection.Begin()
 	if err != nil {
@@ -457,67 +505,94 @@ func (impl *HelmAppServiceImpl) DeleteDBLinkedHelmApplication(ctx context.Contex
 	}
 	// Rollback tx on error.
 	defer tx.Rollback()
+	var isAppLinkedToChartStore bool // if true, entry present in both app and installed_app table
 
-	// For Helm deployments ReleaseName is App.Name
-	model, err := impl.installedAppRepository.GetInstalledAppByAppName(app.ReleaseName)
-	if err != nil {
-		impl.logger.Errorw("error in fetching installed app", "appName", app.ReleaseName, "err", err)
+	installedAppModel, err := impl.getInstalledAppForAppIdentifier(appIdentifier)
+	if err != nil && !util.IsErrNoRows(err) {
+		impl.logger.Errorw("DeleteDBLinkedHelmApplication, error in fetching installed app for app identifier", "appIdentifier", appIdentifier, "err", err)
 		return nil, err
 	}
-
-	// If there are two releases with same name but in different namespace (eg: test -n demo-1 {Hyperion App}, test -n demo-2 {Externally Installed});
-	// And if the request is received for the externally installed app, the below condition will handle
-	if model.Environment.Namespace != app.Namespace {
-		return nil, pg.ErrNoRows
+	if installedAppModel.Id > 0 {
+		isAppLinkedToChartStore = true
 	}
 
-	// App Delete --> Start
-	//soft delete app
-	appModel := &model.App
-	appModel.Active = false
-	appModel.UpdatedBy = userId
-	appModel.UpdatedOn = time.Now()
-	err = impl.appRepository.UpdateWithTxn(appModel, tx)
-	if err != nil {
-		impl.logger.Errorw("error in deleting appModel", "app", appModel)
-		return nil, err
-	}
-	// App Delete --> End
+	if isAppLinkedToChartStore {
+		// If there are two releases with same name but in different namespace (eg: test -n demo-1 {Hyperion App}, test -n demo-2 {Externally Installed});
+		// And if the request is received for the externally installed app, the below condition will handle
+		if installedAppModel.Environment.Namespace != appIdentifier.Namespace {
+			return nil, pg.ErrNoRows
+		}
 
-	// InstalledApp Delete --> Start
-	// soft delete install app
-	model.Active = false
-	model.UpdatedBy = userId
-	model.UpdatedOn = time.Now()
-	_, err = impl.installedAppRepository.UpdateInstalledApp(model, tx)
-	if err != nil {
-		impl.logger.Errorw("error while deleting install app", "error", err)
-		return nil, err
-	}
-	// InstalledApp Delete --> End
+		// App Delete --> Start
+		//soft delete app
+		appModel := &installedAppModel.App
+		appModel.Active = false
+		appModel.UpdatedBy = userId
+		appModel.UpdatedOn = time.Now()
+		err = impl.appRepository.UpdateWithTxn(appModel, tx)
+		if err != nil {
+			impl.logger.Errorw("error in deleting appModel", "app", appModel)
+			return nil, err
+		}
+		// App Delete --> End
+
+		// InstalledApp Delete --> Start
+		// soft delete install app
+		installedAppModel.Active = false
+		installedAppModel.UpdatedBy = userId
+		installedAppModel.UpdatedOn = time.Now()
+		_, err = impl.installedAppRepository.UpdateInstalledApp(installedAppModel, tx)
+		if err != nil {
+			impl.logger.Errorw("error while deleting install app", "error", err)
+			return nil, err
+		}
+		// InstalledApp Delete --> End
 
-	// InstalledAppVersions Delete --> Start
-	models, err := impl.installedAppRepository.GetInstalledAppVersionByInstalledAppId(model.Id)
-	if err != nil {
-		impl.logger.Errorw("error while fetching install app versions", "error", err)
-		return nil, err
-	}
+		// InstalledAppVersions Delete --> Start
+		models, err := impl.installedAppRepository.GetInstalledAppVersionByInstalledAppId(installedAppModel.Id)
+		if err != nil {
+			impl.logger.Errorw("error while fetching install app versions", "error", err)
+			return nil, err
+		}
 
-	// soft delete install app versions
-	for _, item := range models {
-		item.Active = false
-		item.UpdatedBy = userId
-		item.UpdatedOn = time.Now()
-		_, err = impl.installedAppRepository.UpdateInstalledAppVersion(item, tx)
+		// soft delete install app versions
+		for _, item := range models {
+			item.Active = false
+			item.UpdatedBy = userId
+			item.UpdatedOn = time.Now()
+			_, err = impl.installedAppRepository.UpdateInstalledAppVersion(item, tx)
+			if err != nil {
+				impl.logger.Errorw("error while fetching from db", "error", err)
+				return nil, err
+			}
+		}
+		// InstalledAppVersions Delete --> End
+	} else {
+		//this means app not found in installed_apps, but a scenario where an external app is only
+		//assigned project and not linked to devtron, in that case only entry in app will be found.
+		appModel, err := impl.getAppForAppIdentifier(appIdentifier)
 		if err != nil {
-			impl.logger.Errorw("error while fetching from db", "error", err)
+			impl.logger.Errorw("DeleteDBLinkedHelmApplication, error in fetching app from appIdentifier", "appIdentifier", appIdentifier, "err", err)
 			return nil, err
 		}
+		if appModel != nil && appModel.Id > 0 {
+			// App Delete --> Start
+			//soft delete app
+			appModel.Active = false
+			appModel.UpdatedBy = userId
+			appModel.UpdatedOn = time.Now()
+			err = impl.appRepository.UpdateWithTxn(appModel, tx)
+			if err != nil {
+				impl.logger.Errorw("error in deleting appModel", "app", appModel)
+				return nil, err
+			}
+			// App Delete --> End
+		}
 	}
-	// InstalledAppVersions Delete --> End
-	res, err := impl.DeleteApplication(ctx, app)
+
+	res, err := impl.DeleteApplication(ctx, appIdentifier)
 	if err != nil {
-		impl.logger.Errorw("error in deleting helm application", "error", err, "appIdentifier", app)
+		impl.logger.Errorw("error in deleting helm application", "error", err, "appIdentifier", appIdentifier)
 		return nil, err
 	}
 
@@ -1005,29 +1080,32 @@ type AppIdentifier struct {
 	ReleaseName string `json:"releaseName"`
 }
 
+// GetUniqueAppNameIdentifier returns unique app name identifier, we store all helm releases in kubelink cache with key
+// as what is returned from this func, this is the case where an app across diff namespace or cluster can have same name,
+// so to identify then uniquely below implementation would serve as good unique identifier for an external app.
+func (r *AppIdentifier) GetUniqueAppNameIdentifier() string {
+	return fmt.Sprintf("%s-%s-%s", r.ReleaseName, r.Namespace, strconv.Itoa(r.ClusterId))
+}
+
+func (r *AppIdentifier) GetUniqueAppIdentifierForGivenNamespaceAndCluster(namespace, clusterId string) string {
+	return fmt.Sprintf("%s-%s-%s", r.ReleaseName, namespace, clusterId)
+}
+
 func (impl *HelmAppServiceImpl) DecodeAppId(appId string) (*AppIdentifier, error) {
-	component := strings.Split(appId, "|")
-	if len(component) != 3 {
-		return nil, fmt.Errorf("malformed app id %s", appId)
-	}
-	clusterId, err := strconv.Atoi(component[0])
-	if err != nil {
-		return nil, err
-	}
-	if clusterId <= 0 {
-		return nil, fmt.Errorf("target cluster is not provided")
-	}
-	return &AppIdentifier{
-		ClusterId:   clusterId,
-		Namespace:   component[1],
-		ReleaseName: component[2],
-	}, nil
+	return DecodeExternalAppAppId(appId)
 }
 
 func (impl *HelmAppServiceImpl) EncodeAppId(appIdentifier *AppIdentifier) string {
 	return fmt.Sprintf("%d|%s|%s", appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.ReleaseName)
 }
 
+func isSameAppName(deployedAppName string, appDto app.App) bool {
+	if len(appDto.DisplayName) > 0 {
+		return deployedAppName == appDto.DisplayName
+	}
+	return deployedAppName == appDto.AppName
+}
+
 func (impl *HelmAppServiceImpl) appListRespProtoTransformer(deployedApps *gRPC.DeployedAppList, token string, helmAuth func(token string, object string) bool, helmCdPipelines []*pipelineConfig.Pipeline, installedHelmApps []*repository.InstalledApps) openapi.AppList {
 	applicationType := "HELM-APP"
 	appList := openapi.AppList{ClusterIds: &[]int32{deployedApps.ClusterId}, ApplicationType: &applicationType}
@@ -1055,7 +1133,7 @@ func (impl *HelmAppServiceImpl) appListRespProtoTransformer(deployedApps *gRPC.D
 
 			// do not add helm apps in the list which are created using app_store
 			for _, installedHelmApp := range installedHelmApps {
-				if deployedapp.AppName == installedHelmApp.App.AppName && int(deployedapp.EnvironmentDetail.ClusterId) == installedHelmApp.Environment.ClusterId && deployedapp.EnvironmentDetail.Namespace == installedHelmApp.Environment.Namespace {
+				if isSameAppName(deployedapp.AppName, installedHelmApp.App) && int(deployedapp.EnvironmentDetail.ClusterId) == installedHelmApp.Environment.ClusterId && deployedapp.EnvironmentDetail.Namespace == installedHelmApp.Environment.Namespace {
 					toExcludeFromList = true
 					break
 				}

api/helm-app/service/helper.go

@@ -0,0 +1,26 @@
+package service
+
+import (
+	"fmt"
+	"strconv"
+	"strings"
+)
+
+func DecodeExternalAppAppId(appId string) (*AppIdentifier, error) {
+	component := strings.Split(appId, "|")
+	if len(component) != 3 {
+		return nil, fmt.Errorf("malformed app id %s", appId)
+	}
+	clusterId, err := strconv.Atoi(component[0])
+	if err != nil {
+		return nil, err
+	}
+	if clusterId <= 0 {
+		return nil, fmt.Errorf("target cluster is not provided")
+	}
+	return &AppIdentifier{
+		ClusterId:   clusterId,
+		Namespace:   component[1],
+		ReleaseName: component[2],
+	}, nil
+}

api/restHandler/app/appList/AppListingRestHandler.go

@@ -28,6 +28,7 @@ import (
 	argoApplication "github.com/devtron-labs/devtron/client/argocdServer/bean"
 	"github.com/devtron-labs/devtron/pkg/appStore/installedApp/service/FullMode"
 	"github.com/devtron-labs/devtron/pkg/appStore/installedApp/service/FullMode/resource"
+	util4 "github.com/devtron-labs/devtron/pkg/appStore/util"
 	bean2 "github.com/devtron-labs/devtron/pkg/cluster/repository/bean"
 	"net/http"
 	"strconv"
@@ -909,6 +910,10 @@ func (handler AppListingRestHandlerImpl) GetHostUrlsByBatch(w http.ResponseWrite
 			common.WriteJsonResp(w, err, "App not found in database", http.StatusBadRequest)
 			return
 		}
+		if util4.IsExternalChartStoreApp(installedApp.App.DisplayName) {
+			//this is external app case where app_name is a unique identifier, and we want to fetch resource based on display_name
+			handler.installedAppService.ChangeAppNameToDisplayNameForInstalledApp(installedApp)
+		}
 		resourceTreeAndNotesContainer := bean.AppDetailsContainer{}
 		resourceTreeAndNotesContainer, err = handler.fetchResourceTreeFromInstallAppService(w, r, resourceTreeAndNotesContainer, *installedApp)
 		if err != nil {