Skip to content

Commit

Permalink
Log formatted JSON error for UAA request errors
Browse files Browse the repository at this point in the history
- Change ambiguous unknown error nomenclature

[#167050837]

Signed-off-by: Joshua Casey <[email protected]>
  • Loading branch information
Birdrock authored and cf-uaa committed Aug 23, 2019
1 parent 7a9468e commit ac9c7db
Show file tree
Hide file tree
Showing 13 changed files with 107 additions and 42 deletions.
11 changes: 11 additions & 0 deletions cli/printer.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cli

import (
"bytes"
"encoding/json"
)

Expand All @@ -24,6 +25,16 @@ func (jp JsonPrinter) Print(obj interface{}) error {
return nil
}

func (jp JsonPrinter) PrintError(b []byte) error {
var out bytes.Buffer
err := json.Indent(&out, b, "", " ")
if err != nil {
return err
}
jp.Log.Error(out.String())
return nil
}

type TestPrinter struct {
CallData map[string]interface{}
}
Expand Down
52 changes: 36 additions & 16 deletions cli/printer_test.go
Original file line number Diff line number Diff line change
@@ -1,33 +1,53 @@
package cli_test

import (
"bytes"
. "code.cloudfoundry.org/uaa-cli/cli"

"encoding/json"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
. "github.com/onsi/gomega/gbytes"
"io/ioutil"
)

var _ = Describe("JsonPrinter", func() {
It("prints things to the Robots log", func() {
logBuf := NewBuffer()
printer := NewJsonPrinter(NewLogger(ioutil.Discard, logBuf, ioutil.Discard, ioutil.Discard))

printer.Print(struct {
Foo string
Bar string
}{"foo", "bar"})
var infoLogBuf, robotLogBuf, warnLogBuf, errorLogBuf *Buffer
var printer JsonPrinter

Expect(logBuf.Contents()).To(MatchJSON(`{"Foo":"foo","Bar":"bar"}`))
BeforeEach(func() {
infoLogBuf, robotLogBuf, warnLogBuf, errorLogBuf = NewBuffer(), NewBuffer(), NewBuffer(), NewBuffer()
printer = NewJsonPrinter(NewLogger(infoLogBuf, robotLogBuf, warnLogBuf, errorLogBuf))
})
Describe("Print", func() {
It("prints things to the Robots log", func() {
printer.Print(struct {
Foo string
Bar string
}{"foo", "bar"})

Expect(robotLogBuf.Contents()).To(MatchJSON(`{"Foo":"foo","Bar":"bar"}`))
})

It("returns error when cannot marhsal into json", func() {
unJsonifiableObj := make(chan bool)
err := printer.Print(unJsonifiableObj)

Expect(err).To(HaveOccurred())
})
})

It("returns error when cannot marhsal into json", func() {
printer := NewJsonPrinter(NewLogger(ioutil.Discard, ioutil.Discard, ioutil.Discard, ioutil.Discard))
Describe("PrintError", func () {
It("prints a json buffer to Error log", func() {
jsonData := struct {
Foo string
Bar string
}{"foo", "bar"}
jsonRaw, _ := json.Marshal(jsonData)
var out bytes.Buffer
_ = json.Indent(&out, jsonRaw, "", " ")

unJsonifiableObj := make(chan bool)
err := printer.Print(unJsonifiableObj)
printer.PrintError(jsonRaw)

Expect(err).To(HaveOccurred())
Expect(string(errorLogBuf.Contents())).To(ContainSubstring(out.String()))
})
})
})
4 changes: 2 additions & 2 deletions cmd/create_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ var _ = Describe("CreateClient", func() {
"--clone", "shiny",
"--client_secret", "secretsecret")

Expect(session.Err).To(Say("An unknown error occurred while calling"))
Expect(session.Err).To(Say("An error occurred while calling"))
Expect(session).Should(Exit(1))
})

Expand Down Expand Up @@ -313,7 +313,7 @@ var _ = Describe("CreateClient", func() {
"--authorities", "notifications.write,notifications.read",
)

Eventually(session.Err).Should(Say("An unknown error occurred while calling"))
Eventually(session.Err).Should(Say("An error occurred while calling"))
Eventually(session).Should(Exit(1))
})
})
Expand Down
2 changes: 1 addition & 1 deletion cmd/delete_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ var _ = Describe("DeleteClient", func() {

session := runCommand("delete-client", "clientid")

Expect(session.Err).To(Say("An unknown error occurred while calling " + server.URL() + "/oauth/clients/clientid"))
Expect(session.Err).To(Say("An error occurred while calling " + server.URL() + "/oauth/clients/clientid"))
Eventually(session).Should(Exit(1))
})
})
Expand Down
9 changes: 8 additions & 1 deletion cmd/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"code.cloudfoundry.org/uaa-cli/config"
"errors"
"fmt"
"github.com/cloudfoundry-community/go-uaa"
"github.com/spf13/cobra"
"os"
)
Expand Down Expand Up @@ -47,7 +48,13 @@ func NotifyValidationErrors(err error, cmd *cobra.Command, log cli.Logger) {

func NotifyErrorsWithRetry(err error, log cli.Logger) {
if err != nil {
log.Error(err.Error())
switch t := err.(type) {
case uaa.RequestError:
log.Error(err.Error())
cli.NewJsonPrinter(log).PrintError(t.ErrorResponse)
default:
log.Error(err.Error())
}
VerboseRetryMsg(GetSavedConfig())
os.Exit(1)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/get_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ var _ = Describe("GetClient", func() {

session := runCommand("get-client", "clientid")

Expect(session.Err).To(Say("An unknown error occurred while calling " + server.URL() + "/oauth/clients/clientid"))
Expect(session.Err).To(Say("An error occurred while calling " + server.URL() + "/oauth/clients/clientid"))
Eventually(session).Should(Exit(1))
})
})
Expand Down
2 changes: 1 addition & 1 deletion cmd/info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ var _ = Describe("Info", func() {
session := runCommand("info")

Eventually(session).Should(Exit(1))
Expect(session.Err).To(Say("An unknown error occurred while calling " + server.URL() + "/info"))
Expect(session.Err).To(Say("An error occurred while calling " + server.URL() + "/info"))
})

Context("with --verbose", func() {
Expand Down
2 changes: 1 addition & 1 deletion cmd/list_clients_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ var _ = Describe("ListClients", func() {
It("handles request errors", func() {
session := runCommand("list-clients")

Expect(session.Err).To(Say("An unknown error occurred while calling " + server.URL() + "/oauth/clients"))
Expect(session.Err).To(Say("An error occurred while calling " + server.URL() + "/oauth/clients"))
Eventually(session).Should(Exit(1))
})
})
Expand Down
53 changes: 36 additions & 17 deletions cmd/list_group_mappings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,48 @@ var _ = Describe("ListGroupMappings", func() {
err := config.WriteConfig(cfg)
Expect(err).NotTo(HaveOccurred())
})
Describe("By default", func() {

It("requests group mappings from the backend with default parameters", func() {
server.RouteToHandler("GET", "/Groups/External", CombineHandlers(
VerifyRequest("GET", "/Groups/External", "startIndex=1&count=100"),
RespondWith(http.StatusOK, ExternalGroupsApiResponse, contentTypeJson),
))
It("requests group mappings from the backend with default parameters", func() {
server.RouteToHandler("GET", "/Groups/External", CombineHandlers(
VerifyRequest("GET", "/Groups/External", "startIndex=1&count=100"),
RespondWith(http.StatusOK, ExternalGroupsApiResponse, contentTypeJson),
))

session := runCommand("list-group-mappings")
session := runCommand("list-group-mappings")

Eventually(session).Should(Exit(0))
Eventually(session).Should(Exit(0))

// We can't verify that the right JSON was output
// There seems to be a gap in the tooling.
// We can test a regex against a buffer
// We can test JSON against a string
// But we can't test JSON against a buffer
Eventually(session.Out).Should(gbytes.Say("organizations.acme"))
// We can't verify that the right JSON was output
// There seems to be a gap in the tooling.
// We can test a regex against a buffer
// We can test JSON against a string
// But we can't test JSON against a buffer
Eventually(session.Out).Should(gbytes.Say("organizations.acme"))
})

It("prints a useful description in the help menu", func() {
session := runCommand("list-group-mappings", "-h")

Eventually(session).Should(Exit(0))
Eventually(session.Out).Should(gbytes.Say("List all the mappings between uaa scopes and external groups"))
})
})
Describe("When the context has insufficient scope", func() {
It("returns an error", func() {
server.RouteToHandler("GET", "/Groups/External", CombineHandlers(
VerifyRequest("GET", "/Groups/External", "startIndex=1&count=100"),
RespondWith(http.StatusForbidden, ExternalGroupsApiResponseInsufficientScope, contentTypeJson),
))

It("prints a useful description in the help menu", func() {
session := runCommand("list-group-mappings", "-h")
session := runCommand("list-group-mappings")

Eventually(session).Should(Exit(0))
Eventually(session.Out).Should(gbytes.Say("List all the mappings between uaa scopes and external groups"))
Eventually(session).Should(Exit(1))
Eventually(session.Err).Should(gbytes.Say(`An error occurred while calling http://127.0.0.1:(\d+)/Groups/External\?count=100&startIndex=1`))
Eventually(session.Err).Should(gbytes.Say(`"error": "insufficient_scope"`))
Eventually(session.Err).Should(gbytes.Say(`"error_description": "Insufficient scope for this resource"`))
Eventually(session.Err).Should(gbytes.Say(`"scope": "uaa.admin scim.read zones.uaa.admin"`))
Eventually(session.Out).Should(gbytes.Say("Retry with --verbose for more information."))
})
})
})
2 changes: 1 addition & 1 deletion cmd/userinfo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ var _ = Describe("Userinfo", func() {
session := runCommand("userinfo")

Eventually(session).Should(Exit(1))
Expect(session.Err).To(Say("An unknown error occurred while calling " + server.URL() + "/userinfo"))
Expect(session.Err).To(Say("An error occurred while calling " + server.URL() + "/userinfo"))
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,9 @@ const ExternalGroupsApiResponse = `{
"urn:scim:schemas:core:1.0"
]
}`

const ExternalGroupsApiResponseInsufficientScope = `{
"error": "insufficient_scope",
"error_description": "Insufficient scope for this resource",
"scope": "uaa.admin scim.read zones.uaa.admin"
}`
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module code.cloudfoundry.org/uaa-cli
go 1.12

require (
github.com/cloudfoundry-community/go-uaa v0.2.4
github.com/cloudfoundry-community/go-uaa v0.2.5
github.com/fatih/color v1.7.0
github.com/inconshreveable/mousetrap v1.0.0 // indirect
github.com/mattn/go-runewidth v0.0.2 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ github.com/cloudfoundry-community/go-uaa v0.2.3 h1:fsUpl/IK/pARdZj2EmPTMyiYEQ4t8
github.com/cloudfoundry-community/go-uaa v0.2.3/go.mod h1:8xbs4AE7Onyn0879a757RewaLbRrUw41CPNxn8EdmRM=
github.com/cloudfoundry-community/go-uaa v0.2.4 h1:Lc0jP1DoDS7rxXLeMSuVQF9cKt93TzhFvp6W+Hfj1w0=
github.com/cloudfoundry-community/go-uaa v0.2.4/go.mod h1:8xbs4AE7Onyn0879a757RewaLbRrUw41CPNxn8EdmRM=
github.com/cloudfoundry-community/go-uaa v0.2.5 h1:wn/f4n4hDxPF4NtpjG6dT5yFyAxYp/fSBvLSPFyG8E0=
github.com/cloudfoundry-community/go-uaa v0.2.5/go.mod h1:8xbs4AE7Onyn0879a757RewaLbRrUw41CPNxn8EdmRM=
github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys=
github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I=
Expand Down

0 comments on commit ac9c7db

Please sign in to comment.