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

fix(client): ensure urls have trailing slashes #361

Merged
merged 5 commits into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions _about/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,14 @@ A helper function named `validateCloudEndpoint` in the `internal/client` package
to validate that a `workspace_id` is configured, and provide an informative error if not.

If the API endpoint does not require a `workspace_id`, such as `accounts`, you can omit this helper function.

### API route considerations

Certain API routes require a traling slash (`/`). These are most often for `POST` methods used
for creating resources.

If the trailing slash is not provided, it can lead to errors such as 404, 405, 307, etc.

A helper script is available in `../scripts/trailing-slash-routes`. Running this will produce JSON output that
lists which routes end with a slash, along with the method and description to more easily identify which functions
to check under `../internal/client/*.go`.
2 changes: 1 addition & 1 deletion internal/client/deployments.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (c *Client) Deployments(accountID uuid.UUID, workspaceID uuid.UUID) (api.De
func (c *DeploymentsClient) Create(ctx context.Context, data api.DeploymentCreate) (*api.Deployment, error) {
cfg := requestConfig{
method: http.MethodPost,
url: c.routePrefix,
url: c.routePrefix + "/",
body: &data,
apiKey: c.apiKey,
successCodes: successCodesStatusCreated,
Expand Down
2 changes: 1 addition & 1 deletion internal/client/flows.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (c *Client) Flows(accountID uuid.UUID, workspaceID uuid.UUID) (api.FlowsCli
func (c *FlowsClient) Create(ctx context.Context, data api.FlowCreate) (*api.Flow, error) {
cfg := requestConfig{
method: http.MethodPost,
url: c.routePrefix,
url: c.routePrefix + "/",
body: &data,
apiKey: c.apiKey,
successCodes: successCodesStatusCreated,
Expand Down
2 changes: 1 addition & 1 deletion internal/client/workspace_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (c *Client) WorkspaceRoles(accountID uuid.UUID) (api.WorkspaceRolesClient,
func (c *WorkspaceRolesClient) Create(ctx context.Context, data api.WorkspaceRoleUpsert) (*api.WorkspaceRole, error) {
cfg := requestConfig{
method: http.MethodPost,
url: c.routePrefix,
url: c.routePrefix + "/",
body: &data,
apiKey: c.apiKey,
successCodes: successCodesStatusCreated,
Expand Down
2 changes: 1 addition & 1 deletion internal/client/workspaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (c *Client) Workspaces(accountID uuid.UUID) (api.WorkspacesClient, error) {
func (c *WorkspacesClient) Create(ctx context.Context, data api.WorkspaceCreate) (*api.Workspace, error) {
cfg := requestConfig{
method: http.MethodPost,
url: c.routePrefix,
url: c.routePrefix + "/",
body: &data,
apiKey: c.apiKey,
successCodes: successCodesStatusCreated,
Expand Down
36 changes: 36 additions & 0 deletions scripts/trailing-slash-routes
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#!/usr/bin/env bash

# This script checks for all endpoints that require a trailing slash.
# Files in internal/client/*.go should be checked to confirm that any
# `url` fields in the request configuration include the trailing slash
# when needed.
#
# This helps avoid errors like https://github.com/PrefectHQ/terraform-provider-prefect/issues/359.
#
# It can be helpful to redirect the output to a file for further analysis:
#
# ./scripts/trailing-slash-routes > trailing-slash-routes.json
#
# You can then use tools like `jq` to parse the results.
#
# Example output:
#
# [
# {
# "path": "/api/accounts/{account_id}/teams/",
# "method": "post",
# "desc": "Create or update a team's metadata.\n\nRequired account permissions: `create:team`"
# },
# {
# "path": "/api/accounts/{account_id}/bots/",
# "method": "post",
# "desc": "Create a Bot\n\nRequired account permissions: `create:bot`"
# },
# ...
# }

curl -s https://api.prefect.cloud/api/openapi.json |
jq '.paths
| to_entries
| map({path: .key, method: (.value | to_entries[0].key), desc: (.value | to_entries[0].value.description)})
| map(select(.path | test("/$")))'
Comment on lines +32 to +36
Copy link
Contributor

Choose a reason for hiding this comment

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

this is great! Is there anywhere in our development guides that we should add a note/reference to this to point community contributors to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea, I'll add a note in the Contributing guide 👌

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 38edad2

Loading