From 20f4afacac146b9e46a669b6907c087e409a4494 Mon Sep 17 00:00:00 2001 From: Christian Ehrig Date: Sun, 26 Jul 2020 13:50:52 +0200 Subject: [PATCH 01/12] Adds implementation for Rules Lists --- ip_list.go | 358 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 358 insertions(+) create mode 100644 ip_list.go diff --git a/ip_list.go b/ip_list.go new file mode 100644 index 00000000000..b9355509eb2 --- /dev/null +++ b/ip_list.go @@ -0,0 +1,358 @@ +package cloudflare + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "time" + + "github.com/pkg/errors" +) + +const ( + IP_LIST_TYPE_IP = "ip" +) + +// BulkOperation contains information about a Bulk Operation +type BulkOperation struct { + ID string `json:"id"` + Status string `json:"status"` + Error string `json:"error"` + Completed *time.Time `json:"completed"` +} + +// IPList contains information about an IP List +type IPList struct { + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description"` + Kind string `json:"kind"` + NumItems int `json:"num_items"` + NumReferencingFilters int `json:"num_referencing_filters"` + CreatedOn *time.Time `json:"created_on"` + ModifiedOn *time.Time `json:"modified_on"` +} + +// IPListItem contains information about a single IP List Item +type IPListItem struct { + ID string `json:"id"` + IP string `json:"ip"` + Comment string `json:"comment"` + CreatedOn *time.Time `json:"created_on"` + ModifiedOn *time.Time `json:"modified_on"` +} + +// IPListCreateRequest contains data for a new IP List +type IPListCreateRequest struct { + Name string `json:"name"` + Description string `json:"description"` + Kind string `json:"kind"` +} + +// IPListItemCreateRequest contains data for a new IP List Item +type IPListItemCreateRequest struct { + IP string `json:"ip"` + Comment string `json:"comment"` +} + +// IPListItemDeleteRequest wraps IP List Items that shall be deleted +type IPListItemDeleteRequest struct { + Items []IPListItemDeleteItemRequest `json:"items"` +} + +// IPListItemDeleteItemRequest contains single IP List Items that shall be deleted +type IPListItemDeleteItemRequest struct { + ID string `json:"id"` +} + +// IPListUpdateRequest contains data for an IP List update +type IPListUpdateRequest struct { + Description string `json:"description"` +} + +// IPListResponse contains a single IP Lists +type IPListResponse struct { + Response + Result IPList `json:"result"` +} + +// IPListItemResponse contains information about the creation of an IP List Item +type IPListItemResponse struct { + Response + Result struct { + OperationID string `json:"operation_id"` + } `json:"result"` +} + +// IPListListResponse contains a slice of IP Lists +type IPListListResponse struct { + Response + Result []IPList `json:"result"` +} + +// BulkOperationResponse contains a slice of IP Lists +type BulkOperationResponse struct { + Response + Result BulkOperation `json:"result"` +} + +// IPListDeleteResponse contains information about the deletion of an IP List +type IPListDeleteResponse struct { + Response + Result struct { + ID string `json:"id"` + } `json:"result"` +} + +// IPListItemsListResponse contains information about IP List Items +type IPListItemsListResponse struct { + Response + ResultInfo `json:"result_info"` + Result []IPListItem `json:"result"` +} + +// IPListItemDeleteResponse contains information about the deletion of an IP List Item +type IPListItemDeleteResponse struct { + Response + Result struct { + OperationID string `json:"operation_id"` + } `json:"result"` +} + +// IPListItemsGetResponse contains information about a single IP List Item +type IPListItemsGetResponse struct { + Response + Result IPListItem `json:"result"` +} + +// ListIPLists lists all IP Lists +// API reference: https://api.cloudflare.com/#rules-lists-list-lists +func (api *API) ListIPLists(ctx context.Context) ([]IPList, error) { + uri := fmt.Sprintf("/accounts/%s/rules/lists", api.AccountID) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) + if err != nil { + return []IPList{}, errors.Wrap(err, errMakeRequestError) + } + + result := IPListListResponse{} + if err := json.Unmarshal(res, &result); err != nil { + return []IPList{}, errors.Wrap(err, errUnmarshalError) + } + + return result.Result, nil +} + +// CreateIPList creates a new IP List +// API reference: https://api.cloudflare.com/#rules-lists-create-list +func (api *API) CreateIPList(ctx context.Context, name string, description string, kind string) (IPListResponse, + error) { + uri := fmt.Sprintf("/accounts/%s/rules/lists", api.AccountID) + res, err := api.makeRequestContext(ctx, http.MethodPost, uri, + IPListCreateRequest{Name: name, Description: description, Kind: kind}) + if err != nil { + return IPListResponse{}, errors.Wrap(err, errMakeRequestError) + } + + result := IPListResponse{} + if err := json.Unmarshal(res, &result); err != nil { + return IPListResponse{}, errors.Wrap(err, errUnmarshalError) + } + + return result, nil +} + +// GetIPList returns a single IP List +// API reference: https://api.cloudflare.com/#rules-lists-get-list +func (api *API) GetIPList(ctx context.Context, id string) (IPList, error) { + uri := fmt.Sprintf("/accounts/%s/rules/lists/%s", api.AccountID, id) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) + if err != nil { + return IPList{}, errors.Wrap(err, errMakeRequestError) + } + + result := IPListResponse{} + if err := json.Unmarshal(res, &result); err != nil { + return IPList{}, errors.Wrap(err, errUnmarshalError) + } + + return result.Result, nil +} + +// UpdateIPList updates the description of an existing IP List +// API reference: https://api.cloudflare.com/#rules-lists-update-list +func (api *API) UpdateIPList(ctx context.Context, id string, description string) (IPListResponse, error) { + uri := fmt.Sprintf("/accounts/%s/rules/lists/%s", api.AccountID, id) + res, err := api.makeRequestContext(ctx, http.MethodPut, uri, IPListUpdateRequest{Description: description}) + if err != nil { + return IPListResponse{}, errors.Wrap(err, errMakeRequestError) + } + + result := IPListResponse{} + if err := json.Unmarshal(res, &result); err != nil { + return IPListResponse{}, errors.Wrap(err, errUnmarshalError) + } + + return result, nil +} + +// DeleteIPList deletes an IP List +// API reference: https://api.cloudflare.com/#rules-lists-delete-list +func (api *API) DeleteIPList(ctx context.Context, id string) (IPListDeleteResponse, error) { + uri := fmt.Sprintf("/accounts/%s/rules/lists/%s", api.AccountID, id) + res, err := api.makeRequestContext(ctx, http.MethodDelete, uri, nil) + if err != nil { + return IPListDeleteResponse{}, errors.Wrap(err, errMakeRequestError) + } + + result := IPListDeleteResponse{} + if err := json.Unmarshal(res, &result); err != nil { + return IPListDeleteResponse{}, errors.Wrap(err, errUnmarshalError) + } + + return result, nil +} + +// ListIPListItems returns a list with all items in an IP List +// API reference: https://api.cloudflare.com/#rules-lists-list-list-items +func (api *API) ListIPListItems(ctx context.Context, id string) ([]IPListItem, error) { + var list []IPListItem + var cursor string + var cursorQuery string + + for { + if len(cursor) > 0 { + cursorQuery = fmt.Sprintf("?cursor=%s", cursor) + } + uri := fmt.Sprintf("/accounts/%s/rules/lists/%s/items%s", api.AccountID, id, cursorQuery) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) + if err != nil { + return []IPListItem{}, errors.Wrap(err, errMakeRequestError) + } + + result := IPListItemsListResponse{} + if err := json.Unmarshal(res, &result); err != nil { + return []IPListItem{}, errors.Wrap(err, errUnmarshalError) + } + + list = append(list, result.Result...) + if cursor = result.ResultInfo.Cursors.After; cursor == "" { + break + } + } + + return list, nil +} + +// CreateIPListItem creates a new IP List Item +// TODO Responses won't contain an error code if we are trying to create more IPs than allowed +// TODO shall we keep the method asynchronously or poll the bulk operations endpoint? +// API reference: https://api.cloudflare.com/#rules-lists-create-list-items +func (api *API) CreateIPListItem(ctx context.Context, id string, ip string, comment string) (IPListItemResponse, + error) { + uri := fmt.Sprintf("/accounts/%s/rules/lists/%s/items", api.AccountID, id) + res, err := api.makeRequestContext(ctx, http.MethodPost, uri, []IPListItemCreateRequest{{IP: ip, Comment: comment}}) + if err != nil { + return IPListItemResponse{}, errors.Wrap(err, errMakeRequestError) + } + + result := IPListItemResponse{} + if err := json.Unmarshal(res, &result); err != nil { + return IPListItemResponse{}, errors.Wrap(err, errUnmarshalError) + } + + return result, nil +} + +// CreateIPListItems bulk creates many IP List Items +// TODO shall we keep the method asynchronously or poll the bulk operations endpoint? +// API reference: https://api.cloudflare.com/#rules-lists-create-list-items +func (api *API) CreateIPListItems(ctx context.Context, id string, items []IPListItemCreateRequest) (IPListItemResponse, + error) { + uri := fmt.Sprintf("/accounts/%s/rules/lists/%s/items", api.AccountID, id) + res, err := api.makeRequestContext(ctx, http.MethodPost, uri, items) + if err != nil { + return IPListItemResponse{}, errors.Wrap(err, errMakeRequestError) + } + + result := IPListItemResponse{} + if err := json.Unmarshal(res, &result); err != nil { + return IPListItemResponse{}, errors.Wrap(err, errUnmarshalError) + } + + return result, nil +} + +// ReplaceIPListItems replace all IP List items with a new set +// TODO shall we keep the method asynchronously or poll the bulk operations endpoint? +// API reference: https://api.cloudflare.com/#rules-lists-replace-list-items +func (api *API) ReplaceIPListItems(ctx context.Context, id string, items []IPListItemCreateRequest) (IPListItemResponse, + error) { + uri := fmt.Sprintf("/accounts/%s/rules/lists/%s/items", api.AccountID, id) + res, err := api.makeRequestContext(ctx, http.MethodPut, uri, items) + if err != nil { + return IPListItemResponse{}, errors.Wrap(err, errMakeRequestError) + } + + result := IPListItemResponse{} + if err := json.Unmarshal(res, &result); err != nil { + return IPListItemResponse{}, errors.Wrap(err, errUnmarshalError) + } + + return result, nil +} + +// DeleteIPListItems removes specific Items of an IP List by their ID +// TODO shall we keep the method asynchronously or poll the bulk operations endpoint? +// API reference: https://api.cloudflare.com/#rules-lists-delete-list-items +func (api *API) DeleteIPListItems(ctx context.Context, id string, items IPListItemDeleteRequest) (IPListItemDeleteResponse, + error) { + uri := fmt.Sprintf("/accounts/%s/rules/lists/%s/items", api.AccountID, id) + res, err := api.makeRequestContext(ctx, http.MethodDelete, uri, items) + test, _ := json.Marshal(items) + fmt.Printf("%+v\n", string(test)) + if err != nil { + return IPListItemDeleteResponse{}, errors.Wrap(err, errMakeRequestError) + } + + result := IPListItemDeleteResponse{} + if err := json.Unmarshal(res, &result); err != nil { + return IPListItemDeleteResponse{}, errors.Wrap(err, errUnmarshalError) + } + + return result, nil +} + +// GetIPListItem returns a single IP List Item +// API reference: https://api.cloudflare.com/#rules-lists-get-list-item +func (api *API) GetIPListItem(ctx context.Context, listID string, id string) (IPListItem, error) { + uri := fmt.Sprintf("/accounts/%s/rules/lists/%s/items/%s", api.AccountID, listID, id) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) + if err != nil { + return IPListItem{}, errors.Wrap(err, errMakeRequestError) + } + + result := IPListItemsGetResponse{} + if err := json.Unmarshal(res, &result); err != nil { + return IPListItem{}, errors.Wrap(err, errUnmarshalError) + } + + return result.Result, nil +} + +// GetBulkOperation returns the status of a bulk operation +// API reference: https://api.cloudflare.com/#rules-lists-get-list +func (api *API) GetBulkOperation(ctx context.Context, id string) (BulkOperation, error) { + uri := fmt.Sprintf("/accounts/%s/rules/lists/bulk_operations/%s", api.AccountID, id) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) + if err != nil { + return BulkOperation{}, errors.Wrap(err, errMakeRequestError) + } + + result := BulkOperationResponse{} + if err := json.Unmarshal(res, &result); err != nil { + return BulkOperation{}, errors.Wrap(err, errUnmarshalError) + } + + return result.Result, nil +} From efbe4d80dc72716a51b1377e83a9edd142df072f Mon Sep 17 00:00:00 2001 From: Christian Ehrig Date: Sun, 26 Jul 2020 14:14:37 +0200 Subject: [PATCH 02/12] Adds Cursors to ResultInfo --- cloudflare.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/cloudflare.go b/cloudflare.go index 73dba8fcdc5..bbd9745cba9 100644 --- a/cloudflare.go +++ b/cloudflare.go @@ -363,14 +363,21 @@ type Response struct { Messages []ResponseInfo `json:"messages"` } +// ResultInfoCursors contains information about cursors. +type ResultInfoCursors struct { + Before string `json:"before"` + After string `json:"after"` +} + // ResultInfo contains metadata about the Response. type ResultInfo struct { - Page int `json:"page"` - PerPage int `json:"per_page"` - TotalPages int `json:"total_pages"` - Count int `json:"count"` - Total int `json:"total_count"` - Cursor string `json:"cursor"` + Page int `json:"page"` + PerPage int `json:"per_page"` + TotalPages int `json:"total_pages"` + Count int `json:"count"` + Total int `json:"total_count"` + Cursor string `json:"cursor"` + Cursors ResultInfoCursors `json:"cursors"` } // RawResponse keeps the result as JSON form From b56960c283d9e6baf9dcdb6749a6faadb1fc5fea Mon Sep 17 00:00:00 2001 From: Christian Ehrig Date: Sun, 26 Jul 2020 14:14:53 +0200 Subject: [PATCH 03/12] golint --- ip_list.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ip_list.go b/ip_list.go index b9355509eb2..c0c0c8b5bbd 100644 --- a/ip_list.go +++ b/ip_list.go @@ -11,7 +11,8 @@ import ( ) const ( - IP_LIST_TYPE_IP = "ip" + // IPListTypeIP specifies a list containing IP addresses + IPListTypeIP = "ip" ) // BulkOperation contains information about a Bulk Operation From 1244339712021c9d4e794c3d8648096d8ca8a1a7 Mon Sep 17 00:00:00 2001 From: Christian Ehrig Date: Sun, 26 Jul 2020 15:03:31 +0200 Subject: [PATCH 04/12] Returns an IPList struct from UpdateIPList() instead of the full response --- ip_list.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ip_list.go b/ip_list.go index c0c0c8b5bbd..6d2891c99ae 100644 --- a/ip_list.go +++ b/ip_list.go @@ -182,19 +182,19 @@ func (api *API) GetIPList(ctx context.Context, id string) (IPList, error) { // UpdateIPList updates the description of an existing IP List // API reference: https://api.cloudflare.com/#rules-lists-update-list -func (api *API) UpdateIPList(ctx context.Context, id string, description string) (IPListResponse, error) { +func (api *API) UpdateIPList(ctx context.Context, id string, description string) (IPList, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists/%s", api.AccountID, id) res, err := api.makeRequestContext(ctx, http.MethodPut, uri, IPListUpdateRequest{Description: description}) if err != nil { - return IPListResponse{}, errors.Wrap(err, errMakeRequestError) + return IPList{}, errors.Wrap(err, errMakeRequestError) } result := IPListResponse{} if err := json.Unmarshal(res, &result); err != nil { - return IPListResponse{}, errors.Wrap(err, errUnmarshalError) + return IPList{}, errors.Wrap(err, errUnmarshalError) } - return result, nil + return result.Result, nil } // DeleteIPList deletes an IP List From c10b32b0601857bfd5661139b45427029bc2de38 Mon Sep 17 00:00:00 2001 From: Christian Ehrig Date: Sun, 26 Jul 2020 15:10:50 +0200 Subject: [PATCH 05/12] Returns an IPList struct from CreateIPList() instead of the full response --- ip_list.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ip_list.go b/ip_list.go index 6d2891c99ae..68ec465a8b0 100644 --- a/ip_list.go +++ b/ip_list.go @@ -146,21 +146,21 @@ func (api *API) ListIPLists(ctx context.Context) ([]IPList, error) { // CreateIPList creates a new IP List // API reference: https://api.cloudflare.com/#rules-lists-create-list -func (api *API) CreateIPList(ctx context.Context, name string, description string, kind string) (IPListResponse, +func (api *API) CreateIPList(ctx context.Context, name string, description string, kind string) (IPList, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists", api.AccountID) res, err := api.makeRequestContext(ctx, http.MethodPost, uri, IPListCreateRequest{Name: name, Description: description, Kind: kind}) if err != nil { - return IPListResponse{}, errors.Wrap(err, errMakeRequestError) + return IPList{}, errors.Wrap(err, errMakeRequestError) } result := IPListResponse{} if err := json.Unmarshal(res, &result); err != nil { - return IPListResponse{}, errors.Wrap(err, errUnmarshalError) + return IPList{}, errors.Wrap(err, errUnmarshalError) } - return result, nil + return result.Result, nil } // GetIPList returns a single IP List From 66a1429f1b2dd9093da399919f6ee47f41d5fa47 Mon Sep 17 00:00:00 2001 From: Christian Ehrig Date: Sun, 26 Jul 2020 15:57:21 +0200 Subject: [PATCH 06/12] Removes Debug Output --- ip_list.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/ip_list.go b/ip_list.go index 68ec465a8b0..e70952aa7ac 100644 --- a/ip_list.go +++ b/ip_list.go @@ -310,8 +310,6 @@ func (api *API) DeleteIPListItems(ctx context.Context, id string, items IPListIt error) { uri := fmt.Sprintf("/accounts/%s/rules/lists/%s/items", api.AccountID, id) res, err := api.makeRequestContext(ctx, http.MethodDelete, uri, items) - test, _ := json.Marshal(items) - fmt.Printf("%+v\n", string(test)) if err != nil { return IPListItemDeleteResponse{}, errors.Wrap(err, errMakeRequestError) } From 378caaac94996f607f8aad5c882a15bbe50329e0 Mon Sep 17 00:00:00 2001 From: Christian Ehrig Date: Sun, 26 Jul 2020 16:02:09 +0200 Subject: [PATCH 07/12] Adds acceptance tests --- ip_list_test.go | 458 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 458 insertions(+) create mode 100644 ip_list_test.go diff --git a/ip_list_test.go b/ip_list_test.go new file mode 100644 index 00000000000..6d36de770b1 --- /dev/null +++ b/ip_list_test.go @@ -0,0 +1,458 @@ +package cloudflare + +import ( + "context" + "fmt" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestListIPLists(t *testing.T) { + setup(UsingAccount("foo")) + defer teardown() + + handler := func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, r.Method, "GET", "Expected method 'GET', got %s", r.Method) + w.Header().Set("content-type", "application/json") + fmt.Fprint(w, `{ + "result": [ + { + "id": "2c0fc9fa937b11eaa1b71c4d701ab86e", + "name": "list1", + "description": "This is a note.", + "kind": "ip", + "num_items": 10, + "num_referencing_filters": 2, + "created_on": "2020-01-01T08:00:00Z", + "modified_on": "2020-01-10T14:00:00Z" + } + ], + "success": true, + "errors": [], + "messages": [] + }`) + } + + mux.HandleFunc("/accounts/foo/rules/lists", handler) + + createdOn, _ := time.Parse(time.RFC3339, "2020-01-01T08:00:00Z") + modifiedOn, _ := time.Parse(time.RFC3339, "2020-01-10T14:00:00Z") + + want := []IPList{ + { + ID: "2c0fc9fa937b11eaa1b71c4d701ab86e", + Name: "list1", + Description: "This is a note.", + Kind: "ip", + NumItems: 10, + NumReferencingFilters: 2, + CreatedOn: &createdOn, + ModifiedOn: &modifiedOn, + }, + } + + actual, err := client.ListIPLists(context.Background()) + if assert.NoError(t, err) { + assert.Equal(t, want, actual) + } +} + +func TestCreateIPList(t *testing.T) { + setup(UsingAccount("foo")) + 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.Fprint(w, `{ + "result": { + "id": "2c0fc9fa937b11eaa1b71c4d701ab86e", + "name": "list1", + "description": "This is a note.", + "kind": "ip", + "num_items": 10, + "num_referencing_filters": 2, + "created_on": "2020-01-01T08:00:00Z", + "modified_on": "2020-01-10T14:00:00Z" + }, + "success": true, + "errors": [], + "messages": [] + }`) + } + + mux.HandleFunc("/accounts/foo/rules/lists", handler) + + createdOn, _ := time.Parse(time.RFC3339, "2020-01-01T08:00:00Z") + modifiedOn, _ := time.Parse(time.RFC3339, "2020-01-10T14:00:00Z") + + want := IPList{ + ID: "2c0fc9fa937b11eaa1b71c4d701ab86e", + Name: "list1", + Description: "This is a note.", + Kind: "ip", + NumItems: 10, + NumReferencingFilters: 2, + CreatedOn: &createdOn, + ModifiedOn: &modifiedOn, + } + + actual, err := client.CreateIPList(context.Background(), "list1", "This is a note.", "ip") + if assert.NoError(t, err) { + assert.Equal(t, want, actual) + } +} + +func TestGetIPList(t *testing.T) { + setup(UsingAccount("foo")) + defer teardown() + + handler := func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, r.Method, "GET", "Expected method 'GET', got %s", r.Method) + w.Header().Set("content-type", "application/json") + fmt.Fprint(w, `{ + "result": { + "id": "2c0fc9fa937b11eaa1b71c4d701ab86e", + "name": "list1", + "description": "This is a note.", + "kind": "ip", + "num_items": 10, + "num_referencing_filters": 2, + "created_on": "2020-01-01T08:00:00Z", + "modified_on": "2020-01-10T14:00:00Z" + }, + "success": true, + "errors": [], + "messages": [] + }`) + } + + mux.HandleFunc("/accounts/foo/rules/lists/2c0fc9fa937b11eaa1b71c4d701ab86e", handler) + + createdOn, _ := time.Parse(time.RFC3339, "2020-01-01T08:00:00Z") + modifiedOn, _ := time.Parse(time.RFC3339, "2020-01-10T14:00:00Z") + + want := IPList{ + ID: "2c0fc9fa937b11eaa1b71c4d701ab86e", + Name: "list1", + Description: "This is a note.", + Kind: "ip", + NumItems: 10, + NumReferencingFilters: 2, + CreatedOn: &createdOn, + ModifiedOn: &modifiedOn, + } + + actual, err := client.GetIPList(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e") + if assert.NoError(t, err) { + assert.Equal(t, want, actual) + } +} + +func TestUpdateIPList(t *testing.T) { + setup(UsingAccount("foo")) + defer teardown() + + handler := func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, r.Method, "PUT", "Expected method 'PUT', got %s", r.Method) + w.Header().Set("content-type", "application/json") + fmt.Fprint(w, `{ + "result": { + "id": "2c0fc9fa937b11eaa1b71c4d701ab86e", + "name": "list1", + "description": "This note was updated.", + "kind": "ip", + "num_items": 10, + "num_referencing_filters": 2, + "created_on": "2020-01-01T08:00:00Z", + "modified_on": "2020-01-10T14:00:00Z" + }, + "success": true, + "errors": [], + "messages": [] + }`) + } + + mux.HandleFunc("/accounts/foo/rules/lists/2c0fc9fa937b11eaa1b71c4d701ab86e", handler) + + createdOn, _ := time.Parse(time.RFC3339, "2020-01-01T08:00:00Z") + modifiedOn, _ := time.Parse(time.RFC3339, "2020-01-10T14:00:00Z") + + want := IPList{ + ID: "2c0fc9fa937b11eaa1b71c4d701ab86e", + Name: "list1", + Description: "This note was updated.", + Kind: "ip", + NumItems: 10, + NumReferencingFilters: 2, + CreatedOn: &createdOn, + ModifiedOn: &modifiedOn, + } + + actual, err := client.UpdateIPList(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", "This note was updated.") + if assert.NoError(t, err) { + assert.Equal(t, want, actual) + } +} + +func TestDeleteIPList(t *testing.T) { + setup(UsingAccount("foo")) + defer teardown() + + handler := func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, r.Method, "DELETE", "Expected method 'DELETE', got %s", r.Method) + w.Header().Set("content-type", "application/json") + fmt.Fprint(w, `{ + "result": { + "id": "34b12448945f11eaa1b71c4d701ab86e" + }, + "success": true, + "errors": [], + "messages": [] + }`) + } + + mux.HandleFunc("/accounts/foo/rules/lists/2c0fc9fa937b11eaa1b71c4d701ab86e", handler) + + want := IPListDeleteResponse{} + want.Success = true + want.Errors = []ResponseInfo{} + want.Messages = []ResponseInfo{} + want.Result.ID = "34b12448945f11eaa1b71c4d701ab86e" + + actual, err := client.DeleteIPList(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e") + if assert.NoError(t, err) { + assert.Equal(t, want, actual) + } +} + +func TestListIPListsItems(t *testing.T) { + setup(UsingAccount("foo")) + defer teardown() + + handler := func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, r.Method, "GET", "Expected method 'GET', got %s", r.Method) + w.Header().Set("content-type", "application/json") + + if len(r.URL.Query().Get("cursor")) > 0 && r.URL.Query().Get("cursor") == "yyy" { + fmt.Fprint(w, `{ + "result": [ + { + "id": "2c0fc9fa937b11eaa1b71c4d701ab86e", + "ip": "10.0.0.2", + "comment": "Another Private IP address", + "created_on": "2020-01-01T08:00:00Z", + "modified_on": "2020-01-10T14:00:00Z" + } + ], + "result_info": { + "cursors": { + "before": "xxx" + } + }, + "success": true, + "errors": [], + "messages": [] + }`) + } else { + fmt.Fprint(w, `{ + "result": [ + { + "id": "2c0fc9fa937b11eaa1b71c4d701ab86e", + "ip": "10.0.0.1", + "comment": "Private IP address", + "created_on": "2020-01-01T08:00:00Z", + "modified_on": "2020-01-10T14:00:00Z" + } + ], + "result_info": { + "cursors": { + "before": "xxx", + "after": "yyy" + } + }, + "success": true, + "errors": [], + "messages": [] + }`) + } + + } + + mux.HandleFunc("/accounts/foo/rules/lists/2c0fc9fa937b11eaa1b71c4d701ab86e/items", handler) + + createdOn, _ := time.Parse(time.RFC3339, "2020-01-01T08:00:00Z") + modifiedOn, _ := time.Parse(time.RFC3339, "2020-01-10T14:00:00Z") + + want := []IPListItem{ + { + ID: "2c0fc9fa937b11eaa1b71c4d701ab86e", + IP: "10.0.0.1", + Comment: "Private IP address", + CreatedOn: &createdOn, + ModifiedOn: &modifiedOn, + }, + { + ID: "2c0fc9fa937b11eaa1b71c4d701ab86e", + IP: "10.0.0.2", + Comment: "Another Private IP address", + CreatedOn: &createdOn, + ModifiedOn: &modifiedOn, + }, + } + + actual, err := client.ListIPListItems(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e") + if assert.NoError(t, err) { + assert.Equal(t, want, actual) + } +} + +func TestCreateIPListItems(t *testing.T) { + setup(UsingAccount("foo")) + 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.Fprint(w, `{ + "result": { + "operation_id": "4da8780eeb215e6cb7f48dd981c4ea02" + }, + "success": true, + "errors": [], + "messages": [] + }`) + } + + mux.HandleFunc("/accounts/foo/rules/lists/2c0fc9fa937b11eaa1b71c4d701ab86e/items", handler) + + want := IPListItemResponse{} + want.Success = true + want.Errors = []ResponseInfo{} + want.Messages = []ResponseInfo{} + want.Result.OperationID = "4da8780eeb215e6cb7f48dd981c4ea02" + + actual, err := client.CreateIPListItems(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", []IPListItemCreateRequest{{ + IP: "10.0.0.1", + Comment: "Private IP", + }, { + IP: "10.0.0.2", + Comment: "Another Private IP", + }}) + if assert.NoError(t, err) { + assert.Equal(t, want, actual) + } +} + +func TestReplaceIPListItems(t *testing.T) { + setup(UsingAccount("foo")) + defer teardown() + + handler := func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, r.Method, "PUT", "Expected method 'PUT', got %s", r.Method) + w.Header().Set("content-type", "application/json") + fmt.Fprint(w, `{ + "result": { + "operation_id": "4da8780eeb215e6cb7f48dd981c4ea02" + }, + "success": true, + "errors": [], + "messages": [] + }`) + } + + mux.HandleFunc("/accounts/foo/rules/lists/2c0fc9fa937b11eaa1b71c4d701ab86e/items", handler) + + want := IPListItemResponse{} + want.Success = true + want.Errors = []ResponseInfo{} + want.Messages = []ResponseInfo{} + want.Result.OperationID = "4da8780eeb215e6cb7f48dd981c4ea02" + + actual, err := client.ReplaceIPListItems(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", []IPListItemCreateRequest{{ + IP: "10.0.0.1", + Comment: "Private IP", + }, { + IP: "10.0.0.2", + Comment: "Another Private IP", + }}) + if assert.NoError(t, err) { + assert.Equal(t, want, actual) + } +} + +func TestDeleteIPListItems(t *testing.T) { + setup(UsingAccount("foo")) + defer teardown() + + handler := func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, r.Method, "DELETE", "Expected method 'DELETE', got %s", r.Method) + w.Header().Set("content-type", "application/json") + fmt.Fprint(w, `{ + "result": { + "operation_id": "4da8780eeb215e6cb7f48dd981c4ea02" + }, + "success": true, + "errors": [], + "messages": [] + }`) + } + + mux.HandleFunc("/accounts/foo/rules/lists/2c0fc9fa937b11eaa1b71c4d701ab86e/items", handler) + + want := IPListItemDeleteResponse{} + want.Success = true + want.Errors = []ResponseInfo{} + want.Messages = []ResponseInfo{} + want.Result.OperationID = "4da8780eeb215e6cb7f48dd981c4ea02" + + actual, err := client.DeleteIPListItems(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", IPListItemDeleteRequest{[]IPListItemDeleteItemRequest{{ + ID: "34b12448945f11eaa1b71c4d701ab86e", + }}}) + if assert.NoError(t, err) { + assert.Equal(t, want, actual) + } +} + +func TestGetIPListItem(t *testing.T) { + setup(UsingAccount("foo")) + defer teardown() + + handler := func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, r.Method, "GET", "Expected method 'GET', got %s", r.Method) + w.Header().Set("content-type", "application/json") + fmt.Fprint(w, `{ + "result": { + "id": "2c0fc9fa937b11eaa1b71c4d701ab86e", + "ip": "10.0.0.1", + "comment": "Private IP address", + "created_on": "2020-01-01T08:00:00Z", + "modified_on": "2020-01-10T14:00:00Z" + }, + "success": true, + "errors": [], + "messages": [] + }`) + } + + mux.HandleFunc("/accounts/foo/rules/lists/2c0fc9fa937b11eaa1b71c4d701ab86e/items/34b12448945f11eaa1b71c4d701ab86e", handler) + + createdOn, _ := time.Parse(time.RFC3339, "2020-01-01T08:00:00Z") + modifiedOn, _ := time.Parse(time.RFC3339, "2020-01-10T14:00:00Z") + + want := IPListItem{ + ID: "2c0fc9fa937b11eaa1b71c4d701ab86e", + IP: "10.0.0.1", + Comment: "Private IP address", + CreatedOn: &createdOn, + ModifiedOn: &modifiedOn, + } + + actual, err := client.GetIPListItem(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", "34b12448945f11eaa1b71c4d701ab86e") + if assert.NoError(t, err) { + assert.Equal(t, want, actual) + } +} From 4d5c8edb8c9612304f56198860a06d5d8d5ad4cb Mon Sep 17 00:00:00 2001 From: Christian Ehrig Date: Sun, 26 Jul 2020 16:40:37 +0200 Subject: [PATCH 08/12] Renames BulkOperation to IPListBulkOperation --- ip_list.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/ip_list.go b/ip_list.go index e70952aa7ac..37209538842 100644 --- a/ip_list.go +++ b/ip_list.go @@ -15,8 +15,8 @@ const ( IPListTypeIP = "ip" ) -// BulkOperation contains information about a Bulk Operation -type BulkOperation struct { +// IPListBulkOperation contains information about a Bulk Operation +type IPListBulkOperation struct { ID string `json:"id"` Status string `json:"status"` Error string `json:"error"` @@ -92,10 +92,10 @@ type IPListListResponse struct { Result []IPList `json:"result"` } -// BulkOperationResponse contains a slice of IP Lists -type BulkOperationResponse struct { +// IPListBulkOperationResponse contains information about a Bulk Operation +type IPListBulkOperationResponse struct { Response - Result BulkOperation `json:"result"` + Result IPListBulkOperation `json:"result"` } // IPListDeleteResponse contains information about the deletion of an IP List @@ -339,18 +339,18 @@ func (api *API) GetIPListItem(ctx context.Context, listID string, id string) (IP return result.Result, nil } -// GetBulkOperation returns the status of a bulk operation +// GetIPListBulkOperation returns the status of a bulk operation // API reference: https://api.cloudflare.com/#rules-lists-get-list -func (api *API) GetBulkOperation(ctx context.Context, id string) (BulkOperation, error) { +func (api *API) GetIPListBulkOperation(ctx context.Context, id string) (IPListBulkOperation, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists/bulk_operations/%s", api.AccountID, id) res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { - return BulkOperation{}, errors.Wrap(err, errMakeRequestError) + return IPListBulkOperation{}, errors.Wrap(err, errMakeRequestError) } - result := BulkOperationResponse{} + result := IPListBulkOperationResponse{} if err := json.Unmarshal(res, &result); err != nil { - return BulkOperation{}, errors.Wrap(err, errUnmarshalError) + return IPListBulkOperation{}, errors.Wrap(err, errUnmarshalError) } return result.Result, nil From eb25a11007e46eca4067a3f03a152beb122e7c41 Mon Sep 17 00:00:00 2001 From: Christian Ehrig Date: Mon, 27 Jul 2020 19:09:33 +0200 Subject: [PATCH 09/12] Adds synchronous behavior for asynchronous endpoints --- errors.go | 13 +++-- ip_list.go | 148 ++++++++++++++++++++++++++++++++++++++---------- ip_list_test.go | 50 +++++++++------- 3 files changed, 153 insertions(+), 58 deletions(-) diff --git a/errors.go b/errors.go index 21c38b16801..31f87317509 100644 --- a/errors.go +++ b/errors.go @@ -2,12 +2,13 @@ package cloudflare // Error messages const ( - errEmptyCredentials = "invalid credentials: key & email must not be empty" - errEmptyAPIToken = "invalid credentials: API Token must not be empty" - errMakeRequestError = "error from makeRequest" - errUnmarshalError = "error unmarshalling the JSON response" - errRequestNotSuccessful = "error reported by API" - errMissingAccountID = "account ID is empty and must be provided" + errEmptyCredentials = "invalid credentials: key & email must not be empty" + errEmptyAPIToken = "invalid credentials: API Token must not be empty" + errMakeRequestError = "error from makeRequest" + errUnmarshalError = "error unmarshalling the JSON response" + errRequestNotSuccessful = "error reported by API" + errMissingAccountID = "account ID is empty and must be provided" + errOperationStillRunning = "bulk operation did not finish before timeout" ) var _ Error = &UserError{} diff --git a/ip_list.go b/ip_list.go index 37209538842..5b705f70d63 100644 --- a/ip_list.go +++ b/ip_list.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "math" "net/http" "time" @@ -72,14 +73,14 @@ type IPListUpdateRequest struct { Description string `json:"description"` } -// IPListResponse contains a single IP Lists +// IPListResponse contains a single IP List type IPListResponse struct { Response Result IPList `json:"result"` } -// IPListItemResponse contains information about the creation of an IP List Item -type IPListItemResponse struct { +// IPListItemCreateResponse contains information about the creation of an IP List Item +type IPListItemCreateResponse struct { Response Result struct { OperationID string `json:"operation_id"` @@ -245,69 +246,116 @@ func (api *API) ListIPListItems(ctx context.Context, id string) ([]IPListItem, e return list, nil } -// CreateIPListItem creates a new IP List Item -// TODO Responses won't contain an error code if we are trying to create more IPs than allowed -// TODO shall we keep the method asynchronously or poll the bulk operations endpoint? +// CreateIPListItemAsync creates a new IP List Item asynchronously, Users have to poll the operation status by +// using the operation_id returned by this function. // API reference: https://api.cloudflare.com/#rules-lists-create-list-items -func (api *API) CreateIPListItem(ctx context.Context, id string, ip string, comment string) (IPListItemResponse, - error) { +func (api *API) CreateIPListItemAsync(ctx context.Context, id string, ip string, comment string) ( + IPListItemCreateResponse, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists/%s/items", api.AccountID, id) res, err := api.makeRequestContext(ctx, http.MethodPost, uri, []IPListItemCreateRequest{{IP: ip, Comment: comment}}) if err != nil { - return IPListItemResponse{}, errors.Wrap(err, errMakeRequestError) + return IPListItemCreateResponse{}, errors.Wrap(err, errMakeRequestError) } - result := IPListItemResponse{} + result := IPListItemCreateResponse{} if err := json.Unmarshal(res, &result); err != nil { - return IPListItemResponse{}, errors.Wrap(err, errUnmarshalError) + return IPListItemCreateResponse{}, errors.Wrap(err, errUnmarshalError) } return result, nil } -// CreateIPListItems bulk creates many IP List Items -// TODO shall we keep the method asynchronously or poll the bulk operations endpoint? +// CreateIPListItem creates a new IP List Item synchronously and returns the current set of IP List Items +func (api *API) CreateIPListItem(ctx context.Context, id string, ip string, comment string) ([]IPListItem, error) { + result, err := api.CreateIPListItemAsync(ctx, id, ip, comment) + + if err != nil { + return []IPListItem{}, err + } + + err = api.pollIPListBulkOperation(ctx, result.Result.OperationID) + if err != nil { + return []IPListItem{}, err + } + + return api.ListIPListItems(ctx, id) +} + +// CreateIPListItemsAsync bulk creates many IP List Items asynchronously. Users have to poll the operation status by +// using the operation_id returned by this function. // API reference: https://api.cloudflare.com/#rules-lists-create-list-items -func (api *API) CreateIPListItems(ctx context.Context, id string, items []IPListItemCreateRequest) (IPListItemResponse, - error) { +func (api *API) CreateIPListItemsAsync(ctx context.Context, id string, items []IPListItemCreateRequest) ( + IPListItemCreateResponse, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists/%s/items", api.AccountID, id) res, err := api.makeRequestContext(ctx, http.MethodPost, uri, items) if err != nil { - return IPListItemResponse{}, errors.Wrap(err, errMakeRequestError) + return IPListItemCreateResponse{}, errors.Wrap(err, errMakeRequestError) } - result := IPListItemResponse{} + result := IPListItemCreateResponse{} if err := json.Unmarshal(res, &result); err != nil { - return IPListItemResponse{}, errors.Wrap(err, errUnmarshalError) + return IPListItemCreateResponse{}, errors.Wrap(err, errUnmarshalError) } return result, nil } -// ReplaceIPListItems replace all IP List items with a new set -// TODO shall we keep the method asynchronously or poll the bulk operations endpoint? +// CreateIPListItems bulk creates many IP List Items synchronously and returns the current set of IP List Items +func (api *API) CreateIPListItems(ctx context.Context, id string, items []IPListItemCreateRequest) ( + []IPListItem, error) { + result, err := api.CreateIPListItemsAsync(ctx, id, items) + if err != nil { + return []IPListItem{}, err + } + + err = api.pollIPListBulkOperation(ctx, result.Result.OperationID) + if err != nil { + return []IPListItem{}, err + } + + return api.ListIPListItems(ctx, id) +} + +// ReplaceIPListItemsAsync replaces all IP List Items asynchronously. Users have to poll the operation status by +// using the operation_id returned by this function. // API reference: https://api.cloudflare.com/#rules-lists-replace-list-items -func (api *API) ReplaceIPListItems(ctx context.Context, id string, items []IPListItemCreateRequest) (IPListItemResponse, - error) { +func (api *API) ReplaceIPListItemsAsync(ctx context.Context, id string, items []IPListItemCreateRequest) ( + IPListItemCreateResponse, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists/%s/items", api.AccountID, id) res, err := api.makeRequestContext(ctx, http.MethodPut, uri, items) if err != nil { - return IPListItemResponse{}, errors.Wrap(err, errMakeRequestError) + return IPListItemCreateResponse{}, errors.Wrap(err, errMakeRequestError) } - result := IPListItemResponse{} + result := IPListItemCreateResponse{} if err := json.Unmarshal(res, &result); err != nil { - return IPListItemResponse{}, errors.Wrap(err, errUnmarshalError) + return IPListItemCreateResponse{}, errors.Wrap(err, errUnmarshalError) } return result, nil } -// DeleteIPListItems removes specific Items of an IP List by their ID -// TODO shall we keep the method asynchronously or poll the bulk operations endpoint? +// ReplaceIPListItems replaces all IP List Items synchronously and returns the current set of IP List Items +func (api *API) ReplaceIPListItems(ctx context.Context, id string, items []IPListItemCreateRequest) ( + []IPListItem, error) { + result, err := api.ReplaceIPListItemsAsync(ctx, id, items) + if err != nil { + return []IPListItem{}, err + } + + err = api.pollIPListBulkOperation(ctx, result.Result.OperationID) + if err != nil { + return []IPListItem{}, err + } + + return api.ListIPListItems(ctx, id) +} + +// DeleteIPListItemsAsync removes specific Items of an IP List by their ID asynchronously. Users have to poll the +// operation status by using the operation_id returned by this function. // API reference: https://api.cloudflare.com/#rules-lists-delete-list-items -func (api *API) DeleteIPListItems(ctx context.Context, id string, items IPListItemDeleteRequest) (IPListItemDeleteResponse, - error) { +func (api *API) DeleteIPListItemsAsync(ctx context.Context, id string, items IPListItemDeleteRequest) ( + IPListItemDeleteResponse, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists/%s/items", api.AccountID, id) res, err := api.makeRequestContext(ctx, http.MethodDelete, uri, items) if err != nil { @@ -322,6 +370,23 @@ func (api *API) DeleteIPListItems(ctx context.Context, id string, items IPListIt return result, nil } +// DeleteIPListItemsAsync removes specific Items of an IP List by their ID synchronously and returns the current set +// of IP List Items +func (api *API) DeleteIPListItems(ctx context.Context, id string, items IPListItemDeleteRequest) ( + []IPListItem, error) { + result, err := api.DeleteIPListItemsAsync(ctx, id, items) + if err != nil { + return []IPListItem{}, err + } + + err = api.pollIPListBulkOperation(ctx, result.Result.OperationID) + if err != nil { + return []IPListItem{}, err + } + + return api.ListIPListItems(ctx, id) +} + // GetIPListItem returns a single IP List Item // API reference: https://api.cloudflare.com/#rules-lists-get-list-item func (api *API) GetIPListItem(ctx context.Context, listID string, id string) (IPListItem, error) { @@ -340,7 +405,7 @@ func (api *API) GetIPListItem(ctx context.Context, listID string, id string) (IP } // GetIPListBulkOperation returns the status of a bulk operation -// API reference: https://api.cloudflare.com/#rules-lists-get-list +// API reference: https://api.cloudflare.com/#rules-lists-get-bulk-operation func (api *API) GetIPListBulkOperation(ctx context.Context, id string) (IPListBulkOperation, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists/bulk_operations/%s", api.AccountID, id) res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) @@ -355,3 +420,26 @@ func (api *API) GetIPListBulkOperation(ctx context.Context, id string) (IPListBu return result.Result, nil } + +// this is a helper method to implement synchronous behavior for some asynchronous endpoints. +// bulk-operation status can be either pending, running, failed or completed +func (api *API) pollIPListBulkOperation(ctx context.Context, id string) error { + var i uint8 + for i = 0; i < 16; i++ { + time.Sleep(0x1 << uint8(math.Ceil(float64(i/2))) * time.Second) + + bulkResult, err := api.GetIPListBulkOperation(ctx, id) + if err != nil { + return err + } + + switch bulkResult.Status { + case "failed": + return errors.New(bulkResult.Error) + case "completed": + return nil + } + } + + return errors.New(errOperationStillRunning) +} diff --git a/ip_list_test.go b/ip_list_test.go index 6d36de770b1..6ed8055d4fc 100644 --- a/ip_list_test.go +++ b/ip_list_test.go @@ -192,7 +192,8 @@ func TestUpdateIPList(t *testing.T) { ModifiedOn: &modifiedOn, } - actual, err := client.UpdateIPList(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", "This note was updated.") + actual, err := client.UpdateIPList(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", + "This note was updated.") if assert.NoError(t, err) { assert.Equal(t, want, actual) } @@ -329,19 +330,20 @@ func TestCreateIPListItems(t *testing.T) { mux.HandleFunc("/accounts/foo/rules/lists/2c0fc9fa937b11eaa1b71c4d701ab86e/items", handler) - want := IPListItemResponse{} + want := IPListItemCreateResponse{} want.Success = true want.Errors = []ResponseInfo{} want.Messages = []ResponseInfo{} want.Result.OperationID = "4da8780eeb215e6cb7f48dd981c4ea02" - actual, err := client.CreateIPListItems(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", []IPListItemCreateRequest{{ - IP: "10.0.0.1", - Comment: "Private IP", - }, { - IP: "10.0.0.2", - Comment: "Another Private IP", - }}) + actual, err := client.CreateIPListItemsAsync(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", + []IPListItemCreateRequest{{ + IP: "10.0.0.1", + Comment: "Private IP", + }, { + IP: "10.0.0.2", + Comment: "Another Private IP", + }}) if assert.NoError(t, err) { assert.Equal(t, want, actual) } @@ -366,19 +368,20 @@ func TestReplaceIPListItems(t *testing.T) { mux.HandleFunc("/accounts/foo/rules/lists/2c0fc9fa937b11eaa1b71c4d701ab86e/items", handler) - want := IPListItemResponse{} + want := IPListItemCreateResponse{} want.Success = true want.Errors = []ResponseInfo{} want.Messages = []ResponseInfo{} want.Result.OperationID = "4da8780eeb215e6cb7f48dd981c4ea02" - actual, err := client.ReplaceIPListItems(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", []IPListItemCreateRequest{{ - IP: "10.0.0.1", - Comment: "Private IP", - }, { - IP: "10.0.0.2", - Comment: "Another Private IP", - }}) + actual, err := client.ReplaceIPListItemsAsync(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", + []IPListItemCreateRequest{{ + IP: "10.0.0.1", + Comment: "Private IP", + }, { + IP: "10.0.0.2", + Comment: "Another Private IP", + }}) if assert.NoError(t, err) { assert.Equal(t, want, actual) } @@ -409,9 +412,10 @@ func TestDeleteIPListItems(t *testing.T) { want.Messages = []ResponseInfo{} want.Result.OperationID = "4da8780eeb215e6cb7f48dd981c4ea02" - actual, err := client.DeleteIPListItems(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", IPListItemDeleteRequest{[]IPListItemDeleteItemRequest{{ - ID: "34b12448945f11eaa1b71c4d701ab86e", - }}}) + actual, err := client.DeleteIPListItemsAsync(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", + IPListItemDeleteRequest{[]IPListItemDeleteItemRequest{{ + ID: "34b12448945f11eaa1b71c4d701ab86e", + }}}) if assert.NoError(t, err) { assert.Equal(t, want, actual) } @@ -438,7 +442,8 @@ func TestGetIPListItem(t *testing.T) { }`) } - mux.HandleFunc("/accounts/foo/rules/lists/2c0fc9fa937b11eaa1b71c4d701ab86e/items/34b12448945f11eaa1b71c4d701ab86e", handler) + mux.HandleFunc("/accounts/foo/rules/lists/2c0fc9fa937b11eaa1b71c4d701ab86e/items/"+ + "34b12448945f11eaa1b71c4d701ab86e", handler) createdOn, _ := time.Parse(time.RFC3339, "2020-01-01T08:00:00Z") modifiedOn, _ := time.Parse(time.RFC3339, "2020-01-10T14:00:00Z") @@ -451,7 +456,8 @@ func TestGetIPListItem(t *testing.T) { ModifiedOn: &modifiedOn, } - actual, err := client.GetIPListItem(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", "34b12448945f11eaa1b71c4d701ab86e") + actual, err := client.GetIPListItem(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", + "34b12448945f11eaa1b71c4d701ab86e") if assert.NoError(t, err) { assert.Equal(t, want, actual) } From d23a3356ed1159603c7bc3966f14ad02b01135d8 Mon Sep 17 00:00:00 2001 From: Christian Ehrig Date: Mon, 27 Jul 2020 19:11:59 +0200 Subject: [PATCH 10/12] golint --- ip_list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ip_list.go b/ip_list.go index 5b705f70d63..97b4e9208e4 100644 --- a/ip_list.go +++ b/ip_list.go @@ -370,7 +370,7 @@ func (api *API) DeleteIPListItemsAsync(ctx context.Context, id string, items IPL return result, nil } -// DeleteIPListItemsAsync removes specific Items of an IP List by their ID synchronously and returns the current set +// DeleteIPListItems removes specific Items of an IP List by their ID synchronously and returns the current set // of IP List Items func (api *API) DeleteIPListItems(ctx context.Context, id string, items IPListItemDeleteRequest) ( []IPListItem, error) { From db994f57d93f6f1409f8e1738d92c4f1f408ef60 Mon Sep 17 00:00:00 2001 From: Christian Ehrig Date: Wed, 29 Jul 2020 10:50:57 +0200 Subject: [PATCH 11/12] Apply suggestions from code review Co-authored-by: Jacob Bednarz --- ip_list.go | 23 +++++++++++++++++------ ip_list_test.go | 20 ++++++++++---------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/ip_list.go b/ip_list.go index 97b4e9208e4..26d94e78e72 100644 --- a/ip_list.go +++ b/ip_list.go @@ -129,6 +129,7 @@ type IPListItemsGetResponse struct { } // ListIPLists lists all IP Lists +// // API reference: https://api.cloudflare.com/#rules-lists-list-lists func (api *API) ListIPLists(ctx context.Context) ([]IPList, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists", api.AccountID) @@ -146,6 +147,7 @@ func (api *API) ListIPLists(ctx context.Context) ([]IPList, error) { } // CreateIPList creates a new IP List +// // API reference: https://api.cloudflare.com/#rules-lists-create-list func (api *API) CreateIPList(ctx context.Context, name string, description string, kind string) (IPList, error) { @@ -165,6 +167,7 @@ func (api *API) CreateIPList(ctx context.Context, name string, description strin } // GetIPList returns a single IP List +// // API reference: https://api.cloudflare.com/#rules-lists-get-list func (api *API) GetIPList(ctx context.Context, id string) (IPList, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists/%s", api.AccountID, id) @@ -182,6 +185,7 @@ func (api *API) GetIPList(ctx context.Context, id string) (IPList, error) { } // UpdateIPList updates the description of an existing IP List +// // API reference: https://api.cloudflare.com/#rules-lists-update-list func (api *API) UpdateIPList(ctx context.Context, id string, description string) (IPList, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists/%s", api.AccountID, id) @@ -199,6 +203,7 @@ func (api *API) UpdateIPList(ctx context.Context, id string, description string) } // DeleteIPList deletes an IP List +// // API reference: https://api.cloudflare.com/#rules-lists-delete-list func (api *API) DeleteIPList(ctx context.Context, id string) (IPListDeleteResponse, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists/%s", api.AccountID, id) @@ -216,6 +221,7 @@ func (api *API) DeleteIPList(ctx context.Context, id string) (IPListDeleteRespon } // ListIPListItems returns a list with all items in an IP List +// // API reference: https://api.cloudflare.com/#rules-lists-list-list-items func (api *API) ListIPListItems(ctx context.Context, id string) ([]IPListItem, error) { var list []IPListItem @@ -246,11 +252,11 @@ func (api *API) ListIPListItems(ctx context.Context, id string) ([]IPListItem, e return list, nil } -// CreateIPListItemAsync creates a new IP List Item asynchronously, Users have to poll the operation status by +// CreateIPListItemAsync creates a new IP List Item asynchronously. Users have to poll the operation status by // using the operation_id returned by this function. +// // API reference: https://api.cloudflare.com/#rules-lists-create-list-items -func (api *API) CreateIPListItemAsync(ctx context.Context, id string, ip string, comment string) ( - IPListItemCreateResponse, error) { +func (api *API) CreateIPListItemAsync(ctx context.Context, id, ip, comment string) (IPListItemCreateResponse, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists/%s/items", api.AccountID, id) res, err := api.makeRequestContext(ctx, http.MethodPost, uri, []IPListItemCreateRequest{{IP: ip, Comment: comment}}) if err != nil { @@ -266,7 +272,7 @@ func (api *API) CreateIPListItemAsync(ctx context.Context, id string, ip string, } // CreateIPListItem creates a new IP List Item synchronously and returns the current set of IP List Items -func (api *API) CreateIPListItem(ctx context.Context, id string, ip string, comment string) ([]IPListItem, error) { +func (api *API) CreateIPListItem(ctx context.Context, id, ip, comment string) ([]IPListItem, error) { result, err := api.CreateIPListItemAsync(ctx, id, ip, comment) if err != nil { @@ -283,6 +289,7 @@ func (api *API) CreateIPListItem(ctx context.Context, id string, ip string, comm // CreateIPListItemsAsync bulk creates many IP List Items asynchronously. Users have to poll the operation status by // using the operation_id returned by this function. +// // API reference: https://api.cloudflare.com/#rules-lists-create-list-items func (api *API) CreateIPListItemsAsync(ctx context.Context, id string, items []IPListItemCreateRequest) ( IPListItemCreateResponse, error) { @@ -318,6 +325,7 @@ func (api *API) CreateIPListItems(ctx context.Context, id string, items []IPList // ReplaceIPListItemsAsync replaces all IP List Items asynchronously. Users have to poll the operation status by // using the operation_id returned by this function. +// // API reference: https://api.cloudflare.com/#rules-lists-replace-list-items func (api *API) ReplaceIPListItemsAsync(ctx context.Context, id string, items []IPListItemCreateRequest) ( IPListItemCreateResponse, error) { @@ -353,6 +361,7 @@ func (api *API) ReplaceIPListItems(ctx context.Context, id string, items []IPLis // DeleteIPListItemsAsync removes specific Items of an IP List by their ID asynchronously. Users have to poll the // operation status by using the operation_id returned by this function. +// // API reference: https://api.cloudflare.com/#rules-lists-delete-list-items func (api *API) DeleteIPListItemsAsync(ctx context.Context, id string, items IPListItemDeleteRequest) ( IPListItemDeleteResponse, error) { @@ -388,8 +397,9 @@ func (api *API) DeleteIPListItems(ctx context.Context, id string, items IPListIt } // GetIPListItem returns a single IP List Item +// // API reference: https://api.cloudflare.com/#rules-lists-get-list-item -func (api *API) GetIPListItem(ctx context.Context, listID string, id string) (IPListItem, error) { +func (api *API) GetIPListItem(ctx context.Context, listID, id string) (IPListItem, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists/%s/items/%s", api.AccountID, listID, id) res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { @@ -405,6 +415,7 @@ func (api *API) GetIPListItem(ctx context.Context, listID string, id string) (IP } // GetIPListBulkOperation returns the status of a bulk operation +// // API reference: https://api.cloudflare.com/#rules-lists-get-bulk-operation func (api *API) GetIPListBulkOperation(ctx context.Context, id string) (IPListBulkOperation, error) { uri := fmt.Sprintf("/accounts/%s/rules/lists/bulk_operations/%s", api.AccountID, id) @@ -421,7 +432,7 @@ func (api *API) GetIPListBulkOperation(ctx context.Context, id string) (IPListBu return result.Result, nil } -// this is a helper method to implement synchronous behavior for some asynchronous endpoints. +// pollIPListBulkOperation implements synchronous behaviour for some asynchronous endpoints. // bulk-operation status can be either pending, running, failed or completed func (api *API) pollIPListBulkOperation(ctx context.Context, id string) error { var i uint8 diff --git a/ip_list_test.go b/ip_list_test.go index 6ed8055d4fc..c7ab7b257fb 100644 --- a/ip_list_test.go +++ b/ip_list_test.go @@ -243,7 +243,7 @@ func TestListIPListsItems(t *testing.T) { "result": [ { "id": "2c0fc9fa937b11eaa1b71c4d701ab86e", - "ip": "10.0.0.2", + "ip": "192.0.2.2", "comment": "Another Private IP address", "created_on": "2020-01-01T08:00:00Z", "modified_on": "2020-01-10T14:00:00Z" @@ -263,7 +263,7 @@ func TestListIPListsItems(t *testing.T) { "result": [ { "id": "2c0fc9fa937b11eaa1b71c4d701ab86e", - "ip": "10.0.0.1", + "ip": "192.0.2.1", "comment": "Private IP address", "created_on": "2020-01-01T08:00:00Z", "modified_on": "2020-01-10T14:00:00Z" @@ -291,14 +291,14 @@ func TestListIPListsItems(t *testing.T) { want := []IPListItem{ { ID: "2c0fc9fa937b11eaa1b71c4d701ab86e", - IP: "10.0.0.1", + IP: "192.0.2.1", Comment: "Private IP address", CreatedOn: &createdOn, ModifiedOn: &modifiedOn, }, { ID: "2c0fc9fa937b11eaa1b71c4d701ab86e", - IP: "10.0.0.2", + IP: "192.0.2.2", Comment: "Another Private IP address", CreatedOn: &createdOn, ModifiedOn: &modifiedOn, @@ -338,10 +338,10 @@ func TestCreateIPListItems(t *testing.T) { actual, err := client.CreateIPListItemsAsync(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", []IPListItemCreateRequest{{ - IP: "10.0.0.1", + IP: "192.0.2.1", Comment: "Private IP", }, { - IP: "10.0.0.2", + IP: "192.0.2.2", Comment: "Another Private IP", }}) if assert.NoError(t, err) { @@ -376,10 +376,10 @@ func TestReplaceIPListItems(t *testing.T) { actual, err := client.ReplaceIPListItemsAsync(context.Background(), "2c0fc9fa937b11eaa1b71c4d701ab86e", []IPListItemCreateRequest{{ - IP: "10.0.0.1", + IP: "192.0.2.1", Comment: "Private IP", }, { - IP: "10.0.0.2", + IP: "192.0.2.2", Comment: "Another Private IP", }}) if assert.NoError(t, err) { @@ -431,7 +431,7 @@ func TestGetIPListItem(t *testing.T) { fmt.Fprint(w, `{ "result": { "id": "2c0fc9fa937b11eaa1b71c4d701ab86e", - "ip": "10.0.0.1", + "ip": "192.0.2.1", "comment": "Private IP address", "created_on": "2020-01-01T08:00:00Z", "modified_on": "2020-01-10T14:00:00Z" @@ -450,7 +450,7 @@ func TestGetIPListItem(t *testing.T) { want := IPListItem{ ID: "2c0fc9fa937b11eaa1b71c4d701ab86e", - IP: "10.0.0.1", + IP: "192.0.2.1", Comment: "Private IP address", CreatedOn: &createdOn, ModifiedOn: &modifiedOn, From a3acbedad98163e71626e1455a6d17b48d7d1804 Mon Sep 17 00:00:00 2001 From: Christian Ehrig Date: Wed, 29 Jul 2020 13:34:24 +0200 Subject: [PATCH 12/12] Returns an error if BulkOperation returns an unexpected status --- errors.go | 15 ++++++++------- ip_list.go | 4 ++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/errors.go b/errors.go index 31f87317509..f2ea83a5dc0 100644 --- a/errors.go +++ b/errors.go @@ -2,13 +2,14 @@ package cloudflare // Error messages const ( - errEmptyCredentials = "invalid credentials: key & email must not be empty" - errEmptyAPIToken = "invalid credentials: API Token must not be empty" - errMakeRequestError = "error from makeRequest" - errUnmarshalError = "error unmarshalling the JSON response" - errRequestNotSuccessful = "error reported by API" - errMissingAccountID = "account ID is empty and must be provided" - errOperationStillRunning = "bulk operation did not finish before timeout" + errEmptyCredentials = "invalid credentials: key & email must not be empty" + errEmptyAPIToken = "invalid credentials: API Token must not be empty" + errMakeRequestError = "error from makeRequest" + errUnmarshalError = "error unmarshalling the JSON response" + errRequestNotSuccessful = "error reported by API" + errMissingAccountID = "account ID is empty and must be provided" + errOperationStillRunning = "bulk operation did not finish before timeout" + errOperationUnexpectedStatus = "bulk operation returned an unexpected status" ) var _ Error = &UserError{} diff --git a/ip_list.go b/ip_list.go index 26d94e78e72..4dcf5072ec1 100644 --- a/ip_list.go +++ b/ip_list.go @@ -447,8 +447,12 @@ func (api *API) pollIPListBulkOperation(ctx context.Context, id string) error { switch bulkResult.Status { case "failed": return errors.New(bulkResult.Error) + case "pending", "running": + continue case "completed": return nil + default: + return errors.New(fmt.Sprintf("%s: %s", errOperationUnexpectedStatus, bulkResult.Status)) } }