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.

Sign up for GitHub

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

Jump to bottom

Add support for Get/Post/Delete operations in API Shield Endpoint Management #1397

Merged
merged 4 commits into from
Sep 15, 2023
Merged

Add support for Get/Post/Delete operations in API Shield Endpoint Management #1397

merged 4 commits into from
Sep 15, 2023

Conversation

djhworld
Copy link
Contributor

@djhworld djhworld commented Sep 12, 2023 •
edited by jacobbednarz
Loading

This change adds support for the following API Shield related endpoints related to endpoint management:

Description

This change adds support for API Shield related endpoints related to managing operations in API Shield Endpoint Management. This is so they can be used from the library and will enable this feature to be added to terraform in the future.

Has your change been tested?

  • Unit tests
  • Local testing using API

Types of changes

What sort of change does your code introduce/modify?

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • This change is using publicly documented in cloudflare/api-schemas
    and relies on stable APIs.

Supersedes #1054

@github-actions
Copy link
Contributor

github-actions bot commented Sep 12, 2023 •
edited
Loading

changelog detected ✅

@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2023 •
edited
Loading

Codecov Report

Merging #1397 (2c4380c) into master (b9ac804) will increase coverage by 0.12%.
Report is 279 commits behind head on master.
The diff coverage is 50.93%.

@@            Coverage Diff             @@
##           master    #1397      +/-   ##
==========================================
+ Coverage   48.33%   48.45%   +0.12%     
==========================================
  Files         133      141       +8     
  Lines       13023    13878     +855     
==========================================
+ Hits         6295     6725     +430     
- Misses       5201     5486     +285     
- Partials     1527     1667     +140
Files Changed Coverage Δ
access_audit_log.go 79.31% <ø> (ø)
access_bookmark.go 72.44% <ø> (ø)
access_keys.go 71.42% <ø> (ø)
access_organization.go 53.84% <ø> (ø)
access_service_tokens.go 51.85% <ø> (ø)
account_members.go 65.54% <ø> (ø)
account_roles.go 53.84% <ø> (ø)
accounts.go 50.81% <ø> (ø)
addressing_address_map.go 39.04% <ø> (ø)
addressing_ip_prefix.go 45.45% <ø> (ø)
... and 123 more

... and 1 file with indirect coverage changes

@djhworld djhworld force-pushed the dharper/APISHI-2353 branch 3 times, most recently from 102f406 to f61df55 Compare September 12, 2023 17:46
@djhworld djhworld changed the title Add cloudflare_api_shield_operation resource to cloudflare-go Add support for Get/Post/Delete operations in API Shield Endpoint Management Sep 12, 2023
@djhworld djhworld marked this pull request as ready for review September 13, 2023 08:42
maciej
maciej reviewed Sep 13, 2023
View reviewed changes
Copy link

@maciej maciej left a comment

Choose a reason for hiding this comment

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

lgtm

@djhworld djhworld mentioned this pull request Sep 13, 2023
Merged
jacobbednarz
jacobbednarz reviewed Sep 14, 2023
View reviewed changes
api_shield_operations.go Outdated
)

// APIShieldCreateOperation should be used when creating an operation in API Shield Endpoint Management.
type APIShieldCreateOperation struct {
Copy link
Member

Choose a reason for hiding this comment

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

check out the conventions for naming CRUD operation params - https://github.com/cloudflare/cloudflare-go/blob/master/docs/conventions.md#methods.

Copy link
Contributor Author

@djhworld djhworld Sep 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

fixed in 2c4380c

Copy link
Contributor Author

@djhworld djhworld Sep 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

note this struct is used as part of CreateAPIShieldOperationsParams - it's basically what the user is expected to set when creating an operation, the CreateAPIShieldOperationsParams accepts a slice of these.

I've update the name of this to APIShieldBasicOperation to (hopefully?) make this clearer

@jacobbednarz
Copy link
Member

solid first pass -- only some conventions/consistency based updates here to get it inline with the existing library.

thanks!

hc2116
hc2116 reviewed Sep 14, 2023
View reviewed changes
Copy link

@hc2116 hc2116 left a comment

Choose a reason for hiding this comment

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

The logic itself looks good to me, cannot comment on the integration of this to cloudflare-go

@djhworld
Copy link
Contributor Author

djhworld commented Sep 14, 2023 •
edited
Loading

@jacobbednarz have resolved the comments, fixes are in this commit 2c4380c

@djhworld
Update Get/Post/Delete operations in API Shield Endpoint Management t…
2c4380c 
…o follow library conventions

- Using `github.com/goccy/go-json` over `encoding/json`
- Renamed `*GetOperations` to `*ListOperations`
- Changed methods to accept 3 parameters only, and used method specific
  `*Params` structs
- Renamed `*Param` structs to be paired with the methods they are
  associated with (e.g. `GetAPIShieldOperationParam`)
- Use `buildURI` method + added struct tags so query parameters are
  encoded using go-queryparams (removed custom `Encode` methods)
@jacobbednarz jacobbednarz merged commit 286b9b6 into cloudflare:master Sep 15, 2023
@github-actions github-actions bot added this to the v0.78.0 milestone Sep 15, 2023
@jacobbednarz
Copy link
Member

thanks @djhworld 👏 you rock.

@github-actions
Copy link
Contributor

This functionality has been released in v0.78.0.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hc2116 hc2116 hc2116 left review comments

@jacobbednarz jacobbednarz jacobbednarz left review comments

@maciej maciej maciej left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.78.0
Development

Successfully merging this pull request may close these issues.
5 participants
@djhworld @codecov-commenter @jacobbednarz @maciej @hc2116