Skip to content

Commit

Permalink
logpush: Handle invalid destination configuration (cloudflare#500)
Browse files Browse the repository at this point in the history
In the event that the destination configuration of a logpush job
challenge returns an erorr, we don't currently catch that. Instead, we
return the ownership challenge struct successfully which is slightly
misleading.

This introduces more error checking on the API itself to inspect the
response payload for the `valid` parameter being set to true which
handles the scenarios where ownership validation fails.

Fixes cloudflare#499
  • Loading branch information
jacobbednarz authored Jul 8, 2020
1 parent 246f2f7 commit 1cc2e45
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 0 deletions.
5 changes: 5 additions & 0 deletions logpush.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,11 @@ func (api *API) GetLogpushOwnershipChallenge(zoneID, destinationConf string) (*L
if err != nil {
return nil, errors.Wrap(err, errUnmarshalError)
}

if !r.Result.Valid {
return nil, errors.New(r.Result.Message)
}

return &r.Result, nil
}

Expand Down
33 changes: 33 additions & 0 deletions logpush_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ const (
"valid": true,
"message": ""
}
`
serverLogpushGetOwnershipChallengeInvalidResponseDescription = `{
"filename": "logs/challenge-filename.txt",
"valid": false,
"message": "destination is invalid"
}
`
)

Expand All @@ -51,6 +57,11 @@ var (
Valid: true,
Message: "",
}
expectedLogpushGetOwnershipChallengeInvalidResponseStruct = LogpushGetOwnershipChallenge{
Filename: "logs/challenge-filename.txt",
Valid: false,
Message: "destination is invalid",
}
expectedUpdatedLogpushJobStruct = LogpushJob{
ID: jobID,
Dataset: "http_requests",
Expand Down Expand Up @@ -229,6 +240,28 @@ func TestGetLogpushOwnershipChallenge(t *testing.T) {
}
}

func TestGetLogpushOwnershipChallengeWithInvalidResponse(t *testing.T) {
setup()
defer teardown()

handler := func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, r.Method, "POST", "Expected method 'POST', got %s", r.Method)
w.Header().Set("content-type", "application/json")
fmt.Fprintf(w, `{
"result": %s,
"success": true,
"errors": null,
"messages": null
}
`, serverLogpushGetOwnershipChallengeInvalidResponseDescription)
}

mux.HandleFunc("/zones/"+testZoneID+"/logpush/ownership", handler)
_, err := client.GetLogpushOwnershipChallenge(testZoneID, "destination_conf")

assert.Error(t, err)
}

func TestValidateLogpushOwnershipChallenge(t *testing.T) {
testCases := map[string]struct {
isValid bool
Expand Down

0 comments on commit 1cc2e45

Please sign in to comment.