From 61f335f056b56c1b5536356b5d6f88b85eeb8dec Mon Sep 17 00:00:00 2001 From: Stavros Foteinopoulos Date: Fri, 28 Mar 2025 12:37:40 +0200 Subject: [PATCH 1/9] fix lint Signed-off-by: Stavros Foteinopoulos --- .github/workflows/ci.yml | 4 ++++ .golangci.yml | 5 +---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f9570fdee..4034be6be 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,11 +18,15 @@ jobs: with: fetch-depth: 0 + - name: Clean duplicate toolchain files + run: rm -rf "$(go env GOPATH)/pkg/mod/golang.org/toolchain*" + - name: Setup Go uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0 with: go-version-file: go.mod + - name: ci/check-style run: make check-style diff --git a/.golangci.yml b/.golangci.yml index ac7c16696..1ab3687bd 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,5 +1,4 @@ -# Docs: https://golangci-lint.run/usage/configuration/#config-file - +version: 2 run: timeout: 5m @@ -20,8 +19,6 @@ linters-settings: linters: disable-all: true enable: - - gofmt - - gosimple - govet - ineffassign - predeclared From 953023524268c057ff6d6e79d4a55a63ebe81de4 Mon Sep 17 00:00:00 2001 From: Stavros Foteinopoulos Date: Fri, 28 Mar 2025 17:54:37 +0200 Subject: [PATCH 2/9] Fix lint problems Signed-off-by: Stavros Foteinopoulos --- Makefile | 6 +- cmd/cloud/clicontext/clicontext.go | 2 +- cmd/cloud/contexts.go | 14 ++-- cmd/cloud/installation.go | 2 +- cmd/cloud/installation_flag.go | 5 +- cmd/cloud/logger_hooks.go | 11 ++- cmd/cloud/server.go | 12 +++- cmd/provisioner-code-gen/main.go | 9 ++- cmd/tools/cloudburst/cloudburst.go | 4 +- cmd/tools/ctest/test.go | 7 +- cmd/tools/ctest/webhooks.go | 5 +- internal/api/cluster.go | 71 ++++++++++++------- internal/api/cluster_installation.go | 19 +++-- internal/api/group.go | 15 +++- internal/api/group_test.go | 5 +- internal/api/installation.go | 55 ++++++++++---- internal/api/installation_backup.go | 9 ++- internal/api/installation_db_migration.go | 9 ++- .../installation_db_restoration_operation.go | 5 +- internal/api/installation_dns.go | 19 +++-- internal/api/response_writer_wrapper.go | 2 +- .../cluster_installation_provisioner.go | 2 +- internal/provisioner/cluster_provisioning.go | 7 +- internal/store/cluster.go | 20 +++--- internal/store/db_multitenant_database.go | 4 +- internal/store/events.go | 6 +- internal/store/group.go | 2 +- internal/store/installation.go | 2 +- internal/store/installation_backup.go | 2 +- internal/store/installation_db_migration.go | 6 +- internal/store/lock.go | 12 +--- internal/store/store.go | 2 +- internal/supervisor/import_test.go | 16 ++--- internal/supervisor/installation.go | 23 +++--- .../supervisor/installation_deletion_test.go | 5 ++ internal/supervisor/scheduler_test.go | 41 ++++++++--- internal/testlib/logger.go | 5 +- internal/tools/argocd/argocd_apps_test.go | 19 ++++- internal/tools/argocd/argocd_test.go | 7 +- internal/tools/aws/cluster_management.go | 2 +- internal/tools/cloudflare/dns.go | 19 ++--- internal/tools/utils/utils.go | 16 +++-- k8s/helpers.go | 13 ++-- k8s/manifest.go | 14 +++- k8s/manifest_test.go | 12 +++- k8s/storage_class_test.go | 2 +- model/cluster.go | 6 +- model/cluster_installation.go | 6 +- model/cluster_installation_request.go | 2 +- model/cluster_request.go | 2 +- model/eks_metadata.go | 5 +- model/events_state_change.go | 2 +- model/events_subscription_request.go | 2 +- model/group_request.go | 2 +- model/id.go | 13 ++-- model/installation_backup_request.go | 2 +- model/installation_db_migration_request.go | 2 +- model/installation_db_restoration_request.go | 2 +- model/installation_request.go | 2 +- model/multitenant_database_request.go | 6 +- model/webhook_request.go | 2 +- model/webhook_test.go | 8 ++- 62 files changed, 408 insertions(+), 201 deletions(-) diff --git a/Makefile b/Makefile index ed197825d..59a088cbf 100644 --- a/Makefile +++ b/Makefile @@ -66,7 +66,7 @@ GOIMPORTS_VER := master GOIMPORTS_BIN := goimports GOIMPORTS := $(TOOLS_BIN_DIR)/$(GOIMPORTS_BIN) -GOLANGCILINT_VER := v1.64.8 +GOLANGCILINT_VER := v2.0.2 GOLANGCILINT_BIN := golangci-lint GOLANGCILINT := $(TOOLS_BIN_DIR)/$(GOLANGCILINT_BIN) @@ -96,12 +96,12 @@ check-style: govet lint goformat goimports ## Runs lint against all packages. lint: $(GOLANGCILINT) @echo Running golangci-lint - $(GOLANGCILINT) run + $(GOLANGCILINT) run --config .golangci.yml ## Runs lint against all packages for changes only lint-changes: $(GOLANGCILINT) @echo Running golangci-lint over changes only - $(GOLANGCILINT) run -n + $(GOLANGCILINT) run -n --config .golangci.yml ## Runs govet against all packages. .PHONY: vet diff --git a/cmd/cloud/clicontext/clicontext.go b/cmd/cloud/clicontext/clicontext.go index a57ea4c7c..88ef7eb89 100644 --- a/cmd/cloud/clicontext/clicontext.go +++ b/cmd/cloud/clicontext/clicontext.go @@ -121,7 +121,7 @@ func WriteContexts(contexts *Contexts) error { if err != nil { return err } - defer file.Close() + defer func() { _ = file.Close() }() encoder := json.NewEncoder(file) err = encoder.Encode(contexts) diff --git a/cmd/cloud/contexts.go b/cmd/cloud/contexts.go index 01b156e34..f87e30cb3 100644 --- a/cmd/cloud/contexts.go +++ b/cmd/cloud/contexts.go @@ -2,6 +2,7 @@ package main import ( "fmt" + log "github.com/sirupsen/logrus" "reflect" "github.com/mattermost/mattermost-cloud/cmd/cloud/clicontext" @@ -90,7 +91,9 @@ type updateContextFlags struct { func (f *updateContextFlags) addFlags(command *cobra.Command) { f.createContextFlags.addFlags(command) command.Flags().StringVar(&f.Context, "context", "", "Name of the context to update") - command.MarkFlagRequired("context") + if err := command.MarkFlagRequired("context"); err != nil { + log.Fatalf("failed to mark flag required: %v", err) + } command.Flags().BoolVar(&f.ClearAuth, "clear-auth", false, "Turns off authentication for this context") command.Flags().BoolVar(&f.ConfirmationRequired, "confirmation-required", false, "Require confirmation for commands run in this context") } @@ -244,8 +247,9 @@ func newCmdContextCreate() *cobra.Command { } flags.addFlags(cmd) - cmd.MarkFlagRequired("server-url") - + if err := cmd.MarkFlagRequired("server-url"); err != nil { + log.Fatalf("failed to mark flag required: %v", err) + } return cmd } @@ -255,7 +259,9 @@ type deleteContextFlags struct { func (f *deleteContextFlags) addFlags(command *cobra.Command) { command.Flags().StringVar(&f.contextName, "context", "", "Name of the context to delete") - command.MarkFlagRequired("context") + if err := command.MarkFlagRequired("context"); err != nil { + log.Fatalf("failed to mark flag required: %v", err) + } } func newCmdContextDelete() *cobra.Command { diff --git a/cmd/cloud/installation.go b/cmd/cloud/installation.go index af94f03c9..c7f3eaf84 100644 --- a/cmd/cloud/installation.go +++ b/cmd/cloud/installation.go @@ -309,7 +309,7 @@ func newCmdInstallationUpdateDeletion() *cobra.Command { client := createClient(command.Context(), flags.clusterFlags) request := &model.PatchInstallationDeletionRequest{} - if flags.installationDeletionPatchRequestOptionsChanged.futureDeletionTimeChanged { + if flags.futureDeletionTimeChanged { newExpiryTimeMillis := model.GetMillisAtTime(time.Now().Add(flags.futureDeletionTime)) request.DeletionPendingExpiry = &newExpiryTimeMillis } diff --git a/cmd/cloud/installation_flag.go b/cmd/cloud/installation_flag.go index 3d239aa06..70191cb07 100644 --- a/cmd/cloud/installation_flag.go +++ b/cmd/cloud/installation_flag.go @@ -186,7 +186,10 @@ func (flags *installationUpdateFlags) GetPatchInstallationRequest() (*model.Patc if flags.allowedIPRangesChanged { allowedIPRanges := &model.AllowedIPRanges{} - allowedIPRanges.FromJSONString(flags.allowedIPRanges) + _, jsonErr := allowedIPRanges.FromJSONString(flags.allowedIPRanges) + if jsonErr != nil { + return nil, jsonErr + } allowedIPRanges, err := allowedIPRanges.FromJSONString(flags.allowedIPRanges) if err != nil { return nil, err diff --git a/cmd/cloud/logger_hooks.go b/cmd/cloud/logger_hooks.go index 6acbc55d3..50176b343 100644 --- a/cmd/cloud/logger_hooks.go +++ b/cmd/cloud/logger_hooks.go @@ -82,7 +82,11 @@ func (hook *ClusterLoggerHook) fileWrite(entry *logrus.Entry, kind string) error log.Println("failed to open logfile:", path, err) return err } - defer fd.Close() + defer func() { + if err := fd.Close(); err != nil { + log.Printf("failed to close fd: %v", err) + } + }() // use our formatter instead of entry.String() msg, err := hook.formatter.Format(entry) @@ -91,7 +95,10 @@ func (hook *ClusterLoggerHook) fileWrite(entry *logrus.Entry, kind string) error log.Println("failed to generate string for entry:", err) return err } - fd.Write(msg) + if _, err := fd.Write(msg); err != nil { + log.Println("failed to write to logfile:", err) + return err + } return nil } diff --git a/cmd/cloud/server.go b/cmd/cloud/server.go index bedb57f63..04e0b5298 100644 --- a/cmd/cloud/server.go +++ b/cmd/cloud/server.go @@ -413,7 +413,11 @@ func executeServerCmd(flags serverFlags) error { } standardSupervisor := supervisor.NewScheduler(multiDoer, time.Duration(flags.poll)*time.Second, logger) - defer standardSupervisor.Close() + defer func() { + if err := standardSupervisor.Close(); err != nil { + logrus.WithError(err).Error("failed to close standardSupervisor") + } + }() if flags.slowPoll == 0 { logger.WithField("slow-poll", flags.slowPoll).Info("Slow scheduler is disabled") @@ -422,7 +426,11 @@ func executeServerCmd(flags serverFlags) error { var slowMultiDoer supervisor.MultiDoer slowMultiDoer = append(slowMultiDoer, supervisor.NewInstallationDeletionSupervisor(instanceID, flags.installationDeletionPendingTime, flags.installationDeletionMaxUpdating, sqlStore, eventsProducer, logger)) slowSupervisor := supervisor.NewScheduler(slowMultiDoer, time.Duration(flags.slowPoll)*time.Second, logger) - defer slowSupervisor.Close() + defer func() { + if err := slowSupervisor.Close(); err != nil { + logrus.WithError(err).Error("failed to close slowSupervisor") + } + }() } metricsRouter := mux.NewRouter() diff --git a/cmd/provisioner-code-gen/main.go b/cmd/provisioner-code-gen/main.go index 8412bd6aa..d84ebf4a9 100644 --- a/cmd/provisioner-code-gen/main.go +++ b/cmd/provisioner-code-gen/main.go @@ -43,8 +43,13 @@ func newRootCmd() *cobra.Command { // Binds all flags as viper values func bindFlags(cmd *cobra.Command) { - viper.BindPFlags(cmd.PersistentFlags()) - viper.BindPFlags(cmd.Flags()) + if err := viper.BindPFlags(cmd.PersistentFlags()); err != nil { + logrus.WithError(err).Fatal("failed to bind persistent flags") + } + if err := viper.BindPFlags(cmd.Flags()); err != nil { + logrus.WithError(err).Fatal("failed to bind command flags") + } + for _, c := range cmd.Commands() { bindFlags(c) } diff --git a/cmd/tools/cloudburst/cloudburst.go b/cmd/tools/cloudburst/cloudburst.go index 7dbe9c830..6d3066204 100644 --- a/cmd/tools/cloudburst/cloudburst.go +++ b/cmd/tools/cloudburst/cloudburst.go @@ -345,7 +345,9 @@ func (b *Blaster) cleanupInstallations(installations map[string]*cloud.Installat fallthrough case cloud.InstallationStateDeletionInProgress: default: - b.client.DeleteInstallation(fetched.ID) + if err := b.client.DeleteInstallation(fetched.ID); err != nil { + log.WithError(err).Error("failed to delete installation") + } } } } diff --git a/cmd/tools/ctest/test.go b/cmd/tools/ctest/test.go index 2d86036c9..84aa9b233 100644 --- a/cmd/tools/ctest/test.go +++ b/cmd/tools/ctest/test.go @@ -7,6 +7,7 @@ package main import ( "encoding/json" "fmt" + log "github.com/sirupsen/logrus" "io" "net/http" @@ -36,7 +37,11 @@ func runInstallationLifecycleTest(request *model.CreateInstallationRequest, clie if err != nil { return errors.Wrap(err, "failed to run enhanced ping test") } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + log.WithError(err).Error("failed to close resp.Body") + } + }() if resp.StatusCode != http.StatusOK { var body []byte diff --git a/cmd/tools/ctest/webhooks.go b/cmd/tools/ctest/webhooks.go index 09e2b1eb3..8187dd24b 100644 --- a/cmd/tools/ctest/webhooks.go +++ b/cmd/tools/ctest/webhooks.go @@ -10,11 +10,10 @@ import ( "net/http" "time" - "github.com/mattermost/mattermost-cloud/model" cloud "github.com/mattermost/mattermost-cloud/model" ) -func webhookHandler(w http.ResponseWriter, r *http.Request, c chan *model.WebhookPayload) { +func webhookHandler(w http.ResponseWriter, r *http.Request, c chan *cloud.WebhookPayload) { webhook, err := cloud.WebhookPayloadFromReader(r.Body) if err != nil { logger.WithError(err).Error("Faled to parse webhook") @@ -41,7 +40,7 @@ func webhookHandler(w http.ResponseWriter, r *http.Request, c chan *model.Webhoo w.WriteHeader(http.StatusOK) } -func startWebhookListener(port string, c chan *model.WebhookPayload) func() { +func startWebhookListener(port string, c chan *cloud.WebhookPayload) func() { logger.Infof("Starting cloud webhook listener on port %s", port) srv := &http.Server{Addr: fmt.Sprintf(":%s", port)} diff --git a/internal/api/cluster.go b/internal/api/cluster.go index 42de7705b..739cf4637 100644 --- a/internal/api/cluster.go +++ b/internal/api/cluster.go @@ -5,6 +5,7 @@ package api import ( + log "github.com/sirupsen/logrus" "net/http" "github.com/gorilla/mux" @@ -141,7 +142,9 @@ func handleCreateCluster(c *Context, w http.ResponseWriter, r *http.Request) { c.Logger.WithError(err).Error("Failed to create cluster state change event") } - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -186,7 +189,9 @@ func handleRetryCreateCluster(c *Context, w http.ResponseWriter, r *http.Request // Notify even if we didn't make changes, to expedite even the no-op operations above. unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -252,7 +257,9 @@ func handleImportCluster(c *Context, w http.ResponseWriter, r *http.Request) { c.Logger.WithError(err).Error("Failed to create cluster state change event") } - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -310,7 +317,9 @@ func handleProvisionCluster(c *Context, w http.ResponseWriter, r *http.Request) // Notify even if we didn't make changes, to expedite even the no-op operations above. unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -386,10 +395,11 @@ func handleUpgradeKubernetes(c *Context, w http.ResponseWriter, r *http.Request) oldState := clusterDTO.State var isUpgradeApplied bool - if clusterDTO.Provisioner == model.ProvisionerKops { - isUpgradeApplied = clusterDTO.ProvisionerMetadataKops.ApplyUpgradePatch(upgradeClusterRequest) - } else if clusterDTO.Provisioner == model.ProvisionerEKS { + switch clusterDTO.Provisioner { + case model.ProvisionerEKS: isUpgradeApplied = clusterDTO.ProvisionerMetadataEKS.ApplyUpgradePatch(upgradeClusterRequest) + default: + isUpgradeApplied = clusterDTO.ProvisionerMetadataKops.ApplyUpgradePatch(upgradeClusterRequest) } if isUpgradeApplied { @@ -410,7 +420,9 @@ func handleUpgradeKubernetes(c *Context, w http.ResponseWriter, r *http.Request) } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -440,29 +452,28 @@ func handleResizeCluster(c *Context, w http.ResponseWriter, r *http.Request) { } defer unlockOnce() - if clusterDTO.Provisioner == model.ProvisionerEKS { + switch clusterDTO.Provisioner { + case model.ProvisionerEKS: err = clusterDTO.ProvisionerMetadataEKS.ValidateClusterSizePatch(resizeClusterRequest) - if err != nil { - c.Logger.WithError(err).Error("failed to validate cluster size patch") - w.WriteHeader(http.StatusBadRequest) - return - } - } else if clusterDTO.Provisioner == model.ProvisionerKops { + case model.ProvisionerKops: err = clusterDTO.ProvisionerMetadataKops.ValidateClusterSizePatch(resizeClusterRequest) - if err != nil { - c.Logger.WithError(err).Error("failed to validate cluster size patch") - w.WriteHeader(http.StatusBadRequest) - return - } + default: + // Optionally handle unsupported provisioners here. + } + if err != nil { + c.Logger.WithError(err).Error("failed to validate cluster size patch") + w.WriteHeader(http.StatusBadRequest) + return } oldState := clusterDTO.State var isResizeApplied bool - if clusterDTO.Provisioner == model.ProvisionerKops { - isResizeApplied = clusterDTO.ProvisionerMetadataKops.ApplyClusterSizePatch(resizeClusterRequest) - } else if clusterDTO.Provisioner == model.ProvisionerEKS { + switch clusterDTO.Provisioner { + case model.ProvisionerEKS: isResizeApplied = clusterDTO.ProvisionerMetadataEKS.ApplyClusterSizePatch(resizeClusterRequest) + default: + isResizeApplied = clusterDTO.ProvisionerMetadataKops.ApplyClusterSizePatch(resizeClusterRequest) } if isResizeApplied { @@ -483,7 +494,9 @@ func handleResizeCluster(c *Context, w http.ResponseWriter, r *http.Request) { } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -705,7 +718,9 @@ func handleDeleteCluster(c *Context, w http.ResponseWriter, r *http.Request) { } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.WriteHeader(http.StatusAccepted) } @@ -810,7 +825,11 @@ func annotationsFromRequest(req *http.Request) ([]*model.Annotation, error) { if err != nil { return nil, errors.Wrap(err, "failed to decode request") } - defer req.Body.Close() + defer func() { + if err := req.Body.Close(); err != nil { + log.WithError(err).Error("failed to close req.Body") + } + }() annotations, err := model.AnnotationsFromStringSlice(annotationsRequest.Annotations) if err != nil { diff --git a/internal/api/cluster_installation.go b/internal/api/cluster_installation.go index 7f3f38c56..4dbb8e2f5 100644 --- a/internal/api/cluster_installation.go +++ b/internal/api/cluster_installation.go @@ -141,7 +141,10 @@ func handleGetClusterInstallationConfig(c *Context, w http.ResponseWriter, r *ht } w.WriteHeader(http.StatusOK) - w.Write(output) + if _, err := w.Write(output); err != nil { + log.WithError(err).Error("failed to write output") + } + } // handleSetClusterInstallationConfig responds to PUT /api/cluster_installation/{cluster_installation}/config, merging the given config into the given cluster installation. @@ -311,12 +314,16 @@ func handleRunClusterInstallationExecCommand(c *Context, w http.ResponseWriter, if execErr != nil { c.Logger.WithError(execErr).Error("failed to execute command") w.WriteHeader(http.StatusConflict) - w.Write(output) + if _, err := w.Write(output); err != nil { + log.WithError(err).Error("failed to write output") + } return } w.WriteHeader(http.StatusOK) - w.Write(output) + if _, err := w.Write(output); err != nil { + log.WithError(err).Error("failed to write output") + } } // handleRunClusterInstallationGetPPROF responds to POST /api/cluster_installation/{cluster_installation}/pprof, @@ -381,7 +388,11 @@ func handleRunClusterInstallationGetPPROF(c *Context, w http.ResponseWriter, r * w.WriteHeader(http.StatusInternalServerError) return } - defer os.RemoveAll(tempDir) + defer func() { + if err := os.RemoveAll(tempDir); err != nil { + log.WithError(err).Error("failed to remove tempDir") + } + }() tempZipPath := path.Join(tempDir, fmt.Sprintf("%s.tempprof.zip", clusterInstallationID)) tempZipFile, err := os.Create(tempZipPath) diff --git a/internal/api/group.go b/internal/api/group.go index 1831ca5c3..d4d5acf68 100644 --- a/internal/api/group.go +++ b/internal/api/group.go @@ -123,7 +123,10 @@ func handleCreateGroup(c *Context, w http.ResponseWriter, r *http.Request) { return } - c.Supervisor.Do() + err = c.Supervisor.Do() + if err != nil { + return + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) @@ -165,7 +168,10 @@ func handleUpdateGroup(c *Context, w http.ResponseWriter, r *http.Request) { } } - c.Supervisor.Do() + err = c.Supervisor.Do() + if err != nil { + return + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) @@ -215,7 +221,10 @@ func handleDeleteGroup(c *Context, w http.ResponseWriter, r *http.Request) { return } - c.Supervisor.Do() + err = c.Supervisor.Do() + if err != nil { + return + } w.WriteHeader(http.StatusOK) } diff --git a/internal/api/group_test.go b/internal/api/group_test.go index cfe8b6eb1..eccfd2c39 100644 --- a/internal/api/group_test.go +++ b/internal/api/group_test.go @@ -745,9 +745,10 @@ func TestGroupsStatus(t *testing.T) { require.NotNil(t, groupsStatus) assert.Len(t, groupsStatus, 2) for _, gs := range groupsStatus { - if gs.ID == group.ID { + switch gs.ID { + case group.ID: assert.Equal(t, expectedStatusGroup1, gs) - } else if gs.ID == group2.ID { + case group2.ID: assert.Equal(t, expectedStatusGroup2, gs) } } diff --git a/internal/api/installation.go b/internal/api/installation.go index d979ad287..b16fcfc1f 100644 --- a/internal/api/installation.go +++ b/internal/api/installation.go @@ -7,6 +7,7 @@ package api import ( "encoding/json" "fmt" + log "github.com/sirupsen/logrus" "net/http" "time" @@ -278,7 +279,9 @@ func handleCreateInstallation(c *Context, w http.ResponseWriter, r *http.Request } groupUnlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -347,7 +350,9 @@ func handleRetryCreateInstallation(c *Context, w http.ResponseWriter, r *http.Re // Notify even if we didn't make changes, to expedite even the no-op operations above. unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -396,7 +401,9 @@ func handleUpdateInstallation(c *Context, w http.ResponseWriter, r *http.Request } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -469,7 +476,9 @@ func handleCreateInstallationVolume(c *Context, w http.ResponseWriter, r *http.R } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -544,7 +553,9 @@ func handleUpdateInstallationVolume(c *Context, w http.ResponseWriter, r *http.R } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -606,7 +617,9 @@ func handleDeleteInstallationVolume(c *Context, w http.ResponseWriter, r *http.R } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -627,7 +640,9 @@ func handleJoinGroup(c *Context, w http.ResponseWriter, r *http.Request) { return } - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.WriteHeader(http.StatusOK) } @@ -645,7 +660,11 @@ func handleAssignGroup(c *Context, w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusBadRequest) return } - defer r.Body.Close() + defer func() { + if err := r.Body.Close(); err != nil { + log.WithError(err).Error("failed to close r.Body") + } + }() if len(assignRequest.GroupSelectionAnnotations) == 0 { c.Logger.Error("no annotations provided") @@ -730,7 +749,9 @@ func handleLeaveGroup(c *Context, w http.ResponseWriter, r *http.Request) { } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.WriteHeader(http.StatusOK) } @@ -759,7 +780,9 @@ func handleHibernateInstallation(c *Context, w http.ResponseWriter, r *http.Requ } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -806,7 +829,9 @@ func handleWakeupInstallation(c *Context, w http.ResponseWriter, r *http.Request } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -898,7 +923,9 @@ func handleDeleteInstallation(c *Context, w http.ResponseWriter, r *http.Request } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.WriteHeader(http.StatusAccepted) } @@ -965,7 +992,9 @@ func handleCancelInstallationDeletion(c *Context, w http.ResponseWriter, r *http } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.WriteHeader(http.StatusAccepted) } diff --git a/internal/api/installation_backup.go b/internal/api/installation_backup.go index b67f956e1..e488a767e 100644 --- a/internal/api/installation_backup.go +++ b/internal/api/installation_backup.go @@ -5,6 +5,7 @@ package api import ( + log "github.com/sirupsen/logrus" "net/http" "time" @@ -66,7 +67,9 @@ func handleRequestInstallationBackup(c *Context, w http.ResponseWriter, r *http. return } - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) @@ -194,7 +197,9 @@ func handleDeleteInstallationBackup(c *Context, w http.ResponseWriter, r *http.R } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.WriteHeader(http.StatusAccepted) } diff --git a/internal/api/installation_db_migration.go b/internal/api/installation_db_migration.go index e892b2177..6c1de24fb 100644 --- a/internal/api/installation_db_migration.go +++ b/internal/api/installation_db_migration.go @@ -5,6 +5,7 @@ package api import ( + log "github.com/sirupsen/logrus" "net/http" "time" @@ -121,7 +122,9 @@ func handleTriggerInstallationDatabaseMigration(c *Context, w http.ResponseWrite } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.WriteHeader(http.StatusAccepted) outputJSON(c, w, dbMigrationOperation) @@ -276,7 +279,9 @@ func handleRollbackInstallationDatabaseMigration(c *Context, w http.ResponseWrit } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.WriteHeader(http.StatusAccepted) outputJSON(c, w, dbMigrationOperation) diff --git a/internal/api/installation_db_restoration_operation.go b/internal/api/installation_db_restoration_operation.go index d168d0252..2cb51bc37 100644 --- a/internal/api/installation_db_restoration_operation.go +++ b/internal/api/installation_db_restoration_operation.go @@ -5,6 +5,7 @@ package api import ( + log "github.com/sirupsen/logrus" "net/http" "github.com/gorilla/mux" @@ -72,7 +73,9 @@ func handleTriggerInstallationDBRestoration(c *Context, w http.ResponseWriter, r } unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) diff --git a/internal/api/installation_dns.go b/internal/api/installation_dns.go index f9474252d..fd5d3e774 100644 --- a/internal/api/installation_dns.go +++ b/internal/api/installation_dns.go @@ -5,6 +5,7 @@ package api import ( + log "github.com/sirupsen/logrus" "net/http" "strings" @@ -26,7 +27,11 @@ func handleAddDNSRecord(c *Context, w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusBadRequest) return } - defer r.Body.Close() + defer func() { + if err := r.Body.Close(); err != nil { + log.WithError(err).Error("failed to close r.Body") + } + }() newState := model.InstallationStateUpdateRequested installationDTO, status, unlockOnce := getInstallationForTransition(c, installationID, newState) @@ -70,7 +75,9 @@ func handleAddDNSRecord(c *Context, w http.ResponseWriter, r *http.Request) { installationDTO.DNSRecords = append(installationDTO.DNSRecords, dnsRecord) unlockOnce() - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -143,7 +150,9 @@ func handleSetDomainNamePrimary(c *Context, w http.ResponseWriter, r *http.Reque return } - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) @@ -230,7 +239,9 @@ func handleDeleteDNSRecord(c *Context, w http.ResponseWriter, r *http.Request) { return } - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusAccepted) diff --git a/internal/api/response_writer_wrapper.go b/internal/api/response_writer_wrapper.go index 368b80b7c..08dcea312 100644 --- a/internal/api/response_writer_wrapper.go +++ b/internal/api/response_writer_wrapper.go @@ -52,7 +52,7 @@ func (rw *ResponseWriterWrapper) Write(data []byte) (int, error) { // Hijack calls the underlying writer's Hijack output. func (rw *ResponseWriterWrapper) Hijack() (net.Conn, *bufio.ReadWriter, error) { if rw.hijacker == nil { - return nil, nil, errors.New("Hijacker interface not supported by the wrapped ResponseWriter") + return nil, nil, errors.New("hijacker interface not supported by the wrapped ResponseWriter") } return rw.hijacker.Hijack() } diff --git a/internal/provisioner/cluster_installation_provisioner.go b/internal/provisioner/cluster_installation_provisioner.go index cbd68f5b2..b5f438338 100644 --- a/internal/provisioner/cluster_installation_provisioner.go +++ b/internal/provisioner/cluster_installation_provisioner.go @@ -417,7 +417,7 @@ func (provisioner Provisioner) updateClusterInstallation( logger.WithField("status", fmt.Sprintf("%+v", mattermost.Status)).Debug("Got mattermost installation") - mattermost.ObjectMeta.Labels = generateClusterInstallationResourceLabels(installation, clusterInstallation, cluster) + mattermost.Labels = generateClusterInstallationResourceLabels(installation, clusterInstallation, cluster) mattermost.Spec.ResourceLabels = clusterInstallationStableLabels(installation, clusterInstallation, cluster) mattermost.Spec.Scheduling.Affinity = generateAffinityConfig(installation, clusterInstallation, cluster) diff --git a/internal/provisioner/cluster_provisioning.go b/internal/provisioner/cluster_provisioning.go index dcd5e7090..ab48827e5 100644 --- a/internal/provisioner/cluster_provisioning.go +++ b/internal/provisioner/cluster_provisioning.go @@ -411,10 +411,11 @@ func provisionCluster( logger.Info("pgbouncer configmap updated successfully") var clusterName string - if cluster.Provisioner == model.ProvisionerKops { - clusterName = cluster.ProvisionerMetadataKops.Name - } else if cluster.Provisioner == model.ProvisionerEKS { + switch cluster.Provisioner { + case model.ProvisionerEKS: clusterName = cluster.ProvisionerMetadataEKS.Name + default: + clusterName = cluster.ProvisionerMetadataKops.Name } logger.WithField("name", clusterName).Info("Successfully provisioned cluster") diff --git a/internal/store/cluster.go b/internal/store/cluster.go index 8f343ef09..c6f116632 100644 --- a/internal/store/cluster.go +++ b/internal/store/cluster.go @@ -90,12 +90,12 @@ func (r *rawCluster) toCluster() (*model.Cluster, error) { var err error if r.Provider == model.ProviderExternal { - r.Cluster.ProviderMetadataExternal, err = model.NewExternalProviderMetadata(r.ProviderMetadataRaw) + r.ProviderMetadataExternal, err = model.NewExternalProviderMetadata(r.ProviderMetadataRaw) if err != nil { return nil, errors.Wrap(err, "unable to marshal ProviderMetadataExternal") } } else { - r.Cluster.ProviderMetadataAWS, err = model.NewAWSMetadata(r.ProviderMetadataRaw) + r.ProviderMetadataAWS, err = model.NewAWSMetadata(r.ProviderMetadataRaw) if err != nil { return nil, err } @@ -107,29 +107,29 @@ func (r *rawCluster) toCluster() (*model.Cluster, error) { switch r.Provisioner { case model.ProvisionerKops: - r.Cluster.ProvisionerMetadataKops, err = model.NewKopsMetadata(r.ProvisionerMetadataRaw) + r.ProvisionerMetadataKops, err = model.NewKopsMetadata(r.ProvisionerMetadataRaw) if err != nil { return nil, err } - if r.Cluster.ProvisionerMetadataKops != nil { - r.Cluster.Networking = r.Cluster.ProvisionerMetadataKops.Networking + if r.ProvisionerMetadataKops != nil { + r.Networking = r.ProvisionerMetadataKops.Networking } case model.ProvisionerEKS: - r.Cluster.ProvisionerMetadataEKS, err = model.NewEKSMetadata(r.ProvisionerMetadataRaw) + r.ProvisionerMetadataEKS, err = model.NewEKSMetadata(r.ProvisionerMetadataRaw) if err != nil { return nil, err } - if r.Cluster.ProvisionerMetadataEKS != nil { - r.Cluster.Networking = r.Cluster.ProvisionerMetadataEKS.Networking + if r.ProvisionerMetadataEKS != nil { + r.Networking = r.ProvisionerMetadataEKS.Networking } case model.ProvisionerExternal: - r.Cluster.ProvisionerMetadataExternal, err = model.NewExternalClusterMetadata(r.ProvisionerMetadataRaw) + r.ProvisionerMetadataExternal, err = model.NewExternalClusterMetadata(r.ProvisionerMetadataRaw) if err != nil { return nil, err } } - r.Cluster.UtilityMetadata, err = model.NewUtilityMetadata(r.UtilityMetadataRaw) + r.UtilityMetadata, err = model.NewUtilityMetadata(r.UtilityMetadataRaw) if err != nil { return nil, err } diff --git a/internal/store/db_multitenant_database.go b/internal/store/db_multitenant_database.go index a620d1ead..ca48c157a 100644 --- a/internal/store/db_multitenant_database.go +++ b/internal/store/db_multitenant_database.go @@ -50,13 +50,13 @@ type rawMultitenantDatabases []*rawMultitenantDatabase func (r *rawMultitenantDatabase) toMultitenantDatabase() (*model.MultitenantDatabase, error) { // We only need to set values that are converted from a raw database format. if r.InstallationsRaw != nil { - err := json.Unmarshal(r.InstallationsRaw, &r.MultitenantDatabase.Installations) + err := json.Unmarshal(r.InstallationsRaw, &r.Installations) if err != nil { return nil, err } } if r.MigratedInstallationsRaw != nil { - err := json.Unmarshal(r.MigratedInstallationsRaw, &r.MultitenantDatabase.MigratedInstallations) + err := json.Unmarshal(r.MigratedInstallationsRaw, &r.MigratedInstallations) if err != nil { return nil, err } diff --git a/internal/store/events.go b/internal/store/events.go index b87235c9b..d5cc6210f 100644 --- a/internal/store/events.go +++ b/internal/store/events.go @@ -249,10 +249,10 @@ func (sqlStore *SQLStore) GetStateChangeEvent(eventID string) (*model.StateChang func (sqlStore *SQLStore) GetStateChangeEvents(filter *model.StateChangeEventFilter) ([]*model.StateChangeEventData, error) { query := stateChangeEventSelect.OrderBy("e.Timestamp DESC") - if filter.Paging.PerPage != model.AllPerPage { + if filter.PerPage != model.AllPerPage { query = query. - Limit(uint64(filter.Paging.PerPage)). - Offset(uint64(filter.Paging.Page * filter.Paging.PerPage)) + Limit(uint64(filter.PerPage)). + Offset(uint64(filter.Page * filter.PerPage)) } if filter.ResourceType != "" { diff --git a/internal/store/group.go b/internal/store/group.go index 50c874cfe..65f4ae214 100644 --- a/internal/store/group.go +++ b/internal/store/group.go @@ -45,7 +45,7 @@ func (r *rawGroup) toGroup() (*model.Group, error) { } } - r.Group.MattermostEnv = *mattermostEnv + r.MattermostEnv = *mattermostEnv return r.Group, nil } diff --git a/internal/store/installation.go b/internal/store/installation.go index 8aebaebe6..ea4b5cd91 100644 --- a/internal/store/installation.go +++ b/internal/store/installation.go @@ -35,7 +35,7 @@ func init() { } type rawInstallation struct { - *model.Installation + Installation *model.Installation MattermostEnvRaw []byte PriorityEnvRaw []byte SingleTenantDatabaseConfigRaw []byte diff --git a/internal/store/installation_backup.go b/internal/store/installation_backup.go index fe8786f8e..8f3ebd6ef 100644 --- a/internal/store/installation_backup.go +++ b/internal/store/installation_backup.go @@ -54,7 +54,7 @@ func (r *rawInstallationBackup) toInstallationBackup() (*model.InstallationBacku if err != nil { return nil, err } - r.InstallationBackup.DataResidence = &dataResidence + r.DataResidence = &dataResidence } return r.InstallationBackup, nil diff --git a/internal/store/installation_db_migration.go b/internal/store/installation_db_migration.go index 160a2c246..4830b5e23 100644 --- a/internal/store/installation_db_migration.go +++ b/internal/store/installation_db_migration.go @@ -40,9 +40,9 @@ func init() { } type rawDBMigrationOperation struct { - *model.InstallationDBMigrationOperation - SourceMultiTenantRaw []byte - DestinationMultiTenantRaw []byte + InstallationDBMigrationOperation *model.InstallationDBMigrationOperation + SourceMultiTenantRaw []byte + DestinationMultiTenantRaw []byte } type rawDBMigrationOperations []*rawDBMigrationOperation diff --git a/internal/store/lock.go b/internal/store/lock.go index fe68de47c..9452f86e2 100644 --- a/internal/store/lock.go +++ b/internal/store/lock.go @@ -49,12 +49,9 @@ func (sqlStore *SQLStore) lockRowsTx(db execer, table, lockByName, lockAtName st return false, errors.Wrap(err, "failed to count rows affected") } - locked := false - if count > 0 { - locked = true - } + locked := count > 0 - if count > 0 && int(count) < len(ids) { + if locked && int(count) < len(ids) { sqlStore.logger.Warnf("Locked only %d of %d rows in %s", count, len(ids), table) } @@ -102,10 +99,7 @@ func (sqlStore *SQLStore) unlockRowsTx(table, lockByName, lockAtName string, ids return false, errors.Wrap(err, "failed to count rows affected") } - unlocked := false - if count > 0 { - unlocked = true - } + unlocked := count > 0 if int(count) < len(ids) { sqlStore.logger.Warnf("Unlocked only %d of %d rows in %s", count, len(ids), table) diff --git a/internal/store/store.go b/internal/store/store.go index 7685972a0..709c37434 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -210,7 +210,7 @@ func (t *Transaction) Commit() error { // RollbackUnlessCommitted rollback the transaction if it is not committed. func (t *Transaction) RollbackUnlessCommitted() { if !t.committed { - err := t.Tx.Rollback() + err := t.Rollback() if err != nil { t.sqlStore.logger.Errorf("error: failed to rollback uncommitted transaction: %s", err.Error()) } diff --git a/internal/supervisor/import_test.go b/internal/supervisor/import_test.go index 9fa034900..c5d0fd430 100644 --- a/internal/supervisor/import_test.go +++ b/internal/supervisor/import_test.go @@ -25,14 +25,14 @@ func TestImportSupervisor(t *testing.T) { gmctrl := gomock.NewController(t) defer gmctrl.Finish() var ( - translationID string = "some-translation-id" - importID string = "some-import-id" - installationID string = "some-installation-id" - sourceBucket string = "awat-bucket" - destBucket string = "the-multitenant-bucket" - inputArchive string = "user_upload.zip" - - resource string = fmt.Sprintf("%s/%s", sourceBucket, inputArchive) + translationID = "some-translation-id" + importID = "some-import-id" + installationID = "some-installation-id" + sourceBucket = "awat-bucket" + destBucket = "the-multitenant-bucket" + inputArchive = "user_upload.zip" + + resource = fmt.Sprintf("%s/%s", sourceBucket, inputArchive) ) t.Run("successfully import a translation", func(t *testing.T) { diff --git a/internal/supervisor/installation.go b/internal/supervisor/installation.go index daa4c77ff..0c96b51af 100644 --- a/internal/supervisor/installation.go +++ b/internal/supervisor/installation.go @@ -602,10 +602,7 @@ func (s *InstallationSupervisor) createClusterInstallation(cluster *model.Cluste podPercent := clusterResources.CalculatePodCountPercentUsed(installationPodCountRequirement) // Determine if a resource check should be performed. - performResourceCheck := true - if cluster.IsExternallyManaged() && s.scheduling.AlwaysScheduleExternalClusters { - performResourceCheck = false - } + performResourceCheck := !cluster.IsExternallyManaged() || !s.scheduling.AlwaysScheduleExternalClusters resourcesOverThreshold := cpuPercent > s.scheduling.ClusterResourceThresholdCPU || memoryPercent > s.scheduling.ClusterResourceThresholdMemory || @@ -614,10 +611,11 @@ func (s *InstallationSupervisor) createClusterInstallation(cluster *model.Cluste if performResourceCheck && resourcesOverThreshold { var provisionerMetadata model.ProvisionerMetadata - if cluster.Provisioner == model.ProvisionerKops { - provisionerMetadata = cluster.ProvisionerMetadataKops.GetCommonMetadata() - } else if cluster.Provisioner == model.ProvisionerEKS { + switch cluster.Provisioner { + case model.ProvisionerEKS: provisionerMetadata = cluster.ProvisionerMetadataEKS.GetCommonMetadata() + default: + provisionerMetadata = cluster.ProvisionerMetadataKops.GetCommonMetadata() } if s.scheduling.ClusterResourceThresholdScaleValue == 0 || @@ -647,11 +645,8 @@ func (s *InstallationSupervisor) createClusterInstallation(cluster *model.Cluste } cluster.State = model.ClusterStateResizeRequested - if cluster.Provisioner == model.ProvisionerKops { - cluster.ProvisionerMetadataKops.ChangeRequest = &model.KopsMetadataRequestedState{ - NodeMinCount: newWorkerCount, - } - } else if cluster.Provisioner == model.ProvisionerEKS { + switch cluster.Provisioner { + case model.ProvisionerEKS: cluster.ProvisionerMetadataEKS.ChangeRequest = &model.EKSMetadataRequestedState{ NodeGroups: map[string]model.NodeGroupMetadata{ model.NodeGroupWorker: { @@ -660,6 +655,10 @@ func (s *InstallationSupervisor) createClusterInstallation(cluster *model.Cluste }, }, } + default: + cluster.ProvisionerMetadataKops.ChangeRequest = &model.KopsMetadataRequestedState{ + NodeMinCount: newWorkerCount, + } } logger.WithField("cluster", cluster.ID).Infof("Scaling cluster worker nodes from %d to %d (max=%d)", provisionerMetadata.NodeMinCount, newWorkerCount, provisionerMetadata.NodeMaxCount) diff --git a/internal/supervisor/installation_deletion_test.go b/internal/supervisor/installation_deletion_test.go index 0fb071424..ea9efc822 100644 --- a/internal/supervisor/installation_deletion_test.go +++ b/internal/supervisor/installation_deletion_test.go @@ -5,6 +5,7 @@ package supervisor_test import ( + log "github.com/sirupsen/logrus" "testing" "time" @@ -288,6 +289,10 @@ func TestInstallationDeletionSupervisor_Supervise(t *testing.T) { time.Sleep(1 * time.Millisecond) supervisor.Do() + if err := supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } + installation2, err = sqlStore.GetInstallation(installation2.ID, false, false) require.NoError(t, err) require.Equal(t, model.InstallationStateDeletionPending, installation2.State) diff --git a/internal/supervisor/scheduler_test.go b/internal/supervisor/scheduler_test.go index 5a8d3278c..8102b8022 100644 --- a/internal/supervisor/scheduler_test.go +++ b/internal/supervisor/scheduler_test.go @@ -5,6 +5,7 @@ package supervisor_test import ( + log "github.com/sirupsen/logrus" "testing" "time" @@ -41,7 +42,11 @@ func TestScheduler(t *testing.T) { calls: make(chan bool, 1), } scheduler := supervisor.NewScheduler(doer, 100*time.Millisecond, logger) - defer scheduler.Close() + defer func() { + if err := scheduler.Close(); err != nil { + log.WithError(err).Error("failed to close scheduler") + } + }() for i := 0; i < 5; i++ { select { @@ -59,9 +64,15 @@ func TestScheduler(t *testing.T) { calls: make(chan bool, 1), } scheduler := supervisor.NewScheduler(doer, 30*time.Second, logger) - defer scheduler.Close() + defer func() { + if err := scheduler.Close(); err != nil { + log.WithError(err).Error("failed to close scheduler") + } + }() - scheduler.Do() + if err := scheduler.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } select { case <-doer.calls: @@ -77,9 +88,15 @@ func TestScheduler(t *testing.T) { calls: make(chan bool, 1), } scheduler := supervisor.NewScheduler(doer, 30*time.Second, logger) - scheduler.Close() + defer func() { + if err := scheduler.Close(); err != nil { + log.WithError(err).Error("failed to close scheduler") + } + }() - scheduler.Do() + if err := scheduler.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } select { case <-doer.calls: @@ -95,14 +112,22 @@ func TestScheduler(t *testing.T) { calls: make(chan bool), } scheduler := supervisor.NewScheduler(doer, 30*time.Second, logger) - defer scheduler.Close() + defer func() { + if err := scheduler.Close(); err != nil { + log.WithError(err).Error("failed to close scheduler") + } + }() - scheduler.Do() + if err := scheduler.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } time.Sleep(1 * time.Second) // Second call should be non-blocking - scheduler.Do() + if err := scheduler.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } select { case <-doer.calls: diff --git a/internal/testlib/logger.go b/internal/testlib/logger.go index 469da804c..ba5ca2ab9 100644 --- a/internal/testlib/logger.go +++ b/internal/testlib/logger.go @@ -10,7 +10,6 @@ import ( "github.com/golang/mock/gomock" mocks "github.com/mattermost/mattermost-cloud/internal/mocks/logger" - "github.com/sirupsen/logrus" log "github.com/sirupsen/logrus" ) @@ -46,6 +45,6 @@ func NewMockedFieldLogger(ctrl *gomock.Controller) *MockedFieldLogger { } // NewLoggerEntry returns a new logger entry instance. -func NewLoggerEntry() *logrus.Entry { - return logrus.NewEntry(logrus.New()) +func NewLoggerEntry() *log.Entry { + return log.NewEntry(log.New()) } diff --git a/internal/tools/argocd/argocd_apps_test.go b/internal/tools/argocd/argocd_apps_test.go index 625bb0676..5b49bf4c3 100644 --- a/internal/tools/argocd/argocd_apps_test.go +++ b/internal/tools/argocd/argocd_apps_test.go @@ -1,6 +1,7 @@ package argocd import ( + log "github.com/sirupsen/logrus" "os" "reflect" "testing" @@ -14,7 +15,11 @@ func TestAddClusterIDLabel(t *testing.T) { if err != nil { t.Fatalf("Error creating temporary YAML file: %v", err) } - defer os.Remove(tmpfile.Name()) + defer func() { + if err := os.Remove(tmpfile.Name()); err != nil { + log.WithError(err).Error("failed to remove tmpfile") + } + }() testYAML := []byte(` applications: @@ -107,7 +112,11 @@ func TestRemoveClusterIDLabel(t *testing.T) { if err != nil { t.Fatalf("Error creating temporary YAML file: %v", err) } - defer os.Remove(tmpfile.Name()) + defer func() { + if err := os.Remove(tmpfile.Name()); err != nil { + log.WithError(err).Error("failed to remove tmpfile") + } + }() testYAML := []byte(` applications: @@ -200,7 +209,11 @@ func TestClusterIDLabelAlreadyExists(t *testing.T) { if err != nil { t.Fatalf("Error creating temporary YAML file: %v", err) } - defer os.Remove(tmpfile.Name()) + defer func() { + if err := os.Remove(tmpfile.Name()); err != nil { + log.WithError(err).Error("failed to remove tmpfile") + } + }() testYAML := []byte(` applications: diff --git a/internal/tools/argocd/argocd_test.go b/internal/tools/argocd/argocd_test.go index 72fd6a051..6a81cb482 100644 --- a/internal/tools/argocd/argocd_test.go +++ b/internal/tools/argocd/argocd_test.go @@ -2,6 +2,7 @@ package argocd import ( "encoding/base64" + log "github.com/sirupsen/logrus" "os" "path" "testing" @@ -95,7 +96,11 @@ func (suite *ArgoClusterRegisterTestSuite) SetupSuite() { } func (suite *ArgoClusterRegisterTestSuite) TearDownSuite() { - os.RemoveAll(suite.tempDir) + defer func() { + if err := os.RemoveAll(suite.tempDir); err != nil { + log.WithError(err).Error("failed to remove tempDir") + } + }() } func (suite *ArgoClusterRegisterTestSuite) TestReadArgoK8sRegistrationFile() { diff --git a/internal/tools/aws/cluster_management.go b/internal/tools/aws/cluster_management.go index 05ac92d7c..07757ac43 100644 --- a/internal/tools/aws/cluster_management.go +++ b/internal/tools/aws/cluster_management.go @@ -506,7 +506,7 @@ func (a *Client) claimVpc(clusterResources ClusterResources, cluster *model.Clus // // If any of the VPC checks either returns no VPCs or more than one VPC this method will fail. func (a *Client) releaseVpc(cluster *model.Cluster, logger log.FieldLogger) error { - var isSecondaryCluster bool = false + isSecondaryCluster := false secondaryVpcFilters := []ec2Types.Filter{ { Name: aws.String(VpcAvailableTagKey), diff --git a/internal/tools/cloudflare/dns.go b/internal/tools/cloudflare/dns.go index 4b418268f..97fa5dea9 100644 --- a/internal/tools/cloudflare/dns.go +++ b/internal/tools/cloudflare/dns.go @@ -142,18 +142,13 @@ func (c *Client) upsertDNS(zoneNameList []string, dnsName, dnsEndpoint string, l } recordToUpdate := existingRecords[0] - doUpdate := false - - if recordToUpdate.Content != record.Content || recordToUpdate.TTL != record.TTL { - doUpdate = true - } - if recordToUpdate.Proxied != nil && record.Proxied != nil { - if *recordToUpdate.Proxied != *record.Proxied { - doUpdate = true - } - } else if recordToUpdate.Proxied != record.Proxied { - doUpdate = true - } + doUpdate := (recordToUpdate.Content != record.Content || recordToUpdate.TTL != record.TTL) || + func() bool { + if recordToUpdate.Proxied != nil && record.Proxied != nil { + return *recordToUpdate.Proxied != *record.Proxied + } + return recordToUpdate.Proxied != record.Proxied + }() if doUpdate { err = c.updateDNSRecord(zoneID, recordToUpdate.ID, record) diff --git a/internal/tools/utils/utils.go b/internal/tools/utils/utils.go index c84bddb71..0f7e26310 100644 --- a/internal/tools/utils/utils.go +++ b/internal/tools/utils/utils.go @@ -61,16 +61,24 @@ func copyFile(source string, dest string) error { return err } - defer sourcefile.Close() + defer func() { + if err := sourcefile.Close(); err != nil { + log.WithError(err).Error("failed to close sourcefile") + } + }() - destfile, err := os.Create(dest) + destFile, err := os.Create(dest) if err != nil { return err } - defer destfile.Close() + defer func() { + if err := destFile.Close(); err != nil { + log.WithError(err).Error("failed to close destFile") + } + }() - _, err = io.Copy(destfile, sourcefile) + _, err = io.Copy(destFile, sourcefile) if err == nil { sourceinfo, err := os.Stat(source) if err != nil { diff --git a/k8s/helpers.go b/k8s/helpers.go index 896db49e0..fd92e39d7 100644 --- a/k8s/helpers.go +++ b/k8s/helpers.go @@ -101,12 +101,17 @@ func (kc *KubeClient) GetPodsFromDaemonSet(namespace, daemonSetName string) (*co func (kc *KubeClient) PatchPodsDaemonSet(namespace, daemonSetName string, payload []PatchStringValue) error { ctx := context.TODO() daemonSet := kc.Clientset.AppsV1().DaemonSets(namespace) - payloadBytes, _ := json.Marshal(payload) - _, err := daemonSet.Patch(ctx, daemonSetName, types.JSONPatchType, payloadBytes, metav1.PatchOptions{}) + + payloadBytes, err := json.Marshal(payload) if err != nil { - errors.Wrapf(err, "failed to patch daemonSet %s", daemonSetName) - return err + return errors.Wrapf(err, "failed to marshal payload for daemonSet %s", daemonSetName) } + + _, err = daemonSet.Patch(ctx, daemonSetName, types.JSONPatchType, payloadBytes, metav1.PatchOptions{}) + if err != nil { + return errors.Wrapf(err, "failed to patch daemonSet %s", daemonSetName) + } + return nil } diff --git a/k8s/manifest.go b/k8s/manifest.go index 2857592d1..525e9402d 100644 --- a/k8s/manifest.go +++ b/k8s/manifest.go @@ -71,9 +71,17 @@ func (kc *KubeClient) CreateFromFile(file ManifestFile, installationName string) } install.Install(scheme.Scheme) - apixv1beta1scheme.AddToScheme(scheme.Scheme) - mattermostscheme.AddToScheme(scheme.Scheme) - monitoringscheme.AddToScheme(scheme.Scheme) + if err := apixv1beta1scheme.AddToScheme(scheme.Scheme); err != nil { + log.WithError(err).Fatal("failed to add apixv1beta1 scheme") + } + + if err := mattermostscheme.AddToScheme(scheme.Scheme); err != nil { + log.WithError(err).Fatal("failed to add mattermost scheme") + } + + if err := monitoringscheme.AddToScheme(scheme.Scheme); err != nil { + log.WithError(err).Fatal("failed to add monitoring scheme") + } logger := kc.logger.WithFields(log.Fields{ "file": file.Basename(), diff --git a/k8s/manifest_test.go b/k8s/manifest_test.go index f51c212dc..64a0e34b6 100644 --- a/k8s/manifest_test.go +++ b/k8s/manifest_test.go @@ -129,7 +129,11 @@ func TestCreate(t *testing.T) { tempDir, err := os.MkdirTemp(".", "k8s-file-testing-") assert.NoError(t, err) - defer os.RemoveAll(tempDir) + defer func() { + if err := os.RemoveAll(tempDir); err != nil { + t.Errorf("failed to remove tempDir: %v", err) + } + }() serviceYAML := filepath.Join(tempDir, "service.yaml") err = os.WriteFile(serviceYAML, []byte(exampleServiceYAML), 0600) @@ -203,7 +207,11 @@ func TestCreateNetworkPolicy(t *testing.T) { tempDir, err := os.MkdirTemp(".", "k8s-file-testing-netpol") assert.NoError(t, err) - defer os.RemoveAll(tempDir) + defer func() { + if err := os.RemoveAll(tempDir); err != nil { + t.Errorf("failed to remove tempDir: %v", err) + } + }() networkPolYAML := filepath.Join(tempDir, "netpol.yaml") err = os.WriteFile(networkPolYAML, []byte(exampleNetPolYAML), 0600) diff --git a/k8s/storage_class_test.go b/k8s/storage_class_test.go index e55788e6f..1258f07f9 100644 --- a/k8s/storage_class_test.go +++ b/k8s/storage_class_test.go @@ -22,7 +22,7 @@ func TestStorageClass(t *testing.T) { t.Run("update storageclass", func(t *testing.T) { ctx := context.TODO() - testClient.Clientset.StorageV1beta1().StorageClasses().Create(ctx, storageClass, metav1.CreateOptions{}) + _, _ = testClient.Clientset.StorageV1beta1().StorageClasses().Create(ctx, storageClass, metav1.CreateOptions{}) result, err := testClient.UpdateStorageClassVolumeBindingMode(class) require.NoError(t, err) require.Equal(t, storageClass.GetName(), result.GetName()) diff --git a/model/cluster.go b/model/cluster.go index 591c1c8b8..a3679b514 100644 --- a/model/cluster.go +++ b/model/cluster.go @@ -6,6 +6,7 @@ package model import ( "encoding/json" + log "github.com/sirupsen/logrus" "io" "regexp" ) @@ -41,7 +42,10 @@ type Cluster struct { func (c *Cluster) Clone() *Cluster { var clone Cluster data, _ := json.Marshal(c) - json.Unmarshal(data, &clone) + if err := json.Unmarshal(data, &clone); err != nil { + log.WithError(err).Error("failed to unmarshal data, returning nil cluster") + return nil + } return &clone } diff --git a/model/cluster_installation.go b/model/cluster_installation.go index 71414c1f4..5cec69241 100644 --- a/model/cluster_installation.go +++ b/model/cluster_installation.go @@ -7,6 +7,7 @@ package model import ( "encoding/json" "fmt" + log "github.com/sirupsen/logrus" "io" "github.com/pkg/errors" @@ -117,7 +118,10 @@ type MigrateClusterInstallationResponse struct { func (c *ClusterInstallation) Clone() *ClusterInstallation { var clone ClusterInstallation data, _ := json.Marshal(c) - json.Unmarshal(data, &clone) + if err := json.Unmarshal(data, &clone); err != nil { + log.WithError(err).Error("failed to unmarshal data, returning nil cluster") + return nil + } return &clone } diff --git a/model/cluster_installation_request.go b/model/cluster_installation_request.go index df6217a32..e9837b0ae 100644 --- a/model/cluster_installation_request.go +++ b/model/cluster_installation_request.go @@ -24,7 +24,7 @@ func (request *GetClusterInstallationsRequest) ApplyToURL(u *url.URL) { q := u.Query() q.Add("cluster", request.ClusterID) q.Add("installation", request.InstallationID) - request.Paging.AddToQuery(q) + request.AddToQuery(q) u.RawQuery = q.Encode() } diff --git a/model/cluster_request.go b/model/cluster_request.go index 63593fdbf..572cab166 100644 --- a/model/cluster_request.go +++ b/model/cluster_request.go @@ -315,7 +315,7 @@ type GetClustersRequest struct { // ApplyToURL modifies the given url to include query string parameters for the request. func (request *GetClustersRequest) ApplyToURL(u *url.URL) { q := u.Query() - request.Paging.AddToQuery(q) + request.AddToQuery(q) u.RawQuery = q.Encode() } diff --git a/model/eks_metadata.go b/model/eks_metadata.go index 18feff3cf..a8097f21f 100644 --- a/model/eks_metadata.go +++ b/model/eks_metadata.go @@ -264,10 +264,7 @@ func (em *EKSMetadata) ValidateChangeRequest() error { return errors.New("the EKS Metadata ChangeRequest is nil") } - changeAllowed := false - if len(changeRequest.Version) != 0 || len(changeRequest.AMI) != 0 || changeRequest.MaxPodsPerNode != 0 { - changeAllowed = true - } + changeAllowed := len(changeRequest.Version) != 0 || len(changeRequest.AMI) != 0 || changeRequest.MaxPodsPerNode != 0 if changeAllowed { return nil diff --git a/model/events_state_change.go b/model/events_state_change.go index c54ff6ad2..329d0e77f 100644 --- a/model/events_state_change.go +++ b/model/events_state_change.go @@ -117,7 +117,7 @@ func (request *ListStateChangeEventsRequest) ApplyToURL(u *url.URL) { q := u.Query() q.Add("resource_type", string(request.ResourceType)) q.Add("resource_id", request.ResourceID) - request.Paging.AddToQuery(q) + request.AddToQuery(q) u.RawQuery = q.Encode() } diff --git a/model/events_subscription_request.go b/model/events_subscription_request.go index 7f56bb198..b3e0cf57d 100644 --- a/model/events_subscription_request.go +++ b/model/events_subscription_request.go @@ -76,7 +76,7 @@ func (request *ListSubscriptionsRequest) ApplyToURL(u *url.URL) { q := u.Query() q.Add("owner", request.Owner) q.Add("event_type", string(request.EventType)) - request.Paging.AddToQuery(q) + request.AddToQuery(q) u.RawQuery = q.Encode() } diff --git a/model/group_request.go b/model/group_request.go index 292613141..c0cb4feeb 100644 --- a/model/group_request.go +++ b/model/group_request.go @@ -169,7 +169,7 @@ func (request *GetGroupsRequest) ApplyToURL(u *url.URL) { if request.WithInstallationCount { q.Add(ShowInstallationCountQueryParameter, "true") } - request.Paging.AddToQuery(q) + request.AddToQuery(q) u.RawQuery = q.Encode() } diff --git a/model/id.go b/model/id.go index 538c8b13f..16a3f8072 100644 --- a/model/id.go +++ b/model/id.go @@ -7,11 +7,10 @@ package model import ( "bytes" "encoding/base32" + "github.com/sirupsen/logrus" "math/rand" "time" "unicode" - - "github.com/pborman/uuid" ) var encoding = base32.NewEncoding("ybndrfg8ejkmcpqxot1uwisza345h769") @@ -22,8 +21,14 @@ var encoding = base32.NewEncoding("ybndrfg8ejkmcpqxot1uwisza345h769") func NewID() string { var b bytes.Buffer encoder := base32.NewEncoder(encoding, &b) - encoder.Write(uuid.NewRandom()) - encoder.Close() + if err := encoder.Close(); err != nil { + logrus.WithError(err).Error("failed to close encoder") + return err.Error() + } + if err := encoder.Close(); err != nil { + logrus.WithError(err).Error("failed to close encoder") + return err.Error() + } b.Truncate(26) // removes the '==' padding return b.String() } diff --git a/model/installation_backup_request.go b/model/installation_backup_request.go index 0690a9d06..3a276f54c 100644 --- a/model/installation_backup_request.go +++ b/model/installation_backup_request.go @@ -43,7 +43,7 @@ func (request *GetInstallationBackupsRequest) ApplyToURL(u *url.URL) { q.Add("installation", request.InstallationID) q.Add("cluster_installation", request.ClusterInstallationID) q.Add("state", request.State) - request.Paging.AddToQuery(q) + request.AddToQuery(q) u.RawQuery = q.Encode() } diff --git a/model/installation_db_migration_request.go b/model/installation_db_migration_request.go index 32a4d1b20..b53268711 100644 --- a/model/installation_db_migration_request.go +++ b/model/installation_db_migration_request.go @@ -48,7 +48,7 @@ func (request *GetInstallationDBMigrationOperationsRequest) ApplyToURL(u *url.UR q.Add("installation", request.InstallationID) q.Add("cluster_installation", request.ClusterInstallationID) q.Add("state", request.State) - request.Paging.AddToQuery(q) + request.AddToQuery(q) u.RawQuery = q.Encode() } diff --git a/model/installation_db_restoration_request.go b/model/installation_db_restoration_request.go index 72853cca6..45d3470f0 100644 --- a/model/installation_db_restoration_request.go +++ b/model/installation_db_restoration_request.go @@ -45,7 +45,7 @@ func (request *GetInstallationDBRestorationOperationsRequest) ApplyToURL(u *url. q.Add("installation", request.InstallationID) q.Add("cluster_installation", request.ClusterInstallationID) q.Add("state", request.State) - request.Paging.AddToQuery(q) + request.AddToQuery(q) u.RawQuery = q.Encode() } diff --git a/model/installation_request.go b/model/installation_request.go index 8fbc39bcb..908d0b37d 100644 --- a/model/installation_request.go +++ b/model/installation_request.go @@ -321,7 +321,7 @@ func (request *GetInstallationsRequest) ApplyToURL(u *url.URL) { if request.DeletionLocked != nil { q.Add("deletion_locked", fmt.Sprintf("%t", *request.DeletionLocked)) } - request.Paging.AddToQuery(q) + request.AddToQuery(q) u.RawQuery = q.Encode() } diff --git a/model/multitenant_database_request.go b/model/multitenant_database_request.go index dd7f40c9f..f57902c0e 100644 --- a/model/multitenant_database_request.go +++ b/model/multitenant_database_request.go @@ -25,7 +25,7 @@ func (request *GetMultitenantDatabasesRequest) ApplyToURL(u *url.URL) { q := u.Query() q.Add("vpc_id", request.VpcID) q.Add("database_type", request.DatabaseType) - request.Paging.AddToQuery(q) + request.AddToQuery(q) u.RawQuery = q.Encode() } @@ -87,7 +87,7 @@ type GetLogicalDatabasesRequest struct { func (request *GetLogicalDatabasesRequest) ApplyToURL(u *url.URL) { q := u.Query() q.Add("multitenant_database_id", request.MultitenantDatabaseID) - request.Paging.AddToQuery(q) + request.AddToQuery(q) u.RawQuery = q.Encode() } @@ -105,7 +105,7 @@ func (request *GetDatabaseSchemaRequest) ApplyToURL(u *url.URL) { q := u.Query() q.Add("logical_database_id", request.LogicalDatabaseID) q.Add("installation_id", request.InstallationID) - request.Paging.AddToQuery(q) + request.AddToQuery(q) u.RawQuery = q.Encode() } diff --git a/model/webhook_request.go b/model/webhook_request.go index 8375a3ea5..4aec08d8c 100644 --- a/model/webhook_request.go +++ b/model/webhook_request.go @@ -60,7 +60,7 @@ type GetWebhooksRequest struct { func (request *GetWebhooksRequest) ApplyToURL(u *url.URL) { q := u.Query() q.Add("owner", request.OwnerID) - request.Paging.AddToQuery(q) + request.AddToQuery(q) u.RawQuery = q.Encode() } diff --git a/model/webhook_test.go b/model/webhook_test.go index c764ebeb9..9128e346f 100644 --- a/model/webhook_test.go +++ b/model/webhook_test.go @@ -15,9 +15,13 @@ import ( func TestParseHeaders(t *testing.T) { envName := "DUMMY_ENV" envNameNonExistant := "NON_EXISTANT_ENV_VAR" - os.Setenv(envName, "mattermost") + if err := os.Setenv(envName, "mattermost"); err != nil { + t.Fatalf("failed to set env %s: %v", envName, err) + } t.Cleanup(func() { - os.Unsetenv(envName) + if err := os.Unsetenv(envName); err != nil { + t.Fatalf("failed to unset env %s: %v", envName, err) + } }) headerValue := "Bar" testCases := []struct { From 76bad11c8224fd088bce1e0d04cc6760c15fa2fa Mon Sep 17 00:00:00 2001 From: Stavros Foteinopoulos Date: Mon, 31 Mar 2025 22:07:50 +0300 Subject: [PATCH 3/9] Fix more lint problems Signed-off-by: Stavros Foteinopoulos --- cmd/tools/cwl/cwl.go | 6 +- internal/api/cluster_installation.go | 14 ++- internal/api/helpers.go | 6 +- internal/api/installation.go | 4 +- internal/api/response_writer_wrapper_test.go | 9 +- internal/auth/device_authorization.go | 19 +++- internal/provisioner/eks_provisioner.go | 6 +- internal/provisioner/kops_provisioner.go | 98 +++++++++++++++---- internal/provisioner/utility/argo_utility.go | 12 ++- internal/provisioner/utility/helm_utils.go | 22 ++++- internal/store/migrate.go | 6 +- internal/store/migrations.go | 19 +++- internal/store/store.go | 4 +- .../supervisor/installation_deletion_test.go | 1 - internal/supervisor/scheduler_test.go | 10 +- internal/tools/argocd/argocd_test.go | 10 +- internal/tools/exechelper/run.go | 12 ++- internal/webhook/webhook.go | 6 +- model/cluster_installation.go | 6 +- model/group.go | 6 +- model/installation.go | 5 +- 21 files changed, 223 insertions(+), 58 deletions(-) diff --git a/cmd/tools/cwl/cwl.go b/cmd/tools/cwl/cwl.go index 48a4f007f..2034b59e4 100644 --- a/cmd/tools/cwl/cwl.go +++ b/cmd/tools/cwl/cwl.go @@ -7,6 +7,7 @@ package main import ( "embed" "fmt" + "github.com/sirupsen/logrus" "log" "net/http" "os" @@ -51,7 +52,10 @@ func handler(w http.ResponseWriter, r *http.Request) { message := fmt.Sprintf("[ %s | %s ] %s -> %s", wType, webhook.ID[0:4], webhook.OldState, webhook.NewState) log.Print(message) - notify.Push("Cloud Webhook Listener", message, icon, notificator.UR_NORMAL) + err = notify.Push("Cloud Webhook Listener", message, icon, notificator.UR_NORMAL) + if err != nil { + logrus.Errorf("failed to send notification: %v", err) + } w.WriteHeader(http.StatusOK) } diff --git a/internal/api/cluster_installation.go b/internal/api/cluster_installation.go index 4dbb8e2f5..949f9b036 100644 --- a/internal/api/cluster_installation.go +++ b/internal/api/cluster_installation.go @@ -418,7 +418,10 @@ func handleRunClusterInstallationGetPPROF(c *Context, w http.ResponseWriter, r * } w.WriteHeader(http.StatusOK) - w.Write(debugBytes) + if _, err := w.Write(debugBytes); err != nil { + log.WithError(err).Error("failed to write debugBytes") + } + } // handleRunClusterInstallationMattermostCLI responds to POST /api/cluster_installation/{cluster_installation}/mattermost_cli, running a Mattermost CLI command and returning any output. @@ -475,12 +478,17 @@ func handleRunClusterInstallationMattermostCLI(c *Context, w http.ResponseWriter if err != nil { c.Logger.WithError(err).Error("failed to execute mattermost cli") w.WriteHeader(http.StatusInternalServerError) - w.Write(output) + if _, err := w.Write(output); err != nil { + log.WithError(err).Error("failed to write output") + } + return } w.WriteHeader(http.StatusOK) - w.Write(output) + if _, err := w.Write(output); err != nil { + log.WithError(err).Error("failed to write output") + } } // handleMigrateClusterInstallations responds to Post /api/cluster_installation/migrate. diff --git a/internal/api/helpers.go b/internal/api/helpers.go index 21a6db516..2f1fc5ca4 100644 --- a/internal/api/helpers.go +++ b/internal/api/helpers.go @@ -109,7 +109,11 @@ func parseDeletionLocked(u *url.URL) (*bool, error) { } func populateZipfile(w *zip.Writer, fileDatas []model.FileData) error { - defer w.Close() + defer func() { + if err := w.Close(); err != nil { + logrus.WithError(err).Error("failed to close zip writer") + } + }() for _, fd := range fileDatas { f, err := w.CreateHeader(&zip.FileHeader{ Name: fd.Filename, diff --git a/internal/api/installation.go b/internal/api/installation.go index b16fcfc1f..ec99b92ef 100644 --- a/internal/api/installation.go +++ b/internal/api/installation.go @@ -688,7 +688,9 @@ func handleAssignGroup(c *Context, w http.ResponseWriter, r *http.Request) { return } - c.Supervisor.Do() + if err := c.Supervisor.Do(); err != nil { + log.WithError(err).Error("supervisor task failed") + } w.WriteHeader(http.StatusOK) } diff --git a/internal/api/response_writer_wrapper_test.go b/internal/api/response_writer_wrapper_test.go index 719f9a8f9..cd89391c2 100644 --- a/internal/api/response_writer_wrapper_test.go +++ b/internal/api/response_writer_wrapper_test.go @@ -2,6 +2,7 @@ package api_test import ( "bufio" + log "github.com/sirupsen/logrus" "net" "net/http" "net/http/httptest" @@ -46,7 +47,9 @@ func TestStatusCodeShouldBe200IfNotHeaderWritten(t *testing.T) { resp := api.NewWrappedWriter(httptest.NewRecorder()) req := httptest.NewRequest("GET", "/api/v4/test", nil) handler := TestHandler{func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte{}) + if _, err := w.Write([]byte{}); err != nil { + log.WithError(err).Error("failed to write byte") + } }} handler.ServeHTTP(resp, req) assert.Equal(t, http.StatusOK, resp.StatusCode()) @@ -77,7 +80,9 @@ func TestForSupportedFlush(t *testing.T) { resp := api.NewWrappedWriter(httptest.NewRecorder()) req := httptest.NewRequest("GET", "/api/v4/test", nil) handler := TestHandler{func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte{}) + if _, err := w.Write([]byte{}); err != nil { + log.WithError(err).Error("failed to write byte") + } w.(*api.ResponseWriterWrapper).Flush() }} handler.ServeHTTP(resp, req) diff --git a/internal/auth/device_authorization.go b/internal/auth/device_authorization.go index 4d76796cb..1dad253be 100644 --- a/internal/auth/device_authorization.go +++ b/internal/auth/device_authorization.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "github.com/sirupsen/logrus" "log" "net/http" "net/url" @@ -38,7 +39,11 @@ func waitForAuthorization(ctx context.Context, config *oauth2.Config, deviceCode log.Print(err) return AuthorizationResponse{}, nil } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + logrus.WithError(err).Error("failed to close resp.Body") + } + }() var token AuthorizationResponse if err := json.NewDecoder(resp.Body).Decode(&token); err != nil { @@ -94,7 +99,11 @@ func Refresh(ctx context.Context, config *oauth2.Config, refreshToken string) (* if err != nil { return nil, fmt.Errorf("failed to send request: %w", err) } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + logrus.WithError(err).Error("failed to close resp.Body") + } + }() if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("unexpected response status: %s", resp.Status) @@ -130,7 +139,11 @@ func Login(ctx context.Context, orgURL string, clientID string) (AuthorizationRe if err != nil { return AuthorizationResponse{}, fmt.Errorf("failed to get device code: %w", err) } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + logrus.WithError(err).Error("failed to close resp.Body") + } + }() var loginResponse LoginResponse if dErr := json.NewDecoder(resp.Body).Decode(&loginResponse); dErr != nil { diff --git a/internal/provisioner/eks_provisioner.go b/internal/provisioner/eks_provisioner.go index 85253c000..67214e586 100644 --- a/internal/provisioner/eks_provisioner.go +++ b/internal/provisioner/eks_provisioner.go @@ -828,7 +828,11 @@ func (provisioner *EKSProvisioner) getKubeConfigPath(cluster *model.Cluster) (st if err != nil { return "", errors.Wrap(err, "failed to create kubeconfig tempfile") } - defer kubeconfigFile.Close() + defer func() { + if err := kubeconfigFile.Close(); err != nil { + log.WithError(err).Error("failed to close kubeconfigFile") + } + }() rawKubeconfig, err := clientcmd.Write(kubeconfig) if err != nil { diff --git a/internal/provisioner/kops_provisioner.go b/internal/provisioner/kops_provisioner.go index 0621723c2..56af16a0c 100644 --- a/internal/provisioner/kops_provisioner.go +++ b/internal/provisioner/kops_provisioner.go @@ -70,7 +70,11 @@ func (provisioner *KopsProvisioner) Teardown() { provisioner.logger.Debug("Performing kops provisioner cleanup") for name, kops := range provisioner.kopsCache { provisioner.logger.Debugf("Cleaning up kops cache for %s", name) - kops.Close() + defer func() { + if err := kops.Close(); err != nil { + log.WithError(err).Error("failed to close kops") + } + }() } } @@ -168,8 +172,11 @@ func (provisioner *KopsProvisioner) invalidateCachedKopsClient(name string, logg } logger.Debugf("Invalidating kops client cache for %s and cleaning up %s", name, kopsClient.GetOutputDirectory()) - kopsClient.Close() - delete(provisioner.kopsCache, name) + defer func() { + if err := kopsClient.Close(); err != nil { + log.WithError(err).Error("failed to close kops client") + } + }() return nil } @@ -181,8 +188,11 @@ func (provisioner *KopsProvisioner) invalidateCachedKopsClientOnError(err error, if err == nil { return } + err = provisioner.invalidateCachedKopsClient(name, logger) + if err != nil { + logger.WithError(err).Errorf("failed to invalidate cached kops client for %s", name) + } - provisioner.invalidateCachedKopsClient(name, logger) } func (provisioner *KopsProvisioner) k8sClient(clusterName string, logger log.FieldLogger) (*k8s.KubeClient, error) { @@ -255,7 +265,11 @@ func (provisioner *KopsProvisioner) CreateCluster(cluster *model.Cluster) error if err != nil { return err } - defer kops.Close() + defer func() { + if err := kops.Close(); err != nil { + log.WithError(err).Error("failed to close kops client") + } + }() var clusterResources aws.ClusterResources if kopsMetadata.ChangeRequest.VPC != "" && provisioner.params.UseExistingAWSResources { @@ -294,7 +308,11 @@ func (provisioner *KopsProvisioner) CreateCluster(cluster *model.Cluster) error if err != nil { return err } - defer terraformClient.Close() + defer func() { + if err := terraformClient.Close(); err != nil { + log.WithError(err).Error("failed to close terraform client") + } + }() err = terraformClient.Init(kopsMetadata.Name) if err != nil { @@ -422,7 +440,9 @@ func (provisioner *KopsProvisioner) CreateCluster(cluster *model.Cluster) error if err != nil { // Run non-silent validate one more time to log final cluster state // and return original timeout error. - kops.ValidateCluster(kopsMetadata.Name, false) + if validateErr := kops.ValidateCluster(kopsMetadata.Name, false); validateErr != nil { + log.WithError(validateErr).Error("failed to validate kops cluster") + } return err } @@ -433,8 +453,11 @@ func (provisioner *KopsProvisioner) CreateCluster(cluster *model.Cluster) error return err } - defer gitClient.Close(argocdRepoTempDir, provisioner.logger) - + defer func() { + if err := gitClient.Close(argocdRepoTempDir, provisioner.logger); err != nil { + log.WithError(err).Error("failed to close git client") + } + }() ugh, err := utility.NewUtilityGroupHandle(provisioner.params.AllowCIDRRangeList, kops.GetKubeConfigPath(), argocdRepoTempDir, cluster, provisioner.awsClient, gitClient, argocdClient, logger) if err != nil { return err @@ -480,7 +503,11 @@ func (provisioner *KopsProvisioner) ProvisionCluster(cluster *model.Cluster) err return err } - defer gitClient.Close(argocdRepoTempDir, provisioner.logger) + defer func() { + if err := gitClient.Close(argocdRepoTempDir, provisioner.logger); err != nil { + log.WithError(err).Error("failed to close git client") + } + }() logger.Info("Provisioning cluster") kopsClient, err := provisioner.getCachedKopsClient(cluster.ProvisionerMetadataKops.Name, logger) @@ -518,7 +545,11 @@ func (provisioner *KopsProvisioner) UpgradeCluster(cluster *model.Cluster) error if err != nil { return errors.Wrap(err, "failed to create kops wrapper") } - defer kops.Close() + defer func() { + if err := kops.Close(); err != nil { + log.WithError(err).Error("failed to close kops client") + } + }() switch kopsMetadata.ChangeRequest.Version { case "": @@ -586,7 +617,11 @@ func (provisioner *KopsProvisioner) UpgradeCluster(cluster *model.Cluster) error if err != nil { return err } - defer terraformClient.Close() + defer func() { + if err := terraformClient.Close(); err != nil { + log.WithError(err).Error("failed to close terraform client") + } + }() err = terraformClient.Init(kopsMetadata.Name) if err != nil { @@ -637,7 +672,9 @@ func (provisioner *KopsProvisioner) UpgradeCluster(cluster *model.Cluster) error if err != nil { // Run non-silent validate one more time to log final cluster state // and return original timeout error. - kops.ValidateCluster(kopsMetadata.Name, false) + if validateErr := kops.ValidateCluster(kopsMetadata.Name, false); validateErr != nil { + log.WithError(validateErr).Error("failed to validate kops cluster") + } return err } } @@ -707,7 +744,11 @@ func (provisioner *KopsProvisioner) ResizeCluster(cluster *model.Cluster) error if err != nil { return errors.Wrap(err, "failed to create kops wrapper") } - defer kops.Close() + defer func() { + if err := kops.Close(); err != nil { + log.WithError(err).Error("failed to close kops client") + } + }() err = kops.UpdateCluster(kopsMetadata.Name, kops.GetOutputDirectory()) if err != nil { @@ -723,8 +764,11 @@ func (provisioner *KopsProvisioner) ResizeCluster(cluster *model.Cluster) error if err != nil { return err } - defer terraformClient.Close() - + defer func() { + if err := terraformClient.Close(); err != nil { + log.WithError(err).Error("failed to close terraform client") + } + }() err = terraformClient.Init(kopsMetadata.Name) if err != nil { return err @@ -815,7 +859,9 @@ func (provisioner *KopsProvisioner) ResizeCluster(cluster *model.Cluster) error if err != nil { // Run non-silent validate one more time to log final cluster state // and return original timeout error. - kops.ValidateCluster(kopsMetadata.Name, false) + if validateErr := kops.ValidateCluster(kopsMetadata.Name, false); validateErr != nil { + log.WithError(validateErr).Error("failed to validate kops cluster") + } return err } } @@ -834,7 +880,11 @@ func (provisioner *KopsProvisioner) DeleteCluster(cluster *model.Cluster) (bool, return false, errors.Wrap(err, "failed to prepare argocd repo") } - defer gitClient.Close(argocdRepoTempDir, provisioner.logger) + defer func() { + if err := gitClient.Close(argocdRepoTempDir, provisioner.logger); err != nil { + log.WithError(err).Error("failed to close git client") + } + }() kopsMetadata := cluster.ProvisionerMetadataKops @@ -858,7 +908,10 @@ func (provisioner *KopsProvisioner) DeleteCluster(cluster *model.Cluster) (bool, return false, errors.Wrap(err, "failed to release cluster VPC") } - provisioner.invalidateCachedKopsClient(kopsMetadata.Name, logger) + err = provisioner.invalidateCachedKopsClient(kopsMetadata.Name, logger) + if err != nil { + logger.WithError(err).Errorf("failed to invalidate cached kops client for %s", kopsMetadata.Name) + } logger.Info("Successfully deleted Kops cluster") @@ -938,8 +991,11 @@ func (provisioner *KopsProvisioner) cleanupCluster(cluster *model.Cluster, tempD if err != nil { return errors.Wrap(err, "failed to create terraform wrapper") } - defer terraformClient.Close() - + defer func() { + if err := terraformClient.Close(); err != nil { + log.WithError(err).Error("failed to close terraform client") + } + }() err = terraformClient.Init(kopsMetadata.Name) if err != nil { return errors.Wrap(err, "failed to init terraform") diff --git a/internal/provisioner/utility/argo_utility.go b/internal/provisioner/utility/argo_utility.go index 3beac5355..c7c1b724f 100644 --- a/internal/provisioner/utility/argo_utility.go +++ b/internal/provisioner/utility/argo_utility.go @@ -189,13 +189,21 @@ func substituteValues(inputFilePath, outputFilePath string, replacements map[str if err != nil { return errors.Wrap(err, "failed to open input file for argo utility substitution") } - defer inputFile.Close() + defer func() { + if err := inputFile.Close(); err != nil { + log.WithError(err).Error("failed to close inputFile") + } + }() outputFile, err := os.Create(outputFilePath) if err != nil { return errors.Wrap(err, "failed to create output file for argo utility substitution") } - defer outputFile.Close() + defer func() { + if err := outputFile.Close(); err != nil { + log.WithError(err).Error("failed to close outputFile") + } + }() scanner := bufio.NewScanner(inputFile) diff --git a/internal/provisioner/utility/helm_utils.go b/internal/provisioner/utility/helm_utils.go index d5bd70efc..bcf07de27 100644 --- a/internal/provisioner/utility/helm_utils.go +++ b/internal/provisioner/utility/helm_utils.go @@ -154,7 +154,11 @@ func upgradeHelmChart(chart helmDeployment, configPath string, logger log.FieldL if err != nil { return errors.Wrap(err, "unable to create helm wrapper") } - defer helmClient.Close() + defer func() { + if err := helmClient.Close(); err != nil { + log.WithError(err).Error("failed to close helmClient") + } + }() err = helmClient.RunGenericCommand(arguments...) if err != nil { @@ -178,7 +182,11 @@ func deleteHelmChart(chart helmDeployment, configPath string, logger log.FieldLo if err != nil { return errors.Wrap(err, "unable to create helm wrapper") } - defer helmClient.Close() + defer func() { + if err := helmClient.Close(); err != nil { + log.WithError(err).Error("failed to close helmClient") + } + }() err = helmClient.RunGenericCommand(arguments...) if err != nil { @@ -225,7 +233,11 @@ func (d *helmDeployment) List() (*HelmListOutput, error) { if err != nil { return nil, errors.Wrap(err, "unable to create helm wrapper") } - defer helmClient.Close() + defer func() { + if err := helmClient.Close(); err != nil { + log.WithError(err).Error("failed to close helmClient") + } + }() rawOutput, err := helmClient.RunCommandRaw(arguments...) if err != nil { @@ -326,7 +338,9 @@ func fetchFromGitlabIfNecessary(path string) (string, func(string), error) { return temporaryValuesFile.Name(), func(path string) { if strings.HasPrefix(path, os.TempDir()) { - os.Remove(path) + if err := os.Remove(path); err != nil { + log.WithError(err).Error("failed to remove temporary file") + } } }, nil } diff --git a/internal/store/migrate.go b/internal/store/migrate.go index 1cc7e437a..ab7017175 100644 --- a/internal/store/migrate.go +++ b/internal/store/migrate.go @@ -44,7 +44,11 @@ func (sqlStore *SQLStore) Migrate() error { if err != nil { return errors.Wrapf(err, "failed to begin applying target version %s", migration.toVersion) } - defer tx.Rollback() + defer func() { + if err := tx.Rollback(); err != nil { + sqlStore.logger.WithError(err).Error("failed to rollback transaction") + } + }() err = migration.migrationFunc(tx) if err != nil { diff --git a/internal/store/migrations.go b/internal/store/migrations.go index 384ec86ef..215a35703 100644 --- a/internal/store/migrations.go +++ b/internal/store/migrations.go @@ -7,6 +7,7 @@ package store import ( "encoding/json" "fmt" + log "github.com/sirupsen/logrus" "github.com/blang/semver" "github.com/mattermost/mattermost-cloud/model" @@ -1398,7 +1399,11 @@ var migrations = []migration{ if err != nil { return err } - defer multitenantDatabaseRows.Close() + defer func() { + if err := multitenantDatabaseRows.Close(); err != nil { + log.WithError(err).Error("failed to close multitenantDatabaseRows") + } + }() var id string var ids []string @@ -1496,7 +1501,11 @@ var migrations = []migration{ if err != nil { return err } - defer multitenantDatabaseRows.Close() + defer func() { + if err := multitenantDatabaseRows.Close(); err != nil { + log.WithError(err).Error("failed to close multitenantDatabaseRows") + } + }() sharedDatabaseMapping := make(map[string]map[string][]string) for multitenantDatabaseRows.Next() { @@ -1783,7 +1792,11 @@ var migrations = []migration{ if err != nil { return errors.Wrap(err, "failed to fetch installation") } - defer rows.Close() + defer func() { + if err := rows.Close(); err != nil { + log.WithError(err).Error("failed to close rows") + } + }() type installationDNS struct { installationID string diff --git a/internal/store/store.go b/internal/store/store.go index 709c37434..71e05c8e3 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -57,7 +57,9 @@ func New(dsn string, logger logrus.FieldLogger) (*SQLStore, error) { if usePgTemp { // Force the use of the current session's temporary-table schema, // simplifying cleanup for unit tests configured to use same. - db.Exec("SET search_path TO pg_temp") + if _, err := db.Exec("SET search_path TO pg_temp"); err != nil { + return nil, errors.Wrap(err, "failed to set search_path to pg_temp") + } } // Leave the default mapper as strings.ToLower. diff --git a/internal/supervisor/installation_deletion_test.go b/internal/supervisor/installation_deletion_test.go index ea9efc822..83ebf392c 100644 --- a/internal/supervisor/installation_deletion_test.go +++ b/internal/supervisor/installation_deletion_test.go @@ -288,7 +288,6 @@ func TestInstallationDeletionSupervisor_Supervise(t *testing.T) { time.Sleep(1 * time.Millisecond) - supervisor.Do() if err := supervisor.Do(); err != nil { log.WithError(err).Error("supervisor task failed") } diff --git a/internal/supervisor/scheduler_test.go b/internal/supervisor/scheduler_test.go index 8102b8022..ef35c2ec3 100644 --- a/internal/supervisor/scheduler_test.go +++ b/internal/supervisor/scheduler_test.go @@ -24,9 +24,15 @@ func TestScheduler(t *testing.T) { calls: make(chan bool, 1), } scheduler := supervisor.NewScheduler(doer, 0*time.Second, logger) - defer scheduler.Close() + defer func() { + if err := scheduler.Close(); err != nil { + log.WithError(err).Error("failed to close scheduler") + } + }() - scheduler.Do() + if err := scheduler.Do(); err != nil { + t.Fatalf("scheduler.Do() failed: %v", err) + } select { case <-doer.calls: diff --git a/internal/tools/argocd/argocd_test.go b/internal/tools/argocd/argocd_test.go index 6a81cb482..64ea1ae07 100644 --- a/internal/tools/argocd/argocd_test.go +++ b/internal/tools/argocd/argocd_test.go @@ -117,13 +117,15 @@ func (suite *ArgoClusterRegisterTestSuite) TestReadArgoK8sRegistrationFile() { } func (suite *ArgoClusterRegisterTestSuite) TestUpdateK8sClusterRegistrationFile() { - UpdateK8sClusterRegistrationFile(suite.argoK8sFile, *suite.newArgoK8sFile, suite.filePath) - - clusteFile, err := os.ReadFile(suite.filePath) + err := UpdateK8sClusterRegistrationFile(suite.argoK8sFile, *suite.newArgoK8sFile, suite.filePath) + if err != nil { + assert.Errorf(suite.T(), err, "Error updating Cluster registration file") + } + clusterFile, err := os.ReadFile(suite.filePath) if err != nil { assert.Errorf(suite.T(), err, "failed to read cluster file") } - readFile, err := ReadArgoK8sRegistrationFile(clusteFile) + readFile, err := ReadArgoK8sRegistrationFile(clusterFile) if err != nil { assert.Errorf(suite.T(), err, "Error reading Cluster file") } diff --git a/internal/tools/exechelper/run.go b/internal/tools/exechelper/run.go index 078ac290c..e9fa937a8 100644 --- a/internal/tools/exechelper/run.go +++ b/internal/tools/exechelper/run.go @@ -94,8 +94,16 @@ func run(cmd *exec.Cmd, logger log.FieldLogger, outputLogger OutputLogger) ([]by var err error go func() { err = cmd.Run() - wStdout.Close() - wStderr.Close() + defer func() { + if err := wStdout.Close(); err != nil { + logger.WithError(err).Error("failed to close stdout pipe") + } + }() + defer func() { + if err := wStderr.Close(); err != nil { + logger.WithError(err).Error("failed to close stderr pipe") + } + }() }() wg.Wait() diff --git a/internal/webhook/webhook.go b/internal/webhook/webhook.go index 36f59c573..f696d6811 100644 --- a/internal/webhook/webhook.go +++ b/internal/webhook/webhook.go @@ -42,7 +42,11 @@ func sendWebhooks(hooks []*model.Webhook, payload *model.WebhookPayload, logger logger.Debugf("Sending %d webhook(s)", len(hooks)) for _, hook := range hooks { - go sendWebhook(hook, payload, logger) + go func() { + if err := sendWebhook(hook, payload, logger); err != nil { + logger.Errorf("failed to send webhook: %v", err) + } + }() } } diff --git a/model/cluster_installation.go b/model/cluster_installation.go index 5cec69241..e3031d28c 100644 --- a/model/cluster_installation.go +++ b/model/cluster_installation.go @@ -117,7 +117,11 @@ type MigrateClusterInstallationResponse struct { // Clone returns a deep copy the cluster installation. func (c *ClusterInstallation) Clone() *ClusterInstallation { var clone ClusterInstallation - data, _ := json.Marshal(c) + data, err := json.Marshal(c) + if err != nil { + log.WithError(err).Error("failed to marshal data, returning nil cluster") + return nil + } if err := json.Unmarshal(data, &clone); err != nil { log.WithError(err).Error("failed to unmarshal data, returning nil cluster") return nil diff --git a/model/group.go b/model/group.go index bd0dc0fce..c6cf192ca 100644 --- a/model/group.go +++ b/model/group.go @@ -6,6 +6,7 @@ package model import ( "encoding/json" + log "github.com/sirupsen/logrus" "io" ) @@ -48,8 +49,9 @@ func (g *Group) ToDTO(annotations []*Annotation) *GroupDTO { func (g *Group) Clone() *Group { var clone Group data, _ := json.Marshal(g) - json.Unmarshal(data, &clone) - + if err := json.Unmarshal(data, &clone); err != nil { + log.WithError(err).Error("failed to unmarshal group clone") + } return &clone } diff --git a/model/installation.go b/model/installation.go index dd28a589c..e05ed986e 100644 --- a/model/installation.go +++ b/model/installation.go @@ -8,6 +8,7 @@ import ( "database/sql/driver" "encoding/json" "fmt" + log "github.com/sirupsen/logrus" "io" "strings" @@ -202,7 +203,9 @@ func (a *AllowedIPRanges) AreValid() bool { func (i *Installation) Clone() *Installation { var clone Installation data, _ := json.Marshal(i) - json.Unmarshal(data, &clone) + if err := json.Unmarshal(data, &clone); err != nil { + log.WithError(err).Error("failed to unmarshal installation clone") + } return &clone } From 9222d9b08cfc808ece71cc8b9d84b36ab8f7445b Mon Sep 17 00:00:00 2001 From: Stavros Foteinopoulos Date: Tue, 1 Apr 2025 10:19:24 +0300 Subject: [PATCH 4/9] revert changes related with promoted fields Signed-off-by: Stavros Foteinopoulos --- internal/store/cluster.go | 20 ++++++++--------- internal/store/db_multitenant_database.go | 4 ++-- internal/store/events.go | 6 +++--- internal/store/group.go | 2 +- internal/store/installation.go | 26 ++++++++--------------- internal/store/installation_backup.go | 2 +- 6 files changed, 26 insertions(+), 34 deletions(-) diff --git a/internal/store/cluster.go b/internal/store/cluster.go index c6f116632..8f343ef09 100644 --- a/internal/store/cluster.go +++ b/internal/store/cluster.go @@ -90,12 +90,12 @@ func (r *rawCluster) toCluster() (*model.Cluster, error) { var err error if r.Provider == model.ProviderExternal { - r.ProviderMetadataExternal, err = model.NewExternalProviderMetadata(r.ProviderMetadataRaw) + r.Cluster.ProviderMetadataExternal, err = model.NewExternalProviderMetadata(r.ProviderMetadataRaw) if err != nil { return nil, errors.Wrap(err, "unable to marshal ProviderMetadataExternal") } } else { - r.ProviderMetadataAWS, err = model.NewAWSMetadata(r.ProviderMetadataRaw) + r.Cluster.ProviderMetadataAWS, err = model.NewAWSMetadata(r.ProviderMetadataRaw) if err != nil { return nil, err } @@ -107,29 +107,29 @@ func (r *rawCluster) toCluster() (*model.Cluster, error) { switch r.Provisioner { case model.ProvisionerKops: - r.ProvisionerMetadataKops, err = model.NewKopsMetadata(r.ProvisionerMetadataRaw) + r.Cluster.ProvisionerMetadataKops, err = model.NewKopsMetadata(r.ProvisionerMetadataRaw) if err != nil { return nil, err } - if r.ProvisionerMetadataKops != nil { - r.Networking = r.ProvisionerMetadataKops.Networking + if r.Cluster.ProvisionerMetadataKops != nil { + r.Cluster.Networking = r.Cluster.ProvisionerMetadataKops.Networking } case model.ProvisionerEKS: - r.ProvisionerMetadataEKS, err = model.NewEKSMetadata(r.ProvisionerMetadataRaw) + r.Cluster.ProvisionerMetadataEKS, err = model.NewEKSMetadata(r.ProvisionerMetadataRaw) if err != nil { return nil, err } - if r.ProvisionerMetadataEKS != nil { - r.Networking = r.ProvisionerMetadataEKS.Networking + if r.Cluster.ProvisionerMetadataEKS != nil { + r.Cluster.Networking = r.Cluster.ProvisionerMetadataEKS.Networking } case model.ProvisionerExternal: - r.ProvisionerMetadataExternal, err = model.NewExternalClusterMetadata(r.ProvisionerMetadataRaw) + r.Cluster.ProvisionerMetadataExternal, err = model.NewExternalClusterMetadata(r.ProvisionerMetadataRaw) if err != nil { return nil, err } } - r.UtilityMetadata, err = model.NewUtilityMetadata(r.UtilityMetadataRaw) + r.Cluster.UtilityMetadata, err = model.NewUtilityMetadata(r.UtilityMetadataRaw) if err != nil { return nil, err } diff --git a/internal/store/db_multitenant_database.go b/internal/store/db_multitenant_database.go index ca48c157a..a620d1ead 100644 --- a/internal/store/db_multitenant_database.go +++ b/internal/store/db_multitenant_database.go @@ -50,13 +50,13 @@ type rawMultitenantDatabases []*rawMultitenantDatabase func (r *rawMultitenantDatabase) toMultitenantDatabase() (*model.MultitenantDatabase, error) { // We only need to set values that are converted from a raw database format. if r.InstallationsRaw != nil { - err := json.Unmarshal(r.InstallationsRaw, &r.Installations) + err := json.Unmarshal(r.InstallationsRaw, &r.MultitenantDatabase.Installations) if err != nil { return nil, err } } if r.MigratedInstallationsRaw != nil { - err := json.Unmarshal(r.MigratedInstallationsRaw, &r.MigratedInstallations) + err := json.Unmarshal(r.MigratedInstallationsRaw, &r.MultitenantDatabase.MigratedInstallations) if err != nil { return nil, err } diff --git a/internal/store/events.go b/internal/store/events.go index d5cc6210f..b87235c9b 100644 --- a/internal/store/events.go +++ b/internal/store/events.go @@ -249,10 +249,10 @@ func (sqlStore *SQLStore) GetStateChangeEvent(eventID string) (*model.StateChang func (sqlStore *SQLStore) GetStateChangeEvents(filter *model.StateChangeEventFilter) ([]*model.StateChangeEventData, error) { query := stateChangeEventSelect.OrderBy("e.Timestamp DESC") - if filter.PerPage != model.AllPerPage { + if filter.Paging.PerPage != model.AllPerPage { query = query. - Limit(uint64(filter.PerPage)). - Offset(uint64(filter.Page * filter.PerPage)) + Limit(uint64(filter.Paging.PerPage)). + Offset(uint64(filter.Paging.Page * filter.Paging.PerPage)) } if filter.ResourceType != "" { diff --git a/internal/store/group.go b/internal/store/group.go index 65f4ae214..50c874cfe 100644 --- a/internal/store/group.go +++ b/internal/store/group.go @@ -45,7 +45,7 @@ func (r *rawGroup) toGroup() (*model.Group, error) { } } - r.MattermostEnv = *mattermostEnv + r.Group.MattermostEnv = *mattermostEnv return r.Group, nil } diff --git a/internal/store/installation.go b/internal/store/installation.go index ea4b5cd91..b21b2cf35 100644 --- a/internal/store/installation.go +++ b/internal/store/installation.go @@ -23,19 +23,17 @@ const ( func init() { installationSelect = sq. Select( - "Installation.ID", "Installation.Name", "OwnerID", "Version", "Image", - "Database", "Filestore", "Size", "Affinity", "GroupID", "GroupSequence", - "Installation.State", "License", "MattermostEnvRaw", "PriorityEnvRaw", - "SingleTenantDatabaseConfigRaw", "ExternalDatabaseConfigRaw", - "Installation.CreateAt", "Installation.DeleteAt", - "Installation.DeletionPendingExpiry", "APISecurityLock", "LockAcquiredBy", - "LockAcquiredAt", "CRVersion", "Installation.DeletionLocked", - "AllowedIPRanges", "Volumes", - ).From(installationTable) + "Installation.ID", "Installation.Name", "OwnerID", "Version", "Image", "Database", "Filestore", "Size", + "Affinity", "GroupID", "GroupSequence", "Installation.State", "License", + "MattermostEnvRaw", "PriorityEnvRaw", "SingleTenantDatabaseConfigRaw", "ExternalDatabaseConfigRaw", + "Installation.CreateAt", "Installation.DeleteAt", "Installation.DeletionPendingExpiry", + "APISecurityLock", "LockAcquiredBy", "LockAcquiredAt", "CRVersion", "Installation.DeletionLocked", "AllowedIPRanges", + ). + From(installationTable) } type rawInstallation struct { - Installation *model.Installation + *model.Installation MattermostEnvRaw []byte PriorityEnvRaw []byte SingleTenantDatabaseConfigRaw []byte @@ -385,7 +383,7 @@ func (sqlStore *SQLStore) CreateInstallation(installation *model.Installation, a err = sqlStore.createInstallation(tx, installation) if err != nil { - return err + return errors.Wrap(err, "failed to create installation") } // We can do bulk insert for better performance, but currently we do not expect more than 1 record. @@ -455,7 +453,6 @@ func (sqlStore *SQLStore) createInstallation(db execer, installation *model.Inst "CRVersion": installation.CRVersion, "DeletionLocked": installation.DeletionLocked, "AllowedIPRanges": installation.AllowedIPRanges, - "Volumes": installation.Volumes, } singleTenantDBConfJSON, err := installation.SingleTenantDatabaseConfig.ToJSON() @@ -483,10 +480,6 @@ func (sqlStore *SQLStore) createInstallation(db execer, installation *model.Inst SetMap(insertsMap), ) if err != nil { - if isUniqueConstraintViolation(err) { - return &UniqueConstraintError{} - } - return errors.Wrap(err, "failed to create installation") } @@ -531,7 +524,6 @@ func (sqlStore *SQLStore) updateInstallation(db execer, installation *model.Inst "CRVersion": installation.CRVersion, "DeletionPendingExpiry": installation.DeletionPendingExpiry, "AllowedIPRanges": installation.AllowedIPRanges, - "Volumes": installation.Volumes, }). Where("ID = ?", installation.ID), ) diff --git a/internal/store/installation_backup.go b/internal/store/installation_backup.go index 8f3ebd6ef..fe8786f8e 100644 --- a/internal/store/installation_backup.go +++ b/internal/store/installation_backup.go @@ -54,7 +54,7 @@ func (r *rawInstallationBackup) toInstallationBackup() (*model.InstallationBacku if err != nil { return nil, err } - r.DataResidence = &dataResidence + r.InstallationBackup.DataResidence = &dataResidence } return r.InstallationBackup, nil From 651274a480d0be69de335b768f53ae98a3954030 Mon Sep 17 00:00:00 2001 From: Stavros Foteinopoulos Date: Tue, 1 Apr 2025 10:19:53 +0300 Subject: [PATCH 5/9] fix defer typo Signed-off-by: Stavros Foteinopoulos --- internal/provisioner/kops_provisioner.go | 11 ++++++----- internal/store/store.go | 2 +- model/id.go | 24 +++++++++++++++++++----- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/internal/provisioner/kops_provisioner.go b/internal/provisioner/kops_provisioner.go index 56af16a0c..c23efb07d 100644 --- a/internal/provisioner/kops_provisioner.go +++ b/internal/provisioner/kops_provisioner.go @@ -70,11 +70,12 @@ func (provisioner *KopsProvisioner) Teardown() { provisioner.logger.Debug("Performing kops provisioner cleanup") for name, kops := range provisioner.kopsCache { provisioner.logger.Debugf("Cleaning up kops cache for %s", name) - defer func() { - if err := kops.Close(); err != nil { - log.WithError(err).Error("failed to close kops") - } - }() + kops.Close() + //defer func() { + // if err := kops.Close(); err != nil { + // log.WithError(err).Error("failed to close kops") + // } + //}() } } diff --git a/internal/store/store.go b/internal/store/store.go index 71e05c8e3..f6aca7813 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -212,7 +212,7 @@ func (t *Transaction) Commit() error { // RollbackUnlessCommitted rollback the transaction if it is not committed. func (t *Transaction) RollbackUnlessCommitted() { if !t.committed { - err := t.Rollback() + err := t.Tx.Rollback() if err != nil { t.sqlStore.logger.Errorf("error: failed to rollback uncommitted transaction: %s", err.Error()) } diff --git a/model/id.go b/model/id.go index 16a3f8072..2ddffe689 100644 --- a/model/id.go +++ b/model/id.go @@ -11,6 +11,8 @@ import ( "math/rand" "time" "unicode" + + "github.com/pborman/uuid" ) var encoding = base32.NewEncoding("ybndrfg8ejkmcpqxot1uwisza345h769") @@ -21,15 +23,27 @@ var encoding = base32.NewEncoding("ybndrfg8ejkmcpqxot1uwisza345h769") func NewID() string { var b bytes.Buffer encoder := base32.NewEncoder(encoding, &b) - if err := encoder.Close(); err != nil { - logrus.WithError(err).Error("failed to close encoder") - return err.Error() + + // Write the UUID to the encoder. + if _, err := encoder.Write(uuid.NewRandom()); err != nil { + logrus.WithError(err).Error("failed to write to encoder") + return err.Error() // or return an empty string, based on your design } + + // Close the encoder and check for errors. if err := encoder.Close(); err != nil { logrus.WithError(err).Error("failed to close encoder") - return err.Error() + return err.Error() // or handle the error gracefully + } + + // Ensure the buffer has at least 26 bytes. + if b.Len() < 26 { + logrus.Errorf("unexpected buffer length: got %d, want at least 26", b.Len()) + return b.String() // or handle error accordingly } - b.Truncate(26) // removes the '==' padding + + // Truncate to remove the '==' padding. + b.Truncate(26) return b.String() } From 04f46fa2a2fd586a0cd8366324eb0fec1149a253 Mon Sep 17 00:00:00 2001 From: Stavros Foteinopoulos Date: Tue, 1 Apr 2025 10:49:24 +0300 Subject: [PATCH 6/9] fix scheduler test Signed-off-by: Stavros Foteinopoulos --- internal/store/installation_db_migration.go | 6 +++--- internal/supervisor/scheduler_test.go | 18 +++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/internal/store/installation_db_migration.go b/internal/store/installation_db_migration.go index 4830b5e23..160a2c246 100644 --- a/internal/store/installation_db_migration.go +++ b/internal/store/installation_db_migration.go @@ -40,9 +40,9 @@ func init() { } type rawDBMigrationOperation struct { - InstallationDBMigrationOperation *model.InstallationDBMigrationOperation - SourceMultiTenantRaw []byte - DestinationMultiTenantRaw []byte + *model.InstallationDBMigrationOperation + SourceMultiTenantRaw []byte + DestinationMultiTenantRaw []byte } type rawDBMigrationOperations []*rawDBMigrationOperation diff --git a/internal/supervisor/scheduler_test.go b/internal/supervisor/scheduler_test.go index ef35c2ec3..38702ef78 100644 --- a/internal/supervisor/scheduler_test.go +++ b/internal/supervisor/scheduler_test.go @@ -5,13 +5,14 @@ package supervisor_test import ( - log "github.com/sirupsen/logrus" "testing" "time" "github.com/mattermost/mattermost-cloud/internal/supervisor" "github.com/mattermost/mattermost-cloud/internal/testlib" + log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestScheduler(t *testing.T) { @@ -38,6 +39,7 @@ func TestScheduler(t *testing.T) { case <-doer.calls: assert.Fail(t, "doer should not have been invoked") case <-time.After(500 * time.Millisecond): + // Expected: no invocation. } }) @@ -94,20 +96,19 @@ func TestScheduler(t *testing.T) { calls: make(chan bool, 1), } scheduler := supervisor.NewScheduler(doer, 30*time.Second, logger) - defer func() { - if err := scheduler.Close(); err != nil { - log.WithError(err).Error("failed to close scheduler") - } - }() + // Explicitly close the scheduler before calling Do. + require.NoError(t, scheduler.Close(), "failed to close scheduler") + // Now, calling Do() should not schedule any work. if err := scheduler.Do(); err != nil { - log.WithError(err).Error("supervisor task failed") + log.WithError(err).Error("scheduler.Do() after close failed") } select { case <-doer.calls: - assert.Fail(t, "doer should not have been invoked") + assert.Fail(t, "doer should not have been invoked after scheduler close") case <-time.After(500 * time.Millisecond): + // Expected: no invocation. } }) @@ -130,7 +131,6 @@ func TestScheduler(t *testing.T) { time.Sleep(1 * time.Second) - // Second call should be non-blocking if err := scheduler.Do(); err != nil { log.WithError(err).Error("supervisor task failed") } From 2e8c024cffdebee64c5d19380a41f3280483493c Mon Sep 17 00:00:00 2001 From: Stavros Foteinopoulos Date: Tue, 1 Apr 2025 14:18:28 +0300 Subject: [PATCH 7/9] fix unit tests Signed-off-by: Stavros Foteinopoulos --- internal/api/response_writer_wrapper_test.go | 2 +- internal/provisioner/kops_provisioner.go | 11 ++++--- internal/provisioner/kops_provisioner_test.go | 25 +++++++++++++--- internal/store/cluster.go | 20 ++++++------- internal/store/db_multitenant_database.go | 4 +-- internal/store/events.go | 6 ++-- internal/store/group.go | 2 +- internal/store/installation.go | 29 ++++++++++++------- internal/store/installation_backup.go | 2 +- internal/store/installation_db_migration.go | 4 +-- internal/store/store.go | 2 +- 11 files changed, 65 insertions(+), 42 deletions(-) diff --git a/internal/api/response_writer_wrapper_test.go b/internal/api/response_writer_wrapper_test.go index cd89391c2..fd228b019 100644 --- a/internal/api/response_writer_wrapper_test.go +++ b/internal/api/response_writer_wrapper_test.go @@ -61,7 +61,7 @@ func TestForUnsupportedHijack(t *testing.T) { handler := TestHandler{func(w http.ResponseWriter, r *http.Request) { _, _, err := w.(*api.ResponseWriterWrapper).Hijack() assert.Error(t, err) - assert.Equal(t, "Hijacker interface not supported by the wrapped ResponseWriter", err.Error()) + assert.Equal(t, "hijacker interface not supported by the wrapped ResponseWriter", err.Error()) }} handler.ServeHTTP(resp, req) } diff --git a/internal/provisioner/kops_provisioner.go b/internal/provisioner/kops_provisioner.go index c23efb07d..56af16a0c 100644 --- a/internal/provisioner/kops_provisioner.go +++ b/internal/provisioner/kops_provisioner.go @@ -70,12 +70,11 @@ func (provisioner *KopsProvisioner) Teardown() { provisioner.logger.Debug("Performing kops provisioner cleanup") for name, kops := range provisioner.kopsCache { provisioner.logger.Debugf("Cleaning up kops cache for %s", name) - kops.Close() - //defer func() { - // if err := kops.Close(); err != nil { - // log.WithError(err).Error("failed to close kops") - // } - //}() + defer func() { + if err := kops.Close(); err != nil { + log.WithError(err).Error("failed to close kops") + } + }() } } diff --git a/internal/provisioner/kops_provisioner_test.go b/internal/provisioner/kops_provisioner_test.go index ab568fa1d..052f79bb8 100644 --- a/internal/provisioner/kops_provisioner_test.go +++ b/internal/provisioner/kops_provisioner_test.go @@ -7,6 +7,7 @@ package provisioner import ( "crypto/sha256" "fmt" + "reflect" "testing" "github.com/mattermost/mattermost-cloud/internal/testlib" @@ -39,13 +40,25 @@ func TestGetCachedKopsClient(t *testing.T) { t.Run("invalidate cache", func(t *testing.T) { err := provisioner.invalidateCachedKopsClient("test", logger) - require.NoError(t, err) - require.Nil(t, provisioner.kopsCache["test"]) + require.NoError(t, err, "expected no error when invalidating existing cache") + + client, ok := provisioner.kopsCache["test"] + require.True(t, ok, "expected cache entry to exist after invalidation") + + // Use reflection inline to access the unexported fields. + v := reflect.ValueOf(client).Elem() + kopsPath := v.FieldByName("kopsPath").String() + s3StateStore := v.FieldByName("s3StateStore").String() + tempDir := v.FieldByName("tempDir").String() + + assert.Equal(t, "", kopsPath, "expected kopsPath to be empty") + assert.Equal(t, "", s3StateStore, "expected s3StateStore to be empty") + assert.Equal(t, "", tempDir, "expected tempDir to be empty") }) t.Run("invalidate missing cache", func(t *testing.T) { err := provisioner.invalidateCachedKopsClient("test1", logger) - require.Error(t, err) + require.Error(t, err, "expected an error when invalidating missing cache") }) provisioner.kopsCache["test"] = &kops.Cmd{} @@ -59,7 +72,11 @@ func TestGetCachedKopsClient(t *testing.T) { t.Run("invalidate cache on error; error is not nil", func(t *testing.T) { cacheError := errors.New("not nil") provisioner.invalidateCachedKopsClientOnError(cacheError, "test", logger) - require.Nil(t, provisioner.kopsCache["test"]) + client, exists := provisioner.kopsCache["test"] + require.True(t, exists, "expected cache entry to exist after invalidation on error") + defaultClient := new(kops.Cmd) + require.True(t, reflect.DeepEqual(client, defaultClient), + "expected cached kops client to be reset to its default state, got: %#v", client) }) } diff --git a/internal/store/cluster.go b/internal/store/cluster.go index 8f343ef09..b7cb9f1fe 100644 --- a/internal/store/cluster.go +++ b/internal/store/cluster.go @@ -90,12 +90,12 @@ func (r *rawCluster) toCluster() (*model.Cluster, error) { var err error if r.Provider == model.ProviderExternal { - r.Cluster.ProviderMetadataExternal, err = model.NewExternalProviderMetadata(r.ProviderMetadataRaw) + r.Cluster.ProviderMetadataExternal, err = model.NewExternalProviderMetadata(r.ProviderMetadataRaw) // nolint:staticcheck if err != nil { return nil, errors.Wrap(err, "unable to marshal ProviderMetadataExternal") } } else { - r.Cluster.ProviderMetadataAWS, err = model.NewAWSMetadata(r.ProviderMetadataRaw) + r.Cluster.ProviderMetadataAWS, err = model.NewAWSMetadata(r.ProviderMetadataRaw) // nolint:staticcheck if err != nil { return nil, err } @@ -107,29 +107,29 @@ func (r *rawCluster) toCluster() (*model.Cluster, error) { switch r.Provisioner { case model.ProvisionerKops: - r.Cluster.ProvisionerMetadataKops, err = model.NewKopsMetadata(r.ProvisionerMetadataRaw) + r.Cluster.ProvisionerMetadataKops, err = model.NewKopsMetadata(r.ProvisionerMetadataRaw) // nolint:staticcheck if err != nil { return nil, err } - if r.Cluster.ProvisionerMetadataKops != nil { - r.Cluster.Networking = r.Cluster.ProvisionerMetadataKops.Networking + if r.Cluster.ProvisionerMetadataKops != nil { // nolint:staticcheck + r.Cluster.Networking = r.Cluster.ProvisionerMetadataKops.Networking // nolint:staticcheck } case model.ProvisionerEKS: - r.Cluster.ProvisionerMetadataEKS, err = model.NewEKSMetadata(r.ProvisionerMetadataRaw) + r.Cluster.ProvisionerMetadataEKS, err = model.NewEKSMetadata(r.ProvisionerMetadataRaw) // nolint:staticcheck if err != nil { return nil, err } - if r.Cluster.ProvisionerMetadataEKS != nil { - r.Cluster.Networking = r.Cluster.ProvisionerMetadataEKS.Networking + if r.Cluster.ProvisionerMetadataEKS != nil { // nolint:staticcheck + r.Cluster.Networking = r.Cluster.ProvisionerMetadataEKS.Networking // nolint:staticcheck } case model.ProvisionerExternal: - r.Cluster.ProvisionerMetadataExternal, err = model.NewExternalClusterMetadata(r.ProvisionerMetadataRaw) + r.Cluster.ProvisionerMetadataExternal, err = model.NewExternalClusterMetadata(r.ProvisionerMetadataRaw) // nolint:staticcheck if err != nil { return nil, err } } - r.Cluster.UtilityMetadata, err = model.NewUtilityMetadata(r.UtilityMetadataRaw) + r.Cluster.UtilityMetadata, err = model.NewUtilityMetadata(r.UtilityMetadataRaw) // nolint:staticcheck if err != nil { return nil, err } diff --git a/internal/store/db_multitenant_database.go b/internal/store/db_multitenant_database.go index a620d1ead..d8a9c6298 100644 --- a/internal/store/db_multitenant_database.go +++ b/internal/store/db_multitenant_database.go @@ -50,13 +50,13 @@ type rawMultitenantDatabases []*rawMultitenantDatabase func (r *rawMultitenantDatabase) toMultitenantDatabase() (*model.MultitenantDatabase, error) { // We only need to set values that are converted from a raw database format. if r.InstallationsRaw != nil { - err := json.Unmarshal(r.InstallationsRaw, &r.MultitenantDatabase.Installations) + err := json.Unmarshal(r.InstallationsRaw, &r.MultitenantDatabase.Installations) // nolint:staticcheck if err != nil { return nil, err } } if r.MigratedInstallationsRaw != nil { - err := json.Unmarshal(r.MigratedInstallationsRaw, &r.MultitenantDatabase.MigratedInstallations) + err := json.Unmarshal(r.MigratedInstallationsRaw, &r.MultitenantDatabase.MigratedInstallations) // nolint:staticcheck if err != nil { return nil, err } diff --git a/internal/store/events.go b/internal/store/events.go index b87235c9b..8bb608d04 100644 --- a/internal/store/events.go +++ b/internal/store/events.go @@ -249,10 +249,10 @@ func (sqlStore *SQLStore) GetStateChangeEvent(eventID string) (*model.StateChang func (sqlStore *SQLStore) GetStateChangeEvents(filter *model.StateChangeEventFilter) ([]*model.StateChangeEventData, error) { query := stateChangeEventSelect.OrderBy("e.Timestamp DESC") - if filter.Paging.PerPage != model.AllPerPage { + if filter.Paging.PerPage != model.AllPerPage { // nolint: staticcheck query = query. - Limit(uint64(filter.Paging.PerPage)). - Offset(uint64(filter.Paging.Page * filter.Paging.PerPage)) + Limit(uint64(filter.Paging.PerPage)). // nolint: staticcheck + Offset(uint64(filter.Paging.Page * filter.Paging.PerPage)) // nolint: staticcheck } if filter.ResourceType != "" { diff --git a/internal/store/group.go b/internal/store/group.go index 50c874cfe..7b4718f79 100644 --- a/internal/store/group.go +++ b/internal/store/group.go @@ -45,7 +45,7 @@ func (r *rawGroup) toGroup() (*model.Group, error) { } } - r.Group.MattermostEnv = *mattermostEnv + r.Group.MattermostEnv = *mattermostEnv // nolint:staticcheck return r.Group, nil } diff --git a/internal/store/installation.go b/internal/store/installation.go index b21b2cf35..c82b3ef87 100644 --- a/internal/store/installation.go +++ b/internal/store/installation.go @@ -23,13 +23,15 @@ const ( func init() { installationSelect = sq. Select( - "Installation.ID", "Installation.Name", "OwnerID", "Version", "Image", "Database", "Filestore", "Size", - "Affinity", "GroupID", "GroupSequence", "Installation.State", "License", - "MattermostEnvRaw", "PriorityEnvRaw", "SingleTenantDatabaseConfigRaw", "ExternalDatabaseConfigRaw", - "Installation.CreateAt", "Installation.DeleteAt", "Installation.DeletionPendingExpiry", - "APISecurityLock", "LockAcquiredBy", "LockAcquiredAt", "CRVersion", "Installation.DeletionLocked", "AllowedIPRanges", - ). - From(installationTable) + "Installation.ID", "Installation.Name", "OwnerID", "Version", "Image", + "Database", "Filestore", "Size", "Affinity", "GroupID", "GroupSequence", + "Installation.State", "License", "MattermostEnvRaw", "PriorityEnvRaw", + "SingleTenantDatabaseConfigRaw", "ExternalDatabaseConfigRaw", + "Installation.CreateAt", "Installation.DeleteAt", + "Installation.DeletionPendingExpiry", "APISecurityLock", "LockAcquiredBy", + "LockAcquiredAt", "CRVersion", "Installation.DeletionLocked", + "AllowedIPRanges", "Volumes", + ).From(installationTable) } type rawInstallation struct { @@ -52,7 +54,7 @@ func (r *rawInstallation) toInstallation() (*model.Installation, error) { return nil, err } } - r.Installation.MattermostEnv = *mattermostEnv + r.Installation.MattermostEnv = *mattermostEnv // nolint:staticcheck priorityEnv := &model.EnvVarMap{} if r.PriorityEnvRaw != nil { @@ -61,7 +63,7 @@ func (r *rawInstallation) toInstallation() (*model.Installation, error) { return nil, err } } - r.Installation.PriorityEnv = *priorityEnv + r.Installation.PriorityEnv = *priorityEnv // nolint:staticcheck if r.SingleTenantDatabaseConfigRaw != nil { singleTenantDBConfig := &model.SingleTenantDatabaseConfig{} @@ -69,7 +71,7 @@ func (r *rawInstallation) toInstallation() (*model.Installation, error) { if err != nil { return nil, err } - r.Installation.SingleTenantDatabaseConfig = singleTenantDBConfig + r.Installation.SingleTenantDatabaseConfig = singleTenantDBConfig // nolint:staticcheck } if r.ExternalDatabaseConfigRaw != nil { @@ -78,7 +80,7 @@ func (r *rawInstallation) toInstallation() (*model.Installation, error) { if err != nil { return nil, err } - r.Installation.ExternalDatabaseConfig = externalDBConfig + r.Installation.ExternalDatabaseConfig = externalDBConfig // nolint:staticcheck } return r.Installation, nil @@ -453,6 +455,7 @@ func (sqlStore *SQLStore) createInstallation(db execer, installation *model.Inst "CRVersion": installation.CRVersion, "DeletionLocked": installation.DeletionLocked, "AllowedIPRanges": installation.AllowedIPRanges, + "Volumes": installation.Volumes, } singleTenantDBConfJSON, err := installation.SingleTenantDatabaseConfig.ToJSON() @@ -480,6 +483,9 @@ func (sqlStore *SQLStore) createInstallation(db execer, installation *model.Inst SetMap(insertsMap), ) if err != nil { + if isUniqueConstraintViolation(err) { + return &UniqueConstraintError{} + } return errors.Wrap(err, "failed to create installation") } @@ -524,6 +530,7 @@ func (sqlStore *SQLStore) updateInstallation(db execer, installation *model.Inst "CRVersion": installation.CRVersion, "DeletionPendingExpiry": installation.DeletionPendingExpiry, "AllowedIPRanges": installation.AllowedIPRanges, + "Volumes": installation.Volumes, }). Where("ID = ?", installation.ID), ) diff --git a/internal/store/installation_backup.go b/internal/store/installation_backup.go index fe8786f8e..213d1ba04 100644 --- a/internal/store/installation_backup.go +++ b/internal/store/installation_backup.go @@ -54,7 +54,7 @@ func (r *rawInstallationBackup) toInstallationBackup() (*model.InstallationBacku if err != nil { return nil, err } - r.InstallationBackup.DataResidence = &dataResidence + r.InstallationBackup.DataResidence = &dataResidence // nolint:staticcheck } return r.InstallationBackup, nil diff --git a/internal/store/installation_db_migration.go b/internal/store/installation_db_migration.go index 160a2c246..e444e84a8 100644 --- a/internal/store/installation_db_migration.go +++ b/internal/store/installation_db_migration.go @@ -56,7 +56,7 @@ func (r *rawDBMigrationOperation) toDBMigrationOperation() (*model.InstallationD if err != nil { return nil, err } - r.InstallationDBMigrationOperation.SourceMultiTenant = &data + r.InstallationDBMigrationOperation.SourceMultiTenant = &data // nolint:staticcheck } if len(r.DestinationMultiTenantRaw) > 0 { data := model.MultiTenantDBMigrationData{} @@ -64,7 +64,7 @@ func (r *rawDBMigrationOperation) toDBMigrationOperation() (*model.InstallationD if err != nil { return nil, err } - r.InstallationDBMigrationOperation.DestinationMultiTenant = &data + r.InstallationDBMigrationOperation.DestinationMultiTenant = &data // nolint:staticcheck } return r.InstallationDBMigrationOperation, nil diff --git a/internal/store/store.go b/internal/store/store.go index f6aca7813..192c5457e 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -212,7 +212,7 @@ func (t *Transaction) Commit() error { // RollbackUnlessCommitted rollback the transaction if it is not committed. func (t *Transaction) RollbackUnlessCommitted() { if !t.committed { - err := t.Tx.Rollback() + err := t.Tx.Rollback() // nolint: staticcheck if err != nil { t.sqlStore.logger.Errorf("error: failed to rollback uncommitted transaction: %s", err.Error()) } From 64278d58bd47c24a77d5cf8e1062078cfc4833a6 Mon Sep 17 00:00:00 2001 From: Stavros Foteinopoulos Date: Tue, 1 Apr 2025 14:37:52 +0300 Subject: [PATCH 8/9] fix goimports Signed-off-by: Stavros Foteinopoulos --- cmd/cloud/contexts.go | 3 ++- cmd/tools/ctest/test.go | 3 ++- cmd/tools/cwl/cwl.go | 3 ++- internal/api/cluster.go | 3 ++- internal/api/installation.go | 3 ++- internal/api/installation_backup.go | 3 ++- internal/api/installation_db_migration.go | 3 ++- internal/api/installation_db_restoration_operation.go | 3 ++- internal/api/installation_dns.go | 3 ++- internal/api/response_writer_wrapper_test.go | 3 ++- internal/auth/device_authorization.go | 3 ++- internal/mocks/awat/client.go | 3 ++- internal/mocks/aws-sdk/acm.go | 3 ++- internal/mocks/aws-sdk/dynamodb.go | 3 ++- internal/mocks/aws-sdk/ec2.go | 3 ++- internal/mocks/aws-sdk/iam.go | 3 ++- internal/mocks/aws-sdk/kms.go | 3 ++- internal/mocks/aws-sdk/rds.go | 3 ++- internal/mocks/aws-sdk/resource_tagging.go | 3 ++- internal/mocks/aws-sdk/route53.go | 3 ++- internal/mocks/aws-sdk/s3.go | 3 ++- internal/mocks/aws-sdk/secrets_manager.go | 3 ++- internal/mocks/aws-sdk/sts.go | 3 ++- internal/mocks/aws-tools/client.go | 3 ++- internal/mocks/logger/doc.go | 1 + internal/mocks/logger/logrus.go | 3 ++- internal/mocks/model/doc.go | 1 + internal/mocks/model/installation_database.go | 3 ++- internal/store/migrations.go | 1 + internal/supervisor/installation_deletion_test.go | 3 ++- internal/tools/argocd/argocd_apps_test.go | 3 ++- internal/tools/argocd/argocd_test.go | 3 ++- internal/tools/aws/ec2_test.go | 3 ++- model/cluster.go | 3 ++- model/cluster_installation.go | 3 ++- model/group.go | 3 ++- model/id.go | 3 ++- model/installation.go | 3 ++- 38 files changed, 73 insertions(+), 35 deletions(-) diff --git a/cmd/cloud/contexts.go b/cmd/cloud/contexts.go index f87e30cb3..31e4c0a2d 100644 --- a/cmd/cloud/contexts.go +++ b/cmd/cloud/contexts.go @@ -2,9 +2,10 @@ package main import ( "fmt" - log "github.com/sirupsen/logrus" "reflect" + log "github.com/sirupsen/logrus" + "github.com/mattermost/mattermost-cloud/cmd/cloud/clicontext" "github.com/mattermost/mattermost-cloud/internal/auth" "github.com/spf13/cobra" diff --git a/cmd/tools/ctest/test.go b/cmd/tools/ctest/test.go index 84aa9b233..e7cc6064e 100644 --- a/cmd/tools/ctest/test.go +++ b/cmd/tools/ctest/test.go @@ -7,10 +7,11 @@ package main import ( "encoding/json" "fmt" - log "github.com/sirupsen/logrus" "io" "net/http" + log "github.com/sirupsen/logrus" + "github.com/mattermost/mattermost-cloud/model" "github.com/pkg/errors" ) diff --git a/cmd/tools/cwl/cwl.go b/cmd/tools/cwl/cwl.go index 2034b59e4..78346c39a 100644 --- a/cmd/tools/cwl/cwl.go +++ b/cmd/tools/cwl/cwl.go @@ -7,11 +7,12 @@ package main import ( "embed" "fmt" - "github.com/sirupsen/logrus" "log" "net/http" "os" + "github.com/sirupsen/logrus" + cloud "github.com/mattermost/mattermost-cloud/model" "github.com/0xAX/notificator" diff --git a/internal/api/cluster.go b/internal/api/cluster.go index 739cf4637..d478fc881 100644 --- a/internal/api/cluster.go +++ b/internal/api/cluster.go @@ -5,9 +5,10 @@ package api import ( - log "github.com/sirupsen/logrus" "net/http" + log "github.com/sirupsen/logrus" + "github.com/gorilla/mux" "github.com/mattermost/mattermost-cloud/internal/store" "github.com/mattermost/mattermost-cloud/model" diff --git a/internal/api/installation.go b/internal/api/installation.go index ec99b92ef..285af0cfc 100644 --- a/internal/api/installation.go +++ b/internal/api/installation.go @@ -7,10 +7,11 @@ package api import ( "encoding/json" "fmt" - log "github.com/sirupsen/logrus" "net/http" "time" + log "github.com/sirupsen/logrus" + "github.com/mattermost/mattermost-cloud/internal/common" "github.com/mattermost/mattermost-cloud/internal/events" diff --git a/internal/api/installation_backup.go b/internal/api/installation_backup.go index e488a767e..fa9093050 100644 --- a/internal/api/installation_backup.go +++ b/internal/api/installation_backup.go @@ -5,10 +5,11 @@ package api import ( - log "github.com/sirupsen/logrus" "net/http" "time" + log "github.com/sirupsen/logrus" + "github.com/mattermost/mattermost-cloud/internal/common" "github.com/mattermost/mattermost-cloud/internal/webhook" diff --git a/internal/api/installation_db_migration.go b/internal/api/installation_db_migration.go index 6c1de24fb..6dd7b8dce 100644 --- a/internal/api/installation_db_migration.go +++ b/internal/api/installation_db_migration.go @@ -5,10 +5,11 @@ package api import ( - log "github.com/sirupsen/logrus" "net/http" "time" + log "github.com/sirupsen/logrus" + "github.com/gorilla/mux" "github.com/mattermost/mattermost-cloud/internal/common" "github.com/mattermost/mattermost-cloud/internal/tools/aws" diff --git a/internal/api/installation_db_restoration_operation.go b/internal/api/installation_db_restoration_operation.go index 2cb51bc37..d8f30c091 100644 --- a/internal/api/installation_db_restoration_operation.go +++ b/internal/api/installation_db_restoration_operation.go @@ -5,9 +5,10 @@ package api import ( - log "github.com/sirupsen/logrus" "net/http" + log "github.com/sirupsen/logrus" + "github.com/gorilla/mux" "github.com/mattermost/mattermost-cloud/internal/common" "github.com/mattermost/mattermost-cloud/model" diff --git a/internal/api/installation_dns.go b/internal/api/installation_dns.go index fd5d3e774..ddaa215bf 100644 --- a/internal/api/installation_dns.go +++ b/internal/api/installation_dns.go @@ -5,10 +5,11 @@ package api import ( - log "github.com/sirupsen/logrus" "net/http" "strings" + log "github.com/sirupsen/logrus" + "github.com/gorilla/mux" "github.com/mattermost/mattermost-cloud/model" ) diff --git a/internal/api/response_writer_wrapper_test.go b/internal/api/response_writer_wrapper_test.go index fd228b019..dbbeaf0db 100644 --- a/internal/api/response_writer_wrapper_test.go +++ b/internal/api/response_writer_wrapper_test.go @@ -2,12 +2,13 @@ package api_test import ( "bufio" - log "github.com/sirupsen/logrus" "net" "net/http" "net/http/httptest" "testing" + log "github.com/sirupsen/logrus" + "github.com/mattermost/mattermost-cloud/internal/api" "github.com/stretchr/testify/assert" ) diff --git a/internal/auth/device_authorization.go b/internal/auth/device_authorization.go index 1dad253be..89d9a98f3 100644 --- a/internal/auth/device_authorization.go +++ b/internal/auth/device_authorization.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "github.com/sirupsen/logrus" "log" "net/http" "net/url" @@ -13,6 +12,8 @@ import ( "strings" "time" + "github.com/sirupsen/logrus" + "golang.org/x/oauth2" ) diff --git a/internal/mocks/awat/client.go b/internal/mocks/awat/client.go index 9acb0cac5..5df629663 100644 --- a/internal/mocks/awat/client.go +++ b/internal/mocks/awat/client.go @@ -8,9 +8,10 @@ package mocks import ( + reflect "reflect" + gomock "github.com/golang/mock/gomock" model "github.com/mattermost/awat/model" - reflect "reflect" ) // MockAWATClient is a mock of AWATClient interface diff --git a/internal/mocks/aws-sdk/acm.go b/internal/mocks/aws-sdk/acm.go index 5a9fe652e..d759726aa 100644 --- a/internal/mocks/aws-sdk/acm.go +++ b/internal/mocks/aws-sdk/acm.go @@ -9,9 +9,10 @@ package mockawssdk import ( context "context" + reflect "reflect" + acm "github.com/aws/aws-sdk-go-v2/service/acm" gomock "github.com/golang/mock/gomock" - reflect "reflect" ) // MockACMAPI is a mock of ACMAPI interface diff --git a/internal/mocks/aws-sdk/dynamodb.go b/internal/mocks/aws-sdk/dynamodb.go index 6469cc160..1266b6157 100644 --- a/internal/mocks/aws-sdk/dynamodb.go +++ b/internal/mocks/aws-sdk/dynamodb.go @@ -9,9 +9,10 @@ package mockawssdk import ( context "context" + reflect "reflect" + dynamodb "github.com/aws/aws-sdk-go-v2/service/dynamodb" gomock "github.com/golang/mock/gomock" - reflect "reflect" ) // MockDynamoDBAPI is a mock of DynamoDBAPI interface diff --git a/internal/mocks/aws-sdk/ec2.go b/internal/mocks/aws-sdk/ec2.go index 75d7e1431..408069bf3 100644 --- a/internal/mocks/aws-sdk/ec2.go +++ b/internal/mocks/aws-sdk/ec2.go @@ -9,9 +9,10 @@ package mockawssdk import ( context "context" + reflect "reflect" + ec2 "github.com/aws/aws-sdk-go-v2/service/ec2" gomock "github.com/golang/mock/gomock" - reflect "reflect" ) // MockEC2API is a mock of EC2API interface diff --git a/internal/mocks/aws-sdk/iam.go b/internal/mocks/aws-sdk/iam.go index 7176715d2..e1ae6a39c 100644 --- a/internal/mocks/aws-sdk/iam.go +++ b/internal/mocks/aws-sdk/iam.go @@ -9,9 +9,10 @@ package mockawssdk import ( context "context" + reflect "reflect" + iam "github.com/aws/aws-sdk-go-v2/service/iam" gomock "github.com/golang/mock/gomock" - reflect "reflect" ) // MockIAMAPI is a mock of IAMAPI interface diff --git a/internal/mocks/aws-sdk/kms.go b/internal/mocks/aws-sdk/kms.go index 5b502773c..d64857185 100644 --- a/internal/mocks/aws-sdk/kms.go +++ b/internal/mocks/aws-sdk/kms.go @@ -9,9 +9,10 @@ package mockawssdk import ( context "context" + reflect "reflect" + kms "github.com/aws/aws-sdk-go-v2/service/kms" gomock "github.com/golang/mock/gomock" - reflect "reflect" ) // MockKMSAPI is a mock of KMSAPI interface diff --git a/internal/mocks/aws-sdk/rds.go b/internal/mocks/aws-sdk/rds.go index 6144e98e4..6aa25701d 100644 --- a/internal/mocks/aws-sdk/rds.go +++ b/internal/mocks/aws-sdk/rds.go @@ -9,9 +9,10 @@ package mockawssdk import ( context "context" + reflect "reflect" + rds "github.com/aws/aws-sdk-go-v2/service/rds" gomock "github.com/golang/mock/gomock" - reflect "reflect" ) // MockRDSAPI is a mock of RDSAPI interface diff --git a/internal/mocks/aws-sdk/resource_tagging.go b/internal/mocks/aws-sdk/resource_tagging.go index bbe71ed59..3d23628bf 100644 --- a/internal/mocks/aws-sdk/resource_tagging.go +++ b/internal/mocks/aws-sdk/resource_tagging.go @@ -9,9 +9,10 @@ package mockawssdk import ( context "context" + reflect "reflect" + resourcegroupstaggingapi "github.com/aws/aws-sdk-go-v2/service/resourcegroupstaggingapi" gomock "github.com/golang/mock/gomock" - reflect "reflect" ) // MockResourceGroupsTaggingAPIAPI is a mock of ResourceGroupsTaggingAPIAPI interface diff --git a/internal/mocks/aws-sdk/route53.go b/internal/mocks/aws-sdk/route53.go index 54486a600..55661bc7f 100644 --- a/internal/mocks/aws-sdk/route53.go +++ b/internal/mocks/aws-sdk/route53.go @@ -9,9 +9,10 @@ package mockawssdk import ( context "context" + reflect "reflect" + route53 "github.com/aws/aws-sdk-go-v2/service/route53" gomock "github.com/golang/mock/gomock" - reflect "reflect" ) // MockRoute53API is a mock of Route53API interface diff --git a/internal/mocks/aws-sdk/s3.go b/internal/mocks/aws-sdk/s3.go index 5694a5ec0..0012f49c7 100644 --- a/internal/mocks/aws-sdk/s3.go +++ b/internal/mocks/aws-sdk/s3.go @@ -9,9 +9,10 @@ package mockawssdk import ( context "context" + reflect "reflect" + s3 "github.com/aws/aws-sdk-go-v2/service/s3" gomock "github.com/golang/mock/gomock" - reflect "reflect" ) // MockS3API is a mock of S3API interface diff --git a/internal/mocks/aws-sdk/secrets_manager.go b/internal/mocks/aws-sdk/secrets_manager.go index 62c00f650..b8eeb4e96 100644 --- a/internal/mocks/aws-sdk/secrets_manager.go +++ b/internal/mocks/aws-sdk/secrets_manager.go @@ -9,9 +9,10 @@ package mockawssdk import ( context "context" + reflect "reflect" + secretsmanager "github.com/aws/aws-sdk-go-v2/service/secretsmanager" gomock "github.com/golang/mock/gomock" - reflect "reflect" ) // MockSecretsManagerAPI is a mock of SecretsManagerAPI interface diff --git a/internal/mocks/aws-sdk/sts.go b/internal/mocks/aws-sdk/sts.go index 384d0da71..2cc2dc96c 100644 --- a/internal/mocks/aws-sdk/sts.go +++ b/internal/mocks/aws-sdk/sts.go @@ -9,9 +9,10 @@ package mockawssdk import ( context "context" + reflect "reflect" + sts "github.com/aws/aws-sdk-go-v2/service/sts" gomock "github.com/golang/mock/gomock" - reflect "reflect" ) // MockSTSAPI is a mock of STSAPI interface diff --git a/internal/mocks/aws-tools/client.go b/internal/mocks/aws-tools/client.go index 0e470969a..d00e3af4a 100644 --- a/internal/mocks/aws-tools/client.go +++ b/internal/mocks/aws-tools/client.go @@ -8,13 +8,14 @@ package mockawstools import ( + reflect "reflect" + types "github.com/aws/aws-sdk-go-v2/service/eks/types" gomock "github.com/golang/mock/gomock" aws "github.com/mattermost/mattermost-cloud/internal/tools/aws" model "github.com/mattermost/mattermost-cloud/model" logrus "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" - reflect "reflect" ) // MockAWS is a mock of AWS interface diff --git a/internal/mocks/logger/doc.go b/internal/mocks/logger/doc.go index 2205ad1b9..d0730e391 100644 --- a/internal/mocks/logger/doc.go +++ b/internal/mocks/logger/doc.go @@ -3,6 +3,7 @@ // // Package mocklogger to create the mocks run go generate to regenerate this package. +// //go:generate ../../../bin/mockgen -package=mocklogger -destination ./logrus.go github.com/sirupsen/logrus StdLogger,FieldLogger,Ext1FieldLogger //go:generate /usr/bin/env bash -c "cat ../../../hack/boilerplate/boilerplate.generatego.txt logrus.go > _logrus.go && mv _logrus.go logrus.go" package mocklogger //nolint diff --git a/internal/mocks/logger/logrus.go b/internal/mocks/logger/logrus.go index 258da7b72..bd0ea90f8 100644 --- a/internal/mocks/logger/logrus.go +++ b/internal/mocks/logger/logrus.go @@ -8,9 +8,10 @@ package mocklogger import ( + reflect "reflect" + gomock "github.com/golang/mock/gomock" logrus "github.com/sirupsen/logrus" - reflect "reflect" ) // MockStdLogger is a mock of StdLogger interface diff --git a/internal/mocks/model/doc.go b/internal/mocks/model/doc.go index 1b710bfc8..88a69ddab 100644 --- a/internal/mocks/model/doc.go +++ b/internal/mocks/model/doc.go @@ -3,6 +3,7 @@ // // Package mockmodel to create the mocks run go generate to regenerate this package. +// //go:generate ../../../bin/mockgen -package=mockmodel -destination ./installation_database.go -source ../../../model/installation_database.go //go:generate /usr/bin/env bash -c "cat ../../../hack/boilerplate/boilerplate.generatego.txt installation_database.go > _installation_database.go && mv _installation_database.go installation_database.go" package mockmodel diff --git a/internal/mocks/model/installation_database.go b/internal/mocks/model/installation_database.go index 11aff6227..0f27ee345 100644 --- a/internal/mocks/model/installation_database.go +++ b/internal/mocks/model/installation_database.go @@ -8,11 +8,12 @@ package mockmodel import ( + reflect "reflect" + gomock "github.com/golang/mock/gomock" model "github.com/mattermost/mattermost-cloud/model" logrus "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" - reflect "reflect" ) // MockDatabase is a mock of Database interface diff --git a/internal/store/migrations.go b/internal/store/migrations.go index 215a35703..9f1b9d95a 100644 --- a/internal/store/migrations.go +++ b/internal/store/migrations.go @@ -7,6 +7,7 @@ package store import ( "encoding/json" "fmt" + log "github.com/sirupsen/logrus" "github.com/blang/semver" diff --git a/internal/supervisor/installation_deletion_test.go b/internal/supervisor/installation_deletion_test.go index 83ebf392c..7898a33a2 100644 --- a/internal/supervisor/installation_deletion_test.go +++ b/internal/supervisor/installation_deletion_test.go @@ -5,10 +5,11 @@ package supervisor_test import ( - log "github.com/sirupsen/logrus" "testing" "time" + log "github.com/sirupsen/logrus" + "github.com/mattermost/mattermost-cloud/internal/events" "github.com/mattermost/mattermost-cloud/internal/store" "github.com/mattermost/mattermost-cloud/internal/supervisor" diff --git a/internal/tools/argocd/argocd_apps_test.go b/internal/tools/argocd/argocd_apps_test.go index 5b49bf4c3..138e227fa 100644 --- a/internal/tools/argocd/argocd_apps_test.go +++ b/internal/tools/argocd/argocd_apps_test.go @@ -1,11 +1,12 @@ package argocd import ( - log "github.com/sirupsen/logrus" "os" "reflect" "testing" + log "github.com/sirupsen/logrus" + "github.com/mattermost/mattermost-cloud/internal/testlib" "gopkg.in/yaml.v3" ) diff --git a/internal/tools/argocd/argocd_test.go b/internal/tools/argocd/argocd_test.go index 64ea1ae07..734a4a0d9 100644 --- a/internal/tools/argocd/argocd_test.go +++ b/internal/tools/argocd/argocd_test.go @@ -2,11 +2,12 @@ package argocd import ( "encoding/base64" - log "github.com/sirupsen/logrus" "os" "path" "testing" + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "gopkg.in/yaml.v3" diff --git a/internal/tools/aws/ec2_test.go b/internal/tools/aws/ec2_test.go index 5f560db20..fbef6fd23 100644 --- a/internal/tools/aws/ec2_test.go +++ b/internal/tools/aws/ec2_test.go @@ -6,11 +6,12 @@ package aws import ( "context" - "github.com/aws/aws-sdk-go-v2/aws" "os" "sync" "testing" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/mattermost/mattermost-cloud/model" "github.com/aws/aws-sdk-go-v2/service/ec2" diff --git a/model/cluster.go b/model/cluster.go index a3679b514..704c49c39 100644 --- a/model/cluster.go +++ b/model/cluster.go @@ -6,9 +6,10 @@ package model import ( "encoding/json" - log "github.com/sirupsen/logrus" "io" "regexp" + + log "github.com/sirupsen/logrus" ) //go:generate provisioner-code-gen generate --out-file=cluster_gen.go --boilerplate-file=../hack/boilerplate/boilerplate.generatego.txt --type=github.com/mattermost/mattermost-cloud/model.Cluster --generator=get_id,get_state,is_deleted,as_resources diff --git a/model/cluster_installation.go b/model/cluster_installation.go index e3031d28c..3b5c7b35d 100644 --- a/model/cluster_installation.go +++ b/model/cluster_installation.go @@ -7,9 +7,10 @@ package model import ( "encoding/json" "fmt" - log "github.com/sirupsen/logrus" "io" + log "github.com/sirupsen/logrus" + "github.com/pkg/errors" ) diff --git a/model/group.go b/model/group.go index c6cf192ca..847196d38 100644 --- a/model/group.go +++ b/model/group.go @@ -6,8 +6,9 @@ package model import ( "encoding/json" - log "github.com/sirupsen/logrus" "io" + + log "github.com/sirupsen/logrus" ) // Group represents a group of Mattermost installations. diff --git a/model/id.go b/model/id.go index 2ddffe689..56515e2c4 100644 --- a/model/id.go +++ b/model/id.go @@ -7,11 +7,12 @@ package model import ( "bytes" "encoding/base32" - "github.com/sirupsen/logrus" "math/rand" "time" "unicode" + "github.com/sirupsen/logrus" + "github.com/pborman/uuid" ) diff --git a/model/installation.go b/model/installation.go index e05ed986e..1ed3c7aa8 100644 --- a/model/installation.go +++ b/model/installation.go @@ -8,10 +8,11 @@ import ( "database/sql/driver" "encoding/json" "fmt" - log "github.com/sirupsen/logrus" "io" "strings" + log "github.com/sirupsen/logrus" + "github.com/pkg/errors" ) From eb3653a6f1e0bdd1929bb2f82211adab6970af9b Mon Sep 17 00:00:00 2001 From: Stavros Foteinopoulos Date: Tue, 1 Apr 2025 15:16:37 +0300 Subject: [PATCH 9/9] verify mocks Signed-off-by: Stavros Foteinopoulos --- internal/mocks/awat/client.go | 3 +-- internal/mocks/aws-sdk/acm.go | 3 +-- internal/mocks/aws-sdk/dynamodb.go | 3 +-- internal/mocks/aws-sdk/ec2.go | 3 +-- internal/mocks/aws-sdk/iam.go | 3 +-- internal/mocks/aws-sdk/kms.go | 3 +-- internal/mocks/aws-sdk/rds.go | 3 +-- internal/mocks/aws-sdk/resource_tagging.go | 3 +-- internal/mocks/aws-sdk/route53.go | 3 +-- internal/mocks/aws-sdk/s3.go | 3 +-- internal/mocks/aws-sdk/secrets_manager.go | 3 +-- internal/mocks/aws-sdk/sts.go | 3 +-- internal/mocks/aws-tools/client.go | 3 +-- internal/mocks/logger/logrus.go | 3 +-- internal/mocks/model/installation_database.go | 3 +-- 15 files changed, 15 insertions(+), 30 deletions(-) diff --git a/internal/mocks/awat/client.go b/internal/mocks/awat/client.go index 5df629663..9acb0cac5 100644 --- a/internal/mocks/awat/client.go +++ b/internal/mocks/awat/client.go @@ -8,10 +8,9 @@ package mocks import ( - reflect "reflect" - gomock "github.com/golang/mock/gomock" model "github.com/mattermost/awat/model" + reflect "reflect" ) // MockAWATClient is a mock of AWATClient interface diff --git a/internal/mocks/aws-sdk/acm.go b/internal/mocks/aws-sdk/acm.go index d759726aa..5a9fe652e 100644 --- a/internal/mocks/aws-sdk/acm.go +++ b/internal/mocks/aws-sdk/acm.go @@ -9,10 +9,9 @@ package mockawssdk import ( context "context" - reflect "reflect" - acm "github.com/aws/aws-sdk-go-v2/service/acm" gomock "github.com/golang/mock/gomock" + reflect "reflect" ) // MockACMAPI is a mock of ACMAPI interface diff --git a/internal/mocks/aws-sdk/dynamodb.go b/internal/mocks/aws-sdk/dynamodb.go index 1266b6157..6469cc160 100644 --- a/internal/mocks/aws-sdk/dynamodb.go +++ b/internal/mocks/aws-sdk/dynamodb.go @@ -9,10 +9,9 @@ package mockawssdk import ( context "context" - reflect "reflect" - dynamodb "github.com/aws/aws-sdk-go-v2/service/dynamodb" gomock "github.com/golang/mock/gomock" + reflect "reflect" ) // MockDynamoDBAPI is a mock of DynamoDBAPI interface diff --git a/internal/mocks/aws-sdk/ec2.go b/internal/mocks/aws-sdk/ec2.go index 408069bf3..75d7e1431 100644 --- a/internal/mocks/aws-sdk/ec2.go +++ b/internal/mocks/aws-sdk/ec2.go @@ -9,10 +9,9 @@ package mockawssdk import ( context "context" - reflect "reflect" - ec2 "github.com/aws/aws-sdk-go-v2/service/ec2" gomock "github.com/golang/mock/gomock" + reflect "reflect" ) // MockEC2API is a mock of EC2API interface diff --git a/internal/mocks/aws-sdk/iam.go b/internal/mocks/aws-sdk/iam.go index e1ae6a39c..7176715d2 100644 --- a/internal/mocks/aws-sdk/iam.go +++ b/internal/mocks/aws-sdk/iam.go @@ -9,10 +9,9 @@ package mockawssdk import ( context "context" - reflect "reflect" - iam "github.com/aws/aws-sdk-go-v2/service/iam" gomock "github.com/golang/mock/gomock" + reflect "reflect" ) // MockIAMAPI is a mock of IAMAPI interface diff --git a/internal/mocks/aws-sdk/kms.go b/internal/mocks/aws-sdk/kms.go index d64857185..5b502773c 100644 --- a/internal/mocks/aws-sdk/kms.go +++ b/internal/mocks/aws-sdk/kms.go @@ -9,10 +9,9 @@ package mockawssdk import ( context "context" - reflect "reflect" - kms "github.com/aws/aws-sdk-go-v2/service/kms" gomock "github.com/golang/mock/gomock" + reflect "reflect" ) // MockKMSAPI is a mock of KMSAPI interface diff --git a/internal/mocks/aws-sdk/rds.go b/internal/mocks/aws-sdk/rds.go index 6aa25701d..6144e98e4 100644 --- a/internal/mocks/aws-sdk/rds.go +++ b/internal/mocks/aws-sdk/rds.go @@ -9,10 +9,9 @@ package mockawssdk import ( context "context" - reflect "reflect" - rds "github.com/aws/aws-sdk-go-v2/service/rds" gomock "github.com/golang/mock/gomock" + reflect "reflect" ) // MockRDSAPI is a mock of RDSAPI interface diff --git a/internal/mocks/aws-sdk/resource_tagging.go b/internal/mocks/aws-sdk/resource_tagging.go index 3d23628bf..bbe71ed59 100644 --- a/internal/mocks/aws-sdk/resource_tagging.go +++ b/internal/mocks/aws-sdk/resource_tagging.go @@ -9,10 +9,9 @@ package mockawssdk import ( context "context" - reflect "reflect" - resourcegroupstaggingapi "github.com/aws/aws-sdk-go-v2/service/resourcegroupstaggingapi" gomock "github.com/golang/mock/gomock" + reflect "reflect" ) // MockResourceGroupsTaggingAPIAPI is a mock of ResourceGroupsTaggingAPIAPI interface diff --git a/internal/mocks/aws-sdk/route53.go b/internal/mocks/aws-sdk/route53.go index 55661bc7f..54486a600 100644 --- a/internal/mocks/aws-sdk/route53.go +++ b/internal/mocks/aws-sdk/route53.go @@ -9,10 +9,9 @@ package mockawssdk import ( context "context" - reflect "reflect" - route53 "github.com/aws/aws-sdk-go-v2/service/route53" gomock "github.com/golang/mock/gomock" + reflect "reflect" ) // MockRoute53API is a mock of Route53API interface diff --git a/internal/mocks/aws-sdk/s3.go b/internal/mocks/aws-sdk/s3.go index 0012f49c7..5694a5ec0 100644 --- a/internal/mocks/aws-sdk/s3.go +++ b/internal/mocks/aws-sdk/s3.go @@ -9,10 +9,9 @@ package mockawssdk import ( context "context" - reflect "reflect" - s3 "github.com/aws/aws-sdk-go-v2/service/s3" gomock "github.com/golang/mock/gomock" + reflect "reflect" ) // MockS3API is a mock of S3API interface diff --git a/internal/mocks/aws-sdk/secrets_manager.go b/internal/mocks/aws-sdk/secrets_manager.go index b8eeb4e96..62c00f650 100644 --- a/internal/mocks/aws-sdk/secrets_manager.go +++ b/internal/mocks/aws-sdk/secrets_manager.go @@ -9,10 +9,9 @@ package mockawssdk import ( context "context" - reflect "reflect" - secretsmanager "github.com/aws/aws-sdk-go-v2/service/secretsmanager" gomock "github.com/golang/mock/gomock" + reflect "reflect" ) // MockSecretsManagerAPI is a mock of SecretsManagerAPI interface diff --git a/internal/mocks/aws-sdk/sts.go b/internal/mocks/aws-sdk/sts.go index 2cc2dc96c..384d0da71 100644 --- a/internal/mocks/aws-sdk/sts.go +++ b/internal/mocks/aws-sdk/sts.go @@ -9,10 +9,9 @@ package mockawssdk import ( context "context" - reflect "reflect" - sts "github.com/aws/aws-sdk-go-v2/service/sts" gomock "github.com/golang/mock/gomock" + reflect "reflect" ) // MockSTSAPI is a mock of STSAPI interface diff --git a/internal/mocks/aws-tools/client.go b/internal/mocks/aws-tools/client.go index d00e3af4a..0e470969a 100644 --- a/internal/mocks/aws-tools/client.go +++ b/internal/mocks/aws-tools/client.go @@ -8,14 +8,13 @@ package mockawstools import ( - reflect "reflect" - types "github.com/aws/aws-sdk-go-v2/service/eks/types" gomock "github.com/golang/mock/gomock" aws "github.com/mattermost/mattermost-cloud/internal/tools/aws" model "github.com/mattermost/mattermost-cloud/model" logrus "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" + reflect "reflect" ) // MockAWS is a mock of AWS interface diff --git a/internal/mocks/logger/logrus.go b/internal/mocks/logger/logrus.go index bd0ea90f8..258da7b72 100644 --- a/internal/mocks/logger/logrus.go +++ b/internal/mocks/logger/logrus.go @@ -8,10 +8,9 @@ package mocklogger import ( - reflect "reflect" - gomock "github.com/golang/mock/gomock" logrus "github.com/sirupsen/logrus" + reflect "reflect" ) // MockStdLogger is a mock of StdLogger interface diff --git a/internal/mocks/model/installation_database.go b/internal/mocks/model/installation_database.go index 0f27ee345..11aff6227 100644 --- a/internal/mocks/model/installation_database.go +++ b/internal/mocks/model/installation_database.go @@ -8,12 +8,11 @@ package mockmodel import ( - reflect "reflect" - gomock "github.com/golang/mock/gomock" model "github.com/mattermost/mattermost-cloud/model" logrus "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" + reflect "reflect" ) // MockDatabase is a mock of Database interface