17944: Vocabulary check errors return status 400 instead of 500.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima@curii.com>

diff --git a/lib/controller/handler_test.go b/lib/controller/handler_test.go
index c99faba730ce38afcb5c342b8e278ae647cbca61..728c760af2b30852f87fd5c920db7dc5a2e87801 100644 (file)
--- a/lib/controller/handler_test.go
+++ b/lib/controller/handler_test.go
@@ -138,6 +138,54 @@ func (s *HandlerSuite) TestVocabularyExport(c *check.C) {
        }
 }
 
+func (s *HandlerSuite) TestVocabularyFailedCheckStatus(c *check.C) {
+       voc := `{
+               "strict_tags": false,
+               "tags": {
+                       "IDTAGIMPORTANCE": {
+                               "strict": true,
+                               "labels": [{"label": "Importance"}],
+                               "values": {
+                                       "HIGH": {
+                                               "labels": [{"label": "High"}]
+                                       },
+                                       "LOW": {
+                                               "labels": [{"label": "Low"}]
+                                       }
+                               }
+                       }
+               }
+       }`
+       f, err := os.CreateTemp("", "test-vocabulary-*.json")
+       c.Assert(err, check.IsNil)
+       defer os.Remove(f.Name())
+       _, err = f.WriteString(voc)
+       c.Assert(err, check.IsNil)
+       f.Close()
+       s.cluster.API.VocabularyPath = f.Name()
+
+       req := httptest.NewRequest("POST", "/arvados/v1/collections",
+               strings.NewReader(`{
+                       "collection": {
+                               "properties": {
+                                       "IDTAGIMPORTANCE": "Critical"
+                               }
+                       }
+               }`))
+       req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+       req.Header.Set("Content-type", "application/json")
+
+       resp := httptest.NewRecorder()
+       s.handler.ServeHTTP(resp, req)
+       c.Log(resp.Body.String())
+       c.Assert(resp.Code, check.Equals, http.StatusBadRequest)
+       var jresp httpserver.ErrorResponse
+       err = json.Unmarshal(resp.Body.Bytes(), &jresp)
+       c.Check(err, check.IsNil)
+       c.Assert(len(jresp.Errors), check.Equals, 1)
+       c.Check(jresp.Errors[0], check.Matches, `.*tag value.*for key.*is not listed as valid.*`)
+}
+
 func (s *HandlerSuite) TestProxyDiscoveryDoc(c *check.C) {
        req := httptest.NewRequest("GET", "/discovery/v1/apis/arvados/v1/rest", nil)
        resp := httptest.NewRecorder()

diff --git a/lib/controller/localdb/conn.go b/lib/controller/localdb/conn.go
index 0fae35e7d36a0d2515e6dd97f70e3080aa0948a0..edbbcb09cd30a13b8676f940c947d3e38713da08 100644 (file)
--- a/lib/controller/localdb/conn.go
+++ b/lib/controller/localdb/conn.go
@@ -8,6 +8,7 @@ import (
        "context"
        "encoding/json"
        "fmt"
+       "net/http"
        "os"
        "strings"
 
@@ -15,6 +16,7 @@ import (
        "git.arvados.org/arvados.git/lib/controller/rpc"
        "git.arvados.org/arvados.git/sdk/go/arvados"
        "git.arvados.org/arvados.git/sdk/go/ctxlog"
+       "git.arvados.org/arvados.git/sdk/go/httpserver"
        "github.com/fsnotify/fsnotify"
        "github.com/sirupsen/logrus"
 )
@@ -60,7 +62,11 @@ func (conn *Conn) checkProperties(ctx context.Context, properties interface{}) e
        if err != nil {
                return err
        }
-       return voc.Check(props)
+       err = voc.Check(props)
+       if err != nil {
+               return httpErrorf(http.StatusBadRequest, voc.Check(props).Error())
+       }
+       return nil
 }
 
 func watchVocabulary(logger logrus.FieldLogger, vocPath string, fn func()) {
@@ -209,3 +215,7 @@ func (conn *Conn) GroupContents(ctx context.Context, options arvados.GroupConten
 
        return conn.railsProxy.GroupContents(ctx, options)
 }
+
+func httpErrorf(code int, format string, args ...interface{}) error {
+       return httpserver.ErrorWithStatus(fmt.Errorf(format, args...), code)
+}