diff --git a/go.mod b/go.mod index 090bb7d725d..e90977b58b4 100644 --- a/go.mod +++ b/go.mod @@ -58,7 +58,7 @@ require ( github.com/google/uuid v1.3.0 github.com/google/wire v0.5.0 github.com/gorilla/websocket v1.5.0 - github.com/grafana/alerting v0.0.0-20230428095912-33c5aa68a5ba + github.com/grafana/alerting v0.0.0-20230505134339-793c67215ba1 github.com/grafana/cuetsy v0.1.9 github.com/grafana/grafana-aws-sdk v0.15.0 github.com/grafana/grafana-azure-sdk-go v1.7.0 diff --git a/go.sum b/go.sum index d585689210c..49c497c136a 100644 --- a/go.sum +++ b/go.sum @@ -1358,6 +1358,8 @@ github.com/gotestyourself/gotestyourself v1.3.0/go.mod h1:zZKM6oeNM8k+FRljX1mnzV github.com/gotestyourself/gotestyourself v2.2.0+incompatible/go.mod h1:zZKM6oeNM8k+FRljX1mnzVYeS8wiGgQyvST1/GafPbY= github.com/grafana/alerting v0.0.0-20230428095912-33c5aa68a5ba h1:aNTu22ojw4XY24DYNAuvw8v/5iFUNk2bkdTeeWQ5+0o= github.com/grafana/alerting v0.0.0-20230428095912-33c5aa68a5ba/go.mod h1:5edgy6tQY4+W2wuJdi8g2GjbVmpJufguy7QGIRl2q4o= +github.com/grafana/alerting v0.0.0-20230505134339-793c67215ba1 h1:ed1jueGwj1SWHboT4JpmNf5wWEHaGchfksRjIxFSox0= +github.com/grafana/alerting v0.0.0-20230505134339-793c67215ba1/go.mod h1:5edgy6tQY4+W2wuJdi8g2GjbVmpJufguy7QGIRl2q4o= github.com/grafana/codejen v0.0.3 h1:tAWxoTUuhgmEqxJPOLtJoxlPBbMULFwKFOcRsPRPXDw= github.com/grafana/codejen v0.0.3/go.mod h1:zmwwM/DRyQB7pfuBjTWII3CWtxcXh8LTwAYGfDfpR6s= github.com/grafana/cuetsy v0.1.9 h1:EwT8BqHoC0B3B4Y6Lg/D1aeYUKnQC1+2jjOHNsOuUfU= diff --git a/pkg/services/ngalert/notifier/alertmanager.go b/pkg/services/ngalert/notifier/alertmanager.go index cb9df23ee88..f1caefcab91 100644 --- a/pkg/services/ngalert/notifier/alertmanager.go +++ b/pkg/services/ngalert/notifier/alertmanager.go @@ -316,7 +316,7 @@ func (am *Alertmanager) buildReceiverIntegrations(receiver *alertingNotify.APIRe return nil, err } s := &sender{am.NotificationService} - img := newImageStore(am.Store) + img := newImageProvider(am.Store, log.New("ngalert.notifier.image-provider")) integrations, err := alertingNotify.BuildReceiverIntegrations( receiverCfg, tmpl, diff --git a/pkg/services/ngalert/notifier/email_test.go b/pkg/services/ngalert/notifier/email_test.go index 01da729b9bd..391fba8eac1 100644 --- a/pkg/services/ngalert/notifier/email_test.go +++ b/pkg/services/ngalert/notifier/email_test.go @@ -6,7 +6,7 @@ import ( "os" "testing" - "github.com/grafana/alerting/images" + alertingImages "github.com/grafana/alerting/images" alertingLogging "github.com/grafana/alerting/logging" "github.com/grafana/alerting/receivers" alertingEmail "github.com/grafana/alerting/receivers/email" @@ -200,7 +200,7 @@ func createSut(t *testing.T, messageTmpl string, subjectTmpl string, emailTmpl * }, Message: messageTmpl, Subject: subjectTmpl, - }, receivers.Metadata{}, emailTmpl, ns, &images.UnavailableImageStore{}, &alertingLogging.FakeLogger{}) + }, receivers.Metadata{}, emailTmpl, ns, &alertingImages.UnavailableProvider{}, &alertingLogging.FakeLogger{}) } func getSingleSentMessage(t *testing.T, ns *emailSender) *notifications.Message { diff --git a/pkg/services/ngalert/notifier/images.go b/pkg/services/ngalert/notifier/images.go index 10a29b00663..8f2662125e5 100644 --- a/pkg/services/ngalert/notifier/images.go +++ b/pkg/services/ngalert/notifier/images.go @@ -2,45 +2,156 @@ package notifier import ( "context" + "errors" + "io" + "os" + "path/filepath" "strings" - "github.com/grafana/alerting/images" + alertingImages "github.com/grafana/alerting/images" + alertingModels "github.com/grafana/alerting/models" + alertingNotify "github.com/grafana/alerting/notify" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/store" ) -type imageStore struct { - store store.ImageStore +type imageProvider struct { + store store.ImageStore + logger log.Logger } -func newImageStore(store store.ImageStore) images.ImageStore { - return &imageStore{ - store: store, +func newImageProvider(store store.ImageStore, logger log.Logger) alertingImages.Provider { + return &imageProvider{ + store: store, + logger: logger, } } -func (i imageStore) GetImage(ctx context.Context, uri string) (*images.Image, error) { - var ( - image *models.Image - err error - ) - - // Check whether the uri is a URL or a token to know how to query the DB. - if strings.HasPrefix(uri, "http") { - image, err = i.store.GetImageByURL(ctx, uri) - } else { - token := strings.TrimPrefix(uri, "token://") - image, err = i.store.GetImage(ctx, token) - } +func (i imageProvider) GetImage(ctx context.Context, uri string) (*alertingImages.Image, error) { + image, err := i.getImageFromURI(ctx, uri) if err != nil { + if errors.Is(err, models.ErrImageNotFound) { + i.logger.Info("Image not found in database") + return nil, alertingImages.ErrImageNotFound + } return nil, err } - return &images.Image{ + return &alertingImages.Image{ Token: image.Token, Path: image.Path, URL: image.URL, CreatedAt: image.CreatedAt, }, nil } + +func (i imageProvider) GetImageURL(ctx context.Context, alert *alertingNotify.Alert) (string, error) { + uri, err := getImageURI(alert) + if err != nil { + return "", err + } + + // If the identifier is a URL, validate that it corresponds to a stored, non-expired image. + if strings.HasPrefix(uri, "http") { + i.logger.Debug("Received an image URL in annotations", "alert", alert) + exists, err := i.store.URLExists(ctx, uri) + if err != nil { + return "", err + } + if !exists { + i.logger.Info("Image URL not found in database", "alert", alert) + return "", alertingImages.ErrImageNotFound + } + return uri, nil + } + + // If the identifier is a token, remove the prefix, get the image and return the URL. + token := strings.TrimPrefix(uri, "token://") + i.logger.Debug("Received an image token in annotations", "alert", alert, "token", token) + return i.getImageURLFromToken(ctx, token) +} + +// getImageURLFromToken takes a token and returns the URL of the image that token belongs to. +func (i imageProvider) getImageURLFromToken(ctx context.Context, token string) (string, error) { + image, err := i.store.GetImage(ctx, token) + if err != nil { + if errors.Is(err, models.ErrImageNotFound) { + i.logger.Info("Image not found in database", "token", token) + return "", alertingImages.ErrImageNotFound + } + return "", err + } + + if !image.HasURL() { + return "", alertingImages.ErrImagesNoURL + } + return image.URL, nil +} + +func (i imageProvider) GetRawImage(ctx context.Context, alert *alertingNotify.Alert) (io.ReadCloser, string, error) { + uri, err := getImageURI(alert) + if err != nil { + return nil, "", err + } + + image, err := i.getImageFromURI(ctx, uri) + if err != nil { + if errors.Is(err, models.ErrImageNotFound) { + i.logger.Info("Image not found in database", "alert", alert) + return nil, "", alertingImages.ErrImageNotFound + } + return nil, "", err + } + + if !image.HasPath() { + return nil, "", alertingImages.ErrImagesNoPath + } + + // Return image bytes and filename. + readCloser, err := openImage(image.Path) + if err != nil { + i.logger.Error("Error looking for image on disk", "alert", alert, "path", image.Path, "error", err) + return nil, "", err + } + filename := filepath.Base(image.Path) + return readCloser, filename, nil +} + +func (i imageProvider) getImageFromURI(ctx context.Context, uri string) (*models.Image, error) { + // Check whether the uri is a URL or a token to know how to query the DB. + if strings.HasPrefix(uri, "http") { + i.logger.Debug("Received an image URL in annotations") + return i.store.GetImageByURL(ctx, uri) + } + + token := strings.TrimPrefix(uri, "token://") + i.logger.Debug("Received an image token in annotations", "token", token) + return i.store.GetImage(ctx, token) +} + +// getImageURI is a helper function to retrieve the image URI from the alert annotations as a string. +func getImageURI(alert *alertingNotify.Alert) (string, error) { + uri, ok := alert.Annotations[alertingModels.ImageTokenAnnotation] + if !ok { + return "", alertingImages.ErrNoImageForAlert + } + return string(uri), nil +} + +// openImage returns an the io representation of an image from the given path. +func openImage(path string) (io.ReadCloser, error) { + fp := filepath.Clean(path) + _, err := os.Stat(fp) + if os.IsNotExist(err) || os.IsPermission(err) { + return nil, alertingImages.ErrImageNotFound + } + + f, err := os.Open(fp) + if err != nil { + return nil, err + } + + return f, nil +} diff --git a/pkg/services/ngalert/notifier/images_test.go b/pkg/services/ngalert/notifier/images_test.go index ba67ba52cb2..b16bd0e6360 100644 --- a/pkg/services/ngalert/notifier/images_test.go +++ b/pkg/services/ngalert/notifier/images_test.go @@ -2,16 +2,25 @@ package notifier import ( "context" + "io" + "os" + "path/filepath" "testing" + "time" + alertingImages "github.com/grafana/alerting/images" + alertingModels "github.com/grafana/alerting/models" + alertingNotify "github.com/grafana/alerting/notify" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/store" + "github.com/prometheus/common/model" "github.com/stretchr/testify/require" ) func TestGetImage(t *testing.T) { fakeImageStore := store.NewFakeImageStore(t) - store := newImageStore(fakeImageStore) + store := newImageProvider(fakeImageStore, log.NewNopLogger()) t.Run("queries by token when it gets a token", func(tt *testing.T) { img := models.Image{ @@ -22,6 +31,7 @@ func TestGetImage(t *testing.T) { err := fakeImageStore.SaveImage(context.Background(), &img) require.NoError(tt, err) + // nolint:staticcheck savedImg, err := store.GetImage(context.Background(), "token://"+img.Token) require.NoError(tt, err) require.Equal(tt, savedImg.Token, img.Token) @@ -38,6 +48,7 @@ func TestGetImage(t *testing.T) { err := fakeImageStore.SaveImage(context.Background(), &img) require.NoError(tt, err) + // nolint:staticcheck savedImg, err := store.GetImage(context.Background(), img.URL) require.NoError(tt, err) require.Equal(tt, savedImg.Token, img.Token) @@ -45,3 +56,170 @@ func TestGetImage(t *testing.T) { require.Equal(tt, savedImg.Path, img.Path) }) } + +func TestGetImageURL(t *testing.T) { + var ( + imageWithoutURL = models.Image{ + Token: "test-no-url", + CreatedAt: time.Now().UTC(), + ExpiresAt: time.Now().UTC().Add(24 * time.Hour), + } + testImage = models.Image{ + Token: "test", + URL: "https://test.com", + CreatedAt: time.Now().UTC(), + ExpiresAt: time.Now().UTC().Add(24 * time.Hour), + } + ) + + fakeImageStore := store.NewFakeImageStore(t, &imageWithoutURL, &testImage) + store := newImageProvider(fakeImageStore, log.NewNopLogger()) + + tests := []struct { + name string + uri string + expURL string + expErr error + }{ + { + "URL does not exist", + "https://invalid.com/test", + "", + alertingImages.ErrImageNotFound, + }, { + "existing URL", + testImage.URL, + testImage.URL, + nil, + }, { + "token does not exist", + "token://invalid", + "", + alertingImages.ErrImageNotFound, + }, { + "existing token", + "token://" + testImage.Token, + testImage.URL, + nil, + }, { + "image has no URL", + "token://" + imageWithoutURL.Token, + "", + alertingImages.ErrImagesNoURL, + }, + } + + for _, test := range tests { + t.Run(test.name, func(tt *testing.T) { + alert := alertingNotify.Alert{ + Alert: model.Alert{ + Annotations: model.LabelSet{alertingModels.ImageTokenAnnotation: model.LabelValue(test.uri)}, + }, + } + url, err := store.GetImageURL(context.Background(), &alert) + require.ErrorIs(tt, err, test.expErr) + require.Equal(tt, test.expURL, url) + }) + } +} + +func TestGetRawImage(t *testing.T) { + var ( + testBytes = []byte("some test bytes") + testPath = generateTestFile(t, testBytes) + imageWithoutPath = models.Image{ + Token: "test-no-path", + URL: "https://test-no-path.com", + CreatedAt: time.Now().UTC(), + ExpiresAt: time.Now().UTC().Add(24 * time.Hour), + } + testImage = models.Image{ + Token: "test", + URL: "https://test.com", + Path: testPath, + CreatedAt: time.Now().UTC(), + ExpiresAt: time.Now().UTC().Add(24 * time.Hour), + } + ) + + fakeImageStore := store.NewFakeImageStore(t, &imageWithoutPath, &testImage) + store := newImageProvider(fakeImageStore, log.NewNopLogger()) + + tests := []struct { + name string + uri string + expFilename string + expBytes []byte + expErr error + }{ + { + "URL does not exist", + "https://invalid.com/test", + "", + nil, + alertingImages.ErrImageNotFound, + }, { + "existing URL", + testImage.URL, + filepath.Base(testPath), + testBytes, + nil, + }, { + "token does not exist", + "token://invalid", + "", + nil, + alertingImages.ErrImageNotFound, + }, { + "existing token", + "token://" + testImage.Token, + filepath.Base(testPath), + testBytes, + nil, + }, { + "image has no path", + "token://" + imageWithoutPath.Token, + "", + nil, + alertingImages.ErrImagesNoPath, + }, + } + + for _, test := range tests { + t.Run(test.name, func(tt *testing.T) { + alert := alertingNotify.Alert{ + Alert: model.Alert{ + Annotations: model.LabelSet{alertingModels.ImageTokenAnnotation: model.LabelValue(test.uri)}, + }, + } + readCloser, filename, err := store.GetRawImage(context.Background(), &alert) + require.ErrorIs(tt, err, test.expErr) + require.Equal(tt, test.expFilename, filename) + + if test.expBytes != nil { + b, err := io.ReadAll(readCloser) + require.NoError(tt, err) + require.Equal(tt, test.expBytes, b) + require.NoError(t, readCloser.Close()) + } + }) + } +} + +func generateTestFile(t *testing.T, b []byte) string { + t.Helper() + f, err := os.CreateTemp("/tmp", "image") + require.NoError(t, err) + defer func(f *os.File) { + _ = f.Close() + }(f) + + t.Cleanup(func() { + require.NoError(t, os.RemoveAll(f.Name())) + }) + + _, err = f.Write(b) + require.NoError(t, err) + + return f.Name() +} diff --git a/pkg/services/ngalert/notifier/testing.go b/pkg/services/ngalert/notifier/testing.go index 31606030bf6..857bb5ec55c 100644 --- a/pkg/services/ngalert/notifier/testing.go +++ b/pkg/services/ngalert/notifier/testing.go @@ -10,6 +10,7 @@ import ( "testing" "time" + alertingImages "github.com/grafana/alerting/images" "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/store" @@ -24,19 +25,23 @@ type fakeConfigStore struct { // Saves the image or returns an error. func (f *fakeConfigStore) SaveImage(ctx context.Context, img *models.Image) error { - return models.ErrImageNotFound + return alertingImages.ErrImageNotFound } func (f *fakeConfigStore) GetImage(ctx context.Context, token string) (*models.Image, error) { - return nil, models.ErrImageNotFound + return nil, alertingImages.ErrImageNotFound } func (f *fakeConfigStore) GetImageByURL(ctx context.Context, url string) (*models.Image, error) { - return nil, models.ErrImageNotFound + return nil, alertingImages.ErrImageNotFound +} + +func (f *fakeConfigStore) URLExists(ctx context.Context, url string) (bool, error) { + return false, alertingImages.ErrImageNotFound } func (f *fakeConfigStore) GetImages(ctx context.Context, tokens []string) ([]models.Image, []string, error) { - return nil, nil, models.ErrImageNotFound + return nil, nil, alertingImages.ErrImageNotFound } func NewFakeConfigStore(t *testing.T, configs map[int64]*models.AlertConfiguration) *fakeConfigStore { diff --git a/pkg/services/ngalert/store/image.go b/pkg/services/ngalert/store/image.go index 095098138bb..3c35326d15b 100644 --- a/pkg/services/ngalert/store/image.go +++ b/pkg/services/ngalert/store/image.go @@ -31,6 +31,10 @@ type ImageStore interface { // SaveImage saves the image or returns an error. SaveImage(ctx context.Context, img *models.Image) error + + // URLExists takes a URL and returns a boolean indicating whether or not + // we have an image for that URL. + URLExists(ctx context.Context, url string) (bool, error) } type ImageAdminStore interface { @@ -75,6 +79,19 @@ func (st DBstore) GetImageByURL(ctx context.Context, url string) (*models.Image, return &image, nil } +func (st DBstore) URLExists(ctx context.Context, url string) (bool, error) { + var exists bool + err := st.SQLStore.WithDbSession(ctx, func(sess *db.Session) error { + ok, err := sess.Table("alert_image").Where("url = ? AND expires_at > ?", url, TimeNow().UTC()).Exist() + if err != nil { + return err + } + exists = ok + return nil + }) + return exists, err +} + func (st DBstore) GetImages(ctx context.Context, tokens []string) ([]models.Image, []string, error) { var images []models.Image if err := st.SQLStore.WithDbSession(ctx, func(sess *db.Session) error { diff --git a/pkg/services/ngalert/store/testing.go b/pkg/services/ngalert/store/testing.go index 6d28dd7005e..0fb622e146c 100644 --- a/pkg/services/ngalert/store/testing.go +++ b/pkg/services/ngalert/store/testing.go @@ -27,10 +27,15 @@ import ( "github.com/grafana/grafana/pkg/setting" ) -func NewFakeImageStore(t *testing.T) *FakeImageStore { +func NewFakeImageStore(t *testing.T, images ...*models.Image) *FakeImageStore { + imageMap := make(map[string]*models.Image) + for _, image := range images { + imageMap[image.Token] = image + } + return &FakeImageStore{ t: t, - images: make(map[string]*models.Image), + images: imageMap, } } @@ -61,6 +66,17 @@ func (s *FakeImageStore) GetImageByURL(_ context.Context, url string) (*models.I return nil, models.ErrImageNotFound } +func (s *FakeImageStore) URLExists(_ context.Context, url string) (bool, error) { + s.mtx.Lock() + defer s.mtx.Unlock() + for _, image := range s.images { + if image.URL == url { + return true, nil + } + } + return false, nil +} + func (s *FakeImageStore) GetImages(_ context.Context, tokens []string) ([]models.Image, []string, error) { s.mtx.Lock() defer s.mtx.Unlock() diff --git a/pkg/services/sqlstore/migrations/ualert/tables.go b/pkg/services/sqlstore/migrations/ualert/tables.go index 5f4001e74d2..a4287d93c0d 100644 --- a/pkg/services/sqlstore/migrations/ualert/tables.go +++ b/pkg/services/sqlstore/migrations/ualert/tables.go @@ -24,6 +24,11 @@ func AddTablesMigrations(mg *migrator.Migrator) { mg.AddMigration("add last_applied column to alert_configuration_history", migrator.NewAddColumnMigration(migrator.Table{Name: "alert_configuration_history"}, &migrator.Column{ Name: "last_applied", Type: migrator.DB_Int, Nullable: false, Default: "0", })) + + mg.AddMigration("add index in alert_image on url", migrator.NewAddIndexMigration(migrator.Table{Name: "alert_image"}, &migrator.Index{ + Cols: []string{"url"}, Type: migrator.IndexType, + })) + // End of migration log, add new migrations above this line. } // historicalTableMigrations contains those migrations that existed prior to creating the improved messaging around migration immutability.