Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Get/Patch API Shield API Discovery Operations #1413

Merged
merged 3 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/1413.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
Add support for Get/Patch API Shield API Discovery Operations
jacobbednarz marked this conversation as resolved.
Show resolved Hide resolved
```
174 changes: 174 additions & 0 deletions api_shield_api_discovery.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
package cloudflare

import (
"context"
"fmt"
"net/http"
"time"

"github.com/goccy/go-json"
)

// APIShieldDiscoveryOperation is an operation that was discovered by API Discovery.
type APIShieldDiscoveryOperation struct {
// ID represents the ID of the operation
ID string `json:"id"`
djhworld marked this conversation as resolved.
Show resolved Hide resolved
// Origin represents the API discovery engine(s) that discovered this operation
Origin []string `json:"origin"`
// State represents the state of operation in API Discovery
State string `json:"state"`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we provide the set of valid state strings as consts that one can pass here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entirely up to you folks. we do half and half in other places but once we do SDK generation, we will have allowed enums.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have opted to add typed field + consts for these fields 80d9650

// LastUpdated timestamp of when this operation was last updated
LastUpdated *time.Time `json:"last_updated,omitempty"`
djhworld marked this conversation as resolved.
Show resolved Hide resolved
// Features are additional data about the operation
Features map[string]any `json:"features,omitempty"`

Method string `json:"method"`
Host string `json:"host"`
Endpoint string `json:"endpoint"`
}

// ListAPIShieldDiscoveryOperationsParams represents the parameters to pass when retrieving discovered operations.
//
// API documentation: https://developers.cloudflare.com/api/operations/api-shield-api-discovery-retrieve-discovered-operations-on-a-zone
type ListAPIShieldDiscoveryOperationsParams struct {
// Direction to order results.
Direction string `url:"direction,omitempty"`
// OrderBy when requesting a feature, the feature keys are available for ordering as well, e.g., thresholds.suggested_threshold.
OrderBy string `url:"order,omitempty"`
// Filters to only return operations that match filtering criteria, see APIShieldGetOperationsFilters
APIShieldListDiscoveryOperationsFilters
// Pagination options to apply to the request.
PaginationOptions
}

// APIShieldListDiscoveryOperationsFilters represents the filtering query parameters to set when retrieving discovery operations.
//
// API documentation: https://developers.cloudflare.com/api/operations/api-shield-api-discovery-retrieve-discovered-operations-on-a-zone
type APIShieldListDiscoveryOperationsFilters struct {
jacobbednarz marked this conversation as resolved.
Show resolved Hide resolved
// Hosts filters results to only include the specified hosts.
Hosts []string `url:"host,omitempty"`
// Methods filters results to only include the specified methods.
Methods []string `url:"method,omitempty"`
// Endpoint filter results to only include endpoints containing this pattern.
djhworld marked this conversation as resolved.
Show resolved Hide resolved
Endpoint string `url:"endpoint,omitempty"`
// Diff when true, only return API Discovery results that are not saved into API Shield Endpoint Management
Diff bool `url:"diff,omitempty"`
// Origin filter results to only include discovery results sourced from a particular discovery engine
Origin string `url:"origin,omitempty"`
djhworld marked this conversation as resolved.
Show resolved Hide resolved
// State filter results to only include discovery results in a particular state.
State string `url:"state,omitempty"`
djhworld marked this conversation as resolved.
Show resolved Hide resolved
}

// PatchAPIShieldDiscoveryOperationParams represents the parameters to pass to patch a discovery operation
//
// API documentation: https://developers.cloudflare.com/api/operations/api-shield-api-patch-discovered-operation
type PatchAPIShieldDiscoveryOperationParams struct {
// OperationID is the operation to be patched
OperationID string `json:"-" url:"-"`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this not a uuid?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we pass string for the ID field in normal operations too https://github.com/cloudflare/cloudflare-go/blob/master/api_shield_operations.go#L13-L18

I'm not sure if it's a good idea to enforce the type here or not (we'd need cloudflare-go to depend on a uuid library like github.com/google/uuid)

@jacobbednarz what do you think? I'm inclined to just stick with string. If a non uuid is provided then the API will respond with an invalid input error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

string seems fine to me here.


PatchAPIShieldDiscoveryOperation
}

// PatchAPIShieldDiscoveryOperationsParams maps discovery operation IDs to PatchAPIShieldDiscoveryOperation structs
//
// Example:
//
// PatchAPIShieldDiscoveryOperations{
// "99522293-a505-45e5-bbad-bbc339f5dc40": PatchAPIShieldDiscoveryOperation{ State: "review" },
// }
//
// API documentation: https://developers.cloudflare.com/api/operations/api-shield-api-patch-discovered-operations
type PatchAPIShieldDiscoveryOperationsParams map[string]PatchAPIShieldDiscoveryOperation

// PatchAPIShieldDiscoveryOperation represents the state to set on a discovery operation.
type PatchAPIShieldDiscoveryOperation struct {
// State is the state to set on the operation
State string `json:"state" url:"-"`
djhworld marked this conversation as resolved.
Show resolved Hide resolved
}

// APIShieldListDiscoveryOperationsResponse represents the response from the api_gateway/discovery/operations endpoint.
type APIShieldListDiscoveryOperationsResponse struct {
Result []APIShieldDiscoveryOperation `json:"result"`
ResultInfo `json:"result_info"`
Response
}

// APIShieldPatchDiscoveryOperationResponse represents the response from the PATCH api_gateway/discovery/operations/{id} endpoint.
type APIShieldPatchDiscoveryOperationResponse struct {
Result PatchAPIShieldDiscoveryOperation `json:"result"`
Response
}

// APIShieldPatchDiscoveryOperationsResponse represents the response from the PATCH api_gateway/discovery/operations endpoint.
type APIShieldPatchDiscoveryOperationsResponse struct {
Result PatchAPIShieldDiscoveryOperationsParams `json:"result"`
Response
}

// ListAPIShieldDiscoveryOperations retrieve the most up to date view of discovered operations.
//
// API documentation: https://developers.cloudflare.com/api/operations/api-shield-api-discovery-retrieve-discovered-operations-on-a-zone
func (api *API) ListAPIShieldDiscoveryOperations(ctx context.Context, rc *ResourceContainer, params ListAPIShieldDiscoveryOperationsParams) ([]APIShieldDiscoveryOperation, ResultInfo, error) {
path := fmt.Sprintf("/zones/%s/api_gateway/discovery/operations", rc.Identifier)

uri := buildURI(path, params)

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

var asResponse APIShieldListDiscoveryOperationsResponse
err = json.Unmarshal(res, &asResponse)
if err != nil {
return nil, ResultInfo{}, fmt.Errorf("%s: %w", errUnmarshalError, err)
}

return asResponse.Result, asResponse.ResultInfo, nil
}

// PatchAPIShieldDiscoveryOperation updates certain fields on a discovered operation
//
// API Documentation: https://developers.cloudflare.com/api/operations/api-shield-api-patch-discovered-operation
func (api *API) PatchAPIShieldDiscoveryOperation(ctx context.Context, rc *ResourceContainer, params PatchAPIShieldDiscoveryOperationParams) (*PatchAPIShieldDiscoveryOperation, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of Patch, we prefer to use Update unless the HTTP method is important context to have for the caller. more details in https://github.com/cloudflare/cloudflare-go/blob/master/docs/experimental.md#consistent-crud-method-signatures

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense to me - thanks!

if params.OperationID == "" {
return nil, fmt.Errorf("params.OperationID must be provided")
}

uri := fmt.Sprintf("/zones/%s/api_gateway/discovery/operations/%s", rc.Identifier, params.OperationID)

res, err := api.makeRequestContext(ctx, http.MethodPatch, uri, params)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check: Does the .makeRequestContext function add the respective request body and corresponding headers?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it know that it should only add the state field and not the operation ID to the request body?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or should we rather pass the UpdateAPIShieldDiscoveryOperation type here?

Copy link
Contributor Author

@djhworld djhworld Oct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the params (of type UpdateAPIShieldDiscoveryOperationParams) is what gets put in the body.

The params struct is serialized to JSON here

cloudflare-go/cloudflare.go

Lines 222 to 227 in 217beeb

var jsonBody []byte
jsonBody, err = json.Marshal(params)
if err != nil {
return nil, fmt.Errorf("error marshalling params to JSON: %w", err)
}
reqBody = bytes.NewReader(jsonBody)

and the content-type header is set to application/json if none is set here

cloudflare-go/cloudflare.go

Lines 382 to 384 in 217beeb

if req.Header.Get("Content-Type") == "" {
req.Header.Set("Content-Type", "application/json")
}

The operation ID does not get serialized as we set the json tag to - https://github.com/cloudflare/cloudflare-go/pull/1413/files#diff-a8b0a1c475237e4f8ba6786369d22fa9008203858112922a8a8c1426ce423e6dR60

if err != nil {
return nil, err
}

// Result should be the updated schema that was patched
var asResponse APIShieldPatchDiscoveryOperationResponse
err = json.Unmarshal(res, &asResponse)
if err != nil {
return nil, fmt.Errorf("%s: %w", errUnmarshalError, err)
}

return &asResponse.Result, nil
}

// PatchAPIShieldDiscoveryOperations bulk updates certain fields on multiple discovered operations
//
// API documentation: https://developers.cloudflare.com/api/operations/api-shield-api-patch-discovered-operations
func (api *API) PatchAPIShieldDiscoveryOperations(ctx context.Context, rc *ResourceContainer, params PatchAPIShieldDiscoveryOperationsParams) (*PatchAPIShieldDiscoveryOperationsParams, error) {
jacobbednarz marked this conversation as resolved.
Show resolved Hide resolved
uri := fmt.Sprintf("/zones/%s/api_gateway/discovery/operations", rc.Identifier)

res, err := api.makeRequestContext(ctx, http.MethodPatch, uri, params)
if err != nil {
return nil, err
}

// Result should be the updated schema that was patched
var asResponse APIShieldPatchDiscoveryOperationsResponse
err = json.Unmarshal(res, &asResponse)
if err != nil {
return nil, fmt.Errorf("%s: %w", errUnmarshalError, err)
}

return &asResponse.Result, nil
}