-
Notifications
You must be signed in to change notification settings - Fork 13
wip: support asm #695
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
base: main
Are you sure you want to change the base?
wip: support asm #695
Conversation
a93c4e8
to
2ec14e9
Compare
main proxy code
its not used
also added some small configs like `appsec_rules` and `appsec_waf_timeout`
so it can use decide whether to act on appsec
346a130
to
ba7a1da
Compare
0e3ae0b
to
ba7a1da
Compare
97a4a9e
to
e0902ba
Compare
a177439
to
cd8e2cf
Compare
async fn extract(self) -> HttpData; | ||
} | ||
|
||
pub(super) async fn extract_request_address_data(body: &Bytes) -> Option<(HttpData, RequestType)> { |
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.
I remember you mentioned maybe we want to eventually consolidate this with the triggers – overall, great
Deserializing into type might be slow since payloads can technically be up to 6MB
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.
Ah, next file is doing a similar check from triggers code
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.
I see that specific logic, as you mentioned privately, was ported from Go, maybe for the ones that already exist here in the triggers, could we use that same logic and for the events that we don't cover for inferred spans, I.E. Kong, we maintain the Go port?
for (key, value) in appsec_context.tags() { | ||
self.invocation_span | ||
.meta | ||
.entry(key.clone()) | ||
.or_insert(value.clone()); | ||
} |
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.
Can't we use extend
here?
|
||
// Note: We intentionally don't set `actor.ip` because we don't have a definitive signal here. | ||
|
||
//TODO(romain.marcadier): Figure out whether we can set "_dd.runtime_family" here; and to what value. |
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.
if there is a set of valid values, we can talk about how and when to set it, we get the specific runtime at a later point of init, but we guarantee it will arrive
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.
My main concern is if there's a better way to tie contexts, I won't know until I inspect the code thoroughly, but so far this looks good
if let Some(appsec_processor) = appsec_processor { | ||
if let Some(request_id) = intercepted_parts | ||
.headers | ||
.get("Lambda-Runtime-Aws-Request-Id") |
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.
I remember either in next or response we can get this by checking at the Path, we can leverage Axum for it, will check again which one was it
1a2a3d6
to
d17cc48
Compare
No description provided.