Skip to content

Commit 377095a

Browse files
committed
incorporate code review comment feat. nishant sir
1 parent 6184233 commit 377095a

File tree

9 files changed

+224
-122
lines changed

9 files changed

+224
-122
lines changed

Wire.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ func InitializeApp() (*App, error) {
533533
wire.Bind(new(chartConfig.ConfigMapRepository), new(*chartConfig.ConfigMapRepositoryImpl)),
534534

535535
draftAwareConfigService.NewDraftAwareResourceServiceImpl,
536-
wire.Bind(new(draftAwareConfigService.DraftAwareResourceService), new(*draftAwareConfigService.DraftAwareResourceServiceImpl)),
536+
wire.Bind(new(draftAwareConfigService.DraftAwareConfigService), new(*draftAwareConfigService.DraftAwareConfigServiceImpl)),
537537

538538
config.WireSet,
539539

api/restHandler/ConfigMapRestHandler.go

Lines changed: 58 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"encoding/json"
2121
"fmt"
2222
"github.com/devtron-labs/devtron/pkg/pipeline/draftAwareConfigService"
23-
"github.com/devtron-labs/devtron/util"
2423
"net/http"
2524
"strconv"
2625

@@ -74,14 +73,14 @@ type ConfigMapRestHandlerImpl struct {
7473
pipelineRepository pipelineConfig.PipelineRepository
7574
enforcerUtil rbac.EnforcerUtil
7675
configMapService pipeline.ConfigMapService
77-
draftAwareResourceService draftAwareConfigService.DraftAwareResourceService
76+
draftAwareResourceService draftAwareConfigService.DraftAwareConfigService
7877
}
7978

8079
func NewConfigMapRestHandlerImpl(pipelineBuilder pipeline.PipelineBuilder, Logger *zap.SugaredLogger,
8180
chartService chart.ChartService, userAuthService user.UserService, teamService team.TeamService,
8281
enforcer casbin.Enforcer, pipelineRepository pipelineConfig.PipelineRepository,
8382
enforcerUtil rbac.EnforcerUtil, configMapService pipeline.ConfigMapService,
84-
draftAwareResourceService draftAwareConfigService.DraftAwareResourceService,
83+
draftAwareResourceService draftAwareConfigService.DraftAwareConfigService,
8584
) *ConfigMapRestHandlerImpl {
8685
return &ConfigMapRestHandlerImpl{
8786
pipelineBuilder: pipelineBuilder,
@@ -124,10 +123,14 @@ func (handler ConfigMapRestHandlerImpl) CMGlobalAddUpdate(w http.ResponseWriter,
124123
return
125124
}
126125
//RBAC END
127-
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
128126
ctx := r.Context()
129-
ctx = util.SetSuperAdminInContext(ctx, isSuperAdmin)
130-
res, err := handler.draftAwareResourceService.CMGlobalAddUpdate(ctx, &configMapRequest)
127+
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
128+
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
129+
if err != nil {
130+
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
131+
return
132+
}
133+
res, err := handler.draftAwareResourceService.CMGlobalAddUpdate(ctx, &configMapRequest, isSuperAdmin, userEmail)
131134
if err != nil {
132135
handler.Logger.Errorw("service err, CMGlobalAddUpdate", "err", err, "payload", configMapRequest)
133136
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -170,10 +173,14 @@ func (handler ConfigMapRestHandlerImpl) CMEnvironmentAddUpdate(w http.ResponseWr
170173
}
171174
}
172175
//RBAC END
173-
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
174176
ctx := r.Context()
175-
ctx = util.SetSuperAdminInContext(ctx, isSuperAdmin)
176-
res, err := handler.draftAwareResourceService.CMEnvironmentAddUpdate(ctx, &configMapRequest)
177+
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
178+
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
179+
if err != nil {
180+
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
181+
return
182+
}
183+
res, err := handler.draftAwareResourceService.CMEnvironmentAddUpdate(ctx, &configMapRequest, isSuperAdmin, userEmail)
177184
if err != nil {
178185
handler.Logger.Errorw("service err, CMEnvironmentAddUpdate", "err", err, "payload", configMapRequest)
179186
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -369,10 +376,14 @@ func (handler ConfigMapRestHandlerImpl) CSGlobalAddUpdate(w http.ResponseWriter,
369376
return
370377
}
371378
//RBAC END
372-
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
373379
ctx := r.Context()
374-
ctx = util.SetSuperAdminInContext(ctx, isSuperAdmin)
375-
res, err := handler.draftAwareResourceService.CSGlobalAddUpdate(ctx, &configMapRequest)
380+
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
381+
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
382+
if err != nil {
383+
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
384+
return
385+
}
386+
res, err := handler.draftAwareResourceService.CSGlobalAddUpdate(ctx, &configMapRequest, isSuperAdmin, userEmail)
376387
if err != nil {
377388
handler.Logger.Errorw("service err, CSGlobalAddUpdate", "err", err, "payload", configMapRequest)
378389
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -416,10 +427,14 @@ func (handler ConfigMapRestHandlerImpl) CSEnvironmentAddUpdate(w http.ResponseWr
416427
}
417428
}
418429
//RBAC END
419-
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
420430
ctx := r.Context()
421-
ctx = util.SetSuperAdminInContext(ctx, isSuperAdmin)
422-
res, err := handler.draftAwareResourceService.CSEnvironmentAddUpdate(ctx, &configMapRequest)
431+
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
432+
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
433+
if err != nil {
434+
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
435+
return
436+
}
437+
res, err := handler.draftAwareResourceService.CSEnvironmentAddUpdate(ctx, &configMapRequest, isSuperAdmin, userEmail)
423438
if err != nil {
424439
handler.Logger.Errorw("service err, CSEnvironmentAddUpdate", "err", err, "payload", configMapRequest)
425440
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -531,15 +546,19 @@ func (handler ConfigMapRestHandlerImpl) CMGlobalDelete(w http.ResponseWriter, r
531546
return
532547
}
533548
//RBAC END
534-
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
535549
ctx := r.Context()
536-
ctx = util.SetSuperAdminInContext(ctx, isSuperAdmin)
550+
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
551+
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
552+
if err != nil {
553+
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
554+
return
555+
}
537556
deleteReq := &bean.ConfigDataRequest{
538557
Id: id,
539558
AppId: appId,
540559
UserId: userId,
541560
}
542-
res, err := handler.draftAwareResourceService.CMGlobalDelete(ctx, name, deleteReq)
561+
res, err := handler.draftAwareResourceService.CMGlobalDelete(ctx, name, deleteReq, isSuperAdmin, userEmail)
543562
if err != nil {
544563
handler.Logger.Errorw("service err, CMGlobalDelete", "err", err, "appId", appId, "id", id, "name", name)
545564
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -593,15 +612,19 @@ func (handler ConfigMapRestHandlerImpl) CMEnvironmentDelete(w http.ResponseWrite
593612
}
594613
}
595614
//RBAC END
596-
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
597615
ctx := r.Context()
598-
ctx = util.SetSuperAdminInContext(ctx, isSuperAdmin)
616+
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
617+
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
618+
if err != nil {
619+
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
620+
return
621+
}
599622
deleteReq := &bean.ConfigDataRequest{
600623
Id: id,
601624
AppId: appId,
602625
UserId: userId,
603626
}
604-
res, err := handler.draftAwareResourceService.CMEnvironmentDelete(ctx, name, deleteReq)
627+
res, err := handler.draftAwareResourceService.CMEnvironmentDelete(ctx, name, deleteReq, isSuperAdmin, userEmail)
605628
if err != nil {
606629
handler.Logger.Errorw("service err, CMEnvironmentDelete", "err", err, "appId", appId, "envId", envId, "id", id)
607630
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -641,15 +664,19 @@ func (handler ConfigMapRestHandlerImpl) CSGlobalDelete(w http.ResponseWriter, r
641664
return
642665
}
643666
//RBAC END
644-
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
645667
ctx := r.Context()
646-
ctx = util.SetSuperAdminInContext(ctx, isSuperAdmin)
668+
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
669+
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
670+
if err != nil {
671+
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
672+
return
673+
}
647674
deleteReq := &bean.ConfigDataRequest{
648675
Id: id,
649676
AppId: appId,
650677
UserId: userId,
651678
}
652-
res, err := handler.draftAwareResourceService.CSGlobalDelete(ctx, name, deleteReq)
679+
res, err := handler.draftAwareResourceService.CSGlobalDelete(ctx, name, deleteReq, isSuperAdmin, userEmail)
653680
if err != nil {
654681
handler.Logger.Errorw("service err, CSGlobalDelete", "err", err, "appId", appId, "id", id, "name", name)
655682
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -703,15 +730,19 @@ func (handler ConfigMapRestHandlerImpl) CSEnvironmentDelete(w http.ResponseWrite
703730
}
704731
}
705732
//RBAC END
706-
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
707733
ctx := r.Context()
708-
ctx = util.SetSuperAdminInContext(ctx, isSuperAdmin)
734+
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
735+
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
736+
if err != nil {
737+
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
738+
return
739+
}
709740
deleteReq := &bean.ConfigDataRequest{
710741
Id: id,
711742
AppId: appId,
712743
UserId: userId,
713744
}
714-
res, err := handler.draftAwareResourceService.CSEnvironmentDelete(ctx, name, deleteReq)
745+
res, err := handler.draftAwareResourceService.CSEnvironmentDelete(ctx, name, deleteReq, isSuperAdmin, userEmail)
715746
if err != nil {
716747
handler.Logger.Errorw("service err, CSEnvironmentDelete", "err", err, "appId", appId, "envId", envId, "id", id)
717748
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)

api/restHandler/app/pipeline/configure/DeploymentPipelineRestHandler.go

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,13 @@ func (handler *PipelineConfigRestHandlerImpl) ConfigureDeploymentTemplateForApp(
176176
}(ctx.Done(), cn.CloseNotify())
177177
}
178178
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
179-
ctx = util2.SetSuperAdminInContext(ctx, isSuperAdmin)
179+
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
180+
if err != nil {
181+
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
182+
return
183+
}
180184

181-
createResp, err := handler.draftAwareResourceService.Create(ctx, templateRequest)
185+
createResp, err := handler.draftAwareResourceService.Create(ctx, templateRequest, isSuperAdmin, userEmail)
182186
if err != nil {
183187
handler.Logger.Errorw("service err, ConfigureDeploymentTemplateForApp", "err", err, "payload", templateRequest)
184188
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -693,6 +697,7 @@ func (handler *PipelineConfigRestHandlerImpl) EnvConfigOverrideCreate(w http.Res
693697
}
694698
envConfigProperties.UserId = userId
695699
envConfigProperties.EnvironmentId = environmentId
700+
envConfigProperties.AppId = appId
696701
handler.Logger.Infow("request payload, EnvConfigOverrideCreate", "payload", envConfigProperties)
697702

698703
resourceName := handler.enforcerUtil.GetAppRBACNameByAppId(appId)
@@ -730,8 +735,12 @@ func (handler *PipelineConfigRestHandlerImpl) EnvConfigOverrideCreate(w http.Res
730735
}(ctx.Done(), cn.CloseNotify())
731736
}
732737
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
733-
ctx = util2.SetSuperAdminInContext(ctx, isSuperAdmin)
734-
createResp, err := handler.draftAwareResourceService.CreateEnvironmentPropertiesAndBaseIfNeeded(ctx, appId, &envConfigProperties)
738+
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
739+
if err != nil {
740+
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
741+
return
742+
}
743+
createResp, err := handler.draftAwareResourceService.CreateEnvironmentPropertiesAndBaseIfNeeded(ctx, &envConfigProperties, isSuperAdmin, userEmail)
735744
if err != nil {
736745
handler.Logger.Errorw("service err, CreateEnvironmentPropertiesAndBaseIfNeeded", "payload", envConfigProperties, "err", err)
737746
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -770,6 +779,7 @@ func (handler *PipelineConfigRestHandlerImpl) EnvConfigOverrideUpdate(w http.Res
770779
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
771780
return
772781
}
782+
envConfigProperties.AppId = envConfigOverride.Chart.AppId
773783
appId := envConfigOverride.Chart.AppId
774784
envId := envConfigOverride.TargetEnvironment
775785
resourceName := handler.enforcerUtil.GetAppRBACNameByAppId(appId)
@@ -797,8 +807,12 @@ func (handler *PipelineConfigRestHandlerImpl) EnvConfigOverrideUpdate(w http.Res
797807
}
798808
ctx := r.Context()
799809
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
800-
ctx = util2.SetSuperAdminInContext(ctx, isSuperAdmin)
801-
createResp, err := handler.draftAwareResourceService.UpdateEnvironmentProperties(ctx, appId, &envConfigProperties, token)
810+
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
811+
if err != nil {
812+
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
813+
return
814+
}
815+
createResp, err := handler.draftAwareResourceService.UpdateEnvironmentProperties(ctx, &envConfigProperties, token, isSuperAdmin, userEmail)
802816
if err != nil {
803817
handler.Logger.Errorw("service err, EnvConfigOverrideUpdate", "err", err, "payload", envConfigProperties)
804818
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -1389,10 +1403,14 @@ func (handler *PipelineConfigRestHandlerImpl) UpdateAppOverride(w http.ResponseW
13891403
return
13901404
}
13911405
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
1392-
ctx = util2.SetSuperAdminInContext(ctx, isSuperAdmin)
1406+
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
1407+
if err != nil {
1408+
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
1409+
return
1410+
}
13931411

13941412
_, span = otel.Tracer("orchestrator").Start(ctx, "chartService.UpdateAppOverride")
1395-
createResp, err := handler.draftAwareResourceService.UpdateAppOverride(ctx, &templateRequest, token)
1413+
createResp, err := handler.draftAwareResourceService.UpdateAppOverride(ctx, &templateRequest, token, isSuperAdmin, userEmail)
13961414
span.End()
13971415
if err != nil {
13981416
handler.Logger.Errorw("service err, UpdateAppOverride", "err", err, "payload", templateRequest)
@@ -1529,14 +1547,18 @@ func (handler *PipelineConfigRestHandlerImpl) EnvConfigOverrideReset(w http.Resp
15291547
}
15301548
ctx := r.Context()
15311549
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
1532-
ctx = util2.SetSuperAdminInContext(ctx, isSuperAdmin)
1550+
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
1551+
if err != nil {
1552+
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
1553+
return
1554+
}
15331555
envProperties := &pipelineBean.EnvironmentProperties{
15341556
Id: id,
15351557
EnvironmentId: environmentId,
15361558
UserId: userId,
15371559
AppId: appId,
15381560
}
1539-
isSuccess, err := handler.draftAwareResourceService.ResetEnvironmentProperties(ctx, envProperties)
1561+
isSuccess, err := handler.draftAwareResourceService.ResetEnvironmentProperties(ctx, envProperties, isSuperAdmin, userEmail)
15401562
if err != nil {
15411563
handler.Logger.Errorw("service err, EnvConfigOverrideReset", "err", err, "appId", appId, "environmentId", environmentId)
15421564
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)

api/restHandler/app/pipeline/configure/PipelineConfigRestHandler.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ type PipelineConfigRestHandlerImpl struct {
139139
teamReadService read3.TeamReadService
140140
environmentRepository repository2.EnvironmentRepository
141141
chartReadService read5.ChartReadService
142-
draftAwareResourceService draftAwareConfigService.DraftAwareResourceService
142+
draftAwareResourceService draftAwareConfigService.DraftAwareConfigService
143143
}
144144

145145
func NewPipelineRestHandlerImpl(pipelineBuilder pipeline.PipelineBuilder, Logger *zap.SugaredLogger,
@@ -174,7 +174,7 @@ func NewPipelineRestHandlerImpl(pipelineBuilder pipeline.PipelineBuilder, Logger
174174
teamReadService read3.TeamReadService,
175175
EnvironmentRepository repository2.EnvironmentRepository,
176176
chartReadService read5.ChartReadService,
177-
draftAwareResourceService draftAwareConfigService.DraftAwareResourceService,
177+
draftAwareResourceService draftAwareConfigService.DraftAwareConfigService,
178178
) *PipelineConfigRestHandlerImpl {
179179
envConfig := &PipelineRestHandlerEnvConfig{}
180180
err := env.Parse(envConfig)

pkg/pipeline/bean/EnvironmentProperties.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,9 @@ type EnvironmentPropertiesResponse struct {
6969
Schema json.RawMessage `json:"schema"`
7070
Readme string `json:"readme"`
7171
}
72+
73+
type DeploymentConfigMetadata struct {
74+
AppId int
75+
EnvId int
76+
ResourceName string // if base then BaseDeploymentTemplate or if at env level{envName-DeploymentTemplateOverride}
77+
}

0 commit comments

Comments
 (0)