From faca64442fa7f872310f56bd23212c0b37e0b8bb Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Thu, 4 Jul 2024 19:23:53 -0300 Subject: [PATCH] chore(code): use cmp.Or() EE-7333 (#12009) --- api/cmd/portainer/main.go | 20 ++--- .../customtemplates/customtemplate_update.go | 15 ++-- .../edgestacks/edgestack_status_update.go | 15 ++-- api/http/handler/endpoints/endpoint_update.go | 23 ++--- .../handler/registries/registry_update.go | 36 +++----- api/http/handler/settings/settings_update.go | 87 +++++-------------- api/http/handler/ssl/ssl_update.go | 9 +- api/http/handler/stacks/stack_update.go | 2 +- api/http/handler/stacks/stack_update_git.go | 46 ++++------ .../stacks/stack_update_git_redeploy.go | 46 ++++------ api/http/handler/teams/team_update.go | 6 +- api/http/handler/users/user_update.go | 24 ++--- 12 files changed, 111 insertions(+), 218 deletions(-) diff --git a/api/cmd/portainer/main.go b/api/cmd/portainer/main.go index ef7f1b224..1296e1d0b 100644 --- a/api/cmd/portainer/main.go +++ b/api/cmd/portainer/main.go @@ -1,6 +1,7 @@ package main import ( + "cmp" "context" "crypto/sha256" "os" @@ -258,21 +259,10 @@ func updateSettingsFromFlags(dataStore dataservices.DataStore, flags *portainer. return err } - if *flags.SnapshotInterval != "" { - settings.SnapshotInterval = *flags.SnapshotInterval - } - - if *flags.Logo != "" { - settings.LogoURL = *flags.Logo - } - - if *flags.EnableEdgeComputeFeatures { - settings.EnableEdgeComputeFeatures = *flags.EnableEdgeComputeFeatures - } - - if *flags.Templates != "" { - settings.TemplatesURL = *flags.Templates - } + settings.SnapshotInterval = *cmp.Or(flags.SnapshotInterval, &settings.SnapshotInterval) + settings.LogoURL = *cmp.Or(flags.Logo, &settings.LogoURL) + settings.EnableEdgeComputeFeatures = *cmp.Or(flags.EnableEdgeComputeFeatures, &settings.EnableEdgeComputeFeatures) + settings.TemplatesURL = *cmp.Or(flags.Templates, &settings.TemplatesURL) if *flags.Labels != nil { settings.BlackListedLabels = *flags.Labels diff --git a/api/http/handler/customtemplates/customtemplate_update.go b/api/http/handler/customtemplates/customtemplate_update.go index 7baed4983..f3e62cf40 100644 --- a/api/http/handler/customtemplates/customtemplate_update.go +++ b/api/http/handler/customtemplates/customtemplate_update.go @@ -67,18 +67,23 @@ func (payload *customTemplateUpdatePayload) Validate(r *http.Request) error { if govalidator.IsNull(payload.Title) { return errors.New("Invalid custom template title") } + if govalidator.IsNull(payload.FileContent) && govalidator.IsNull(payload.RepositoryURL) { return errors.New("Either file content or git repository url need to be provided") } + if payload.Type != portainer.KubernetesStack && payload.Platform != portainer.CustomTemplatePlatformLinux && payload.Platform != portainer.CustomTemplatePlatformWindows { return errors.New("Invalid custom template platform") } + if payload.Type != portainer.KubernetesStack && payload.Type != portainer.DockerSwarmStack && payload.Type != portainer.DockerComposeStack { return errors.New("Invalid custom template type") } + if govalidator.IsNull(payload.Description) { return errors.New("Invalid custom template description") } + if !isValidNote(payload.Note) { return errors.New("Invalid note. tag is not supported") } @@ -86,12 +91,12 @@ func (payload *customTemplateUpdatePayload) Validate(r *http.Request) error { if payload.RepositoryAuthentication && (govalidator.IsNull(payload.RepositoryUsername) || govalidator.IsNull(payload.RepositoryPassword)) { return errors.New("Invalid repository credentials. Username and password must be specified when authentication is enabled") } + if govalidator.IsNull(payload.ComposeFilePathInRepository) { payload.ComposeFilePathInRepository = filesystem.ComposeFileDefaultName } - err := validateVariablesDefinitions(payload.Variables) - if err != nil { + if err := validateVariablesDefinitions(payload.Variables); err != nil { return err } @@ -122,8 +127,7 @@ func (handler *Handler) customTemplateUpdate(w http.ResponseWriter, r *http.Requ } var payload customTemplateUpdatePayload - err = request.DecodeAndValidateJSONPayload(r, &payload) - if err != nil { + if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { return httperror.BadRequest("Invalid request payload", err) } @@ -219,8 +223,7 @@ func (handler *Handler) customTemplateUpdate(w http.ResponseWriter, r *http.Requ customTemplate.ProjectPath = projectPath } - err = handler.DataStore.CustomTemplate().Update(customTemplate.ID, customTemplate) - if err != nil { + if err := handler.DataStore.CustomTemplate().Update(customTemplate.ID, customTemplate); err != nil { return httperror.InternalServerError("Unable to persist custom template changes inside the database", err) } diff --git a/api/http/handler/edgestacks/edgestack_status_update.go b/api/http/handler/edgestacks/edgestack_status_update.go index 837a292a1..63b9399b5 100644 --- a/api/http/handler/edgestacks/edgestack_status_update.go +++ b/api/http/handler/edgestacks/edgestack_status_update.go @@ -63,17 +63,15 @@ func (handler *Handler) edgeStackStatusUpdate(w http.ResponseWriter, r *http.Req } var payload updateStatusPayload - err = request.DecodeAndValidateJSONPayload(r, &payload) - if err != nil { + if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { return httperror.BadRequest("Invalid request payload", err) } var stack *portainer.EdgeStack - err = handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { + if err := handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { stack, err = handler.updateEdgeStackStatus(tx, r, portainer.EdgeStackID(stackID), payload) return err - }) - if err != nil { + }); err != nil { var httpErr *httperror.HandlerError if errors.As(err, &httpErr) { return httpErr @@ -106,8 +104,7 @@ func (handler *Handler) updateEdgeStackStatus(tx dataservices.DataStoreTx, r *ht return nil, handler.handlerDBErr(err, "Unable to find an environment with the specified identifier inside the database") } - err = handler.requestBouncer.AuthorizedEdgeEndpointOperation(r, endpoint) - if err != nil { + if err := handler.requestBouncer.AuthorizedEdgeEndpointOperation(r, endpoint); err != nil { return nil, httperror.Forbidden("Permission denied to access environment", err) } @@ -126,8 +123,7 @@ func (handler *Handler) updateEdgeStackStatus(tx dataservices.DataStoreTx, r *ht updateEnvStatus(payload.EndpointID, stack, deploymentStatus) - err = tx.EdgeStack().UpdateEdgeStack(stackID, stack) - if err != nil { + if err := tx.EdgeStack().UpdateEdgeStack(stackID, stack); err != nil { return nil, handler.handlerDBErr(err, "Unable to persist the stack changes inside the database") } @@ -137,6 +133,7 @@ func (handler *Handler) updateEdgeStackStatus(tx dataservices.DataStoreTx, r *ht func updateEnvStatus(environmentId portainer.EndpointID, stack *portainer.EdgeStack, deploymentStatus portainer.EdgeStackDeploymentStatus) { if deploymentStatus.Type == portainer.EdgeStackStatusRemoved { delete(stack.Status, environmentId) + return } diff --git a/api/http/handler/endpoints/endpoint_update.go b/api/http/handler/endpoints/endpoint_update.go index 68ce79a49..0f57136df 100644 --- a/api/http/handler/endpoints/endpoint_update.go +++ b/api/http/handler/endpoints/endpoint_update.go @@ -1,6 +1,7 @@ package endpoints import ( + "cmp" "net/http" "reflect" "strconv" @@ -97,12 +98,9 @@ func (handler *Handler) endpointUpdate(w http.ResponseWriter, r *http.Request) * if payload.Name != nil { name := *payload.Name - isUnique, err := handler.isNameUnique(name, endpoint.ID) - if err != nil { + if isUnique, err := handler.isNameUnique(name, endpoint.ID); err != nil { return httperror.InternalServerError("Unable to check if name is unique", err) - } - - if !isUnique { + } else if !isUnique { return httperror.Conflict("Name is not unique", nil) } @@ -114,17 +112,12 @@ func (handler *Handler) endpointUpdate(w http.ResponseWriter, r *http.Request) * updateEndpointProxy = true } - if payload.PublicURL != nil { - endpoint.PublicURL = *payload.PublicURL - } - if payload.Gpus != nil { endpoint.Gpus = payload.Gpus } - if payload.EdgeCheckinInterval != nil { - endpoint.EdgeCheckinInterval = *payload.EdgeCheckinInterval - } + endpoint.PublicURL = *cmp.Or(payload.PublicURL, &endpoint.PublicURL) + endpoint.EdgeCheckinInterval = *cmp.Or(payload.EdgeCheckinInterval, &endpoint.EdgeCheckinInterval) updateRelations := false @@ -304,9 +297,5 @@ func shouldReloadTLSConfiguration(endpoint *portainer.Endpoint, payload *endpoin return true } - if payload.TLSSkipClientVerify != nil && !*payload.TLSSkipClientVerify { - return true - } - - return false + return payload.TLSSkipClientVerify != nil && !*payload.TLSSkipClientVerify } diff --git a/api/http/handler/registries/registry_update.go b/api/http/handler/registries/registry_update.go index ef62375b4..001960b31 100644 --- a/api/http/handler/registries/registry_update.go +++ b/api/http/handler/registries/registry_update.go @@ -1,6 +1,7 @@ package registries import ( + "cmp" "errors" "net/http" @@ -83,18 +84,16 @@ func (handler *Handler) registryUpdate(w http.ResponseWriter, r *http.Request) * } var payload registryUpdatePayload - err = request.DecodeAndValidateJSONPayload(r, &payload) - if err != nil { + if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { return httperror.BadRequest("Invalid request payload", err) } - if payload.Name != nil { - registry.Name = *payload.Name - } - // enforce name uniqueness across registries - // check is performed even if Name didn't change (Name not in payload) as we need - // to enforce this rule on updates not performed with frontend (e.g. on direct API requests) - // see https://portainer.atlassian.net/browse/EE-2706 for more details + registry.Name = *cmp.Or(payload.Name, ®istry.Name) + + // Enforce name uniqueness across registries check is performed even if Name + // didn't change (Name not in payload) as we need to enforce this rule on + // updates not performed with frontend (e.g. on direct API requests) + // See https://portainer.atlassian.net/browse/EE-2706 for more details for _, r := range registries { if r.ID != registry.ID && r.Name == registry.Name { return httperror.Conflict("Another registry with the same name already exists", errors.New("A registry is already defined with this name")) @@ -172,12 +171,9 @@ func (handler *Handler) registryUpdate(w http.ResponseWriter, r *http.Request) * } } - if payload.Quay != nil { - registry.Quay = *payload.Quay - } + registry.Quay = *cmp.Or(payload.Quay, ®istry.Quay) - err = handler.DataStore.Registry().Update(registry.ID, registry) - if err != nil { + if err := handler.DataStore.Registry().Update(registry.ID, registry); err != nil { return httperror.InternalServerError("Unable to persist registry changes inside the database", err) } @@ -185,10 +181,7 @@ func (handler *Handler) registryUpdate(w http.ResponseWriter, r *http.Request) * } func syncConfig(registry *portainer.Registry) *portainer.RegistryManagementConfiguration { - config := registry.ManagementConfiguration - if config == nil { - config = &portainer.RegistryManagementConfiguration{} - } + config := cmp.Or(registry.ManagementConfiguration, &portainer.RegistryManagementConfiguration{}) config.Authentication = registry.Authentication config.Username = registry.Username @@ -200,20 +193,17 @@ func syncConfig(registry *portainer.Registry) *portainer.RegistryManagementConfi } func (handler *Handler) updateEndpointRegistryAccess(endpoint *portainer.Endpoint, registry *portainer.Registry, endpointAccess portainer.RegistryAccessPolicies) error { - cli, err := handler.K8sClientFactory.GetKubeClient(endpoint) if err != nil { return err } for _, namespace := range endpointAccess.Namespaces { - err := cli.DeleteRegistrySecret(registry.ID, namespace) - if err != nil { + if err := cli.DeleteRegistrySecret(registry.ID, namespace); err != nil { return err } - err = cli.CreateRegistrySecret(registry, namespace) - if err != nil { + if err := cli.CreateRegistrySecret(registry, namespace); err != nil { return err } } diff --git a/api/http/handler/settings/settings_update.go b/api/http/handler/settings/settings_update.go index c7a1df833..d44a54a9f 100644 --- a/api/http/handler/settings/settings_update.go +++ b/api/http/handler/settings/settings_update.go @@ -1,6 +1,7 @@ package settings import ( + "cmp" "net/http" "strings" "time" @@ -76,22 +77,19 @@ func (payload *settingsUpdatePayload) Validate(r *http.Request) error { } if payload.UserSessionTimeout != nil { - _, err := time.ParseDuration(*payload.UserSessionTimeout) - if err != nil { + if _, err := time.ParseDuration(*payload.UserSessionTimeout); err != nil { return errors.New("Invalid user session timeout") } } if payload.KubeconfigExpiry != nil { - _, err := time.ParseDuration(*payload.KubeconfigExpiry) - if err != nil { + if _, err := time.ParseDuration(*payload.KubeconfigExpiry); err != nil { return errors.New("Invalid Kubeconfig Expiry") } } if payload.EdgePortainerURL != nil && *payload.EdgePortainerURL != "" { - _, err := edge.ParseHostForEdge(*payload.EdgePortainerURL) - if err != nil { + if _, err := edge.ParseHostForEdge(*payload.EdgePortainerURL); err != nil { return err } } @@ -101,6 +99,7 @@ func (payload *settingsUpdatePayload) Validate(r *http.Request) error { return errors.New("Invalid OAuth AuthStyle") } } + return nil } @@ -153,18 +152,11 @@ func (handler *Handler) updateSettings(tx dataservices.DataStoreTx, payload sett settings.AuthenticationMethod = portainer.AuthenticationMethod(*payload.AuthenticationMethod) } - if payload.LogoURL != nil { - settings.LogoURL = *payload.LogoURL - } + settings.LogoURL = *cmp.Or(payload.LogoURL, &settings.LogoURL) + settings.TemplatesURL = *cmp.Or(payload.TemplatesURL, &settings.TemplatesURL) - if payload.TemplatesURL != nil { - settings.TemplatesURL = *payload.TemplatesURL - } - - // update the global deployment options, and the environment deployment options if they have changed - if payload.GlobalDeploymentOptions != nil { - settings.GlobalDeploymentOptions = *payload.GlobalDeploymentOptions - } + // Update the global deployment options, and the environment deployment options if they have changed + settings.GlobalDeploymentOptions = *cmp.Or(payload.GlobalDeploymentOptions, &settings.GlobalDeploymentOptions) if payload.ShowKomposeBuildOption != nil { settings.ShowKomposeBuildOption = *payload.ShowKomposeBuildOption @@ -197,16 +189,8 @@ func (handler *Handler) updateSettings(tx dataservices.DataStoreTx, payload sett } if payload.LDAPSettings != nil { - ldapReaderDN := settings.LDAPSettings.ReaderDN - ldapPassword := settings.LDAPSettings.Password - - if payload.LDAPSettings.ReaderDN != "" { - ldapReaderDN = payload.LDAPSettings.ReaderDN - } - - if payload.LDAPSettings.Password != "" { - ldapPassword = payload.LDAPSettings.Password - } + ldapReaderDN := cmp.Or(payload.LDAPSettings.ReaderDN, settings.LDAPSettings.ReaderDN) + ldapPassword := cmp.Or(payload.LDAPSettings.Password, settings.LDAPSettings.Password) settings.LDAPSettings = *payload.LDAPSettings settings.LDAPSettings.ReaderDN = ldapReaderDN @@ -229,36 +213,19 @@ func (handler *Handler) updateSettings(tx dataservices.DataStoreTx, payload sett settings.OAuthSettings.AuthStyle = payload.OAuthSettings.AuthStyle } - if payload.EnableEdgeComputeFeatures != nil { - settings.EnableEdgeComputeFeatures = *payload.EnableEdgeComputeFeatures - } - - if payload.TrustOnFirstConnect != nil { - settings.TrustOnFirstConnect = *payload.TrustOnFirstConnect - } - - if payload.EnforceEdgeID != nil { - settings.EnforceEdgeID = *payload.EnforceEdgeID - } - - if payload.EdgePortainerURL != nil { - settings.EdgePortainerURL = *payload.EdgePortainerURL - } + settings.EnableEdgeComputeFeatures = *cmp.Or(payload.EnableEdgeComputeFeatures, &settings.EnableEdgeComputeFeatures) + settings.TrustOnFirstConnect = *cmp.Or(payload.TrustOnFirstConnect, &settings.TrustOnFirstConnect) + settings.EnforceEdgeID = *cmp.Or(payload.EnforceEdgeID, &settings.EnforceEdgeID) + settings.EdgePortainerURL = *cmp.Or(payload.EdgePortainerURL, &settings.EdgePortainerURL) if payload.SnapshotInterval != nil && *payload.SnapshotInterval != settings.SnapshotInterval { - err := handler.updateSnapshotInterval(settings, *payload.SnapshotInterval) - if err != nil { + if err := handler.updateSnapshotInterval(settings, *payload.SnapshotInterval); err != nil { return nil, httperror.InternalServerError("Unable to update snapshot interval", err) } } - if payload.EdgeAgentCheckinInterval != nil { - settings.EdgeAgentCheckinInterval = *payload.EdgeAgentCheckinInterval - } - - if payload.KubeconfigExpiry != nil { - settings.KubeconfigExpiry = *payload.KubeconfigExpiry - } + settings.EdgeAgentCheckinInterval = *cmp.Or(payload.EdgeAgentCheckinInterval, &settings.EdgeAgentCheckinInterval) + settings.KubeconfigExpiry = *cmp.Or(payload.KubeconfigExpiry, &settings.KubeconfigExpiry) if payload.UserSessionTimeout != nil { settings.UserSessionTimeout = *payload.UserSessionTimeout @@ -268,21 +235,15 @@ func (handler *Handler) updateSettings(tx dataservices.DataStoreTx, payload sett handler.JWTService.SetUserSessionDuration(userSessionDuration) } - if payload.EnableTelemetry != nil { - settings.EnableTelemetry = *payload.EnableTelemetry - } + settings.EnableTelemetry = *cmp.Or(payload.EnableTelemetry, &settings.EnableTelemetry) - err = handler.updateTLS(settings) - if err != nil { + if err := handler.updateTLS(settings); err != nil { return nil, err } - if payload.KubectlShellImage != nil { - settings.KubectlShellImage = *payload.KubectlShellImage - } + settings.KubectlShellImage = *cmp.Or(payload.KubectlShellImage, &settings.KubectlShellImage) - err = tx.Settings().UpdateSettings(settings) - if err != nil { + if err := tx.Settings().UpdateSettings(settings); err != nil { return nil, httperror.InternalServerError("Unable to persist settings changes inside the database", err) } @@ -304,8 +265,8 @@ func (handler *Handler) updateTLS(settings *portainer.Settings) error { } settings.LDAPSettings.TLSConfig.TLSCACertPath = "" - err := handler.FileService.DeleteTLSFiles(filesystem.LDAPStorePath) - if err != nil { + + if err := handler.FileService.DeleteTLSFiles(filesystem.LDAPStorePath); err != nil { return httperror.InternalServerError("Unable to remove TLS files from disk", err) } diff --git a/api/http/handler/ssl/ssl_update.go b/api/http/handler/ssl/ssl_update.go index 1e8d74543..71aa30331 100644 --- a/api/http/handler/ssl/ssl_update.go +++ b/api/http/handler/ssl/ssl_update.go @@ -41,21 +41,18 @@ func (payload *sslUpdatePayload) Validate(r *http.Request) error { // @router /ssl [put] func (handler *Handler) sslUpdate(w http.ResponseWriter, r *http.Request) *httperror.HandlerError { var payload sslUpdatePayload - err := request.DecodeAndValidateJSONPayload(r, &payload) - if err != nil { + if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { return httperror.BadRequest("Invalid request payload", err) } if payload.Cert != nil { - err = handler.SSLService.SetCertificates([]byte(*payload.Cert), []byte(*payload.Key)) - if err != nil { + if err := handler.SSLService.SetCertificates([]byte(*payload.Cert), []byte(*payload.Key)); err != nil { return httperror.InternalServerError("Failed to save certificate", err) } } if payload.HTTPEnabled != nil { - err = handler.SSLService.SetHTTPEnabled(*payload.HTTPEnabled) - if err != nil { + if err := handler.SSLService.SetHTTPEnabled(*payload.HTTPEnabled); err != nil { return httperror.InternalServerError("Failed to force https", err) } } diff --git a/api/http/handler/stacks/stack_update.go b/api/http/handler/stacks/stack_update.go index 98d125e0c..c4ed63117 100644 --- a/api/http/handler/stacks/stack_update.go +++ b/api/http/handler/stacks/stack_update.go @@ -153,7 +153,7 @@ func (handler *Handler) stackUpdate(w http.ResponseWriter, r *http.Request) *htt } if stack.GitConfig != nil && stack.GitConfig.Authentication != nil && stack.GitConfig.Authentication.Password != "" { - // sanitize password in the http response to minimise possible security leaks + // Sanitize password in the http response to minimise possible security leaks stack.GitConfig.Authentication.Password = "" } diff --git a/api/http/handler/stacks/stack_update_git.go b/api/http/handler/stacks/stack_update_git.go index 63218e6f8..8d0687694 100644 --- a/api/http/handler/stacks/stack_update_git.go +++ b/api/http/handler/stacks/stack_update_git.go @@ -30,10 +30,7 @@ type stackGitUpdatePayload struct { } func (payload *stackGitUpdatePayload) Validate(r *http.Request) error { - if err := update.ValidateAutoUpdateSettings(payload.AutoUpdate); err != nil { - return err - } - return nil + return update.ValidateAutoUpdateSettings(payload.AutoUpdate) } // @id StackUpdateGit @@ -61,8 +58,7 @@ func (handler *Handler) stackUpdateGit(w http.ResponseWriter, r *http.Request) * } var payload stackGitUpdatePayload - err = request.DecodeAndValidateJSONPayload(r, &payload) - if err != nil { + if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { return httperror.BadRequest("Invalid request payload", err) } @@ -94,8 +90,7 @@ func (handler *Handler) stackUpdateGit(w http.ResponseWriter, r *http.Request) * return httperror.InternalServerError("Unable to find the environment associated to the stack inside the database", err) } - err = handler.requestBouncer.AuthorizedEndpointOperation(r, endpoint) - if err != nil { + if err := handler.requestBouncer.AuthorizedEndpointOperation(r, endpoint); err != nil { return httperror.Forbidden("Permission denied to access environment", err) } @@ -115,20 +110,16 @@ func (handler *Handler) stackUpdateGit(w http.ResponseWriter, r *http.Request) * return httperror.InternalServerError("Unable to retrieve a resource control associated to the stack", err) } - access, err := handler.userCanAccessStack(securityContext, endpoint.ID, resourceControl) - if err != nil { + if access, err := handler.userCanAccessStack(securityContext, endpoint.ID, resourceControl); err != nil { return httperror.InternalServerError("Unable to verify user authorizations to validate stack access", err) - } - if !access { + } else if !access { return httperror.Forbidden("Access denied to resource", httperrors.ErrResourceAccessDenied) } } - canManage, err := handler.userCanManageStacks(securityContext, endpoint) - if err != nil { + if canManage, err := handler.userCanManageStacks(securityContext, endpoint); err != nil { return httperror.InternalServerError("Unable to verify user authorizations to validate stack deletion", err) - } - if !canManage { + } else if !canManage { errMsg := "Stack editing is disabled for non-admin users" return httperror.Forbidden(errMsg, errors.New(errMsg)) } @@ -147,9 +138,7 @@ func (handler *Handler) stackUpdateGit(w http.ResponseWriter, r *http.Request) * stack.UpdateDate = time.Now().Unix() if stack.Type == portainer.DockerSwarmStack { - stack.Option = &portainer.StackOption{ - Prune: payload.Prune, - } + stack.Option = &portainer.StackOption{Prune: payload.Prune} } if payload.RepositoryAuthentication { @@ -160,12 +149,13 @@ func (handler *Handler) stackUpdateGit(w http.ResponseWriter, r *http.Request) * if password == "" && stack.GitConfig != nil && stack.GitConfig.Authentication != nil { password = stack.GitConfig.Authentication.Password } + stack.GitConfig.Authentication = &gittypes.GitAuthentication{ Username: payload.RepositoryUsername, Password: password, } - _, err = handler.GitService.LatestCommitID(stack.GitConfig.URL, stack.GitConfig.ReferenceName, stack.GitConfig.Authentication.Username, stack.GitConfig.Authentication.Password, stack.GitConfig.TLSSkipVerify) - if err != nil { + + if _, err := handler.GitService.LatestCommitID(stack.GitConfig.URL, stack.GitConfig.ReferenceName, stack.GitConfig.Authentication.Username, stack.GitConfig.Authentication.Password, stack.GitConfig.TLSSkipVerify); err != nil { return httperror.InternalServerError("Unable to fetch git repository", err) } } else { @@ -173,17 +163,15 @@ func (handler *Handler) stackUpdateGit(w http.ResponseWriter, r *http.Request) * } if payload.AutoUpdate != nil && payload.AutoUpdate.Interval != "" { - jobID, e := deployments.StartAutoupdate(stack.ID, stack.AutoUpdate.Interval, handler.Scheduler, handler.StackDeployer, handler.DataStore, handler.GitService) - if e != nil { - return e + if jobID, err := deployments.StartAutoupdate(stack.ID, stack.AutoUpdate.Interval, handler.Scheduler, handler.StackDeployer, handler.DataStore, handler.GitService); err != nil { + return err + } else { + stack.AutoUpdate.JobID = jobID } - - stack.AutoUpdate.JobID = jobID } - //save the updated stack to DB - err = handler.DataStore.Stack().Update(stack.ID, stack) - if err != nil { + // Save the updated stack to DB + if err := handler.DataStore.Stack().Update(stack.ID, stack); err != nil { return httperror.InternalServerError("Unable to persist the stack changes inside the database", err) } diff --git a/api/http/handler/stacks/stack_update_git_redeploy.go b/api/http/handler/stacks/stack_update_git_redeploy.go index dcf97b8ea..6c58df2c3 100644 --- a/api/http/handler/stacks/stack_update_git_redeploy.go +++ b/api/http/handler/stacks/stack_update_git_redeploy.go @@ -88,8 +88,7 @@ func (handler *Handler) stackGitRedeploy(w http.ResponseWriter, r *http.Request) return httperror.InternalServerError("Unable to find the environment associated to the stack inside the database", err) } - err = handler.requestBouncer.AuthorizedEndpointOperation(r, endpoint) - if err != nil { + if err := handler.requestBouncer.AuthorizedEndpointOperation(r, endpoint); err != nil { return httperror.Forbidden("Permission denied to access environment", err) } @@ -98,44 +97,36 @@ func (handler *Handler) stackGitRedeploy(w http.ResponseWriter, r *http.Request) return httperror.InternalServerError("Unable to retrieve info from request context", err) } - //only check resource control when it is a DockerSwarmStack or a DockerComposeStack + // Only check resource control when it is a DockerSwarmStack or a DockerComposeStack if stack.Type == portainer.DockerSwarmStack || stack.Type == portainer.DockerComposeStack { - resourceControl, err := handler.DataStore.ResourceControl().ResourceControlByResourceIDAndType(stackutils.ResourceControlID(stack.EndpointID, stack.Name), portainer.StackResourceControl) if err != nil { return httperror.InternalServerError("Unable to retrieve a resource control associated to the stack", err) } - access, err := handler.userCanAccessStack(securityContext, endpoint.ID, resourceControl) - if err != nil { + if access, err := handler.userCanAccessStack(securityContext, endpoint.ID, resourceControl); err != nil { return httperror.InternalServerError("Unable to verify user authorizations to validate stack access", err) - } - if !access { + } else if !access { return httperror.Forbidden("Access denied to resource", httperrors.ErrResourceAccessDenied) } } - canManage, err := handler.userCanManageStacks(securityContext, endpoint) - if err != nil { + if canManage, err := handler.userCanManageStacks(securityContext, endpoint); err != nil { return httperror.InternalServerError("Unable to verify user authorizations to validate stack deletion", err) - } - if !canManage { + } else if !canManage { errMsg := "Stack management is disabled for non-admin users" return httperror.Forbidden(errMsg, errors.New(errMsg)) } var payload stackGitRedployPayload - err = request.DecodeAndValidateJSONPayload(r, &payload) - if err != nil { + if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { return httperror.BadRequest("Invalid request payload", err) } stack.GitConfig.ReferenceName = payload.RepositoryReferenceName stack.Env = payload.Env if stack.Type == portainer.DockerSwarmStack { - stack.Option = &portainer.StackOption{ - Prune: payload.Prune, - } + stack.Option = &portainer.StackOption{Prune: payload.Prune} } if stack.Type == portainer.KubernetesStack { @@ -171,9 +162,8 @@ func (handler *Handler) stackGitRedeploy(w http.ResponseWriter, r *http.Request) defer clean() - httpErr := handler.deployStack(r, stack, payload.PullImage, endpoint) - if httpErr != nil { - return httpErr + if err := handler.deployStack(r, stack, payload.PullImage, endpoint); err != nil { + return err } newHash, err := handler.GitService.LatestCommitID(stack.GitConfig.URL, stack.GitConfig.ReferenceName, repositoryUsername, repositoryPassword, stack.GitConfig.TLSSkipVerify) @@ -190,13 +180,12 @@ func (handler *Handler) stackGitRedeploy(w http.ResponseWriter, r *http.Request) stack.UpdateDate = time.Now().Unix() stack.Status = portainer.StackStatusActive - err = handler.DataStore.Stack().Update(stack.ID, stack) - if err != nil { + if err := handler.DataStore.Stack().Update(stack.ID, stack); err != nil { return httperror.InternalServerError("Unable to persist the stack changes inside the database", errors.Wrap(err, "failed to update the stack")) } if stack.GitConfig != nil && stack.GitConfig.Authentication != nil && stack.GitConfig.Authentication.Password != "" { - // sanitize password in the http response to minimise possible security leaks + // Sanitize password in the http response to minimise possible security leaks stack.GitConfig.Authentication.Password = "" } @@ -204,10 +193,7 @@ func (handler *Handler) stackGitRedeploy(w http.ResponseWriter, r *http.Request) } func (handler *Handler) deployStack(r *http.Request, stack *portainer.Stack, pullImage bool, endpoint *portainer.Endpoint) *httperror.HandlerError { - var ( - deploymentConfiger deployments.StackDeploymentConfiger - err error - ) + var deploymentConfiger deployments.StackDeploymentConfiger switch stack.Type { case portainer.DockerSwarmStack: @@ -238,6 +224,7 @@ func (handler *Handler) deployStack(r *http.Request, stack *portainer.Stack, pul if err != nil { return httperror.InternalServerError(err.Error(), err) } + case portainer.KubernetesStack: handler.stackCreationMutex.Lock() defer handler.stackCreationMutex.Unlock() @@ -263,13 +250,14 @@ func (handler *Handler) deployStack(r *http.Request, stack *portainer.Stack, pul if err != nil { return httperror.InternalServerError(err.Error(), err) } + default: return httperror.InternalServerError("Unsupported stack", errors.Errorf("unsupported stack type: %v", stack.Type)) } - err = deploymentConfiger.Deploy() - if err != nil { + if err := deploymentConfiger.Deploy(); err != nil { return httperror.InternalServerError(err.Error(), err) } + return nil } diff --git a/api/http/handler/teams/team_update.go b/api/http/handler/teams/team_update.go index ee04c2706..3ad4a3151 100644 --- a/api/http/handler/teams/team_update.go +++ b/api/http/handler/teams/team_update.go @@ -43,8 +43,7 @@ func (handler *Handler) teamUpdate(w http.ResponseWriter, r *http.Request) *http } var payload teamUpdatePayload - err = request.DecodeAndValidateJSONPayload(r, &payload) - if err != nil { + if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { return httperror.BadRequest("Invalid request payload", err) } @@ -59,8 +58,7 @@ func (handler *Handler) teamUpdate(w http.ResponseWriter, r *http.Request) *http team.Name = payload.Name } - err = handler.DataStore.Team().Update(team.ID, team) - if err != nil { + if err := handler.DataStore.Team().Update(team.ID, team); err != nil { return httperror.NotFound("Unable to persist team changes inside the database", err) } diff --git a/api/http/handler/users/user_update.go b/api/http/handler/users/user_update.go index d656fd8ec..30a0eda3d 100644 --- a/api/http/handler/users/user_update.go +++ b/api/http/handler/users/user_update.go @@ -1,6 +1,7 @@ package users import ( + "cmp" "errors" "net/http" "time" @@ -78,8 +79,7 @@ func (handler *Handler) userUpdate(w http.ResponseWriter, r *http.Request) *http } var payload userUpdatePayload - err = request.DecodeAndValidateJSONPayload(r, &payload) - if err != nil { + if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { return httperror.BadRequest("Invalid request payload", err) } @@ -99,11 +99,9 @@ func (handler *Handler) userUpdate(w http.ResponseWriter, r *http.Request) *http return httperror.Forbidden("Permission denied. Unable to update username", httperrors.ErrResourceAccessDenied) } - sameNameUser, err := handler.DataStore.User().UserByUsername(payload.Username) - if err != nil && !handler.DataStore.IsErrObjectNotFound(err) { + if sameNameUser, err := handler.DataStore.User().UserByUsername(payload.Username); err != nil && !handler.DataStore.IsErrObjectNotFound(err) { return httperror.InternalServerError("Unable to retrieve users from the database", err) - } - if sameNameUser != nil && sameNameUser.ID != portainer.UserID(userID) { + } else if sameNameUser != nil && sameNameUser.ID != portainer.UserID(userID) { return httperror.Conflict("Another user with the same username already exists", errUserAlreadyExists) } @@ -121,8 +119,7 @@ func (handler *Handler) userUpdate(w http.ResponseWriter, r *http.Request) *http if payload.NewPassword != "" { // Non-admins need to supply the previous password if tokenData.Role != portainer.AdministratorRole { - err := handler.CryptoService.CompareHashAndData(user.Password, payload.Password) - if err != nil { + if err := handler.CryptoService.CompareHashAndData(user.Password, payload.Password); err != nil { return httperror.Forbidden("Current password doesn't match. Password left unchanged", errors.New("Current password does not match the password provided. Please try again")) } } @@ -139,22 +136,17 @@ func (handler *Handler) userUpdate(w http.ResponseWriter, r *http.Request) *http } if payload.Theme != nil { - if payload.Theme.Color != nil { - user.ThemeSettings.Color = *payload.Theme.Color - } + user.ThemeSettings.Color = *cmp.Or(payload.Theme.Color, &user.ThemeSettings.Color) } - if payload.UseCache != nil { - user.UseCache = *payload.UseCache - } + user.UseCache = *cmp.Or(payload.UseCache, &user.UseCache) if payload.Role != 0 { user.Role = portainer.UserRole(payload.Role) user.TokenIssueAt = time.Now().Unix() } - err = handler.DataStore.User().Update(user.ID, user) - if err != nil { + if err := handler.DataStore.User().Update(user.ID, user); err != nil { return httperror.InternalServerError("Unable to persist user changes inside the database", err) }