feat: Add support for enterprise audit log streaming API#4035
feat: Add support for enterprise audit log streaming API#4035amreshh wants to merge 6 commits intogoogle:masterfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
4881578 to
c903947
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4035 +/- ##
==========================================
- Coverage 94.08% 93.53% -0.56%
==========================================
Files 207 209 +2
Lines 19217 19403 +186
==========================================
+ Hits 18081 18149 +68
- Misses 938 1056 +118
Partials 198 198 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
584acab to
b4904a1
Compare
Signed-off-by: amreshh <20923769+amreshh@users.noreply.github.com>
…i operations updated
alexandear
left a comment
There was a problem hiding this comment.
Is it possible to tidy up the PR's description? See https://github.com/google/go-github/blob/master/CONTRIBUTING.md#tips for tips.
The verification plan is not complete. You need to run ./script/lint.sh and ./script/test.sh according to https://github.com/google/go-github/blob/master/CONTRIBUTING.md#submitting-a-patch
- fixed linter issues where api docs were not correct in the comments for audit log streams
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @amreshh!
One minor nit and a question, otherwise LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
cc: @alexandear
| // AuditLogStreamVendorConfig is a marker interface for vendor-specific audit log | ||
| // stream configurations. |
There was a problem hiding this comment.
This interface immediately looked odd to me because you are not exporting the one interface method, however below it looks like this was absolutely intensional, and therefore should be noted in the comment.
But then I'm left wondering - would it be beneficial for a user of this client library to provide their own method? I don't know.
In the meantime, how about this?
| // AuditLogStreamVendorConfig is a marker interface for vendor-specific audit log | |
| // stream configurations. | |
| // AuditLogStreamVendorConfig is a sealed marker interface for vendor-specific audit log | |
| // stream configurations. Only this package can define implementations. |
There was a problem hiding this comment.
@gmlewis: Yes that one interface is sealed on purpose, I thought to do it this way to catch any invalid vendor configs early on, and mimic the api strictly to what they support.
I could also export it but still you are dependent on what the GitHub api provides, so any other input would still result in an error from the api which needs to be handled.
There was a problem hiding this comment.
OK, perfect... then I think we should document this fact somehow. If you want to come up with your own wording, that is fine by me.
| KeyID *string `json:"key_id,omitempty"` | ||
| } | ||
|
|
||
| // Implement the marker interface for all vendor config types. |
There was a problem hiding this comment.
| // Implement the marker interface for all vendor config types. | |
| // Implement the sealed marker interface for all vendor config types. |
|
Also, @amreshh - if you could add unit tests to cover all the new
|

This PR implements the enterprise audit logs api (https://docs.github.com/en/enterprise-cloud@latest/rest/enterprise-admin/audit-log?apiVersion=2022-11-28)
This includes:
Besides the api implementation, I also added examples/auditlogstream/main.go run a simple real world test for creating a audit log stream towards Azure Blob Storage.
Closes #3663