Skip to content

Commit

Permalink
Merge pull request #1016 from cloudflare/tamas/fix-double-api-calls
Browse files Browse the repository at this point in the history 
filter/firewall-rules fix for double API calls
  • Loading branch information
@jacobbednarz
jacobbednarz authored Aug 10, 2022
2 parents fca99c6 + 4f1d602 commit d9c0391
Show file tree
Hide file tree
Showing 5 changed files with 269 additions and 92 deletions.
7 changes: 7 additions & 0 deletions .changelog/1016.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:enhancement
firewall_rule: fix double endpoint calls & moving towards common method signature
```

```release-note:enhancement
filter: fix double endpoint calls & moving towards common method signature
```
95 changes: 55 additions & 40 deletions filter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,26 @@ type FilterValidationExpressionMessage struct {
Message string `json:"message"`
}

// FilterCreateParams contains required and optional params
// for creating a filter.
type FilterCreateParams struct {
ID string `json:"id,omitempty"`
Expression string `json:"expression"`
Paused bool `json:"paused"`
Description string `json:"description"`
Ref string `json:"ref,omitempty"`
}

// FilterUpdateParams contains required and optional params
// for updating a filter.
type FilterUpdateParams struct {
ID string `json:"id"`
Expression string `json:"expression"`
Paused bool `json:"paused"`
Description string `json:"description"`
Ref string `json:"ref,omitempty"`
}

type FilterListParams struct {
ResultInfo
}
Expand All @@ -84,62 +104,57 @@ func (api *API) Filter(ctx context.Context, rc *ResourceContainer, filterID stri
return filterResponse.Result, nil
}

// Filters returns all filters for a zone.
// Filters returns filters for a zone.
//
// Automatically paginates all results unless `params.PerPage` and `params.Page`
// is set.
//
// API reference: https://developers.cloudflare.com/firewall/api/cf-filters/get/#get-all-filters
func (api *API) Filters(ctx context.Context, rc *ResourceContainer, params FilterListParams) ([]Filter, *ResultInfo, error) {
uri := buildURI(fmt.Sprintf("/zones/%s/filters", rc.Identifier), params)

res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil)
if err != nil {
return []Filter{}, &ResultInfo{}, err
}

var filtersResponse FiltersDetailResponse
err = json.Unmarshal(res, &filtersResponse)

if err != nil {
return []Filter{}, &ResultInfo{}, fmt.Errorf("%s: %w", errUnmarshalError, err)
autoPaginate := true
if params.PerPage >= 1 || params.Page >= 1 {
autoPaginate = false
}

if params.PerPage < 1 && params.Page < 1 {
var filters []Filter
if params.PerPage < 1 {
params.PerPage = 50
}
if params.Page < 1 {
params.Page = 1
}

for !params.ResultInfo.Done() {
uri := buildURI(fmt.Sprintf("/zones/%s/filters", rc.Identifier), params)
var filters []Filter
var fResponse FiltersDetailResponse
for {
uri := buildURI(fmt.Sprintf("/zones/%s/filters", rc.Identifier), params)

res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil)
if err != nil {
return []Filter{}, &ResultInfo{}, err
}
res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil)
if err != nil {
return []Filter{}, &ResultInfo{}, err
}

err = json.Unmarshal(res, &fResponse)
if err != nil {
return []Filter{}, &ResultInfo{}, fmt.Errorf("failed to unmarshal filters JSON data: %w", err)
}

var fResponse FiltersDetailResponse
err = json.Unmarshal(res, &fResponse)
if err != nil {
return []Filter{}, &ResultInfo{}, fmt.Errorf("failed to unmarshal filters JSON data: %w", err)
}
filters = append(filters, fResponse.Result...)
params.ResultInfo = fResponse.ResultInfo.Next()

filters = append(filters, fResponse.Result...)
params.ResultInfo = fResponse.ResultInfo.Next()
if params.ResultInfo.Done() || !autoPaginate {
break
}
filtersResponse.Result = filters
}

return filtersResponse.Result, &filtersResponse.ResultInfo, nil
return filters, &fResponse.ResultInfo, nil
}

// CreateFilters creates new filters.
//
// API reference: https://developers.cloudflare.com/firewall/api/cf-filters/post/
func (api *API) CreateFilters(ctx context.Context, rc *ResourceContainer, filters []Filter) ([]Filter, error) {
func (api *API) CreateFilters(ctx context.Context, rc *ResourceContainer, params []FilterCreateParams) ([]Filter, error) {
uri := fmt.Sprintf("/zones/%s/filters", rc.Identifier)

res, err := api.makeRequestContext(ctx, http.MethodPost, uri, filters)
res, err := api.makeRequestContext(ctx, http.MethodPost, uri, params)
if err != nil {
return []Filter{}, err
}
Expand All @@ -156,14 +171,14 @@ func (api *API) CreateFilters(ctx context.Context, rc *ResourceContainer, filter
// UpdateFilter updates a single filter.
//
// API reference: https://developers.cloudflare.com/firewall/api/cf-filters/put/#update-a-single-filter
func (api *API) UpdateFilter(ctx context.Context, rc *ResourceContainer, filter Filter) (Filter, error) {
if filter.ID == "" {
func (api *API) UpdateFilter(ctx context.Context, rc *ResourceContainer, params FilterUpdateParams) (Filter, error) {
if params.ID == "" {
return Filter{}, fmt.Errorf("filter ID cannot be empty")
}

uri := fmt.Sprintf("/zones/%s/filters/%s", rc.Identifier, filter.ID)
uri := fmt.Sprintf("/zones/%s/filters/%s", rc.Identifier, params.ID)

res, err := api.makeRequestContext(ctx, http.MethodPut, uri, filter)
res, err := api.makeRequestContext(ctx, http.MethodPut, uri, params)
if err != nil {
return Filter{}, err
}
Expand All @@ -180,16 +195,16 @@ func (api *API) UpdateFilter(ctx context.Context, rc *ResourceContainer, filter
// UpdateFilters updates many filters at once.
//
// API reference: https://developers.cloudflare.com/firewall/api/cf-filters/put/#update-multiple-filters
func (api *API) UpdateFilters(ctx context.Context, rc *ResourceContainer, filters []Filter) ([]Filter, error) {
for _, filter := range filters {
func (api *API) UpdateFilters(ctx context.Context, rc *ResourceContainer, params []FilterUpdateParams) ([]Filter, error) {
for _, filter := range params {
if filter.ID == "" {
return []Filter{}, fmt.Errorf("filter ID cannot be empty")
}
}

uri := fmt.Sprintf("/zones/%s/filters", rc.Identifier)

res, err := api.makeRequestContext(ctx, http.MethodPut, uri, filters)
res, err := api.makeRequestContext(ctx, http.MethodPut, uri, params)
if err != nil {
return []Filter{}, err
}
Expand Down
54 changes: 50 additions & 4 deletions filter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,15 @@ func TestCreateSingleFilter(t *testing.T) {
}

mux.HandleFunc("/zones/d56084adb405e0b7e32c52321bf07be6/filters", handler)
params := []FilterCreateParams{
{
ID: "b7ff25282d394be7b945e23c7106ce8a",
Paused: false,
Description: "Login from office",
Expression: "ip.src eq 127.0.0.1",
},
}

want := []Filter{
{
ID: "b7ff25282d394be7b945e23c7106ce8a",
Expand All @@ -166,7 +175,7 @@ func TestCreateSingleFilter(t *testing.T) {
},
}

actual, err := client.CreateFilters(context.Background(), ZoneIdentifier("d56084adb405e0b7e32c52321bf07be6"), want)
actual, err := client.CreateFilters(context.Background(), ZoneIdentifier("d56084adb405e0b7e32c52321bf07be6"), params)

if assert.NoError(t, err) {
assert.Equal(t, want, actual)
Expand Down Expand Up @@ -203,6 +212,21 @@ func TestCreateMultipleFilters(t *testing.T) {
}

mux.HandleFunc("/zones/d56084adb405e0b7e32c52321bf07be6/filters", handler)
params := []FilterCreateParams{
{
ID: "b7ff25282d394be7b945e23c7106ce8a",
Paused: false,
Description: "Login from office",
Expression: "ip.src eq 127.0.0.1",
},
{
ID: "b7ff25282d394be7b945e23c7106ce8a",
Paused: false,
Description: "Login from second office",
Expression: "ip.src eq 10.0.0.1",
},
}

want := []Filter{
{
ID: "b7ff25282d394be7b945e23c7106ce8a",
Expand All @@ -218,7 +242,7 @@ func TestCreateMultipleFilters(t *testing.T) {
},
}

actual, err := client.CreateFilters(context.Background(), ZoneIdentifier("d56084adb405e0b7e32c52321bf07be6"), want)
actual, err := client.CreateFilters(context.Background(), ZoneIdentifier("d56084adb405e0b7e32c52321bf07be6"), params)

if assert.NoError(t, err) {
assert.Equal(t, want, actual)
Expand Down Expand Up @@ -247,14 +271,21 @@ func TestUpdateSingleFilter(t *testing.T) {
}

mux.HandleFunc("/zones/d56084adb405e0b7e32c52321bf07be6/filters/60ee852f9cbb4802978d15600c7f3110", handler)
params := FilterUpdateParams{
ID: "60ee852f9cbb4802978d15600c7f3110",
Paused: false,
Description: "IP of example.org",
Expression: "ip.src eq 93.184.216.0",
}

want := Filter{
ID: "60ee852f9cbb4802978d15600c7f3110",
Paused: false,
Description: "IP of example.org",
Expression: "ip.src eq 93.184.216.0",
}

actual, err := client.UpdateFilter(context.Background(), ZoneIdentifier("d56084adb405e0b7e32c52321bf07be6"), want)
actual, err := client.UpdateFilter(context.Background(), ZoneIdentifier("d56084adb405e0b7e32c52321bf07be6"), params)

if assert.NoError(t, err) {
assert.Equal(t, want, actual)
Expand Down Expand Up @@ -291,6 +322,21 @@ func TestUpdateMultipleFilters(t *testing.T) {
}

mux.HandleFunc("/zones/d56084adb405e0b7e32c52321bf07be6/filters", handler)
params := []FilterUpdateParams{
{
ID: "60ee852f9cbb4802978d15600c7f3110",
Paused: false,
Description: "IP of example.org",
Expression: "ip.src eq 93.184.216.0",
},
{
ID: "c218c536b2bd406f958f278cf0fa8c0f",
Paused: false,
Description: "IP of example.com",
Expression: "ip.src ne 127.0.0.1",
},
}

want := []Filter{
{
ID: "60ee852f9cbb4802978d15600c7f3110",
Expand All @@ -306,7 +352,7 @@ func TestUpdateMultipleFilters(t *testing.T) {
},
}

actual, err := client.UpdateFilters(context.Background(), ZoneIdentifier("d56084adb405e0b7e32c52321bf07be6"), want)
actual, err := client.UpdateFilters(context.Background(), ZoneIdentifier("d56084adb405e0b7e32c52321bf07be6"), params)

if assert.NoError(t, err) {
assert.Equal(t, want, actual)
Expand Down
Loading

0 comments on commit d9c0391

Please sign in to comment.