Merge branch 'main' into rbac-fix-terminal

pkg/appStore/deployment/common/AppStoreDeploymentCommonService.go

@@ -26,6 +26,7 @@ import (
 	appStoreBean "github.com/devtron-labs/devtron/pkg/appStore/bean"
 	"github.com/devtron-labs/devtron/pkg/appStore/deployment/repository"
 	appStoreDiscoverRepository "github.com/devtron-labs/devtron/pkg/appStore/discover/repository"
+	util2 "github.com/devtron-labs/devtron/pkg/appStore/util"
 	repository2 "github.com/devtron-labs/devtron/pkg/cluster/repository"
 	"github.com/go-pg/pg"
 	"github.com/google/go-github/github"
@@ -49,7 +50,7 @@ type AppStoreDeploymentCommonService interface {
 	ParseGitRepoErrorResponse(err error) (bool, error)
 	GetValuesAndRequirementGitConfig(installAppVersionRequest *appStoreBean.InstallAppVersionDTO) (*util.ChartConfig, *util.ChartConfig, error)
 	CreateChartProxyAndGetPath(installAppVersionRequest *appStoreBean.InstallAppVersionDTO) (*util.ChartCreateResponse, error)
-	CreateGitOpsRepoAndPushChart(installAppVersionRequest *appStoreBean.InstallAppVersionDTO, builtChartPath string) (*util.ChartGitAttribute, error)
+	CreateGitOpsRepoAndPushChart(installAppVersionRequest *appStoreBean.InstallAppVersionDTO, builtChartPath string, requirementsConfig *util.ChartConfig, valuesConfig *util.ChartConfig) (*util.ChartGitAttribute, bool, string, error)
 	CommitConfigToGit(chartConfig *util.ChartConfig) (gitHash string, err error)
 	GetGitCommitConfig(installAppVersionRequest *appStoreBean.InstallAppVersionDTO, fileString string, filename string) (*util.ChartConfig, error)
 	GetValuesString(chartName, valuesOverrideYaml string) (string, error)
@@ -380,7 +381,7 @@ func (impl AppStoreDeploymentCommonServiceImpl) GenerateManifest(installAppVersi
 //}
 
 // CreateGitOpsRepo creates a gitOps repo with readme
-func (impl AppStoreDeploymentCommonServiceImpl) CreateGitOpsRepo(installAppVersionRequest *appStoreBean.InstallAppVersionDTO) (string, error) {
+func (impl AppStoreDeploymentCommonServiceImpl) CreateGitOpsRepo(installAppVersionRequest *appStoreBean.InstallAppVersionDTO) (string, bool, error) {
 
 	if len(installAppVersionRequest.GitOpsRepoName) == 0 {
 		//here gitops repo will be the app name, to breaking the mono repo structure
@@ -393,7 +394,7 @@ func (impl AppStoreDeploymentCommonServiceImpl) CreateGitOpsRepo(installAppVersi
 			err = nil
 			gitOpsConfigBitbucket.BitBucketWorkspaceId = ""
 		} else {
-			return "", err
+			return "", false, err
 		}
 	}
 	//getting user name & emailId for commit author data
@@ -406,18 +407,18 @@ func (impl AppStoreDeploymentCommonServiceImpl) CreateGitOpsRepo(installAppVersi
 		BitBucketWorkspaceId: gitOpsConfigBitbucket.BitBucketWorkspaceId,
 		BitBucketProjectKey:  gitOpsConfigBitbucket.BitBucketProjectKey,
 	}
-	repoUrl, _, detailedError := impl.gitFactory.Client.CreateRepository(gitRepoRequest)
+	repoUrl, isNew, detailedError := impl.gitFactory.Client.CreateRepository(gitRepoRequest)
 	for _, err := range detailedError.StageErrorMap {
 		if err != nil {
 			impl.logger.Errorw("error in creating git project", "name", installAppVersionRequest.GitOpsRepoName, "err", err)
-			return "", err
+			return "", false, err
 		}
 	}
-	return repoUrl, err
+	return repoUrl, isNew, err
 }
 
 // PushChartToGitopsRepo pushes built chart to gitOps repo
-func (impl AppStoreDeploymentCommonServiceImpl) PushChartToGitopsRepo(PushChartToGitRequest *appStoreBean.PushChartToGitRequestDTO) (*util.ChartGitAttribute, error) {
+func (impl AppStoreDeploymentCommonServiceImpl) PushChartToGitopsRepo(PushChartToGitRequest *appStoreBean.PushChartToGitRequestDTO, requirementsConfig *util.ChartConfig, valuesConfig *util.ChartConfig) (*util.ChartGitAttribute, string, error) {
 	space := regexp.MustCompile(`\s+`)
 	appStoreName := space.ReplaceAllString(PushChartToGitRequest.ChartAppStoreName, "-")
 	chartDir := fmt.Sprintf("%s-%s", PushChartToGitRequest.AppName, impl.chartTemplateService.GetDir())
@@ -426,25 +427,35 @@ func (impl AppStoreDeploymentCommonServiceImpl) PushChartToGitopsRepo(PushChartT
 		clonedDir, err = impl.gitFactory.GitService.Clone(PushChartToGitRequest.RepoURL, chartDir)
 		if err != nil {
 			impl.logger.Errorw("error in cloning repo", "url", PushChartToGitRequest.RepoURL, "err", err)
-			return nil, err
+			return nil, "", err
 		}
 	} else {
 		err = impl.chartTemplateService.GitPull(clonedDir, PushChartToGitRequest.RepoURL, appStoreName)
 		if err != nil {
-			return nil, err
+			return nil, "", err
 		}
 	}
 	acdAppName := fmt.Sprintf("%s-%s", PushChartToGitRequest.AppName, PushChartToGitRequest.EnvName)
 	dir := filepath.Join(clonedDir, acdAppName)
 	err := os.MkdirAll(dir, os.ModePerm)
 	if err != nil {
 		impl.logger.Errorw("error in making dir", "err", err)
-		return nil, err
+		return nil, "", err
 	}
 	err = dirCopy.Copy(PushChartToGitRequest.TempChartRefDir, dir)
 	if err != nil {
 		impl.logger.Errorw("error copying dir", "err", err)
-		return nil, err
+		return nil, "", err
+	}
+	err = impl.AddConfigFileToChart(requirementsConfig, dir, clonedDir)
+	if err != nil {
+		impl.logger.Errorw("error in adding requirements.yaml to chart", "err", err, "appName", PushChartToGitRequest.AppName)
+		return nil, "", err
+	}
+	err = impl.AddConfigFileToChart(valuesConfig, dir, clonedDir)
+	if err != nil {
+		impl.logger.Errorw("error in adding values.yaml to chart", "err", err, "appName", PushChartToGitRequest.AppName)
+		return nil, "", err
 	}
 	userEmailId, userName := impl.chartTemplateService.GetUserEmailIdAndNameForGitOpsCommit(PushChartToGitRequest.UserId)
 	commit, err := impl.gitFactory.GitService.CommitAndPushAllChanges(clonedDir, "first commit", userName, userEmailId)
@@ -453,38 +464,62 @@ func (impl AppStoreDeploymentCommonServiceImpl) PushChartToGitopsRepo(PushChartT
 		impl.logger.Warn("re-trying, taking pull and then push again")
 		err = impl.chartTemplateService.GitPull(clonedDir, PushChartToGitRequest.RepoURL, acdAppName)
 		if err != nil {
-			return nil, err
+			impl.logger.Errorw("error in git pull", "err", err, "appName", acdAppName)
+			return nil, "", err
 		}
 		err = dirCopy.Copy(PushChartToGitRequest.TempChartRefDir, dir)
 		if err != nil {
 			impl.logger.Errorw("error copying dir", "err", err)
-			return nil, err
+			return nil, "", err
 		}
 		commit, err = impl.gitFactory.GitService.CommitAndPushAllChanges(clonedDir, "first commit", userName, userEmailId)
 		if err != nil {
 			impl.logger.Errorw("error in pushing git", "err", err)
-			return nil, err
+			return nil, "", err
 		}
 	}
 	impl.logger.Debugw("template committed", "url", PushChartToGitRequest.RepoURL, "commit", commit)
 	defer impl.chartTemplateService.CleanDir(clonedDir)
-	return &util.ChartGitAttribute{RepoUrl: PushChartToGitRequest.RepoURL, ChartLocation: filepath.Join("", acdAppName)}, nil
+	return &util.ChartGitAttribute{RepoUrl: PushChartToGitRequest.RepoURL, ChartLocation: acdAppName}, commit, err
+}
+
+// AddConfigFileToChart will override requirements.yaml file in chart
+func (impl AppStoreDeploymentCommonServiceImpl) AddConfigFileToChart(config *util.ChartConfig, dir string, clonedDir string) error {
+	filePath := filepath.Join(clonedDir, config.FileName)
+	file, err := os.Create(filePath)
+	if err != nil {
+		impl.logger.Errorw("error in creating file", "err", err, "fileName", config.FileName)
+		return err
+	}
+	defer file.Close()
+	_, err = file.Write([]byte(config.FileContent))
+	if err != nil {
+		impl.logger.Errorw("error in writing file content", "err", err, "fileName", config.FileName)
+		return err
+	}
+	destinationFilePath := filepath.Join(dir, config.FileName)
+	err = util2.MoveFileToDestination(filePath, destinationFilePath)
+	if err != nil {
+		impl.logger.Errorw("error in moving file from source to destination", "err", err)
+		return err
+	}
+	return nil
 }
 
 // CreateGitOpsRepoAndPushChart is a wrapper for creating gitops repo and pushing chart to created repo
-func (impl AppStoreDeploymentCommonServiceImpl) CreateGitOpsRepoAndPushChart(installAppVersionRequest *appStoreBean.InstallAppVersionDTO, builtChartPath string) (*util.ChartGitAttribute, error) {
-	repoURL, err := impl.CreateGitOpsRepo(installAppVersionRequest)
+func (impl AppStoreDeploymentCommonServiceImpl) CreateGitOpsRepoAndPushChart(installAppVersionRequest *appStoreBean.InstallAppVersionDTO, builtChartPath string, requirementsConfig *util.ChartConfig, valuesConfig *util.ChartConfig) (*util.ChartGitAttribute, bool, string, error) {
+	repoURL, isNew, err := impl.CreateGitOpsRepo(installAppVersionRequest)
 	if err != nil {
 		impl.logger.Errorw("Error in creating gitops repo for ", "appName", installAppVersionRequest.AppName, "err", err)
-		return nil, err
+		return nil, false, "", err
 	}
 	pushChartToGitRequest := ParseChartGitPushRequest(installAppVersionRequest, repoURL, builtChartPath)
-	chartGitAttribute, err := impl.PushChartToGitopsRepo(pushChartToGitRequest)
+	chartGitAttribute, commitHash, err := impl.PushChartToGitopsRepo(pushChartToGitRequest, requirementsConfig, valuesConfig)
 	if err != nil {
 		impl.logger.Errorw("error in pushing chart to git", "err", err)
-		return nil, err
+		return nil, false, "", err
 	}
-	return chartGitAttribute, err
+	return chartGitAttribute, isNew, commitHash, err
 }
 
 // CommitConfigToGit is used for committing values.yaml and requirements.yaml file config
@@ -508,34 +543,39 @@ func (impl AppStoreDeploymentCommonServiceImpl) CommitConfigToGit(chartConfig *u
 
 func (impl AppStoreDeploymentCommonServiceImpl) GitOpsOperations(manifestResponse *AppStoreManifestResponse, installAppVersionRequest *appStoreBean.InstallAppVersionDTO) (*AppStoreGitOpsResponse, error) {
 	appStoreGitOpsResponse := &AppStoreGitOpsResponse{}
-	chartGitAttribute, err := impl.CreateGitOpsRepoAndPushChart(installAppVersionRequest, manifestResponse.ChartResponse.BuiltChartPath)
+	chartGitAttribute, isNew, githash, err := impl.CreateGitOpsRepoAndPushChart(installAppVersionRequest, manifestResponse.ChartResponse.BuiltChartPath, manifestResponse.RequirementsConfig, manifestResponse.ValuesConfig)
 	if err != nil {
 		impl.logger.Errorw("Error in pushing chart to git", "err", err)
 		return appStoreGitOpsResponse, err
 	}
-	// step-2 commit dependencies and values in git
-	_, err = impl.CommitConfigToGit(manifestResponse.RequirementsConfig)
-	if err != nil {
-		impl.logger.Errorw("error in committing dependency config to git", "err", err)
-		return appStoreGitOpsResponse, err
-	}
 	space := regexp.MustCompile(`\s+`)
 	appStoreName := space.ReplaceAllString(installAppVersionRequest.AppName, "-")
 	clonedDir := impl.gitFactory.GitWorkingDir + "" + appStoreName
-	err = impl.chartTemplateService.GitPull(clonedDir, chartGitAttribute.RepoUrl, appStoreName)
-	if err != nil {
-		impl.logger.Errorw("error in git pull", "err", err)
-		return appStoreGitOpsResponse, err
-	}
-	githash, err := impl.CommitConfigToGit(manifestResponse.ValuesConfig)
-	if err != nil {
-		impl.logger.Errorw("error in committing values config to git", "err", err)
-		return appStoreGitOpsResponse, err
-	}
-	err = impl.chartTemplateService.GitPull(clonedDir, chartGitAttribute.RepoUrl, appStoreName)
-	if err != nil {
-		impl.logger.Errorw("error in git pull", "err", err)
-		return appStoreGitOpsResponse, err
+
+	// Checking this is the first time chart has been pushed , if yes requirements.yaml has been already pushed with chart as there was sync-delay with github api.
+	// step-2 commit dependencies and values in git
+	if !isNew {
+		_, err = impl.CommitConfigToGit(manifestResponse.RequirementsConfig)
+		if err != nil {
+			impl.logger.Errorw("error in committing dependency config to git", "err", err)
+			return appStoreGitOpsResponse, err
+		}
+		err = impl.chartTemplateService.GitPull(clonedDir, chartGitAttribute.RepoUrl, appStoreName)
+		if err != nil {
+			impl.logger.Errorw("error in git pull", "err", err)
+			return appStoreGitOpsResponse, err
+		}
+
+		githash, err = impl.CommitConfigToGit(manifestResponse.ValuesConfig)
+		if err != nil {
+			impl.logger.Errorw("error in committing values config to git", "err", err)
+			return appStoreGitOpsResponse, err
+		}
+		err = impl.chartTemplateService.GitPull(clonedDir, chartGitAttribute.RepoUrl, appStoreName)
+		if err != nil {
+			impl.logger.Errorw("error in git pull", "err", err)
+			return appStoreGitOpsResponse, err
+		}
 	}
 	appStoreGitOpsResponse.ChartGitAttribute = chartGitAttribute
 	appStoreGitOpsResponse.GitHash = githash

pkg/appStore/util/util.go

@@ -0,0 +1,11 @@
+package util
+
+import "os"
+
+func MoveFileToDestination(filePath, destinationPath string) error {
+	err := os.Rename(filePath, destinationPath)
+	if err != nil {
+		return err
+	}
+	return nil
+}

pkg/user/UserService.go

@@ -36,9 +36,15 @@ import (
 	"go.uber.org/zap"
 	"net/http"
 	"strings"
+	"sync"
 	"time"
 )
 
+const (
+	ConcurrentRequestLockError   = "there is an ongoing request for this user, please try after some time"
+	ConcurrentRequestUnlockError = "cannot block request that is not in process"
+)
+
 type UserService interface {
 	CreateUser(userInfo *bean.UserInfo, token string, managerAuth func(resource, token string, object string) bool) ([]*bean.UserInfo, error)
 	SelfRegisterUserIfNotExists(userInfo *bean.UserInfo) ([]*bean.UserInfo, error)
@@ -62,6 +68,10 @@ type UserService interface {
 }
 
 type UserServiceImpl struct {
+	userReqLock sync.RWMutex
+	//map of userId and current lock-state of their serving ability;
+	//if TRUE then it means that some request is ongoing & unable to serve and FALSE then it is open to serve
+	userReqState        map[int32]bool
 	userAuthRepository  repository2.UserAuthRepository
 	logger              *zap.SugaredLogger
 	userRepository      repository2.UserRepository
@@ -77,6 +87,7 @@ func NewUserServiceImpl(userAuthRepository repository2.UserAuthRepository,
 	userGroupRepository repository2.RoleGroupRepository,
 	sessionManager2 *middleware.SessionManager, userCommonService UserCommonService, userAuditService UserAuditService) *UserServiceImpl {
 	serviceImpl := &UserServiceImpl{
+		userReqState:        make(map[int32]bool),
 		userAuthRepository:  userAuthRepository,
 		logger:              logger,
 		userRepository:      userRepository,
@@ -89,6 +100,36 @@ func NewUserServiceImpl(userAuthRepository repository2.UserAuthRepository,
 	return serviceImpl
 }
 
+func (impl *UserServiceImpl) getUserReqLockStateById(userId int32) bool {
+	defer impl.userReqLock.RUnlock()
+	impl.userReqLock.RLock()
+	return impl.userReqState[userId]
+}
+
+// FreeUnfreeUserReqState - free sets the userId free for serving, meaning removing the lock(removing entry). Unfree locks the user for other requests
+func (impl *UserServiceImpl) lockUnlockUserReqState(userId int32, lock bool) error {
+	var err error
+	defer impl.userReqLock.Unlock()
+	impl.userReqLock.Lock()
+	if lock {
+		//checking again if someone changed or not
+		if !impl.userReqState[userId] {
+			//available to serve, locking
+			impl.userReqState[userId] = true
+		} else {
+			err = &util.ApiError{Code: "409", HttpStatusCode: http.StatusConflict, UserMessage: ConcurrentRequestLockError}
+		}
+	} else {
+		if impl.userReqState[userId] {
+			//in serving state, unlocking
+			delete(impl.userReqState, userId)
+		} else {
+			err = &util.ApiError{Code: "409", HttpStatusCode: http.StatusConflict, UserMessage: ConcurrentRequestUnlockError}
+		}
+	}
+	return err
+}
+
 func (impl *UserServiceImpl) validateUserRequest(userInfo *bean.UserInfo) (bool, error) {
 	if len(userInfo.RoleFilters) == 1 &&
 		userInfo.RoleFilters[0].Team == "" && userInfo.RoleFilters[0].Environment == "" && userInfo.RoleFilters[0].Action == "" {
@@ -617,6 +658,23 @@ func (impl *UserServiceImpl) mergeGroups(oldGroups []string, newGroups []string)
 }
 
 func (impl *UserServiceImpl) UpdateUser(userInfo *bean.UserInfo, token string, managerAuth func(resource, token string, object string) bool) (*bean.UserInfo, bool, bool, []string, error) {
+	//checking if request for same user is being processed
+	isLocked := impl.getUserReqLockStateById(userInfo.Id)
+	if isLocked {
+		impl.logger.Errorw("received concurrent request for user update, UpdateUser", "userId", userInfo.Id)
+		return nil, false, false, nil, &util.ApiError{
+			Code:           "409",
+			HttpStatusCode: http.StatusConflict,
+			UserMessage:    ConcurrentRequestLockError,
+		}
+	} else {
+		//locking state for this user since it's ready to serve
+		err := impl.lockUnlockUserReqState(userInfo.Id, true)
+		if err != nil {
+			impl.logger.Errorw("error in locking, lockUnlockUserReqState", "userId", userInfo.Id)
+			return nil, false, false, nil, err
+		}
+	}
 	//validating if action user is not admin and trying to update user who has super admin polices, return 403
 	isUserSuperAdmin, err := impl.IsSuperAdmin(int(userInfo.Id))
 	if err != nil {
@@ -802,6 +860,12 @@ func (impl *UserServiceImpl) UpdateUser(userInfo *bean.UserInfo, token string, m
 	//loading policy for syncing orchestrator to casbin with newly added policies
 	casbin2.LoadPolicy()
 
+	err = impl.lockUnlockUserReqState(userInfo.Id, false)
+	if err != nil {
+		impl.logger.Errorw("error in unlocking, lockUnlockUserReqState", "userId", userInfo.Id)
+		return nil, false, false, nil, err
+	}
+
 	return userInfo, rolesChanged, groupsModified, restrictedGroups, nil
 }