Skip to content

Commit bd51187

Browse files
kripanshdevtronkartik-579
kripanshdevtron
and
kartik-579
authored
fix: Ea rbac fixes (#5813)
* app found using display or app name * single query optimization * display name handling for new apps --------- Co-authored-by: kartik-579 <[email protected]>
1 parent 8f92d3f commit bd51187

File tree

4 files changed

+89
-7
lines changed

4 files changed

+89
-7
lines changed

‎pkg/appStore/bean/bean.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"fmt"
2222
apiBean "github.com/devtron-labs/devtron/api/bean/gitOps"
2323
openapi "github.com/devtron-labs/devtron/api/helm-app/openapiClient"
24+
bean3 "github.com/devtron-labs/devtron/api/helm-app/service/bean"
2425
"github.com/devtron-labs/devtron/pkg/cluster/repository/bean"
2526
bean2 "github.com/devtron-labs/devtron/pkg/deployment/common/bean"
2627
"github.com/devtron-labs/devtron/util/gitUtil"
@@ -120,6 +121,19 @@ type InstallAppVersionDTO struct {
120121
DisplayName string `json:"-"` // used only for external apps
121122
}
122123

124+
func (chart *InstallAppVersionDTO) GetAppIdentifierString() string {
125+
displayName := chart.DisplayName
126+
if len(displayName) == 0 {
127+
displayName = chart.AppName
128+
}
129+
appIdentifier := &bean3.AppIdentifier{
130+
ClusterId: chart.ClusterId,
131+
Namespace: chart.Namespace,
132+
ReleaseName: displayName,
133+
}
134+
return appIdentifier.GetUniqueAppNameIdentifier()
135+
}
136+
123137
// UpdateDeploymentAppType updates deploymentAppType to InstallAppVersionDTO
124138
func (chart *InstallAppVersionDTO) UpdateDeploymentAppType(deploymentAppType string) {
125139
if chart == nil {

‎pkg/appStore/installedApp/repository/InstalledAppRepository.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ type InstalledAppRepository interface {
139139
GetInstalledAppVersionByClusterIds(clusterIds []int) ([]*InstalledAppVersions, error) //unused
140140
GetInstalledAppVersionByClusterIdsV2(clusterIds []int) ([]*InstalledAppVersions, error)
141141
GetInstalledApplicationByClusterIdAndNamespaceAndAppName(clusterId int, namespace string, appName string) (*InstalledApps, error)
142+
GetInstalledApplicationByClusterIdAndNamespaceAndAppIdentifier(clusterId int, namespace string, appIdentifier string, appName string) (*InstalledApps, error)
142143
GetAppAndEnvDetailsForDeploymentAppTypeInstalledApps(deploymentAppType string, clusterIds []int) ([]*InstalledApps, error)
143144
GetDeploymentSuccessfulStatusCountForTelemetry() (int, error)
144145
GetGitOpsInstalledAppsWhereArgoAppDeletedIsTrue(installedAppId int, envId int) (InstalledApps, error)
@@ -672,6 +673,38 @@ func (impl InstalledAppRepositoryImpl) GetInstalledAppVersionByClusterIdsV2(clus
672673
return installedAppVersions, err
673674
}
674675

676+
func (impl InstalledAppRepositoryImpl) GetInstalledApplicationByClusterIdAndNamespaceAndAppIdentifier(clusterId int, namespace string, appIdentifier string, appName string) (*InstalledApps, error) {
677+
var installedApps []*InstalledApps
678+
err := impl.dbConnection.Model(&installedApps).
679+
Column("installed_apps.*", "App", "Environment", "App.Team").
680+
Where("environment.cluster_id = ?", clusterId).
681+
Where("environment.namespace = ?", namespace).
682+
Where("app.app_name = ? OR app.display_name = ?", appName, appName).
683+
Where("installed_apps.active = ?", true).
684+
Where("app.active = ?", true).
685+
Where("environment.active = ?", true).
686+
Select()
687+
// extract app which has matching display name and app name
688+
for _, installedApp := range installedApps {
689+
appObj := installedApp.App
690+
if appObj.DisplayName == appName && appObj.AppName == appIdentifier {
691+
return installedApp, nil
692+
}
693+
}
694+
// if not found any matching app in above case, then return app with only app name
695+
for _, installedApp := range installedApps {
696+
appObj := installedApp.App
697+
if appObj.DisplayName == "" && appObj.AppName == appName {
698+
return installedApp, nil
699+
}
700+
}
701+
if err == nil {
702+
err = pg.ErrNoRows
703+
}
704+
705+
return &InstalledApps{}, err
706+
}
707+
675708
func (impl InstalledAppRepositoryImpl) GetInstalledApplicationByClusterIdAndNamespaceAndAppName(clusterId int, namespace string, appName string) (*InstalledApps, error) {
676709
model := &InstalledApps{}
677710
err := impl.dbConnection.Model(model).

‎pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,10 @@ func (impl *AppStoreDeploymentDBServiceImpl) AppStoreDeployOperationDB(installRe
148148
appCreateRequest.AppType = helper.ExternalChartStoreApp
149149
appCreateRequest.DisplayName = installRequest.DisplayName
150150
}
151+
if globalUtil.IsBaseStack() || globalUtil.IsHelmApp(installRequest.AppOfferingMode) {
152+
appCreateRequest.DisplayName = installRequest.AppName
153+
appCreateRequest.AppName = installRequest.GetAppIdentifierString()
154+
}
151155
appCreateRequest, err = impl.createAppForAppStore(appCreateRequest, tx, getAppInstallationMode(installRequest.AppOfferingMode))
152156
if err != nil {
153157
impl.logger.Errorw("error while creating app", "error", err)
@@ -603,6 +607,7 @@ func (impl *AppStoreDeploymentDBServiceImpl) createAppForAppStore(createRequest
603607
TeamId: createRequest.TeamId,
604608
AppType: helper.ChartStoreApp,
605609
AppOfferingMode: appInstallationMode,
610+
DisplayName: createRequest.DisplayName,
606611
}
607612
if createRequest.AppType == helper.ExternalChartStoreApp {
608613
//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

‎util/rbac/EnforcerUtilHelm.go‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package rbac
1818

1919
import (
2020
"fmt"
21+
"github.com/devtron-labs/devtron/api/helm-app/service/bean"
2122
"github.com/devtron-labs/devtron/internal/sql/repository/app"
2223
repository2 "github.com/devtron-labs/devtron/pkg/appStore/installedApp/repository"
2324
"github.com/devtron-labs/devtron/pkg/cluster/repository"
@@ -78,8 +79,7 @@ func (impl EnforcerUtilHelmImpl) GetHelmObjectByTeamIdAndClusterId(teamId int, c
7879

7980
func (impl EnforcerUtilHelmImpl) GetHelmObjectByClusterIdNamespaceAndAppName(clusterId int, namespace string, appName string) (string, string) {
8081

81-
installedApp, installedAppErr := impl.InstalledAppRepository.GetInstalledApplicationByClusterIdAndNamespaceAndAppName(clusterId, namespace, appName)
82-
82+
installedApp, installedAppErr := impl.getInstalledApp(clusterId, namespace, appName)
8383
if installedAppErr != nil && installedAppErr != pg.ErrNoRows {
8484
impl.logger.Errorw("error on fetching data for rbac object from installed app repository", "err", installedAppErr)
8585
return "", ""
@@ -93,19 +93,18 @@ func (impl EnforcerUtilHelmImpl) GetHelmObjectByClusterIdNamespaceAndAppName(clu
9393

9494
if installedApp == nil || installedAppErr == pg.ErrNoRows {
9595
// for cli apps which are not yet linked
96-
97-
app, err := impl.appRepository.FindAppAndProjectByAppName(appName)
96+
app, err := impl.getAppObject(clusterId, namespace, appName)
9897
if err != nil && err != pg.ErrNoRows {
9998
impl.logger.Errorw("error in fetching app details", "err", err)
10099
return "", ""
101100
}
102101

103102
if app.TeamId == 0 {
104103
// case if project is not assigned to cli app
105-
return fmt.Sprintf("%s/%s__%s/%s", team.UNASSIGNED_PROJECT, cluster.ClusterName, namespace, appName), ""
104+
return fmt.Sprintf("%s/%s__%s/%s", team.UNASSIGNED_PROJECT, cluster.ClusterName, namespace, appName), fmt.Sprintf("%s/%s/%s", team.UNASSIGNED_PROJECT, namespace, appName)
106105
} else {
107106
// case if project is assigned
108-
return fmt.Sprintf("%s/%s__%s/%s", app.Team.Name, cluster.ClusterName, namespace, appName), ""
107+
return fmt.Sprintf("%s/%s__%s/%s", app.Team.Name, cluster.ClusterName, namespace, appName), fmt.Sprintf("%s/%s/%s", app.Team.Name, namespace, appName)
109108
}
110109

111110
}
@@ -118,7 +117,7 @@ func (impl EnforcerUtilHelmImpl) GetHelmObjectByClusterIdNamespaceAndAppName(clu
118117
} else {
119118
if installedApp.EnvironmentId == 0 {
120119
// for apps in EA mode, initally env can be 0.
121-
return fmt.Sprintf("%s/%s__%s/%s", installedApp.App.Team.Name, cluster.ClusterName, namespace, appName), ""
120+
return fmt.Sprintf("%s/%s__%s/%s", installedApp.App.Team.Name, cluster.ClusterName, namespace, appName), fmt.Sprintf("%s/%s/%s", installedApp.App.Team.Name, namespace, appName)
122121
}
123122
// for apps which are assigned to a project and have env ID
124123
rbacOne := fmt.Sprintf("%s/%s/%s", installedApp.App.Team.Name, installedApp.Environment.EnvironmentIdentifier, appName)
@@ -131,6 +130,37 @@ func (impl EnforcerUtilHelmImpl) GetHelmObjectByClusterIdNamespaceAndAppName(clu
131130

132131
}
133132

133+
func (impl EnforcerUtilHelmImpl) getAppObject(clusterId int, namespace string, appName string) (*app.App, error) {
134+
appIdentifier := &bean.AppIdentifier{
135+
ClusterId: clusterId,
136+
Namespace: namespace,
137+
ReleaseName: appName,
138+
}
139+
appNameIdentifier := appIdentifier.GetUniqueAppNameIdentifier()
140+
appObj, err := impl.appRepository.FindAppAndProjectByAppName(appNameIdentifier)
141+
if appObj == nil || err == pg.ErrNoRows {
142+
impl.logger.Warnw("appObj not found, going to find app using display name ", "appIdentifier", appNameIdentifier, "appName", appName)
143+
appObj, err = impl.appRepository.FindAppAndProjectByAppName(appName)
144+
}
145+
return appObj, err
146+
}
147+
148+
func (impl EnforcerUtilHelmImpl) getInstalledApp(clusterId int, namespace string, appName string) (*repository2.InstalledApps, error) {
149+
appIdentifier := &bean.AppIdentifier{
150+
ClusterId: clusterId,
151+
Namespace: namespace,
152+
ReleaseName: appName,
153+
}
154+
appNameIdentifier := appIdentifier.GetUniqueAppNameIdentifier()
155+
//installedApp, installedAppErr := impl.InstalledAppRepository.GetInstalledApplicationByClusterIdAndNamespaceAndAppName(clusterId, namespace, appNameIdentifier)
156+
//if installedApp == nil || installedAppErr == pg.ErrNoRows {
157+
// impl.logger.Warnw("installed app not found, going to find app using display name ", "appIdentifier", appNameIdentifier, "appName", appName)
158+
// installedApp, installedAppErr = impl.InstalledAppRepository.GetInstalledApplicationByClusterIdAndNamespaceAndAppName(clusterId, namespace, appName)
159+
//}
160+
return impl.InstalledAppRepository.GetInstalledApplicationByClusterIdAndNamespaceAndAppIdentifier(clusterId, namespace, appNameIdentifier, appName)
161+
//return installedApp, installedAppErr
162+
}
163+
134164
func (impl EnforcerUtilHelmImpl) GetAppRBACNameByInstalledAppId(installedAppVersionId int) (string, string) {
135165

136166
InstalledApp, err := impl.InstalledAppRepository.GetInstalledApp(installedAppVersionId)

0 commit comments

Comments
 (0)