feat: Add support for enterprise audit log streaming API#4035
feat: Add support for enterprise audit log streaming API#4035amreshh wants to merge 18 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.64% -0.45%
==========================================
Files 207 209 +2
Lines 19217 19403 +186
==========================================
+ Hits 18081 18169 +88
- Misses 938 1036 +98
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
|
Also, @amreshh - if you could add unit tests to cover all the new
|
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
- Added testcases for each vendor config TestNewAzureHubStreamConfig TestNewAmazonS3OIDCStreamConfig TestNewAmazonAccessKeysStreamConfig TestNewSplunkStreamConfig TestNewHecStreamConfig TestNewGoogleCloudStreamConfig
|
@gmlewis : Good catch about the test cases, added those as well. |
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @amreshh!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
cc: @alexandear since you already commented.
| } | ||
| } | ||
|
|
||
| func runCreate(args []string) { |
There was a problem hiding this comment.
Both runCreate and runDelete duplicate the common setup:
- Reading GITHUB_AUTH_TOKEN and GITHUB_API_URL environment variables
- Creating client
- Defining and validating the
enterpriseflag
I suggest extracting common login into a helper function.
| // AzureBlobConfig represents vendor-specific config for Azure Blob Storage. | ||
| type AzureBlobConfig struct { | ||
| KeyID *string `json:"key_id,omitempty"` | ||
| EncryptedSasURL *string `json:"encrypted_sas_url,omitempty"` |
There was a problem hiding this comment.
I propose rename EncryptedSasURL to EncryptedSASURL
There was a problem hiding this comment.
@alexandear I tried renaming it, but then the linter complains.
Should we add SAS to the initialisms or leave the name as EncryptedSasURL
❯ script/lint.sh
linting .
github/enterprise_audit_log_stream.go:46:2: change Go field name "EncryptedSASURL" to "EncryptedSasURL" for json tag "encrypted_sas_url" in struct "AzureBlobConfig" (structfield)
EncryptedSASURL string `json:"encrypted_sas_url"`
^
There was a problem hiding this comment.
@alexandear I tried renaming it, but then the linter complains. Should we add
SASto the initialisms or leave the name asEncryptedSasURL❯ script/lint.sh linting . github/enterprise_audit_log_stream.go:46:2: change Go field name "EncryptedSASURL" to "EncryptedSasURL" for json tag "encrypted_sas_url" in struct "AzureBlobConfig" (structfield) EncryptedSASURL string `json:"encrypted_sas_url"` ^
Ah! We need to update the linter to add "SAS" as a common acronym. I'll update it in a separate PR, merge to master, then you can merge master into this repo. Sorry about that, @amreshh! One moment...
There was a problem hiding this comment.
OK, @amreshh - #4054 has been merged to master. Please switch to master, then git pull then switch back to your PR branch and git merge master into it. You will need to remove ./bin/* in this repo, then rerun your step 4 scripts, and you should be good now. Please let me know if you have any problems.
There was a problem hiding this comment.
Please fix all fields in *Config structs by removing omitempty for required in schema.
| func (*GoogleCloudConfig) isAuditLogStreamVendorConfig() {} | ||
| func (*DatadogConfig) isAuditLogStreamVendorConfig() {} | ||
|
|
||
| // Helper constructors for AuditLogStreamConfig. |
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>

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