From 0d1e1354b8a3b73084f5dc16e34f7c37da9b78c1 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Tue, 16 Apr 2024 18:00:20 +0530 Subject: [PATCH 01/35] 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) --- .../AppStoreDeploymentRestHandler.go | 2 ++ api/helm-app/service/HelmAppService.go | 4 +++ .../AppListingRepositoryQueryBuilder.go | 2 ++ pkg/appStore/bean/bean.go | 12 ++++---- .../service/AppStoreDeploymentDBService.go | 19 ++++++++++-- .../service/AppStoreDeploymentService.go | 30 +++++++++---------- pkg/bean/app.go | 1 + 7 files changed, 48 insertions(+), 22 deletions(-) diff --git a/api/appStore/deployment/AppStoreDeploymentRestHandler.go b/api/appStore/deployment/AppStoreDeploymentRestHandler.go index 0c739e360c..87b7db2f72 100644 --- a/api/appStore/deployment/AppStoreDeploymentRestHandler.go +++ b/api/appStore/deployment/AppStoreDeploymentRestHandler.go @@ -579,6 +579,7 @@ func (handler AppStoreDeploymentRestHandlerImpl) UpdateProjectHelmApp(w http.Res handler.Logger.Errorw("error in decoding app id", "err", err) common.WriteJsonResp(w, err, "error in decoding app id", http.StatusBadRequest) } + request.UniqueAppIdentifier = appIdentifier.GetUniqueAppNameIdentifier() // this rbac object checks that whether user have permission to change current project. rbacObjectForCurrentProject, rbacObjectForCurrentProject2 := handler.enforcerUtilHelm.GetHelmObjectByClusterIdNamespaceAndAppName(appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.ReleaseName) ok := handler.enforcer.Enforce(token, casbin.ResourceHelmApp, casbin.ActionUpdate, rbacObjectForCurrentProject) || handler.enforcer.Enforce(token, casbin.ResourceHelmApp, casbin.ActionUpdate, rbacObjectForCurrentProject2) @@ -595,6 +596,7 @@ func (handler AppStoreDeploymentRestHandlerImpl) UpdateProjectHelmApp(w http.Res handler.Logger.Errorw("service err, InstalledAppId", "err", err, "InstalledAppId", request.InstalledAppId) common.WriteJsonResp(w, fmt.Errorf("Unable to fetch installed app details"), nil, http.StatusBadRequest) } + request.UniqueAppIdentifier = installedApp.AppName if installedApp.IsVirtualEnvironment { rbacObjectForCurrentProject, _ := handler.enforcerUtilHelm.GetAppRBACNameByInstalledAppId(request.InstalledAppId) ok := handler.enforcer.Enforce(token, casbin.ResourceHelmApp, casbin.ActionUpdate, rbacObjectForCurrentProject) diff --git a/api/helm-app/service/HelmAppService.go b/api/helm-app/service/HelmAppService.go index f6f19732ce..41d9f26093 100644 --- a/api/helm-app/service/HelmAppService.go +++ b/api/helm-app/service/HelmAppService.go @@ -1000,6 +1000,10 @@ type AppIdentifier struct { ReleaseName string `json:"releaseName"` } +func (r *AppIdentifier) GetUniqueAppNameIdentifier() string { + return r.ReleaseName + "-" + r.Namespace + "-" + strconv.Itoa(r.ClusterId) +} + func (impl *HelmAppServiceImpl) DecodeAppId(appId string) (*AppIdentifier, error) { component := strings.Split(appId, "|") if len(component) != 3 { diff --git a/internal/sql/repository/helper/AppListingRepositoryQueryBuilder.go b/internal/sql/repository/helper/AppListingRepositoryQueryBuilder.go index 655bf2b314..4a9b9acf26 100644 --- a/internal/sql/repository/helper/AppListingRepositoryQueryBuilder.go +++ b/internal/sql/repository/helper/AppListingRepositoryQueryBuilder.go @@ -31,6 +31,8 @@ const ( CustomApp AppType = 0 // cicd app ChartStoreApp AppType = 1 // helm app Job AppType = 2 // jobs + // ExternalChartStoreApp app-type is not stored in db + ExternalChartStoreApp AppType = 3 // external helm app ) type AppListingRepositoryQueryBuilder struct { diff --git a/pkg/appStore/bean/bean.go b/pkg/appStore/bean/bean.go index ce81af93b2..d28ac552c3 100644 --- a/pkg/appStore/bean/bean.go +++ b/pkg/appStore/bean/bean.go @@ -116,6 +116,7 @@ type InstallAppVersionDTO struct { EnvironmentName string `json:"-"` InstallAppVersionChartDTO *InstallAppVersionChartDTO `json:"-"` AppStoreApplicationVersionId int + DisplayName string `json:"-"` } // UpdateDeploymentAppType updates deploymentAppType to InstallAppVersionDTO @@ -351,11 +352,12 @@ type ChartRepoSearch struct { } type UpdateProjectHelmAppDTO struct { - AppId string `json:"appId"` - InstalledAppId int `json:"installedAppId"` - AppName string `json:"appName"` - TeamId int `json:"teamId"` - UserId int32 `json:"userId"` + AppId string `json:"appId"` + InstalledAppId int `json:"installedAppId"` + AppName string `json:"appName"` + TeamId int `json:"teamId"` + UserId int32 `json:"userId"` + UniqueAppIdentifier string `json:"-"` } type AppstoreDeploymentStatus int diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go index 0caf8fbf91..f87b9f8eca 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go @@ -42,7 +42,7 @@ type AppStoreDeploymentDBService interface { // UpdateInstalledAppVersionHistoryWithGitHash updates GitHash in the repository.InstalledAppVersionHistory UpdateInstalledAppVersionHistoryWithGitHash(versionHistoryId int, gitHash string, userId int32) error // UpdateProjectForHelmApp updates TeamId in the app.App - UpdateProjectForHelmApp(appName string, teamId int, userId int32) error + UpdateProjectForHelmApp(appName, displayName string, teamId int, userId int32) error // InstallAppPostDbOperation is used to perform Post-Install DB operations in App Store deployments InstallAppPostDbOperation(installAppVersionRequest *appStoreBean.InstallAppVersionDTO) error // MarkInstalledAppVersionsInactiveByInstalledAppId will mark the repository.InstalledAppVersions inactive for the given InstalledAppId @@ -115,6 +115,11 @@ func (impl *AppStoreDeploymentDBServiceImpl) AppStoreDeployOperationDB(installRe TeamId: installRequest.TeamId, UserId: installRequest.UserId, } + if len(installRequest.DisplayName) > 0 { + //this is the case of linking external helm app to devtron chart store + appCreateRequest.AppType = helper.ExternalChartStoreApp + appCreateRequest.DisplayName = installRequest.DisplayName + } appCreateRequest, err = impl.createAppForAppStore(appCreateRequest, tx, getAppInstallationMode(installRequest.AppOfferingMode)) if err != nil { impl.logger.Errorw("error while creating app", "error", err) @@ -317,7 +322,7 @@ func (impl *AppStoreDeploymentDBServiceImpl) UpdateInstalledAppVersionHistoryWit return nil } -func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName string, teamId int, userId int32) error { +func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, displayName string, teamId int, userId int32) error { appModel, err := impl.appRepository.FindActiveByName(appName) if err != nil && !util.IsErrNoRows(err) { impl.logger.Errorw("error in fetching appModel", "err", err) @@ -344,6 +349,10 @@ func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName str UserId: userId, TeamId: teamId, } + if len(displayName) > 0 { + createAppRequest.AppType = helper.ExternalChartStoreApp + createAppRequest.DisplayName = displayName + } _, err = impl.createAppForAppStore(&createAppRequest, tx, appInstallationMode) if err != nil { impl.logger.Errorw("error while creating appModel", "error", err) @@ -497,6 +506,12 @@ func (impl *AppStoreDeploymentDBServiceImpl) createAppForAppStore(createRequest AppType: helper.ChartStoreApp, AppOfferingMode: appInstallationMode, } + if createRequest.AppType == helper.ExternalChartStoreApp { + //when linking ext helm app to chart store, there can be a case that two (or more) external apps can have same name, in diff namespaces or diff + //clusters, so now we are storing display_name also to get rid of multiple installed apps pointing to the same app, which caused unwarranted + //behaviours. appName in this case will be displayName-namespace-clusterId + appModel.DisplayName = createRequest.DisplayName + } appModel.CreateAuditLog(createRequest.UserId) err = impl.appRepository.SaveWithTxn(appModel, tx) if err != nil { diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go index fb45f3f076..2ec0ae5de7 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go @@ -363,7 +363,7 @@ func (impl *AppStoreDeploymentServiceImpl) LinkHelmApplicationToChartStore(ctx c // Initialise bean installAppVersionRequestDto := &appStoreBean.InstallAppVersionDTO{ - AppName: appIdentifier.ReleaseName, + AppName: appIdentifier.GetUniqueAppNameIdentifier(), UserId: userId, AppOfferingMode: util2.SERVER_MODE_HYPERION, ClusterId: appIdentifier.ClusterId, @@ -373,6 +373,7 @@ func (impl *AppStoreDeploymentServiceImpl) LinkHelmApplicationToChartStore(ctx c ReferenceValueId: int(request.GetReferenceValueId()), ReferenceValueKind: request.GetReferenceValueKind(), DeploymentAppType: util.PIPELINE_DEPLOYMENT_TYPE_HELM, + DisplayName: appIdentifier.ReleaseName, } // STEP-2 InstallApp with only DB operations @@ -387,21 +388,23 @@ func (impl *AppStoreDeploymentServiceImpl) LinkHelmApplicationToChartStore(ctx c return res, isChartRepoActive, nil } -func (impl *AppStoreDeploymentServiceImpl) UpdateProjectHelmApp(updateAppRequest *appStoreBean.UpdateProjectHelmAppDTO) error { - - appIdSplitted := strings.Split(updateAppRequest.AppId, "|") - - appName := updateAppRequest.AppName +func isExternalHelmApp(appId string) bool { + // for external helm apps, updateAppRequest.AppId is of the form clusterId|namespace|displayAppName + return len(strings.Split(appId, "|")) > 1 +} - if len(appIdSplitted) > 1 { - // app id is zero for CLI apps - appIdentifier, _ := impl.helmAppService.DecodeAppId(updateAppRequest.AppId) - appName = appIdentifier.ReleaseName +func (impl *AppStoreDeploymentServiceImpl) UpdateProjectHelmApp(updateAppRequest *appStoreBean.UpdateProjectHelmAppDTO) error { + var appName string + var displayName string + appName = updateAppRequest.AppName + if isExternalHelmApp(updateAppRequest.AppId) { + appName = updateAppRequest.UniqueAppIdentifier + displayName = updateAppRequest.AppName } impl.logger.Infow("update helm project request", updateAppRequest) - err := impl.appStoreDeploymentDBService.UpdateProjectForHelmApp(appName, updateAppRequest.TeamId, updateAppRequest.UserId) + err := impl.appStoreDeploymentDBService.UpdateProjectForHelmApp(appName, displayName, updateAppRequest.TeamId, updateAppRequest.UserId) if err != nil { - impl.logger.Errorw("error in linking project to helm app", "appName", appName, "err", err) + impl.logger.Errorw("error in linking project to helm app", "appName", updateAppRequest.AppName, "err", err) return err } return nil @@ -845,9 +848,6 @@ func (impl *AppStoreDeploymentServiceImpl) linkHelmApplicationToChartStore(insta // Rollback tx on error. defer tx.Rollback() - // skipAppCreation flag is set for CLI apps because for CLI Helm apps if project is created first before linking to chart store then app is created during project update time. - // skipAppCreation - This flag will skip app creation if app already exists. - //step 1 db operation initiated appModel, err := impl.appRepository.FindActiveByName(installAppVersionRequest.AppName) if err != nil && !util.IsErrNoRows(err) { diff --git a/pkg/bean/app.go b/pkg/bean/app.go index 3c388bd841..bbdd0dd746 100644 --- a/pkg/bean/app.go +++ b/pkg/bean/app.go @@ -58,6 +58,7 @@ type CreateAppDTO struct { AppLabels []*Label `json:"labels,omitempty" validate:"dive"` GenericNote *bean2.GenericNoteResponseBean `json:"genericNote,omitempty"` AppType helper.AppType `json:"appType" validate:"gt=-1,lt=3"` //TODO: Change Validation if new AppType is introduced + DisplayName string `json:"-"` //not exposed to UI } type CreateMaterialDTO struct { From 723332ff980d542343c6e51239f1768c9b44083c Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Fri, 19 Apr 2024 13:26:21 +0530 Subject: [PATCH 02/35] comments --- api/appStore/deployment/AppStoreDeploymentRestHandler.go | 1 + api/helm-app/service/HelmAppService.go | 3 +++ 2 files changed, 4 insertions(+) diff --git a/api/appStore/deployment/AppStoreDeploymentRestHandler.go b/api/appStore/deployment/AppStoreDeploymentRestHandler.go index 87b7db2f72..c87abeb786 100644 --- a/api/appStore/deployment/AppStoreDeploymentRestHandler.go +++ b/api/appStore/deployment/AppStoreDeploymentRestHandler.go @@ -596,6 +596,7 @@ func (handler AppStoreDeploymentRestHandlerImpl) UpdateProjectHelmApp(w http.Res handler.Logger.Errorw("service err, InstalledAppId", "err", err, "InstalledAppId", request.InstalledAppId) common.WriteJsonResp(w, fmt.Errorf("Unable to fetch installed app details"), nil, http.StatusBadRequest) } + // handle case for already linked ext app request.UniqueAppIdentifier = installedApp.AppName if installedApp.IsVirtualEnvironment { rbacObjectForCurrentProject, _ := handler.enforcerUtilHelm.GetAppRBACNameByInstalledAppId(request.InstalledAppId) diff --git a/api/helm-app/service/HelmAppService.go b/api/helm-app/service/HelmAppService.go index 20c17b548e..8e942accd1 100644 --- a/api/helm-app/service/HelmAppService.go +++ b/api/helm-app/service/HelmAppService.go @@ -1001,6 +1001,9 @@ type AppIdentifier struct { } func (r *AppIdentifier) GetUniqueAppNameIdentifier() string { + //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 a external app. return r.ReleaseName + "-" + r.Namespace + "-" + strconv.Itoa(r.ClusterId) } From 7c09365b2f717d1c91e93602ce8c997d3e3ede64 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Fri, 19 Apr 2024 17:34:08 +0530 Subject: [PATCH 03/35] appName and display name handling at other places as well for external app --- .../deployment/CommonDeploymentRestHandler.go | 3 ++- api/helm-app/HelmAppRestHandler.go | 4 +++- api/helm-app/service/ClusterUtils.go | 11 ++++++----- api/helm-app/service/HelmAppService.go | 9 +++++++++ pkg/appStore/adapter/Adapter.go | 4 ++++ .../installedApp/repository/InstalledAppModels.go | 1 + .../installedApp/repository/InstalledAppRepository.go | 2 +- .../service/AppStoreDeploymentDBService.go | 3 +++ .../service/EAMode/InstalledAppDBService.go | 4 ++++ 9 files changed, 33 insertions(+), 8 deletions(-) diff --git a/api/appStore/deployment/CommonDeploymentRestHandler.go b/api/appStore/deployment/CommonDeploymentRestHandler.go index 1963952319..726d2ba227 100644 --- a/api/appStore/deployment/CommonDeploymentRestHandler.go +++ b/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 diff --git a/api/helm-app/HelmAppRestHandler.go b/api/helm-app/HelmAppRestHandler.go index 6990ad279e..7ee96a51d8 100644 --- a/api/helm-app/HelmAppRestHandler.go +++ b/api/helm-app/HelmAppRestHandler.go @@ -224,7 +224,9 @@ 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) + //for external-apps appName would be uniqueIdentifier + appName := appIdentifier.GetUniqueAppNameIdentifier() + installedApp, err := handler.installedAppService.GetInstalledAppByClusterNamespaceAndName(appIdentifier.ClusterId, appIdentifier.Namespace, appName) if err != nil { common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) return diff --git a/api/helm-app/service/ClusterUtils.go b/api/helm-app/service/ClusterUtils.go index 199d200bef..db97fc9c11 100644 --- a/api/helm-app/service/ClusterUtils.go +++ b/api/helm-app/service/ClusterUtils.go @@ -5,11 +5,12 @@ import ( ) const ( - DaemonSetPodDeleteError = "cannot delete DaemonSet-managed Pods" - DnsLookupNoSuchHostError = "no such host" - TimeoutError = "timeout" - NotFound = "not found" - ConnectionRefused = "connection refused" + DaemonSetPodDeleteError = "cannot delete DaemonSet-managed Pods" + DnsLookupNoSuchHostError = "no such host" + TimeoutError = "timeout" + NotFound = "not found" + ConnectionRefused = "connection refused" + AppNotLinkedToChartStoreErr = "app not yet linked with any chart from chart-store in devtron, hence can't perform update operation from devtron ui" ) func IsClusterUnReachableError(err error) bool { diff --git a/api/helm-app/service/HelmAppService.go b/api/helm-app/service/HelmAppService.go index 8e942accd1..9c3ac685c7 100644 --- a/api/helm-app/service/HelmAppService.go +++ b/api/helm-app/service/HelmAppService.go @@ -599,6 +599,15 @@ func (impl *HelmAppServiceImpl) checkIfNsExists(app *AppIdentifier) (bool, error } func (impl *HelmAppServiceImpl) UpdateApplication(ctx context.Context, app *AppIdentifier, request *bean.UpdateApplicationRequestDto) (*openapi.UpdateReleaseResponse, error) { + if request.SourceAppType == bean.SOURCE_EXTERNAL_HELM_APP { + _, err := impl.installedAppRepository.GetInstalledAppByAppName(app.GetUniqueAppNameIdentifier()) + if err != nil && util.IsErrNoRows(err) { + return nil, &util.ApiError{HttpStatusCode: http.StatusUnprocessableEntity, Code: strconv.Itoa(http.StatusUnprocessableEntity), InternalMessage: AppNotLinkedToChartStoreErr, UserMessage: AppNotLinkedToChartStoreErr} + } else { + impl.logger.Errorw("error in fetching installed app from appName unique identifier", "appNameUniqueIdentifier", app.GetUniqueAppNameIdentifier()) + return nil, err + } + } config, err := impl.GetClusterConf(app.ClusterId) if err != nil { impl.logger.Errorw("error in fetching cluster detail", "clusterId", app.ClusterId, "err", err) diff --git a/pkg/appStore/adapter/Adapter.go b/pkg/appStore/adapter/Adapter.go index f48dad09e7..51bdec5223 100644 --- a/pkg/appStore/adapter/Adapter.go +++ b/pkg/appStore/adapter/Adapter.go @@ -216,6 +216,10 @@ func UpdateAppDetails(request *appStoreBean.InstallAppVersionDTO, app *app.App) request.AppName = app.AppName request.TeamId = app.TeamId request.AppOfferingMode = app.AppOfferingMode + // for external apps, AppName is unique identifier(appName-ns-clusterId), hence DisplayName should be used in that case + if len(app.DisplayName) > 0 { + request.AppName = app.DisplayName + } } // UpdateInstallAppDetails update repository.InstalledApps data into the same InstallAppVersionDTO diff --git a/pkg/appStore/installedApp/repository/InstalledAppModels.go b/pkg/appStore/installedApp/repository/InstalledAppModels.go index 320f0fbe48..ec9e4163ba 100644 --- a/pkg/appStore/installedApp/repository/InstalledAppModels.go +++ b/pkg/appStore/installedApp/repository/InstalledAppModels.go @@ -20,6 +20,7 @@ type InstalledAppsWithChartDetails struct { ChartRepoName string `json:"chart_repo_name"` DockerArtifactStoreId string `json:"docker_artifact_store_id"` AppName string `json:"app_name"` + DisplayName string `json:"display_name"` EnvironmentName string `json:"environment_name"` InstalledAppVersionId int `json:"installed_app_version_id"` AppStoreApplicationVersionId int `json:"app_store_application_version_id"` diff --git a/pkg/appStore/installedApp/repository/InstalledAppRepository.go b/pkg/appStore/installedApp/repository/InstalledAppRepository.go index 31f1048e9a..7969e50357 100644 --- a/pkg/appStore/installedApp/repository/InstalledAppRepository.go +++ b/pkg/appStore/installedApp/repository/InstalledAppRepository.go @@ -371,7 +371,7 @@ func (impl InstalledAppRepositoryImpl) GetAllInstalledApps(filter *appStoreBean. var installedAppsWithChartDetails []InstalledAppsWithChartDetails var query string query = "select iav.updated_on, iav.id as installed_app_version_id, ch.name as chart_repo_name, das.id as docker_artifact_store_id," - query = query + " env.environment_name, env.id as environment_id, env.is_virtual_environment, a.app_name, a.app_offering_mode, asav.icon, asav.name as app_store_application_name," + query = query + " env.environment_name, env.id as environment_id, env.is_virtual_environment, a.app_name, a.display_name, a.app_offering_mode, asav.icon, asav.name as app_store_application_name," query = query + " env.namespace, cluster.cluster_name, a.team_id, cluster.id as cluster_id, " query = query + " asav.id as app_store_application_version_id, ia.id , asav.deprecated , app_status.status as app_status, ia.deployment_app_delete_request" query = query + " from installed_app_versions iav" diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go index f87b9f8eca..45154e4239 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go @@ -327,6 +327,9 @@ func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, di if err != nil && !util.IsErrNoRows(err) { impl.logger.Errorw("error in fetching appModel", "err", err) return err + } else { + // for already linked project to app-> this case needs to be handled here + // in that case appModel will come as empty. in that case it will go and create new entry in app find app name by display name only } var appInstallationMode string diff --git a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go index 58833815c4..d8e5ff794c 100644 --- a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go +++ b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go @@ -122,6 +122,10 @@ func (impl *InstalledAppDBServiceImpl) GetAll(filter *appStoreBean.AppStoreFilte LastDeployedAt: &appLocal.UpdatedOn, AppStatus: &appLocal.AppStatus, } + if len(appLocal.DisplayName) > 0 { + //case of external app where display name is stored in app table + helmAppResp.AppName = &appLocal.DisplayName + } helmAppsResponse = append(helmAppsResponse, helmAppResp) } installedAppsResponse.HelmApps = &helmAppsResponse From 1e016f5d477d59b5215f814c7c33710ee40bebcd Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Sun, 21 Apr 2024 15:04:44 +0530 Subject: [PATCH 04/35] handling display name in GetHelmAppMetaInfo in AppCrudOperationService.go --- env_gen.md | 4 ++-- pkg/app/AppCrudOperationService.go | 20 ++++++++++++++++---- wire_gen.go | 2 +- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/env_gen.md b/env_gen.md index ea8c8de083..7b220f4793 100644 --- a/env_gen.md +++ b/env_gen.md @@ -196,9 +196,9 @@ | REQ_CI_MEM | 3G | | | RESOURCE_LIST_FOR_REPLICAS | Deployment,Rollout,StatefulSet,ReplicaSet | | | RESOURCE_LIST_FOR_REPLICAS_BATCH_SIZE | 5 | | - | REVISION_HISTORY_LIMIT_DEVTRON_APP | 0 | | + | REVISION_HISTORY_LIMIT_DEVTRON_APP | 1 | | | REVISION_HISTORY_LIMIT_EXTERNAL_HELM_APP | 0 | | - | REVISION_HISTORY_LIMIT_HELM_APP | 0 | | + | REVISION_HISTORY_LIMIT_HELM_APP | 1 | | | RUNTIME_CONFIG_LOCAL_DEV | false | | | RUN_HELM_INSTALL_IN_ASYNC_MODE_HELM_APPS | false | | | SCOPED_VARIABLE_ENABLED | false | | diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index 377144e0bf..7d18a2a888 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -20,6 +20,7 @@ package app import ( "encoding/json" "fmt" + client "github.com/devtron-labs/devtron/api/helm-app/service" "regexp" "strconv" "strings" @@ -64,13 +65,15 @@ type AppCrudOperationServiceImpl struct { installedAppRepository repository2.InstalledAppRepository genericNoteService genericNotes.GenericNoteService gitMaterialRepository pipelineConfig.MaterialRepository + helmAppService client.HelmAppService } func NewAppCrudOperationServiceImpl(appLabelRepository pipelineConfig.AppLabelRepository, logger *zap.SugaredLogger, appRepository appRepository.AppRepository, userRepository repository.UserRepository, installedAppRepository repository2.InstalledAppRepository, genericNoteService genericNotes.GenericNoteService, - gitMaterialRepository pipelineConfig.MaterialRepository) *AppCrudOperationServiceImpl { + gitMaterialRepository pipelineConfig.MaterialRepository, + helmAppService client.HelmAppService) *AppCrudOperationServiceImpl { return &AppCrudOperationServiceImpl{ appLabelRepository: appLabelRepository, logger: logger, @@ -79,6 +82,7 @@ func NewAppCrudOperationServiceImpl(appLabelRepository pipelineConfig.AppLabelRe installedAppRepository: installedAppRepository, genericNoteService: genericNoteService, gitMaterialRepository: gitMaterialRepository, + helmAppService: helmAppService, } } @@ -433,14 +437,19 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. var err error impl.logger.Info("request payload, appId", appId) if len(appIdSplitted) > 1 { - appName := appIdSplitted[2] + appIdDecoded, err := impl.helmAppService.DecodeAppId(appId) + if err != nil { + impl.logger.Errorw("error in decoding app id for external app", "appId", appId, "err", err) + return nil, err + } + appName := appIdDecoded.GetUniqueAppNameIdentifier() app, err = impl.appRepository.FindAppAndProjectByAppName(appName) if err != nil && err != pg.ErrNoRows { impl.logger.Errorw("error in fetching app meta data", "err", err) return nil, err } if app.Id == 0 { - app.AppName = appName + app.AppName = appIdDecoded.ReleaseName } } else { installedAppIdInt, err := strconv.Atoi(appId) @@ -459,7 +468,10 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. app.Team.Name = InstalledApp.App.Team.Name app.CreatedBy = InstalledApp.App.CreatedBy app.Active = InstalledApp.App.Active - + if len(InstalledApp.App.DisplayName) > 0 { + // in case of external apps, we will send display name as appName will be a unique identifier + app.AppName = InstalledApp.App.DisplayName + } if err != nil { impl.logger.Errorw("error in fetching App Meta Info", "error", err) return nil, err diff --git a/wire_gen.go b/wire_gen.go index 5a60b944d1..2159084712 100644 --- a/wire_gen.go +++ b/wire_gen.go @@ -477,7 +477,7 @@ func InitializeApp() (*App, error) { ciTemplateServiceImpl := pipeline.NewCiTemplateServiceImpl(sugaredLogger, ciBuildConfigServiceImpl, ciTemplateRepositoryImpl, ciTemplateOverrideRepositoryImpl) appLabelRepositoryImpl := pipelineConfig.NewAppLabelRepositoryImpl(db) materialRepositoryImpl := pipelineConfig.NewMaterialRepositoryImpl(db) - appCrudOperationServiceImpl := app2.NewAppCrudOperationServiceImpl(appLabelRepositoryImpl, sugaredLogger, appRepositoryImpl, userRepositoryImpl, installedAppRepositoryImpl, genericNoteServiceImpl, materialRepositoryImpl) + appCrudOperationServiceImpl := app2.NewAppCrudOperationServiceImpl(appLabelRepositoryImpl, sugaredLogger, appRepositoryImpl, userRepositoryImpl, installedAppRepositoryImpl, genericNoteServiceImpl, materialRepositoryImpl, helmAppServiceImpl) imageTagRepositoryImpl := repository2.NewImageTagRepository(db, sugaredLogger) customTagServiceImpl := pipeline.NewCustomTagService(sugaredLogger, imageTagRepositoryImpl) pluginInputVariableParserImpl := pipeline.NewPluginInputVariableParserImpl(sugaredLogger, dockerRegistryConfigImpl, customTagServiceImpl) From f69202f48b6490914822013f3d407d8fd41d6f0f Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Mon, 22 Apr 2024 02:03:11 +0530 Subject: [PATCH 05/35] FetchAppDetailsForInstalledAppV2 send display name if present --- cmd/external-app/wire_gen.go | 2 +- .../installedApp/service/EAMode/InstalledAppDBService.go | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/external-app/wire_gen.go b/cmd/external-app/wire_gen.go index 4019867777..817ec93616 100644 --- a/cmd/external-app/wire_gen.go +++ b/cmd/external-app/wire_gen.go @@ -373,7 +373,7 @@ func InitializeApp() (*App, error) { attributesRouterImpl := router.NewAttributesRouterImpl(attributesRestHandlerImpl) appLabelRepositoryImpl := pipelineConfig.NewAppLabelRepositoryImpl(db) materialRepositoryImpl := pipelineConfig.NewMaterialRepositoryImpl(db) - appCrudOperationServiceImpl := app2.NewAppCrudOperationServiceImpl(appLabelRepositoryImpl, sugaredLogger, appRepositoryImpl, userRepositoryImpl, installedAppRepositoryImpl, genericNoteServiceImpl, materialRepositoryImpl) + appCrudOperationServiceImpl := app2.NewAppCrudOperationServiceImpl(appLabelRepositoryImpl, sugaredLogger, appRepositoryImpl, userRepositoryImpl, installedAppRepositoryImpl, genericNoteServiceImpl, materialRepositoryImpl, helmAppServiceImpl) appInfoRestHandlerImpl := appInfo.NewAppInfoRestHandlerImpl(sugaredLogger, appCrudOperationServiceImpl, userServiceImpl, validate, enforcerUtilImpl, enforcerImpl, helmAppServiceImpl, enforcerUtilHelmImpl, genericNoteServiceImpl) appInfoRouterImpl := appInfo2.NewAppInfoRouterImpl(sugaredLogger, appInfoRestHandlerImpl) appFilteringRestHandlerImpl := appList.NewAppFilteringRestHandlerImpl(sugaredLogger, teamServiceImpl, enforcerImpl, userServiceImpl, clusterServiceImpl, environmentServiceImpl) diff --git a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go index 5971bfa50e..5919280cc1 100644 --- a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go +++ b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go @@ -202,6 +202,9 @@ func (impl *InstalledAppDBServiceImpl) FindAppDetailsForAppstoreApplication(inst installedAppVerison.InstalledApp.Environment.Name, installedAppVerison.InstalledApp.UpdatedOn), } + if len(installedAppVerison.InstalledApp.App.DisplayName) > 0 { + deploymentContainer.AppName = installedAppVerison.InstalledApp.App.DisplayName + } userInfo, err := impl.UserService.GetByIdIncludeDeleted(installedAppVerison.AuditLog.UpdatedBy) if err != nil { impl.Logger.Errorw("error fetching user info", "err", err) From e5212e12cf9ca2c5596fc22508211250a28b291e Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Mon, 22 Apr 2024 13:21:42 +0530 Subject: [PATCH 06/35] DeleteDBLinkedHelmApplication deletion handling for ext app with uniq app identifier and if not found by unique app identifier, then find by display name --- api/helm-app/service/HelmAppService.go | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/api/helm-app/service/HelmAppService.go b/api/helm-app/service/HelmAppService.go index 9c3ac685c7..d15a11d38c 100644 --- a/api/helm-app/service/HelmAppService.go +++ b/api/helm-app/service/HelmAppService.go @@ -454,12 +454,27 @@ func (impl *HelmAppServiceImpl) DeleteDBLinkedHelmApplication(ctx context.Contex } // Rollback tx on error. defer tx.Rollback() - + //for ext apps search app from unique identifier + appUniqueIdentifier := app.GetUniqueAppNameIdentifier() + model := &repository.InstalledApps{} // For Helm deployments ReleaseName is App.Name - model, err := impl.installedAppRepository.GetInstalledAppByAppName(app.ReleaseName) + model, err = impl.installedAppRepository.GetInstalledAppByAppName(appUniqueIdentifier) if err != nil { - impl.logger.Errorw("error in fetching installed app", "appName", app.ReleaseName, "err", err) - return nil, err + 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. + impl.logger.Debugw("DeleteDBLinkedHelmApplication, not able to find installed app of external app by app unique identifier, hence finding by display name", "appUniqueIdentifier", appUniqueIdentifier) + displayName := app.ReleaseName + model, err = impl.installedAppRepository.GetInstalledAppByAppName(displayName) + if err != nil { + impl.logger.Errorw("error in fetching installed app from display name", "appDisplayName", app.ReleaseName, "err", err) + return nil, err + } + } else { + impl.logger.Errorw("error in fetching installed app by app unique identifier", "appUniqueIdentifier", appUniqueIdentifier, "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}); From b535b32f3cc3d92800cb94ccf0a7c63418d03017 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Mon, 22 Apr 2024 15:16:07 +0530 Subject: [PATCH 07/35] applist method incorporated checks on display name if exists instead of appName --- api/helm-app/service/HelmAppService.go | 9 ++++++++- pkg/app/AppCrudOperationService.go | 10 +++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/api/helm-app/service/HelmAppService.go b/api/helm-app/service/HelmAppService.go index d15a11d38c..bf8261b694 100644 --- a/api/helm-app/service/HelmAppService.go +++ b/api/helm-app/service/HelmAppService.go @@ -1054,6 +1054,13 @@ 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} @@ -1081,7 +1088,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 } diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index 7d18a2a888..5d17a4502f 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -435,6 +435,7 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. appIdSplitted := strings.Split(appId, "|") app := &appRepository.App{} var err error + var displayName string impl.logger.Info("request payload, appId", appId) if len(appIdSplitted) > 1 { appIdDecoded, err := impl.helmAppService.DecodeAppId(appId) @@ -451,6 +452,9 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. if app.Id == 0 { app.AppName = appIdDecoded.ReleaseName } + if len(app.DisplayName) > 0 { + displayName = app.DisplayName + } } else { installedAppIdInt, err := strconv.Atoi(appId) if err != nil { @@ -470,7 +474,7 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. app.Active = InstalledApp.App.Active if len(InstalledApp.App.DisplayName) > 0 { // in case of external apps, we will send display name as appName will be a unique identifier - app.AppName = InstalledApp.App.DisplayName + displayName = InstalledApp.App.DisplayName } if err != nil { impl.logger.Errorw("error in fetching App Meta Info", "error", err) @@ -500,6 +504,10 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. CreatedOn: app.CreatedOn, Active: app.Active, } + if len(displayName) > 0 { + //special handling for ext-helm apps where name visible on UI is display name + info.AppName = displayName + } return info, nil } From bfc4031615ed9ae5e0f92ae08d0f9c8bf4f94901 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Mon, 22 Apr 2024 15:40:48 +0530 Subject: [PATCH 08/35] query change in GetAppAndEnvDetailsForDeploymentAppTypeInstalledApps to include also display_name --- pkg/appStore/installedApp/repository/InstalledAppRepository.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/appStore/installedApp/repository/InstalledAppRepository.go b/pkg/appStore/installedApp/repository/InstalledAppRepository.go index 9ebb2d44c0..f649eba64a 100644 --- a/pkg/appStore/installedApp/repository/InstalledAppRepository.go +++ b/pkg/appStore/installedApp/repository/InstalledAppRepository.go @@ -674,7 +674,7 @@ func (impl InstalledAppRepositoryImpl) GetAppAndEnvDetailsForDeploymentAppTypeIn var installedApps []*InstalledApps err := impl.dbConnection. Model(&installedApps). - Column("installed_apps.id", "App.app_name", "Environment.cluster_id", "Environment.namespace"). + Column("installed_apps.id", "App.app_name", "App.display_name", "Environment.cluster_id", "Environment.namespace"). Where("environment.cluster_id in (?)", pg.In(clusterIds)). Where("installed_apps.deployment_app_type = ?", deploymentAppType). Where("app.active = ?", true). From 5c299a50a069410590b120ac4b6c339a2c7474e5 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Mon, 22 Apr 2024 15:58:29 +0530 Subject: [PATCH 09/35] GenerateInstallAppVersionMinDTO if display name exists then send appname as display name --- pkg/appStore/adapter/Adapter.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/appStore/adapter/Adapter.go b/pkg/appStore/adapter/Adapter.go index 51bdec5223..f55fb5b92b 100644 --- a/pkg/appStore/adapter/Adapter.go +++ b/pkg/appStore/adapter/Adapter.go @@ -142,7 +142,7 @@ func GenerateInstallAppVersionDTO(installedApp *repository.InstalledApps, instal // GenerateInstallAppVersionMinDTO converts repository.InstalledApps db object to appStoreBean.InstallAppVersionDTO bean; // Note: It only generates a minimal DTO and doesn't include repository.InstalledAppVersions data func GenerateInstallAppVersionMinDTO(installedApp *repository.InstalledApps) *appStoreBean.InstallAppVersionDTO { - return &appStoreBean.InstallAppVersionDTO{ + installAppVersionDto := &appStoreBean.InstallAppVersionDTO{ EnvironmentId: installedApp.EnvironmentId, InstalledAppId: installedApp.Id, AppId: installedApp.AppId, @@ -156,6 +156,10 @@ func GenerateInstallAppVersionMinDTO(installedApp *repository.InstalledApps) *ap DeploymentAppType: installedApp.DeploymentAppType, IsVirtualEnvironment: installedApp.Environment.IsVirtualEnvironment, } + if len(installedApp.App.DisplayName) > 0 { + installAppVersionDto.AppName = installedApp.App.DisplayName + } + return installAppVersionDto } func GetGeneratedHelmPackageName(appName, envName string, updatedOn time.Time) string { From d5caddaf4c56a845f74884b6f6b9b6c67e00df4e Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Mon, 22 Apr 2024 16:09:37 +0530 Subject: [PATCH 10/35] FindAppDetailsForAppstoreApplication, considering display name as well while preparing helm package name deploymentContainer.HelmPackageName --- .../installedApp/service/EAMode/InstalledAppDBService.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go index 5919280cc1..7ee2f13023 100644 --- a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go +++ b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go @@ -197,14 +197,11 @@ func (impl *InstalledAppDBServiceImpl) FindAppDetailsForAppstoreApplication(inst IsVirtualEnvironment: installedAppVerison.InstalledApp.Environment.IsVirtualEnvironment, HelmReleaseInstallStatus: helmReleaseInstallStatus, Status: status, - HelmPackageName: adapter.GetGeneratedHelmPackageName( - installedAppVerison.InstalledApp.App.AppName, - installedAppVerison.InstalledApp.Environment.Name, - installedAppVerison.InstalledApp.UpdatedOn), } if len(installedAppVerison.InstalledApp.App.DisplayName) > 0 { deploymentContainer.AppName = installedAppVerison.InstalledApp.App.DisplayName } + deploymentContainer.HelmPackageName = adapter.GetGeneratedHelmPackageName(deploymentContainer.AppName, deploymentContainer.EnvironmentName, installedAppVerison.InstalledApp.UpdatedOn) userInfo, err := impl.UserService.GetByIdIncludeDeleted(installedAppVerison.AuditLog.UpdatedBy) if err != nil { impl.Logger.Errorw("error fetching user info", "err", err) From c37ed41c152ee7a4a16b4d294f114c390ab62942 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Tue, 23 Apr 2024 17:54:58 +0530 Subject: [PATCH 11/35] multiple fixes around corner cases around display names --- .../AppStoreDeploymentRestHandler.go | 5 +- .../app/appInfo/AppInfoRestHandler.go | 2 +- pkg/app/AppCrudOperationService.go | 58 +++++++++++++++++-- .../service/AppStoreDeploymentDBService.go | 45 +++++++++++--- .../service/AppStoreDeploymentService.go | 11 +++- .../service/FullMode/resource/NotesService.go | 19 +++--- 6 files changed, 112 insertions(+), 28 deletions(-) diff --git a/api/appStore/deployment/AppStoreDeploymentRestHandler.go b/api/appStore/deployment/AppStoreDeploymentRestHandler.go index c87abeb786..198d25e954 100644 --- a/api/appStore/deployment/AppStoreDeploymentRestHandler.go +++ b/api/appStore/deployment/AppStoreDeploymentRestHandler.go @@ -579,7 +579,7 @@ func (handler AppStoreDeploymentRestHandlerImpl) UpdateProjectHelmApp(w http.Res handler.Logger.Errorw("error in decoding app id", "err", err) common.WriteJsonResp(w, err, "error in decoding app id", http.StatusBadRequest) } - request.UniqueAppIdentifier = appIdentifier.GetUniqueAppNameIdentifier() + // this rbac object checks that whether user have permission to change current project. rbacObjectForCurrentProject, rbacObjectForCurrentProject2 := handler.enforcerUtilHelm.GetHelmObjectByClusterIdNamespaceAndAppName(appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.ReleaseName) ok := handler.enforcer.Enforce(token, casbin.ResourceHelmApp, casbin.ActionUpdate, rbacObjectForCurrentProject) || handler.enforcer.Enforce(token, casbin.ResourceHelmApp, casbin.ActionUpdate, rbacObjectForCurrentProject2) @@ -596,8 +596,7 @@ func (handler AppStoreDeploymentRestHandlerImpl) UpdateProjectHelmApp(w http.Res handler.Logger.Errorw("service err, InstalledAppId", "err", err, "InstalledAppId", request.InstalledAppId) common.WriteJsonResp(w, fmt.Errorf("Unable to fetch installed app details"), nil, http.StatusBadRequest) } - // handle case for already linked ext app - request.UniqueAppIdentifier = installedApp.AppName + if installedApp.IsVirtualEnvironment { rbacObjectForCurrentProject, _ := handler.enforcerUtilHelm.GetAppRBACNameByInstalledAppId(request.InstalledAppId) ok := handler.enforcer.Enforce(token, casbin.ResourceHelmApp, casbin.ActionUpdate, rbacObjectForCurrentProject) diff --git a/api/restHandler/app/appInfo/AppInfoRestHandler.go b/api/restHandler/app/appInfo/AppInfoRestHandler.go index b6e614c1d7..035100d144 100644 --- a/api/restHandler/app/appInfo/AppInfoRestHandler.go +++ b/api/restHandler/app/appInfo/AppInfoRestHandler.go @@ -179,7 +179,7 @@ func (handler AppInfoRestHandlerImpl) GetHelmAppMetaInfo(w http.ResponseWriter, return } } - res, err := handler.appService.GetHelmAppMetaInfo(appIdReq) + res, err := handler.appService.GetHelmAppMetaInfo(appIdReq, userId) if err != nil { handler.logger.Errorw("service err, GetAppMetaInfo", "err", err) common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index 5d17a4502f..6bef651dcf 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -21,6 +21,7 @@ import ( "encoding/json" "fmt" client "github.com/devtron-labs/devtron/api/helm-app/service" + "github.com/devtron-labs/devtron/internal/util" "regexp" "strconv" "strings" @@ -48,13 +49,14 @@ type AppCrudOperationService interface { FindById(id int) (*bean.AppLabelDto, error) FindAll() ([]*bean.AppLabelDto, error) GetAppMetaInfo(appId int, installedAppId int, envId int) (*bean.AppMetaInfoDto, error) - GetHelmAppMetaInfo(appId string) (*bean.AppMetaInfoDto, error) + GetHelmAppMetaInfo(appId string, userId int32) (*bean.AppMetaInfoDto, error) GetLabelsByAppIdForDeployment(appId int) ([]byte, error) GetLabelsByAppId(appId int) (map[string]string, error) UpdateApp(request *bean.CreateAppDTO) (*bean.CreateAppDTO, error) UpdateProjectForApps(request *bean.UpdateProjectBulkAppsRequest) (*bean.UpdateProjectBulkAppsRequest, error) GetAppMetaInfoByAppName(appName string) (*bean.AppMetaInfoDto, error) GetAppListByTeamIds(teamIds []int, appType string) ([]*TeamAppBean, error) + IsExternalAppLinkedToChartStore(appId int, releaseName string) (bool, string, error) } type AppCrudOperationServiceImpl struct { @@ -428,7 +430,20 @@ func convertUrlToHttpsIfSshType(url string) string { return httpsURL } -func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean.AppMetaInfoDto, error) { +func (impl AppCrudOperationServiceImpl) IsExternalAppLinkedToChartStore(appId int, releaseName string) (bool, string, error) { + installedApp, err := impl.installedAppRepository.GetInstalledAppsByAppId(appId) + if err != nil && err != pg.ErrNoRows { + impl.logger.Errorw("IsExternalAppLinkedToChartStore, error in fetching installed app by app id for external apps", "appId", appId, "err", err) + return false, "", err + } + if installedApp.Id > 0 { + uniqueIdentifierForAlreadyLinkedApp := releaseName + "-" + installedApp.Environment.Namespace + "-" + strconv.Itoa(installedApp.Environment.ClusterId) + return true, uniqueIdentifierForAlreadyLinkedApp, nil + } + return false, "", nil +} + +func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId int32) (*bean.AppMetaInfoDto, error) { // adding separate function for helm apps because for CLI helm apps, apps can be of form "1|clusterName|releaseName" // In this case app details can be fetched using app name / release Name. @@ -443,12 +458,45 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. impl.logger.Errorw("error in decoding app id for external app", "appId", appId, "err", err) return nil, err } - appName := appIdDecoded.GetUniqueAppNameIdentifier() - app, err = impl.appRepository.FindAppAndProjectByAppName(appName) + appNameUniqueIdentifier := appIdDecoded.GetUniqueAppNameIdentifier() + app, err = impl.appRepository.FindAppAndProjectByAppName(appNameUniqueIdentifier) if err != nil && err != pg.ErrNoRows { - impl.logger.Errorw("error in fetching app meta data", "err", err) + impl.logger.Errorw("GetHelmAppMetaInfo, error in fetching app meta data by unique app identifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) return nil, err } + if util.IsErrNoRows(err) { + //find app by display name if not found by unique identifier + app, err = impl.appRepository.FindAppAndProjectByAppName(appIdDecoded.ReleaseName) + if err != nil && err != pg.ErrNoRows { + impl.logger.Errorw("GetHelmAppMetaInfo, error in fetching app meta data by display name", "displayName", appIdDecoded.ReleaseName, "err", err) + return nil, err + } + if app.Id > 0 { + //if isLinked is true then installed_app found for this app then this app name is already linked to an installed app then + //update this app's app_name with unique identifier along with display_name. + isLinked, uniqueIdentifierForAlreadyLinkedApp, err := impl.IsExternalAppLinkedToChartStore(app.Id, appIdDecoded.ReleaseName) + if err != nil { + impl.logger.Errorw("GetHelmAppMetaInfo, error in checking IsExternalAppLinkedToChartStore", "appId", appId, "err", err) + return nil, err + } + if isLinked { + // if installed_app is already present for that display_name then migrate the app_name to unique identifier with installed_app ns and cluster id data + app.AppName = uniqueIdentifierForAlreadyLinkedApp + } else { + //app not found with unique identifier but displayName, hence migrate the app_name to unique identifier and also update display_name + app.AppName = appNameUniqueIdentifier + } + + app.DisplayName = appIdDecoded.ReleaseName + app.UpdatedBy = userId + app.UpdatedOn = time.Now() + err = impl.appRepository.Update(app) + if err != nil { + impl.logger.Errorw("GetHelmAppMetaInfo, error in migrating displayName and appName to unique identifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) + return nil, err + } + } + } if app.Id == 0 { app.AppName = appIdDecoded.ReleaseName } diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go index 45154e4239..278c58d831 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go @@ -42,7 +42,7 @@ type AppStoreDeploymentDBService interface { // UpdateInstalledAppVersionHistoryWithGitHash updates GitHash in the repository.InstalledAppVersionHistory UpdateInstalledAppVersionHistoryWithGitHash(versionHistoryId int, gitHash string, userId int32) error // UpdateProjectForHelmApp updates TeamId in the app.App - UpdateProjectForHelmApp(appName, displayName string, teamId int, userId int32) error + UpdateProjectForHelmApp(appName, displayName, namespace string, teamId int, userId int32) error // InstallAppPostDbOperation is used to perform Post-Install DB operations in App Store deployments InstallAppPostDbOperation(installAppVersionRequest *appStoreBean.InstallAppVersionDTO) error // MarkInstalledAppVersionsInactiveByInstalledAppId will mark the repository.InstalledAppVersions inactive for the given InstalledAppId @@ -322,14 +322,41 @@ func (impl *AppStoreDeploymentDBServiceImpl) UpdateInstalledAppVersionHistoryWit return nil } -func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, displayName string, teamId int, userId int32) error { +func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, displayName, namespace string, teamId int, userId int32) error { appModel, err := impl.appRepository.FindActiveByName(appName) - if err != nil && !util.IsErrNoRows(err) { - impl.logger.Errorw("error in fetching appModel", "err", err) - return err + if err != nil && util.IsErrNoRows(err) { + // if we can't find by appName then find by display name, if app is found via display name then app was previously assigned with project or linked with devtron with app_name as display_name + appModel, err = impl.appRepository.FindActiveByName(displayName) + if err != nil && !util.IsErrNoRows(err) { + impl.logger.Errorw("error in fetching appModel by displayName", "displayName", displayName, "err", err) + return err + } + // external app will have a display name, so checking the following case only for external apps + if appModel != nil && appModel.Id > 0 && len(displayName) > 0 { + /* + 1. now we will check if for that appModel, installed_app entry present or not, + 2. if not, then let the normal flow continue as we can change the app_name with app unique identifier. + 3. if exists then we will check if request's namespace is same as what present in installed_apps. + - if ns doesn't match then update proj. req. is for some other app found in app table via display_name, + in this case create a new entry in app table for the request. + - if ns matches, then update proj. req. is for same app present in installed_apps, in that case we will update + the app_name with unique identifier and display_name along with team_id. + */ + installedApp, err := impl.installedAppRepository.GetInstalledAppsByAppId(appModel.Id) + if err != nil && !util.IsErrNoRows(err) { + impl.logger.Errorw("error in fetching installed app by appId", "appId", appModel.Id, "appName", appName, "err", err) + return err + } + if installedApp.Id > 0 { + if namespace != installedApp.Environment.Namespace { + //assigning appModel as nil, so that it will create a new entry in app for the request + appModel = nil + } + } + } } else { - // for already linked project to app-> this case needs to be handled here - // in that case appModel will come as empty. in that case it will go and create new entry in app find app name by display name only + impl.logger.Errorw("error in fetching appModel by appName", "appName", appName, "err", err) + return err } var appInstallationMode string @@ -362,6 +389,10 @@ func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, di return err } } else { + //this will handle the case when ext-helm app is already assigned to a project and an entry already exist in app table + //then this will override app_name with unique identifier app name and update display_name also + appModel.AppName = appName + appModel.DisplayName = displayName // update team id if appModel exist appModel.TeamId = teamId appModel.UpdateAuditLog(userId) diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go index af7201647b..5f05022a0f 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go @@ -396,13 +396,20 @@ func isExternalHelmApp(appId string) bool { func (impl *AppStoreDeploymentServiceImpl) UpdateProjectHelmApp(updateAppRequest *appStoreBean.UpdateProjectHelmAppDTO) error { var appName string var displayName string + var namespace string appName = updateAppRequest.AppName if isExternalHelmApp(updateAppRequest.AppId) { - appName = updateAppRequest.UniqueAppIdentifier + appIdentifier, err := impl.helmAppService.DecodeAppId(updateAppRequest.AppId) + if err != nil { + impl.logger.Errorw("error in decoding app id for external helm apps", "err", err) + return err + } + appName = appIdentifier.GetUniqueAppNameIdentifier() displayName = updateAppRequest.AppName + namespace = appIdentifier.Namespace } impl.logger.Infow("update helm project request", updateAppRequest) - err := impl.appStoreDeploymentDBService.UpdateProjectForHelmApp(appName, displayName, updateAppRequest.TeamId, updateAppRequest.UserId) + err := impl.appStoreDeploymentDBService.UpdateProjectForHelmApp(appName, displayName, namespace, updateAppRequest.TeamId, updateAppRequest.UserId) if err != nil { impl.logger.Errorw("error in linking project to helm app", "appName", updateAppRequest.AppName, "err", err) return err diff --git a/pkg/appStore/installedApp/service/FullMode/resource/NotesService.go b/pkg/appStore/installedApp/service/FullMode/resource/NotesService.go index df03d62471..743c53d10a 100644 --- a/pkg/appStore/installedApp/service/FullMode/resource/NotesService.go +++ b/pkg/appStore/installedApp/service/FullMode/resource/NotesService.go @@ -44,7 +44,7 @@ func (impl *InstalledAppResourceServiceImpl) FetchChartNotes(installedAppId int, } //if notes is not present in db then below call will happen if installedApp.Notes == "" { - notes, _, err := impl.findNotesForArgoApplication(installedAppId, envId) + notes, err := impl.findNotesForArgoApplication(installedAppId, envId) if err != nil { impl.logger.Errorw("error fetching notes", "err", err) return "", err @@ -58,25 +58,24 @@ func (impl *InstalledAppResourceServiceImpl) FetchChartNotes(installedAppId int, return installedApp.Notes, nil } -func (impl *InstalledAppResourceServiceImpl) findNotesForArgoApplication(installedAppId, envId int) (string, string, error) { +func (impl *InstalledAppResourceServiceImpl) findNotesForArgoApplication(installedAppId, envId int) (string, error) { installedAppVerison, err := impl.installedAppRepository.GetInstalledAppVersionByInstalledAppIdAndEnvId(installedAppId, envId) if err != nil { impl.logger.Errorw("error fetching installed app version in installed app service", "err", err) - return "", "", err + return "", err } var notes string - appName := installedAppVerison.InstalledApp.App.AppName if util.IsAcdApp(installedAppVerison.InstalledApp.DeploymentAppType) { appStoreAppVersion, err := impl.appStoreApplicationVersionRepository.FindById(installedAppVerison.AppStoreApplicationVersion.Id) if err != nil { impl.logger.Errorw("error fetching app store app version in installed app service", "err", err) - return notes, appName, err + return notes, err } k8sServerVersion, err := impl.K8sUtil.GetKubeVersion() if err != nil { impl.logger.Errorw("exception caught in getting k8sServerVersion", "err", err) - return notes, appName, err + return notes, err } installReleaseRequest := &gRPC.InstallReleaseRequest{ @@ -100,23 +99,23 @@ func (impl *InstalledAppResourceServiceImpl) findNotesForArgoApplication(install config, err := impl.helmAppService.GetClusterConf(clusterId) if err != nil { impl.logger.Errorw("error in fetching cluster detail", "clusterId", clusterId, "err", err) - return "", appName, err + return "", err } installReleaseRequest.ReleaseIdentifier.ClusterConfig = config notes, err = impl.helmAppService.GetNotes(context.Background(), installReleaseRequest) if err != nil { impl.logger.Errorw("error in fetching notes", "err", err) - return notes, appName, err + return notes, err } _, err = impl.updateNotesForInstalledApp(installedAppId, notes) if err != nil { impl.logger.Errorw("error in updating notes in db ", "err", err) - return notes, appName, err + return notes, err } } - return notes, appName, nil + return notes, nil } // updateNotesForInstalledApp will update the notes in repository.InstalledApps table From bc9e81ab50a62302dcb7d8a1ad7349855d98cec9 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Tue, 23 Apr 2024 18:18:11 +0530 Subject: [PATCH 12/35] minor fix --- .../installedApp/service/AppStoreDeploymentDBService.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go index 278c58d831..058ace621b 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go @@ -354,7 +354,7 @@ func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, di } } } - } else { + } else if err != nil { impl.logger.Errorw("error in fetching appModel by appName", "appName", appName, "err", err) return err } From df58a7bb2a8a527281408c956055bd70bdf20381 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Tue, 23 Apr 2024 19:35:05 +0530 Subject: [PATCH 13/35] fixes for fetching resoirce trees for ext apps using display name instead of appName --- api/appStore/InstalledAppRestHandler.go | 8 ++++++++ api/restHandler/app/appList/AppListingRestHandler.go | 4 ++++ pkg/app/AppCrudOperationService.go | 2 +- .../installedApp/repository/InstalledAppRepository.go | 4 ++++ .../service/EAMode/EAModeDeploymentService.go | 6 +++++- .../installedApp/service/EAMode/InstalledAppDBService.go | 6 ++++++ 6 files changed, 28 insertions(+), 2 deletions(-) diff --git a/api/appStore/InstalledAppRestHandler.go b/api/appStore/InstalledAppRestHandler.go index eec150392f..f4a7900d56 100644 --- a/api/appStore/InstalledAppRestHandler.go +++ b/api/appStore/InstalledAppRestHandler.go @@ -603,6 +603,10 @@ func (handler *InstalledAppRestHandlerImpl) FetchAppDetailsForInstalledApp(w htt common.WriteJsonResp(w, err, "App not found in database", http.StatusBadRequest) return } + if len(installedApp.App.DisplayName) > 0 { + //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 +728,10 @@ func (handler *InstalledAppRestHandlerImpl) FetchResourceTree(w http.ResponseWri common.WriteJsonResp(w, err, "App not found in database", http.StatusBadRequest) return } + if len(installedApp.App.DisplayName) > 0 { + //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 diff --git a/api/restHandler/app/appList/AppListingRestHandler.go b/api/restHandler/app/appList/AppListingRestHandler.go index 373f14bf85..1dfac0db92 100644 --- a/api/restHandler/app/appList/AppListingRestHandler.go +++ b/api/restHandler/app/appList/AppListingRestHandler.go @@ -900,6 +900,10 @@ func (handler AppListingRestHandlerImpl) GetHostUrlsByBatch(w http.ResponseWrite common.WriteJsonResp(w, err, "App not found in database", http.StatusBadRequest) return } + if len(installedApp.App.DisplayName) > 0 { + //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 { diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index 6bef651dcf..dc68c500f9 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -348,7 +348,7 @@ func (impl AppCrudOperationServiceImpl) GetAppMetaInfo(appId int, installedAppId } } appName := app.AppName - if app.AppType == helper.Job { + if app.AppType == helper.Job || len(app.DisplayName) > 0 { appName = app.DisplayName } noteResp, err := impl.genericNoteService.GetGenericNotesForAppIds([]int{app.Id}) diff --git a/pkg/appStore/installedApp/repository/InstalledAppRepository.go b/pkg/appStore/installedApp/repository/InstalledAppRepository.go index f649eba64a..087f118f7b 100644 --- a/pkg/appStore/installedApp/repository/InstalledAppRepository.go +++ b/pkg/appStore/installedApp/repository/InstalledAppRepository.go @@ -75,6 +75,10 @@ func (model *InstalledApps) UpdateGitOpsRepository(gitOpsRepoUrl string, isCusto model.GitOpsRepoName = gitUtil.GetGitRepoNameFromGitRepoUrl(gitOpsRepoUrl) // Handled for backward compatibility } +func (model *InstalledApps) ChangeAppNameToDisplayName() { + model.App.AppName = model.App.DisplayName +} + type InstalledAppVersions struct { TableName struct{} `sql:"installed_app_versions" pg:",discard_unknown_columns"` Id int `sql:"id,pk"` diff --git a/pkg/appStore/installedApp/service/EAMode/EAModeDeploymentService.go b/pkg/appStore/installedApp/service/EAMode/EAModeDeploymentService.go index 932f267e78..4914c63e86 100644 --- a/pkg/appStore/installedApp/service/EAMode/EAModeDeploymentService.go +++ b/pkg/appStore/installedApp/service/EAMode/EAModeDeploymentService.go @@ -252,6 +252,10 @@ func (impl *EAModeDeploymentServiceImpl) updateApplicationWithChartInfo(ctx cont impl.Logger.Errorw("error in getting in installedApp", "installedAppId", installedAppId, "err", err) return err } + appName := installedApp.App.AppName + if len(installedApp.App.DisplayName) > 0 { + appName = installedApp.App.DisplayName + } appStoreApplicationVersion, err := impl.appStoreApplicationVersionRepository.FindById(appStoreApplicationVersionId) if err != nil { impl.Logger.Errorw("error in getting in appStoreApplicationVersion", "appStoreApplicationVersionId", appStoreApplicationVersionId, "err", err) @@ -300,7 +304,7 @@ func (impl *EAModeDeploymentServiceImpl) updateApplicationWithChartInfo(ctx cont ValuesYaml: valuesOverrideYaml, ReleaseIdentifier: &gRPC.ReleaseIdentifier{ ReleaseNamespace: installedApp.Environment.Namespace, - ReleaseName: installedApp.App.AppName, + ReleaseName: appName, }, ChartName: appStoreApplicationVersion.Name, ChartVersion: appStoreApplicationVersion.Version, diff --git a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go index 7ee2f13023..2530bfcb4b 100644 --- a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go +++ b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go @@ -49,6 +49,8 @@ type InstalledAppDBService interface { GetInstalledAppVersion(id int, userId int32) (*appStoreBean.InstallAppVersionDTO, error) CreateInstalledAppVersion(installAppVersionRequest *appStoreBean.InstallAppVersionDTO, tx *pg.Tx) (*appStoreRepo.InstalledAppVersions, error) UpdateInstalledAppVersion(installedAppVersion *appStoreRepo.InstalledAppVersions, installAppVersionRequest *appStoreBean.InstallAppVersionDTO, tx *pg.Tx) (*appStoreRepo.InstalledAppVersions, error) + + ChangeAppNameToDisplayNameForInstalledApp(installedApp *appStoreRepo.InstalledApps) } type InstalledAppDBServiceImpl struct { @@ -312,3 +314,7 @@ func (impl *InstalledAppDBServiceImpl) UpdateInstalledAppVersion(installedAppVer } return installedAppVersion, nil } + +func (impl *InstalledAppDBServiceImpl) ChangeAppNameToDisplayNameForInstalledApp(installedApp *appStoreRepo.InstalledApps) { + installedApp.ChangeAppNameToDisplayName() +} From 7abc37a0df75aad8fc4972e669d03e461ee02b3a Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Wed, 24 Apr 2024 11:37:12 +0530 Subject: [PATCH 14/35] bug fixes --- .../service/AppStoreDeploymentDBService.go | 10 +++++---- .../service/AppStoreDeploymentService.go | 21 ++++++++++++++++++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go index 058ace621b..f6bf18d3e0 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go @@ -389,10 +389,12 @@ func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, di return err } } else { - //this will handle the case when ext-helm app is already assigned to a project and an entry already exist in app table - //then this will override app_name with unique identifier app name and update display_name also - appModel.AppName = appName - appModel.DisplayName = displayName + if len(displayName) > 0 { + //handling the case when ext-helm app is already assigned to a project and an entry already exist in app table + //then this will override app_name with unique identifier app name and update display_name also + appModel.AppName = appName + appModel.DisplayName = displayName + } // update team id if appModel exist appModel.TeamId = teamId appModel.UpdateAuditLog(userId) diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go index 5f05022a0f..eaa479c25f 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go @@ -47,6 +47,7 @@ import ( "go.uber.org/zap" "net/http" "regexp" + "strconv" "strings" "time" ) @@ -397,7 +398,6 @@ func (impl *AppStoreDeploymentServiceImpl) UpdateProjectHelmApp(updateAppRequest var appName string var displayName string var namespace string - appName = updateAppRequest.AppName if isExternalHelmApp(updateAppRequest.AppId) { appIdentifier, err := impl.helmAppService.DecodeAppId(updateAppRequest.AppId) if err != nil { @@ -407,6 +407,15 @@ func (impl *AppStoreDeploymentServiceImpl) UpdateProjectHelmApp(updateAppRequest appName = appIdentifier.GetUniqueAppNameIdentifier() displayName = updateAppRequest.AppName namespace = appIdentifier.Namespace + } else { + //in case the external app is linked, then it's unique identifier is set in app_name col. hence retrieving appName + //for this case, although this will also handle the case for non-external apps + appId, err := strconv.Atoi(updateAppRequest.AppId) + if err != nil { + impl.logger.Errorw("UpdateProjectHelmApp, error in converting appId into int", "appId", updateAppRequest.AppId, "err", err) + return err + } + appName = impl.getAppNameForInstalledApp(appId) } impl.logger.Infow("update helm project request", updateAppRequest) err := impl.appStoreDeploymentDBService.UpdateProjectForHelmApp(appName, displayName, namespace, updateAppRequest.TeamId, updateAppRequest.UserId) @@ -997,3 +1006,13 @@ func (impl *AppStoreDeploymentServiceImpl) handleGitOpsRepoUrlMigration(tx *pg.T } return err } + +// getAppNameForInstalledApp will fetch and returns AppName from app table +func (impl *AppStoreDeploymentServiceImpl) getAppNameForInstalledApp(appId int) string { + app, err := impl.appRepository.FindById(appId) + if err != nil { + impl.logger.Errorw("UpdateProjectHelmApp, error in finding app by appId", "appId", appId, "err", err) + return "" + } + return app.AppName +} From 9795efc557551337725f484c8e4822de1df302e4 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Wed, 24 Apr 2024 12:23:15 +0530 Subject: [PATCH 15/35] linkHelmApplicationToChartStore, use displayName instead of appName for UpdateApplicationWithChartInfo --- pkg/appStore/installedApp/service/AppStoreDeploymentService.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go index eaa479c25f..036b0e72f7 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go @@ -904,7 +904,7 @@ func (impl *AppStoreDeploymentServiceImpl) linkHelmApplicationToChartStore(insta ChartVersion: appStoreAppVersion.Version, ReleaseIdentifier: &bean4.ReleaseIdentifier{ ReleaseNamespace: installAppVersionRequest.Namespace, - ReleaseName: installAppVersionRequest.AppName, + ReleaseName: installAppVersionRequest.DisplayName, }, }, SourceAppType: bean3.SOURCE_HELM_APP, From 2e492c03d44a222a8973dae13165340dc79a24b0 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Wed, 24 Apr 2024 12:57:36 +0530 Subject: [PATCH 16/35] LinkHelmApplicationToChartStore: server mode full from hyperion when going to link a chart --- pkg/appStore/installedApp/service/AppStoreDeploymentService.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go index 036b0e72f7..9ef8abf7da 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go @@ -366,7 +366,7 @@ func (impl *AppStoreDeploymentServiceImpl) LinkHelmApplicationToChartStore(ctx c installAppVersionRequestDto := &appStoreBean.InstallAppVersionDTO{ AppName: appIdentifier.GetUniqueAppNameIdentifier(), UserId: userId, - AppOfferingMode: util2.SERVER_MODE_HYPERION, + AppOfferingMode: util2.SERVER_MODE_FULL, ClusterId: appIdentifier.ClusterId, Namespace: appIdentifier.Namespace, AppStoreVersion: int(request.GetAppStoreApplicationVersionId()), From ed3ed9a292c977d93ff4cfbd1ed21b9f1d054731 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Wed, 24 Apr 2024 13:00:55 +0530 Subject: [PATCH 17/35] revert:- LinkHelmApplicationToChartStore: server mode full from hyperion when going to link a chart --- pkg/appStore/installedApp/service/AppStoreDeploymentService.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go index 9ef8abf7da..036b0e72f7 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go @@ -366,7 +366,7 @@ func (impl *AppStoreDeploymentServiceImpl) LinkHelmApplicationToChartStore(ctx c installAppVersionRequestDto := &appStoreBean.InstallAppVersionDTO{ AppName: appIdentifier.GetUniqueAppNameIdentifier(), UserId: userId, - AppOfferingMode: util2.SERVER_MODE_FULL, + AppOfferingMode: util2.SERVER_MODE_HYPERION, ClusterId: appIdentifier.ClusterId, Namespace: appIdentifier.Namespace, AppStoreVersion: int(request.GetAppStoreApplicationVersionId()), From 4d15cd8436d85cd121bc23bdb6df722b29103e2a Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Wed, 24 Apr 2024 13:17:46 +0530 Subject: [PATCH 18/35] on update proj, update app offering mode to full and, getAppNameForInstalledApp changed param to installed app id from appId which was wrong --- .../service/AppStoreDeploymentDBService.go | 1 + .../service/AppStoreDeploymentService.go | 21 ++++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go index f6bf18d3e0..c0c7a45ba7 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go @@ -397,6 +397,7 @@ func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, di } // update team id if appModel exist appModel.TeamId = teamId + appModel.AppOfferingMode = globalUtil.SERVER_MODE_FULL appModel.UpdateAuditLog(userId) err = impl.appRepository.UpdateWithTxn(appModel, tx) if err != nil { diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go index 036b0e72f7..6927989213 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go @@ -47,7 +47,6 @@ import ( "go.uber.org/zap" "net/http" "regexp" - "strconv" "strings" "time" ) @@ -398,6 +397,7 @@ func (impl *AppStoreDeploymentServiceImpl) UpdateProjectHelmApp(updateAppRequest var appName string var displayName string var namespace string + appName = updateAppRequest.AppName if isExternalHelmApp(updateAppRequest.AppId) { appIdentifier, err := impl.helmAppService.DecodeAppId(updateAppRequest.AppId) if err != nil { @@ -410,12 +410,10 @@ func (impl *AppStoreDeploymentServiceImpl) UpdateProjectHelmApp(updateAppRequest } else { //in case the external app is linked, then it's unique identifier is set in app_name col. hence retrieving appName //for this case, although this will also handle the case for non-external apps - appId, err := strconv.Atoi(updateAppRequest.AppId) - if err != nil { - impl.logger.Errorw("UpdateProjectHelmApp, error in converting appId into int", "appId", updateAppRequest.AppId, "err", err) - return err + appNameUniqueIdentifier := impl.getAppNameForInstalledApp(updateAppRequest.InstalledAppId) + if len(appNameUniqueIdentifier) > 0 { + appName = appNameUniqueIdentifier } - appName = impl.getAppNameForInstalledApp(appId) } impl.logger.Infow("update helm project request", updateAppRequest) err := impl.appStoreDeploymentDBService.UpdateProjectForHelmApp(appName, displayName, namespace, updateAppRequest.TeamId, updateAppRequest.UserId) @@ -1008,11 +1006,14 @@ func (impl *AppStoreDeploymentServiceImpl) handleGitOpsRepoUrlMigration(tx *pg.T } // getAppNameForInstalledApp will fetch and returns AppName from app table -func (impl *AppStoreDeploymentServiceImpl) getAppNameForInstalledApp(appId int) string { - app, err := impl.appRepository.FindById(appId) +func (impl *AppStoreDeploymentServiceImpl) getAppNameForInstalledApp(installedAppId int) string { + installedApp, err := impl.installedAppRepository.GetInstalledApp(installedAppId) if err != nil { - impl.logger.Errorw("UpdateProjectHelmApp, error in finding app by appId", "appId", appId, "err", err) + impl.logger.Errorw("UpdateProjectHelmApp, error in finding app by installedAppId", "installedAppId", installedAppId, "err", err) return "" } - return app.AppName + if installedApp != nil { + return installedApp.App.AppName + } + return "" } From 238b51e5d0cc7ce32cd23067cc86e1c806aa3b69 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Wed, 24 Apr 2024 16:21:51 +0530 Subject: [PATCH 19/35] handled corner cases in DeleteDBLinkedHelmApplication --- api/helm-app/service/HelmAppService.go | 174 +++++++++++++++++-------- 1 file changed, 118 insertions(+), 56 deletions(-) diff --git a/api/helm-app/service/HelmAppService.go b/api/helm-app/service/HelmAppService.go index bf8261b694..61509ad61b 100644 --- a/api/helm-app/service/HelmAppService.go +++ b/api/helm-app/service/HelmAppService.go @@ -56,7 +56,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(ctx context.Context, app *AppIdentifier, request *bean.UpdateApplicationRequestDto) (*openapi.UpdateReleaseResponse, error) GetDeploymentDetail(ctx context.Context, app *AppIdentifier, version int32) (*openapi.HelmAppDeploymentManifestDetail, error) InstallRelease(ctx context.Context, clusterId int, installReleaseRequest *gRPC.InstallReleaseRequest) (*gRPC.InstallReleaseResponse, error) @@ -445,30 +445,19 @@ 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) { - dbConnection := impl.appRepository.GetConnection() - tx, err := dbConnection.Begin() - if err != nil { - impl.logger.Errorw("error in beginning transaction", "err", err) - return nil, err - } - // Rollback tx on error. - defer tx.Rollback() +func (impl *HelmAppServiceImpl) getInstalledAppForAppIdentifier(appIdentifier *AppIdentifier) (*repository.InstalledApps, error) { //for ext apps search app from unique identifier - appUniqueIdentifier := app.GetUniqueAppNameIdentifier() - model := &repository.InstalledApps{} - // For Helm deployments ReleaseName is App.Name - model, err = impl.installedAppRepository.GetInstalledAppByAppName(appUniqueIdentifier) + 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. - impl.logger.Debugw("DeleteDBLinkedHelmApplication, not able to find installed app of external app by app unique identifier, hence finding by display name", "appUniqueIdentifier", appUniqueIdentifier) - displayName := app.ReleaseName + displayName := appIdentifier.ReleaseName model, err = impl.installedAppRepository.GetInstalledAppByAppName(displayName) if err != nil { - impl.logger.Errorw("error in fetching installed app from display name", "appDisplayName", app.ReleaseName, "err", err) + impl.logger.Errorw("error in fetching installed app from display name", "appDisplayName", displayName, "err", err) return nil, err } } else { @@ -476,60 +465,133 @@ func (impl *HelmAppServiceImpl) DeleteDBLinkedHelmApplication(ctx context.Contex return nil, err } } + return model, nil +} - // 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 - } - - // 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) +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 { - impl.logger.Errorw("error in deleting appModel", "app", appModel) - return nil, err + 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 + } } - // App Delete --> End + return model, nil +} - // InstalledApp Delete --> Start - // soft delete install app - model.Active = false - model.UpdatedBy = userId - model.UpdatedOn = time.Now() - _, err = impl.installedAppRepository.UpdateInstalledApp(model, tx) +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 { - impl.logger.Errorw("error while deleting install app", "error", err) + impl.logger.Errorw("error in beginning transaction", "err", err) return nil, err } - // InstalledApp Delete --> End + // Rollback tx on error. + defer tx.Rollback() + //model := &repository.InstalledApps{} + appModel := &app.App{} + var isAppLinkedToChartStore bool // if true, entry present in both app and installed_app table - // InstalledAppVersions Delete --> Start - models, err := impl.installedAppRepository.GetInstalledAppVersionByInstalledAppId(model.Id) - if err != nil { - impl.logger.Errorw("error while fetching install app versions", "error", 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 installedAppModel != nil && installedAppModel.Id > 0 { + isAppLinkedToChartStore = true + } + if !isAppLinkedToChartStore { + //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("DeleteDBLinkedHelmApplication, error in fetching app from appIdentifier", "appIdentifier", appIdentifier, "err", err) + return nil, err + } + } + + 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 + } + + // 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 - // 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) + // InstalledAppVersions Delete --> Start + models, err := impl.installedAppRepository.GetInstalledAppVersionByInstalledAppId(installedAppModel.Id) if err != nil { - impl.logger.Errorw("error while fetching from db", "error", err) + 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) + if err != nil { + impl.logger.Errorw("error while fetching from db", "error", err) + return nil, err + } + } + // InstalledAppVersions Delete --> End + } else { + 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 } @@ -618,7 +680,7 @@ func (impl *HelmAppServiceImpl) UpdateApplication(ctx context.Context, app *AppI _, err := impl.installedAppRepository.GetInstalledAppByAppName(app.GetUniqueAppNameIdentifier()) if err != nil && util.IsErrNoRows(err) { return nil, &util.ApiError{HttpStatusCode: http.StatusUnprocessableEntity, Code: strconv.Itoa(http.StatusUnprocessableEntity), InternalMessage: AppNotLinkedToChartStoreErr, UserMessage: AppNotLinkedToChartStoreErr} - } else { + } else if err != nil { impl.logger.Errorw("error in fetching installed app from appName unique identifier", "appNameUniqueIdentifier", app.GetUniqueAppNameIdentifier()) return nil, err } From f870ab6d0c771f0750e78c3883c4ca49e0fc5b40 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Wed, 24 Apr 2024 18:58:23 +0530 Subject: [PATCH 20/35] code refactoring --- api/helm-app/HelmAppRestHandler.go | 6 +- api/helm-app/HelmAppRouter.go | 1 + api/helm-app/service/HelmAppService.go | 11 ++- pkg/app/AppCrudOperationService.go | 93 ++++++++++++------- pkg/appStore/bean/bean.go | 11 +-- .../service/EAMode/InstalledAppDBService.go | 26 ++++++ 6 files changed, 99 insertions(+), 49 deletions(-) diff --git a/api/helm-app/HelmAppRestHandler.go b/api/helm-app/HelmAppRestHandler.go index 7ee96a51d8..15ae30c452 100644 --- a/api/helm-app/HelmAppRestHandler.go +++ b/api/helm-app/HelmAppRestHandler.go @@ -224,9 +224,7 @@ func (handler *HelmAppRestHandlerImpl) GetReleaseInfo(w http.ResponseWriter, r * common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) return } - //for external-apps appName would be uniqueIdentifier - appName := appIdentifier.GetUniqueAppNameIdentifier() - installedApp, err := handler.installedAppService.GetInstalledAppByClusterNamespaceAndName(appIdentifier.ClusterId, appIdentifier.Namespace, appName) + installedAppVersionDto, err := handler.installedAppService.GetReleaseInfo(appIdentifier) if err != nil { common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) return @@ -234,7 +232,7 @@ func (handler *HelmAppRestHandlerImpl) GetReleaseInfo(w http.ResponseWriter, r * res := &bean.ReleaseAndInstalledAppInfo{ ReleaseInfo: releaseInfo, - InstalledAppInfo: bean.ConvertToInstalledAppInfo(installedApp), + InstalledAppInfo: bean.ConvertToInstalledAppInfo(installedAppVersionDto), } common.WriteJsonResp(w, err, res, http.StatusOK) diff --git a/api/helm-app/HelmAppRouter.go b/api/helm-app/HelmAppRouter.go index 6036b36b9c..132708fb46 100644 --- a/api/helm-app/HelmAppRouter.go +++ b/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") diff --git a/api/helm-app/service/HelmAppService.go b/api/helm-app/service/HelmAppService.go index 61509ad61b..b69b85c878 100644 --- a/api/helm-app/service/HelmAppService.go +++ b/api/helm-app/service/HelmAppService.go @@ -446,9 +446,11 @@ func (impl *HelmAppServiceImpl) GetDesiredManifest(ctx context.Context, app *App } 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) + 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 @@ -458,11 +460,11 @@ func (impl *HelmAppServiceImpl) getInstalledAppForAppIdentifier(appIdentifier *A 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 nil, err + return model, err } } else { impl.logger.Errorw("error in fetching installed app by app unique identifier", "appUniqueIdentifier", appUniqueIdentifier, "err", err) - return nil, err + return model, err } } return model, nil @@ -500,7 +502,6 @@ func (impl *HelmAppServiceImpl) DeleteDBLinkedHelmApplication(ctx context.Contex } // Rollback tx on error. defer tx.Rollback() - //model := &repository.InstalledApps{} appModel := &app.App{} var isAppLinkedToChartStore bool // if true, entry present in both app and installed_app table @@ -509,7 +510,7 @@ func (impl *HelmAppServiceImpl) DeleteDBLinkedHelmApplication(ctx context.Contex impl.logger.Errorw("DeleteDBLinkedHelmApplication, error in fetching installed app for app identifier", "appIdentifier", appIdentifier, "err", err) return nil, err } - if installedAppModel != nil && installedAppModel.Id > 0 { + if installedAppModel.Id > 0 { isAppLinkedToChartStore = true } if !isAppLinkedToChartStore { diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index dc68c500f9..6348b8bd7a 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -443,6 +443,56 @@ func (impl AppCrudOperationServiceImpl) IsExternalAppLinkedToChartStore(appId in return false, "", nil } +// getAppAndProjectForAppIdentifier, returns app db model for an app unique identifier or from display_name if both exists else it throws pg.ErrNoRows +func (impl AppCrudOperationServiceImpl) getAppAndProjectForAppIdentifier(appIdentifier *client.AppIdentifier) (*appRepository.App, error) { + app := &appRepository.App{} + var err error + appNameUniqueIdentifier := appIdentifier.GetUniqueAppNameIdentifier() + app, err = impl.appRepository.FindAppAndProjectByAppName(appNameUniqueIdentifier) + if err != nil && err != pg.ErrNoRows { + impl.logger.Errorw("error in fetching app meta data by unique app identifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) + return app, err + } + if util.IsErrNoRows(err) { + //find app by display name if not found by unique identifier + app, err = impl.appRepository.FindAppAndProjectByAppName(appIdentifier.ReleaseName) + if err != nil { + impl.logger.Errorw("error in fetching app meta data by display name", "displayName", appIdentifier.ReleaseName, "err", err) + return app, err + } + } + return app, nil +} + +// updateAppNameToUniqueAppIdentifierInApp, migrates values of app_name col. in app table to unique identifier and also updates display_name with releaseName +func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp(app *appRepository.App, appIdentifier *client.AppIdentifier, userId int32) error { + appNameUniqueIdentifier := appIdentifier.GetUniqueAppNameIdentifier() + //if isLinked is true then installed_app found for this app then this app name is already linked to an installed app then + //update this app's app_name with unique identifier along with display_name. + isLinked, uniqueIdentifierForAlreadyLinkedApp, err := impl.IsExternalAppLinkedToChartStore(app.Id, appIdentifier.ReleaseName) + if err != nil { + impl.logger.Errorw("error in checking IsExternalAppLinkedToChartStore", "appId", app.Id, "err", err) + return err + } + if isLinked { + // if installed_app is already present for that display_name then migrate the app_name to unique identifier with installed_app ns and cluster id data + app.AppName = uniqueIdentifierForAlreadyLinkedApp + } else { + //app not found with unique identifier but displayName, hence migrate the app_name to unique identifier and also update display_name + app.AppName = appNameUniqueIdentifier + } + + app.DisplayName = appIdentifier.ReleaseName + app.UpdatedBy = userId + app.UpdatedOn = time.Now() + err = impl.appRepository.Update(app) + if err != nil { + impl.logger.Errorw("error in migrating displayName and appName to unique identifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) + return err + } + return nil +} + func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId int32) (*bean.AppMetaInfoDto, error) { // adding separate function for helm apps because for CLI helm apps, apps can be of form "1|clusterName|releaseName" @@ -458,43 +508,18 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId impl.logger.Errorw("error in decoding app id for external app", "appId", appId, "err", err) return nil, err } - appNameUniqueIdentifier := appIdDecoded.GetUniqueAppNameIdentifier() - app, err = impl.appRepository.FindAppAndProjectByAppName(appNameUniqueIdentifier) - if err != nil && err != pg.ErrNoRows { - impl.logger.Errorw("GetHelmAppMetaInfo, error in fetching app meta data by unique app identifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) + app, err = impl.getAppAndProjectForAppIdentifier(appIdDecoded) + if err != nil && !util.IsErrNoRows(err) { + impl.logger.Errorw("GetHelmAppMetaInfo, error in getAppAndProjectForAppIdentifier for external apps", "appIdentifier", appIdDecoded, "err", err) return nil, err } - if util.IsErrNoRows(err) { - //find app by display name if not found by unique identifier - app, err = impl.appRepository.FindAppAndProjectByAppName(appIdDecoded.ReleaseName) - if err != nil && err != pg.ErrNoRows { - impl.logger.Errorw("GetHelmAppMetaInfo, error in fetching app meta data by display name", "displayName", appIdDecoded.ReleaseName, "err", err) - return nil, err - } - if app.Id > 0 { - //if isLinked is true then installed_app found for this app then this app name is already linked to an installed app then - //update this app's app_name with unique identifier along with display_name. - isLinked, uniqueIdentifierForAlreadyLinkedApp, err := impl.IsExternalAppLinkedToChartStore(app.Id, appIdDecoded.ReleaseName) - if err != nil { - impl.logger.Errorw("GetHelmAppMetaInfo, error in checking IsExternalAppLinkedToChartStore", "appId", appId, "err", err) - return nil, err - } - if isLinked { - // if installed_app is already present for that display_name then migrate the app_name to unique identifier with installed_app ns and cluster id data - app.AppName = uniqueIdentifierForAlreadyLinkedApp - } else { - //app not found with unique identifier but displayName, hence migrate the app_name to unique identifier and also update display_name - app.AppName = appNameUniqueIdentifier - } - app.DisplayName = appIdDecoded.ReleaseName - app.UpdatedBy = userId - app.UpdatedOn = time.Now() - err = impl.appRepository.Update(app) - if err != nil { - impl.logger.Errorw("GetHelmAppMetaInfo, error in migrating displayName and appName to unique identifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) - return nil, err - } + if app.Id > 0 { + // migrate app_name with unique identifier and also update display_name + err = impl.updateAppNameToUniqueAppIdentifierInApp(app, appIdDecoded, userId) + if err != nil { + impl.logger.Errorw("GetHelmAppMetaInfo, error in migrating displayName and appName to unique identifier for external apps", "appIdentifier", appIdDecoded, "err", err) + //not returning from here as we need to show helm app metadata even if migration of app_name fails, then migration can happen on project update } } if app.Id == 0 { diff --git a/pkg/appStore/bean/bean.go b/pkg/appStore/bean/bean.go index d28ac552c3..7d0e639094 100644 --- a/pkg/appStore/bean/bean.go +++ b/pkg/appStore/bean/bean.go @@ -352,12 +352,11 @@ type ChartRepoSearch struct { } type UpdateProjectHelmAppDTO struct { - AppId string `json:"appId"` - InstalledAppId int `json:"installedAppId"` - AppName string `json:"appName"` - TeamId int `json:"teamId"` - UserId int32 `json:"userId"` - UniqueAppIdentifier string `json:"-"` + AppId string `json:"appId"` + InstalledAppId int `json:"installedAppId"` + AppName string `json:"appName"` + TeamId int `json:"teamId"` + UserId int32 `json:"userId"` } type AppstoreDeploymentStatus int diff --git a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go index 2530bfcb4b..2e5f238808 100644 --- a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go +++ b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go @@ -18,6 +18,7 @@ package EAMode import ( + "github.com/devtron-labs/devtron/api/helm-app/service" "github.com/devtron-labs/devtron/pkg/cluster" "net/http" "strconv" @@ -51,6 +52,7 @@ type InstalledAppDBService interface { UpdateInstalledAppVersion(installedAppVersion *appStoreRepo.InstalledAppVersions, installAppVersionRequest *appStoreBean.InstallAppVersionDTO, tx *pg.Tx) (*appStoreRepo.InstalledAppVersions, error) ChangeAppNameToDisplayNameForInstalledApp(installedApp *appStoreRepo.InstalledApps) + GetReleaseInfo(appIdentifier *service.AppIdentifier) (*appStoreBean.InstallAppVersionDTO, error) } type InstalledAppDBServiceImpl struct { @@ -318,3 +320,27 @@ func (impl *InstalledAppDBServiceImpl) UpdateInstalledAppVersion(installedAppVer func (impl *InstalledAppDBServiceImpl) ChangeAppNameToDisplayNameForInstalledApp(installedApp *appStoreRepo.InstalledApps) { installedApp.ChangeAppNameToDisplayName() } + +func (impl *InstalledAppDBServiceImpl) GetReleaseInfo(appIdentifier *service.AppIdentifier) (*appStoreBean.InstallAppVersionDTO, error) { + //for external-apps appName would be uniqueIdentifier + appName := appIdentifier.GetUniqueAppNameIdentifier() + installedAppVersionDto, err := impl.GetInstalledAppByClusterNamespaceAndName(appIdentifier.ClusterId, appIdentifier.Namespace, appName) + if err != nil && !util.IsErrNoRows(err) { + impl.Logger.Errorw("GetReleaseInfo, error in getting installed app by clusterId, namespace and appUniqueIdentifierName", "appIdentifier", appIdentifier, "appUniqueIdentifierName", appName, "error", err) + return nil, err + } + if util.IsErrNoRows(err) { + // when app_name is not yet migrated to unique identifier + installedAppVersionDto, err = impl.GetInstalledAppByClusterNamespaceAndName(appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.ReleaseName) + if err != nil { + impl.Logger.Errorw("GetReleaseInfo, error in getting installed app by clusterId, namespace and releaseName", "appIdentifier", appIdentifier, "error", err) + return nil, err + } + //if dto found, check if release-info request is for the same namespace app as stored in installed_app because two ext-apps can have same + //release name but in different namespaces, if they differ then release info request is for another ext-app with same name but in diff namespace + if installedAppVersionDto != nil && installedAppVersionDto.Id > 0 && installedAppVersionDto.Namespace != appIdentifier.Namespace { + installedAppVersionDto = nil + } + } + return installedAppVersionDto, nil +} From 7beee964c490a4d1687cca8e65aaad3a05300d18 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Wed, 24 Apr 2024 22:51:41 +0530 Subject: [PATCH 21/35] remove unwanted error check --- pkg/app/AppCrudOperationService.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index 6348b8bd7a..ef61137d0e 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -549,10 +549,6 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId // in case of external apps, we will send display name as appName will be a unique identifier displayName = InstalledApp.App.DisplayName } - if err != nil { - impl.logger.Errorw("error in fetching App Meta Info", "error", err) - return nil, err - } } user, err := impl.userRepository.GetByIdIncludeDeleted(app.CreatedBy) From df5d3353372acf337ebe7157267a93bad4682c50 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Thu, 25 Apr 2024 11:31:05 +0530 Subject: [PATCH 22/35] removed logic to block update and deploy when not linked to devtron --- api/helm-app/service/ClusterUtils.go | 11 +++++------ api/helm-app/service/HelmAppService.go | 9 --------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/api/helm-app/service/ClusterUtils.go b/api/helm-app/service/ClusterUtils.go index db97fc9c11..199d200bef 100644 --- a/api/helm-app/service/ClusterUtils.go +++ b/api/helm-app/service/ClusterUtils.go @@ -5,12 +5,11 @@ import ( ) const ( - DaemonSetPodDeleteError = "cannot delete DaemonSet-managed Pods" - DnsLookupNoSuchHostError = "no such host" - TimeoutError = "timeout" - NotFound = "not found" - ConnectionRefused = "connection refused" - AppNotLinkedToChartStoreErr = "app not yet linked with any chart from chart-store in devtron, hence can't perform update operation from devtron ui" + DaemonSetPodDeleteError = "cannot delete DaemonSet-managed Pods" + DnsLookupNoSuchHostError = "no such host" + TimeoutError = "timeout" + NotFound = "not found" + ConnectionRefused = "connection refused" ) func IsClusterUnReachableError(err error) bool { diff --git a/api/helm-app/service/HelmAppService.go b/api/helm-app/service/HelmAppService.go index b69b85c878..9d9c0df2d8 100644 --- a/api/helm-app/service/HelmAppService.go +++ b/api/helm-app/service/HelmAppService.go @@ -677,15 +677,6 @@ func (impl *HelmAppServiceImpl) checkIfNsExists(app *AppIdentifier) (bool, error } func (impl *HelmAppServiceImpl) UpdateApplication(ctx context.Context, app *AppIdentifier, request *bean.UpdateApplicationRequestDto) (*openapi.UpdateReleaseResponse, error) { - if request.SourceAppType == bean.SOURCE_EXTERNAL_HELM_APP { - _, err := impl.installedAppRepository.GetInstalledAppByAppName(app.GetUniqueAppNameIdentifier()) - if err != nil && util.IsErrNoRows(err) { - return nil, &util.ApiError{HttpStatusCode: http.StatusUnprocessableEntity, Code: strconv.Itoa(http.StatusUnprocessableEntity), InternalMessage: AppNotLinkedToChartStoreErr, UserMessage: AppNotLinkedToChartStoreErr} - } else if err != nil { - impl.logger.Errorw("error in fetching installed app from appName unique identifier", "appNameUniqueIdentifier", app.GetUniqueAppNameIdentifier()) - return nil, err - } - } config, err := impl.GetClusterConf(app.ClusterId) if err != nil { impl.logger.Errorw("error in fetching cluster detail", "clusterId", app.ClusterId, "err", err) From 290452c49f2274a87873dc26e832c4a3d8628965 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Thu, 25 Apr 2024 19:23:01 +0530 Subject: [PATCH 23/35] code review incorporations and code refactoring --- api/appStore/InstalledAppRestHandler.go | 5 +- .../AppStoreDeploymentRestHandler.go | 2 - api/helm-app/service/HelmAppService.go | 7 +- .../app/appList/AppListingRestHandler.go | 3 +- cmd/external-app/wire_gen.go | 10 +-- internal/sql/repository/app/AppRepository.go | 4 + pkg/app/AppCrudOperationService.go | 65 +++++++++----- pkg/appStore/adapter/Adapter.go | 8 +- pkg/appStore/bean/bean.go | 4 +- .../service/AppStoreDeploymentDBService.go | 90 +++++++++++-------- .../service/EAMode/EAModeDeploymentService.go | 3 +- .../service/EAMode/InstalledAppDBService.go | 5 +- pkg/appStore/util/util.go | 4 + .../values/service/AppStoreValuesService.go | 7 ++ wire_gen.go | 4 +- 15 files changed, 139 insertions(+), 82 deletions(-) diff --git a/api/appStore/InstalledAppRestHandler.go b/api/appStore/InstalledAppRestHandler.go index f4a7900d56..a5cad4921c 100644 --- a/api/appStore/InstalledAppRestHandler.go +++ b/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,7 +604,7 @@ func (handler *InstalledAppRestHandlerImpl) FetchAppDetailsForInstalledApp(w htt common.WriteJsonResp(w, err, "App not found in database", http.StatusBadRequest) return } - if len(installedApp.App.DisplayName) > 0 { + 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) } @@ -728,7 +729,7 @@ func (handler *InstalledAppRestHandlerImpl) FetchResourceTree(w http.ResponseWri common.WriteJsonResp(w, err, "App not found in database", http.StatusBadRequest) return } - if len(installedApp.App.DisplayName) > 0 { + 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) } diff --git a/api/appStore/deployment/AppStoreDeploymentRestHandler.go b/api/appStore/deployment/AppStoreDeploymentRestHandler.go index 198d25e954..0c739e360c 100644 --- a/api/appStore/deployment/AppStoreDeploymentRestHandler.go +++ b/api/appStore/deployment/AppStoreDeploymentRestHandler.go @@ -579,7 +579,6 @@ func (handler AppStoreDeploymentRestHandlerImpl) UpdateProjectHelmApp(w http.Res handler.Logger.Errorw("error in decoding app id", "err", err) common.WriteJsonResp(w, err, "error in decoding app id", http.StatusBadRequest) } - // this rbac object checks that whether user have permission to change current project. rbacObjectForCurrentProject, rbacObjectForCurrentProject2 := handler.enforcerUtilHelm.GetHelmObjectByClusterIdNamespaceAndAppName(appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.ReleaseName) ok := handler.enforcer.Enforce(token, casbin.ResourceHelmApp, casbin.ActionUpdate, rbacObjectForCurrentProject) || handler.enforcer.Enforce(token, casbin.ResourceHelmApp, casbin.ActionUpdate, rbacObjectForCurrentProject2) @@ -596,7 +595,6 @@ func (handler AppStoreDeploymentRestHandlerImpl) UpdateProjectHelmApp(w http.Res handler.Logger.Errorw("service err, InstalledAppId", "err", err, "InstalledAppId", request.InstalledAppId) common.WriteJsonResp(w, fmt.Errorf("Unable to fetch installed app details"), nil, http.StatusBadRequest) } - if installedApp.IsVirtualEnvironment { rbacObjectForCurrentProject, _ := handler.enforcerUtilHelm.GetAppRBACNameByInstalledAppId(request.InstalledAppId) ok := handler.enforcer.Enforce(token, casbin.ResourceHelmApp, casbin.ActionUpdate, rbacObjectForCurrentProject) diff --git a/api/helm-app/service/HelmAppService.go b/api/helm-app/service/HelmAppService.go index 4c88a99e37..ae7e557849 100644 --- a/api/helm-app/service/HelmAppService.go +++ b/api/helm-app/service/HelmAppService.go @@ -445,6 +445,7 @@ func (impl *HelmAppServiceImpl) GetDesiredManifest(ctx context.Context, app *App return response, nil } +// 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 @@ -1086,10 +1087,10 @@ 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 { - //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 a external app. return r.ReleaseName + "-" + r.Namespace + "-" + strconv.Itoa(r.ClusterId) } diff --git a/api/restHandler/app/appList/AppListingRestHandler.go b/api/restHandler/app/appList/AppListingRestHandler.go index 1dfac0db92..162032f8fd 100644 --- a/api/restHandler/app/appList/AppListingRestHandler.go +++ b/api/restHandler/app/appList/AppListingRestHandler.go @@ -27,6 +27,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" @@ -900,7 +901,7 @@ func (handler AppListingRestHandlerImpl) GetHostUrlsByBatch(w http.ResponseWrite common.WriteJsonResp(w, err, "App not found in database", http.StatusBadRequest) return } - if len(installedApp.App.DisplayName) > 0 { + 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) } diff --git a/cmd/external-app/wire_gen.go b/cmd/external-app/wire_gen.go index 7173eef13c..13a5835986 100644 --- a/cmd/external-app/wire_gen.go +++ b/cmd/external-app/wire_gen.go @@ -1,6 +1,6 @@ // Code generated by Wire. DO NOT EDIT. -//go:generate go run -mod=mod github.com/google/wire/cmd/wire +//go:generate go run github.com/google/wire/cmd/wire //go:build !wireinject // +build !wireinject @@ -248,7 +248,10 @@ func InitializeApp() (*App, error) { deploymentTypeOverrideServiceImpl := providerConfig.NewDeploymentTypeOverrideServiceImpl(sugaredLogger, environmentVariables, attributesServiceImpl) eaModeDeploymentServiceImpl := EAMode.NewEAModeDeploymentServiceImpl(sugaredLogger, helmAppServiceImpl, appStoreApplicationVersionRepositoryImpl, helmAppClientImpl, installedAppRepositoryImpl, ociRegistryConfigRepositoryImpl) appStoreValidatorImpl := service2.NewAppAppStoreValidatorImpl(sugaredLogger) - appStoreDeploymentDBServiceImpl := service2.NewAppStoreDeploymentDBServiceImpl(sugaredLogger, installedAppRepositoryImpl, appStoreApplicationVersionRepositoryImpl, appRepositoryImpl, environmentServiceImpl, clusterServiceImpl, installedAppVersionHistoryRepositoryImpl, environmentVariables, gitOpsConfigReadServiceImpl, deploymentTypeOverrideServiceImpl, eaModeDeploymentServiceImpl, appStoreValidatorImpl) + appLabelRepositoryImpl := pipelineConfig.NewAppLabelRepositoryImpl(db) + materialRepositoryImpl := pipelineConfig.NewMaterialRepositoryImpl(db) + appCrudOperationServiceImpl := app2.NewAppCrudOperationServiceImpl(appLabelRepositoryImpl, sugaredLogger, appRepositoryImpl, userRepositoryImpl, installedAppRepositoryImpl, genericNoteServiceImpl, materialRepositoryImpl, helmAppServiceImpl) + appStoreDeploymentDBServiceImpl := service2.NewAppStoreDeploymentDBServiceImpl(sugaredLogger, installedAppRepositoryImpl, appStoreApplicationVersionRepositoryImpl, appRepositoryImpl, environmentServiceImpl, clusterServiceImpl, installedAppVersionHistoryRepositoryImpl, environmentVariables, gitOpsConfigReadServiceImpl, deploymentTypeOverrideServiceImpl, eaModeDeploymentServiceImpl, appStoreValidatorImpl, appCrudOperationServiceImpl) chartGroupDeploymentRepositoryImpl := repository7.NewChartGroupDeploymentRepositoryImpl(db, sugaredLogger) acdConfig, err := argocdServer.GetACDDeploymentConfig() if err != nil { @@ -371,9 +374,6 @@ func InitializeApp() (*App, error) { userTerminalAccessRouterImpl := terminal2.NewUserTerminalAccessRouterImpl(userTerminalAccessRestHandlerImpl) attributesRestHandlerImpl := restHandler.NewAttributesRestHandlerImpl(sugaredLogger, enforcerImpl, userServiceImpl, attributesServiceImpl) attributesRouterImpl := router.NewAttributesRouterImpl(attributesRestHandlerImpl) - appLabelRepositoryImpl := pipelineConfig.NewAppLabelRepositoryImpl(db) - materialRepositoryImpl := pipelineConfig.NewMaterialRepositoryImpl(db) - appCrudOperationServiceImpl := app2.NewAppCrudOperationServiceImpl(appLabelRepositoryImpl, sugaredLogger, appRepositoryImpl, userRepositoryImpl, installedAppRepositoryImpl, genericNoteServiceImpl, materialRepositoryImpl, helmAppServiceImpl) appInfoRestHandlerImpl := appInfo.NewAppInfoRestHandlerImpl(sugaredLogger, appCrudOperationServiceImpl, userServiceImpl, validate, enforcerUtilImpl, enforcerImpl, helmAppServiceImpl, enforcerUtilHelmImpl, genericNoteServiceImpl) appInfoRouterImpl := appInfo2.NewAppInfoRouterImpl(sugaredLogger, appInfoRestHandlerImpl) appFilteringRestHandlerImpl := appList.NewAppFilteringRestHandlerImpl(sugaredLogger, teamServiceImpl, enforcerImpl, userServiceImpl, clusterServiceImpl, environmentServiceImpl) diff --git a/internal/sql/repository/app/AppRepository.go b/internal/sql/repository/app/AppRepository.go index 4988783ec6..73ca2a541b 100644 --- a/internal/sql/repository/app/AppRepository.go +++ b/internal/sql/repository/app/AppRepository.go @@ -41,6 +41,10 @@ type App struct { sql.AuditLog } +func (r *App) IsAppJobOrExternalType() bool { + return len(r.DisplayName) > 0 +} + type AppRepository interface { SaveWithTxn(pipelineGroup *App, tx *pg.Tx) error Update(app *App) error diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index ef61137d0e..1ef27a1499 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -22,6 +22,7 @@ import ( "fmt" client "github.com/devtron-labs/devtron/api/helm-app/service" "github.com/devtron-labs/devtron/internal/util" + util2 "github.com/devtron-labs/devtron/pkg/appStore/util" "regexp" "strconv" "strings" @@ -29,7 +30,6 @@ import ( "github.com/devtron-labs/common-lib/utils/k8s" appRepository "github.com/devtron-labs/devtron/internal/sql/repository/app" - "github.com/devtron-labs/devtron/internal/sql/repository/helper" "github.com/devtron-labs/devtron/internal/sql/repository/pipelineConfig" repository2 "github.com/devtron-labs/devtron/pkg/appStore/installedApp/repository" "github.com/devtron-labs/devtron/pkg/auth/user/repository" @@ -56,7 +56,8 @@ type AppCrudOperationService interface { UpdateProjectForApps(request *bean.UpdateProjectBulkAppsRequest) (*bean.UpdateProjectBulkAppsRequest, error) GetAppMetaInfoByAppName(appName string) (*bean.AppMetaInfoDto, error) GetAppListByTeamIds(teamIds []int, appType string) ([]*TeamAppBean, error) - IsExternalAppLinkedToChartStore(appId int, releaseName string) (bool, string, error) + + IsExternalAppLinkedToChartStore(appId int) (bool, int, string, error) } type AppCrudOperationServiceImpl struct { @@ -348,7 +349,7 @@ func (impl AppCrudOperationServiceImpl) GetAppMetaInfo(appId int, installedAppId } } appName := app.AppName - if app.AppType == helper.Job || len(app.DisplayName) > 0 { + if app.IsAppJobOrExternalType() { appName = app.DisplayName } noteResp, err := impl.genericNoteService.GetGenericNotesForAppIds([]int{app.Id}) @@ -430,17 +431,18 @@ func convertUrlToHttpsIfSshType(url string) string { return httpsURL } -func (impl AppCrudOperationServiceImpl) IsExternalAppLinkedToChartStore(appId int, releaseName string) (bool, string, error) { +// IsExternalAppLinkedToChartStore checks for an appId, if that app is linked to any chart-store app or not, +// if it's linked then it returns true along with clusterId and namespace for that linked installed_app +func (impl AppCrudOperationServiceImpl) IsExternalAppLinkedToChartStore(appId int) (bool, int, string, error) { installedApp, err := impl.installedAppRepository.GetInstalledAppsByAppId(appId) if err != nil && err != pg.ErrNoRows { impl.logger.Errorw("IsExternalAppLinkedToChartStore, error in fetching installed app by app id for external apps", "appId", appId, "err", err) - return false, "", err + return false, 0, "", err } if installedApp.Id > 0 { - uniqueIdentifierForAlreadyLinkedApp := releaseName + "-" + installedApp.Environment.Namespace + "-" + strconv.Itoa(installedApp.Environment.ClusterId) - return true, uniqueIdentifierForAlreadyLinkedApp, nil + return true, installedApp.Environment.ClusterId, installedApp.Environment.Namespace, nil } - return false, "", nil + return false, 0, "", nil } // getAppAndProjectForAppIdentifier, returns app db model for an app unique identifier or from display_name if both exists else it throws pg.ErrNoRows @@ -465,18 +467,26 @@ func (impl AppCrudOperationServiceImpl) getAppAndProjectForAppIdentifier(appIden } // updateAppNameToUniqueAppIdentifierInApp, migrates values of app_name col. in app table to unique identifier and also updates display_name with releaseName -func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp(app *appRepository.App, appIdentifier *client.AppIdentifier, userId int32) error { +// returns is requested external app is migrated or other app (linked to chart store) with same name is migrated(which is tracked via namespace). +func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp(app *appRepository.App, appIdentifier *client.AppIdentifier, userId int32) (bool, error) { + var isOtherExtAppMigrated bool appNameUniqueIdentifier := appIdentifier.GetUniqueAppNameIdentifier() + //if isLinked is true then installed_app found for this app then this app name is already linked to an installed app then //update this app's app_name with unique identifier along with display_name. - isLinked, uniqueIdentifierForAlreadyLinkedApp, err := impl.IsExternalAppLinkedToChartStore(app.Id, appIdentifier.ReleaseName) + isLinked, clusterId, namespace, err := impl.IsExternalAppLinkedToChartStore(app.Id) if err != nil { impl.logger.Errorw("error in checking IsExternalAppLinkedToChartStore", "appId", app.Id, "err", err) - return err + return false, err } if isLinked { // if installed_app is already present for that display_name then migrate the app_name to unique identifier with installed_app ns and cluster id data - app.AppName = uniqueIdentifierForAlreadyLinkedApp + // this namespace and clusterId belongs to the app linked to devtron whose entry in installed_apps is present. + app.AppName = appIdentifier.ReleaseName + "-" + namespace + "-" + strconv.Itoa(clusterId) + if namespace != appIdentifier.Namespace { + //this means that migration request is for the same app whose appIdentifier is sent in request and what's present in linked installed_app + isOtherExtAppMigrated = true + } } else { //app not found with unique identifier but displayName, hence migrate the app_name to unique identifier and also update display_name app.AppName = appNameUniqueIdentifier @@ -488,9 +498,9 @@ func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp( err = impl.appRepository.Update(app) if err != nil { impl.logger.Errorw("error in migrating displayName and appName to unique identifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) - return err + return false, err } - return nil + return isOtherExtAppMigrated, nil } func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId int32) (*bean.AppMetaInfoDto, error) { @@ -503,6 +513,7 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId var displayName string impl.logger.Info("request payload, appId", appId) if len(appIdSplitted) > 1 { + var isOtherExtAppMigrated bool appIdDecoded, err := impl.helmAppService.DecodeAppId(appId) if err != nil { impl.logger.Errorw("error in decoding app id for external app", "appId", appId, "err", err) @@ -513,10 +524,9 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId impl.logger.Errorw("GetHelmAppMetaInfo, error in getAppAndProjectForAppIdentifier for external apps", "appIdentifier", appIdDecoded, "err", err) return nil, err } - - if app.Id > 0 { - // migrate app_name with unique identifier and also update display_name - err = impl.updateAppNameToUniqueAppIdentifierInApp(app, appIdDecoded, userId) + // if app.DisplayName is empty then that app_name is not yet migrated to app name unique identifier + if app.Id > 0 && len(app.DisplayName) == 0 { + isOtherExtAppMigrated, err = impl.updateAppNameToUniqueAppIdentifierInApp(app, appIdDecoded, userId) if err != nil { impl.logger.Errorw("GetHelmAppMetaInfo, error in migrating displayName and appName to unique identifier for external apps", "appIdentifier", appIdDecoded, "err", err) //not returning from here as we need to show helm app metadata even if migration of app_name fails, then migration can happen on project update @@ -525,9 +535,14 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId if app.Id == 0 { app.AppName = appIdDecoded.ReleaseName } - if len(app.DisplayName) > 0 { + if util2.IsExternalChartStoreApp(app.DisplayName) { displayName = app.DisplayName } + // we have migrated for other app with same name linked to installed app not the one coming from request, in that case + // requested app in not assigned to any project. + if isOtherExtAppMigrated { + app.TeamId = 0 + } } else { installedAppIdInt, err := strconv.Atoi(appId) if err != nil { @@ -545,7 +560,7 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId app.Team.Name = InstalledApp.App.Team.Name app.CreatedBy = InstalledApp.App.CreatedBy app.Active = InstalledApp.App.Active - if len(InstalledApp.App.DisplayName) > 0 { + if util2.IsExternalChartStoreApp(InstalledApp.App.DisplayName) { // in case of external apps, we will send display name as appName will be a unique identifier displayName = InstalledApp.App.DisplayName } @@ -573,7 +588,7 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId CreatedOn: app.CreatedOn, Active: app.Active, } - if len(displayName) > 0 { + if util2.IsExternalChartStoreApp(displayName) { //special handling for ext-helm apps where name visible on UI is display name info.AppName = displayName } @@ -667,12 +682,16 @@ func (impl AppCrudOperationServiceImpl) GetAppListByTeamIds(teamIds []int, appTy return nil, err } for _, app := range apps { + appName := app.AppName + if util2.IsExternalChartStoreApp(app.DisplayName) { + appName = app.DisplayName + } if _, ok := teamMap[app.TeamId]; ok { - teamMap[app.TeamId].AppList = append(teamMap[app.TeamId].AppList, &AppBean{Id: app.Id, Name: app.AppName}) + teamMap[app.TeamId].AppList = append(teamMap[app.TeamId].AppList, &AppBean{Id: app.Id, Name: appName}) } else { teamMap[app.TeamId] = &TeamAppBean{ProjectId: app.Team.Id, ProjectName: app.Team.Name} - teamMap[app.TeamId].AppList = append(teamMap[app.TeamId].AppList, &AppBean{Id: app.Id, Name: app.AppName}) + teamMap[app.TeamId].AppList = append(teamMap[app.TeamId].AppList, &AppBean{Id: app.Id, Name: appName}) } } diff --git a/pkg/appStore/adapter/Adapter.go b/pkg/appStore/adapter/Adapter.go index f55fb5b92b..464d1124b3 100644 --- a/pkg/appStore/adapter/Adapter.go +++ b/pkg/appStore/adapter/Adapter.go @@ -8,6 +8,7 @@ import ( appStoreBean "github.com/devtron-labs/devtron/pkg/appStore/bean" appStoreDiscoverRepository "github.com/devtron-labs/devtron/pkg/appStore/discover/repository" "github.com/devtron-labs/devtron/pkg/appStore/installedApp/repository" + util4 "github.com/devtron-labs/devtron/pkg/appStore/util" "github.com/devtron-labs/devtron/pkg/bean" "github.com/devtron-labs/devtron/pkg/cluster/adapter" clutserBean "github.com/devtron-labs/devtron/pkg/cluster/repository/bean" @@ -140,7 +141,8 @@ func GenerateInstallAppVersionDTO(installedApp *repository.InstalledApps, instal } // GenerateInstallAppVersionMinDTO converts repository.InstalledApps db object to appStoreBean.InstallAppVersionDTO bean; -// Note: It only generates a minimal DTO and doesn't include repository.InstalledAppVersions data +// Note: It only generates a minimal DTO and doesn't include repository.InstalledAppVersions data, also it's safe not to +// use this bean for creating db model again func GenerateInstallAppVersionMinDTO(installedApp *repository.InstalledApps) *appStoreBean.InstallAppVersionDTO { installAppVersionDto := &appStoreBean.InstallAppVersionDTO{ EnvironmentId: installedApp.EnvironmentId, @@ -156,7 +158,7 @@ func GenerateInstallAppVersionMinDTO(installedApp *repository.InstalledApps) *ap DeploymentAppType: installedApp.DeploymentAppType, IsVirtualEnvironment: installedApp.Environment.IsVirtualEnvironment, } - if len(installedApp.App.DisplayName) > 0 { + if util4.IsExternalChartStoreApp(installedApp.App.DisplayName) { installAppVersionDto.AppName = installedApp.App.DisplayName } return installAppVersionDto @@ -221,7 +223,7 @@ func UpdateAppDetails(request *appStoreBean.InstallAppVersionDTO, app *app.App) request.TeamId = app.TeamId request.AppOfferingMode = app.AppOfferingMode // for external apps, AppName is unique identifier(appName-ns-clusterId), hence DisplayName should be used in that case - if len(app.DisplayName) > 0 { + if util4.IsExternalChartStoreApp(app.DisplayName) { request.AppName = app.DisplayName } } diff --git a/pkg/appStore/bean/bean.go b/pkg/appStore/bean/bean.go index 7d0e639094..c3163323f2 100644 --- a/pkg/appStore/bean/bean.go +++ b/pkg/appStore/bean/bean.go @@ -76,7 +76,7 @@ const ( type InstallAppVersionDTO struct { Id int `json:"id,omitempty"` // TODO: redundant data; refers to InstalledAppVersionId AppId int `json:"appId,omitempty"` - AppName string `json:"appName,omitempty"` + AppName string `json:"appName,omitempty"` // AppName can be display_name in case of external-apps (which is not unique in that case) TeamId int `json:"teamId,omitempty"` TeamName string `json:"teamName,omitempty"` EnvironmentId int `json:"environmentId,omitempty"` @@ -116,7 +116,7 @@ type InstallAppVersionDTO struct { EnvironmentName string `json:"-"` InstallAppVersionChartDTO *InstallAppVersionChartDTO `json:"-"` AppStoreApplicationVersionId int - DisplayName string `json:"-"` + DisplayName string `json:"-"` // used only for external apps } // UpdateDeploymentAppType updates deploymentAppType to InstallAppVersionDTO diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go index c0c7a45ba7..ed2cc6c1ee 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go @@ -9,11 +9,13 @@ import ( "github.com/devtron-labs/devtron/internal/sql/repository/helper" "github.com/devtron-labs/devtron/internal/sql/repository/pipelineConfig" "github.com/devtron-labs/devtron/internal/util" + app2 "github.com/devtron-labs/devtron/pkg/app" "github.com/devtron-labs/devtron/pkg/appStore/adapter" appStoreBean "github.com/devtron-labs/devtron/pkg/appStore/bean" discoverRepository "github.com/devtron-labs/devtron/pkg/appStore/discover/repository" "github.com/devtron-labs/devtron/pkg/appStore/installedApp/repository" "github.com/devtron-labs/devtron/pkg/appStore/installedApp/service/FullMode/deployment" + util4 "github.com/devtron-labs/devtron/pkg/appStore/util" "github.com/devtron-labs/devtron/pkg/bean" clusterService "github.com/devtron-labs/devtron/pkg/cluster" clutserBean "github.com/devtron-labs/devtron/pkg/cluster/repository/bean" @@ -53,6 +55,8 @@ type AppStoreDeploymentDBService interface { MarkHelmInstalledAppDeploymentSucceeded(versionHistoryId int) error // UpdateInstalledAppVersionHistoryStatus will update the Status in the repository.InstalledAppVersionHistory UpdateInstalledAppVersionHistoryStatus(versionHistoryId int, status string) error + // GetActiveAppForAppIdentifierOrReleaseName returns app db model for an app unique identifier or from display_name if either exists else it throws pg.ErrNoRows + GetActiveAppForAppIdentifierOrReleaseName(appNameUniqueIdentifier, releaseName string) (*app.App, error) } type AppStoreDeploymentDBServiceImpl struct { @@ -68,6 +72,7 @@ type AppStoreDeploymentDBServiceImpl struct { deploymentTypeOverrideService providerConfig.DeploymentTypeOverrideService fullModeDeploymentService deployment.FullModeDeploymentService appStoreValidator AppStoreValidator + appCrudOperationService app2.AppCrudOperationService } func NewAppStoreDeploymentDBServiceImpl(logger *zap.SugaredLogger, @@ -80,7 +85,8 @@ func NewAppStoreDeploymentDBServiceImpl(logger *zap.SugaredLogger, envVariables *globalUtil.EnvironmentVariables, gitOpsConfigReadService config.GitOpsConfigReadService, deploymentTypeOverrideService providerConfig.DeploymentTypeOverrideService, - fullModeDeploymentService deployment.FullModeDeploymentService, appStoreValidator AppStoreValidator) *AppStoreDeploymentDBServiceImpl { + fullModeDeploymentService deployment.FullModeDeploymentService, appStoreValidator AppStoreValidator, + appCrudOperationService app2.AppCrudOperationService) *AppStoreDeploymentDBServiceImpl { return &AppStoreDeploymentDBServiceImpl{ logger: logger, installedAppRepository: installedAppRepository, @@ -94,6 +100,7 @@ func NewAppStoreDeploymentDBServiceImpl(logger *zap.SugaredLogger, deploymentTypeOverrideService: deploymentTypeOverrideService, fullModeDeploymentService: fullModeDeploymentService, appStoreValidator: appStoreValidator, + appCrudOperationService: appCrudOperationService, } } @@ -115,7 +122,7 @@ func (impl *AppStoreDeploymentDBServiceImpl) AppStoreDeployOperationDB(installRe TeamId: installRequest.TeamId, UserId: installRequest.UserId, } - if len(installRequest.DisplayName) > 0 { + if util4.IsExternalChartStoreApp(installRequest.DisplayName) { //this is the case of linking external helm app to devtron chart store appCreateRequest.AppType = helper.ExternalChartStoreApp appCreateRequest.DisplayName = installRequest.DisplayName @@ -322,41 +329,52 @@ func (impl *AppStoreDeploymentDBServiceImpl) UpdateInstalledAppVersionHistoryWit return nil } +func (impl *AppStoreDeploymentDBServiceImpl) GetActiveAppForAppIdentifierOrReleaseName(appNameUniqueIdentifier, releaseName string) (*app.App, error) { + app, err := impl.appRepository.FindActiveByName(appNameUniqueIdentifier) + if err != nil && err != pg.ErrNoRows { + impl.logger.Errorw("error in fetching app meta data by unique app identifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) + return nil, err + } + if util.IsErrNoRows(err) { + //find app by displayName/releaseName if not found by unique identifier + app, err = impl.appRepository.FindActiveByName(releaseName) + if err != nil { + impl.logger.Errorw("error in fetching app meta data by display name", "displayName", releaseName, "err", err) + return nil, err + } + } + return app, nil +} + func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, displayName, namespace string, teamId int, userId int32) error { - appModel, err := impl.appRepository.FindActiveByName(appName) - if err != nil && util.IsErrNoRows(err) { - // if we can't find by appName then find by display name, if app is found via display name then app was previously assigned with project or linked with devtron with app_name as display_name - appModel, err = impl.appRepository.FindActiveByName(displayName) - if err != nil && !util.IsErrNoRows(err) { - impl.logger.Errorw("error in fetching appModel by displayName", "displayName", displayName, "err", err) + appModel, err := impl.GetActiveAppForAppIdentifierOrReleaseName(appName, displayName) + if err != nil && !util.IsErrNoRows(err) { + impl.logger.Errorw("error in fetching appModel by appName", "appName", appName, "err", err) + return err + } + + // only external app will have a display name, so checking the following case only for external apps + if appModel != nil && appModel.Id > 0 && len(displayName) > 0 { + /* + 1. now we will check if for that appModel, installed_app entry present or not i.e. linked to devtron or not, + 2. if not, then let the normal flow continue as we can change the app_name with app unique identifier. + 3. if exists then we will check if request's namespace is same as what present in installed_apps. + - if ns doesn't match then update proj. req. is for some other app found in app table via display_name, + in this case create a new entry in app table for the request. + - if ns matches, then update proj. req. is for same app present in installed_apps, in that case we will update + the app_name with unique identifier and display_name along with team_id. + */ + isLinkedToDevtron, _, linkedInstalledAppNamespace, err := impl.appCrudOperationService.IsExternalAppLinkedToChartStore(appModel.Id) + if err != nil { + impl.logger.Errorw("error in checking IsExternalAppLinkedToChartStore", "appId", appModel.Id, "err", err) return err } - // external app will have a display name, so checking the following case only for external apps - if appModel != nil && appModel.Id > 0 && len(displayName) > 0 { - /* - 1. now we will check if for that appModel, installed_app entry present or not, - 2. if not, then let the normal flow continue as we can change the app_name with app unique identifier. - 3. if exists then we will check if request's namespace is same as what present in installed_apps. - - if ns doesn't match then update proj. req. is for some other app found in app table via display_name, - in this case create a new entry in app table for the request. - - if ns matches, then update proj. req. is for same app present in installed_apps, in that case we will update - the app_name with unique identifier and display_name along with team_id. - */ - installedApp, err := impl.installedAppRepository.GetInstalledAppsByAppId(appModel.Id) - if err != nil && !util.IsErrNoRows(err) { - impl.logger.Errorw("error in fetching installed app by appId", "appId", appModel.Id, "appName", appName, "err", err) - return err - } - if installedApp.Id > 0 { - if namespace != installedApp.Environment.Namespace { - //assigning appModel as nil, so that it will create a new entry in app for the request - appModel = nil - } + if isLinkedToDevtron { + if namespace != linkedInstalledAppNamespace { + //assigning appModel as nil, so that it will create a new entry in app for the request + appModel = nil } } - } else if err != nil { - impl.logger.Errorw("error in fetching appModel by appName", "appName", appName, "err", err) - return err } var appInstallationMode string @@ -379,7 +397,7 @@ func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, di UserId: userId, TeamId: teamId, } - if len(displayName) > 0 { + if util4.IsExternalChartStoreApp(displayName) { createAppRequest.AppType = helper.ExternalChartStoreApp createAppRequest.DisplayName = displayName } @@ -389,9 +407,9 @@ func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, di return err } } else { - if len(displayName) > 0 { - //handling the case when ext-helm app is already assigned to a project and an entry already exist in app table - //then this will override app_name with unique identifier app name and update display_name also + if util4.IsExternalChartStoreApp(displayName) { + //handling the case when ext-helm app is already assigned to a project and an entry already exist in app table but + //not yet migrated, then this will override app_name with unique identifier app name and update display_name also appModel.AppName = appName appModel.DisplayName = displayName } diff --git a/pkg/appStore/installedApp/service/EAMode/EAModeDeploymentService.go b/pkg/appStore/installedApp/service/EAMode/EAModeDeploymentService.go index 4914c63e86..3e7f15012c 100644 --- a/pkg/appStore/installedApp/service/EAMode/EAModeDeploymentService.go +++ b/pkg/appStore/installedApp/service/EAMode/EAModeDeploymentService.go @@ -8,6 +8,7 @@ import ( client "github.com/devtron-labs/devtron/api/helm-app/service" repository2 "github.com/devtron-labs/devtron/internal/sql/repository/dockerRegistry" "github.com/devtron-labs/devtron/pkg/appStore/installedApp/service/bean" + util2 "github.com/devtron-labs/devtron/pkg/appStore/util" commonBean "github.com/devtron-labs/devtron/pkg/deployment/gitOps/common/bean" validationBean "github.com/devtron-labs/devtron/pkg/deployment/gitOps/validation/bean" "net/http" @@ -253,7 +254,7 @@ func (impl *EAModeDeploymentServiceImpl) updateApplicationWithChartInfo(ctx cont return err } appName := installedApp.App.AppName - if len(installedApp.App.DisplayName) > 0 { + if util2.IsExternalChartStoreApp(installedApp.App.DisplayName) { appName = installedApp.App.DisplayName } appStoreApplicationVersion, err := impl.appStoreApplicationVersionRepository.FindById(appStoreApplicationVersionId) diff --git a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go index 2e5f238808..93c174cb86 100644 --- a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go +++ b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go @@ -19,6 +19,7 @@ package EAMode import ( "github.com/devtron-labs/devtron/api/helm-app/service" + util4 "github.com/devtron-labs/devtron/pkg/appStore/util" "github.com/devtron-labs/devtron/pkg/cluster" "net/http" "strconv" @@ -126,7 +127,7 @@ func (impl *InstalledAppDBServiceImpl) GetAll(filter *appStoreBean.AppStoreFilte LastDeployedAt: &appLocal.UpdatedOn, AppStatus: &appLocal.AppStatus, } - if len(appLocal.DisplayName) > 0 { + if util4.IsExternalChartStoreApp(appLocal.DisplayName) { //case of external app where display name is stored in app table helmAppResp.AppName = &appLocal.DisplayName } @@ -202,7 +203,7 @@ func (impl *InstalledAppDBServiceImpl) FindAppDetailsForAppstoreApplication(inst HelmReleaseInstallStatus: helmReleaseInstallStatus, Status: status, } - if len(installedAppVerison.InstalledApp.App.DisplayName) > 0 { + if util4.IsExternalChartStoreApp(installedAppVerison.InstalledApp.App.DisplayName) { deploymentContainer.AppName = installedAppVerison.InstalledApp.App.DisplayName } deploymentContainer.HelmPackageName = adapter.GetGeneratedHelmPackageName(deploymentContainer.AppName, deploymentContainer.EnvironmentName, installedAppVerison.InstalledApp.UpdatedOn) diff --git a/pkg/appStore/util/util.go b/pkg/appStore/util/util.go index c0599c9ed6..6cd28b75ed 100644 --- a/pkg/appStore/util/util.go +++ b/pkg/appStore/util/util.go @@ -47,3 +47,7 @@ func CheckAppReleaseNotExist(err error) bool { func CheckPermissionErrorForArgoCd(err error) bool { return strings.Contains(err.Error(), PermissionDenied) } + +func IsExternalChartStoreApp(displayName string) bool { + return len(displayName) > 0 +} diff --git a/pkg/appStore/values/service/AppStoreValuesService.go b/pkg/appStore/values/service/AppStoreValuesService.go index 94d5e51195..b38ba6bd74 100644 --- a/pkg/appStore/values/service/AppStoreValuesService.go +++ b/pkg/appStore/values/service/AppStoreValuesService.go @@ -19,6 +19,7 @@ package service import ( "fmt" + util2 "github.com/devtron-labs/devtron/pkg/appStore/util" "time" "github.com/devtron-labs/devtron/internal/util" @@ -240,6 +241,9 @@ func (impl AppStoreValuesServiceImpl) FindValuesByAppStoreId(appStoreId int, ins ChartVersion: installedAppVersion.AppStoreApplicationVersion.Version, EnvironmentName: installedAppVersion.InstalledApp.Environment.Name, } + if util2.IsExternalChartStoreApp(installedAppVersion.InstalledApp.App.DisplayName) { + appStoreVersion.Name = installedAppVersion.InstalledApp.App.DisplayName + } installedVal.Values = append(installedVal.Values, appStoreVersion) } @@ -260,6 +264,9 @@ func (impl AppStoreValuesServiceImpl) FindValuesByAppStoreId(appStoreId int, ins ChartVersion: installedAppVersion.AppStoreApplicationVersion.Version, EnvironmentName: installedAppVersion.InstalledApp.Environment.Name, } + if util2.IsExternalChartStoreApp(installedAppVersion.InstalledApp.App.DisplayName) { + appStoreVersion.Name = installedAppVersion.InstalledApp.App.DisplayName + } existingVal.Values = append(existingVal.Values, appStoreVersion) } diff --git a/wire_gen.go b/wire_gen.go index d65ae2f2cb..2b2f231b82 100644 --- a/wire_gen.go +++ b/wire_gen.go @@ -1,6 +1,6 @@ // Code generated by Wire. DO NOT EDIT. -//go:generate go run -mod=mod github.com/google/wire/cmd/wire +//go:generate go run github.com/google/wire/cmd/wire //go:build !wireinject // +build !wireinject @@ -655,7 +655,7 @@ func InitializeApp() (*App, error) { appStoreDeploymentCommonServiceImpl := appStoreDeploymentCommon.NewAppStoreDeploymentCommonServiceImpl(sugaredLogger, appStoreApplicationVersionRepositoryImpl, chartTemplateServiceImpl) fullModeDeploymentServiceImpl := deployment.NewFullModeDeploymentServiceImpl(sugaredLogger, applicationServiceClientImpl, argoK8sClientImpl, acdAuthConfig, chartGroupDeploymentRepositoryImpl, installedAppRepositoryImpl, installedAppVersionHistoryRepositoryImpl, argoUserServiceImpl, appStoreDeploymentCommonServiceImpl, helmAppServiceImpl, appStatusServiceImpl, pipelineStatusTimelineServiceImpl, userServiceImpl, pipelineStatusTimelineRepositoryImpl, appStoreApplicationVersionRepositoryImpl, argoClientWrapperServiceImpl, acdConfig, gitOperationServiceImpl, gitOpsConfigReadServiceImpl, gitOpsValidationServiceImpl, environmentRepositoryImpl) appStoreValidatorImpl := service3.NewAppAppStoreValidatorImpl(sugaredLogger) - appStoreDeploymentDBServiceImpl := service3.NewAppStoreDeploymentDBServiceImpl(sugaredLogger, installedAppRepositoryImpl, appStoreApplicationVersionRepositoryImpl, appRepositoryImpl, environmentServiceImpl, clusterServiceImplExtended, installedAppVersionHistoryRepositoryImpl, environmentVariables, gitOpsConfigReadServiceImpl, deploymentTypeOverrideServiceImpl, fullModeDeploymentServiceImpl, appStoreValidatorImpl) + appStoreDeploymentDBServiceImpl := service3.NewAppStoreDeploymentDBServiceImpl(sugaredLogger, installedAppRepositoryImpl, appStoreApplicationVersionRepositoryImpl, appRepositoryImpl, environmentServiceImpl, clusterServiceImplExtended, installedAppVersionHistoryRepositoryImpl, environmentVariables, gitOpsConfigReadServiceImpl, deploymentTypeOverrideServiceImpl, fullModeDeploymentServiceImpl, appStoreValidatorImpl, appCrudOperationServiceImpl) eaModeDeploymentServiceImpl := EAMode.NewEAModeDeploymentServiceImpl(sugaredLogger, helmAppServiceImpl, appStoreApplicationVersionRepositoryImpl, helmAppClientImpl, installedAppRepositoryImpl, ociRegistryConfigRepositoryImpl) deletePostProcessorImpl := service3.NewDeletePostProcessorImpl(sugaredLogger) appStoreDeploymentServiceImpl := service3.NewAppStoreDeploymentServiceImpl(sugaredLogger, installedAppRepositoryImpl, installedAppDBServiceImpl, appStoreDeploymentDBServiceImpl, chartGroupDeploymentRepositoryImpl, appStoreApplicationVersionRepositoryImpl, appRepositoryImpl, eaModeDeploymentServiceImpl, fullModeDeploymentServiceImpl, environmentServiceImpl, helmAppServiceImpl, installedAppVersionHistoryRepositoryImpl, environmentVariables, acdConfig, gitOpsConfigReadServiceImpl, deletePostProcessorImpl, appStoreValidatorImpl) From 99e89a3e656e0d7f31a232bc2c5e505b390db104 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Fri, 26 Apr 2024 01:08:44 +0530 Subject: [PATCH 24/35] handling display name in migrate app type and trigger for migrated app type --- .../InstalledAppDeploymentTypeChangeService.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/pkg/appStore/installedApp/service/FullMode/deploymentTypeChange/InstalledAppDeploymentTypeChangeService.go b/pkg/appStore/installedApp/service/FullMode/deploymentTypeChange/InstalledAppDeploymentTypeChangeService.go index 7ca63390ac..4e5937faa5 100644 --- a/pkg/appStore/installedApp/service/FullMode/deploymentTypeChange/InstalledAppDeploymentTypeChangeService.go +++ b/pkg/appStore/installedApp/service/FullMode/deploymentTypeChange/InstalledAppDeploymentTypeChangeService.go @@ -141,8 +141,13 @@ func (impl *InstalledAppDeploymentTypeChangeServiceImpl) MigrateDeploymentType(c return response, err } var installedAppIds []int - for _, item := range installedApps { - installedAppIds = append(installedAppIds, item.Id) + for _, installedApp := range installedApps { + if util2.IsExternalChartStoreApp(installedApp.App.DisplayName) { + //for ext-apps, appName is a unique identifier pertaining to devtron environment hence changing appName to ReleaseName, as going + //further interactions with helm/argo-cd will happen via release name only so refrain from doing any db updates using this installed apps + installedApp.App.AppName = installedApp.App.DisplayName + } + installedAppIds = append(installedAppIds, installedApp.Id) } if len(installedAppIds) == 0 { @@ -400,8 +405,13 @@ func (impl *InstalledAppDeploymentTypeChangeServiceImpl) TriggerAfterMigration(c } var installedAppIds []int - for _, item := range installedApps { - installedAppIds = append(installedAppIds, item.Id) + for _, installedApp := range installedApps { + if util2.IsExternalChartStoreApp(installedApp.App.DisplayName) { + //for ext-apps, appName is a unique identifier pertaining to devtron environment hence changing appName to ReleaseName, as going + //further interactions with helm/argo-cd will happen via release name only so refrain from doing any db updates using this installed apps + installedApp.App.AppName = installedApp.App.DisplayName + } + installedAppIds = append(installedAppIds, installedApp.Id) } if len(installedAppIds) == 0 { From e28f4fbe1186f8121c312af08385a5ecb52cd938 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Wed, 1 May 2024 11:32:18 +0530 Subject: [PATCH 25/35] minor fix --- pkg/app/AppCrudOperationService.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index 1ef27a1499..81b0a5f5c6 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -542,6 +542,7 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId // requested app in not assigned to any project. if isOtherExtAppMigrated { app.TeamId = 0 + app.Team.Name = "" } } else { installedAppIdInt, err := strconv.Atoi(appId) From f6ac09385fe7e21dfb31b96850b8f93da5dec117 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Wed, 1 May 2024 11:52:04 +0530 Subject: [PATCH 26/35] minor fix --- pkg/app/AppCrudOperationService.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index 81b0a5f5c6..a787444fac 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -531,6 +531,12 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId impl.logger.Errorw("GetHelmAppMetaInfo, error in migrating displayName and appName to unique identifier for external apps", "appIdentifier", appIdDecoded, "err", err) //not returning from here as we need to show helm app metadata even if migration of app_name fails, then migration can happen on project update } + // we have migrated for other app with same name linked to installed app not the one coming from request, in that case + // requested app in not assigned to any project. + if isOtherExtAppMigrated { + app.TeamId = 0 + app.Team.Name = "" + } } if app.Id == 0 { app.AppName = appIdDecoded.ReleaseName @@ -538,12 +544,7 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId if util2.IsExternalChartStoreApp(app.DisplayName) { displayName = app.DisplayName } - // we have migrated for other app with same name linked to installed app not the one coming from request, in that case - // requested app in not assigned to any project. - if isOtherExtAppMigrated { - app.TeamId = 0 - app.Team.Name = "" - } + } else { installedAppIdInt, err := strconv.Atoi(appId) if err != nil { From 632f3da2d82333a78da486b4de21722b40ffc011 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Thu, 2 May 2024 14:09:33 +0530 Subject: [PATCH 27/35] code review incorporation and refactoring --- api/helm-app/service/HelmAppService.go | 41 ++++++------------- api/helm-app/service/helper.go | 26 ++++++++++++ .../app/appInfo/AppInfoRestHandler.go | 2 +- cmd/external-app/wire_gen.go | 8 ++-- pkg/app/AppCrudOperationService.go | 39 +++++++----------- .../service/AppStoreDeploymentDBService.go | 13 +++--- .../service/EAMode/InstalledAppDBService.go | 18 +++++++- wire_gen.go | 6 +-- 8 files changed, 83 insertions(+), 70 deletions(-) create mode 100644 api/helm-app/service/helper.go diff --git a/api/helm-app/service/HelmAppService.go b/api/helm-app/service/HelmAppService.go index ce75e7a4cb..d767b85d21 100644 --- a/api/helm-app/service/HelmAppService.go +++ b/api/helm-app/service/HelmAppService.go @@ -14,7 +14,6 @@ import ( "net/http" "reflect" "strconv" - "strings" "time" "github.com/caarlos0/env/v6" @@ -505,7 +504,6 @@ func (impl *HelmAppServiceImpl) DeleteDBLinkedHelmApplication(ctx context.Contex } // Rollback tx on error. defer tx.Rollback() - appModel := &app.App{} var isAppLinkedToChartStore bool // if true, entry present in both app and installed_app table installedAppModel, err := impl.getInstalledAppForAppIdentifier(appIdentifier) @@ -516,15 +514,6 @@ func (impl *HelmAppServiceImpl) DeleteDBLinkedHelmApplication(ctx context.Contex if installedAppModel.Id > 0 { isAppLinkedToChartStore = true } - if !isAppLinkedToChartStore { - //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("DeleteDBLinkedHelmApplication, error in fetching app from appIdentifier", "appIdentifier", appIdentifier, "err", err) - return nil, err - } - } 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}); @@ -578,6 +567,13 @@ func (impl *HelmAppServiceImpl) DeleteDBLinkedHelmApplication(ctx context.Contex } // 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("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 @@ -1093,26 +1089,15 @@ type AppIdentifier struct { // 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 r.ReleaseName + "-" + r.Namespace + "-" + strconv.Itoa(r.ClusterId) + 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 { diff --git a/api/helm-app/service/helper.go b/api/helm-app/service/helper.go new file mode 100644 index 0000000000..dc835ea9a3 --- /dev/null +++ b/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 +} diff --git a/api/restHandler/app/appInfo/AppInfoRestHandler.go b/api/restHandler/app/appInfo/AppInfoRestHandler.go index 035100d144..b6e614c1d7 100644 --- a/api/restHandler/app/appInfo/AppInfoRestHandler.go +++ b/api/restHandler/app/appInfo/AppInfoRestHandler.go @@ -179,7 +179,7 @@ func (handler AppInfoRestHandlerImpl) GetHelmAppMetaInfo(w http.ResponseWriter, return } } - res, err := handler.appService.GetHelmAppMetaInfo(appIdReq, userId) + res, err := handler.appService.GetHelmAppMetaInfo(appIdReq) if err != nil { handler.logger.Errorw("service err, GetAppMetaInfo", "err", err) common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) diff --git a/cmd/external-app/wire_gen.go b/cmd/external-app/wire_gen.go index 13a5835986..e2c258a341 100644 --- a/cmd/external-app/wire_gen.go +++ b/cmd/external-app/wire_gen.go @@ -248,10 +248,7 @@ func InitializeApp() (*App, error) { deploymentTypeOverrideServiceImpl := providerConfig.NewDeploymentTypeOverrideServiceImpl(sugaredLogger, environmentVariables, attributesServiceImpl) eaModeDeploymentServiceImpl := EAMode.NewEAModeDeploymentServiceImpl(sugaredLogger, helmAppServiceImpl, appStoreApplicationVersionRepositoryImpl, helmAppClientImpl, installedAppRepositoryImpl, ociRegistryConfigRepositoryImpl) appStoreValidatorImpl := service2.NewAppAppStoreValidatorImpl(sugaredLogger) - appLabelRepositoryImpl := pipelineConfig.NewAppLabelRepositoryImpl(db) - materialRepositoryImpl := pipelineConfig.NewMaterialRepositoryImpl(db) - appCrudOperationServiceImpl := app2.NewAppCrudOperationServiceImpl(appLabelRepositoryImpl, sugaredLogger, appRepositoryImpl, userRepositoryImpl, installedAppRepositoryImpl, genericNoteServiceImpl, materialRepositoryImpl, helmAppServiceImpl) - appStoreDeploymentDBServiceImpl := service2.NewAppStoreDeploymentDBServiceImpl(sugaredLogger, installedAppRepositoryImpl, appStoreApplicationVersionRepositoryImpl, appRepositoryImpl, environmentServiceImpl, clusterServiceImpl, installedAppVersionHistoryRepositoryImpl, environmentVariables, gitOpsConfigReadServiceImpl, deploymentTypeOverrideServiceImpl, eaModeDeploymentServiceImpl, appStoreValidatorImpl, appCrudOperationServiceImpl) + appStoreDeploymentDBServiceImpl := service2.NewAppStoreDeploymentDBServiceImpl(sugaredLogger, installedAppRepositoryImpl, appStoreApplicationVersionRepositoryImpl, appRepositoryImpl, environmentServiceImpl, clusterServiceImpl, installedAppVersionHistoryRepositoryImpl, environmentVariables, gitOpsConfigReadServiceImpl, deploymentTypeOverrideServiceImpl, eaModeDeploymentServiceImpl, appStoreValidatorImpl, installedAppDBServiceImpl) chartGroupDeploymentRepositoryImpl := repository7.NewChartGroupDeploymentRepositoryImpl(db, sugaredLogger) acdConfig, err := argocdServer.GetACDDeploymentConfig() if err != nil { @@ -374,6 +371,9 @@ func InitializeApp() (*App, error) { userTerminalAccessRouterImpl := terminal2.NewUserTerminalAccessRouterImpl(userTerminalAccessRestHandlerImpl) attributesRestHandlerImpl := restHandler.NewAttributesRestHandlerImpl(sugaredLogger, enforcerImpl, userServiceImpl, attributesServiceImpl) attributesRouterImpl := router.NewAttributesRouterImpl(attributesRestHandlerImpl) + appLabelRepositoryImpl := pipelineConfig.NewAppLabelRepositoryImpl(db) + materialRepositoryImpl := pipelineConfig.NewMaterialRepositoryImpl(db) + appCrudOperationServiceImpl := app2.NewAppCrudOperationServiceImpl(appLabelRepositoryImpl, sugaredLogger, appRepositoryImpl, userRepositoryImpl, installedAppRepositoryImpl, genericNoteServiceImpl, materialRepositoryImpl, helmAppServiceImpl, installedAppDBServiceImpl) appInfoRestHandlerImpl := appInfo.NewAppInfoRestHandlerImpl(sugaredLogger, appCrudOperationServiceImpl, userServiceImpl, validate, enforcerUtilImpl, enforcerImpl, helmAppServiceImpl, enforcerUtilHelmImpl, genericNoteServiceImpl) appInfoRouterImpl := appInfo2.NewAppInfoRouterImpl(sugaredLogger, appInfoRestHandlerImpl) appFilteringRestHandlerImpl := appList.NewAppFilteringRestHandlerImpl(sugaredLogger, teamServiceImpl, enforcerImpl, userServiceImpl, clusterServiceImpl, environmentServiceImpl) diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index a787444fac..92d2b03ebe 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -22,7 +22,9 @@ import ( "fmt" client "github.com/devtron-labs/devtron/api/helm-app/service" "github.com/devtron-labs/devtron/internal/util" + "github.com/devtron-labs/devtron/pkg/appStore/installedApp/service/EAMode" util2 "github.com/devtron-labs/devtron/pkg/appStore/util" + bean2 "github.com/devtron-labs/devtron/pkg/auth/user/bean" "regexp" "strconv" "strings" @@ -49,15 +51,13 @@ type AppCrudOperationService interface { FindById(id int) (*bean.AppLabelDto, error) FindAll() ([]*bean.AppLabelDto, error) GetAppMetaInfo(appId int, installedAppId int, envId int) (*bean.AppMetaInfoDto, error) - GetHelmAppMetaInfo(appId string, userId int32) (*bean.AppMetaInfoDto, error) + GetHelmAppMetaInfo(appId string) (*bean.AppMetaInfoDto, error) GetLabelsByAppIdForDeployment(appId int) ([]byte, error) GetLabelsByAppId(appId int) (map[string]string, error) UpdateApp(request *bean.CreateAppDTO) (*bean.CreateAppDTO, error) UpdateProjectForApps(request *bean.UpdateProjectBulkAppsRequest) (*bean.UpdateProjectBulkAppsRequest, error) GetAppMetaInfoByAppName(appName string) (*bean.AppMetaInfoDto, error) GetAppListByTeamIds(teamIds []int, appType string) ([]*TeamAppBean, error) - - IsExternalAppLinkedToChartStore(appId int) (bool, int, string, error) } type AppCrudOperationServiceImpl struct { @@ -69,6 +69,7 @@ type AppCrudOperationServiceImpl struct { genericNoteService genericNotes.GenericNoteService gitMaterialRepository pipelineConfig.MaterialRepository helmAppService client.HelmAppService + installedAppDbService EAMode.InstalledAppDBService } func NewAppCrudOperationServiceImpl(appLabelRepository pipelineConfig.AppLabelRepository, @@ -76,7 +77,8 @@ func NewAppCrudOperationServiceImpl(appLabelRepository pipelineConfig.AppLabelRe installedAppRepository repository2.InstalledAppRepository, genericNoteService genericNotes.GenericNoteService, gitMaterialRepository pipelineConfig.MaterialRepository, - helmAppService client.HelmAppService) *AppCrudOperationServiceImpl { + helmAppService client.HelmAppService, + installedAppDbService EAMode.InstalledAppDBService) *AppCrudOperationServiceImpl { return &AppCrudOperationServiceImpl{ appLabelRepository: appLabelRepository, logger: logger, @@ -86,6 +88,7 @@ func NewAppCrudOperationServiceImpl(appLabelRepository pipelineConfig.AppLabelRe genericNoteService: genericNoteService, gitMaterialRepository: gitMaterialRepository, helmAppService: helmAppService, + installedAppDbService: installedAppDbService, } } @@ -431,20 +434,6 @@ func convertUrlToHttpsIfSshType(url string) string { return httpsURL } -// IsExternalAppLinkedToChartStore checks for an appId, if that app is linked to any chart-store app or not, -// if it's linked then it returns true along with clusterId and namespace for that linked installed_app -func (impl AppCrudOperationServiceImpl) IsExternalAppLinkedToChartStore(appId int) (bool, int, string, error) { - installedApp, err := impl.installedAppRepository.GetInstalledAppsByAppId(appId) - if err != nil && err != pg.ErrNoRows { - impl.logger.Errorw("IsExternalAppLinkedToChartStore, error in fetching installed app by app id for external apps", "appId", appId, "err", err) - return false, 0, "", err - } - if installedApp.Id > 0 { - return true, installedApp.Environment.ClusterId, installedApp.Environment.Namespace, nil - } - return false, 0, "", nil -} - // getAppAndProjectForAppIdentifier, returns app db model for an app unique identifier or from display_name if both exists else it throws pg.ErrNoRows func (impl AppCrudOperationServiceImpl) getAppAndProjectForAppIdentifier(appIdentifier *client.AppIdentifier) (*appRepository.App, error) { app := &appRepository.App{} @@ -468,13 +457,13 @@ func (impl AppCrudOperationServiceImpl) getAppAndProjectForAppIdentifier(appIden // updateAppNameToUniqueAppIdentifierInApp, migrates values of app_name col. in app table to unique identifier and also updates display_name with releaseName // returns is requested external app is migrated or other app (linked to chart store) with same name is migrated(which is tracked via namespace). -func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp(app *appRepository.App, appIdentifier *client.AppIdentifier, userId int32) (bool, error) { +func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp(app *appRepository.App, appIdentifier *client.AppIdentifier) (bool, error) { var isOtherExtAppMigrated bool appNameUniqueIdentifier := appIdentifier.GetUniqueAppNameIdentifier() //if isLinked is true then installed_app found for this app then this app name is already linked to an installed app then //update this app's app_name with unique identifier along with display_name. - isLinked, clusterId, namespace, err := impl.IsExternalAppLinkedToChartStore(app.Id) + isLinked, clusterId, namespace, err := impl.installedAppDbService.IsExternalAppLinkedToChartStore(app.Id) if err != nil { impl.logger.Errorw("error in checking IsExternalAppLinkedToChartStore", "appId", app.Id, "err", err) return false, err @@ -482,7 +471,7 @@ func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp( if isLinked { // if installed_app is already present for that display_name then migrate the app_name to unique identifier with installed_app ns and cluster id data // this namespace and clusterId belongs to the app linked to devtron whose entry in installed_apps is present. - app.AppName = appIdentifier.ReleaseName + "-" + namespace + "-" + strconv.Itoa(clusterId) + app.AppName = appIdentifier.GetUniqueAppIdentifierForGivenNamespaceAndCluster(namespace, strconv.Itoa(clusterId)) if namespace != appIdentifier.Namespace { //this means that migration request is for the same app whose appIdentifier is sent in request and what's present in linked installed_app isOtherExtAppMigrated = true @@ -493,7 +482,7 @@ func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp( } app.DisplayName = appIdentifier.ReleaseName - app.UpdatedBy = userId + app.UpdatedBy = bean2.SystemUserId app.UpdatedOn = time.Now() err = impl.appRepository.Update(app) if err != nil { @@ -503,7 +492,7 @@ func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp( return isOtherExtAppMigrated, nil } -func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId int32) (*bean.AppMetaInfoDto, error) { +func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean.AppMetaInfoDto, error) { // adding separate function for helm apps because for CLI helm apps, apps can be of form "1|clusterName|releaseName" // In this case app details can be fetched using app name / release Name. @@ -514,7 +503,7 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId impl.logger.Info("request payload, appId", appId) if len(appIdSplitted) > 1 { var isOtherExtAppMigrated bool - appIdDecoded, err := impl.helmAppService.DecodeAppId(appId) + appIdDecoded, err := client.DecodeExternalAppAppId(appId) if err != nil { impl.logger.Errorw("error in decoding app id for external app", "appId", appId, "err", err) return nil, err @@ -526,7 +515,7 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string, userId } // if app.DisplayName is empty then that app_name is not yet migrated to app name unique identifier if app.Id > 0 && len(app.DisplayName) == 0 { - isOtherExtAppMigrated, err = impl.updateAppNameToUniqueAppIdentifierInApp(app, appIdDecoded, userId) + isOtherExtAppMigrated, err = impl.updateAppNameToUniqueAppIdentifierInApp(app, appIdDecoded) if err != nil { impl.logger.Errorw("GetHelmAppMetaInfo, error in migrating displayName and appName to unique identifier for external apps", "appIdentifier", appIdDecoded, "err", err) //not returning from here as we need to show helm app metadata even if migration of app_name fails, then migration can happen on project update diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go index ed2cc6c1ee..2106ab8a80 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go @@ -9,11 +9,11 @@ import ( "github.com/devtron-labs/devtron/internal/sql/repository/helper" "github.com/devtron-labs/devtron/internal/sql/repository/pipelineConfig" "github.com/devtron-labs/devtron/internal/util" - app2 "github.com/devtron-labs/devtron/pkg/app" "github.com/devtron-labs/devtron/pkg/appStore/adapter" appStoreBean "github.com/devtron-labs/devtron/pkg/appStore/bean" discoverRepository "github.com/devtron-labs/devtron/pkg/appStore/discover/repository" "github.com/devtron-labs/devtron/pkg/appStore/installedApp/repository" + "github.com/devtron-labs/devtron/pkg/appStore/installedApp/service/EAMode" "github.com/devtron-labs/devtron/pkg/appStore/installedApp/service/FullMode/deployment" util4 "github.com/devtron-labs/devtron/pkg/appStore/util" "github.com/devtron-labs/devtron/pkg/bean" @@ -72,7 +72,7 @@ type AppStoreDeploymentDBServiceImpl struct { deploymentTypeOverrideService providerConfig.DeploymentTypeOverrideService fullModeDeploymentService deployment.FullModeDeploymentService appStoreValidator AppStoreValidator - appCrudOperationService app2.AppCrudOperationService + installedAppDbService EAMode.InstalledAppDBService } func NewAppStoreDeploymentDBServiceImpl(logger *zap.SugaredLogger, @@ -86,7 +86,7 @@ func NewAppStoreDeploymentDBServiceImpl(logger *zap.SugaredLogger, gitOpsConfigReadService config.GitOpsConfigReadService, deploymentTypeOverrideService providerConfig.DeploymentTypeOverrideService, fullModeDeploymentService deployment.FullModeDeploymentService, appStoreValidator AppStoreValidator, - appCrudOperationService app2.AppCrudOperationService) *AppStoreDeploymentDBServiceImpl { + installedAppDbService EAMode.InstalledAppDBService) *AppStoreDeploymentDBServiceImpl { return &AppStoreDeploymentDBServiceImpl{ logger: logger, installedAppRepository: installedAppRepository, @@ -100,7 +100,7 @@ func NewAppStoreDeploymentDBServiceImpl(logger *zap.SugaredLogger, deploymentTypeOverrideService: deploymentTypeOverrideService, fullModeDeploymentService: fullModeDeploymentService, appStoreValidator: appStoreValidator, - appCrudOperationService: appCrudOperationService, + installedAppDbService: installedAppDbService, } } @@ -334,8 +334,7 @@ func (impl *AppStoreDeploymentDBServiceImpl) GetActiveAppForAppIdentifierOrRelea if err != nil && err != pg.ErrNoRows { impl.logger.Errorw("error in fetching app meta data by unique app identifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) return nil, err - } - if util.IsErrNoRows(err) { + } else if util.IsErrNoRows(err) { //find app by displayName/releaseName if not found by unique identifier app, err = impl.appRepository.FindActiveByName(releaseName) if err != nil { @@ -364,7 +363,7 @@ func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, di - if ns matches, then update proj. req. is for same app present in installed_apps, in that case we will update the app_name with unique identifier and display_name along with team_id. */ - isLinkedToDevtron, _, linkedInstalledAppNamespace, err := impl.appCrudOperationService.IsExternalAppLinkedToChartStore(appModel.Id) + isLinkedToDevtron, _, linkedInstalledAppNamespace, err := impl.installedAppDbService.IsExternalAppLinkedToChartStore(appModel.Id) if err != nil { impl.logger.Errorw("error in checking IsExternalAppLinkedToChartStore", "appId", appModel.Id, "err", err) return err diff --git a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go index 93c174cb86..27d015fc56 100644 --- a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go +++ b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go @@ -54,6 +54,7 @@ type InstalledAppDBService interface { ChangeAppNameToDisplayNameForInstalledApp(installedApp *appStoreRepo.InstalledApps) GetReleaseInfo(appIdentifier *service.AppIdentifier) (*appStoreBean.InstallAppVersionDTO, error) + IsExternalAppLinkedToChartStore(appId int) (bool, int, string, error) } type InstalledAppDBServiceImpl struct { @@ -329,8 +330,7 @@ func (impl *InstalledAppDBServiceImpl) GetReleaseInfo(appIdentifier *service.App if err != nil && !util.IsErrNoRows(err) { impl.Logger.Errorw("GetReleaseInfo, error in getting installed app by clusterId, namespace and appUniqueIdentifierName", "appIdentifier", appIdentifier, "appUniqueIdentifierName", appName, "error", err) return nil, err - } - if util.IsErrNoRows(err) { + } else if util.IsErrNoRows(err) { // when app_name is not yet migrated to unique identifier installedAppVersionDto, err = impl.GetInstalledAppByClusterNamespaceAndName(appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.ReleaseName) if err != nil { @@ -345,3 +345,17 @@ func (impl *InstalledAppDBServiceImpl) GetReleaseInfo(appIdentifier *service.App } return installedAppVersionDto, nil } + +// IsExternalAppLinkedToChartStore checks for an appId, if that app is linked to any chart-store app or not, +// if it's linked then it returns true along with clusterId and namespace for that linked installed_app +func (impl *InstalledAppDBServiceImpl) IsExternalAppLinkedToChartStore(appId int) (bool, int, string, error) { + installedApp, err := impl.InstalledAppRepository.GetInstalledAppsByAppId(appId) + if err != nil && err != pg.ErrNoRows { + impl.Logger.Errorw("IsExternalAppLinkedToChartStore, error in fetching installed app by app id for external apps", "appId", appId, "err", err) + return false, 0, "", err + } + if installedApp.Id > 0 { + return true, installedApp.Environment.ClusterId, installedApp.Environment.Namespace, nil + } + return false, 0, "", nil +} diff --git a/wire_gen.go b/wire_gen.go index 045eede282..c5d9944eab 100644 --- a/wire_gen.go +++ b/wire_gen.go @@ -477,7 +477,8 @@ func InitializeApp() (*App, error) { ciTemplateServiceImpl := pipeline.NewCiTemplateServiceImpl(sugaredLogger, ciBuildConfigServiceImpl, ciTemplateRepositoryImpl, ciTemplateOverrideRepositoryImpl) appLabelRepositoryImpl := pipelineConfig.NewAppLabelRepositoryImpl(db) materialRepositoryImpl := pipelineConfig.NewMaterialRepositoryImpl(db) - appCrudOperationServiceImpl := app2.NewAppCrudOperationServiceImpl(appLabelRepositoryImpl, sugaredLogger, appRepositoryImpl, userRepositoryImpl, installedAppRepositoryImpl, genericNoteServiceImpl, materialRepositoryImpl, helmAppServiceImpl) + installedAppDBServiceImpl := EAMode.NewInstalledAppDBServiceImpl(sugaredLogger, installedAppRepositoryImpl, appRepositoryImpl, userServiceImpl, environmentServiceImpl, installedAppVersionHistoryRepositoryImpl) + appCrudOperationServiceImpl := app2.NewAppCrudOperationServiceImpl(appLabelRepositoryImpl, sugaredLogger, appRepositoryImpl, userRepositoryImpl, installedAppRepositoryImpl, genericNoteServiceImpl, materialRepositoryImpl, helmAppServiceImpl, installedAppDBServiceImpl) imageTagRepositoryImpl := repository2.NewImageTagRepository(db, sugaredLogger) customTagServiceImpl := pipeline.NewCustomTagService(sugaredLogger, imageTagRepositoryImpl) pluginInputVariableParserImpl := pipeline.NewPluginInputVariableParserImpl(sugaredLogger, dockerRegistryConfigImpl, customTagServiceImpl) @@ -563,7 +564,6 @@ func InitializeApp() (*App, error) { devtronAppConfigServiceImpl := pipeline.NewDevtronAppConfigServiceImpl(sugaredLogger, ciCdPipelineOrchestratorImpl, appRepositoryImpl, pipelineRepositoryImpl, resourceGroupServiceImpl, enforcerUtilImpl, ciMaterialConfigServiceImpl) pipelineBuilderImpl := pipeline.NewPipelineBuilderImpl(sugaredLogger, materialRepositoryImpl, chartRepositoryImpl, ciPipelineConfigServiceImpl, ciMaterialConfigServiceImpl, appArtifactManagerImpl, devtronAppCMCSServiceImpl, devtronAppStrategyServiceImpl, appDeploymentTypeChangeManagerImpl, cdPipelineConfigServiceImpl, devtronAppConfigServiceImpl) deploymentTemplateValidationServiceImpl := deploymentTemplate.NewDeploymentTemplateValidationServiceImpl(sugaredLogger, chartRefServiceImpl, scopedVariableManagerImpl) - installedAppDBServiceImpl := EAMode.NewInstalledAppDBServiceImpl(sugaredLogger, installedAppRepositoryImpl, appRepositoryImpl, userServiceImpl, environmentServiceImpl, installedAppVersionHistoryRepositoryImpl) installedAppDBExtendedServiceImpl := FullMode.NewInstalledAppDBExtendedServiceImpl(installedAppDBServiceImpl, appStatusServiceImpl, gitOpsConfigReadServiceImpl) gitOpsValidationServiceImpl := validation.NewGitOpsValidationServiceImpl(sugaredLogger, gitFactory, gitOperationServiceImpl, gitOpsConfigReadServiceImpl, chartTemplateServiceImpl, chartServiceImpl, installedAppDBExtendedServiceImpl) devtronAppGitOpConfigServiceImpl := gitOpsConfig.NewDevtronAppGitOpConfigServiceImpl(sugaredLogger, chartRepositoryImpl, chartServiceImpl, gitOpsConfigReadServiceImpl, gitOpsValidationServiceImpl, argoClientWrapperServiceImpl) @@ -657,7 +657,7 @@ func InitializeApp() (*App, error) { appStoreDeploymentCommonServiceImpl := appStoreDeploymentCommon.NewAppStoreDeploymentCommonServiceImpl(sugaredLogger, appStoreApplicationVersionRepositoryImpl, chartTemplateServiceImpl) fullModeDeploymentServiceImpl := deployment.NewFullModeDeploymentServiceImpl(sugaredLogger, applicationServiceClientImpl, argoK8sClientImpl, acdAuthConfig, chartGroupDeploymentRepositoryImpl, installedAppRepositoryImpl, installedAppVersionHistoryRepositoryImpl, argoUserServiceImpl, appStoreDeploymentCommonServiceImpl, helmAppServiceImpl, appStatusServiceImpl, pipelineStatusTimelineServiceImpl, userServiceImpl, pipelineStatusTimelineRepositoryImpl, appStoreApplicationVersionRepositoryImpl, argoClientWrapperServiceImpl, acdConfig, gitOperationServiceImpl, gitOpsConfigReadServiceImpl, gitOpsValidationServiceImpl, environmentRepositoryImpl) appStoreValidatorImpl := service3.NewAppAppStoreValidatorImpl(sugaredLogger) - appStoreDeploymentDBServiceImpl := service3.NewAppStoreDeploymentDBServiceImpl(sugaredLogger, installedAppRepositoryImpl, appStoreApplicationVersionRepositoryImpl, appRepositoryImpl, environmentServiceImpl, clusterServiceImplExtended, installedAppVersionHistoryRepositoryImpl, environmentVariables, gitOpsConfigReadServiceImpl, deploymentTypeOverrideServiceImpl, fullModeDeploymentServiceImpl, appStoreValidatorImpl, appCrudOperationServiceImpl) + appStoreDeploymentDBServiceImpl := service3.NewAppStoreDeploymentDBServiceImpl(sugaredLogger, installedAppRepositoryImpl, appStoreApplicationVersionRepositoryImpl, appRepositoryImpl, environmentServiceImpl, clusterServiceImplExtended, installedAppVersionHistoryRepositoryImpl, environmentVariables, gitOpsConfigReadServiceImpl, deploymentTypeOverrideServiceImpl, fullModeDeploymentServiceImpl, appStoreValidatorImpl, installedAppDBServiceImpl) eaModeDeploymentServiceImpl := EAMode.NewEAModeDeploymentServiceImpl(sugaredLogger, helmAppServiceImpl, appStoreApplicationVersionRepositoryImpl, helmAppClientImpl, installedAppRepositoryImpl, ociRegistryConfigRepositoryImpl) deletePostProcessorImpl := service3.NewDeletePostProcessorImpl(sugaredLogger) appStoreDeploymentServiceImpl := service3.NewAppStoreDeploymentServiceImpl(sugaredLogger, installedAppRepositoryImpl, installedAppDBServiceImpl, appStoreDeploymentDBServiceImpl, chartGroupDeploymentRepositoryImpl, appStoreApplicationVersionRepositoryImpl, appRepositoryImpl, eaModeDeploymentServiceImpl, fullModeDeploymentServiceImpl, environmentServiceImpl, helmAppServiceImpl, installedAppVersionHistoryRepositoryImpl, environmentVariables, acdConfig, gitOpsConfigReadServiceImpl, deletePostProcessorImpl, appStoreValidatorImpl) From bbd9fdeed35dc47186eb97ec9c313d6e6f636fa3 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Thu, 2 May 2024 14:19:15 +0530 Subject: [PATCH 28/35] wire fix --- cmd/external-app/wire_gen.go | 2 +- pkg/app/AppCrudOperationService.go | 3 --- wire_gen.go | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/cmd/external-app/wire_gen.go b/cmd/external-app/wire_gen.go index e2c258a341..da02c19572 100644 --- a/cmd/external-app/wire_gen.go +++ b/cmd/external-app/wire_gen.go @@ -373,7 +373,7 @@ func InitializeApp() (*App, error) { attributesRouterImpl := router.NewAttributesRouterImpl(attributesRestHandlerImpl) appLabelRepositoryImpl := pipelineConfig.NewAppLabelRepositoryImpl(db) materialRepositoryImpl := pipelineConfig.NewMaterialRepositoryImpl(db) - appCrudOperationServiceImpl := app2.NewAppCrudOperationServiceImpl(appLabelRepositoryImpl, sugaredLogger, appRepositoryImpl, userRepositoryImpl, installedAppRepositoryImpl, genericNoteServiceImpl, materialRepositoryImpl, helmAppServiceImpl, installedAppDBServiceImpl) + appCrudOperationServiceImpl := app2.NewAppCrudOperationServiceImpl(appLabelRepositoryImpl, sugaredLogger, appRepositoryImpl, userRepositoryImpl, installedAppRepositoryImpl, genericNoteServiceImpl, materialRepositoryImpl, installedAppDBServiceImpl) appInfoRestHandlerImpl := appInfo.NewAppInfoRestHandlerImpl(sugaredLogger, appCrudOperationServiceImpl, userServiceImpl, validate, enforcerUtilImpl, enforcerImpl, helmAppServiceImpl, enforcerUtilHelmImpl, genericNoteServiceImpl) appInfoRouterImpl := appInfo2.NewAppInfoRouterImpl(sugaredLogger, appInfoRestHandlerImpl) appFilteringRestHandlerImpl := appList.NewAppFilteringRestHandlerImpl(sugaredLogger, teamServiceImpl, enforcerImpl, userServiceImpl, clusterServiceImpl, environmentServiceImpl) diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index 92d2b03ebe..3434f01165 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -68,7 +68,6 @@ type AppCrudOperationServiceImpl struct { installedAppRepository repository2.InstalledAppRepository genericNoteService genericNotes.GenericNoteService gitMaterialRepository pipelineConfig.MaterialRepository - helmAppService client.HelmAppService installedAppDbService EAMode.InstalledAppDBService } @@ -77,7 +76,6 @@ func NewAppCrudOperationServiceImpl(appLabelRepository pipelineConfig.AppLabelRe installedAppRepository repository2.InstalledAppRepository, genericNoteService genericNotes.GenericNoteService, gitMaterialRepository pipelineConfig.MaterialRepository, - helmAppService client.HelmAppService, installedAppDbService EAMode.InstalledAppDBService) *AppCrudOperationServiceImpl { return &AppCrudOperationServiceImpl{ appLabelRepository: appLabelRepository, @@ -87,7 +85,6 @@ func NewAppCrudOperationServiceImpl(appLabelRepository pipelineConfig.AppLabelRe installedAppRepository: installedAppRepository, genericNoteService: genericNoteService, gitMaterialRepository: gitMaterialRepository, - helmAppService: helmAppService, installedAppDbService: installedAppDbService, } } diff --git a/wire_gen.go b/wire_gen.go index c5d9944eab..6bc4740ab8 100644 --- a/wire_gen.go +++ b/wire_gen.go @@ -478,7 +478,7 @@ func InitializeApp() (*App, error) { appLabelRepositoryImpl := pipelineConfig.NewAppLabelRepositoryImpl(db) materialRepositoryImpl := pipelineConfig.NewMaterialRepositoryImpl(db) installedAppDBServiceImpl := EAMode.NewInstalledAppDBServiceImpl(sugaredLogger, installedAppRepositoryImpl, appRepositoryImpl, userServiceImpl, environmentServiceImpl, installedAppVersionHistoryRepositoryImpl) - appCrudOperationServiceImpl := app2.NewAppCrudOperationServiceImpl(appLabelRepositoryImpl, sugaredLogger, appRepositoryImpl, userRepositoryImpl, installedAppRepositoryImpl, genericNoteServiceImpl, materialRepositoryImpl, helmAppServiceImpl, installedAppDBServiceImpl) + appCrudOperationServiceImpl := app2.NewAppCrudOperationServiceImpl(appLabelRepositoryImpl, sugaredLogger, appRepositoryImpl, userRepositoryImpl, installedAppRepositoryImpl, genericNoteServiceImpl, materialRepositoryImpl, installedAppDBServiceImpl) imageTagRepositoryImpl := repository2.NewImageTagRepository(db, sugaredLogger) customTagServiceImpl := pipeline.NewCustomTagService(sugaredLogger, imageTagRepositoryImpl) pluginInputVariableParserImpl := pipeline.NewPluginInputVariableParserImpl(sugaredLogger, dockerRegistryConfigImpl, customTagServiceImpl) From c10aa810e8fa2817c58a75be426a7ad0834cf0c3 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Thu, 16 May 2024 14:54:43 +0530 Subject: [PATCH 29/35] fixes for multiple installed apps with same app pointer linked to chart store --- internal/sql/repository/app/AppRepository.go | 6 ++ pkg/app/AppCrudOperationService.go | 42 +++++-------- .../repository/InstalledAppRepository.go | 27 +++++++++ .../service/AppStoreDeploymentDBService.go | 25 ++++---- .../service/AppStoreDeploymentService.go | 4 +- .../service/EAMode/InstalledAppDBService.go | 59 ++++++++++++++++--- 6 files changed, 111 insertions(+), 52 deletions(-) diff --git a/internal/sql/repository/app/AppRepository.go b/internal/sql/repository/app/AppRepository.go index 73ca2a541b..67173aa22b 100644 --- a/internal/sql/repository/app/AppRepository.go +++ b/internal/sql/repository/app/AppRepository.go @@ -47,6 +47,7 @@ func (r *App) IsAppJobOrExternalType() bool { type AppRepository interface { SaveWithTxn(pipelineGroup *App, tx *pg.Tx) error + SaveInBulkWithTxn(apps []*App, tx *pg.Tx) error Update(app *App) error UpdateWithTxn(app *App, tx *pg.Tx) error SetDescription(id int, description string, userId int32) error @@ -479,3 +480,8 @@ func (repo AppRepositoryImpl) UpdateAppOfferingModeForAppIds(successAppIds []*in _, err := repo.dbConnection.Query(app, query, appOfferingMode, userId, time.Now(), pg.In(successAppIds)) return err } + +func (repo AppRepositoryImpl) SaveInBulkWithTxn(apps []*App, tx *pg.Tx) error { + err := tx.Insert(&apps) + return err +} diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index 3434f01165..01f795847f 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -454,39 +454,36 @@ func (impl AppCrudOperationServiceImpl) getAppAndProjectForAppIdentifier(appIden // updateAppNameToUniqueAppIdentifierInApp, migrates values of app_name col. in app table to unique identifier and also updates display_name with releaseName // returns is requested external app is migrated or other app (linked to chart store) with same name is migrated(which is tracked via namespace). -func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp(app *appRepository.App, appIdentifier *client.AppIdentifier) (bool, error) { - var isOtherExtAppMigrated bool +func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp(app *appRepository.App, appIdentifier *client.AppIdentifier) error { appNameUniqueIdentifier := appIdentifier.GetUniqueAppNameIdentifier() - //if isLinked is true then installed_app found for this app then this app name is already linked to an installed app then - //update this app's app_name with unique identifier along with display_name. - isLinked, clusterId, namespace, err := impl.installedAppDbService.IsExternalAppLinkedToChartStore(app.Id) + isLinked, installedApps, err := impl.installedAppDbService.IsExternalAppLinkedToChartStore(app.Id) if err != nil { impl.logger.Errorw("error in checking IsExternalAppLinkedToChartStore", "appId", app.Id, "err", err) - return false, err + return err } + //if isLinked is true then installed_app found for this app then this app name is already linked to an installed app then + //create new appEntry for all those installedApps and link installedApp.AppId to the newly created app. if isLinked { - // if installed_app is already present for that display_name then migrate the app_name to unique identifier with installed_app ns and cluster id data - // this namespace and clusterId belongs to the app linked to devtron whose entry in installed_apps is present. - app.AppName = appIdentifier.GetUniqueAppIdentifierForGivenNamespaceAndCluster(namespace, strconv.Itoa(clusterId)) - if namespace != appIdentifier.Namespace { - //this means that migration request is for the same app whose appIdentifier is sent in request and what's present in linked installed_app - isOtherExtAppMigrated = true + // if installed_apps are already present for that display_name then migrate the app_name to unique identifier with installedApp's ns and clusterId. + // creating new entry for app all installedApps with uniqueAppNameIdentifier and display name + err := impl.installedAppDbService.CreateNewAppEntryForAllInstalledApps(installedApps) + if err != nil { + impl.logger.Errorw("error in CreateNewAppEntryForAllInstalledApps", "appName", app.AppName, "err", err) + //not returning from here as we have to migrate the app for requested ext-app and return the response for meta info } - } else { - //app not found with unique identifier but displayName, hence migrate the app_name to unique identifier and also update display_name - app.AppName = appNameUniqueIdentifier } - + // migrating the requested ext-app + app.AppName = appNameUniqueIdentifier app.DisplayName = appIdentifier.ReleaseName app.UpdatedBy = bean2.SystemUserId app.UpdatedOn = time.Now() err = impl.appRepository.Update(app) if err != nil { impl.logger.Errorw("error in migrating displayName and appName to unique identifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) - return false, err + return err } - return isOtherExtAppMigrated, nil + return nil } func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean.AppMetaInfoDto, error) { @@ -499,7 +496,6 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. var displayName string impl.logger.Info("request payload, appId", appId) if len(appIdSplitted) > 1 { - var isOtherExtAppMigrated bool appIdDecoded, err := client.DecodeExternalAppAppId(appId) if err != nil { impl.logger.Errorw("error in decoding app id for external app", "appId", appId, "err", err) @@ -512,17 +508,11 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. } // if app.DisplayName is empty then that app_name is not yet migrated to app name unique identifier if app.Id > 0 && len(app.DisplayName) == 0 { - isOtherExtAppMigrated, err = impl.updateAppNameToUniqueAppIdentifierInApp(app, appIdDecoded) + err = impl.updateAppNameToUniqueAppIdentifierInApp(app, appIdDecoded) if err != nil { impl.logger.Errorw("GetHelmAppMetaInfo, error in migrating displayName and appName to unique identifier for external apps", "appIdentifier", appIdDecoded, "err", err) //not returning from here as we need to show helm app metadata even if migration of app_name fails, then migration can happen on project update } - // we have migrated for other app with same name linked to installed app not the one coming from request, in that case - // requested app in not assigned to any project. - if isOtherExtAppMigrated { - app.TeamId = 0 - app.Team.Name = "" - } } if app.Id == 0 { app.AppName = appIdDecoded.ReleaseName diff --git a/pkg/appStore/installedApp/repository/InstalledAppRepository.go b/pkg/appStore/installedApp/repository/InstalledAppRepository.go index 087f118f7b..6cd3ad60fd 100644 --- a/pkg/appStore/installedApp/repository/InstalledAppRepository.go +++ b/pkg/appStore/installedApp/repository/InstalledAppRepository.go @@ -18,12 +18,14 @@ package repository import ( + "fmt" "github.com/devtron-labs/common-lib/utils/k8s/health" "github.com/devtron-labs/devtron/internal/sql/repository/app" "github.com/devtron-labs/devtron/internal/sql/repository/pipelineConfig" util2 "github.com/devtron-labs/devtron/internal/util" appStoreBean "github.com/devtron-labs/devtron/pkg/appStore/bean" appStoreDiscoverRepository "github.com/devtron-labs/devtron/pkg/appStore/discover/repository" + util3 "github.com/devtron-labs/devtron/pkg/appStore/util" "github.com/devtron-labs/devtron/pkg/cluster/repository" "github.com/devtron-labs/devtron/pkg/sql" "github.com/devtron-labs/devtron/util" @@ -79,6 +81,14 @@ func (model *InstalledApps) ChangeAppNameToDisplayName() { model.App.AppName = model.App.DisplayName } +func (model *InstalledApps) GetUniqueAppNameIdentifier() string { + if util3.IsExternalChartStoreApp(model.App.DisplayName) { + //if display name is set then that installedApp's app is already migrated + return model.App.AppName + } + return fmt.Sprintf("%s-%s-%s", model.App.AppName, model.Environment.Namespace, strconv.Itoa(model.Environment.ClusterId)) +} + type InstalledAppVersions struct { TableName struct{} `sql:"installed_app_versions" pg:",discard_unknown_columns"` Id int `sql:"id,pk"` @@ -147,6 +157,8 @@ type InstalledAppRepository interface { GetActiveInstalledAppByEnvIdAndDeploymentType(envId int, deploymentType string, excludeAppIds []string, includeAppIds []string) ([]*InstalledApps, error) UpdateDeploymentAppTypeInInstalledApp(deploymentAppType string, installedAppIdIncludes []int, userId int32, deployStatus int) error FindInstalledAppByIds(ids []int) ([]*InstalledApps, error) + // FindInstalledAppsByAppId returns multiple installed apps for an appId, this only happens for external-apps with same name installed in diff namespaces + FindInstalledAppsByAppId(appId int) ([]*InstalledApps, error) } type InstalledAppRepositoryImpl struct { @@ -895,3 +907,18 @@ func (impl InstalledAppRepositoryImpl) FindInstalledAppByIds(ids []int) ([]*Inst } return installedApps, err } + +func (impl InstalledAppRepositoryImpl) FindInstalledAppsByAppId(appId int) ([]*InstalledApps, error) { + var installedApps []*InstalledApps + err := impl.dbConnection.Model(&installedApps). + Column("installed_apps.*", "App", "Environment"). + Join("inner join app a on installed_apps.app_id = a.id"). + Join("inner join environment e on installed_apps.environment_id = e.id"). + Where("installed_apps.app_id = ?", appId). + Where("installed_apps.active = true"). + Select() + if err != nil { + impl.Logger.Errorw("error on fetching installed apps by appId", "appId", appId) + } + return installedApps, err +} diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go index 2106ab8a80..df0b6ac026 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go @@ -44,7 +44,7 @@ type AppStoreDeploymentDBService interface { // UpdateInstalledAppVersionHistoryWithGitHash updates GitHash in the repository.InstalledAppVersionHistory UpdateInstalledAppVersionHistoryWithGitHash(versionHistoryId int, gitHash string, userId int32) error // UpdateProjectForHelmApp updates TeamId in the app.App - UpdateProjectForHelmApp(appName, displayName, namespace string, teamId int, userId int32) error + UpdateProjectForHelmApp(appName, displayName string, teamId int, userId int32) error // InstallAppPostDbOperation is used to perform Post-Install DB operations in App Store deployments InstallAppPostDbOperation(installAppVersionRequest *appStoreBean.InstallAppVersionDTO) error // MarkInstalledAppVersionsInactiveByInstalledAppId will mark the repository.InstalledAppVersions inactive for the given InstalledAppId @@ -345,7 +345,7 @@ func (impl *AppStoreDeploymentDBServiceImpl) GetActiveAppForAppIdentifierOrRelea return app, nil } -func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, displayName, namespace string, teamId int, userId int32) error { +func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, displayName string, teamId int, userId int32) error { appModel, err := impl.GetActiveAppForAppIdentifierOrReleaseName(appName, displayName) if err != nil && !util.IsErrNoRows(err) { impl.logger.Errorw("error in fetching appModel by appName", "appName", appName, "err", err) @@ -355,23 +355,20 @@ func (impl *AppStoreDeploymentDBServiceImpl) UpdateProjectForHelmApp(appName, di // only external app will have a display name, so checking the following case only for external apps if appModel != nil && appModel.Id > 0 && len(displayName) > 0 { /* - 1. now we will check if for that appModel, installed_app entry present or not i.e. linked to devtron or not, - 2. if not, then let the normal flow continue as we can change the app_name with app unique identifier. - 3. if exists then we will check if request's namespace is same as what present in installed_apps. - - if ns doesn't match then update proj. req. is for some other app found in app table via display_name, - in this case create a new entry in app table for the request. - - if ns matches, then update proj. req. is for same app present in installed_apps, in that case we will update - the app_name with unique identifier and display_name along with team_id. + 1. now we will check if for that appModel, installed_app entries are present or not i.e. linked to devtron or not, + 2. if not, then let the normal flow continue as we can change the app_name with app unique identifier. + 3. if exists then we will create new app entries with uniqueAppNameIdentifier for all installed apps. */ - isLinkedToDevtron, _, linkedInstalledAppNamespace, err := impl.installedAppDbService.IsExternalAppLinkedToChartStore(appModel.Id) + isLinkedToDevtron, installedApps, err := impl.installedAppDbService.IsExternalAppLinkedToChartStore(appModel.Id) if err != nil { - impl.logger.Errorw("error in checking IsExternalAppLinkedToChartStore", "appId", appModel.Id, "err", err) + impl.logger.Errorw("UpdateProjectForHelmApp, error in checking IsExternalAppLinkedToChartStore", "appId", appModel.Id, "err", err) return err } if isLinkedToDevtron { - if namespace != linkedInstalledAppNamespace { - //assigning appModel as nil, so that it will create a new entry in app for the request - appModel = nil + err := impl.installedAppDbService.CreateNewAppEntryForAllInstalledApps(installedApps) + if err != nil { + impl.logger.Errorw("UpdateProjectForHelmApp, error in CreateNewAppEntryForAllInstalledApps", "appName", displayName, "err", err) + //not returning from here, project update req is yet to be processed for requested ext-app } } } diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go index d764686d10..d08d03f654 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentService.go @@ -396,7 +396,6 @@ func isExternalHelmApp(appId string) bool { func (impl *AppStoreDeploymentServiceImpl) UpdateProjectHelmApp(updateAppRequest *appStoreBean.UpdateProjectHelmAppDTO) error { var appName string var displayName string - var namespace string appName = updateAppRequest.AppName if isExternalHelmApp(updateAppRequest.AppId) { appIdentifier, err := impl.helmAppService.DecodeAppId(updateAppRequest.AppId) @@ -406,7 +405,6 @@ func (impl *AppStoreDeploymentServiceImpl) UpdateProjectHelmApp(updateAppRequest } appName = appIdentifier.GetUniqueAppNameIdentifier() displayName = updateAppRequest.AppName - namespace = appIdentifier.Namespace } else { //in case the external app is linked, then it's unique identifier is set in app_name col. hence retrieving appName //for this case, although this will also handle the case for non-external apps @@ -416,7 +414,7 @@ func (impl *AppStoreDeploymentServiceImpl) UpdateProjectHelmApp(updateAppRequest } } impl.logger.Infow("update helm project request", updateAppRequest) - err := impl.appStoreDeploymentDBService.UpdateProjectForHelmApp(appName, displayName, namespace, updateAppRequest.TeamId, updateAppRequest.UserId) + err := impl.appStoreDeploymentDBService.UpdateProjectForHelmApp(appName, displayName, updateAppRequest.TeamId, updateAppRequest.UserId) if err != nil { impl.logger.Errorw("error in linking project to helm app", "appName", updateAppRequest.AppName, "err", err) return err diff --git a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go index 27d015fc56..d1746dbd06 100644 --- a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go +++ b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go @@ -19,7 +19,9 @@ package EAMode import ( "github.com/devtron-labs/devtron/api/helm-app/service" + "github.com/devtron-labs/devtron/internal/sql/repository/helper" util4 "github.com/devtron-labs/devtron/pkg/appStore/util" + bean3 "github.com/devtron-labs/devtron/pkg/auth/user/bean" "github.com/devtron-labs/devtron/pkg/cluster" "net/http" "strconv" @@ -54,7 +56,8 @@ type InstalledAppDBService interface { ChangeAppNameToDisplayNameForInstalledApp(installedApp *appStoreRepo.InstalledApps) GetReleaseInfo(appIdentifier *service.AppIdentifier) (*appStoreBean.InstallAppVersionDTO, error) - IsExternalAppLinkedToChartStore(appId int) (bool, int, string, error) + IsExternalAppLinkedToChartStore(appId int) (bool, []*appStoreRepo.InstalledApps, error) + CreateNewAppEntryForAllInstalledApps(installedApps []*appStoreRepo.InstalledApps) error } type InstalledAppDBServiceImpl struct { @@ -347,15 +350,53 @@ func (impl *InstalledAppDBServiceImpl) GetReleaseInfo(appIdentifier *service.App } // IsExternalAppLinkedToChartStore checks for an appId, if that app is linked to any chart-store app or not, -// if it's linked then it returns true along with clusterId and namespace for that linked installed_app -func (impl *InstalledAppDBServiceImpl) IsExternalAppLinkedToChartStore(appId int) (bool, int, string, error) { - installedApp, err := impl.InstalledAppRepository.GetInstalledAppsByAppId(appId) +// if it's linked then it returns true along with all the installedApps linked to that appId +func (impl *InstalledAppDBServiceImpl) IsExternalAppLinkedToChartStore(appId int) (bool, []*appStoreRepo.InstalledApps, error) { + installedApps, err := impl.InstalledAppRepository.FindInstalledAppsByAppId(appId) if err != nil && err != pg.ErrNoRows { - impl.Logger.Errorw("IsExternalAppLinkedToChartStore, error in fetching installed app by app id for external apps", "appId", appId, "err", err) - return false, 0, "", err + impl.Logger.Errorw("IsExternalAppLinkedToChartStore, error in fetching installed apps by app id for external apps", "appId", appId, "err", err) + return false, nil, err } - if installedApp.Id > 0 { - return true, installedApp.Environment.ClusterId, installedApp.Environment.Namespace, nil + if installedApps != nil && len(installedApps) > 0 { + return true, installedApps, nil + } + return false, nil, nil +} + +func (impl *InstalledAppDBServiceImpl) CreateNewAppEntryForAllInstalledApps(installedApps []*appStoreRepo.InstalledApps) error { + // db operations + dbConnection := impl.InstalledAppRepository.GetConnection() + tx, err := dbConnection.Begin() + if err != nil { + return err + } + // Rollback tx on error. + defer tx.Rollback() + for _, installedApp := range installedApps { + appModel := &app.App{ + Active: true, + AppName: installedApp.GetUniqueAppNameIdentifier(), + TeamId: installedApp.App.TeamId, + AppType: helper.ChartStoreApp, + AppOfferingMode: installedApp.App.AppOfferingMode, + DisplayName: installedApp.App.AppName, + } + appModel.CreateAuditLog(bean3.SystemUserId) + err := impl.AppRepository.SaveWithTxn(appModel, tx) + if err != nil { + impl.Logger.Errorw("error saving appModel", "err", err) + return err + } + //updating the installedApp.AppId with new app entry + installedApp.AppId = appModel.Id + installedApp.UpdateAuditLog(bean3.SystemUserId) + _, err = impl.InstalledAppRepository.UpdateInstalledApp(installedApp, tx) + if err != nil { + impl.Logger.Errorw("error saving updating installed app with new appId", "installedAppId", installedApp.Id, "err", err) + return err + } } - return false, 0, "", nil + + tx.Commit() + return nil } From 973c05d44144253d3d55bbc82910dd7617c17ce9 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Thu, 16 May 2024 15:08:14 +0530 Subject: [PATCH 30/35] remove unused code --- internal/sql/repository/app/AppRepository.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/internal/sql/repository/app/AppRepository.go b/internal/sql/repository/app/AppRepository.go index 67173aa22b..73ca2a541b 100644 --- a/internal/sql/repository/app/AppRepository.go +++ b/internal/sql/repository/app/AppRepository.go @@ -47,7 +47,6 @@ func (r *App) IsAppJobOrExternalType() bool { type AppRepository interface { SaveWithTxn(pipelineGroup *App, tx *pg.Tx) error - SaveInBulkWithTxn(apps []*App, tx *pg.Tx) error Update(app *App) error UpdateWithTxn(app *App, tx *pg.Tx) error SetDescription(id int, description string, userId int32) error @@ -480,8 +479,3 @@ func (repo AppRepositoryImpl) UpdateAppOfferingModeForAppIds(successAppIds []*in _, err := repo.dbConnection.Query(app, query, appOfferingMode, userId, time.Now(), pg.In(successAppIds)) return err } - -func (repo AppRepositoryImpl) SaveInBulkWithTxn(apps []*App, tx *pg.Tx) error { - err := tx.Insert(&apps) - return err -} From 4aa2febb5fa8219d02df3e47ee946ecad51a6f4d Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Sun, 19 May 2024 15:53:33 +0530 Subject: [PATCH 31/35] fix for showing unassigned proj to ext app that's already linked to devtron with same name --- pkg/app/AppCrudOperationService.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index 87a15317c2..4c1cbd9b0b 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -20,12 +20,12 @@ package app import ( "encoding/json" "fmt" - bean3 "github.com/devtron-labs/devtron/pkg/deployment/manifest/bean" client "github.com/devtron-labs/devtron/api/helm-app/service" "github.com/devtron-labs/devtron/internal/util" "github.com/devtron-labs/devtron/pkg/appStore/installedApp/service/EAMode" util2 "github.com/devtron-labs/devtron/pkg/appStore/util" bean2 "github.com/devtron-labs/devtron/pkg/auth/user/bean" + bean3 "github.com/devtron-labs/devtron/pkg/deployment/manifest/bean" "regexp" "strconv" "strings" @@ -455,13 +455,13 @@ func (impl AppCrudOperationServiceImpl) getAppAndProjectForAppIdentifier(appIden // updateAppNameToUniqueAppIdentifierInApp, migrates values of app_name col. in app table to unique identifier and also updates display_name with releaseName // returns is requested external app is migrated or other app (linked to chart store) with same name is migrated(which is tracked via namespace). -func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp(app *appRepository.App, appIdentifier *client.AppIdentifier) error { +func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp(app *appRepository.App, appIdentifier *client.AppIdentifier) (bool, error) { appNameUniqueIdentifier := appIdentifier.GetUniqueAppNameIdentifier() - + var isOtherExtAppMigrated bool isLinked, installedApps, err := impl.installedAppDbService.IsExternalAppLinkedToChartStore(app.Id) if err != nil { impl.logger.Errorw("error in checking IsExternalAppLinkedToChartStore", "appId", app.Id, "err", err) - return err + return isOtherExtAppMigrated, err } //if isLinked is true then installed_app found for this app then this app name is already linked to an installed app then //create new appEntry for all those installedApps and link installedApp.AppId to the newly created app. @@ -473,6 +473,7 @@ func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp( impl.logger.Errorw("error in CreateNewAppEntryForAllInstalledApps", "appName", app.AppName, "err", err) //not returning from here as we have to migrate the app for requested ext-app and return the response for meta info } + isOtherExtAppMigrated = true } // migrating the requested ext-app app.AppName = appNameUniqueIdentifier @@ -482,9 +483,9 @@ func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp( err = impl.appRepository.Update(app) if err != nil { impl.logger.Errorw("error in migrating displayName and appName to unique identifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) - return err + return isOtherExtAppMigrated, err } - return nil + return isOtherExtAppMigrated, nil } func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean.AppMetaInfoDto, error) { @@ -497,6 +498,7 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. var displayName string impl.logger.Info("request payload, appId", appId) if len(appIdSplitted) > 1 { + var isOtherExtAppMigrated bool appIdDecoded, err := client.DecodeExternalAppAppId(appId) if err != nil { impl.logger.Errorw("error in decoding app id for external app", "appId", appId, "err", err) @@ -509,12 +511,18 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. } // if app.DisplayName is empty then that app_name is not yet migrated to app name unique identifier if app.Id > 0 && len(app.DisplayName) == 0 { - err = impl.updateAppNameToUniqueAppIdentifierInApp(app, appIdDecoded) + isOtherExtAppMigrated, err = impl.updateAppNameToUniqueAppIdentifierInApp(app, appIdDecoded) if err != nil { impl.logger.Errorw("GetHelmAppMetaInfo, error in migrating displayName and appName to unique identifier for external apps", "appIdentifier", appIdDecoded, "err", err) //not returning from here as we need to show helm app metadata even if migration of app_name fails, then migration can happen on project update } } + // we have migrated for other app with same name linked to installed app not the one coming from request, in that case + // requested app in not assigned to any project. + if isOtherExtAppMigrated { + app.TeamId = 0 + app.Team.Name = "" + } if app.Id == 0 { app.AppName = appIdDecoded.ReleaseName } From 947a29df256743db31fe2624f78e4dca62808df4 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Sun, 19 May 2024 16:56:16 +0530 Subject: [PATCH 32/35] fix --- pkg/app/AppCrudOperationService.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index 4c1cbd9b0b..7f40945ba1 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -478,6 +478,7 @@ func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp( // migrating the requested ext-app app.AppName = appNameUniqueIdentifier app.DisplayName = appIdentifier.ReleaseName + app.TeamId = 0 app.UpdatedBy = bean2.SystemUserId app.UpdatedOn = time.Now() err = impl.appRepository.Update(app) @@ -520,7 +521,6 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. // we have migrated for other app with same name linked to installed app not the one coming from request, in that case // requested app in not assigned to any project. if isOtherExtAppMigrated { - app.TeamId = 0 app.Team.Name = "" } if app.Id == 0 { From e8c0f8ddd27f17f5590f8aa1a23154d431e77f27 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Sun, 19 May 2024 17:23:49 +0530 Subject: [PATCH 33/35] fix --- pkg/app/AppCrudOperationService.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index 7f40945ba1..8781a33f1e 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -478,7 +478,6 @@ func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp( // migrating the requested ext-app app.AppName = appNameUniqueIdentifier app.DisplayName = appIdentifier.ReleaseName - app.TeamId = 0 app.UpdatedBy = bean2.SystemUserId app.UpdatedOn = time.Now() err = impl.appRepository.Update(app) From be13836b0c63a3480149fcaa7cd081c7978a7b7b Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Mon, 20 May 2024 10:55:32 +0530 Subject: [PATCH 34/35] revert --- pkg/app/AppCrudOperationService.go | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index 8781a33f1e..813f736458 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -455,13 +455,12 @@ func (impl AppCrudOperationServiceImpl) getAppAndProjectForAppIdentifier(appIden // updateAppNameToUniqueAppIdentifierInApp, migrates values of app_name col. in app table to unique identifier and also updates display_name with releaseName // returns is requested external app is migrated or other app (linked to chart store) with same name is migrated(which is tracked via namespace). -func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp(app *appRepository.App, appIdentifier *client.AppIdentifier) (bool, error) { +func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp(app *appRepository.App, appIdentifier *client.AppIdentifier) error { appNameUniqueIdentifier := appIdentifier.GetUniqueAppNameIdentifier() - var isOtherExtAppMigrated bool isLinked, installedApps, err := impl.installedAppDbService.IsExternalAppLinkedToChartStore(app.Id) if err != nil { impl.logger.Errorw("error in checking IsExternalAppLinkedToChartStore", "appId", app.Id, "err", err) - return isOtherExtAppMigrated, err + return err } //if isLinked is true then installed_app found for this app then this app name is already linked to an installed app then //create new appEntry for all those installedApps and link installedApp.AppId to the newly created app. @@ -473,7 +472,6 @@ func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp( impl.logger.Errorw("error in CreateNewAppEntryForAllInstalledApps", "appName", app.AppName, "err", err) //not returning from here as we have to migrate the app for requested ext-app and return the response for meta info } - isOtherExtAppMigrated = true } // migrating the requested ext-app app.AppName = appNameUniqueIdentifier @@ -483,9 +481,9 @@ func (impl AppCrudOperationServiceImpl) updateAppNameToUniqueAppIdentifierInApp( err = impl.appRepository.Update(app) if err != nil { impl.logger.Errorw("error in migrating displayName and appName to unique identifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) - return isOtherExtAppMigrated, err + return err } - return isOtherExtAppMigrated, nil + return nil } func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean.AppMetaInfoDto, error) { @@ -498,7 +496,6 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. var displayName string impl.logger.Info("request payload, appId", appId) if len(appIdSplitted) > 1 { - var isOtherExtAppMigrated bool appIdDecoded, err := client.DecodeExternalAppAppId(appId) if err != nil { impl.logger.Errorw("error in decoding app id for external app", "appId", appId, "err", err) @@ -511,17 +508,12 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. } // if app.DisplayName is empty then that app_name is not yet migrated to app name unique identifier if app.Id > 0 && len(app.DisplayName) == 0 { - isOtherExtAppMigrated, err = impl.updateAppNameToUniqueAppIdentifierInApp(app, appIdDecoded) + err = impl.updateAppNameToUniqueAppIdentifierInApp(app, appIdDecoded) if err != nil { impl.logger.Errorw("GetHelmAppMetaInfo, error in migrating displayName and appName to unique identifier for external apps", "appIdentifier", appIdDecoded, "err", err) //not returning from here as we need to show helm app metadata even if migration of app_name fails, then migration can happen on project update } } - // we have migrated for other app with same name linked to installed app not the one coming from request, in that case - // requested app in not assigned to any project. - if isOtherExtAppMigrated { - app.Team.Name = "" - } if app.Id == 0 { app.AppName = appIdDecoded.ReleaseName } From d7cf67803ee703969d97b1a47431d7fb34e68b8c Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Mon, 20 May 2024 12:45:34 +0530 Subject: [PATCH 35/35] fix --- pkg/appStore/installedApp/repository/InstalledAppModels.go | 1 + .../installedApp/repository/InstalledAppRepository.go | 2 +- .../installedApp/service/AppStoreDeploymentDBService.go | 4 +++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/appStore/installedApp/repository/InstalledAppModels.go b/pkg/appStore/installedApp/repository/InstalledAppModels.go index ec9e4163ba..18669d4423 100644 --- a/pkg/appStore/installedApp/repository/InstalledAppModels.go +++ b/pkg/appStore/installedApp/repository/InstalledAppModels.go @@ -47,6 +47,7 @@ type InstalledAppAndEnvDetails struct { EnvironmentName string `json:"environment_name"` EnvironmentId int `json:"environment_id"` AppName string `json:"app_name"` + DisplayName string `json:"display_name"` AppOfferingMode string `json:"appOfferingMode"` UpdatedOn time.Time `json:"updated_on"` EmailId string `json:"email_id"` diff --git a/pkg/appStore/installedApp/repository/InstalledAppRepository.go b/pkg/appStore/installedApp/repository/InstalledAppRepository.go index 6cd3ad60fd..f201eb0951 100644 --- a/pkg/appStore/installedApp/repository/InstalledAppRepository.go +++ b/pkg/appStore/installedApp/repository/InstalledAppRepository.go @@ -438,7 +438,7 @@ func (impl InstalledAppRepositoryImpl) GetAllInstalledApps(filter *appStoreBean. func (impl InstalledAppRepositoryImpl) GetAllInstalledAppsByAppStoreId(appStoreId int) ([]InstalledAppAndEnvDetails, error) { var installedAppAndEnvDetails []InstalledAppAndEnvDetails - var queryTemp = "select env.environment_name, env.id as environment_id, a.app_name, a.app_offering_mode, ia.updated_on, u.email_id," + + var queryTemp = "select env.environment_name, env.id as environment_id, a.app_name, a.display_name, a.app_offering_mode, ia.updated_on, u.email_id," + " asav.id as app_store_application_version_id, iav.id as installed_app_version_id, ia.id as installed_app_id, ia.app_id, ia.deployment_app_type, app_status.status as app_status" + " from installed_app_versions iav inner join installed_apps ia on iav.installed_app_id = ia.id" + " inner join app a on a.id = ia.app_id " + diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go index 8e09dc7816..9ed53bb5dc 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go @@ -311,7 +311,9 @@ func (impl *AppStoreDeploymentDBServiceImpl) GetAllInstalledAppsByAppStoreId(app installedAppRes.ClusterId = environment.ClusterId installedAppRes.Namespace = environment.Namespace } - + if util4.IsExternalChartStoreApp(a.DisplayName) { + installedAppRes.AppName = a.DisplayName + } installedAppsEnvResponse = append(installedAppsEnvResponse, installedAppRes) } return installedAppsEnvResponse, nil