Skip to content

Commit

Permalink
fix errors from client side
Browse files Browse the repository at this point in the history
  • Loading branch information
aliculPix4D committed Sep 26, 2024
1 parent 71b7bf7 commit 8255e89
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 12 deletions.
2 changes: 1 addition & 1 deletion atc/policy/opa/agent_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
. "github.com/onsi/gomega"
)

func TestOpaAgent(t *testing.T) {
func TestPolicyAgent(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Policy Agent Suite")
}
8 changes: 4 additions & 4 deletions atc/policy/opa/opa.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, fmt.Errorf("opa server connection error: %s", err)
return nil, fmt.Errorf("OPA server: connecting: %w", err)
}
defer resp.Body.Close()

statusCode := resp.StatusCode
if statusCode != http.StatusOK {
return nil, fmt.Errorf("opa server 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 server returned no response: %s", err.Error())
return nil, fmt.Errorf("OPA server: reading response body: %s", err)
}

result, err := ParseOpaResult(body, c.config)
if err != nil {
return nil, fmt.Errorf("parsing opa response: %w", err)
return nil, fmt.Errorf("parsing OPA results: %w", err)
}

return result, nil
Expand Down
6 changes: 3 additions & 3 deletions atc/policy/opa/opa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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("opa server connection error: .* connection refused"))
Expect(err.Error()).To(MatchRegexp("OPA server: connecting: .* connection refused"))
Expect(result).To(BeNil())
})
})
Expand All @@ -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 server returned status: 404"))
Expect(err.Error()).To(Equal("OPA server returned status: 404"))
Expect(result).To(BeNil())
})
})
Expand All @@ -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("JSON parsing: error: invalid character 'h' looking for beginning of value. Hint: response from opa server is not a valid JSON"))
Expect(err.Error()).To(ContainSubstring("parsing JSON: invalid character 'h' looking for beginning of value"))
Expect(result).To(BeNil())
})
})
Expand Down
2 changes: 1 addition & 1 deletion atc/policy/opa/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}, fmt.Errorf("JSON parsing: error: %s. Hint: response from opa server is not a valid JSON", err)
return opaResult{}, fmt.Errorf("parsing JSON: %w", err)
}

var allowed, shouldBlock, ok bool
Expand Down
6 changes: 5 additions & 1 deletion go-concourse/concourse/configs.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,11 @@ func (team *team) CreateOrUpdatePipelineConfig(pipelineRef atc.PipelineRef, conf
var validationErr atc.SaveConfigResponse
err = json.Unmarshal(body, &validationErr)
if err != nil {
return false, false, []ConfigWarning{}, err
return false, false, []ConfigWarning{}, internal.UnexpectedResponseError{
StatusCode: response.StatusCode,
Status: response.Status,
Body: string(body),
}
}
return false, false, []ConfigWarning{}, InvalidConfigError{Errors: validationErr.Errors}
case http.StatusForbidden:
Expand Down
1 change: 1 addition & 0 deletions go-concourse/concourse/configs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ var _ = Describe("ATC Handler Configs", func() {
It("returns an error", func() {
_, _, _, err := team.CreateOrUpdatePipelineConfig(pipelineRef, expectedVersion, expectedConfig, checkCredentials)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("bad-json"))
})
})
})
Expand Down
5 changes: 4 additions & 1 deletion go-concourse/concourse/internal/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,10 @@ func (connection *connection) populateResponse(response *http.Response, returnRe
}

if response.StatusCode == http.StatusForbidden {
return ErrForbidden
body, _ := io.ReadAll(response.Body)
return ForbiddenError{
Reason: string(body),
}
}

if response.StatusCode < 200 || response.StatusCode >= 300 {
Expand Down
30 changes: 29 additions & 1 deletion go-concourse/concourse/internal/connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ var _ = Describe("ATC Connection", func() {
})
})

Describe("403 response", func() {
Describe("HTTPAgent: 403 response", func() {
BeforeEach(func() {
atcServer = ghttp.NewServer()

Expand Down Expand Up @@ -381,6 +381,34 @@ var _ = Describe("ATC Connection", func() {
})
})

Describe("Connection: 403 response", func() {
BeforeEach(func() {
atcServer = ghttp.NewServer()

connection = NewConnection(atcServer.URL(), nil, tracing)

atcServer.AppendHandlers(
ghttp.CombineHandlers(
ghttp.VerifyRequest("DELETE", "/api/v1/teams/main/pipelines/foo"),
ghttp.RespondWith(http.StatusForbidden, "problem"),
),
)
})

It("returns back ForbiddenError", func() {
err := connection.Send(Request{
RequestName: atc.DeletePipeline,
Params: rata.Params{
"pipeline_name": "foo",
"team_name": atc.DefaultTeamName,
},
}, nil)
fe, ok := err.(ForbiddenError)
Expect(ok).To(BeTrue())
Expect(fe.Reason).To(Equal("problem"))
})
})

Describe("404 response", func() {
Context("when the response does not contain JSONAPI errors", func() {
BeforeEach(func() {
Expand Down

0 comments on commit 8255e89

Please sign in to comment.