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.
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
Access Log Policy Controller #424
Access Log Policy Controller #424
Changes from all commits
9a4e9e7
833839a
309cfc5
f322ba4
a26ce02
6beaf45
6e0b988
f91f62a
6c8346a
2697b16
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a general question on the feature. How are we planning on reporting back the actual log paths? From what I recall, logs are based on unique resource ids, not names. How will folks using the controller know which logs apply to which k8s resources?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100% sure I follow, so please correct me if I'm going down the wrong path here. When a user creates a Gateway or Route, that resource is associated to 1 VPC Lattice Service Network or Service. When they create an AccessLogPolicy with a targetRef of a Gateway or Route, it's the equivalent of creating a VPC Lattice Access Log Subscription for the corresponding ServiceNetwork or Service.
So if I create Gateway
my-hotel
, which in VPC Lattice is Service Networksn-12345678901234567
, and then create an AccessLogPolicy which refers to Gatewaymy-hotel
, then it will create an AccessLogSubscription with resourceIdentifiersn-12345678901234567
, and any access logs flowing through that Service Network will wind up in the log destination I specified in the AccessLogPolicy.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. So the path in S3 would look like [1]:
[bucket]/[prefix]/AWSLogs/[accountId]/VpcLattice/AccessLogs/[region]/[YYYY/MM/DD]/[resource-id]/[accountId]_VpcLatticeAccessLogs_[region]_[resource-id]_YYYYMMDDTHHmmZ_[hash].json.gz
Let's assume I have a number of gateways I'm managing - how will I know the Lattice resource ID for each? For example, do we patch the gateway with the Lattice service network ID? I see it as a message here: https://github.com/aws/aws-application-networking-k8s/blob/main/controllers/gateway_controller.go#L279. Ideally, I would be able to find out the ID by doing something like
kubectl get gateway
.Similarly, it may be difficult to map from a k8s route to a log line, or from a log line back to the k8s objects, since these log entries also use unique IDs rather than k8s-based names. Maybe for now just enabling the logs is enough, then customers can use the Lattice API out of band to determine the ID. Longer term we should look a improving this experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Patch the status(or annotation) of
gateway
andk8sService
resource with lattice resource id and Patch AccessLogPolicy with als id seems a good approach to make mapping cleanerThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, this would absolutely improve clarity for users. I'll look into making this an annotation for ALP (don't think status is the right place for this kind of data), and will add it as a followup once the Create workflow is done. We can also create an issue to add this behaviour to other resources, like Gateway and Route.