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

feat(automation_tokens): Add support for automation-tokens #547

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

Jesse-Hills
Copy link
Contributor

This PR aims to add support for Automation Tokens, as requested in #474.

There are a few things worth mentioning about this implementation, mostly due to the Automation Tokens docs being inaccurate.

  • DELETE doesn't work for the /automation-tokens endpoint. The examples don't work as documented, and inspecting requests in the web UI using developer tools shows the web UI using the /tokens endpoint, even for automation tokens. (it appears to be an issue with the endpoint expecting a customer_id in the request, but there is no documentation on it being required or how to pass it in the request)
  • The request/response data is inconsistent with the documentation and what is actually expected/returned from the API, some appear to return some form of JSON:API, while others return regular JSON.
  • /automation-tokens/{id}/services is supposed to return a list of services associated with the provided token id, also doesn't appear to work, so I haven't included it in this PR, but the same can be achieved using GetAutomationToken and accessing the Services field of the returned AutomationToken.

@kpfleming
Copy link
Contributor

This is not a review yet (although I'll assign myself), but all of those documentation problems should be addressed in the api-documentation repository, because they mean that our generated API clients are also broken since they are generated from those documented schemas.

@kpfleming kpfleming self-requested a review September 23, 2024 18:56
Copy link
Contributor

@kpfleming kpfleming left a comment

Choose a reason for hiding this comment

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

Just some minor doc comments.

fastly/automation_token.go Outdated Show resolved Hide resolved
fastly/automation_token.go Show resolved Hide resolved
@kpfleming kpfleming merged commit 8283d41 into fastly:main Sep 24, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants