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 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..31e4c0a2d 100644 --- a/cmd/cloud/contexts.go +++ b/cmd/cloud/contexts.go @@ -4,6 +4,8 @@ import ( "fmt" "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" @@ -90,7 +92,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 +248,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 +260,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..e7cc6064e 100644 --- a/cmd/tools/ctest/test.go +++ b/cmd/tools/ctest/test.go @@ -10,6 +10,8 @@ import ( "io" "net/http" + log "github.com/sirupsen/logrus" + "github.com/mattermost/mattermost-cloud/model" "github.com/pkg/errors" ) @@ -36,7 +38,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/cmd/tools/cwl/cwl.go b/cmd/tools/cwl/cwl.go index 48a4f007f..78346c39a 100644 --- a/cmd/tools/cwl/cwl.go +++ b/cmd/tools/cwl/cwl.go @@ -11,6 +11,8 @@ import ( "net/http" "os" + "github.com/sirupsen/logrus" + cloud "github.com/mattermost/mattermost-cloud/model" "github.com/0xAX/notificator" @@ -51,7 +53,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.go b/internal/api/cluster.go index 42de7705b..d478fc881 100644 --- a/internal/api/cluster.go +++ b/internal/api/cluster.go @@ -7,6 +7,8 @@ package api import ( "net/http" + log "github.com/sirupsen/logrus" + "github.com/gorilla/mux" "github.com/mattermost/mattermost-cloud/internal/store" "github.com/mattermost/mattermost-cloud/model" @@ -141,7 +143,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 +190,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 +258,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 +318,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 +396,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 +421,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 +453,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 +495,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 +719,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 +826,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..949f9b036 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) @@ -407,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. @@ -464,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/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/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 d979ad287..285af0cfc 100644 --- a/internal/api/installation.go +++ b/internal/api/installation.go @@ -10,6 +10,8 @@ import ( "net/http" "time" + log "github.com/sirupsen/logrus" + "github.com/mattermost/mattermost-cloud/internal/common" "github.com/mattermost/mattermost-cloud/internal/events" @@ -278,7 +280,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 +351,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 +402,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 +477,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 +554,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 +618,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 +641,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 +661,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") @@ -669,7 +689,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) } @@ -730,7 +752,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 +783,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 +832,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 +926,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 +995,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..fa9093050 100644 --- a/internal/api/installation_backup.go +++ b/internal/api/installation_backup.go @@ -8,6 +8,8 @@ import ( "net/http" "time" + log "github.com/sirupsen/logrus" + "github.com/mattermost/mattermost-cloud/internal/common" "github.com/mattermost/mattermost-cloud/internal/webhook" @@ -66,7 +68,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 +198,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..6dd7b8dce 100644 --- a/internal/api/installation_db_migration.go +++ b/internal/api/installation_db_migration.go @@ -8,6 +8,8 @@ import ( "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" @@ -121,7 +123,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 +280,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..d8f30c091 100644 --- a/internal/api/installation_db_restoration_operation.go +++ b/internal/api/installation_db_restoration_operation.go @@ -7,6 +7,8 @@ package api import ( "net/http" + log "github.com/sirupsen/logrus" + "github.com/gorilla/mux" "github.com/mattermost/mattermost-cloud/internal/common" "github.com/mattermost/mattermost-cloud/model" @@ -72,7 +74,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..ddaa215bf 100644 --- a/internal/api/installation_dns.go +++ b/internal/api/installation_dns.go @@ -8,6 +8,8 @@ import ( "net/http" "strings" + log "github.com/sirupsen/logrus" + "github.com/gorilla/mux" "github.com/mattermost/mattermost-cloud/model" ) @@ -26,7 +28,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 +76,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 +151,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 +240,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/api/response_writer_wrapper_test.go b/internal/api/response_writer_wrapper_test.go index 719f9a8f9..dbbeaf0db 100644 --- a/internal/api/response_writer_wrapper_test.go +++ b/internal/api/response_writer_wrapper_test.go @@ -7,6 +7,8 @@ import ( "net/http/httptest" "testing" + log "github.com/sirupsen/logrus" + "github.com/mattermost/mattermost-cloud/internal/api" "github.com/stretchr/testify/assert" ) @@ -46,7 +48,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()) @@ -58,7 +62,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) } @@ -77,7 +81,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..89d9a98f3 100644 --- a/internal/auth/device_authorization.go +++ b/internal/auth/device_authorization.go @@ -12,6 +12,8 @@ import ( "strings" "time" + "github.com/sirupsen/logrus" + "golang.org/x/oauth2" ) @@ -38,7 +40,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 +100,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 +140,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/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/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/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/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/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/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/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 8aebaebe6..c82b3ef87 100644 --- a/internal/store/installation.go +++ b/internal/store/installation.go @@ -54,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 { @@ -63,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{} @@ -71,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 { @@ -80,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 @@ -385,7 +385,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. @@ -486,7 +486,6 @@ func (sqlStore *SQLStore) createInstallation(db execer, installation *model.Inst if isUniqueConstraintViolation(err) { return &UniqueConstraintError{} } - return errors.Wrap(err, "failed to create installation") } 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/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/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..9f1b9d95a 100644 --- a/internal/store/migrations.go +++ b/internal/store/migrations.go @@ -8,6 +8,8 @@ import ( "encoding/json" "fmt" + log "github.com/sirupsen/logrus" + "github.com/blang/semver" "github.com/mattermost/mattermost-cloud/model" "github.com/pkg/errors" @@ -1398,7 +1400,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 +1502,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 +1793,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 7685972a0..192c5457e 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. @@ -210,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()) } 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..7898a33a2 100644 --- a/internal/supervisor/installation_deletion_test.go +++ b/internal/supervisor/installation_deletion_test.go @@ -8,6 +8,8 @@ import ( "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" @@ -287,7 +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..38702ef78 100644 --- a/internal/supervisor/scheduler_test.go +++ b/internal/supervisor/scheduler_test.go @@ -10,7 +10,9 @@ import ( "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) { @@ -23,14 +25,21 @@ 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: assert.Fail(t, "doer should not have been invoked") case <-time.After(500 * time.Millisecond): + // Expected: no invocation. } }) @@ -41,7 +50,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 +72,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,14 +96,19 @@ func TestScheduler(t *testing.T) { calls: make(chan bool, 1), } scheduler := supervisor.NewScheduler(doer, 30*time.Second, logger) - scheduler.Close() + // Explicitly close the scheduler before calling Do. + require.NoError(t, scheduler.Close(), "failed to close scheduler") - scheduler.Do() + // Now, calling Do() should not schedule any work. + if err := scheduler.Do(); err != nil { + 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. } }) @@ -95,14 +119,21 @@ 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..138e227fa 100644 --- a/internal/tools/argocd/argocd_apps_test.go +++ b/internal/tools/argocd/argocd_apps_test.go @@ -5,6 +5,8 @@ import ( "reflect" "testing" + log "github.com/sirupsen/logrus" + "github.com/mattermost/mattermost-cloud/internal/testlib" "gopkg.in/yaml.v3" ) @@ -14,7 +16,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 +113,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 +210,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..734a4a0d9 100644 --- a/internal/tools/argocd/argocd_test.go +++ b/internal/tools/argocd/argocd_test.go @@ -6,6 +6,8 @@ import ( "path" "testing" + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "gopkg.in/yaml.v3" @@ -95,7 +97,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() { @@ -112,13 +118,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/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/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/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/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/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/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/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..704c49c39 100644 --- a/model/cluster.go +++ b/model/cluster.go @@ -8,6 +8,8 @@ import ( "encoding/json" "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 @@ -41,7 +43,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..3b5c7b35d 100644 --- a/model/cluster_installation.go +++ b/model/cluster_installation.go @@ -9,6 +9,8 @@ import ( "fmt" "io" + log "github.com/sirupsen/logrus" + "github.com/pkg/errors" ) @@ -116,8 +118,15 @@ type MigrateClusterInstallationResponse struct { // Clone returns a deep copy the cluster installation. func (c *ClusterInstallation) Clone() *ClusterInstallation { var clone ClusterInstallation - data, _ := json.Marshal(c) - json.Unmarshal(data, &clone) + 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 + } 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.go b/model/group.go index bd0dc0fce..847196d38 100644 --- a/model/group.go +++ b/model/group.go @@ -7,6 +7,8 @@ package model import ( "encoding/json" "io" + + log "github.com/sirupsen/logrus" ) // Group represents a group of Mattermost installations. @@ -48,8 +50,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/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..56515e2c4 100644 --- a/model/id.go +++ b/model/id.go @@ -11,6 +11,8 @@ import ( "time" "unicode" + "github.com/sirupsen/logrus" + "github.com/pborman/uuid" ) @@ -22,9 +24,27 @@ var encoding = base32.NewEncoding("ybndrfg8ejkmcpqxot1uwisza345h769") func NewID() string { var b bytes.Buffer encoder := base32.NewEncoder(encoding, &b) - encoder.Write(uuid.NewRandom()) - encoder.Close() - b.Truncate(26) // removes the '==' padding + + // 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() // 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 + } + + // Truncate to remove the '==' padding. + b.Truncate(26) return b.String() } diff --git a/model/installation.go b/model/installation.go index dd28a589c..1ed3c7aa8 100644 --- a/model/installation.go +++ b/model/installation.go @@ -11,6 +11,8 @@ import ( "io" "strings" + log "github.com/sirupsen/logrus" + "github.com/pkg/errors" ) @@ -202,7 +204,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 } 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 {