Status API: add http_code to response (open-policy-agent#4328)

... so applications can perform more informed error handling, e.g. refresh credentials on 403.

Fixes open-policy-agent#4259.

Signed-off-by: Jakob Schmid <[email protected]>

docs/content/management-status.md

@@ -199,6 +199,7 @@ Status updates contain the following fields:
 | `bundles[_].metrics` | `object` | Metrics from the last update of the bundle. |
 | `bundles[_].code` | `string` | If present, indicates error(s) occurred activating this bundle. |
 | `bundles[_].message` | `string` | Human readable messages describing the error(s). |
+| `bundles[_].http_code` | `number` | If present, indicates an erroneous HTTP status code that OPA received downloading this bundle. |
 | `bundles[_].errors` | `array` | Collection of detailed parse or compile errors that occurred during activation of this bundle. |
 | `discovery.name` | `string` | Name of discovery bundle that the OPA instance is configured to download. |
 | `discovery.active_revision` | `string` | Opaque revision identifier of the last successful discovery activation. |

download/download.go

@@ -342,15 +342,19 @@ func (d *Downloader) download(ctx context.Context, m metrics.Metrics) (*download
 			etag:     etag,
 			longPoll: d.longPollingEnabled,
 		}, nil
-	case http.StatusNotFound:
-		return nil, fmt.Errorf("server replied with not found")
-	case http.StatusUnauthorized:
-		return nil, fmt.Errorf("server replied with not authorized")
 	default:
-		return nil, fmt.Errorf("server replied with HTTP %v", resp.StatusCode)
+		return nil, HTTPError{StatusCode: resp.StatusCode}
 	}
 }
 
 func isLongPollSupported(header http.Header) bool {
 	return header.Get("Content-Type") == "application/vnd.openpolicyagent.bundles"
 }
+
+type HTTPError struct {
+	StatusCode int
+}
+
+func (e HTTPError) Error() string {
+	return fmt.Sprintf("server replied with %s", http.StatusText(e.StatusCode))
+}

download/download_test.go

@@ -11,6 +11,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -20,11 +21,10 @@ import (
 	"testing"
 	"time"
 
-	"github.com/open-policy-agent/opa/metrics"
-	"github.com/open-policy-agent/opa/plugins"
-
 	"github.com/open-policy-agent/opa/bundle"
 	"github.com/open-policy-agent/opa/keys"
+	"github.com/open-policy-agent/opa/metrics"
+	"github.com/open-policy-agent/opa/plugins"
 	"github.com/open-policy-agent/opa/plugins/rest"
 )
 
@@ -413,6 +413,13 @@ func TestFailureUnexpected(t *testing.T) {
 	if err == nil {
 		t.Fatal("expected error")
 	}
+	var hErr HTTPError
+	if !errors.As(err, &hErr) {
+		t.Fatal("expected HTTPError")
+	}
+	if hErr.StatusCode != 500 {
+		t.Fatal("expected status code 500")
+	}
 }
 
 func TestEtagInResponse(t *testing.T) {

plugins/bundle/plugin_test.go

@@ -817,6 +817,45 @@ func TestPluginOneShotCompileError(t *testing.T) {
 
 }
 
+func TestPluginOneShotHTTPError(t *testing.T) {
+	ctx := context.Background()
+	manager := getTestManager()
+	plugin := New(&Config{}, manager)
+	bundleName := "test-bundle"
+	plugin.status[bundleName] = &Status{Name: bundleName}
+	plugin.downloaders[bundleName] = download.New(download.Config{}, plugin.manager.Client(""), bundleName)
+	ch := make(chan Status)
+	listenerName := "test"
+	plugin.Register(listenerName, func(status Status) {
+		ch <- status
+	})
+	go plugin.oneShot(ctx, bundleName, download.Update{Error: download.HTTPError{StatusCode: 403}})
+	s := <-ch
+	if s.HTTPCode != "403" {
+		t.Fatal("expected http_code to be 403 instead of ", s.HTTPCode)
+	}
+
+	module := "package foo\n\ncorge=1"
+	b := bundle.Bundle{
+		Manifest: bundle.Manifest{Revision: "quickbrownfaux"},
+		Data:     util.MustUnmarshalJSON([]byte(`{"foo": {"bar": 1, "baz": "qux"}}`)).(map[string]interface{}),
+		Modules: []bundle.ModuleFile{
+			{
+				Path:   "/foo/bar",
+				Parsed: ast.MustParseModule(module),
+				Raw:    []byte(module),
+			},
+		},
+	}
+
+	b.Manifest.Init()
+	go plugin.oneShot(ctx, bundleName, download.Update{Bundle: &b})
+	s = <-ch
+	if s.HTTPCode != "" {
+		t.Fatal("expected http_code to be empty instead of ", s.HTTPCode)
+	}
+}
+
 func TestPluginOneShotActivationRemovesOld(t *testing.T) {
 
 	ctx := context.Background()

plugins/bundle/status.go

@@ -5,11 +5,14 @@
 package bundle
 
 import (
+	"encoding/json"
+	"strconv"
 	"time"
 
 	"github.com/pkg/errors"
 
 	"github.com/open-policy-agent/opa/ast"
+	"github.com/open-policy-agent/opa/download"
 	"github.com/open-policy-agent/opa/metrics"
 	"github.com/open-policy-agent/opa/server/types"
 )
@@ -30,6 +33,7 @@ type Status struct {
 	Message                  string          `json:"message,omitempty"`
 	Errors                   []error         `json:"errors,omitempty"`
 	Metrics                  metrics.Metrics `json:"metrics,omitempty"`
+	HTTPCode                 json.Number     `json:"http_code,omitempty"`
 }
 
 // SetActivateSuccess updates the status object to reflect a successful
@@ -56,22 +60,29 @@ func (s *Status) SetError(err error) {
 
 	if err == nil {
 		s.Code = ""
+		s.HTTPCode = ""
 		s.Message = ""
 		s.Errors = nil
 		return
 	}
 
-	cause := errors.Cause(err)
-
-	if astErr, ok := cause.(ast.Errors); ok {
+	switch cause := errors.Cause(err).(type) {
+	case ast.Errors:
 		s.Code = errCode
+		s.HTTPCode = ""
 		s.Message = types.MsgCompileModuleError
-		s.Errors = make([]error, len(astErr))
-		for i := range astErr {
-			s.Errors[i] = astErr[i]
+		s.Errors = make([]error, len(cause))
+		for i := range cause {
+			s.Errors[i] = cause[i]
 		}
-	} else {
+	case download.HTTPError:
+		s.Code = errCode
+		s.HTTPCode = json.Number(strconv.Itoa(cause.StatusCode))
+		s.Message = err.Error()
+		s.Errors = nil
+	default:
 		s.Code = errCode
+		s.HTTPCode = ""
 		s.Message = err.Error()
 		s.Errors = nil
 	}

server/server_test.go

@@ -2713,7 +2713,8 @@ func TestStatusV1(t *testing.T) {
 	// Expect HTTP 200 and updated status after bundle update occurs
 	bs.BulkUpdateBundleStatus(map[string]*pluginBundle.Status{
 		"test": {
-			Name: "test",
+			Name:     "test",
+			HTTPCode: "403",
 		},
 	})
 
@@ -2729,17 +2730,22 @@ func TestStatusV1(t *testing.T) {
 		Result struct {
 			Bundles struct {
 				Test struct {
-					Name string
+					Name     string
+					HTTPCode json.Number `json:"http_code"`
 				}
 			}
 		}
 	}
 
 	if err := util.NewJSONDecoder(f.recorder.Body).Decode(&resp2); err != nil {
 		t.Fatal(err)
-	} else if resp2.Result.Bundles.Test.Name != "test" {
+	}
+	if resp2.Result.Bundles.Test.Name != "test" {
 		t.Fatal("expected bundle to exist in status response but got:", resp2)
 	}
+	if resp2.Result.Bundles.Test.HTTPCode != "403" {
+		t.Fatal("expected HTTPCode to equal 403 but got:", resp2)
+	}
 }
 
 func TestQueryPostBasic(t *testing.T) {