Skip to content

Commit 03693a4

Browse files
committed
code review corporation :- level 2
1 parent 6036c8b commit 03693a4

File tree

6 files changed

+96
-128
lines changed

6 files changed

+96
-128
lines changed

api/restHandler/ConfigMapRestHandler.go

Lines changed: 17 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package restHandler
1919
import (
2020
"encoding/json"
2121
"fmt"
22+
"github.com/devtron-labs/devtron/pkg/auth/user/util"
2223
"github.com/devtron-labs/devtron/pkg/pipeline/draftAwareConfigService"
2324
"net/http"
2425
"strconv"
@@ -125,12 +126,8 @@ func (handler ConfigMapRestHandlerImpl) CMGlobalAddUpdate(w http.ResponseWriter,
125126
//RBAC END
126127
ctx := r.Context()
127128
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)
129+
userMetadata := util.GetUserMetadata(r.Context(), userId, isSuperAdmin)
130+
res, err := handler.draftAwareResourceService.CMGlobalAddUpdate(ctx, &configMapRequest, userMetadata)
134131
if err != nil {
135132
handler.Logger.Errorw("service err, CMGlobalAddUpdate", "err", err, "payload", configMapRequest)
136133
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -175,12 +172,8 @@ func (handler ConfigMapRestHandlerImpl) CMEnvironmentAddUpdate(w http.ResponseWr
175172
//RBAC END
176173
ctx := r.Context()
177174
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)
175+
userMetadata := util.GetUserMetadata(r.Context(), userId, isSuperAdmin)
176+
res, err := handler.draftAwareResourceService.CMEnvironmentAddUpdate(ctx, &configMapRequest, userMetadata)
184177
if err != nil {
185178
handler.Logger.Errorw("service err, CMEnvironmentAddUpdate", "err", err, "payload", configMapRequest)
186179
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -378,12 +371,8 @@ func (handler ConfigMapRestHandlerImpl) CSGlobalAddUpdate(w http.ResponseWriter,
378371
//RBAC END
379372
ctx := r.Context()
380373
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)
374+
userMetadata := util.GetUserMetadata(r.Context(), userId, isSuperAdmin)
375+
res, err := handler.draftAwareResourceService.CSGlobalAddUpdate(ctx, &configMapRequest, userMetadata)
387376
if err != nil {
388377
handler.Logger.Errorw("service err, CSGlobalAddUpdate", "err", err, "payload", configMapRequest)
389378
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -429,12 +418,8 @@ func (handler ConfigMapRestHandlerImpl) CSEnvironmentAddUpdate(w http.ResponseWr
429418
//RBAC END
430419
ctx := r.Context()
431420
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)
421+
userMetadata := util.GetUserMetadata(r.Context(), userId, isSuperAdmin)
422+
res, err := handler.draftAwareResourceService.CSEnvironmentAddUpdate(ctx, &configMapRequest, userMetadata)
438423
if err != nil {
439424
handler.Logger.Errorw("service err, CSEnvironmentAddUpdate", "err", err, "payload", configMapRequest)
440425
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -548,17 +533,13 @@ func (handler ConfigMapRestHandlerImpl) CMGlobalDelete(w http.ResponseWriter, r
548533
//RBAC END
549534
ctx := r.Context()
550535
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-
}
536+
userMetadata := util.GetUserMetadata(r.Context(), userId, isSuperAdmin)
556537
deleteReq := &bean.ConfigDataRequest{
557538
Id: id,
558539
AppId: appId,
559540
UserId: userId,
560541
}
561-
res, err := handler.draftAwareResourceService.CMGlobalDelete(ctx, name, deleteReq, isSuperAdmin, userEmail)
542+
res, err := handler.draftAwareResourceService.CMGlobalDelete(ctx, name, deleteReq, userMetadata)
562543
if err != nil {
563544
handler.Logger.Errorw("service err, CMGlobalDelete", "err", err, "appId", appId, "id", id, "name", name)
564545
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -614,17 +595,13 @@ func (handler ConfigMapRestHandlerImpl) CMEnvironmentDelete(w http.ResponseWrite
614595
//RBAC END
615596
ctx := r.Context()
616597
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-
}
598+
userMetadata := util.GetUserMetadata(r.Context(), userId, isSuperAdmin)
622599
deleteReq := &bean.ConfigDataRequest{
623600
Id: id,
624601
AppId: appId,
625602
UserId: userId,
626603
}
627-
res, err := handler.draftAwareResourceService.CMEnvironmentDelete(ctx, name, deleteReq, isSuperAdmin, userEmail)
604+
res, err := handler.draftAwareResourceService.CMEnvironmentDelete(ctx, name, deleteReq, userMetadata)
628605
if err != nil {
629606
handler.Logger.Errorw("service err, CMEnvironmentDelete", "err", err, "appId", appId, "envId", envId, "id", id)
630607
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -666,17 +643,13 @@ func (handler ConfigMapRestHandlerImpl) CSGlobalDelete(w http.ResponseWriter, r
666643
//RBAC END
667644
ctx := r.Context()
668645
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-
}
646+
userMetadata := util.GetUserMetadata(r.Context(), userId, isSuperAdmin)
674647
deleteReq := &bean.ConfigDataRequest{
675648
Id: id,
676649
AppId: appId,
677650
UserId: userId,
678651
}
679-
res, err := handler.draftAwareResourceService.CSGlobalDelete(ctx, name, deleteReq, isSuperAdmin, userEmail)
652+
res, err := handler.draftAwareResourceService.CSGlobalDelete(ctx, name, deleteReq, userMetadata)
680653
if err != nil {
681654
handler.Logger.Errorw("service err, CSGlobalDelete", "err", err, "appId", appId, "id", id, "name", name)
682655
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -732,17 +705,13 @@ func (handler ConfigMapRestHandlerImpl) CSEnvironmentDelete(w http.ResponseWrite
732705
//RBAC END
733706
ctx := r.Context()
734707
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-
}
708+
userMetadata := util.GetUserMetadata(r.Context(), userId, isSuperAdmin)
740709
deleteReq := &bean.ConfigDataRequest{
741710
Id: id,
742711
AppId: appId,
743712
UserId: userId,
744713
}
745-
res, err := handler.draftAwareResourceService.CSEnvironmentDelete(ctx, name, deleteReq, isSuperAdmin, userEmail)
714+
res, err := handler.draftAwareResourceService.CSEnvironmentDelete(ctx, name, deleteReq, userMetadata)
746715
if err != nil {
747716
handler.Logger.Errorw("service err, CSEnvironmentDelete", "err", err, "appId", appId, "envId", envId, "id", id)
748717
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)

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

Lines changed: 11 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"fmt"
2424
models2 "github.com/devtron-labs/devtron/internal/sql/models"
2525
bean4 "github.com/devtron-labs/devtron/pkg/auth/user/bean"
26+
util3 "github.com/devtron-labs/devtron/pkg/auth/user/util"
2627
bean3 "github.com/devtron-labs/devtron/pkg/chart/bean"
2728

2829
devtronAppGitOpConfigBean "github.com/devtron-labs/devtron/pkg/chart/gitOpsConfig/bean"
@@ -178,13 +179,9 @@ func (handler *PipelineConfigRestHandlerImpl) ConfigureDeploymentTemplateForApp(
178179
}(ctx.Done(), cn.CloseNotify())
179180
}
180181
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
181-
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
182-
if err != nil {
183-
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
184-
return
185-
}
182+
userMetadata := util3.GetUserMetadata(r.Context(), userId, isSuperAdmin)
186183

187-
createResp, err := handler.draftAwareResourceService.Create(ctx, templateRequest, isSuperAdmin, userEmail)
184+
createResp, err := handler.draftAwareResourceService.Create(ctx, templateRequest, userMetadata)
188185
if err != nil {
189186
handler.Logger.Errorw("service err, ConfigureDeploymentTemplateForApp", "err", err, "payload", templateRequest)
190187
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -684,12 +681,8 @@ func (handler *PipelineConfigRestHandlerImpl) EnvConfigOverrideCreate(w http.Res
684681
}(ctx.Done(), cn.CloseNotify())
685682
}
686683
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
687-
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
688-
if err != nil {
689-
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
690-
return
691-
}
692-
createResp, err := handler.draftAwareResourceService.CreateEnvironmentPropertiesAndBaseIfNeeded(ctx, &envConfigProperties, isSuperAdmin, userEmail)
684+
userMetadata := util3.GetUserMetadata(r.Context(), userId, isSuperAdmin)
685+
createResp, err := handler.draftAwareResourceService.CreateEnvironmentPropertiesAndBaseIfNeeded(ctx, &envConfigProperties, userMetadata)
693686
if err != nil {
694687
handler.Logger.Errorw("service err, CreateEnvironmentPropertiesAndBaseIfNeeded", "payload", envConfigProperties, "err", err)
695688
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -756,12 +749,8 @@ func (handler *PipelineConfigRestHandlerImpl) EnvConfigOverrideUpdate(w http.Res
756749
}
757750
ctx := r.Context()
758751
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
759-
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
760-
if err != nil {
761-
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
762-
return
763-
}
764-
createResp, err := handler.draftAwareResourceService.UpdateEnvironmentProperties(ctx, &envConfigProperties, token, isSuperAdmin, userEmail)
752+
userMetadata := util3.GetUserMetadata(r.Context(), userId, isSuperAdmin)
753+
createResp, err := handler.draftAwareResourceService.UpdateEnvironmentProperties(ctx, &envConfigProperties, token, userMetadata)
765754
if err != nil {
766755
handler.Logger.Errorw("service err, EnvConfigOverrideUpdate", "err", err, "payload", envConfigProperties)
767756
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
@@ -1358,14 +1347,10 @@ func (handler *PipelineConfigRestHandlerImpl) UpdateAppOverride(w http.ResponseW
13581347
return
13591348
}
13601349
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
1361-
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
1362-
if err != nil {
1363-
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
1364-
return
1365-
}
1350+
userMetadata := util3.GetUserMetadata(r.Context(), userId, isSuperAdmin)
13661351

13671352
_, span = otel.Tracer("orchestrator").Start(ctx, "chartService.UpdateAppOverride")
1368-
createResp, err := handler.draftAwareResourceService.UpdateAppOverride(ctx, &templateRequest, token, isSuperAdmin, userEmail)
1353+
createResp, err := handler.draftAwareResourceService.UpdateAppOverride(ctx, &templateRequest, token, userMetadata)
13691354
span.End()
13701355
if err != nil {
13711356
handler.Logger.Errorw("service err, UpdateAppOverride", "err", err, "payload", templateRequest)
@@ -1507,18 +1492,14 @@ func (handler *PipelineConfigRestHandlerImpl) EnvConfigOverrideReset(w http.Resp
15071492
}
15081493
ctx := r.Context()
15091494
isSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionCreate, "*")
1510-
userEmail, err := handler.userAuthService.GetActiveEmailById(userId)
1511-
if err != nil {
1512-
common.WriteJsonResp(w, fmt.Errorf("userEmail not found by userId"), "userEmail not found by userId", http.StatusNotFound)
1513-
return
1514-
}
1495+
userMetadata := util3.GetUserMetadata(r.Context(), userId, isSuperAdmin)
15151496
envProperties := &pipelineBean.EnvironmentProperties{
15161497
Id: id,
15171498
EnvironmentId: environmentId,
15181499
UserId: userId,
15191500
AppId: appId,
15201501
}
1521-
isSuccess, err := handler.draftAwareResourceService.ResetEnvironmentProperties(ctx, envProperties, isSuperAdmin, userEmail)
1502+
isSuccess, err := handler.draftAwareResourceService.ResetEnvironmentProperties(ctx, envProperties, userMetadata)
15221503
if err != nil {
15231504
handler.Logger.Errorw("service err, EnvConfigOverrideReset", "err", err, "appId", appId, "environmentId", environmentId)
15241505
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)

internal/sql/repository/pipelineConfig/CdWorfkflowRepository.go

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -106,28 +106,28 @@ type CdWorkflowRunnerWithExtraFields struct {
106106
}
107107

108108
type CdWorkflowRunner struct {
109-
tableName struct{} `sql:"cd_workflow_runner" pg:",discard_unknown_columns"`
110-
Id int `sql:"id,pk"`
111-
Name string `sql:"name"`
112-
WorkflowType apiBean.WorkflowType `sql:"workflow_type"` // pre,post,deploy
113-
ExecutorType cdWorkflow.WorkflowExecutorType `sql:"executor_type"` // awf, system
114-
Status string `sql:"status"`
115-
PodStatus string `sql:"pod_status"`
116-
Message string `sql:"message"`
117-
StartedOn time.Time `sql:"started_on"`
118-
FinishedOn time.Time `sql:"finished_on"`
119-
Namespace string `sql:"namespace"`
120-
LogLocation string `sql:"log_file_path"`
121-
CdArtifactLocation string `sql:"cd_artifact_location"`
122-
IsArtifactUploaded workflow.ArtifactUploadedType `sql:"is_artifact_uploaded"`
123-
TriggeredBy int32 `sql:"triggered_by"`
124-
CdWorkflowId int `sql:"cd_workflow_id"`
125-
PodName string `sql:"pod_name"`
126-
BlobStorageEnabled bool `sql:"blob_storage_enabled,notnull"`
127-
RefCdWorkflowRunnerId int `sql:"ref_cd_workflow_runner_id,notnull"`
128-
ImagePathReservationIds []int `sql:"image_path_reservation_ids" pg:",array,notnull"`
129-
ReferenceId *string `sql:"reference_id"`
130-
ImageState constants.ImageStateWhileDeployment `sql:"image_state"` // image_state currently not utilized in oss
109+
tableName struct{} `sql:"cd_workflow_runner" pg:",discard_unknown_columns"`
110+
Id int `sql:"id,pk"`
111+
Name string `sql:"name"`
112+
WorkflowType apiBean.WorkflowType `sql:"workflow_type"` // pre,post,deploy
113+
ExecutorType cdWorkflow.WorkflowExecutorType `sql:"executor_type"` // awf, system
114+
Status string `sql:"status"`
115+
PodStatus string `sql:"pod_status"`
116+
Message string `sql:"message"`
117+
StartedOn time.Time `sql:"started_on"`
118+
FinishedOn time.Time `sql:"finished_on"`
119+
Namespace string `sql:"namespace"`
120+
LogLocation string `sql:"log_file_path"`
121+
CdArtifactLocation string `sql:"cd_artifact_location"`
122+
IsArtifactUploaded workflow.ArtifactUploadedType `sql:"is_artifact_uploaded"`
123+
TriggeredBy int32 `sql:"triggered_by"`
124+
CdWorkflowId int `sql:"cd_workflow_id"`
125+
PodName string `sql:"pod_name"`
126+
BlobStorageEnabled bool `sql:"blob_storage_enabled,notnull"`
127+
RefCdWorkflowRunnerId int `sql:"ref_cd_workflow_runner_id,notnull"`
128+
ImagePathReservationIds []int `sql:"image_path_reservation_ids" pg:",array,notnull"`
129+
ReferenceId *string `sql:"reference_id"`
130+
ArtifactState constants.ArtifactStateWhileDeployment `sql:"artifact_state"` // artifact_state currently not utilized in oss
131131
CdWorkflow *CdWorkflow
132132
sql.AuditLog
133133
}

pkg/auth/user/util/util.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,12 @@
1616

1717
package util
1818

19-
import "strings"
19+
import (
20+
"context"
21+
"github.com/devtron-labs/devtron/pkg/auth/user/bean"
22+
util2 "github.com/devtron-labs/devtron/util"
23+
"strings"
24+
)
2025

2126
const (
2227
ApiTokenPrefix = "API-TOKEN:"
@@ -39,3 +44,15 @@ func CheckIfAdminOrApiToken(email string) bool {
3944
func CheckIfApiToken(email string) bool {
4045
return strings.HasPrefix(email, ApiTokenPrefix)
4146
}
47+
48+
func GetUserMetadata(ctx context.Context, userId int32, isSuperAdmin bool) *bean.UserMetadata {
49+
// Get user email from context
50+
userEmail := util2.GetEmailFromContext(ctx)
51+
52+
// Create and return the UserMetadata object
53+
return &bean.UserMetadata{
54+
UserEmailId: userEmail,
55+
IsUserSuperAdmin: isSuperAdmin,
56+
UserId: userId,
57+
}
58+
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
package constants
22

3-
type ImageStateWhileDeployment string
3+
type ArtifactStateWhileDeployment string

0 commit comments

Comments
 (0)