From 779e913181fc5636193bc762b5ec24faada4870c Mon Sep 17 00:00:00 2001 From: Prakash Date: Thu, 15 Aug 2024 23:23:54 +0530 Subject: [PATCH 1/9] migration number changes (#5692) --- ...an_plugin.down.sql => 270_improved_image_scan_plugin.down.sql} | 0 ...e_scan_plugin.up.sql => 270_improved_image_scan_plugin.up.sql} | 0 ...rent_metadata.down.sql => 271_plugin_parent_metadata.down.sql} | 0 ...n_parent_metadata.up.sql => 271_plugin_parent_metadata.up.sql} | 0 ...lugin_metadata.down.sql => 272_alter_plugin_metadata.down.sql} | 0 ...er_plugin_metadata.up.sql => 272_alter_plugin_metadata.up.sql} | 0 ...ls_support_in_git.down.sql => 273_tls_support_in_git.down.sql} | 0 ...72_tls_support_in_git.up.sql => 273_tls_support_in_git.up.sql} | 0 ..._system_controller.down.sql => 274_system_controller.down.sql} | 0 ...{273_system_controller.up.sql => 274_system_controller.up.sql} | 0 10 files changed, 0 insertions(+), 0 deletions(-) rename scripts/sql/{274_improved_image_scan_plugin.down.sql => 270_improved_image_scan_plugin.down.sql} (100%) rename scripts/sql/{274_improved_image_scan_plugin.up.sql => 270_improved_image_scan_plugin.up.sql} (100%) rename scripts/sql/{270_plugin_parent_metadata.down.sql => 271_plugin_parent_metadata.down.sql} (100%) rename scripts/sql/{270_plugin_parent_metadata.up.sql => 271_plugin_parent_metadata.up.sql} (100%) rename scripts/sql/{271_alter_plugin_metadata.down.sql => 272_alter_plugin_metadata.down.sql} (100%) rename scripts/sql/{271_alter_plugin_metadata.up.sql => 272_alter_plugin_metadata.up.sql} (100%) rename scripts/sql/{272_tls_support_in_git.down.sql => 273_tls_support_in_git.down.sql} (100%) rename scripts/sql/{272_tls_support_in_git.up.sql => 273_tls_support_in_git.up.sql} (100%) rename scripts/sql/{273_system_controller.down.sql => 274_system_controller.down.sql} (100%) rename scripts/sql/{273_system_controller.up.sql => 274_system_controller.up.sql} (100%) diff --git a/scripts/sql/274_improved_image_scan_plugin.down.sql b/scripts/sql/270_improved_image_scan_plugin.down.sql similarity index 100% rename from scripts/sql/274_improved_image_scan_plugin.down.sql rename to scripts/sql/270_improved_image_scan_plugin.down.sql diff --git a/scripts/sql/274_improved_image_scan_plugin.up.sql b/scripts/sql/270_improved_image_scan_plugin.up.sql similarity index 100% rename from scripts/sql/274_improved_image_scan_plugin.up.sql rename to scripts/sql/270_improved_image_scan_plugin.up.sql diff --git a/scripts/sql/270_plugin_parent_metadata.down.sql b/scripts/sql/271_plugin_parent_metadata.down.sql similarity index 100% rename from scripts/sql/270_plugin_parent_metadata.down.sql rename to scripts/sql/271_plugin_parent_metadata.down.sql diff --git a/scripts/sql/270_plugin_parent_metadata.up.sql b/scripts/sql/271_plugin_parent_metadata.up.sql similarity index 100% rename from scripts/sql/270_plugin_parent_metadata.up.sql rename to scripts/sql/271_plugin_parent_metadata.up.sql diff --git a/scripts/sql/271_alter_plugin_metadata.down.sql b/scripts/sql/272_alter_plugin_metadata.down.sql similarity index 100% rename from scripts/sql/271_alter_plugin_metadata.down.sql rename to scripts/sql/272_alter_plugin_metadata.down.sql diff --git a/scripts/sql/271_alter_plugin_metadata.up.sql b/scripts/sql/272_alter_plugin_metadata.up.sql similarity index 100% rename from scripts/sql/271_alter_plugin_metadata.up.sql rename to scripts/sql/272_alter_plugin_metadata.up.sql diff --git a/scripts/sql/272_tls_support_in_git.down.sql b/scripts/sql/273_tls_support_in_git.down.sql similarity index 100% rename from scripts/sql/272_tls_support_in_git.down.sql rename to scripts/sql/273_tls_support_in_git.down.sql diff --git a/scripts/sql/272_tls_support_in_git.up.sql b/scripts/sql/273_tls_support_in_git.up.sql similarity index 100% rename from scripts/sql/272_tls_support_in_git.up.sql rename to scripts/sql/273_tls_support_in_git.up.sql diff --git a/scripts/sql/273_system_controller.down.sql b/scripts/sql/274_system_controller.down.sql similarity index 100% rename from scripts/sql/273_system_controller.down.sql rename to scripts/sql/274_system_controller.down.sql diff --git a/scripts/sql/273_system_controller.up.sql b/scripts/sql/274_system_controller.up.sql similarity index 100% rename from scripts/sql/273_system_controller.up.sql rename to scripts/sql/274_system_controller.up.sql From 4f04d6b5eee794f768f025cf58dea0e9ce5f80f0 Mon Sep 17 00:00:00 2001 From: Prakash Date: Tue, 20 Aug 2024 12:35:33 +0530 Subject: [PATCH 2/9] refrain from checkin autoscalingCheckBeforeTrigger for virt clus (#5696) --- pkg/deployment/manifest/ManifestCreationService.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/deployment/manifest/ManifestCreationService.go b/pkg/deployment/manifest/ManifestCreationService.go index 86641dfeac..7547e06729 100644 --- a/pkg/deployment/manifest/ManifestCreationService.go +++ b/pkg/deployment/manifest/ManifestCreationService.go @@ -275,10 +275,12 @@ func (impl *ManifestCreationServiceImpl) GetValuesOverrideForTrigger(overrideReq // error is not returned as it's not blocking for deployment process // blocking deployments based on this use case can vary for user to user } - mergedValues, err = impl.autoscalingCheckBeforeTrigger(newCtx, appName, envOverride.Namespace, mergedValues, overrideRequest) - if err != nil { - impl.logger.Errorw("error in autoscaling check before trigger", "pipelineId", overrideRequest.PipelineId, "err", err) - return valuesOverrideResponse, err + if !envOverride.Environment.IsVirtualEnvironment { + mergedValues, err = impl.autoscalingCheckBeforeTrigger(newCtx, appName, envOverride.Namespace, mergedValues, overrideRequest) + if err != nil { + impl.logger.Errorw("error in autoscaling check before trigger", "pipelineId", overrideRequest.PipelineId, "err", err) + return valuesOverrideResponse, err + } } // handle image pull secret if access given mergedValues, err = impl.dockerRegistryIpsConfigService.HandleImagePullSecretOnApplicationDeployment(newCtx, envOverride.Environment, artifact, pipeline.CiPipelineId, mergedValues) From 2e58e77959a458671d0b70a7bab4041a9f5c4894 Mon Sep 17 00:00:00 2001 From: Prakash Date: Tue, 20 Aug 2024 12:50:03 +0530 Subject: [PATCH 3/9] fix: Decode secret fix on add update oss (#5695) * ValidateEncodedDataByDecoding in case add or update secret * wire fix from main * minor refactor * comment --- cmd/external-app/wire_gen.go | 4 +-- pkg/pipeline/ConfigMapService.go | 42 ++++++++++++++++++++++++++------ util/encoding-utils.go | 10 ++++++++ wire_gen.go | 2 +- 4 files changed, 48 insertions(+), 10 deletions(-) diff --git a/cmd/external-app/wire_gen.go b/cmd/external-app/wire_gen.go index c3a4512ff8..72533cab86 100644 --- a/cmd/external-app/wire_gen.go +++ b/cmd/external-app/wire_gen.go @@ -199,8 +199,8 @@ func InitializeApp() (*App, error) { userAuthServiceImpl := user.NewUserAuthServiceImpl(userAuthRepositoryImpl, sessionManager, loginService, sugaredLogger, userRepositoryImpl, roleGroupRepositoryImpl, userServiceImpl) teamServiceImpl := team.NewTeamServiceImpl(sugaredLogger, teamRepositoryImpl, userAuthServiceImpl) clusterRepositoryImpl := repository2.NewClusterRepositoryImpl(db, sugaredLogger) - v := informer.NewGlobalMapClusterNamespace() - k8sInformerFactoryImpl := informer.NewK8sInformerFactoryImpl(sugaredLogger, v, k8sServiceImpl) + syncMap := informer.NewGlobalMapClusterNamespace() + k8sInformerFactoryImpl := informer.NewK8sInformerFactoryImpl(sugaredLogger, syncMap, k8sServiceImpl) clusterServiceImpl := cluster.NewClusterServiceImpl(clusterRepositoryImpl, sugaredLogger, k8sServiceImpl, k8sInformerFactoryImpl, userAuthRepositoryImpl, userRepositoryImpl, roleGroupRepositoryImpl) appStatusRepositoryImpl := appStatus.NewAppStatusRepositoryImpl(db, sugaredLogger) environmentRepositoryImpl := repository2.NewEnvironmentRepositoryImpl(db, sugaredLogger, appStatusRepositoryImpl) diff --git a/pkg/pipeline/ConfigMapService.go b/pkg/pipeline/ConfigMapService.go index 77e996dcce..49c08772a1 100644 --- a/pkg/pipeline/ConfigMapService.go +++ b/pkg/pipeline/ConfigMapService.go @@ -34,7 +34,9 @@ import ( util2 "github.com/devtron-labs/devtron/util" "github.com/go-pg/pg" "go.uber.org/zap" + "net/http" "regexp" + "strconv" "time" ) @@ -504,12 +506,19 @@ func (impl ConfigMapServiceImpl) CSGlobalAddUpdate(configMapRequest *bean.Config return nil, fmt.Errorf("invalid request multiple config found for add or update") } configData := configMapRequest.ConfigData[0] + // validating config/secret data at service layer since this func is consumed in multiple flows, hence preventing code duplication valid, err := impl.validateConfigData(configData) if err != nil && !valid { impl.logger.Errorw("error in validating", "error", err) return configMapRequest, err } + valid, err = impl.validateConfigDataForSecretsOnly(configData) + if err != nil && !valid { + impl.logger.Errorw("error in validating secrets only data", "error", err) + return configMapRequest, err + } + valid, err = impl.validateExternalSecretChartCompatibility(configMapRequest.AppId, configMapRequest.EnvironmentId, configData) if err != nil && !valid { impl.logger.Errorw("error in validating", "error", err) @@ -704,11 +713,17 @@ func (impl ConfigMapServiceImpl) CSEnvironmentAddUpdate(configMapRequest *bean.C } configData := configMapRequest.ConfigData[0] + // validating config/secret data at service layer since this func is consumed in multiple flows, hence preventing code duplication valid, err := impl.validateConfigData(configData) if err != nil && !valid { impl.logger.Errorw("error in validating", "error", err) return configMapRequest, err } + valid, err = impl.validateConfigDataForSecretsOnly(configData) + if err != nil && !valid { + impl.logger.Errorw("error in validating secrets only data", "error", err) + return configMapRequest, err + } valid, err = impl.validateExternalSecretChartCompatibility(configMapRequest.AppId, configMapRequest.EnvironmentId, configData) if err != nil && !valid { @@ -795,13 +810,6 @@ func (impl ConfigMapServiceImpl) CSEnvironmentAddUpdate(configMapRequest *bean.C } configMapRequest.Id = configMap.Id } - //VARIABLE_MAPPING_UPDATE - //sl := bean.SecretsList{} - //data, err := sl.GetTransformedDataForSecretList(model.SecretData, util2.DecodeSecret) - //if err != nil { - // return nil, err - //} - //err = impl.extractAndMapVariables(data, model.Id, repository5.EntityTypeSecretEnvLevel, configMapRequest.UserId) err = impl.scopedVariableManager.CreateVariableMappingsForSecretEnv(model) if err != nil { return nil, err @@ -1545,6 +1553,26 @@ func (impl ConfigMapServiceImpl) validateConfigData(configData *bean.ConfigData) return true, nil } +func (impl ConfigMapServiceImpl) validateConfigDataForSecretsOnly(configData *bean.ConfigData) (bool, error) { + + // check encoding in base64 for secret data + if len(configData.Data) > 0 { + dataMap := make(map[string]string) + err := json.Unmarshal(configData.Data, &dataMap) + if err != nil { + impl.logger.Errorw("error while unmarshalling secret data ", "error", err) + return false, err + } + err = util2.ValidateEncodedDataByDecoding(dataMap) + if err != nil { + impl.logger.Errorw("error in decoding secret data", "error", err) + return false, util.NewApiError().WithHttpStatusCode(http.StatusUnprocessableEntity).WithCode(strconv.Itoa(http.StatusUnprocessableEntity)). + WithUserMessage("error in decoding data, make sure the secret data is encoded properly") + } + } + return true, nil +} + func (impl ConfigMapServiceImpl) updateConfigData(configData *bean.ConfigData, syncRequest *bean.BulkPatchRequest) (*bean.ConfigData, error) { dataMap := make(map[string]string) var updatedData json.RawMessage diff --git a/util/encoding-utils.go b/util/encoding-utils.go index 88064a26bd..82837c229c 100644 --- a/util/encoding-utils.go +++ b/util/encoding-utils.go @@ -53,3 +53,13 @@ func GetDecodedAndEncodedData(data json.RawMessage, transformer SecretTransformM } return marshal, nil } + +func ValidateEncodedDataByDecoding(dataMap map[string]string) error { + for _, value := range dataMap { + _, err := base64.StdEncoding.DecodeString(value) + if err != nil { + return err + } + } + return nil +} diff --git a/wire_gen.go b/wire_gen.go index b1ef4f1028..d83a360fd0 100644 --- a/wire_gen.go +++ b/wire_gen.go @@ -714,7 +714,7 @@ func InitializeApp() (*App, error) { if err != nil { return nil, err } - installedAppResourceServiceImpl := resource.NewInstalledAppResourceServiceImpl(sugaredLogger, installedAppRepositoryImpl, appStoreApplicationVersionRepositoryImpl, applicationServiceClientImpl, acdAuthConfig, installedAppVersionHistoryRepositoryImpl, argoUserServiceImpl, helmAppClientImpl, helmAppServiceImpl, appStatusServiceImpl, k8sCommonServiceImpl, k8sApplicationServiceImpl, k8sServiceImpl, deploymentConfigServiceImpl) + installedAppResourceServiceImpl := resource.NewInstalledAppResourceServiceImpl(sugaredLogger, installedAppRepositoryImpl, appStoreApplicationVersionRepositoryImpl, applicationServiceClientImpl, acdAuthConfig, installedAppVersionHistoryRepositoryImpl, argoUserServiceImpl, helmAppClientImpl, helmAppServiceImpl, appStatusServiceImpl, k8sCommonServiceImpl, k8sApplicationServiceImpl, k8sServiceImpl, deploymentConfigServiceImpl, ociRegistryConfigRepositoryImpl) chartGroupEntriesRepositoryImpl := repository17.NewChartGroupEntriesRepositoryImpl(db, sugaredLogger) chartGroupReposotoryImpl := repository17.NewChartGroupReposotoryImpl(db, sugaredLogger) chartGroupDeploymentRepositoryImpl := repository17.NewChartGroupDeploymentRepositoryImpl(db, sugaredLogger) From bf2351544aa57137980cbc3b6561b88d21e359b6 Mon Sep 17 00:00:00 2001 From: Prakash Date: Tue, 20 Aug 2024 12:55:08 +0530 Subject: [PATCH 4/9] saving pco concurrency case handled (#5688) --- .../sql/repository/chartConfig/PipelineOverrideRepository.go | 5 +++++ pkg/deployment/manifest/ManifestCreationService.go | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/sql/repository/chartConfig/PipelineOverrideRepository.go b/internal/sql/repository/chartConfig/PipelineOverrideRepository.go index aba3d7554c..db8476d913 100644 --- a/internal/sql/repository/chartConfig/PipelineOverrideRepository.go +++ b/internal/sql/repository/chartConfig/PipelineOverrideRepository.go @@ -58,6 +58,7 @@ type PipelineConfigOverrideMetadata struct { type PipelineOverrideRepository interface { Save(*PipelineOverride) error + Update(pipelineOverride *PipelineOverride) error UpdateStatusByRequestIdentifier(requestId string, newStatus models.ChartStatus) (int, error) GetLatestConfigByRequestIdentifier(requestIdentifier string) (pipelineOverride *PipelineOverride, err error) GetLatestConfigByEnvironmentConfigOverrideId(envConfigOverrideId int) (pipelineOverride *PipelineOverride, err error) @@ -85,6 +86,10 @@ func (impl PipelineOverrideRepositoryImpl) Save(pipelineOverride *PipelineOverri return impl.dbConnection.Insert(pipelineOverride) } +func (impl PipelineOverrideRepositoryImpl) Update(pipelineOverride *PipelineOverride) error { + return impl.dbConnection.Update(pipelineOverride) +} + func (impl PipelineOverrideRepositoryImpl) UpdatePipelineMergedValues(ctx context.Context, tx *pg.Tx, id int, pipelineMergedValues string, userId int32) error { _, span := otel.Tracer("orchestrator").Start(ctx, "PipelineOverrideRepositoryImpl.UpdatePipelineMergedValues") defer span.End() diff --git a/pkg/deployment/manifest/ManifestCreationService.go b/pkg/deployment/manifest/ManifestCreationService.go index 7547e06729..9b2cc19927 100644 --- a/pkg/deployment/manifest/ManifestCreationService.go +++ b/pkg/deployment/manifest/ManifestCreationService.go @@ -808,7 +808,7 @@ func (impl *ManifestCreationServiceImpl) checkAndFixDuplicateReleaseNo(override return err } override.PipelineReleaseCounter = currentReleaseNo + 1 - err = impl.pipelineOverrideRepository.Save(override) + err = impl.pipelineOverrideRepository.Update(override) if err != nil { return err } From 694831c209a2a28883b299cb8c073ef2441dddca Mon Sep 17 00:00:00 2001 From: Prakash Date: Wed, 21 Aug 2024 13:30:04 +0530 Subject: [PATCH 5/9] fix: script for pipelineStageStepVariable, making input value and default_value text from varchar255 (#5701) * script for pipelineStageStepVariable, making input value and default_value text from varchar255 * erro log fix --- pkg/deployment/gitOps/git/GitOpsHelper.go | 1 + pkg/deployment/gitOps/git/GitServiceGithub.go | 1 + pkg/deployment/gitOps/git/commandManager/GitCliManager.go | 3 +++ pkg/deployment/gitOps/git/commandManager/GoGitSdkManager.go | 3 +++ scripts/sql/276_alter_pipeline_stage_step_variable.down.sql | 3 +++ scripts/sql/276_alter_pipeline_stage_step_variable.up.sql | 2 ++ 6 files changed, 13 insertions(+) create mode 100644 scripts/sql/276_alter_pipeline_stage_step_variable.down.sql create mode 100644 scripts/sql/276_alter_pipeline_stage_step_variable.up.sql diff --git a/pkg/deployment/gitOps/git/GitOpsHelper.go b/pkg/deployment/gitOps/git/GitOpsHelper.go index 2d204ef8a0..bc7add2ff9 100644 --- a/pkg/deployment/gitOps/git/GitOpsHelper.go +++ b/pkg/deployment/gitOps/git/GitOpsHelper.go @@ -89,6 +89,7 @@ func (impl *GitOpsHelper) Clone(url, targetDir string) (clonedDir string, err er } } if errMsg != "" { + impl.logger.Errorw("error in git fetch command", "errMsg", errMsg, "err", err) return "", fmt.Errorf(errMsg) } return clonedDir, nil diff --git a/pkg/deployment/gitOps/git/GitServiceGithub.go b/pkg/deployment/gitOps/git/GitServiceGithub.go index 589d79c2ac..b48d8b5ab4 100644 --- a/pkg/deployment/gitOps/git/GitServiceGithub.go +++ b/pkg/deployment/gitOps/git/GitServiceGithub.go @@ -259,6 +259,7 @@ func (impl GitHubClient) GetRepoUrl(config *bean2.GitOpsConfigDto) (repoUrl stri ctx := context.Background() repo, _, err := impl.client.Repositories.Get(ctx, impl.org, config.GitRepoName) if err != nil { + impl.logger.Errorw("error in getting repo url by repo name", "org", impl.org, "gitRepoName", config.GitRepoName, "err", err) return "", err } return *repo.CloneURL, nil diff --git a/pkg/deployment/gitOps/git/commandManager/GitCliManager.go b/pkg/deployment/gitOps/git/commandManager/GitCliManager.go index 0501f67cb3..b5b3a3a146 100644 --- a/pkg/deployment/gitOps/git/commandManager/GitCliManager.go +++ b/pkg/deployment/gitOps/git/commandManager/GitCliManager.go @@ -77,6 +77,9 @@ func (impl *GitCliManagerImpl) Pull(ctx GitContext, repoRoot string) (err error) return err } response, errMsg, err := impl.PullCli(ctx, repoRoot, "origin/master") + if err != nil { + impl.logger.Errorw("error in git pull from cli", "errMsg", errMsg, "err", err) + } if strings.Contains(response, "already up-to-date") || strings.Contains(errMsg, "already up-to-date") { err = nil diff --git a/pkg/deployment/gitOps/git/commandManager/GoGitSdkManager.go b/pkg/deployment/gitOps/git/commandManager/GoGitSdkManager.go index 950a0bf451..3d70a73c31 100644 --- a/pkg/deployment/gitOps/git/commandManager/GoGitSdkManager.go +++ b/pkg/deployment/gitOps/git/commandManager/GoGitSdkManager.go @@ -56,6 +56,9 @@ func (impl GoGitSDKManagerImpl) Pull(ctx GitContext, repoRoot string) (err error } err = workTree.PullContext(ctx, pullOptions) + if err != nil { + impl.logger.Errorw("error in git pull from go-git", "err", err) + } if err != nil && err.Error() == "already up-to-date" { err = nil return nil diff --git a/scripts/sql/276_alter_pipeline_stage_step_variable.down.sql b/scripts/sql/276_alter_pipeline_stage_step_variable.down.sql new file mode 100644 index 0000000000..bad0b4c492 --- /dev/null +++ b/scripts/sql/276_alter_pipeline_stage_step_variable.down.sql @@ -0,0 +1,3 @@ +ALTER TABLE pipeline_stage_step_variable ALTER COLUMN default_value TYPE VARCHAR(255); +ALTER TABLE pipeline_stage_step_variable ALTER COLUMN value TYPE VARCHAR(255); + diff --git a/scripts/sql/276_alter_pipeline_stage_step_variable.up.sql b/scripts/sql/276_alter_pipeline_stage_step_variable.up.sql new file mode 100644 index 0000000000..cbcf6515c9 --- /dev/null +++ b/scripts/sql/276_alter_pipeline_stage_step_variable.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE pipeline_stage_step_variable ALTER COLUMN value TYPE text; +ALTER TABLE pipeline_stage_step_variable ALTER COLUMN default_value TYPE text; From 3e31f49f95d373f92b13afbe1806606ac4a39d85 Mon Sep 17 00:00:00 2001 From: Rajeev Ranjan <90333766+RajeevRanjan27@users.noreply.github.com> Date: Wed, 21 Aug 2024 17:09:24 +0530 Subject: [PATCH 6/9] fix: ea fixes for helm app (#5708) * added the ea apps entry app table * resolved the ea mode multiple rows error during configuration of app * modified the ea dockerfile in ca-certificates cmd * uncommented the code and left the ea helm app making way untouched * remodified the dockerfile as previous state * modified the docker file ea mode * dockerfile exit code 100 due to ap install alternative in ea mode dockerfile * execute make after main merge * modified changes in dockerfile ea mode * resolved comments after first level review --- pkg/app/AppCrudOperationService.go | 78 +++++++++++++++++-- pkg/app/helper.go | 23 +++++- .../service/AppStoreDeploymentDBService.go | 1 - .../service/EAMode/InstalledAppDBService.go | 54 +++++++++++++ 4 files changed, 149 insertions(+), 7 deletions(-) diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index a7035d7603..3c62046235 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -19,6 +19,7 @@ package app import ( "context" "encoding/json" + "errors" "fmt" "github.com/caarlos0/env" client "github.com/devtron-labs/devtron/api/helm-app/service" @@ -457,13 +458,67 @@ func convertUrlToHttpsIfSshType(url string) string { return httpsURL } +// handleDuplicateAppEntries identifies and resolves duplicate app entries based on creation time. +// It marks the most recent duplicate entry as inactive and updates the corresponding installed app. +func (impl AppCrudOperationServiceImpl) handleDuplicateAppEntries(appNameUniqueIdentifier string) (*appRepository.App, error) { + // Fetch app IDs by name + appIds, err := impl.getAppIdsByName(appNameUniqueIdentifier) + if err != nil { + impl.logger.Errorw("error in fetching app Ids by appIdentifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) + return nil, err + } + + // Fetch apps by IDs from App table for duplicated entries + apps, err := impl.appRepository.FindByIds(appIds) + if err != nil || errors.Is(err, pg.ErrNoRows) { + impl.logger.Errorw("error in fetching app List by appIds", "appIds", appIds, "err", err) + return nil, err + } + + // Identify the earliest and duplicated app entries + earliestApp, duplicatedApp := identifyDuplicateApps(apps) + + // Fetch the installed app associated with the duplicated app + installedApp, err := impl.installedAppRepository.GetInstalledAppsByAppId(duplicatedApp.Id) + if err != nil { + impl.logger.Errorw("error in fetching installed app by appId", "appId", duplicatedApp.Id, "err", err) + return nil, err + } + // Update duplicated app entries + err = impl.installedAppDbService.UpdateDuplicatedEntriesInAppAndInstalledApps(earliestApp, duplicatedApp, &installedApp) + if err != nil { + impl.logger.Errorw("error in updating duplicated entries", "earliestApp", earliestApp, "duplicatedApp", duplicatedApp, "err", err) + return nil, err + } + + impl.logger.Debug("Successfully resolved duplicate app entries", "earliestApp", earliestApp, "duplicatedApp", duplicatedApp) + return earliestApp, nil + +} + +// getAppIdsByName fetches app IDs by the app name unique identifier [for duplicated active app] +func (impl AppCrudOperationServiceImpl) getAppIdsByName(appNameUniqueIdentifier string) ([]*int, error) { + slice := []string{appNameUniqueIdentifier} + appIds, err := impl.appRepository.FindIdsByNames(slice) + if err != nil { + return nil, err + } + + // Convert each element to a pointer and store in a slice of pointers + ids := make([]*int, len(appIds)) + for i := range appIds { + ids[i] = &appIds[i] + } + return ids, 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 *helmBean.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 { + if err != nil && !errors.Is(err, pg.ErrNoRows) && !errors.Is(err, pg.ErrMultiRows) { impl.logger.Errorw("error in fetching app meta data by unique app identifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) return app, err } @@ -475,6 +530,14 @@ func (impl AppCrudOperationServiceImpl) getAppAndProjectForAppIdentifier(appIden return app, err } } + if errors.Is(err, pg.ErrMultiRows) { + + app, err = impl.handleDuplicateAppEntries(appNameUniqueIdentifier) + if err != nil { + impl.logger.Errorw("error in handling Duplicate entries in the app", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) + return app, err + } + } return app, nil } @@ -532,17 +595,17 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. return nil, err } // 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 { + if app != nil && app.Id > 0 && len(app.DisplayName) == 0 { 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 } } - if app.Id == 0 { + if app != nil && app.Id == 0 { app.AppName = appIdDecoded.ReleaseName } - if util2.IsExternalChartStoreApp(app.DisplayName) { + if app != nil && util2.IsExternalChartStoreApp(app.DisplayName) { displayName = app.DisplayName } @@ -568,9 +631,14 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. displayName = InstalledApp.App.DisplayName } } + // Safeguard against nil app cases + if app == nil { + impl.logger.Errorw("no rows found for the requested app", "appId", appId, "error", err) + return nil, fmt.Errorf("no rows found for the requested app, %q", pg.ErrNoRows) + } user, err := impl.userRepository.GetByIdIncludeDeleted(app.CreatedBy) - if err != nil && err != pg.ErrNoRows { + if err != nil && !errors.Is(err, pg.ErrNoRows) { impl.logger.Errorw("error in fetching user for app meta info", "error", err) return nil, err } diff --git a/pkg/app/helper.go b/pkg/app/helper.go index 2ca99c6b14..0e8fff65a4 100644 --- a/pkg/app/helper.go +++ b/pkg/app/helper.go @@ -16,7 +16,10 @@ package app -import "strings" +import ( + appRepository "github.com/devtron-labs/devtron/internal/sql/repository/app" + "strings" +) // LabelMatchingRegex is the official k8s label matching regex, pls refer https://github.com/kubernetes/apimachinery/blob/bfd2aff97e594f6aad77acbe2cbbe190acc93cbc/pkg/util/validation/validation.go#L167 const LabelMatchingRegex = "^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$" @@ -43,3 +46,21 @@ func sanitizeLabels(extraAppLabels map[string]string) map[string]string { } return extraAppLabels } + +// identifyDuplicateApps identifies the earliest created app and the most recent duplicate app. +func identifyDuplicateApps(apps []*appRepository.App) (earliestApp *appRepository.App, duplicatedApp *appRepository.App) { + if len(apps) == 0 { + return nil, nil + } + earliestApp = apps[0] + duplicatedApp = apps[0] + for _, app := range apps[1:] { + if app.AuditLog.CreatedOn.Before(earliestApp.AuditLog.CreatedOn) { + earliestApp = app + } + if app.AuditLog.CreatedOn.After(duplicatedApp.AuditLog.CreatedOn) { + duplicatedApp = app + } + } + return earliestApp, duplicatedApp +} diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go index 33f1a7b9a1..53d9b8b5e8 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go @@ -133,7 +133,6 @@ func (impl *AppStoreDeploymentDBServiceImpl) AppStoreDeployOperationDB(installRe } // setting additional env data required in appStoreBean.InstallAppVersionDTO adapter.UpdateAdditionalEnvDetails(installRequest, environment) - impl.appStoreValidator.Validate(installRequest, environment) // Stage 1: Create App in tx (Only if AppId is not set already) diff --git a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go index 6eb4156e24..810f3c3b8c 100644 --- a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go +++ b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go @@ -58,6 +58,7 @@ type InstalledAppDBService interface { GetReleaseInfo(appIdentifier *helmBean.AppIdentifier) (*appStoreBean.InstallAppVersionDTO, error) IsExternalAppLinkedToChartStore(appId int) (bool, []*appStoreRepo.InstalledApps, error) CreateNewAppEntryForAllInstalledApps(installedApps []*appStoreRepo.InstalledApps) error + UpdateDuplicatedEntriesInAppAndInstalledApps(earlyApp *app.App, duplicatedApp *app.App, installedApp *appStoreRepo.InstalledApps) error } type InstalledAppDBServiceImpl struct { @@ -399,6 +400,17 @@ func (impl *InstalledAppDBServiceImpl) CreateNewAppEntryForAllInstalledApps(inst // Rollback tx on error. defer tx.Rollback() for _, installedApp := range installedApps { + + //check if there is any app from its appName is exits and active ...if yes then we will not insert any extra entry in the db + appMetadataByAppName, err := impl.AppRepository.FindActiveByName(installedApp.App.AppName) + if err != nil && !util.IsErrNoRows(err) { + impl.Logger.Errorw("error in fetching app by unique app identifier", "appNameUniqueIdentifier", installedApp.GetUniqueAppNameIdentifier(), "err", err) + return err + } + if appMetadataByAppName != nil && appMetadataByAppName.Id > 0 { + //app already exists for this unique identifier hence not creating new app entry for this as it will get modified after this function + continue + } //check if for this unique identifier name an app already exists, if yes then continue appMetadata, err := impl.AppRepository.FindActiveByName(installedApp.GetUniqueAppNameIdentifier()) if err != nil && !util.IsErrNoRows(err) { @@ -437,3 +449,45 @@ func (impl *InstalledAppDBServiceImpl) CreateNewAppEntryForAllInstalledApps(inst tx.Commit() return nil } + +// UpdateDuplicatedEntriesInAppAndInstalledApps performs the updation in app table and installedApps table for the cases when multiple active app found [typically two due to migration], here we are updating the db with its previous value in the installedApps table and early created app id +func (impl *InstalledAppDBServiceImpl) UpdateDuplicatedEntriesInAppAndInstalledApps(earlyApp *app.App, duplicatedApp *app.App, installedApp *appStoreRepo.InstalledApps) error { + // db operations + dbConnection := impl.InstalledAppRepository.GetConnection() + tx, err := dbConnection.Begin() + if err != nil { + return err + } + // Rollback tx on error. + defer func(tx *pg.Tx) { + err := tx.Rollback() + if err != nil { + impl.Logger.Errorw("Rollback error", "err", err) + } + }(tx) + + //updated the app table with active column as false for the duplicated app + duplicatedApp.Active = false + duplicatedApp.CreateAuditLog(bean3.SystemUserId) + err = impl.AppRepository.UpdateWithTxn(duplicatedApp, tx) + if err != nil { + impl.Logger.Errorw("error saving appModel", "err", err) + return err + } + + // updating the installedApps table with its appId column with the previous app + installedApp.AppId = earlyApp.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 + } + + err = tx.Commit() + if err != nil { + impl.Logger.Errorw("error saving appModel", "err", err) + return err + } + return nil +} From 8de88d77a1cb5d0db32714473062cd88a8b20039 Mon Sep 17 00:00:00 2001 From: Rajeev Ranjan <90333766+RajeevRanjan27@users.noreply.github.com> Date: Thu, 22 Aug 2024 12:38:03 +0530 Subject: [PATCH 7/9] Revert "fix: ea fixes for helm app (#5708)" (#5713) This reverts commit 3e31f49f95d373f92b13afbe1806606ac4a39d85. --- pkg/app/AppCrudOperationService.go | 78 ++----------------- pkg/app/helper.go | 23 +----- .../service/AppStoreDeploymentDBService.go | 1 + .../service/EAMode/InstalledAppDBService.go | 54 ------------- 4 files changed, 7 insertions(+), 149 deletions(-) diff --git a/pkg/app/AppCrudOperationService.go b/pkg/app/AppCrudOperationService.go index 3c62046235..a7035d7603 100644 --- a/pkg/app/AppCrudOperationService.go +++ b/pkg/app/AppCrudOperationService.go @@ -19,7 +19,6 @@ package app import ( "context" "encoding/json" - "errors" "fmt" "github.com/caarlos0/env" client "github.com/devtron-labs/devtron/api/helm-app/service" @@ -458,67 +457,13 @@ func convertUrlToHttpsIfSshType(url string) string { return httpsURL } -// handleDuplicateAppEntries identifies and resolves duplicate app entries based on creation time. -// It marks the most recent duplicate entry as inactive and updates the corresponding installed app. -func (impl AppCrudOperationServiceImpl) handleDuplicateAppEntries(appNameUniqueIdentifier string) (*appRepository.App, error) { - // Fetch app IDs by name - appIds, err := impl.getAppIdsByName(appNameUniqueIdentifier) - if err != nil { - impl.logger.Errorw("error in fetching app Ids by appIdentifier", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) - return nil, err - } - - // Fetch apps by IDs from App table for duplicated entries - apps, err := impl.appRepository.FindByIds(appIds) - if err != nil || errors.Is(err, pg.ErrNoRows) { - impl.logger.Errorw("error in fetching app List by appIds", "appIds", appIds, "err", err) - return nil, err - } - - // Identify the earliest and duplicated app entries - earliestApp, duplicatedApp := identifyDuplicateApps(apps) - - // Fetch the installed app associated with the duplicated app - installedApp, err := impl.installedAppRepository.GetInstalledAppsByAppId(duplicatedApp.Id) - if err != nil { - impl.logger.Errorw("error in fetching installed app by appId", "appId", duplicatedApp.Id, "err", err) - return nil, err - } - // Update duplicated app entries - err = impl.installedAppDbService.UpdateDuplicatedEntriesInAppAndInstalledApps(earliestApp, duplicatedApp, &installedApp) - if err != nil { - impl.logger.Errorw("error in updating duplicated entries", "earliestApp", earliestApp, "duplicatedApp", duplicatedApp, "err", err) - return nil, err - } - - impl.logger.Debug("Successfully resolved duplicate app entries", "earliestApp", earliestApp, "duplicatedApp", duplicatedApp) - return earliestApp, nil - -} - -// getAppIdsByName fetches app IDs by the app name unique identifier [for duplicated active app] -func (impl AppCrudOperationServiceImpl) getAppIdsByName(appNameUniqueIdentifier string) ([]*int, error) { - slice := []string{appNameUniqueIdentifier} - appIds, err := impl.appRepository.FindIdsByNames(slice) - if err != nil { - return nil, err - } - - // Convert each element to a pointer and store in a slice of pointers - ids := make([]*int, len(appIds)) - for i := range appIds { - ids[i] = &appIds[i] - } - return ids, 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 *helmBean.AppIdentifier) (*appRepository.App, error) { app := &appRepository.App{} var err error appNameUniqueIdentifier := appIdentifier.GetUniqueAppNameIdentifier() app, err = impl.appRepository.FindAppAndProjectByAppName(appNameUniqueIdentifier) - if err != nil && !errors.Is(err, pg.ErrNoRows) && !errors.Is(err, pg.ErrMultiRows) { + 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 } @@ -530,14 +475,6 @@ func (impl AppCrudOperationServiceImpl) getAppAndProjectForAppIdentifier(appIden return app, err } } - if errors.Is(err, pg.ErrMultiRows) { - - app, err = impl.handleDuplicateAppEntries(appNameUniqueIdentifier) - if err != nil { - impl.logger.Errorw("error in handling Duplicate entries in the app", "appNameUniqueIdentifier", appNameUniqueIdentifier, "err", err) - return app, err - } - } return app, nil } @@ -595,17 +532,17 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. return nil, err } // if app.DisplayName is empty then that app_name is not yet migrated to app name unique identifier - if app != nil && app.Id > 0 && len(app.DisplayName) == 0 { + if app.Id > 0 && len(app.DisplayName) == 0 { 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 } } - if app != nil && app.Id == 0 { + if app.Id == 0 { app.AppName = appIdDecoded.ReleaseName } - if app != nil && util2.IsExternalChartStoreApp(app.DisplayName) { + if util2.IsExternalChartStoreApp(app.DisplayName) { displayName = app.DisplayName } @@ -631,14 +568,9 @@ func (impl AppCrudOperationServiceImpl) GetHelmAppMetaInfo(appId string) (*bean. displayName = InstalledApp.App.DisplayName } } - // Safeguard against nil app cases - if app == nil { - impl.logger.Errorw("no rows found for the requested app", "appId", appId, "error", err) - return nil, fmt.Errorf("no rows found for the requested app, %q", pg.ErrNoRows) - } user, err := impl.userRepository.GetByIdIncludeDeleted(app.CreatedBy) - if err != nil && !errors.Is(err, pg.ErrNoRows) { + if err != nil && err != pg.ErrNoRows { impl.logger.Errorw("error in fetching user for app meta info", "error", err) return nil, err } diff --git a/pkg/app/helper.go b/pkg/app/helper.go index 0e8fff65a4..2ca99c6b14 100644 --- a/pkg/app/helper.go +++ b/pkg/app/helper.go @@ -16,10 +16,7 @@ package app -import ( - appRepository "github.com/devtron-labs/devtron/internal/sql/repository/app" - "strings" -) +import "strings" // LabelMatchingRegex is the official k8s label matching regex, pls refer https://github.com/kubernetes/apimachinery/blob/bfd2aff97e594f6aad77acbe2cbbe190acc93cbc/pkg/util/validation/validation.go#L167 const LabelMatchingRegex = "^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$" @@ -46,21 +43,3 @@ func sanitizeLabels(extraAppLabels map[string]string) map[string]string { } return extraAppLabels } - -// identifyDuplicateApps identifies the earliest created app and the most recent duplicate app. -func identifyDuplicateApps(apps []*appRepository.App) (earliestApp *appRepository.App, duplicatedApp *appRepository.App) { - if len(apps) == 0 { - return nil, nil - } - earliestApp = apps[0] - duplicatedApp = apps[0] - for _, app := range apps[1:] { - if app.AuditLog.CreatedOn.Before(earliestApp.AuditLog.CreatedOn) { - earliestApp = app - } - if app.AuditLog.CreatedOn.After(duplicatedApp.AuditLog.CreatedOn) { - duplicatedApp = app - } - } - return earliestApp, duplicatedApp -} diff --git a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go index 53d9b8b5e8..33f1a7b9a1 100644 --- a/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go +++ b/pkg/appStore/installedApp/service/AppStoreDeploymentDBService.go @@ -133,6 +133,7 @@ func (impl *AppStoreDeploymentDBServiceImpl) AppStoreDeployOperationDB(installRe } // setting additional env data required in appStoreBean.InstallAppVersionDTO adapter.UpdateAdditionalEnvDetails(installRequest, environment) + impl.appStoreValidator.Validate(installRequest, environment) // Stage 1: Create App in tx (Only if AppId is not set already) diff --git a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go index 810f3c3b8c..6eb4156e24 100644 --- a/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go +++ b/pkg/appStore/installedApp/service/EAMode/InstalledAppDBService.go @@ -58,7 +58,6 @@ type InstalledAppDBService interface { GetReleaseInfo(appIdentifier *helmBean.AppIdentifier) (*appStoreBean.InstallAppVersionDTO, error) IsExternalAppLinkedToChartStore(appId int) (bool, []*appStoreRepo.InstalledApps, error) CreateNewAppEntryForAllInstalledApps(installedApps []*appStoreRepo.InstalledApps) error - UpdateDuplicatedEntriesInAppAndInstalledApps(earlyApp *app.App, duplicatedApp *app.App, installedApp *appStoreRepo.InstalledApps) error } type InstalledAppDBServiceImpl struct { @@ -400,17 +399,6 @@ func (impl *InstalledAppDBServiceImpl) CreateNewAppEntryForAllInstalledApps(inst // Rollback tx on error. defer tx.Rollback() for _, installedApp := range installedApps { - - //check if there is any app from its appName is exits and active ...if yes then we will not insert any extra entry in the db - appMetadataByAppName, err := impl.AppRepository.FindActiveByName(installedApp.App.AppName) - if err != nil && !util.IsErrNoRows(err) { - impl.Logger.Errorw("error in fetching app by unique app identifier", "appNameUniqueIdentifier", installedApp.GetUniqueAppNameIdentifier(), "err", err) - return err - } - if appMetadataByAppName != nil && appMetadataByAppName.Id > 0 { - //app already exists for this unique identifier hence not creating new app entry for this as it will get modified after this function - continue - } //check if for this unique identifier name an app already exists, if yes then continue appMetadata, err := impl.AppRepository.FindActiveByName(installedApp.GetUniqueAppNameIdentifier()) if err != nil && !util.IsErrNoRows(err) { @@ -449,45 +437,3 @@ func (impl *InstalledAppDBServiceImpl) CreateNewAppEntryForAllInstalledApps(inst tx.Commit() return nil } - -// UpdateDuplicatedEntriesInAppAndInstalledApps performs the updation in app table and installedApps table for the cases when multiple active app found [typically two due to migration], here we are updating the db with its previous value in the installedApps table and early created app id -func (impl *InstalledAppDBServiceImpl) UpdateDuplicatedEntriesInAppAndInstalledApps(earlyApp *app.App, duplicatedApp *app.App, installedApp *appStoreRepo.InstalledApps) error { - // db operations - dbConnection := impl.InstalledAppRepository.GetConnection() - tx, err := dbConnection.Begin() - if err != nil { - return err - } - // Rollback tx on error. - defer func(tx *pg.Tx) { - err := tx.Rollback() - if err != nil { - impl.Logger.Errorw("Rollback error", "err", err) - } - }(tx) - - //updated the app table with active column as false for the duplicated app - duplicatedApp.Active = false - duplicatedApp.CreateAuditLog(bean3.SystemUserId) - err = impl.AppRepository.UpdateWithTxn(duplicatedApp, tx) - if err != nil { - impl.Logger.Errorw("error saving appModel", "err", err) - return err - } - - // updating the installedApps table with its appId column with the previous app - installedApp.AppId = earlyApp.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 - } - - err = tx.Commit() - if err != nil { - impl.Logger.Errorw("error saving appModel", "err", err) - return err - } - return nil -} From 378c2d9805e38892aa5a999858a3da04235b99e4 Mon Sep 17 00:00:00 2001 From: Prakash Date: Thu, 22 Aug 2024 14:12:41 +0530 Subject: [PATCH 8/9] fix: SkipCiBuildCachePushPull code incorporated with minor refac in handle runtime params validation (#5712) * SkipCiBuildCachePushPull code incorporated with minor refac in handle runtime params validation * minor refactor * minor refactor --- env_gen.md | 1 + pkg/pipeline/CiService.go | 30 ++++++++++++------- pkg/pipeline/bean/CiPipeline/CiBuildConfig.go | 5 ++++ pkg/pipeline/types/CiCdConfig.go | 1 + 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/env_gen.md b/env_gen.md index 6e3c3e4f9f..2f4d37a3d2 100644 --- a/env_gen.md +++ b/env_gen.md @@ -231,6 +231,7 @@ | SCOPED_VARIABLE_HANDLE_PRIMITIVES | false | | | SCOPED_VARIABLE_NAME_REGEX | ^[a-zA-Z][a-zA-Z0-9_-]{0,62}[a-zA-Z0-9]$ | | | SHOW_DOCKER_BUILD_ARGS | true | | + | SKIP_CI_JOB_BUILD_CACHE_PUSH_PULL | false | | | SKIP_CREATING_ECR_REPO | false | | | SOCKET_DISCONNECT_DELAY_SECONDS | 5 | | | SOCKET_HEARTBEAT_SECONDS | 25 | | diff --git a/pkg/pipeline/CiService.go b/pkg/pipeline/CiService.go index 2d8701d4dd..d16dd55199 100644 --- a/pkg/pipeline/CiService.go +++ b/pkg/pipeline/CiService.go @@ -158,27 +158,33 @@ func (impl *CiServiceImpl) GetCiMaterials(pipelineId int, ciMaterials []*pipelin } } -func (impl *CiServiceImpl) TriggerCiPipeline(trigger types.Trigger) (int, error) { - impl.Logger.Debug("ci pipeline manual trigger") - ciMaterials, err := impl.GetCiMaterials(trigger.PipelineId, trigger.CiMaterials) - if err != nil { - return 0, err - } - +func (impl *CiServiceImpl) handleRuntimeParamsValidations(trigger types.Trigger, ciMaterials []*pipelineConfig.CiPipelineMaterial) error { // checking if user has given run time parameters for externalCiArtifact, if given then sending git material to Ci-Runner - externalCiArtifact, exists := trigger.ExtraEnvironmentVariables["externalCiArtifact"] + externalCiArtifact, exists := trigger.ExtraEnvironmentVariables[CiPipeline.ExtraEnvVarExternalCiArtifactKey] // validate externalCiArtifact as docker image if exists { if !strings.Contains(externalCiArtifact, ":") { impl.Logger.Errorw("validation error", "externalCiArtifact", externalCiArtifact) - return 0, fmt.Errorf("invalid image name given in externalCiArtifact") + return fmt.Errorf("invalid image name given in externalCiArtifact") } } if trigger.PipelineType == string(CiPipeline.CI_JOB) && len(ciMaterials) != 0 && !exists && externalCiArtifact == "" { - ciMaterials = []*pipelineConfig.CiPipelineMaterial{ciMaterials[0]} ciMaterials[0].GitMaterial = nil ciMaterials[0].GitMaterialId = 0 } + return nil +} + +func (impl *CiServiceImpl) TriggerCiPipeline(trigger types.Trigger) (int, error) { + impl.Logger.Debug("ci pipeline manual trigger") + ciMaterials, err := impl.GetCiMaterials(trigger.PipelineId, trigger.CiMaterials) + if err != nil { + return 0, err + } + err = impl.handleRuntimeParamsValidations(trigger, ciMaterials) + if err != nil { + return 0, err + } ciPipelineScripts, err := impl.ciPipelineRepository.FindCiScriptsByCiPipelineId(trigger.PipelineId) if err != nil && !util.IsErrNoRows(err) { return 0, err @@ -741,6 +747,10 @@ func (impl *CiServiceImpl) buildWfRequestForCiPipeline(pipeline *pipelineConfig. if pipeline.App.AppType == helper.Job { workflowRequest.AppName = pipeline.App.DisplayName } + if trigger.PipelineType == string(CiPipeline.CI_JOB) { + workflowRequest.IgnoreDockerCachePush = impl.config.SkipCiJobBuildCachePushPull + workflowRequest.IgnoreDockerCachePull = impl.config.SkipCiJobBuildCachePushPull + } if dockerRegistry != nil { workflowRequest.DockerRegistryId = dockerRegistry.Id diff --git a/pkg/pipeline/bean/CiPipeline/CiBuildConfig.go b/pkg/pipeline/bean/CiPipeline/CiBuildConfig.go index ef24b43410..0050dc19f3 100644 --- a/pkg/pipeline/bean/CiPipeline/CiBuildConfig.go +++ b/pkg/pipeline/bean/CiPipeline/CiBuildConfig.go @@ -85,3 +85,8 @@ func (pType PipelineType) IsValidPipelineType() bool { return false } } + +const ( + ExtraEnvVarExternalCiArtifactKey = "externalCiArtifact" + ExtraEnvVarImageDigestKey = "imageDigest" +) diff --git a/pkg/pipeline/types/CiCdConfig.go b/pkg/pipeline/types/CiCdConfig.go index bf7e82fe3e..50e7d27271 100644 --- a/pkg/pipeline/types/CiCdConfig.go +++ b/pkg/pipeline/types/CiCdConfig.go @@ -82,6 +82,7 @@ type CiCdConfig struct { ImageScanRetryDelay int `env:"IMAGE_SCAN_RETRY_DELAY" envDefault:"5"` ShowDockerBuildCmdInLogs bool `env:"SHOW_DOCKER_BUILD_ARGS" envDefault:"true"` IgnoreCmCsInCiJob bool `env:"IGNORE_CM_CS_IN_CI_JOB" envDefault:"false"` + SkipCiJobBuildCachePushPull bool `env:"SKIP_CI_JOB_BUILD_CACHE_PUSH_PULL" envDefault:"false"` // from CdConfig CdLimitCpu string `env:"CD_LIMIT_CI_CPU" envDefault:"0.5"` CdLimitMem string `env:"CD_LIMIT_CI_MEM" envDefault:"3G"` From 827608f03c3d9f4d8d0517b5d4051598d91661f1 Mon Sep 17 00:00:00 2001 From: prakhar katiyar <39842461+prkhrkat@users.noreply.github.com> Date: Fri, 23 Aug 2024 12:14:05 +0530 Subject: [PATCH 9/9] migration syn with ent (#5718) --- .../277_insert_devtron_resource_searchable_key_table.down.sql | 1 + .../277_insert_devtron_resource_searchable_key_table.up.sql | 3 +++ 2 files changed, 4 insertions(+) create mode 100644 scripts/sql/277_insert_devtron_resource_searchable_key_table.down.sql create mode 100644 scripts/sql/277_insert_devtron_resource_searchable_key_table.up.sql diff --git a/scripts/sql/277_insert_devtron_resource_searchable_key_table.down.sql b/scripts/sql/277_insert_devtron_resource_searchable_key_table.down.sql new file mode 100644 index 0000000000..1909420ca7 --- /dev/null +++ b/scripts/sql/277_insert_devtron_resource_searchable_key_table.down.sql @@ -0,0 +1 @@ +DELETE from devtron_resource_searchable_key ds where ds."name" in ('CHART_NAME'); \ No newline at end of file diff --git a/scripts/sql/277_insert_devtron_resource_searchable_key_table.up.sql b/scripts/sql/277_insert_devtron_resource_searchable_key_table.up.sql new file mode 100644 index 0000000000..4a987ea145 --- /dev/null +++ b/scripts/sql/277_insert_devtron_resource_searchable_key_table.up.sql @@ -0,0 +1,3 @@ + +INSERT INTO devtron_resource_searchable_key(name, is_removed, created_on, created_by, updated_on, updated_by) +VALUES ('CHART_NAME', false, now(), 1, now(), 1); \ No newline at end of file