Skip to content
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.

Sign up for GitHub

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

configure admin endpoint as a listener/http-filter (e.g. to secure via RBAC) #11367

Closed
wants to merge 2 commits into from
Closed

configure admin endpoint as a listener/http-filter (e.g. to secure via RBAC) #11367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2aa5df9
secure admin
cstrahan May 7, 2020
4eb88e1
fix admin listener initialization
cstrahan May 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ proto_library(
"//envoy/extensions/common/tap/v3:pkg",
"//envoy/extensions/filters/common/fault/v3:pkg",
"//envoy/extensions/filters/http/adaptive_concurrency/v3:pkg",
"//envoy/extensions/filters/http/admin/v3:pkg",
"//envoy/extensions/filters/http/aws_lambda/v3:pkg",
"//envoy/extensions/filters/http/aws_request_signing/v3:pkg",
"//envoy/extensions/filters/http/buffer/v3:pkg",
Expand Down
4 changes: 4 additions & 0 deletions api/envoy/config/bootstrap/v3/bootstrap.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ message Bootstrap {

// Administration interface :ref:`operations documentation
// <operations_admin_interface>`.
// [#next-free-field: 6]
message Admin {
option (udpa.annotations.versioning).previous_message_type = "envoy.config.bootstrap.v2.Admin";

Expand All @@ -200,6 +201,9 @@ message Admin {
// Additional socket options that may not be present in Envoy source code or
// precompiled binaries.
repeated core.v3.SocketOption socket_options = 4;

// Static :ref:`Listener <envoy_api_msg_config.listener.v3.Listener>`.
listener.v3.Listener listener = 5;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question I have on this API is whether we should have a Listener embedded in admin or somehow provide an admin filter that can be hooked into an arbitrary listener? I suspect what you have is the right thing given how admin works today, since it probably wants to be running on the main thread for thread safety with some of the data structures it serves. CC @envoyproxy/api-shepherds

However, by placing a listener here, it's providing a bigger "hole" than is needed. Listeners can be configured to do lots of things, e.g. act as a Thrift proxy, and only a narrow slice of this configuration will apply. Can you document how Listener is intended to be used when configured inside the Admin message?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My gut reaction is this config is right, but we should document the restrictions of the listener config which can be checked in code. I had discussed this offline with @cstrahan. So yeah +1 for documentation on the restrictions of the listener object and all of the check we should do in code on the config.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd rather see it as a normal http filter attached to a normal listener. For things that must happen on the main thread, have the filter send a message back and forth for that work.

I'm worried that running arbitrary code on the main thread will expose issues because nobody intended all the non-admin filters or listener code to run on the main thread.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For things that must happen on the main thread, have the filter send a message back and forth for that work.

I think this will likely end up being a really huge effort, though it's technically possible for sure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way of accomplishing the same thing is to post the entire request from the worker to the main thread, and post the response back to the worker. So leave everything done by the current admin endpoint on the main thread, and leave all the normal listener and filter-chain stuff on the worker thread.

}

// Cluster manager :ref:`architecture overview <arch_overview_cluster_manager>`.
Expand Down
4 changes: 4 additions & 0 deletions api/envoy/config/bootstrap/v4alpha/bootstrap.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ message Bootstrap {

// Administration interface :ref:`operations documentation
// <operations_admin_interface>`.
// [#next-free-field: 6]
message Admin {
option (udpa.annotations.versioning).previous_message_type = "envoy.config.bootstrap.v3.Admin";

Expand All @@ -192,6 +193,9 @@ message Admin {
// Additional socket options that may not be present in Envoy source code or
// precompiled binaries.
repeated core.v4alpha.SocketOption socket_options = 4;

// Static :ref:`Listener <envoy_api_msg_config.listener.v4alpha.Listener>`.
listener.v4alpha.Listener listener = 5;
}

// Cluster manager :ref:`architecture overview <arch_overview_cluster_manager>`.
Expand Down
9 changes: 9 additions & 0 deletions api/envoy/extensions/filters/http/admin/v3/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# DO NOT EDIT. This file is generated by tools/proto_sync.py.

load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package")

licenses(["notice"]) # Apache 2

api_proto_package(
deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"],
)
22 changes: 22 additions & 0 deletions api/envoy/extensions/filters/http/admin/v3/admin.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
syntax = "proto3";

package envoy.extensions.filters.http.admin.v3;

import "google/protobuf/wrappers.proto";

import "udpa/annotations/migrate.proto";
import "udpa/annotations/status.proto";
import "udpa/annotations/versioning.proto";
import "validate/validate.proto";

option java_package = "io.envoyproxy.envoy.extensions.filters.http.admin.v3";
option java_outer_classname = "AdminProto";
option java_multiple_files = true;
option (udpa.annotations.file_status).package_version_status = ACTIVE;

// [#protodoc-title: Admin]
// Admin:ref:`configuration overview <config_http_filters_admin>`.
// [#extension: envoy.filters.http.admin]

message Admin {
}
1 change: 1 addition & 0 deletions api/versioning/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ proto_library(
"//envoy/extensions/common/tap/v3:pkg",
"//envoy/extensions/filters/common/fault/v3:pkg",
"//envoy/extensions/filters/http/adaptive_concurrency/v3:pkg",
"//envoy/extensions/filters/http/admin/v3:pkg",
"//envoy/extensions/filters/http/aws_lambda/v3:pkg",
"//envoy/extensions/filters/http/aws_request_signing/v3:pkg",
"//envoy/extensions/filters/http/buffer/v3:pkg",
Expand Down
4 changes: 4 additions & 0 deletions generated_api_shadow/envoy/config/bootstrap/v3/bootstrap.proto
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions generated_api_shadow/envoy/config/bootstrap/v4alpha/bootstrap.proto
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions generated_api_shadow/envoy/extensions/filters/http/admin/v3/BUILD
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions generated_api_shadow/envoy/extensions/filters/http/admin/v3/admin.proto
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions source/extensions/filters/http/well_known_names.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ namespace HttpFilters {
*/
class HttpFilterNameValues {
public:
// Admin filter
const std::string Admin = "envoy.filters.http.admin";
// Buffer filter
const std::string Buffer = "envoy.filters.http.buffer";
// Cache filter
Expand Down
4 changes: 4 additions & 0 deletions source/server/http/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ envoy_cc_library(
"//include/envoy/http:request_id_extension_interface",
"//include/envoy/network:filter_interface",
"//include/envoy/network:listen_socket_interface",
"//include/envoy/registry",
"//include/envoy/server:admin_interface",
"//include/envoy/server:hot_restart_interface",
"//include/envoy/server:instance_interface",
Expand Down Expand Up @@ -63,9 +64,12 @@ envoy_cc_library(
"//source/common/stats:isolated_store_lib",
"//source/common/upstream:host_utility_lib",
"//source/extensions/access_loggers/file:file_access_log_lib",
"//source/extensions/filters/http:well_known_names",
"//source/extensions/filters/http/common:factory_base_lib",
"@envoy_api//envoy/admin/v3:pkg_cc_proto",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
"@envoy_api//envoy/config/route/v3:pkg_cc_proto",
"@envoy_api//envoy/extensions/filters/http/admin/v3:pkg_cc_proto",
"@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto",
],
)
Expand Down
14 changes: 14 additions & 0 deletions source/server/http/admin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <algorithm>
#include <cstdint>
#include <fstream>
#include <memory>
#include <string>
#include <unordered_set>
#include <utility>
Expand Down Expand Up @@ -59,6 +60,19 @@
namespace Envoy {
namespace Server {

Http::FilterFactoryCb AdminFilterConfig::createFilterFactoryFromProtoTyped(
const envoy::extensions::filters::http::admin::v3::Admin&, const std::string&,
Server::Configuration::FactoryContext& context) {

return [&context](Http::FilterChainFactoryCallbacks& callbacks) -> void {
// TODO: figure out how to handle this without downcasting
callbacks.addStreamDecoderFilter(std::make_shared<AdminFilter>(
dynamic_cast<AdminImpl&>(context.admin()).createCallbackFunction()));
};
}

REGISTER_FACTORY(AdminFilterConfig, Server::Configuration::NamedHttpFilterConfigFactory);

namespace {

/**
Expand Down
21 changes: 21 additions & 0 deletions source/server/http/admin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@
#include "common/router/scoped_config_impl.h"
#include "common/stats/isolated_store_impl.h"

#include "extensions/filters/http/common/factory_base.h"
#include "extensions/filters/http/well_known_names.h"
#include "envoy/extensions/filters/http/admin/v3/admin.pb.h"
#include "envoy/extensions/filters/http/admin/v3/admin.pb.validate.h"

#include "server/http/admin_filter.h"
#include "server/http/config_tracker_impl.h"
#include "server/http/listeners_handler.h"
Expand All @@ -49,6 +54,22 @@
namespace Envoy {
namespace Server {

/**
* Config registration for the Admin filter. @see NamedHttpFilterConfigFactory.
*/
class AdminFilterConfig : public Extensions::HttpFilters::Common::FactoryBase<
envoy::extensions::filters::http::admin::v3::Admin> {
public:
AdminFilterConfig() : FactoryBase(Extensions::HttpFilters::HttpFilterNames::get().Admin) {}

private:
Http::FilterFactoryCb createFilterFactoryFromProtoTyped(
const envoy::extensions::filters::http::admin::v3::Admin& proto_config, const std::string&,
Server::Configuration::FactoryContext& context) override;

bool isTerminalFilter() override { return true; }
};

class AdminInternalAddressConfig : public Http::InternalAddressConfig {
bool isInternalAddress(const Network::Address::Instance&) const override { return false; }
};
Expand Down
7 changes: 6 additions & 1 deletion source/server/server.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,9 @@ void InstanceImpl::initialize(const Options& options,
// Learn original_start_time_ if our parent is still around to inform us of it.
restarter_.sendParentAdminShutdownRequest(original_start_time_);
admin_ = std::make_unique<AdminImpl>(initial_config.admin().profilePath(), *this);
if (initial_config.admin().address()) {
if (bootstrap_.admin().has_listener()) {
// this case is handled later on.
} else if (initial_config.admin().address()) {
if (initial_config.admin().accessLogPath().empty()) {
throw EnvoyException("An admin access log path is required for a listening server.");
}
Expand Down Expand Up @@ -455,6 +457,9 @@ void InstanceImpl::initialize(const Options& options,
// cluster_manager_factory_ is available.
config_.initialize(bootstrap_, *this, *cluster_manager_factory_);

// TODO: where should this go?
listener_manager_->addOrUpdateListener(bootstrap_.admin().listener(), "", false);

// Instruct the listener manager to create the LDS provider if needed. This must be done later
// because various items do not yet exist when the listener manager is created.
if (bootstrap_.dynamic_resources().has_lds_config()) {
Expand Down