discovery: Add AWS EKS audit log fetching for Access Graph#59568
Merged
discovery: Add AWS EKS audit log fetching for Access Graph#59568
Conversation
94a6def to
225048a
Compare
53c7cf9 to
e981c38
Compare
e981c38 to
46ec433
Compare
225048a to
ffbb3b3
Compare
8141b3e to
3e19084
Compare
7fde12d to
a492e96
Compare
15db093 to
eca596e
Compare
c359b0f to
3eba24d
Compare
c060d92 to
18680a9
Compare
juliaogris
approved these changes
Oct 21, 2025
Contributor
juliaogris
left a comment
There was a problem hiding this comment.
Still good, just a couple of little nits.
tigrato
reviewed
Oct 27, 2025
6c56bd5 to
9cc258f
Compare
tigrato
approved these changes
Nov 4, 2025
Extend the static config for Access Graph discovery to be able to specify the EKS cluster for which apiserver audit logs should be fetched and sent to Access Graph.
Add a watcher to start fetchers for all access graph EKS clusters that
are configured to have Kubernetes apiserver audit logs fetched and send
them to access graph. It receives the set of clusters to fetch audit
logs for from the AWS resource syncer as it discovers EKS clusters.
Those clusters are reconciled against the current set of log fetchers,
with no-longer-needed fetchers stopped and new fetchers started as
needed.
This commit requires go.mod be updated with:
go get github.com/aws/aws-sdk-go-v2/service/cloudwatchlogs@latest
It is left out of this commit for now as it makes rebasing/merging
master easier.
7836522 to
d5c9283
Compare
Run:
go get github.com/aws/aws-sdk-go-v2/service/cloudwatchlogs@latest
make go-mod-tidy-all
# Manually move the go.mod line back to the first section!?!?
This commit is kept separate for easier merging/rebasing.
d5c9283 to
a9eadec
Compare
r0mant
reviewed
Nov 5, 2025
Collaborator
r0mant
left a comment
There was a problem hiding this comment.
Can we get some test coverage going here?
Refactor the eksAuditLog{Watcher,Fetcher} and the aws_sync.Fetcher
cloudwatchlogs to be more testable:
* factor away eksAuditLogFetcher from eksAuditLogWatcher. The watcher
just needs a factory function to create a fetcher, and all the watcher
needs from that fetcher is a `Run()` method. Lift the cancel func out
of the watcher and store it directly in the watcher, as only the
watcher uses it.
* factor away aws_sync.Fetcher from eksAuditLogFetcher. All it needs
from the sync fetcher it calls is one method to fetch cloudwatch logs.
Make that an interface and use just that. This allows a fake source of
cloudwatch logs to be provided for testing. While here, use protobuf
getters rather than accessing fields directly.
* Use protobuf getters in aws_sync.Fetcher cloudwatchlogs instead of
accessing fields directly. In future, we could pass in an interface
with those getters to make the code more testable.
f580eef to
503a9e4
Compare
Add tests for `eksAuditLogWatcher` and `eksAuditLogFetcher`. Copy the grpc stream testing util from the access graph repo into teleport as it is useful for the bidirectional streaming methods uses by access graph, and makes it easier to test on the client side.
503a9e4 to
a2f99a2
Compare
Contributor
Author
@r0mant PTAL. I've added tests for the watcher and fetcher. There are not many existing tests around the access graph sync stuff which really feels like it needs a bit of refactoring to be more testable. The overlap between Teleport discovery and access graph discovery also makes this all a bit confusing, but I feel cleaning this up is too much for this PR. |
r0mant
approved these changes
Nov 11, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extend the static config for Access Graph discovery to be able to
specify the EKS cluster for which apiserver audit logs should be fetched
and sent to Access Graph.
Extend the Access Graph discovery resource syncer to identify clusters
that should have their apiserver audit logs fetched based on discovery
configuration. Send the list of clusters over a channel for a log
fetcher to receive so it can fetch the logs and send them to access
graph.
Add a watcher to start fetchers for all access graph EKS clusters that
are configured to have Kubernetes apiserver audit logs fetched and send
them to access graph. This receives the list of clusters from the resource
discovery syncer and reconciles it against the clusters currently having
their logs fetched, starting and stopping logs fetchers as required.
Issue: https://github.com/gravitational/access-graph/issues/1589
Note: This PR is split into logical commits that can be reviewed separately for
easier reviewing.