From bfa27d91039024da8c78467170f29bbc11f0a3d1 Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Tue, 18 Jun 2024 15:59:12 -0300 Subject: [PATCH] chore(code): clean up the code EE-7251 (#11948) --- api/database/boltdb/db.go | 26 ++-- api/database/boltdb/json.go | 20 +-- api/database/boltdb/tx.go | 3 +- api/docker/consts/labels.go | 4 +- api/docker/container.go | 8 +- api/docker/images/status.go | 6 +- .../customtemplate_git_fetch_test.go | 4 +- .../handler/docker/containers/recreate.go | 20 +-- .../edgestacks/utils_update_stack_version.go | 9 +- .../endpointgroups/endpointgroup_update.go | 39 ++--- api/http/handler/endpoints/endpoint_delete.go | 28 ++-- .../endpoint_force_update_service.go | 2 +- .../handler/registries/registry_delete.go | 22 +-- .../resourcecontrol_delete.go | 5 +- .../resourcecontrol_update.go | 9 +- api/http/handler/stacks/stack_update.go | 57 +++---- api/http/handler/templates/template_file.go | 7 +- api/http/handler/webhooks/webhook_execute.go | 13 +- api/http/offlinegate/offlinegate_test.go | 6 +- api/http/proxy/factory/agent.go | 2 +- api/http/proxy/factory/agent/transport.go | 2 +- .../proxy/factory/azure/containergroup.go | 12 +- api/http/proxy/factory/azure/transport.go | 2 +- api/http/proxy/factory/docker/transport.go | 145 +++++++++--------- api/http/proxy/factory/gitlab/transport.go | 2 +- api/http/proxy/factory/kubernetes.go | 3 +- api/http/proxy/manager.go | 35 +++-- api/http/security/bouncer.go | 2 +- api/internal/endpointutils/endpoint_setup.go | 17 +- api/internal/registryutils/ecr_kube_secret.go | 25 ++- api/internal/set/set.go | 9 +- api/internal/slices/slices_test.go | 6 +- api/internal/snapshot/snapshot.go | 14 +- api/internal/url/url.go | 3 +- api/kubernetes/cli/pod.go | 12 +- api/kubernetes/cli/registries.go | 12 +- api/ldap/ldap.go | 2 +- api/oauth/oauth_resource.go | 4 +- .../stack_file_content_builder.go | 12 +- .../stack_file_upload_builder.go | 10 +- api/stacks/stackbuilders/stack_git_builder.go | 7 +- .../swarm_file_upload_builder.go | 2 + pkg/featureflags/featureflags.go | 6 +- pkg/libhelm/binary/search_repo.go | 2 +- pkg/libhelm/release/release.go | 2 +- pkg/libhelm/validate_repo.go | 2 +- pkg/libhttp/error/error.go | 13 +- pkg/libhttp/request/payload_test.go | 2 +- pkg/libhttp/request/request.go | 25 ++- 49 files changed, 325 insertions(+), 355 deletions(-) diff --git a/api/database/boltdb/db.go b/api/database/boltdb/db.go index 39305e5cf..a0ef4df63 100644 --- a/api/database/boltdb/db.go +++ b/api/database/boltdb/db.go @@ -8,6 +8,7 @@ import ( "math" "os" "path" + "strconv" "time" portainer "github.com/portainer/portainer/api" @@ -73,7 +74,6 @@ func (connection *DbConnection) IsEncryptedStore() bool { // NeedsEncryptionMigration returns true if database encryption is enabled and // we have an un-encrypted DB that requires migration to an encrypted DB func (connection *DbConnection) NeedsEncryptionMigration() (bool, error) { - // Cases: Note, we need to check both portainer.db and portainer.edb // to determine if it's a new store. We only need to differentiate between cases 2,3 and 5 @@ -121,11 +121,11 @@ func (connection *DbConnection) NeedsEncryptionMigration() (bool, error) { // Open opens and initializes the BoltDB database. func (connection *DbConnection) Open() error { - log.Info().Str("filename", connection.GetDatabaseFileName()).Msg("loading PortainerDB") // Now we open the db databasePath := connection.GetDatabaseFilePath() + db, err := bolt.Open(databasePath, 0600, &bolt.Options{ Timeout: 1 * time.Second, InitialMmapSize: connection.InitialMmapSize, @@ -178,6 +178,7 @@ func (connection *DbConnection) ViewTx(fn func(portainer.Transaction) error) err func (connection *DbConnection) BackupTo(w io.Writer) error { return connection.View(func(tx *bolt.Tx) error { _, err := tx.WriteTo(w) + return err }) } @@ -192,6 +193,7 @@ func (connection *DbConnection) ExportRaw(filename string) error { if err != nil { return err } + return os.WriteFile(filename, b, 0600) } @@ -212,7 +214,7 @@ func keyToString(b []byte) string { v := binary.BigEndian.Uint64(b) if v <= math.MaxInt32 { - return fmt.Sprintf("%d", v) + return strconv.FormatUint(v, 10) } return string(b) @@ -321,22 +323,22 @@ func (connection *DbConnection) CreateObjectWithStringId(bucketName string, id [ }) } -func (connection *DbConnection) GetAll(bucketName string, obj interface{}, append func(o interface{}) (interface{}, error)) error { +func (connection *DbConnection) GetAll(bucketName string, obj interface{}, appendFn func(o interface{}) (interface{}, error)) error { return connection.ViewTx(func(tx portainer.Transaction) error { - return tx.GetAll(bucketName, obj, append) + return tx.GetAll(bucketName, obj, appendFn) }) } // TODO: decide which Unmarshal to use, and use one... -func (connection *DbConnection) GetAllWithJsoniter(bucketName string, obj interface{}, append func(o interface{}) (interface{}, error)) error { +func (connection *DbConnection) GetAllWithJsoniter(bucketName string, obj interface{}, appendFn func(o interface{}) (interface{}, error)) error { return connection.ViewTx(func(tx portainer.Transaction) error { - return tx.GetAllWithJsoniter(bucketName, obj, append) + return tx.GetAllWithJsoniter(bucketName, obj, appendFn) }) } -func (connection *DbConnection) GetAllWithKeyPrefix(bucketName string, keyPrefix []byte, obj interface{}, append func(o interface{}) (interface{}, error)) error { +func (connection *DbConnection) GetAllWithKeyPrefix(bucketName string, keyPrefix []byte, obj interface{}, appendFn func(o interface{}) (interface{}, error)) error { return connection.ViewTx(func(tx portainer.Transaction) error { - return tx.GetAllWithKeyPrefix(bucketName, keyPrefix, obj, append) + return tx.GetAllWithKeyPrefix(bucketName, keyPrefix, obj, appendFn) }) } @@ -345,14 +347,13 @@ func (connection *DbConnection) BackupMetadata() (map[string]interface{}, error) buckets := map[string]interface{}{} err := connection.View(func(tx *bolt.Tx) error { - err := tx.ForEach(func(name []byte, bucket *bolt.Bucket) error { + return tx.ForEach(func(name []byte, bucket *bolt.Bucket) error { bucketName := string(name) seqId := bucket.Sequence() buckets[bucketName] = int(seqId) + return nil }) - - return err }) return buckets, err @@ -366,6 +367,7 @@ func (connection *DbConnection) RestoreMetadata(s map[string]interface{}) error id, ok := v.(float64) // JSON ints are unmarshalled to interface as float64. See: https://pkg.go.dev/encoding/json#Decoder.Decode if !ok { log.Error().Str("bucket", bucketName).Msg("failed to restore metadata to bucket, skipped") + continue } diff --git a/api/database/boltdb/json.go b/api/database/boltdb/json.go index 2c9192d4e..47db59ad7 100644 --- a/api/database/boltdb/json.go +++ b/api/database/boltdb/json.go @@ -5,14 +5,13 @@ import ( "crypto/aes" "crypto/cipher" "crypto/rand" - "fmt" "io" "github.com/pkg/errors" "github.com/segmentio/encoding/json" ) -var errEncryptedStringTooShort = fmt.Errorf("encrypted string too short") +var errEncryptedStringTooShort = errors.New("encrypted string too short") // MarshalObject encodes an object to binary format func (connection *DbConnection) MarshalObject(object interface{}) ([]byte, error) { @@ -70,22 +69,20 @@ func encrypt(plaintext []byte, passphrase []byte) (encrypted []byte, err error) if err != nil { return encrypted, err } + nonce := make([]byte, gcm.NonceSize()) if _, err = io.ReadFull(rand.Reader, nonce); err != nil { return encrypted, err } - ciphertextByte := gcm.Seal( - nonce, - nonce, - plaintext, - nil) - return ciphertextByte, nil + + return gcm.Seal(nonce, nonce, plaintext, nil), nil } func decrypt(encrypted []byte, passphrase []byte) (plaintextByte []byte, err error) { if string(encrypted) == "false" { return []byte("false"), nil } + block, err := aes.NewCipher(passphrase) if err != nil { return encrypted, errors.Wrap(err, "Error creating cypher block") @@ -102,11 +99,8 @@ func decrypt(encrypted []byte, passphrase []byte) (plaintextByte []byte, err err } nonce, ciphertextByteClean := encrypted[:nonceSize], encrypted[nonceSize:] - plaintextByte, err = gcm.Open( - nil, - nonce, - ciphertextByteClean, - nil) + + plaintextByte, err = gcm.Open(nil, nonce, ciphertextByteClean, nil) if err != nil { return encrypted, errors.Wrap(err, "Error decrypting text") } diff --git a/api/database/boltdb/tx.go b/api/database/boltdb/tx.go index 073b279bf..be10359bc 100644 --- a/api/database/boltdb/tx.go +++ b/api/database/boltdb/tx.go @@ -74,9 +74,10 @@ func (tx *DbTransaction) DeleteAllObjects(bucketName string, obj interface{}, ma func (tx *DbTransaction) GetNextIdentifier(bucketName string) int { bucket := tx.tx.Bucket([]byte(bucketName)) + id, err := bucket.NextSequence() if err != nil { - log.Error().Err(err).Str("bucket", bucketName).Msg("failed to get the next identifer") + log.Error().Err(err).Str("bucket", bucketName).Msg("failed to get the next identifier") return 0 } diff --git a/api/docker/consts/labels.go b/api/docker/consts/labels.go index 748dff8e2..aeb24ba21 100644 --- a/api/docker/consts/labels.go +++ b/api/docker/consts/labels.go @@ -3,7 +3,7 @@ package consts const ( ComposeStackNameLabel = "com.docker.compose.project" SwarmStackNameLabel = "com.docker.stack.namespace" - SwarmServiceIdLabel = "com.docker.swarm.service.id" - SwarmNodeIdLabel = "com.docker.swarm.node.id" + SwarmServiceIDLabel = "com.docker.swarm.service.id" + SwarmNodeIDLabel = "com.docker.swarm.node.id" HideStackLabel = "io.portainer.hideStack" ) diff --git a/api/docker/container.go b/api/docker/container.go index 2c76f01e6..f5167afbf 100644 --- a/api/docker/container.go +++ b/api/docker/container.go @@ -7,12 +7,12 @@ import ( "github.com/docker/docker/api/types" dockercontainer "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" - "github.com/docker/docker/client" - "github.com/pkg/errors" portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/dataservices" dockerclient "github.com/portainer/portainer/api/docker/client" "github.com/portainer/portainer/api/docker/images" + + "github.com/pkg/errors" "github.com/rs/zerolog/log" ) @@ -37,9 +37,7 @@ func (c *ContainerService) Recreate(ctx context.Context, endpoint *portainer.End return nil, errors.Wrap(err, "create client error") } - defer func(cli *client.Client) { - cli.Close() - }(cli) + defer cli.Close() log.Debug().Str("container_id", containerId).Msg("starting to fetch container information") diff --git a/api/docker/images/status.go b/api/docker/images/status.go index ae9f0787e..2b8ae9c5a 100644 --- a/api/docker/images/status.go +++ b/api/docker/images/status.go @@ -48,11 +48,11 @@ func (c *DigestClient) ContainersImageStatus(ctx context.Context, containers []t statuses := make([]Status, len(containers)) for i, ct := range containers { var nodeName string - if swarmNodeId := ct.Labels[consts.SwarmNodeIdLabel]; swarmNodeId != "" { + if swarmNodeId := ct.Labels[consts.SwarmNodeIDLabel]; swarmNodeId != "" { if swarmNodeName, ok := swarmID2NameCache.Get(swarmNodeId); ok { nodeName, _ = swarmNodeName.(string) } else { - node, _, err := cli.NodeInspectWithRaw(ctx, ct.Labels[consts.SwarmNodeIdLabel]) + node, _, err := cli.NodeInspectWithRaw(ctx, ct.Labels[consts.SwarmNodeIDLabel]) if err != nil { return Error } @@ -160,7 +160,7 @@ func (c *DigestClient) ServiceImageStatus(ctx context.Context, serviceID string, containers, err := cli.ContainerList(ctx, container.ListOptions{ All: true, - Filters: filters.NewArgs(filters.Arg("label", consts.SwarmServiceIdLabel+"="+serviceID)), + Filters: filters.NewArgs(filters.Arg("label", consts.SwarmServiceIDLabel+"="+serviceID)), }) if err != nil { log.Warn().Err(err).Str("serviceID", serviceID).Msg("cannot list container for the service") diff --git a/api/http/handler/customtemplates/customtemplate_git_fetch_test.go b/api/http/handler/customtemplates/customtemplate_git_fetch_test.go index 83cad9b2b..f574a0c9b 100644 --- a/api/http/handler/customtemplates/customtemplate_git_fetch_test.go +++ b/api/http/handler/customtemplates/customtemplate_git_fetch_test.go @@ -90,7 +90,7 @@ func singleAPIRequest(h *Handler, jwt string, is *assert.Assertions, expect stri FileContent string } - req := httptest.NewRequest(http.MethodPut, "/custom_templates/1/git_fetch", bytes.NewBuffer([]byte("{}"))) + req := httptest.NewRequest(http.MethodPut, "/custom_templates/1/git_fetch", bytes.NewBufferString("{}")) testhelpers.AddTestSecurityCookie(req, jwt) rr := httptest.NewRecorder() @@ -196,7 +196,7 @@ func Test_customTemplateGitFetch(t *testing.T) { } h := NewHandler(requestBouncer, store, fileService, invalidGitService) - req := httptest.NewRequest(http.MethodPut, "/custom_templates/1/git_fetch", bytes.NewBuffer([]byte("{}"))) + req := httptest.NewRequest(http.MethodPut, "/custom_templates/1/git_fetch", bytes.NewBufferString("{}")) testhelpers.AddTestSecurityCookie(req, jwt1) rr := httptest.NewRecorder() diff --git a/api/http/handler/docker/containers/recreate.go b/api/http/handler/docker/containers/recreate.go index 17bfd9523..fa4e3c3cb 100644 --- a/api/http/handler/docker/containers/recreate.go +++ b/api/http/handler/docker/containers/recreate.go @@ -11,6 +11,7 @@ import ( httperror "github.com/portainer/portainer/pkg/libhttp/error" "github.com/portainer/portainer/pkg/libhttp/request" "github.com/portainer/portainer/pkg/libhttp/response" + "github.com/rs/zerolog/log" ) @@ -30,8 +31,7 @@ func (handler *Handler) recreate(w http.ResponseWriter, r *http.Request) *httper } var payload RecreatePayload - err = request.DecodeAndValidateJSONPayload(r, &payload) - if err != nil { + if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { return httperror.BadRequest("Invalid request payload", err) } @@ -40,8 +40,7 @@ func (handler *Handler) recreate(w http.ResponseWriter, r *http.Request) *httper return httperror.NotFound("Unable to find an environment on request context", err) } - err = handler.bouncer.AuthorizedEndpointOperation(r, endpoint) - if err != nil { + if err := handler.bouncer.AuthorizedEndpointOperation(r, endpoint); err != nil { return httperror.Forbidden("Permission denied to force update service", err) } @@ -58,8 +57,9 @@ func (handler *Handler) recreate(w http.ResponseWriter, r *http.Request) *httper go func() { images.EvictImageStatus(containerID) images.EvictImageStatus(newContainer.Config.Labels[consts.ComposeStackNameLabel]) - images.EvictImageStatus(newContainer.Config.Labels[consts.SwarmServiceIdLabel]) + images.EvictImageStatus(newContainer.Config.Labels[consts.SwarmServiceIDLabel]) }() + return response.JSON(w, newContainer) } @@ -67,6 +67,7 @@ func (handler *Handler) createResourceControl(oldContainerId string, newContaine resourceControls, err := handler.dataStore.ResourceControl().ReadAll() if err != nil { log.Error().Err(err).Msg("Exporting Resource Controls") + return } @@ -74,11 +75,10 @@ func (handler *Handler) createResourceControl(oldContainerId string, newContaine if resourceControl == nil { return } + resourceControl.ResourceID = newContainerId - err = handler.dataStore.ResourceControl().Create(resourceControl) - if err != nil { + if err := handler.dataStore.ResourceControl().Create(resourceControl); err != nil { log.Error().Err(err).Str("containerId", newContainerId).Msg("Failed to create new resource control for container") - return } } @@ -86,12 +86,12 @@ func (handler *Handler) updateWebhook(oldContainerId string, newContainerId stri webhook, err := handler.dataStore.Webhook().WebhookByResourceID(oldContainerId) if err != nil { log.Error().Err(err).Str("containerId", oldContainerId).Msg("cannot find webhook by containerId") + return } webhook.ResourceID = newContainerId - err = handler.dataStore.Webhook().Update(webhook.ID, webhook) - if err != nil { + if err := handler.dataStore.Webhook().Update(webhook.ID, webhook); err != nil { log.Error().Err(err).Int("webhookId", int(webhook.ID)).Msg("cannot update webhook") } } diff --git a/api/http/handler/edgestacks/utils_update_stack_version.go b/api/http/handler/edgestacks/utils_update_stack_version.go index 79a947c49..2502a88f6 100644 --- a/api/http/handler/edgestacks/utils_update_stack_version.go +++ b/api/http/handler/edgestacks/utils_update_stack_version.go @@ -7,11 +7,11 @@ import ( portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/filesystem" edgestackutils "github.com/portainer/portainer/api/internal/edge/edgestacks" + "github.com/rs/zerolog/log" ) func (handler *Handler) updateStackVersion(stack *portainer.EdgeStack, deploymentType portainer.EdgeStackDeploymentType, config []byte, oldGitHash string, relatedEnvironmentsIDs []portainer.EndpointID) error { - stack.Version = stack.Version + 1 stack.Status = edgestackutils.NewStatus(stack.Status, relatedEnvironmentsIDs) @@ -19,11 +19,9 @@ func (handler *Handler) updateStackVersion(stack *portainer.EdgeStack, deploymen } func (handler *Handler) storeStackFile(stack *portainer.EdgeStack, deploymentType portainer.EdgeStackDeploymentType, config []byte) error { - if deploymentType != stack.DeploymentType { // deployment type was changed - need to delete all old files - err := handler.FileService.RemoveDirectory(stack.ProjectPath) - if err != nil { + if err := handler.FileService.RemoveDirectory(stack.ProjectPath); err != nil { log.Warn().Err(err).Msg("Unable to clear old files") } @@ -50,8 +48,7 @@ func (handler *Handler) storeStackFile(stack *portainer.EdgeStack, deploymentTyp entryPoint = stack.ManifestPath } - _, err := handler.FileService.StoreEdgeStackFileFromBytes(stackFolder, entryPoint, config) - if err != nil { + if _, err := handler.FileService.StoreEdgeStackFileFromBytes(stackFolder, entryPoint, config); err != nil { return fmt.Errorf("unable to persist updated Compose file with version on disk: %w", err) } diff --git a/api/http/handler/endpointgroups/endpointgroup_update.go b/api/http/handler/endpointgroups/endpointgroup_update.go index 446716fb5..2a4e2576d 100644 --- a/api/http/handler/endpointgroups/endpointgroup_update.go +++ b/api/http/handler/endpointgroups/endpointgroup_update.go @@ -7,11 +7,13 @@ import ( portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/dataservices" + "github.com/portainer/portainer/api/internal/endpointutils" "github.com/portainer/portainer/api/internal/tag" "github.com/portainer/portainer/api/pendingactions/handlers" httperror "github.com/portainer/portainer/pkg/libhttp/error" "github.com/portainer/portainer/pkg/libhttp/request" "github.com/portainer/portainer/pkg/libhttp/response" + "github.com/rs/zerolog/log" ) @@ -53,17 +55,17 @@ func (handler *Handler) endpointGroupUpdate(w http.ResponseWriter, r *http.Reque } var payload endpointGroupUpdatePayload - err = request.DecodeAndValidateJSONPayload(r, &payload) - if err != nil { + if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { return httperror.BadRequest("Invalid request payload", err) } var endpointGroup *portainer.EndpointGroup - err = handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { + + if err := handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { endpointGroup, err = handler.updateEndpointGroup(tx, portainer.EndpointGroupID(endpointGroupID), payload) + return err - }) - if err != nil { + }); err != nil { var httpErr *httperror.HandlerError if errors.As(err, &httpErr) { return httpErr @@ -151,25 +153,20 @@ func (handler *Handler) updateEndpointGroup(tx dataservices.DataStoreTx, endpoin } for _, endpoint := range endpoints { - if endpoint.GroupID == endpointGroup.ID { - if endpoint.Type == portainer.KubernetesLocalEnvironment || endpoint.Type == portainer.AgentOnKubernetesEnvironment || endpoint.Type == portainer.EdgeAgentOnKubernetesEnvironment { - err = handler.AuthorizationService.CleanNAPWithOverridePolicies(tx, &endpoint, endpointGroup) - if err != nil { - // Update flag with endpoint and continue - go func(endpointID portainer.EndpointID, endpointGroupID portainer.EndpointGroupID) { - err := handler.PendingActionsService.Create(handlers.NewCleanNAPWithOverridePolicies(endpointID, &endpointGroupID)) - if err != nil { - log.Error().Err(err).Msgf("Unable to create pending action to clean NAP with override policies for endpoint (%d) and endpoint group (%d).", endpointID, endpointGroupID) - } - }(endpoint.ID, endpointGroup.ID) - } + if endpoint.GroupID == endpointGroup.ID && endpointutils.IsKubernetesEndpoint(&endpoint) { + if err := handler.AuthorizationService.CleanNAPWithOverridePolicies(tx, &endpoint, endpointGroup); err != nil { + // Update flag with endpoint and continue + go func(endpointID portainer.EndpointID, endpointGroupID portainer.EndpointGroupID) { + if err := handler.PendingActionsService.Create(handlers.NewCleanNAPWithOverridePolicies(endpointID, &endpointGroupID)); err != nil { + log.Error().Err(err).Msgf("Unable to create pending action to clean NAP with override policies for endpoint (%d) and endpoint group (%d).", endpointID, endpointGroupID) + } + }(endpoint.ID, endpointGroup.ID) } } } } - err = tx.EndpointGroup().Update(endpointGroup.ID, endpointGroup) - if err != nil { + if err := tx.EndpointGroup().Update(endpointGroup.ID, endpointGroup); err != nil { return nil, httperror.InternalServerError("Unable to persist environment group changes inside the database", err) } @@ -177,13 +174,11 @@ func (handler *Handler) updateEndpointGroup(tx dataservices.DataStoreTx, endpoin endpoints, err := tx.Endpoint().Endpoints() if err != nil { return nil, httperror.InternalServerError("Unable to retrieve environments from the database", err) - } for _, endpoint := range endpoints { if endpoint.GroupID == endpointGroup.ID { - err = handler.updateEndpointRelations(tx, &endpoint, endpointGroup) - if err != nil { + if err := handler.updateEndpointRelations(tx, &endpoint, endpointGroup); err != nil { return nil, httperror.InternalServerError("Unable to persist environment relations changes inside the database", err) } } diff --git a/api/http/handler/endpoints/endpoint_delete.go b/api/http/handler/endpoints/endpoint_delete.go index 54ecd2c91..72b28cbbf 100644 --- a/api/http/handler/endpoints/endpoint_delete.go +++ b/api/http/handler/endpoints/endpoint_delete.go @@ -64,10 +64,9 @@ func (handler *Handler) endpointDelete(w http.ResponseWriter, r *http.Request) * return httperror.BadRequest("Invalid boolean query parameter", err) } - err = handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { + if err := handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { return handler.deleteEndpoint(tx, portainer.EndpointID(endpointID), deleteCluster) - }) - if err != nil { + }); err != nil { var handlerError *httperror.HandlerError if errors.As(err, &handlerError) { return handlerError @@ -87,7 +86,7 @@ func (handler *Handler) endpointDelete(w http.ResponseWriter, r *http.Request) * // @security ApiKeyAuth || jwt // @accept json // @produce json -// @param body body endpointDeleteBatchPayload true "List of environments to delete, with optional deleteCluster flag to clean-up assocaited resources (cloud environments only)" +// @param body body endpointDeleteBatchPayload true "List of environments to delete, with optional deleteCluster flag to clean-up associated resources (cloud environments only)" // @success 204 "Environment(s) successfully deleted." // @failure 207 {object} endpointDeleteBatchPartialResponse "Partial success. Some environments were deleted successfully, while others failed." // @failure 400 "Invalid request payload, such as missing required fields or fields not meeting validation criteria." @@ -139,28 +138,24 @@ func (handler *Handler) deleteEndpoint(tx dataservices.DataStoreTx, endpointID p if endpoint.TLSConfig.TLS { folder := strconv.Itoa(int(endpointID)) - err = handler.FileService.DeleteTLSFiles(folder) - if err != nil { + if err := handler.FileService.DeleteTLSFiles(folder); err != nil { log.Error().Err(err).Msgf("Unable to remove TLS files from disk when deleting endpoint %d", endpointID) } } - err = tx.Snapshot().Delete(endpointID) - if err != nil { + if err := tx.Snapshot().Delete(endpointID); err != nil { log.Warn().Err(err).Msgf("Unable to remove the snapshot from the database") } handler.ProxyManager.DeleteEndpointProxy(endpoint.ID) if len(endpoint.UserAccessPolicies) > 0 || len(endpoint.TeamAccessPolicies) > 0 { - err = handler.AuthorizationService.UpdateUsersAuthorizationsTx(tx) - if err != nil { + if err := handler.AuthorizationService.UpdateUsersAuthorizationsTx(tx); err != nil { log.Warn().Err(err).Msgf("Unable to update user authorizations") } } - err = tx.EndpointRelation().DeleteEndpointRelation(endpoint.ID) - if err != nil { + if err := tx.EndpointRelation().DeleteEndpointRelation(endpoint.ID); err != nil { log.Warn().Err(err).Msgf("Unable to remove environment relation from the database") } @@ -189,8 +184,7 @@ func (handler *Handler) deleteEndpoint(tx dataservices.DataStoreTx, endpointID p return e == endpoint.ID }) - err = tx.EdgeGroup().Update(edgeGroup.ID, &edgeGroup) - if err != nil { + if err := tx.EdgeGroup().Update(edgeGroup.ID, &edgeGroup); err != nil { log.Warn().Err(err).Msgf("Unable to update edge group") } } @@ -247,13 +241,11 @@ func (handler *Handler) deleteEndpoint(tx dataservices.DataStoreTx, endpointID p } // delete the pending actions - err = tx.PendingActions().DeleteByEndpointID(endpoint.ID) - if err != nil { + if err := tx.PendingActions().DeleteByEndpointID(endpoint.ID); err != nil { log.Warn().Err(err).Int("endpointId", int(endpoint.ID)).Msgf("Unable to delete pending actions") } - err = tx.Endpoint().DeleteEndpoint(endpointID) - if err != nil { + if err := tx.Endpoint().DeleteEndpoint(endpointID); err != nil { return httperror.InternalServerError("Unable to delete the environment from the database", err) } diff --git a/api/http/handler/endpoints/endpoint_force_update_service.go b/api/http/handler/endpoints/endpoint_force_update_service.go index 9cd0456ed..ed3fab056 100644 --- a/api/http/handler/endpoints/endpoint_force_update_service.go +++ b/api/http/handler/endpoints/endpoint_force_update_service.go @@ -98,7 +98,7 @@ func (handler *Handler) endpointForceUpdateService(w http.ResponseWriter, r *htt // ignore errors from this cleanup function, log them instead containers, err := dockerClient.ContainerList(context.TODO(), container.ListOptions{ All: true, - Filters: filters.NewArgs(filters.Arg("label", consts.SwarmServiceIdLabel+"="+payload.ServiceID)), + Filters: filters.NewArgs(filters.Arg("label", consts.SwarmServiceIDLabel+"="+payload.ServiceID)), }) if err != nil { log.Warn().Err(err).Str("Environment", endpoint.Name).Msg("Error listing containers") diff --git a/api/http/handler/registries/registry_delete.go b/api/http/handler/registries/registry_delete.go index 814169904..6b24365fa 100644 --- a/api/http/handler/registries/registry_delete.go +++ b/api/http/handler/registries/registry_delete.go @@ -32,8 +32,7 @@ func (handler *Handler) registryDelete(w http.ResponseWriter, r *http.Request) * securityContext, err := security.RetrieveRestrictedRequestContext(r) if err != nil { return httperror.InternalServerError("Unable to retrieve info from request context", err) - } - if !securityContext.IsAdmin { + } else if !securityContext.IsAdmin { return httperror.Forbidden("Permission denied to delete registry", httperrors.ErrResourceAccessDenied) } @@ -47,21 +46,16 @@ func (handler *Handler) registryDelete(w http.ResponseWriter, r *http.Request) * return httperror.InternalServerError(fmt.Sprintf("Unable to load registry %q from the database", registry.Name), err) } - err = handler.DataStore.Registry().Delete(portainer.RegistryID(registryID)) - if err != nil { + if err := handler.DataStore.Registry().Delete(portainer.RegistryID(registryID)); err != nil { return httperror.InternalServerError("Unable to remove the registry from the database", err) } - err = handler.deleteKubernetesSecrets(registry) - if err != nil { - return httperror.InternalServerError("Unable to delete registry secrets", err) - } + handler.deleteKubernetesSecrets(registry) return response.Empty(w) } -func (handler *Handler) deleteKubernetesSecrets(registry *portainer.Registry) error { - +func (handler *Handler) deleteKubernetesSecrets(registry *portainer.Registry) { for endpointId, access := range registry.RegistryAccesses { if access.Namespaces != nil { // Obtain a kubeclient for the endpoint @@ -69,6 +63,7 @@ func (handler *Handler) deleteKubernetesSecrets(registry *portainer.Registry) er if err != nil { // Skip environments that can't be loaded from the DB log.Warn().Err(err).Msgf("Unable to load the environment with id %d from the database", endpointId) + continue } @@ -76,13 +71,14 @@ func (handler *Handler) deleteKubernetesSecrets(registry *portainer.Registry) er if err != nil { // Skip environments that can't get a kubeclient from log.Warn().Err(err).Msgf("Unable to get kubernetes client for environment %d", endpointId) + continue } failedNamespaces := make([]string, 0) + for _, ns := range access.Namespaces { - err = cli.DeleteRegistrySecret(registry.ID, ns) - if err != nil { + if err := cli.DeleteRegistrySecret(registry.ID, ns); err != nil { failedNamespaces = append(failedNamespaces, ns) log.Warn().Err(err).Msgf("Unable to delete registry secret %q from namespace %q for environment %d. Retrying offline", cli.RegistrySecretName(registry.ID), ns, endpointId) } @@ -95,6 +91,4 @@ func (handler *Handler) deleteKubernetesSecrets(registry *portainer.Registry) er } } } - - return nil } diff --git a/api/http/handler/resourcecontrols/resourcecontrol_delete.go b/api/http/handler/resourcecontrols/resourcecontrol_delete.go index 2bb308129..1c8ae268d 100644 --- a/api/http/handler/resourcecontrols/resourcecontrol_delete.go +++ b/api/http/handler/resourcecontrols/resourcecontrol_delete.go @@ -32,11 +32,10 @@ func (handler *Handler) resourceControlDelete(w http.ResponseWriter, r *http.Req if handler.DataStore.IsErrObjectNotFound(err) { return httperror.NotFound("Unable to find a resource control with the specified identifier inside the database", err) } else if err != nil { - return httperror.InternalServerError("Unable to find a resource control with with the specified identifier inside the database", err) + return httperror.InternalServerError("Unable to find a resource control with the specified identifier inside the database", err) } - err = handler.DataStore.ResourceControl().Delete(portainer.ResourceControlID(resourceControlID)) - if err != nil { + if err := handler.DataStore.ResourceControl().Delete(portainer.ResourceControlID(resourceControlID)); err != nil { return httperror.InternalServerError("Unable to remove the resource control from the database", err) } diff --git a/api/http/handler/resourcecontrols/resourcecontrol_update.go b/api/http/handler/resourcecontrols/resourcecontrol_update.go index 576ac6811..409ddb73d 100644 --- a/api/http/handler/resourcecontrols/resourcecontrol_update.go +++ b/api/http/handler/resourcecontrols/resourcecontrol_update.go @@ -31,6 +31,7 @@ func (payload *resourceControlUpdatePayload) Validate(r *http.Request) error { if payload.Public && payload.AdministratorsOnly { return errors.New("invalid payload: cannot set public and administrators only") } + return nil } @@ -58,8 +59,7 @@ func (handler *Handler) resourceControlUpdate(w http.ResponseWriter, r *http.Req } var payload resourceControlUpdatePayload - err = request.DecodeAndValidateJSONPayload(r, &payload) - if err != nil { + if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { return httperror.BadRequest("Invalid request payload", err) } @@ -67,7 +67,7 @@ func (handler *Handler) resourceControlUpdate(w http.ResponseWriter, r *http.Req if handler.DataStore.IsErrObjectNotFound(err) { return httperror.NotFound("Unable to find a resource control with the specified identifier inside the database", err) } else if err != nil { - return httperror.InternalServerError("Unable to find a resource control with with the specified identifier inside the database", err) + return httperror.InternalServerError("Unable to find a resource control with the specified identifier inside the database", err) } securityContext, err := security.RetrieveRestrictedRequestContext(r) @@ -106,8 +106,7 @@ func (handler *Handler) resourceControlUpdate(w http.ResponseWriter, r *http.Req return httperror.Forbidden("Permission denied to update the resource control", httperrors.ErrResourceAccessDenied) } - err = handler.DataStore.ResourceControl().Update(resourceControl.ID, resourceControl) - if err != nil { + if err := handler.DataStore.ResourceControl().Update(resourceControl.ID, resourceControl); err != nil { return httperror.InternalServerError("Unable to persist resource control changes inside the database", err) } diff --git a/api/http/handler/stacks/stack_update.go b/api/http/handler/stacks/stack_update.go index ec7ac2ab4..98d125e0c 100644 --- a/api/http/handler/stacks/stack_update.go +++ b/api/http/handler/stacks/stack_update.go @@ -32,6 +32,7 @@ func (payload *updateComposeStackPayload) Validate(r *http.Request) error { if govalidator.IsNull(payload.StackFileContent) { return errors.New("Invalid stack file content") } + return nil } @@ -50,6 +51,7 @@ func (payload *updateSwarmStackPayload) Validate(r *http.Request) error { if govalidator.IsNull(payload.StackFileContent) { return errors.New("Invalid stack file content") } + return nil } @@ -102,8 +104,7 @@ func (handler *Handler) stackUpdate(w http.ResponseWriter, r *http.Request) *htt 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) } @@ -114,45 +115,40 @@ func (handler *Handler) stackUpdate(w http.ResponseWriter, r *http.Request) *htt //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 editing is disabled for non-admin users" + return httperror.Forbidden(errMsg, errors.New(errMsg)) } - updateError := handler.updateAndDeployStack(r, stack, endpoint) - if updateError != nil { - return updateError + if err := handler.updateAndDeployStack(r, stack, endpoint); err != nil { + return err } user, err := handler.DataStore.User().Read(securityContext.UserID) if err != nil { return httperror.BadRequest("Cannot find context user", errors.Wrap(err, "failed to fetch the user")) } + stack.UpdatedBy = user.Username 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", err) } @@ -165,19 +161,20 @@ func (handler *Handler) stackUpdate(w http.ResponseWriter, r *http.Request) *htt } func (handler *Handler) updateAndDeployStack(r *http.Request, stack *portainer.Stack, endpoint *portainer.Endpoint) *httperror.HandlerError { - if stack.Type == portainer.DockerSwarmStack { + switch stack.Type { + case portainer.DockerSwarmStack: stack.Name = handler.SwarmStackManager.NormalizeStackName(stack.Name) return handler.updateSwarmStack(r, stack, endpoint) - } else if stack.Type == portainer.DockerComposeStack { + case portainer.DockerComposeStack: stack.Name = handler.ComposeStackManager.NormalizeStackName(stack.Name) return handler.updateComposeStack(r, stack, endpoint) - } else if stack.Type == portainer.KubernetesStack { + case portainer.KubernetesStack: return handler.updateKubernetesStack(r, stack, endpoint) - } else { - return httperror.InternalServerError("Unsupported stack", errors.Errorf("unsupported stack type: %v", stack.Type)) } + + return httperror.InternalServerError("Unsupported stack", errors.Errorf("unsupported stack type: %v", stack.Type)) } func (handler *Handler) updateComposeStack(r *http.Request, stack *portainer.Stack, endpoint *portainer.Endpoint) *httperror.HandlerError { @@ -191,8 +188,7 @@ func (handler *Handler) updateComposeStack(r *http.Request, stack *portainer.Sta } var payload updateComposeStackPayload - err := request.DecodeAndValidateJSONPayload(r, &payload) - if err != nil { + if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { return httperror.BadRequest("Invalid request payload", err) } @@ -204,8 +200,7 @@ func (handler *Handler) updateComposeStack(r *http.Request, stack *portainer.Sta } stackFolder := strconv.Itoa(int(stack.ID)) - _, err = handler.FileService.UpdateStoreStackFileFromBytes(stackFolder, stack.EntryPoint, []byte(payload.StackFileContent)) - if err != nil { + if _, err := handler.FileService.UpdateStoreStackFileFromBytes(stackFolder, stack.EntryPoint, []byte(payload.StackFileContent)); err != nil { if rollbackErr := handler.FileService.RollbackStackFile(stackFolder, stack.EntryPoint); rollbackErr != nil { log.Warn().Err(rollbackErr).Msg("rollback stack file error") } @@ -236,8 +231,7 @@ func (handler *Handler) updateComposeStack(r *http.Request, stack *portainer.Sta } // Deploy the stack - err = composeDeploymentConfig.Deploy() - if err != nil { + if err := composeDeploymentConfig.Deploy(); err != nil { if rollbackErr := handler.FileService.RollbackStackFile(stackFolder, stack.EntryPoint); rollbackErr != nil { log.Warn().Err(rollbackErr).Msg("rollback stack file error") } @@ -261,8 +255,7 @@ func (handler *Handler) updateSwarmStack(r *http.Request, stack *portainer.Stack } var payload updateSwarmStackPayload - err := request.DecodeAndValidateJSONPayload(r, &payload) - if err != nil { + if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { return httperror.BadRequest("Invalid request payload", err) } @@ -274,8 +267,7 @@ func (handler *Handler) updateSwarmStack(r *http.Request, stack *portainer.Stack } stackFolder := strconv.Itoa(int(stack.ID)) - _, err = handler.FileService.UpdateStoreStackFileFromBytes(stackFolder, stack.EntryPoint, []byte(payload.StackFileContent)) - if err != nil { + if _, err := handler.FileService.UpdateStoreStackFileFromBytes(stackFolder, stack.EntryPoint, []byte(payload.StackFileContent)); err != nil { if rollbackErr := handler.FileService.RollbackStackFile(stackFolder, stack.EntryPoint); rollbackErr != nil { log.Warn().Err(rollbackErr).Msg("rollback stack file error") } @@ -306,8 +298,7 @@ func (handler *Handler) updateSwarmStack(r *http.Request, stack *portainer.Stack } // Deploy the stack - err = swarmDeploymentConfig.Deploy() - if err != nil { + if err := swarmDeploymentConfig.Deploy(); err != nil { if rollbackErr := handler.FileService.RollbackStackFile(stackFolder, stack.EntryPoint); rollbackErr != nil { log.Warn().Err(rollbackErr).Msg("rollback stack file error") } diff --git a/api/http/handler/templates/template_file.go b/api/http/handler/templates/template_file.go index 33687a4bf..b834eeed9 100644 --- a/api/http/handler/templates/template_file.go +++ b/api/http/handler/templates/template_file.go @@ -71,8 +71,7 @@ func (handler *Handler) templateFile(w http.ResponseWriter, r *http.Request) *ht defer handler.cleanUp(projectPath) - err = handler.GitService.CloneRepository(projectPath, template.Repository.URL, "", "", "", false) - if err != nil { + if err := handler.GitService.CloneRepository(projectPath, template.Repository.URL, "", "", "", false); err != nil { return httperror.InternalServerError("Unable to clone git repository", err) } @@ -82,12 +81,10 @@ func (handler *Handler) templateFile(w http.ResponseWriter, r *http.Request) *ht } return response.JSON(w, fileResponse{FileContent: string(fileContent)}) - } func (handler *Handler) cleanUp(projectPath string) { - err := handler.FileService.RemoveDirectory(projectPath) - if err != nil { + if err := handler.FileService.RemoveDirectory(projectPath); err != nil { log.Debug().Err(err).Msg("HTTP error: unable to cleanup stack creation") } } diff --git a/api/http/handler/webhooks/webhook_execute.go b/api/http/handler/webhooks/webhook_execute.go index ab4cc6d20..5d9f1821c 100644 --- a/api/http/handler/webhooks/webhook_execute.go +++ b/api/http/handler/webhooks/webhook_execute.go @@ -3,7 +3,6 @@ package webhooks import ( "context" "errors" - "io" "net/http" "strings" @@ -26,15 +25,12 @@ import ( // @failure 500 // @router /webhooks/{id} [post] func (handler *Handler) webhookExecute(w http.ResponseWriter, r *http.Request) *httperror.HandlerError { - webhookToken, err := request.RetrieveRouteVariableValue(r, "token") - if err != nil { return httperror.InternalServerError("Invalid service id parameter", err) } webhook, err := handler.DataStore.Webhook().WebhookByToken(webhookToken) - if handler.DataStore.IsErrObjectNotFound(err) { return httperror.NotFound("Unable to find a webhook with this token", err) } else if err != nil { @@ -118,15 +114,12 @@ func (handler *Handler) executeServiceWebhook( if err != nil { return httperror.NotFound("Error pulling image with the specified tag", err) } - defer func(rc io.ReadCloser) { - _ = rc.Close() - }(rc) + defer rc.Close() } - _, err = dockerClient.ServiceUpdate(context.Background(), resourceID, service.Version, service.Spec, serviceUpdateOptions) - - if err != nil { + if _, err := dockerClient.ServiceUpdate(context.Background(), resourceID, service.Version, service.Spec, serviceUpdateOptions); err != nil { return httperror.InternalServerError("Error updating service", err) } + return response.Empty(w) } diff --git a/api/http/offlinegate/offlinegate_test.go b/api/http/offlinegate/offlinegate_test.go index 36ee93941..da9bbd787 100644 --- a/api/http/offlinegate/offlinegate_test.go +++ b/api/http/offlinegate/offlinegate_test.go @@ -71,6 +71,7 @@ func Test_waitingMiddleware_executesImmediately_whenNotLocked(t *testing.T) { timeout := 2 * time.Second start := time.Now() + o.WaitingMiddleware(timeout, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { elapsed := time.Since(start) if elapsed >= timeout { @@ -81,7 +82,7 @@ func Test_waitingMiddleware_executesImmediately_whenNotLocked(t *testing.T) { body, _ := io.ReadAll(response.Body) if string(body) != "success" { - t.Error("Didn't receive expected result from the hanlder") + t.Error("Didn't receive expected result from the handler") } } @@ -105,6 +106,7 @@ func Test_waitingMiddleware_waitsForTheLockToBeReleased(t *testing.T) { timeout := 10 * time.Second start := time.Now() + o.WaitingMiddleware(timeout, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { elapsed := time.Since(start) if elapsed >= timeout { @@ -115,7 +117,7 @@ func Test_waitingMiddleware_waitsForTheLockToBeReleased(t *testing.T) { body, _ := io.ReadAll(response.Body) if string(body) != "success" { - t.Error("Didn't receive expected result from the hanlder") + t.Error("Didn't receive expected result from the handler") } } diff --git a/api/http/proxy/factory/agent.go b/api/http/proxy/factory/agent.go index 06b0e8370..ded36b90a 100644 --- a/api/http/proxy/factory/agent.go +++ b/api/http/proxy/factory/agent.go @@ -86,7 +86,7 @@ func (proxy *ProxyServer) start() error { err := proxy.server.Serve(listener) log.Debug().Str("host", proxyHost).Msg("exiting proxy server") - if err != nil && err != http.ErrServerClosed { + if err != nil && !errors.Is(err, http.ErrServerClosed) { log.Debug().Str("host", proxyHost).Err(err).Msg("proxy server exited with an error") } }() diff --git a/api/http/proxy/factory/agent/transport.go b/api/http/proxy/factory/agent/transport.go index 8abe497eb..076a6d1b8 100644 --- a/api/http/proxy/factory/agent/transport.go +++ b/api/http/proxy/factory/agent/transport.go @@ -22,7 +22,7 @@ func NewTransport(signatureService portainer.DigitalSignatureService, httpTransp return transport } -// RoundTrip is the implementation of the the http.RoundTripper interface +// RoundTrip is the implementation of the http.RoundTripper interface func (transport *Transport) RoundTrip(request *http.Request) (*http.Response, error) { signature, err := transport.signatureService.CreateSignature(portainer.PortainerAgentSignatureMessage) if err != nil { diff --git a/api/http/proxy/factory/azure/containergroup.go b/api/http/proxy/factory/azure/containergroup.go index 36a25b0f4..d16d7109a 100644 --- a/api/http/proxy/factory/azure/containergroup.go +++ b/api/http/proxy/factory/azure/containergroup.go @@ -26,17 +26,16 @@ func (transport *Transport) proxyContainerGroupRequest(request *http.Request) (* } func (transport *Transport) proxyContainerGroupPutRequest(request *http.Request) (*http.Response, error) { - tokenData, err := security.RetrieveTokenData(request) if err != nil { return nil, httperror.Forbidden("Permission denied to access environment", err) } - //add a lock before processing existence check + // Add a lock before processing existence check transport.mutex.Lock() defer transport.mutex.Unlock() - //generate a temp http GET request based on the current PUT request + // Generate a temp http GET request based on the current PUT request validationRequest := &http.Request{ Method: http.MethodGet, URL: request.URL, @@ -45,9 +44,9 @@ func (transport *Transport) proxyContainerGroupPutRequest(request *http.Request) }, } - //fire the request to Azure API to validate if there is an existing container instance with the same name - //positive - reject the request - //negative - continue the process + // Fire the request to Azure API to validate if there is an existing container instance with the same name + // positive - reject the request + // negative - continue the process validationResponse, err := http.DefaultTransport.RoundTrip(validationRequest) if err != nil { return validationResponse, err @@ -63,6 +62,7 @@ func (transport *Transport) proxyContainerGroupPutRequest(request *http.Request) "message": "A container instance with the same name already exists inside the selected resource group", } err = utils.RewriteResponse(resp, errObj, http.StatusConflict) + return resp, err } diff --git a/api/http/proxy/factory/azure/transport.go b/api/http/proxy/factory/azure/transport.go index eeff6550e..37fda30ed 100644 --- a/api/http/proxy/factory/azure/transport.go +++ b/api/http/proxy/factory/azure/transport.go @@ -46,7 +46,7 @@ func NewTransport(credentials *portainer.AzureCredentials, dataStore dataservice } } -// RoundTrip is the implementation of the the http.RoundTripper interface +// RoundTrip is the implementation of the http.RoundTripper interface func (transport *Transport) RoundTrip(request *http.Request) (*http.Response, error) { return transport.proxyAzureRequest(request) } diff --git a/api/http/proxy/factory/docker/transport.go b/api/http/proxy/factory/docker/transport.go index 2e9d88c2f..2ecd79674 100644 --- a/api/http/proxy/factory/docker/transport.go +++ b/api/http/proxy/factory/docker/transport.go @@ -79,7 +79,7 @@ func NewTransport(parameters *TransportParameters, httpTransport *http.Transport return transport, nil } -// RoundTrip is the implementation of the the http.RoundTripper interface +// RoundTrip is the implementation of the http.RoundTripper interface func (transport *Transport) RoundTrip(request *http.Request) (*http.Response, error) { return transport.ProxyDockerRequest(request) } @@ -489,63 +489,65 @@ func (transport *Transport) decorateRegistryAuthenticationHeader(request *http.R } func (transport *Transport) restrictedResourceOperation(request *http.Request, resourceID string, dockerResourceID string, resourceType portainer.ResourceControlType, volumeBrowseRestrictionCheck bool) (*http.Response, error) { - var err error tokenData, err := security.RetrieveTokenData(request) if err != nil { return nil, err } - if tokenData.Role != portainer.AdministratorRole { - if volumeBrowseRestrictionCheck { - securitySettings, err := transport.fetchEndpointSecuritySettings() - if err != nil { - return nil, err - } - - if !securitySettings.AllowVolumeBrowserForRegularUsers { - return utils.WriteAccessDeniedResponse() - } - } + if tokenData.Role == portainer.AdministratorRole { + return transport.executeDockerRequest(request) + } - teamMemberships, err := transport.dataStore.TeamMembership().TeamMembershipsByUserID(tokenData.ID) + if volumeBrowseRestrictionCheck { + securitySettings, err := transport.fetchEndpointSecuritySettings() if err != nil { return nil, err } - userTeamIDs := make([]portainer.TeamID, 0) - for _, membership := range teamMemberships { - userTeamIDs = append(userTeamIDs, membership.TeamID) + if !securitySettings.AllowVolumeBrowserForRegularUsers { + return utils.WriteAccessDeniedResponse() } + } - resourceControls, err := transport.dataStore.ResourceControl().ReadAll() - if err != nil { - return nil, err - } + teamMemberships, err := transport.dataStore.TeamMembership().TeamMembershipsByUserID(tokenData.ID) + if err != nil { + return nil, err + } - resourceControl := authorization.GetResourceControlByResourceIDAndType(resourceID, resourceType, resourceControls) - if resourceControl == nil { - agentTargetHeader := request.Header.Get(portainer.PortainerAgentTargetHeader) + userTeamIDs := make([]portainer.TeamID, 0) + for _, membership := range teamMemberships { + userTeamIDs = append(userTeamIDs, membership.TeamID) + } - if dockerResourceID == "" { - dockerResourceID = resourceID - } + resourceControls, err := transport.dataStore.ResourceControl().ReadAll() + if err != nil { + return nil, err + } - // This resource was created outside of portainer, - // is part of a Docker service or part of a Docker Swarm/Compose stack. - inheritedResourceControl, err := transport.getInheritedResourceControlFromServiceOrStack(dockerResourceID, agentTargetHeader, resourceType, resourceControls) - if err != nil { - return nil, err - } + resourceControl := authorization.GetResourceControlByResourceIDAndType(resourceID, resourceType, resourceControls) + if resourceControl == nil { + agentTargetHeader := request.Header.Get(portainer.PortainerAgentTargetHeader) - if inheritedResourceControl == nil || !authorization.UserCanAccessResource(tokenData.ID, userTeamIDs, inheritedResourceControl) { - return utils.WriteAccessDeniedResponse() - } + if dockerResourceID == "" { + dockerResourceID = resourceID + } + + // This resource was created outside of portainer, + // is part of a Docker service or part of a Docker Swarm/Compose stack. + inheritedResourceControl, err := transport.getInheritedResourceControlFromServiceOrStack(dockerResourceID, agentTargetHeader, resourceType, resourceControls) + if err != nil { + return nil, err } - if resourceControl != nil && !authorization.UserCanAccessResource(tokenData.ID, userTeamIDs, resourceControl) { + if inheritedResourceControl == nil || !authorization.UserCanAccessResource(tokenData.ID, userTeamIDs, inheritedResourceControl) { return utils.WriteAccessDeniedResponse() } } + + if resourceControl != nil && !authorization.UserCanAccessResource(tokenData.ID, userTeamIDs, resourceControl) { + return utils.WriteAccessDeniedResponse() + } + return transport.executeDockerRequest(request) } @@ -587,8 +589,7 @@ func (transport *Transport) rewriteOperation(request *http.Request, operation re } func (transport *Transport) interceptAndRewriteRequest(request *http.Request, operation operationRequest) (*http.Response, error) { - err := operation(request) - if err != nil { + if err := operation(request); err != nil { return nil, err } @@ -653,21 +654,22 @@ func (transport *Transport) executeGenericResourceDeletionOperation(request *htt return response, err } - if response.StatusCode == http.StatusNoContent || response.StatusCode == http.StatusOK { - resourceControl, err := transport.dataStore.ResourceControl().ResourceControlByResourceIDAndType(resourceIdentifierAttribute, resourceType) - if err != nil { - if dataservices.IsErrObjectNotFound(err) { - return response, nil - } + if response.StatusCode != http.StatusNoContent && response.StatusCode != http.StatusOK { + return response, nil + } - return response, err + resourceControl, err := transport.dataStore.ResourceControl().ResourceControlByResourceIDAndType(resourceIdentifierAttribute, resourceType) + if err != nil { + if dataservices.IsErrObjectNotFound(err) { + return response, nil } - if resourceControl != nil { - err = transport.dataStore.ResourceControl().Delete(resourceControl.ID) - if err != nil { - return response, err - } + return response, err + } + + if resourceControl != nil { + if err := transport.dataStore.ResourceControl().Delete(resourceControl.ID); err != nil { + return response, err } } @@ -683,6 +685,7 @@ func (transport *Transport) executeRequestAndRewriteResponse(request *http.Reque if response.StatusCode == http.StatusOK { err = operation(response, executor) } + return response, err } @@ -724,17 +727,19 @@ func (transport *Transport) createRegistryAccessContext(request *http.Request) ( } accessContext.registries = registries - if user.Role != portainer.AdministratorRole { - accessContext.isAdmin = false + if user.Role == portainer.AdministratorRole { + return accessContext, nil + } - teamMemberships, err := transport.dataStore.TeamMembership().TeamMembershipsByUserID(tokenData.ID) - if err != nil { - return nil, err - } + accessContext.isAdmin = false - accessContext.teamMemberships = teamMemberships + teamMemberships, err := transport.dataStore.TeamMembership().TeamMembershipsByUserID(tokenData.ID) + if err != nil { + return nil, err } + accessContext.teamMemberships = teamMemberships + return accessContext, nil } @@ -756,22 +761,24 @@ func (transport *Transport) createOperationContext(request *http.Request) (*rest resourceControls: resourceControls, } - if tokenData.Role != portainer.AdministratorRole { - operationContext.isAdmin = false + if tokenData.Role == portainer.AdministratorRole { + return operationContext, nil + } - teamMemberships, err := transport.dataStore.TeamMembership().TeamMembershipsByUserID(tokenData.ID) - if err != nil { - return nil, err - } + operationContext.isAdmin = false - userTeamIDs := make([]portainer.TeamID, 0) - for _, membership := range teamMemberships { - userTeamIDs = append(userTeamIDs, membership.TeamID) - } + teamMemberships, err := transport.dataStore.TeamMembership().TeamMembershipsByUserID(tokenData.ID) + if err != nil { + return nil, err + } - operationContext.userTeamIDs = userTeamIDs + userTeamIDs := make([]portainer.TeamID, 0) + for _, membership := range teamMemberships { + userTeamIDs = append(userTeamIDs, membership.TeamID) } + operationContext.userTeamIDs = userTeamIDs + return operationContext, nil } diff --git a/api/http/proxy/factory/gitlab/transport.go b/api/http/proxy/factory/gitlab/transport.go index 99e0e6dd9..7e1804c45 100644 --- a/api/http/proxy/factory/gitlab/transport.go +++ b/api/http/proxy/factory/gitlab/transport.go @@ -17,7 +17,7 @@ func NewTransport() *Transport { } } -// RoundTrip is the implementation of the the http.RoundTripper interface +// RoundTrip is the implementation of the http.RoundTripper interface func (transport *Transport) RoundTrip(request *http.Request) (*http.Response, error) { token := request.Header.Get("Private-Token") if token == "" { diff --git a/api/http/proxy/factory/kubernetes.go b/api/http/proxy/factory/kubernetes.go index d36b7f609..497096e91 100644 --- a/api/http/proxy/factory/kubernetes.go +++ b/api/http/proxy/factory/kubernetes.go @@ -1,7 +1,6 @@ package factory import ( - "fmt" "net/http" "net/url" @@ -81,7 +80,7 @@ func (factory *ProxyFactory) newKubernetesEdgeHTTPProxy(endpoint *portainer.Endp } func (factory *ProxyFactory) newKubernetesAgentHTTPSProxy(endpoint *portainer.Endpoint) (http.Handler, error) { - endpointURL := fmt.Sprintf("https://%s", endpoint.URL) + endpointURL := "https://" + endpoint.URL remoteURL, err := url.Parse(endpointURL) if err != nil { return nil, err diff --git a/api/http/proxy/manager.go b/api/http/proxy/manager.go index ad7369064..16f822028 100644 --- a/api/http/proxy/manager.go +++ b/api/http/proxy/manager.go @@ -1,28 +1,28 @@ package proxy import ( + "errors" "fmt" "net/http" + portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/dataservices" dockerclient "github.com/portainer/portainer/api/docker/client" + "github.com/portainer/portainer/api/http/proxy/factory" "github.com/portainer/portainer/api/http/proxy/factory/kubernetes" - - cmap "github.com/orcaman/concurrent-map" "github.com/portainer/portainer/api/kubernetes/cli" - portainer "github.com/portainer/portainer/api" - "github.com/portainer/portainer/api/http/proxy/factory" + cmap "github.com/orcaman/concurrent-map" ) -type ( - // Manager represents a service used to manage proxies to environments (endpoints) and extensions. - Manager struct { - proxyFactory *factory.ProxyFactory - endpointProxies cmap.ConcurrentMap - k8sClientFactory *cli.ClientFactory - } -) +var ErrProxyFactoryNotInitialized = errors.New("proxy factory not initialized") + +// Manager represents a service used to manage proxies to environments (endpoints) and extensions. +type Manager struct { + proxyFactory *factory.ProxyFactory + endpointProxies cmap.ConcurrentMap + k8sClientFactory *cli.ClientFactory +} // NewManager initializes a new proxy Service func NewManager(kubernetesClientFactory *cli.ClientFactory) *Manager { @@ -36,11 +36,11 @@ func (manager *Manager) NewProxyFactory(dataStore dataservices.DataStore, signat manager.proxyFactory = factory.NewProxyFactory(dataStore, signatureService, tunnelService, clientFactory, kubernetesClientFactory, kubernetesTokenCacheManager, gitService, snapshotService) } -// CreateAndRegisterEndpointProxy creates a new HTTP reverse proxy based on environment(endpoint) properties and and adds it to the registered proxies. +// CreateAndRegisterEndpointProxy creates a new HTTP reverse proxy based on environment(endpoint) properties and adds it to the registered proxies. // It can also be used to create a new HTTP reverse proxy and replace an already registered proxy. func (manager *Manager) CreateAndRegisterEndpointProxy(endpoint *portainer.Endpoint) (http.Handler, error) { if manager.proxyFactory == nil { - return nil, fmt.Errorf("proxy factory not init") + return nil, ErrProxyFactoryNotInitialized } proxy, err := manager.proxyFactory.NewEndpointProxy(endpoint) @@ -49,6 +49,7 @@ func (manager *Manager) CreateAndRegisterEndpointProxy(endpoint *portainer.Endpo } manager.endpointProxies.Set(fmt.Sprint(endpoint.ID), proxy) + return proxy, nil } @@ -56,8 +57,9 @@ func (manager *Manager) CreateAndRegisterEndpointProxy(endpoint *portainer.Endpo // It can also be used to create a new HTTP reverse proxy and replace an already registered proxy. func (manager *Manager) CreateAgentProxyServer(endpoint *portainer.Endpoint) (*factory.ProxyServer, error) { if manager.proxyFactory == nil { - return nil, fmt.Errorf("proxy factory not init") + return nil, ErrProxyFactoryNotInitialized } + return manager.proxyFactory.NewAgentProxy(endpoint) } @@ -85,7 +87,8 @@ func (manager *Manager) DeleteEndpointProxy(endpointID portainer.EndpointID) { // CreateGitlabProxy creates a new HTTP reverse proxy that can be used to send requests to the Gitlab API func (manager *Manager) CreateGitlabProxy(url string) (http.Handler, error) { if manager.proxyFactory == nil { - return nil, fmt.Errorf("proxy factory not init") + return nil, ErrProxyFactoryNotInitialized } + return manager.proxyFactory.NewGitlabProxy(url) } diff --git a/api/http/security/bouncer.go b/api/http/security/bouncer.go index 4b7095373..864009db1 100644 --- a/api/http/security/bouncer.go +++ b/api/http/security/bouncer.go @@ -275,7 +275,7 @@ func (bouncer *RequestBouncer) mwIsTeamLeader(next http.Handler) http.Handler { } // mwAuthenticateFirst authenticates a request an auth token. -// A result of a first succeded token lookup would be used for the authentication. +// A result of a first succeeded token lookup would be used for the authentication. func (bouncer *RequestBouncer) mwAuthenticateFirst(tokenLookups []tokenLookup, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { var token *portainer.TokenData diff --git a/api/internal/endpointutils/endpoint_setup.go b/api/internal/endpointutils/endpoint_setup.go index 5b0205cd5..3242d8875 100644 --- a/api/internal/endpointutils/endpoint_setup.go +++ b/api/internal/endpointutils/endpoint_setup.go @@ -8,6 +8,7 @@ import ( "github.com/portainer/portainer/api/crypto" "github.com/portainer/portainer/api/dataservices" "github.com/portainer/portainer/api/http/client" + "github.com/rs/zerolog/log" ) @@ -23,10 +24,9 @@ import ( // will save the signature from the first request after the admin user is created, ensuring that // it matches in the event of a backup restoration. func InitEndpoint(shutdownCtx context.Context, adminCreationDone <-chan struct{}, flags *portainer.CLIFlags, dataStore dataservices.DataStore, snapshotService portainer.SnapshotService) { - select { case <-shutdownCtx.Done(): - log.Debug().Msg("shutdown endpoint initalization") + log.Debug().Msg("shutdown endpoint initialization") case <-adminCreationDone: // Wait for the admin user to be created before initializing the primary endpoint @@ -35,8 +35,7 @@ func InitEndpoint(shutdownCtx context.Context, adminCreationDone <-chan struct{} // 2. Using the API with the /api/users/admin/init endpoint log.Debug().Msg("init primary endpoint") - err := initEndpoint(flags, dataStore, snapshotService) - if err != nil { + if err := initEndpoint(flags, dataStore, snapshotService); err != nil { log.Fatal().Err(err).Msg("failed initializing environment") } } @@ -61,6 +60,7 @@ func initEndpoint(flags *portainer.CLIFlags, dataStore dataservices.DataStore, s if *flags.TLS || *flags.TLSSkipVerify { return createTLSSecuredEndpoint(flags, dataStore, snapshotService) } + return createUnsecuredEndpoint(*flags.EndpointURL, dataStore, snapshotService) } @@ -123,8 +123,7 @@ func createTLSSecuredEndpoint(flags *portainer.CLIFlags, dataStore dataservices. } } - err := snapshotService.SnapshotEndpoint(endpoint) - if err != nil { + if err := snapshotService.SnapshotEndpoint(endpoint); err != nil { log.Error(). Str("endpoint", endpoint.Name). Str("URL", endpoint.URL). @@ -137,8 +136,7 @@ func createTLSSecuredEndpoint(flags *portainer.CLIFlags, dataStore dataservices. func createUnsecuredEndpoint(endpointURL string, dataStore dataservices.DataStore, snapshotService portainer.SnapshotService) error { if strings.HasPrefix(endpointURL, "tcp://") { - _, err := client.ExecutePingOperation(endpointURL, nil) - if err != nil { + if _, err := client.ExecutePingOperation(endpointURL, nil); err != nil { return err } } @@ -172,8 +170,7 @@ func createUnsecuredEndpoint(endpointURL string, dataStore dataservices.DataStor }, } - err := snapshotService.SnapshotEndpoint(endpoint) - if err != nil { + if err := snapshotService.SnapshotEndpoint(endpoint); err != nil { log.Error(). Str("endpoint", endpoint.Name). Str("URL", endpoint.URL).Err(err). diff --git a/api/internal/registryutils/ecr_kube_secret.go b/api/internal/registryutils/ecr_kube_secret.go index 2a7f5a868..811cc10ba 100644 --- a/api/internal/registryutils/ecr_kube_secret.go +++ b/api/internal/registryutils/ecr_kube_secret.go @@ -5,20 +5,20 @@ import ( "github.com/portainer/portainer/api/dataservices" ) -func isRegistryAssignedToNamespace(registry portainer.Registry, endpointID portainer.EndpointID, namespace string) (in bool) { +func isRegistryAssignedToNamespace(registry portainer.Registry, endpointID portainer.EndpointID, namespace string) bool { for _, ns := range registry.RegistryAccesses[endpointID].Namespaces { if ns == namespace { return true } } - return + return false } -func RefreshEcrSecret(cli portainer.KubeClient, endpoint *portainer.Endpoint, dataStore dataservices.DataStore, namespace string) (err error) { +func RefreshEcrSecret(cli portainer.KubeClient, endpoint *portainer.Endpoint, dataStore dataservices.DataStore, namespace string) error { registries, err := dataStore.Registry().ReadAll() if err != nil { - return + return err } for _, registry := range registries { @@ -30,21 +30,18 @@ func RefreshEcrSecret(cli portainer.KubeClient, endpoint *portainer.Endpoint, da continue } - err = EnsureRegTokenValid(dataStore, ®istry) - if err != nil { - return + if err := EnsureRegTokenValid(dataStore, ®istry); err != nil { + return err } - err = cli.DeleteRegistrySecret(registry.ID, namespace) - if err != nil { - return + if err := cli.DeleteRegistrySecret(registry.ID, namespace); err != nil { + return err } - err = cli.CreateRegistrySecret(®istry, namespace) - if err != nil { - return + if err := cli.CreateRegistrySecret(®istry, namespace); err != nil { + return err } } - return + return nil } diff --git a/api/internal/set/set.go b/api/internal/set/set.go index 62f4f111c..4ef31a0d7 100644 --- a/api/internal/set/set.go +++ b/api/internal/set/set.go @@ -14,6 +14,7 @@ func (s Set[T]) Add(key T) { // Contains returns true if the set contains the key. func (s Set[T]) Contains(key T) bool { _, ok := s[key] + return ok } @@ -47,18 +48,17 @@ func (s Set[T]) Keys() []T { // Clear removes all keys from the set. func (s Set[T]) Copy() Set[T] { - copy := make(Set[T]) + c := make(Set[T]) for key := range s { - copy.Add(key) + c.Add(key) } - return copy + return c } // Difference returns a new set containing the keys that are in the first set but not in the second set. func (set Set[T]) Difference(second Set[T]) Set[T] { - difference := set.Copy() for key := range second { @@ -106,5 +106,6 @@ func ToSet[T SetKey](keys []T) Set[T] { for _, key := range keys { set.Add(key) } + return set } diff --git a/api/internal/slices/slices_test.go b/api/internal/slices/slices_test.go index 513c2474e..887a5be45 100644 --- a/api/internal/slices/slices_test.go +++ b/api/internal/slices/slices_test.go @@ -15,7 +15,6 @@ type filterTestCase[T any] struct { } func TestFilter(t *testing.T) { - intTestCases := []filterTestCase[int]{ { name: "Filter even numbers", @@ -59,7 +58,6 @@ func TestFilter(t *testing.T) { } runTestCases(t, stringTestCases) - } func runTestCases[T any](t *testing.T, testCases []filterTestCase[T]) { @@ -85,9 +83,7 @@ func TestMap(t *testing.T) { name: "Map integers to strings", input: []int{1, 2, 3, 4, 5}, expected: []string{"1", "2", "3", "4", "5"}, - mapper: func(n int) string { - return strconv.Itoa(n) - }, + mapper: strconv.Itoa, }, } diff --git a/api/internal/snapshot/snapshot.go b/api/internal/snapshot/snapshot.go index e38d5366a..b03b98e59 100644 --- a/api/internal/snapshot/snapshot.go +++ b/api/internal/snapshot/snapshot.go @@ -57,7 +57,7 @@ func NewService( // NewBackgroundSnapshotter queues snapshots of existing edge environments that // do not have one already func NewBackgroundSnapshotter(dataStore dataservices.DataStore, tunnelService portainer.ReverseTunnelService) { - err := dataStore.ViewTx(func(tx dataservices.DataStoreTx) error { + if err := dataStore.ViewTx(func(tx dataservices.DataStoreTx) error { endpoints, err := tx.Endpoint().Endpoints() if err != nil { return err @@ -76,9 +76,9 @@ func NewBackgroundSnapshotter(dataStore dataservices.DataStore, tunnelService po } return nil - }) - if err != nil { + }); err != nil { log.Error().Err(err).Msg("background snapshotter failure") + return } } @@ -138,6 +138,7 @@ func (service *Service) SnapshotEndpoint(endpoint *portainer.Endpoint) error { if endpoint.Type == portainer.AgentOnDockerEnvironment || endpoint.Type == portainer.AgentOnKubernetesEnvironment { var err error var tlsConfig *tls.Config + if endpoint.TLSConfig.TLS { tlsConfig, err = crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig.TLSCACertPath, endpoint.TLSConfig.TLSCertPath, endpoint.TLSConfig.TLSKeyPath, endpoint.TLSConfig.TLSSkipVerify) if err != nil { @@ -243,6 +244,7 @@ func (service *Service) startSnapshotLoop() { case <-service.shutdownCtx.Done(): log.Debug().Msg("shutting down snapshotting") ticker.Stop() + return case interval := <-service.snapshotIntervalCh: ticker.Reset(interval) @@ -265,6 +267,7 @@ func (service *Service) snapshotEndpoints() error { service.dataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { updateEndpointStatus(tx, &endpoint, snapshotError, service.pendingActionsService) + return nil }) } @@ -284,6 +287,7 @@ func updateEndpointStatus(tx dataservices.DataStoreTx, endpoint *portainer.Endpo } latestEndpointReference.Status = portainer.EndpointStatusUp + if snapshotError != nil { log.Debug(). Str("endpoint", endpoint.Name). @@ -295,8 +299,7 @@ func updateEndpointStatus(tx dataservices.DataStoreTx, endpoint *portainer.Endpo latestEndpointReference.Agent.Version = endpoint.Agent.Version - err = tx.Endpoint().UpdateEndpoint(latestEndpointReference.ID, latestEndpointReference) - if err != nil { + if err := tx.Endpoint().UpdateEndpoint(latestEndpointReference.ID, latestEndpointReference); err != nil { log.Debug(). Str("endpoint", endpoint.Name). Str("URL", endpoint.URL).Err(err). @@ -323,5 +326,6 @@ func FetchDockerID(snapshot portainer.DockerSnapshot) (string, error) { } clusterInfo := swarmInfo.Cluster + return clusterInfo.ID, nil } diff --git a/api/internal/url/url.go b/api/internal/url/url.go index dcae11173..8a752b382 100644 --- a/api/internal/url/url.go +++ b/api/internal/url/url.go @@ -1,7 +1,6 @@ package url import ( - "fmt" "net/url" "strings" ) @@ -16,7 +15,7 @@ func ParseURL(endpointURL string) (*url.URL, error) { !strings.HasPrefix(endpointURL, "//") && !strings.HasPrefix(endpointURL, `unix:`) && !strings.HasPrefix(endpointURL, `npipe:`) { - endpointURL = fmt.Sprintf("//%s", endpointURL) + endpointURL = "//" + endpointURL } return url.Parse(endpointURL) diff --git a/api/kubernetes/cli/pod.go b/api/kubernetes/cli/pod.go index aa8dcf600..04806e50f 100644 --- a/api/kubernetes/cli/pod.go +++ b/api/kubernetes/cli/pod.go @@ -2,7 +2,7 @@ package cli import ( "context" - "fmt" + "strconv" "time" portainer "github.com/portainer/portainer/api" @@ -20,7 +20,7 @@ import ( // - The shell pod will be automatically removed after a specified max life (prevent zombie pods) // - The shell pod will be automatically removed if request is cancelled (or client closes websocket connection) func (kcl *KubeClient) CreateUserShellPod(ctx context.Context, serviceAccountName, shellPodImage string) (*portainer.KubernetesShellPod, error) { - maxPodKeepAliveSecondsStr := fmt.Sprintf("%d", int(portainer.WebSocketKeepAlive.Seconds())) + maxPodKeepAliveSecondsStr := strconv.Itoa(int(portainer.WebSocketKeepAlive.Seconds())) podPrefix := userShellPodPrefix(serviceAccountName) @@ -57,9 +57,10 @@ func (kcl *KubeClient) CreateUserShellPod(ctx context.Context, serviceAccountNam // Wait for pod to reach ready state timeoutCtx, cancelFunc := context.WithTimeout(ctx, 20*time.Second) defer cancelFunc() - err = kcl.waitForPodStatus(timeoutCtx, v1.PodRunning, shellPod) - if err != nil { + + if err := kcl.waitForPodStatus(timeoutCtx, v1.PodRunning, shellPod); err != nil { kcl.cli.CoreV1().Pods(portainerNamespace).Delete(context.TODO(), shellPod.Name, metav1.DeleteOptions{}) + return nil, errors.Wrap(err, "aborting pod creation; error waiting for shell pod ready status") } @@ -91,7 +92,6 @@ func (kcl *KubeClient) CreateUserShellPod(ctx context.Context, serviceAccountNam func (kcl *KubeClient) waitForPodStatus(ctx context.Context, phase v1.PodPhase, pod *v1.Pod) error { log.Debug().Str("pod", pod.Name).Msg("waiting for pod ready") - pollDelay := 500 * time.Millisecond for { select { case <-ctx.Done(): @@ -106,7 +106,7 @@ func (kcl *KubeClient) waitForPodStatus(ctx context.Context, phase v1.PodPhase, return nil } - <-time.After(pollDelay) + time.Sleep(500 * time.Millisecond) } } } diff --git a/api/kubernetes/cli/registries.go b/api/kubernetes/cli/registries.go index 5fc261ce0..89ce27a70 100644 --- a/api/kubernetes/cli/registries.go +++ b/api/kubernetes/cli/registries.go @@ -34,18 +34,17 @@ type ( ) func (kcl *KubeClient) DeleteRegistrySecret(registry portainer.RegistryID, namespace string) error { - err := kcl.cli.CoreV1().Secrets(namespace).Delete(context.TODO(), kcl.RegistrySecretName(registry), metav1.DeleteOptions{}) - if err != nil && !k8serrors.IsNotFound(err) { + if err := kcl.cli.CoreV1().Secrets(namespace).Delete(context.TODO(), kcl.RegistrySecretName(registry), metav1.DeleteOptions{}); err != nil && !k8serrors.IsNotFound(err) { return errors.Wrap(err, "failed removing secret") } return nil } -func (kcl *KubeClient) CreateRegistrySecret(registry *portainer.Registry, namespace string) (err error) { +func (kcl *KubeClient) CreateRegistrySecret(registry *portainer.Registry, namespace string) error { username, password, err := registryutils.GetRegEffectiveCredential(registry) if err != nil { - return + return err } config := dockerConfig{ @@ -79,13 +78,11 @@ func (kcl *KubeClient) CreateRegistrySecret(registry *portainer.Registry, namesp Type: v1.SecretTypeDockerConfigJson, } - _, err = kcl.cli.CoreV1().Secrets(namespace).Create(context.TODO(), secret, metav1.CreateOptions{}) - if err != nil && !k8serrors.IsAlreadyExists(err) { + if _, err := kcl.cli.CoreV1().Secrets(namespace).Create(context.TODO(), secret, metav1.CreateOptions{}); err != nil && !k8serrors.IsAlreadyExists(err) { return errors.Wrap(err, "failed saving secret") } return nil - } func (cli *KubeClient) IsRegistrySecret(namespace, secretName string) (bool, error) { @@ -101,7 +98,6 @@ func (cli *KubeClient) IsRegistrySecret(namespace, secretName string) (bool, err isSecret := secret.Type == v1.SecretTypeDockerConfigJson return isSecret, nil - } func (*KubeClient) RegistrySecretName(registryID portainer.RegistryID) string { diff --git a/api/ldap/ldap.go b/api/ldap/ldap.go index 6202c716a..09e8e6450 100644 --- a/api/ldap/ldap.go +++ b/api/ldap/ldap.go @@ -77,7 +77,7 @@ func (*Service) AuthenticateUser(username, password string, settings *portainer. if err != nil { if errors.Is(err, errUserNotFound) { // prevent user enumeration timing attack by attempting the bind with a fake user - // and whatever password was provided should definately fail + // and whatever password was provided should definitely fail // https://en.wikipedia.org/wiki/Timing_attack userDN = "portainer-fake-ldap-username" } else { diff --git a/api/oauth/oauth_resource.go b/api/oauth/oauth_resource.go index 3c5124070..dc0fa324e 100644 --- a/api/oauth/oauth_resource.go +++ b/api/oauth/oauth_resource.go @@ -2,7 +2,7 @@ package oauth import ( "errors" - "fmt" + "strconv" portainer "github.com/portainer/portainer/api" ) @@ -16,7 +16,7 @@ func getUsername(datamap map[string]interface{}, configuration *portainer.OAuthS if !ok { username, ok := datamap[configuration.UserIdentifier].(float64) if ok && username != 0 { - return fmt.Sprint(int(username)), nil + return strconv.Itoa(int(username)), nil } } diff --git a/api/stacks/stackbuilders/stack_file_content_builder.go b/api/stacks/stackbuilders/stack_file_content_builder.go index f70933991..5e29208a6 100644 --- a/api/stacks/stackbuilders/stack_file_content_builder.go +++ b/api/stacks/stackbuilders/stack_file_content_builder.go @@ -16,7 +16,7 @@ type FileContentMethodStackBuildProcess interface { Deploy(payload *StackPayload, endpoint *portainer.Endpoint) FileContentMethodStackBuildProcess // Save the stack information to database SaveStack() (*portainer.Stack, *httperror.HandlerError) - // Get reponse from http request. Use if it is needed + // Get response from HTTP request. Use if it is needed GetResponse() string // Process the file content SetFileContent(payload *StackPayload) FileContentMethodStackBuildProcess @@ -32,19 +32,15 @@ func (b *FileContentMethodStackBuilder) SetGeneralInfo(payload *StackPayload, en b.stack.EndpointID = endpoint.ID b.stack.Status = portainer.StackStatusActive b.stack.CreationDate = time.Now().Unix() + return b } func (b *FileContentMethodStackBuilder) SetUniqueInfo(payload *StackPayload) FileContentMethodStackBuildProcess { - return b } func (b *FileContentMethodStackBuilder) SetFileContent(payload *StackPayload) FileContentMethodStackBuildProcess { - if b.hasError() { - return b - } - return b } @@ -54,10 +50,8 @@ func (b *FileContentMethodStackBuilder) Deploy(payload *StackPayload, endpoint * } // Deploy the stack - err := b.deploymentConfiger.Deploy() - if err != nil { + if err := b.deploymentConfiger.Deploy(); err != nil { b.err = httperror.InternalServerError(err.Error(), err) - return b } return b diff --git a/api/stacks/stackbuilders/stack_file_upload_builder.go b/api/stacks/stackbuilders/stack_file_upload_builder.go index b431a1400..ab8eb597a 100644 --- a/api/stacks/stackbuilders/stack_file_upload_builder.go +++ b/api/stacks/stackbuilders/stack_file_upload_builder.go @@ -17,7 +17,7 @@ type FileUploadMethodStackBuildProcess interface { Deploy(payload *StackPayload, endpoint *portainer.Endpoint) FileUploadMethodStackBuildProcess // Save the stack information to database SaveStack() (*portainer.Stack, *httperror.HandlerError) - // Get reponse from http request. Use if it is needed + // Get response from HTTP request. Use if it is needed GetResponse() string // Process the upload file SetUploadedFile(payload *StackPayload) FileUploadMethodStackBuildProcess @@ -33,11 +33,11 @@ func (b *FileUploadMethodStackBuilder) SetGeneralInfo(payload *StackPayload, end b.stack.EndpointID = endpoint.ID b.stack.Status = portainer.StackStatusActive b.stack.CreationDate = time.Now().Unix() + return b } func (b *FileUploadMethodStackBuilder) SetUniqueInfo(payload *StackPayload) FileUploadMethodStackBuildProcess { - return b } @@ -52,6 +52,7 @@ func (b *FileUploadMethodStackBuilder) SetUploadedFile(payload *StackPayload) Fi b.err = httperror.InternalServerError("Unable to persist Compose file on disk", err) return b } + b.stack.ProjectPath = projectPath return b @@ -63,13 +64,14 @@ func (b *FileUploadMethodStackBuilder) Deploy(payload *StackPayload, endpoint *p } // Deploy the stack - err := b.deploymentConfiger.Deploy() - if err != nil { + if err := b.deploymentConfiger.Deploy(); err != nil { b.err = httperror.InternalServerError(err.Error(), err) + return b } b.doCleanUp = false + return b } diff --git a/api/stacks/stackbuilders/stack_git_builder.go b/api/stacks/stackbuilders/stack_git_builder.go index ed500e317..f794eca32 100644 --- a/api/stacks/stackbuilders/stack_git_builder.go +++ b/api/stacks/stackbuilders/stack_git_builder.go @@ -23,7 +23,7 @@ type GitMethodStackBuildProcess interface { Deploy(payload *StackPayload, endpoint *portainer.Endpoint) GitMethodStackBuildProcess // Save the stack information to database and return the stack object SaveStack() (*portainer.Stack, *httperror.HandlerError) - // Get reponse from http request. Use if it is needed + // Get response from HTTP request. Use if it is needed GetResponse() string // Set git repository configuration SetGitRepository(payload *StackPayload) GitMethodStackBuildProcess @@ -45,11 +45,11 @@ func (b *GitMethodStackBuilder) SetGeneralInfo(payload *StackPayload, endpoint * b.stack.Status = portainer.StackStatusActive b.stack.CreationDate = time.Now().Unix() b.stack.AutoUpdate = payload.AutoUpdate + return b } func (b *GitMethodStackBuilder) SetUniqueInfo(payload *StackPayload) GitMethodStackBuildProcess { - return b } @@ -74,6 +74,7 @@ func (b *GitMethodStackBuilder) SetGitRepository(payload *StackPayload) GitMetho if payload.ComposeFile == "" { repoConfig.ConfigFilePath = filesystem.ComposeFileDefaultName } + // If a manifest file is specified (for kube git apps), then use it instead of the default compose file name if payload.ManifestFile != "" { repoConfig.ConfigFilePath = payload.ManifestFile @@ -97,6 +98,7 @@ func (b *GitMethodStackBuilder) SetGitRepository(payload *StackPayload) GitMetho // Update the latest commit id repoConfig.ConfigHash = commitHash b.stack.GitConfig = &repoConfig + return b } @@ -134,6 +136,7 @@ func (b *GitMethodStackBuilder) SetAutoUpdate(payload *StackPayload) GitMethodSt b.stack.AutoUpdate.JobID = jobID } + return b } diff --git a/api/stacks/stackbuilders/swarm_file_upload_builder.go b/api/stacks/stackbuilders/swarm_file_upload_builder.go index e60fd8f94..d8be9054b 100644 --- a/api/stacks/stackbuilders/swarm_file_upload_builder.go +++ b/api/stacks/stackbuilders/swarm_file_upload_builder.go @@ -38,11 +38,13 @@ func (b *SwarmStackFileUploadBuilder) SetUniqueInfo(payload *StackPayload) FileU if b.hasError() { return b } + b.stack.Name = payload.Name b.stack.Type = portainer.DockerSwarmStack b.stack.SwarmID = payload.SwarmID b.stack.EntryPoint = filesystem.ComposeFileDefaultName b.stack.Env = payload.Env + return b } diff --git a/pkg/featureflags/featureflags.go b/pkg/featureflags/featureflags.go index 95fd68ba6..09e1997c9 100644 --- a/pkg/featureflags/featureflags.go +++ b/pkg/featureflags/featureflags.go @@ -49,6 +49,7 @@ func IsEnabled(feat Feature) bool { // IsSupported returns true if the feature is supported func IsSupported(feat Feature) bool { _, ok := featureFlags[feat] + return ok } @@ -81,14 +82,15 @@ func Parse(features []string, supportedFeatures []Feature) { if env != "" { envFeatures = strings.Split(env, ",") } + features = append(features, envFeatures...) // loop through feature flags to check if they are supported for _, feat := range features { f := Feature(strings.ToLower(feat)) - _, ok := featureFlags[f] - if !ok { + if _, ok := featureFlags[f]; !ok { log.Warn().Str("feature", f.String()).Msgf("unknown feature flag") + continue } diff --git a/pkg/libhelm/binary/search_repo.go b/pkg/libhelm/binary/search_repo.go index fc3d8aef8..60450d2ee 100644 --- a/pkg/libhelm/binary/search_repo.go +++ b/pkg/libhelm/binary/search_repo.go @@ -65,7 +65,7 @@ func (hbpm *helmBinaryPackageManager) SearchRepo(searchRepoOpts options.SearchRe } } - // Allow redirect behavior to be overriden if specified. + // Allow redirect behavior to be overridden if specified. if client.CheckRedirect == nil { client.CheckRedirect = defaultCheckRedirect } diff --git a/pkg/libhelm/release/release.go b/pkg/libhelm/release/release.go index 06b4eb6ff..531b544aa 100644 --- a/pkg/libhelm/release/release.go +++ b/pkg/libhelm/release/release.go @@ -3,7 +3,7 @@ package release import "github.com/portainer/portainer/pkg/libhelm/time" // Release is the struct that holds the information for a helm release. -// The struct definitions have been copied from the offical Helm Golang client/library. +// The struct definitions have been copied from the official Helm Golang client/library. // ReleaseElement is a struct that represents a release // This is the official struct from the helm project (golang codebase) - exported diff --git a/pkg/libhelm/validate_repo.go b/pkg/libhelm/validate_repo.go index 901dd464c..872dad165 100644 --- a/pkg/libhelm/validate_repo.go +++ b/pkg/libhelm/validate_repo.go @@ -28,7 +28,7 @@ func ValidateHelmRepositoryURL(repoUrl string, client *http.Client) error { if client == nil { client = &http.Client{ - Timeout: time.Second * 120, + Timeout: 120 * time.Second, Transport: http.DefaultTransport, } } diff --git a/pkg/libhttp/error/error.go b/pkg/libhttp/error/error.go index 36ea02071..b7d01bb29 100644 --- a/pkg/libhttp/error/error.go +++ b/pkg/libhttp/error/error.go @@ -1,4 +1,4 @@ -// Package error provides error/logging functions that can be used in conjuction with http.Handler. +// Package error provides error/logging functions that can be used in conjunction with http.Handler. package error import ( @@ -21,8 +21,7 @@ type ( ) func (handler LoggerHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) { - err := handler(rw, r) - if err != nil { + if err := handler(rw, r); err != nil { writeErrorResponse(rw, err) } } @@ -34,6 +33,7 @@ func capitalize(s string) string { // Capitalize the first letter of the word or sentence firstLetter := unicode.ToUpper(rune(s[0])) + return string(firstLetter) + s[1:] } @@ -42,7 +42,12 @@ func writeErrorResponse(rw http.ResponseWriter, err *HandlerError) { err.Err = errors.New(capitalize(err.Message)) } - log.Debug().CallerSkipFrame(2).Err(err.Err).Int("status_code", err.StatusCode).Str("msg", err.Message).Msg("HTTP error") + log.Debug(). + CallerSkipFrame(2). + Err(err.Err). + Int("status_code", err.StatusCode). + Str("msg", err.Message). + Msg("HTTP error") rw.Header().Set("Content-Type", "application/json") rw.WriteHeader(err.StatusCode) diff --git a/pkg/libhttp/request/payload_test.go b/pkg/libhttp/request/payload_test.go index 66408f9a7..bc355ab88 100644 --- a/pkg/libhttp/request/payload_test.go +++ b/pkg/libhttp/request/payload_test.go @@ -22,7 +22,6 @@ func (p *requestPayload) Validate(r *http.Request) error { } func Test_GetPayload(t *testing.T) { - payload := requestPayload{ FirstName: "John", LastName: "Doe", @@ -34,6 +33,7 @@ func Test_GetPayload(t *testing.T) { } r := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(payloadJSON)) + newPayload, err := request.GetPayload[requestPayload](r) if err != nil { t.Fatal(err) diff --git a/pkg/libhttp/request/request.go b/pkg/libhttp/request/request.go index 83c4876fe..fe5f7ca39 100644 --- a/pkg/libhttp/request/request.go +++ b/pkg/libhttp/request/request.go @@ -5,7 +5,7 @@ package request import ( "errors" - "io/ioutil" + "io" "net/http" "strconv" @@ -33,10 +33,11 @@ func RetrieveMultiPartFormFile(request *http.Request, requestParameter string) ( } defer file.Close() - fileContent, err := ioutil.ReadAll(file) + fileContent, err := io.ReadAll(file) if err != nil { return nil, "", err } + return fileContent, headers.Filename, nil } @@ -46,10 +47,10 @@ func RetrieveMultiPartFormJSONValue(request *http.Request, name string, target i value, err := RetrieveMultiPartFormValue(request, name, optional) if err != nil { return err - } - if value == "" { + } else if value == "" { return nil } + return json.Unmarshal([]byte(value), target) } @@ -60,6 +61,7 @@ func RetrieveMultiPartFormValue(request *http.Request, name string, optional boo if value == "" && !optional { return "", errors.New(ErrMissingFormDataValue) } + return value, nil } @@ -70,6 +72,7 @@ func RetrieveNumericMultiPartFormValue(request *http.Request, name string, optio if err != nil { return 0, err } + return strconv.Atoi(value) } @@ -80,6 +83,7 @@ func RetrieveBooleanMultiPartFormValue(request *http.Request, name string, optio if err != nil { return false, err } + return value == "true", nil } @@ -89,10 +93,12 @@ func RetrieveRouteVariableValue(request *http.Request, name string) (string, err if routeVariables == nil { return "", errors.New(ErrInvalidRequestURL) } + routeVar := routeVariables[name] if routeVar == "" { return "", errors.New(ErrInvalidRequestURL) } + return routeVar, nil } @@ -102,6 +108,7 @@ func RetrieveNumericRouteVariableValue(request *http.Request, name string) (int, if err != nil { return 0, err } + return strconv.Atoi(routeVar) } @@ -112,6 +119,7 @@ func RetrieveQueryParameter(request *http.Request, name string, optional bool) ( if queryParameter == "" && !optional { return "", errors.New(ErrMissingQueryParameter) } + return queryParameter, nil } @@ -121,10 +129,10 @@ func RetrieveNumericQueryParameter(request *http.Request, name string, optional queryParameter, err := RetrieveQueryParameter(request, name, optional) if err != nil { return 0, err - } - if queryParameter == "" && optional { + } else if queryParameter == "" && optional { return 0, nil } + return strconv.Atoi(queryParameter) } @@ -135,6 +143,7 @@ func RetrieveBooleanQueryParameter(request *http.Request, name string, optional if err != nil { return false, err } + return queryParameter == "true", nil } @@ -144,9 +153,9 @@ func RetrieveJSONQueryParameter(request *http.Request, name string, target inter queryParameter, err := RetrieveQueryParameter(request, name, optional) if err != nil { return err - } - if queryParameter == "" { + } else if queryParameter == "" { return nil } + return json.Unmarshal([]byte(queryParameter), target) }