OPA: improve error messages returned to the end user

Signed-off-by: aliculPix4D <[email protected]>
Pix4D · Sep 25, 2024 · 9f2f114 · 9f2f114
1 parent b418082
commit 9f2f114
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 10 deletions.
diff --git a/atc/api/policychecker/policychecker_suite_test.go b/atc/api/policychecker/policychecker_suite_test.go
@@ -10,7 +10,7 @@ import (
 	"testing"
 )
 
-func TestAccessor(t *testing.T) {
+func TestPolicyChecker(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "API PolicyChecker Suite")
 }

diff --git a/atc/policy/opa/agent_suite_test.go b/atc/policy/opa/agent_suite_test.go
@@ -7,7 +7,7 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestVault(t *testing.T) {
+func TestOpaAgent(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Policy Agent Suite")
 }
diff --git a/atc/policy/opa/opa.go b/atc/policy/opa/opa.go
@@ -60,23 +60,23 @@ func (c opa) Check(input policy.PolicyCheckInput) (policy.PolicyCheckResult, err
 	client.Timeout = c.config.Timeout
 	resp, err := client.Do(req)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("opa server connection error: %s", err)
 	}
 	defer resp.Body.Close()
 
 	statusCode := resp.StatusCode
 	if statusCode != http.StatusOK {
-		return nil, fmt.Errorf("opa returned status: %d", statusCode)
+		return nil, fmt.Errorf("opa server returned status: %d", statusCode)
 	}
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		return nil, fmt.Errorf("opa returned no response: %s", err.Error())
+		return nil, fmt.Errorf("opa server returned no response: %s", err.Error())
 	}
 
 	result, err := ParseOpaResult(body, c.config)
 	if err != nil {
-		return nil, fmt.Errorf("parsing opa results: %w", err)
+		return nil, fmt.Errorf("error while parsing opa response: %w", err)
 	}
 
 	return result, nil

diff --git a/atc/policy/opa/opa_test.go b/atc/policy/opa/opa_test.go
@@ -177,7 +177,7 @@ var _ = Describe("OPA Policy Checker", func() {
 		It("should return error", func() {
 			result, err := agent.Check(policy.PolicyCheckInput{})
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(MatchRegexp("connection refused"))
+			Expect(err.Error()).To(MatchRegexp("opa server connection error: .* connection refused"))
 			Expect(result).To(BeNil())
 		})
 	})
@@ -190,7 +190,7 @@ var _ = Describe("OPA Policy Checker", func() {
 		It("should return error", func() {
 			result, err := agent.Check(policy.PolicyCheckInput{})
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(Equal("opa returned status: 404"))
+			Expect(err.Error()).To(Equal("opa server returned status: 404"))
 			Expect(result).To(BeNil())
 		})
 	})
@@ -205,7 +205,7 @@ var _ = Describe("OPA Policy Checker", func() {
 		It("should return error", func() {
 			result, err := agent.Check(policy.PolicyCheckInput{})
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(ContainSubstring("invalid character 'h' looking for beginning of value"))
+			Expect(err.Error()).To(ContainSubstring("JSON parsing: error: invalid character 'h' looking for beginning of value. Hint: response from opa server is not a valid JSON"))
 			Expect(result).To(BeNil())
 		})
 	})

diff --git a/atc/policy/opa/result.go b/atc/policy/opa/result.go
@@ -30,7 +30,7 @@ func ParseOpaResult(bytesResult []byte, opaConfig OpaConfig) (opaResult, error)
 	var results vars.StaticVariables
 	err := json.Unmarshal(bytesResult, &results)
 	if err != nil {
-		return opaResult{}, err
+		return opaResult{}, fmt.Errorf("JSON parsing: error: %s. Hint: response from opa server is not a valid JSON", err)
 	}
 
 	var allowed, shouldBlock, ok bool