Merge branch 'main' into PMM-4879-defaults-file

.github/check-license.go

@@ -21,53 +21,61 @@
 package main
 
 import (
-	"bufio"
 	"flag"
 	"fmt"
 	"io"
 	"log"
 	"os"
 	"path/filepath"
 	"regexp"
-	"runtime"
 )
 
-func getHeader() string {
-	_, file, _, ok := runtime.Caller(0)
-	if !ok {
-		panic("runtime.Caller(0) failed")
-	}
-	f, err := os.Open(file)
-	if err != nil {
-		log.Fatal(err)
-	}
-	defer f.Close()
+var (
+	generatedHeader = regexp.MustCompile(`^// Code generated .* DO NOT EDIT\.`)
 
-	var header string
-	s := bufio.NewScanner(f)
-	for s.Scan() {
-		if s.Text() == "" {
-			break
-		}
-		header += s.Text() + "\n"
-	}
-	header += "\n"
-	if err := s.Err(); err != nil {
-		log.Fatal(err)
-	}
-	return header
-}
+	sampleCopyright = `// pmm-managed
+// Copyright (C) 2022 Percona LLC
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+`
 
-var generatedHeader = regexp.MustCompile(`^// Code generated .* DO NOT EDIT\.`)
+	copyrightPattern = regexp.MustCompile(`^// pmm-managed
+// Copyright \(C\) 20\d{2} Percona LLC
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// \(at your option\) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+`)
+)
 
-func checkHeader(path string, header string) bool {
+func checkHeader(path string) bool {
 	f, err := os.Open(path)
 	if err != nil {
 		log.Fatal(err)
 	}
 	defer f.Close()
 
-	actual := make([]byte, len(header))
+	actual := make([]byte, len(sampleCopyright))
 	_, err = io.ReadFull(f, actual)
 	if err == io.ErrUnexpectedEOF {
 		err = nil // some files are shorter than license header
@@ -81,7 +89,7 @@ func checkHeader(path string, header string) bool {
 		return true
 	}
 
-	if header != string(actual) {
+	if !copyrightPattern.Match(actual) {
 		log.Print(path)
 		return false
 	}
@@ -96,8 +104,6 @@ func main() {
 	}
 	flag.Parse()
 
-	header := getHeader()
-
 	ok := true
 	filepath.Walk(".", func(path string, info os.FileInfo, err error) error {
 		if err != nil {
@@ -113,7 +119,7 @@ func main() {
 		}
 
 		if filepath.Ext(info.Name()) == ".go" {
-			if !checkHeader(path, header) {
+			if !checkHeader(path) {
 				ok = false
 			}
 		}

.github/workflows/ci.yml

@@ -188,21 +188,19 @@ jobs:
           go run .github/check-license.go
           bin/go-sumtype ./...
           # use GITHUB_TOKEN because only it has access to GitHub Checks API
-          bin/golangci-lint run -c=.golangci-required.yml --out-format=line-number | bin/reviewdog -f=golangci-lint -name='Required checks' -reporter=github-pr-check
+          bin/golangci-lint run -c=.golangci-required.yml --out-format=line-number | bin/reviewdog -f=golangci-lint -reporter=github-pr-review -fail-on-error=true
 
-          # run it like that until some of those issues/PRs are resolved:
-          # * https://github.com/quasilyte/go-consistent/issues/33
+          # run it like this until it will be added to golangci-lint:
           # * https://github.com/golangci/golangci-lint/issues/288
-          # * https://github.com/reviewdog/errorformat/pull/47 (once it is atually used by reviewdog)
-          bin/go-consistent -pedantic -exclude "tests" ./... | bin/reviewdog -f=go-consistent -name='Required go-consistent checks' -reporter=github-pr-check
+          bin/go-consistent -pedantic -exclude "tests" ./... | bin/reviewdog -f=go-consistent -reporter=github-pr-review -fail-on-error=true
 
       - name: Run optional checks/linters
         run: |
           # use ROBOT_TOKEN for better reviewer's name
           if [[ ! -z "${{ secrets.ROBOT_TOKEN }}" ]]; then
-            bin/golangci-lint run --out-format=line-number | env REVIEWDOG_GITHUB_API_TOKEN=${{ secrets.ROBOT_TOKEN }} bin/reviewdog -f=golangci-lint -name='Linters' -reporter=github-pr-review
+            bin/golangci-lint run --out-format=line-number | env REVIEWDOG_GITHUB_API_TOKEN=${{ secrets.ROBOT_TOKEN }} bin/reviewdog -f=golangci-lint -reporter=github-pr-review
           else
-            bin/golangci-lint run --out-format=line-number | env REVIEWDOG_GITHUB_API_TOKEN=${{ secrets.GITHUB_TOKEN }} bin/reviewdog -f=golangci-lint -name='Linters' -reporter=github-pr-review
+            bin/golangci-lint run --out-format=line-number | env REVIEWDOG_GITHUB_API_TOKEN=${{ secrets.GITHUB_TOKEN }} bin/reviewdog -f=golangci-lint -reporter=github-pr-review
           fi
 
       - name: Check that there are no source code changes

main.go

@@ -225,7 +225,7 @@ func runGRPCServer(ctx context.Context, deps *gRPCServerDeps) {
 	dbaasv1beta1.RegisterLogsAPIServer(gRPCServer, managementdbaas.NewLogsService(deps.db, deps.dbaasClient))
 	dbaasv1beta1.RegisterComponentsServer(gRPCServer, managementdbaas.NewComponentsService(deps.db, deps.dbaasClient, deps.versionServiceClient))
 
-	platformService, err := platform.New(deps.db, deps.supervisord, deps.grafanaClient, deps.config.Services.Platform)
+	platformService, err := platform.New(deps.db, deps.supervisord, deps.checksService, deps.grafanaClient, deps.config.Services.Platform)
 	if err == nil {
 		platformpb.RegisterPlatformServer(gRPCServer, platformService)
 	} else {

services/alertmanager/alertmanager.go

@@ -20,7 +20,7 @@ package alertmanager
 import (
 	"context"
 	"crypto/sha256"
-	_ "embed" //nolint:gci
+	_ "embed" // for email templates
 	"encoding/hex"
 	"fmt"
 	"io/ioutil"

services/checks/checks.go

@@ -183,6 +183,7 @@ func (s *Service) Run(ctx context.Context) {
 	s.l.Info("Starting...")
 	defer s.l.Info("Done.")
 
+	s.CollectChecks(ctx)
 	settings, err := models.GetSettings(s.db)
 	if err != nil {
 		s.l.Errorf("Failed to get settings: %+v.", err)
@@ -349,8 +350,8 @@ func (s *Service) ToggleCheckAlert(ctx context.Context, alertID string, silence
 	return err
 }
 
-// runChecksGroup downloads and executes STT checks in synchronous way.
-// If intervalGroup is empty.
+// runChecksGroup downloads and executes Advisors checks that should run in the interval specified by intervalGroup.
+// All checks are executed if intervalGroup is empty.
 func (s *Service) runChecksGroup(ctx context.Context, intervalGroup check.Interval) error {
 	settings, err := models.GetSettings(s.db)
 	if err != nil {
@@ -361,6 +362,7 @@ func (s *Service) runChecksGroup(ctx context.Context, intervalGroup check.Interv
 		return services.ErrSTTDisabled
 	}
 
+	s.CollectChecks(ctx)
 	return s.run(ctx, intervalGroup, nil)
 }
 
@@ -377,7 +379,9 @@ func (s *Service) StartChecks(checkNames []string) error {
 	}
 
 	go func() {
-		if err := s.run(context.Background(), "", checkNames); err != nil {
+		ctx := context.Background()
+		s.CollectChecks(ctx)
+		if err := s.run(ctx, "", checkNames); err != nil {
 			s.l.Errorf("Failed to execute STT checks: %+v.", err)
 		}
 	}()
@@ -390,8 +394,6 @@ func (s *Service) run(ctx context.Context, intervalGroup check.Interval, checkNa
 		return errors.WithStack(err)
 	}
 
-	s.CollectChecks(ctx)
-
 	if err := s.executeChecks(ctx, intervalGroup, checkNames); err != nil {
 		return errors.WithStack(err)
 	}

services/checks/checks_test.go

@@ -428,6 +428,10 @@ func TestStartChecks(t *testing.T) {
 		err = models.SaveSettings(db, settings)
 		require.NoError(t, err)
 
+		s.localChecksFile = testChecksFile
+		s.CollectChecks(context.Background())
+		assert.NotEmpty(t, s.checks)
+
 		err = s.runChecksGroup(context.Background(), "")
 		require.NoError(t, err)
 	})

services/platform/deps.go

@@ -0,0 +1,41 @@
+// pmm-managed
+// Copyright (C) 2022 Percona LLC
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+package platform
+
+import (
+	"context"
+
+	"github.com/percona/pmm-managed/models"
+)
+
+// supervisordService is a subset of methods of supervisord.Service used by this package.
+// We use it instead of real type for testing and to avoid dependency cycle.
+type supervisordService interface {
+	UpdateConfiguration(settings *models.Settings, ssoDetails *models.PerconaSSODetails) error
+}
+
+// type checksService is a subset of methods of checks.Service used by this package.
+// We use it instead of real type for testing and to avoid dependency cycle.
+type checksService interface {
+	CollectChecks(ctx context.Context)
+}
+
+// grafanaClient is a subset of methods of grafana.Client used by this package.
+// We use it instead of real type for testing and to avoid dependency cycle.
+type grafanaClient interface {
+	GetCurrentUserAccessToken(ctx context.Context) (string, error)
+}

services/platform/platform.go

@@ -47,30 +47,21 @@ var (
 	errGetSSODetailsFailed = status.Error(codes.Aborted, "Failed to fetch SSO details.")
 )
 
-// supervisordService is a subset of methods of supervisord.Service used by this package.
-// We use it instead of real type for testing and to avoid dependency cycle.
-type supervisordService interface {
-	UpdateConfiguration(settings *models.Settings, ssoDetails *models.PerconaSSODetails) error
-}
-
 // Service is responsible for interactions with Percona Platform.
 type Service struct {
 	db            *reform.DB
 	host          string
 	l             *logrus.Entry
-	supervisord   supervisordService
 	client        http.Client
 	grafanaClient grafanaClient
+	supervisord   supervisordService
+	checksService checksService
 
 	platformpb.UnimplementedPlatformServer
 }
 
-type grafanaClient interface {
-	GetCurrentUserAccessToken(ctx context.Context) (string, error)
-}
-
 // New returns platform Service.
-func New(db *reform.DB, supervisord supervisordService, grafanaClient grafanaClient, c Config) (*Service, error) {
+func New(db *reform.DB, supervisord supervisordService, checksService checksService, grafanaClient grafanaClient, c Config) (*Service, error) {
 	l := logrus.WithField("component", "platform")
 
 	host, err := envvars.GetSAASHost()
@@ -81,10 +72,11 @@ func New(db *reform.DB, supervisord supervisordService, grafanaClient grafanaCli
 	timeout := envvars.GetPlatformAPITimeout(l)
 
 	s := Service{
-		host:        host,
-		db:          db,
-		l:           l,
-		supervisord: supervisord,
+		host:          host,
+		db:            db,
+		l:             l,
+		supervisord:   supervisord,
+		checksService: checksService,
 		client: http.Client{
 			Timeout: timeout,
 			Transport: &http.Transport{
@@ -140,6 +132,10 @@ func (s *Service) Connect(ctx context.Context, req *platformpb.ConnectRequest) (
 		return nil, errInternalServer
 	}
 
+	if settings.SaaS.STTEnabled {
+		s.checksService.CollectChecks(ctx)
+	}
+
 	if err := s.UpdateSupervisordConfigurations(ctx); err != nil {
 		s.l.Errorf("Failed to update configuration of grafana after connecting PMM to Portal: %s", err)
 		return nil, errInternalServer
@@ -192,6 +188,10 @@ func (s *Service) Disconnect(ctx context.Context, req *platformpb.DisconnectRequ
 		return nil, err // this is already a status error
 	}
 
+	if settings.SaaS.STTEnabled {
+		s.checksService.CollectChecks(ctx)
+	}
+
 	if err = s.UpdateSupervisordConfigurations(ctx); err != nil {
 		s.l.Errorf("Failed to update configuration of grafana after disconnect from Platform: %s", err)
 		return nil, errInternalServer

services/victoriametrics/victoriametrics_test.go

@@ -26,11 +26,14 @@ import (
 	"github.com/AlekSi/pointer"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 	"gopkg.in/reform.v1"
 	"gopkg.in/reform.v1/dialects/postgresql"
 
 	"github.com/percona/pmm-managed/models"
 	"github.com/percona/pmm-managed/utils/testdb"
+	"github.com/percona/pmm-managed/utils/tests"
 )
 
 const configPath = "../../testdata/victoriametrics/promscrape.yml"
@@ -755,19 +758,20 @@ scrape_configs:
 		assert.Errorf(t, err, "error when checking Prometheus config")
 	})
 
-	t.Run("Good scrape config file with unsupported params", func(t *testing.T) {
+	t.Run("Scrape config file with unknown params", func(t *testing.T) {
 		err := svc.configAndReload(context.TODO(), []byte(strings.TrimSpace(`
 # Managed by pmm-managed. DO NOT EDIT.
 ---
 global:
   scrape_interval: 1m
   scrape_timeout: 54s
-remote_write:
-- url: http://some-remote-url
-remote_read:
-- url: http://some-remote-read-url
+unknown_filed: unknown_value
+
 `)))
-		assert.NoError(t, err)
+		tests.AssertGRPCError(t, status.New(codes.Aborted,
+			"yaml: unmarshal errors:\n  line 6: field unknown_filed not found in type promscrape.Config;"+
+				" pass -promscrape.config.strictParse=false command-line flag for ignoring unknown fields in yaml config\n",
+		), err)
 	})
 }