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

Support for Service Authorizations #349

Merged
merged 9 commits into from
Jun 20, 2022

Conversation

noseglid
Copy link
Contributor

When a user has the "engineer" level, specific access to specific
services can be granted using service authorizations.

This change adds support for creating, retrieving, updating and deleting
service authorizations using the Go client.

@noseglid noseglid force-pushed the service-authorization branch 2 times, most recently from cd50287 to 6b72cc6 Compare June 15, 2022 13:16
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

Hi @noseglid 👋🏻

Thank you for opening this PR. It's really appreciated.

For the sake of consistency with the rest of the code base I've requested some changes to help keep the code aligned.

fastly/service_authorization.go Outdated Show resolved Hide resolved
fastly/service_authorization.go Outdated Show resolved Hide resolved
fastly/service_authorization_test.go Outdated Show resolved Hide resolved
fastly/fastly_test.go Outdated Show resolved Hide resolved
noseglid added 2 commits June 16, 2022 16:56
When a user has the "engineer" level, specific access to specific
services can be granted using service authorizations.

This change adds support for creating, retrieving, updating and deleting
service authorizations using the Go client.
We can use the string directly instead of creating new types for
permissions and data types as we do not require any extra functionality
on these types.
@noseglid noseglid force-pushed the service-authorization branch from 8053d3b to e172702 Compare June 16, 2022 14:56
noseglid added 2 commits June 16, 2022 17:03
Using `jsonapi` saves us from constructing a lot of extra models because
the API provided by fastly already follows the conventions outlined by
`jsonapi`.
Anonymizes the IDs used for service authorizations as well as adds a new
"default" id for user, which can be anonymized to a default User ID in
the `fix-fixtures` target of the makefile.
@noseglid noseglid force-pushed the service-authorization branch from e172702 to c508116 Compare June 16, 2022 15:03
Add Godocs to the exposed functions and structs used as inputs.
@noseglid noseglid requested a review from Integralist June 17, 2022 13:04
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

Thanks @noseglid this is looking a lot better. Just a few small comments. Thanks!

fastly/fastly_test.go Outdated Show resolved Hide resolved
fastly/service_authorization.go Outdated Show resolved Hide resolved
fastly/service_authorization.go Outdated Show resolved Hide resolved
fastly/fastly_test.go Outdated Show resolved Hide resolved
Only the serviceid needs to be default and normalized.
@noseglid noseglid force-pushed the service-authorization branch from 6eb50fb to d9bf2fa Compare June 20, 2022 09:14
Instead of re-using amibuous error messages, create and use a dedicated
error when CreateServiceAuthorization is used incorrectly.
@noseglid noseglid force-pushed the service-authorization branch from d9bf2fa to 3b516a2 Compare June 20, 2022 09:21
@noseglid
Copy link
Contributor Author

I have now addressed all your comments

Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

Made two suggested edits (which I'll apply in a moment), but otherwise this looks good to me.

Thanks @noseglid 🙂

fastly/errors.go Outdated Show resolved Hide resolved
fastly/errors.go Outdated Show resolved Hide resolved
@Integralist Integralist merged commit 2c276cc into fastly:main Jun 20, 2022
@noseglid noseglid deleted the service-authorization branch June 20, 2022 13:19
@noseglid
Copy link
Contributor Author

🙌

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