Skip to content

Commit

Permalink
when http client recieve a http.StatusBadRequest response (e.g. due t…
Browse files Browse the repository at this point in the history
…o misconfigured OPA server) the response body is not a parsablble JSON, and hence instead of returning the JSON parser error, we need to return an actual response body containing the reason for the error. e.g. in case of OPA error the ATC server response body will never be a parsable JSON and instead its string (byte) starting with:

> policy check error: reason
This will always be unparsable by the JSON parser used in the HTTP client and instead of returning JSON parser error from the client itself (`invalid character 'p' looking for beginning of value`) we want to return the respons body error msg returned by the ATC server. This is especially important e.g. when a OPA server is misconfigured.

Instead of:
> error: invalid character 'p' looking for beginning of value
end user will see:

> error: Unexpected Response
> Status: 400 Bad Request
> Body: policy check error: reason

Signed-off-by: aliculPix4D <[email protected]>
  • Loading branch information
aliculPix4D committed Sep 26, 2024
1 parent 8a9b087 commit 65b1850
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
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

0 comments on commit 65b1850

Please sign in to comment.