admin: support HTML UI for admin post params#22424
admin: support HTML UI for admin post params#22424yanavlasov merged 103 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…ion. This requires a carefully defined set that does not proactively allocate and deallocate the set element. Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
… StatName, unfortunately. Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
| for (auto level_string_view : spdlog::level::level_string_views) { | ||
| response.add(fmt::format("{} ", level_string_view)); | ||
| } | ||
| Http::Code LogsHandler::handlerLogging(absl::string_view, Http::ResponseHeaderMap&, |
There was a problem hiding this comment.
Is the first parameter used for any handler, since it's now available on the AdminStream? Is this no longer needed on MAKE_ADMIN_HANDLER? Also several functions in admin.cc pass path_and_query around as arguments. Should all of these just get it from AdminStream?
There was a problem hiding this comment.
This would require AdminFilter to store the query params once parsed.
There was a problem hiding this comment.
I will remove the first param to MAKE_ADMIN_HANDLER as a follow-up -- it's a pretty noisy change so I'd like to make that in a separate PR that has no functional impact.
I'm not sure what you mean by 'store the query params' -- the API I provided is that it generates the query-params map and returns it. The source for that is the request body and request headers which contains the "path" as a synthetic header.
There was a problem hiding this comment.
done as a pending follow-up PR: #22571 -- it's not reviewable now but once this merges, that one will be non-functional so that its sprawl is easier to reason about.
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
|
Yan, can you do a senior maintainer review? Thanks! |
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
|
/retest |
|
Retrying Azure Pipelines: |
Signed-off-by: Joshua Marantz <jmarantz@google.com>
|
/retest |
|
Retrying Azure Pipelines: |
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…each code. Signed-off-by: Joshua Marantz <jmarantz@google.com>
Remove superfluous url argument from the args passed to admin callback.s That is available, if needed, from admin_stream.requestHeaders().getPathValue(). Generally it was only used for parsing query-params. Those should be obtained from admin_stream.queryParam() which will also parse query-params out of the body if they came in that way, e.g. from a POST. Risk Level: low -- this is a non-functional change that just removes arguments from an API that is no longer used as of #22424 Testing: //test/... Docs Changes: n/a Release Notes: n/a Platform Specific Features: n/a Signed-off-by: Joshua Marantz <jmarantz@google.com>
5 participants
Commit Message: follow-up to #19546 -- supports parameter declarations for admin POST endpoints. This required an infrastructural improvement to generate queryParams from the AdminStream, which has access to request body, as POST params from HTML form show up as POSTs params.
That in turn required that we ensure that request_headers_->getPathValue() be consistent with the path in test flows, which needed to be sorted out in the test infrastructure. Added an ASSERT to ensure they stay that way.
Additional Description:
Risk Level: low -- incremental change to support POST admin params.
Testing: //test/..., and manually ran server to verify html post behavior, fixing a couple of cases where the HTML controls didn't pass through params as expected by the handlers.
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a