Use image URL or bytes in the Discord integration (#82)

* describe methods to get an image URL/bytes

* UnavailableImageStore -> UnavailableProvider

* WIP: Use Image UR or bytes in Discord integration

* ErrImagesNoPath, return io.ReadCloser from Provider.GetRawImage(), remove unused OpenImage(), alias for upstream Alert type, remove duplicated import

* test images in discord

* check for empty annotation

* rename errors, comments in Provider interface

* typo in comment

* embed quota warning log, tests for embed quota

* avoid unnecessary nesting

* fewer changes in git diff

* add comment and deprecation warning for GetImage()

* add comment about ErrImagesNoURL and ErrImagesNoPath

* refactor logic for creating attachments

* close io.ReadCloser

images/images.go

@@ -17,7 +17,16 @@ var (
  // the maximum number of images has been iterated.
  ErrImagesDone = errors.New("images done")
 
+ // ErrImagesNoPath is returned whenever an image is found but has no path on disk.
+ ErrImagesNoPath = errors.New("no path for image")
+
+ // ErrImagesNoURL is returned whenever an image is found but has no URL.
+ ErrImagesNoURL = errors.New("no URL for image")
+
  ErrImagesUnavailable = errors.New("alert screenshots are unavailable")
+
+ // ErrNoImageForAlert is returned when no image is associated to a given alert.
+ ErrNoImageForAlert = errors.New("no image for alert")
 )
 
 type Image struct {
@@ -32,9 +41,24 @@ func (i Image) HasURL() bool {
 }
 
 type Provider interface {
+ // GetImage takes a token (identifier) and returns the image that token belongs to.
+ // Returns `ErrImageNotFound` if there's no image for said token.
+ //
+ // Deprecated: This method will be removed when all integrations use GetImageURL and/or GetRawImage,
+ // which allow integrations to get just the data they need for adding images to notifications.
+ // Use any of those two methods instead.
  GetImage(ctx context.Context, token string) (*Image, error)
- GetImageURL(ctx context.Context, alert types.Alert) (string, error)
- GetRawImage(ctx context.Context, alert types.Alert) (io.Reader, error)
+
+ // GetImageURL returns the URL of an image associated with a given alert.
+ // - Returns `ErrImageNotFound` if no image is found.
+ // - Returns `ErrImagesNoURL` if the image doesn't have a URL.
+ GetImageURL(ctx context.Context, alert *types.Alert) (string, error)
+
+ // GetRawImage returns an io.Reader to read the bytes of an image associated with a given alert
+ // and a string representing the filename.
+ // - Returns `ErrImageNotFound` if no image is found.
+ // - Returns `ErrImagesNoPath` if the image doesn't have a path on disk.
+ GetRawImage(ctx context.Context, alert *types.Alert) (io.ReadCloser, string, error)
 }
 
 type UnavailableProvider struct{}
@@ -44,12 +68,10 @@ func (u *UnavailableProvider) GetImage(context.Context, string) (*Image, error)
  return nil, ErrImagesUnavailable
 }
 
-// GetImageURL returns the URL of the image associated with a given alert.
-func (u *UnavailableProvider) GetImageURL(context.Context, types.Alert) (string, error) {
+func (u *UnavailableProvider) GetImageURL(context.Context, *types.Alert) (string, error) {
  return "", ErrImagesUnavailable
 }
 
-// GetRawImage returns an io.Reader to read the bytes of the image associated with a given alert.
-func (u *UnavailableProvider) GetRawImage(context.Context, types.Alert) (io.Reader, error) {
- return nil, ErrImagesUnavailable
+func (u *UnavailableProvider) GetRawImage(context.Context, *types.Alert) (io.ReadCloser, string, error) {
+ return nil, "", ErrImagesUnavailable
 }

images/testing.go

@@ -1,12 +1,13 @@
 package images
 
 import (
+ "bytes"
  "context"
  "encoding/base64"
  "fmt"
  "io"
  "os"
- "strings"
+ "path/filepath"
  "testing"
  "time"
 
@@ -16,6 +17,7 @@ import (
 
 type FakeProvider struct {
  Images []*Image
+ Bytes []byte
 }
 
 // GetImage returns an image with the same token.
@@ -29,41 +31,45 @@ func (f *FakeProvider) GetImage(_ context.Context, token string) (*Image, error)
 }
 
 // GetImageURL returns the URL of the image associated with a given alert.
-func (f *FakeProvider) GetImageURL(_ context.Context, alert types.Alert) (string, error) {
+func (f *FakeProvider) GetImageURL(_ context.Context, alert *types.Alert) (string, error) {
  uri, err := getImageURI(alert)
  if err != nil {
  return "", err
  }
 
  for _, img := range f.Images {
  if img.Token == uri || img.URL == uri {
+ if !img.HasURL() {
+ return "", ErrImagesNoURL
+ }
  return img.URL, nil
  }
  }
  return "", ErrImageNotFound
 }
 
 // GetRawImage returns an io.Reader to read the bytes of the image associated with a given alert.
-func (f *FakeProvider) GetRawImage(_ context.Context, alert types.Alert) (io.Reader, error) {
+func (f *FakeProvider) GetRawImage(_ context.Context, alert *types.Alert) (io.ReadCloser, string, error) {
  uri, err := getImageURI(alert)
  if err != nil {
- return nil, err
+ return nil, "", err
  }
 
  uriString := string(uri)
  for _, img := range f.Images {
  if img.Token == uriString || img.URL == uriString {
- return strings.NewReader("test"), nil
+ filename := filepath.Base(img.Path)
+ return io.NopCloser(bytes.NewReader(f.Bytes)), filename, nil
  }
  }
- return nil, ErrImageNotFound
+ return nil, "", ErrImageNotFound
 }
 
 // getImageURI is a helper function to retrieve the image URI from the alert annotations as a string.
-func getImageURI(alert types.Alert) (string, error) {
+func getImageURI(alert *types.Alert) (string, error) {
  uri, ok := alert.Annotations[models.ImageTokenAnnotation]
  if !ok {
- return "", fmt.Errorf("no image uri in annotations")
+ return "", ErrNoImageForAlert
  }
  return string(uri), nil
 }

images/utils.go

@@ -3,9 +3,6 @@ package images
 import (
  "context"
  "errors"
- "io"
- "os"
- "path/filepath"
  "time"
 
  "github.com/prometheus/alertmanager/types"
@@ -70,26 +67,6 @@ func WithStoredImages(ctx context.Context, l logging.Logger, imageProvider Provi
  return nil
 }
 
-// OpenImage returns an the io representation of an image from the given path.
-// The path argument here comes from reading internal image storage, not User
-// input, so we ignore the security check here.
-//
-//nolint:gosec
-func OpenImage(path string) (io.ReadCloser, error) {
- fp := filepath.Clean(path)
- _, err := os.Stat(fp)
- if os.IsNotExist(err) || os.IsPermission(err) {
- return nil, ErrImageNotFound
- }
-
- f, err := os.Open(fp)
- if err != nil {
- return nil, err
- }
-
- return f, nil
-}
-
 func getTokenFromAnnotations(annotations model.LabelSet) string {
  if value, ok := annotations[models.ImageTokenAnnotation]; ok {
  return string(value)

notify/alerts.go

@@ -30,6 +30,8 @@ type Receiver = amv2.Receiver
 type PostableAlerts = amv2.PostableAlerts
 type PostableAlert = amv2.PostableAlert
 
+type Alert = types.Alert
+
 var OpenAPIAlertsToAlerts = v2.OpenAPIAlertsToAlerts
 
 func (am *GrafanaAlertmanager) GetAlerts(active, silenced, inhibited bool, filter []string, receivers string) (GettableAlerts, error) {

notify/grafana_alertmanager_test.go

@@ -10,7 +10,6 @@ import (
 
  "github.com/go-kit/log"
  "github.com/go-openapi/strfmt"
- "github.com/prometheus/alertmanager/api/v2/models"
  amv2 "github.com/prometheus/alertmanager/api/v2/models"
  "github.com/prometheus/alertmanager/notify"
  "github.com/prometheus/alertmanager/provider/mem"
@@ -51,33 +50,33 @@ func TestPutAlert(t *testing.T) {
  title: "Valid alerts with different start/end set",
  postableAlerts: amv2.PostableAlerts{
  { // Start and end set.
- Annotations: models.LabelSet{"msg": "Alert1 annotation"},
- Alert: models.Alert{
- Labels: models.LabelSet{"alertname": "Alert1"},
+ Annotations: amv2.LabelSet{"msg": "Alert1 annotation"},
+ Alert: amv2.Alert{
+ Labels: amv2.LabelSet{"alertname": "Alert1"},
  GeneratorURL: "http://localhost/url1",
  },
  StartsAt: strfmt.DateTime(startTime),
  EndsAt: strfmt.DateTime(endTime),
  }, { // Only end is set.
- Annotations: models.LabelSet{"msg": "Alert2 annotation"},
- Alert: models.Alert{
- Labels: models.LabelSet{"alertname": "Alert2"},
+ Annotations: amv2.LabelSet{"msg": "Alert2 annotation"},
+ Alert: amv2.Alert{
+ Labels: amv2.LabelSet{"alertname": "Alert2"},
  GeneratorURL: "http://localhost/url2",
  },
  StartsAt: strfmt.DateTime{},
  EndsAt: strfmt.DateTime(endTime),
  }, { // Only start is set.
- Annotations: models.LabelSet{"msg": "Alert3 annotation"},
- Alert: models.Alert{
- Labels: models.LabelSet{"alertname": "Alert3"},
+ Annotations: amv2.LabelSet{"msg": "Alert3 annotation"},
+ Alert: amv2.Alert{
+ Labels: amv2.LabelSet{"alertname": "Alert3"},
  GeneratorURL: "http://localhost/url3",
  },
  StartsAt: strfmt.DateTime(startTime),
  EndsAt: strfmt.DateTime{},
  }, { // Both start and end are not set.
- Annotations: models.LabelSet{"msg": "Alert4 annotation"},
- Alert: models.Alert{
- Labels: models.LabelSet{"alertname": "Alert4"},
+ Annotations: amv2.LabelSet{"msg": "Alert4 annotation"},
+ Alert: amv2.Alert{
+ Labels: amv2.LabelSet{"alertname": "Alert4"},
  GeneratorURL: "http://localhost/url4",
  },
  StartsAt: strfmt.DateTime{},
@@ -131,9 +130,9 @@ func TestPutAlert(t *testing.T) {
  title: "Removing empty labels and annotations",
  postableAlerts: amv2.PostableAlerts{
  {
- Annotations: models.LabelSet{"msg": "Alert4 annotation", "empty": ""},
- Alert: models.Alert{
- Labels: models.LabelSet{"alertname": "Alert4", "emptylabel": ""},
+ Annotations: amv2.LabelSet{"msg": "Alert4 annotation", "empty": ""},
+ Alert: amv2.Alert{
+ Labels: amv2.LabelSet{"alertname": "Alert4", "emptylabel": ""},
  GeneratorURL: "http://localhost/url1",
  },
  StartsAt: strfmt.DateTime{},
@@ -159,9 +158,9 @@ func TestPutAlert(t *testing.T) {
  title: "Allow spaces in label and annotation name",
  postableAlerts: amv2.PostableAlerts{
  {
- Annotations: models.LabelSet{"Dashboard URL": "http://localhost:3000"},
- Alert: models.Alert{
- Labels: models.LabelSet{"alertname": "Alert4", "Spaced Label": "works"},
+ Annotations: amv2.LabelSet{"Dashboard URL": "http://localhost:3000"},
+ Alert: amv2.Alert{
+ Labels: amv2.LabelSet{"alertname": "Alert4", "Spaced Label": "works"},
  GeneratorURL: "http://localhost/url1",
  },
  StartsAt: strfmt.DateTime{},
@@ -187,8 +186,8 @@ func TestPutAlert(t *testing.T) {
  title: "Special characters in labels",
  postableAlerts: amv2.PostableAlerts{
  {
- Alert: models.Alert{
- Labels: models.LabelSet{"alertname$": "Alert1", "az3-- __...++!!!£@@312312": "1"},
+ Alert: amv2.Alert{
+ Labels: amv2.LabelSet{"alertname$": "Alert1", "az3-- __...++!!!£@@312312": "1"},
  },
  },
  },
@@ -211,9 +210,9 @@ func TestPutAlert(t *testing.T) {
  title: "Special characters in annotations",
  postableAlerts: amv2.PostableAlerts{
  {
- Annotations: models.LabelSet{"az3-- __...++!!!£@@312312": "Alert4 annotation"},
- Alert: models.Alert{
- Labels: models.LabelSet{"alertname": "Alert4"},
+ Annotations: amv2.LabelSet{"az3-- __...++!!!£@@312312": "Alert4 annotation"},
+ Alert: amv2.Alert{
+ Labels: amv2.LabelSet{"alertname": "Alert4"},
  },
  },
  },
@@ -236,16 +235,16 @@ func TestPutAlert(t *testing.T) {
  title: "No labels after removing empty",
  postableAlerts: amv2.PostableAlerts{
  {
- Alert: models.Alert{
- Labels: models.LabelSet{"alertname": ""},
+ Alert: amv2.Alert{
+ Labels: amv2.LabelSet{"alertname": ""},
  },
  },
  },
  expError: &AlertValidationError{
  Alerts: amv2.PostableAlerts{
  {
- Alert: models.Alert{
- Labels: models.LabelSet{"alertname": ""},
+ Alert: amv2.Alert{
+ Labels: amv2.LabelSet{"alertname": ""},
  },
  },
  },
@@ -255,8 +254,8 @@ func TestPutAlert(t *testing.T) {
  title: "Start should be before end",
  postableAlerts: amv2.PostableAlerts{
  {
- Alert: models.Alert{
- Labels: models.LabelSet{"alertname": ""},
+ Alert: amv2.Alert{
+ Labels: amv2.LabelSet{"alertname": ""},
  },
  StartsAt: strfmt.DateTime(endTime),
  EndsAt: strfmt.DateTime(startTime),
@@ -265,8 +264,8 @@ func TestPutAlert(t *testing.T) {
  expError: &AlertValidationError{
  Alerts: amv2.PostableAlerts{
  {
- Alert: models.Alert{
- Labels: models.LabelSet{"alertname": ""},
+ Alert: amv2.Alert{
+ Labels: amv2.LabelSet{"alertname": ""},
  },
  StartsAt: strfmt.DateTime(endTime),
  EndsAt: strfmt.DateTime(startTime),
@@ -353,10 +352,10 @@ func TestSilenceCleanup(t *testing.T) {
  dt := func(t time.Time) strfmt.DateTime { return strfmt.DateTime(t) }
 
  makeSilence := func(comment string, createdBy string,
- startsAt, endsAt strfmt.DateTime, matchers models.Matchers) *PostableSilence {
+ startsAt, endsAt strfmt.DateTime, matchers amv2.Matchers) *PostableSilence {
  return &PostableSilence{
  ID: "",
- Silence: models.Silence{
+ Silence: amv2.Silence{
  Comment: &comment,
  CreatedBy: &createdBy,
  StartsAt: &startsAt,
@@ -368,7 +367,7 @@ func TestSilenceCleanup(t *testing.T) {
 
  tru := true
  testString := "testName"
- matchers := models.Matchers{&models.Matcher{Name: &testString, IsEqual: &tru, IsRegex: &tru, Value: &testString}}
+ matchers := amv2.Matchers{&amv2.Matcher{Name: &testString, IsEqual: &tru, IsRegex: &tru, Value: &testString}}
  // Create silences - one in the future, one currently active, one expired but
  // retained, one expired and not retained.
  silences := []*PostableSilence{