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.

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

repo reorg: move ext auth filters #2923

Merged
merged 7 commits into from
Mar 28, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
88 changes: 88 additions & 0 deletions REPO_LAYOUT.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# Repository layout overview

This is a high level overview of how the repository is laid out to both aid in code investigation,
as well as to clearly specify how extensions are added to the repository. The top level directories
are:

* [.circleci/](.circleci/): Configuration for [CircleCI](https://circleci.com/gh/envoyproxy).
* [bazel/](bazel/): Configuration for Envoy's use of [Bazel](https://bazel.build/).
* [ci/](ci/): Scripts used both during CI as well as to build Docker containers.
* [configs/](configs/): Example Envoy configurations.
* [docs/](docs/): Project level documentation is well as scripts for publishing final docs during
Copy link
Member

Choose a reason for hiding this comment

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

Nit: s/is well as/as well as/

Copy link
Contributor

@alyssawilk alyssawilk Mar 28, 2018

Choose a reason for hiding this comment

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

pinging this one. otherwise LGTM

Copy link
Member Author

Choose a reason for hiding this comment

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

oops sorry missed it

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

releases.
* [examples/](examples/): Larger Envoy examples using Docker and Docker Compose.
* [include/](include/): "Public" interface headers for "core" Envoy (not extensions). In general,
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a bit confusingly phrased, given it includes the interfaces for major extensions, i.e. tcp proxying, http stream etc.

these are almost entirely 100% abstract classes. There are a few cases of not-abstract classes in
the "public" headers, typically for performance reasons.
* [restarter/](restarter/): Envoy's hot restart wrapper Python script.
* [source/](source/): Source code for core Envoy as well as extensions. The layout of this directory
is discussed in further detail below.
* [support/](support/): Development support scripts (pre-commit Git hooks, etc.)
* [test/](test/): Test code for core Envoy as well as extensions. The layout of this directory is
discussed in further detail below.
* [tools/](tools/): Miscellaneous tools that have not found a home somewhere else.

## [source/](source/)

* [common/](source/[common/): Core Envoy code (not specific to extensions) that is also not
Copy link
Member

Choose a reason for hiding this comment

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

Path typo?

specific to a standalone server implementation. I.e., this is code that could be used if Envoy
were eventually embedded as a library.
* [docs/](source/docs/): Miscellaneous developer/design documentation that is not relevant for
the public user documentation.
* [exe/](source/exe/): Code specific to building the final production Envoy server binary. This is
the only code that is not shared by integration and unit tests.
* [extensions/](source/extensions/): Extensions to the core Envoy code. The layout of this
directory is discussed in further detail below.
* [server/](source/server/): Code specific to running Envoy as a standalone server. E.g,
configuration, server startup, workers, etc. Overtime, the line between `common/` and `server/`
Copy link
Member

Choose a reason for hiding this comment

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

Nit: "over time"

has become somewhat blurred. Use best judgment as to where to place something.

## [test/](test/)

Not every directory within test is described below, but a few highlights:

* Unit tests are found in directories matching their [source/](source/) equivalents. E.g.,
[common/](test/common/), [exe/](test/exe/), and [server/](test/server/).
* Extension unit tests also match their source equivalents in [extensions/](test/extensions/).
* [integration/](test/integration/) holds end-to-end integration tests using roughly the real
Envoy server code, fake downstream clients, and fake upstream servers. Integration tests also
test some of the extensions found in the repository.
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem ideal, aren't we going to push these into the extension specific dirs?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was my plan originally but @alyssawilk objected. Or maybe I misunderstood the objection @alyssawilk?

Copy link
Contributor

Choose a reason for hiding this comment

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

We may - I'd asked we defer that until after the filter moves as I think there's a bunch of code which tests various filters, or tests core components (flow control) using extensions (various filters which buffer).

It's pretty non-trivial to tease these out.

Copy link
Contributor

@alyssawilk alyssawilk Mar 28, 2018

Choose a reason for hiding this comment

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

Also, we've been organizing our HTTP tests around codec so you can "easily" add a cors test for HTTP/2, HTTP, HTTP/2 upstream etc. I think we need an integration test rewrite to make it easier to have "the cors filter test" which runs the test for the varions combinations and permutations, rather than having 3 integration tests per filter which would be really crummy from a scaling perspective. I think if we do that refactor we could encourage folks to add new integration tests under their filter directories, and move the easy-to-tease-apart tests to their directories for reference implementations. Still not sure how we want to handle testing core features which require filters without implementing a bunch of fake test filters (which we could totally do, but don't have yet) so I think this will be a series of TODOs and follow-up PRs.

In the interim we could move the existing tests to http/common and network/common if we think it's worth an interim move.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I agree that more thinking is needed here. I would prefer that we not let "perfect be the enemy of the good" on this so I will add some aspirational text on where we would like this to go.

* [mocks/](test/mocks/) contains mock implementations of all of the core Envoy interfaces found in
[include/](include/).
* Other directories include tooling used for configuration testing, coverage testing, fuzz testing,
common test code, etc.

## [source/extensions](source/extensions/) layout

We maintain a very specific code and namespace layout for extensions. This aids in discovering
code/extensions, and also will allow us in the future to more easily scale out our extension
maintainers by having OWNERS files specific to certain extensions. (As of this writing, this is not
currently implemented but that is the plan moving forward.)

* All extensions are registered in [all_extensions.bzl](source/extensions/all_extensions.bzl). In
the future this mechanism will easily allow us to compile out extensions based on build system
configuration. This is not currently implemented but is the plan moving forward.
* These are the top level extension directories and associated namespaces:
* [access_loggers/](/source/extensions/access_loggers): Access log implementations which use
the `Envoy::Extensions::AccessLoggers` namespace.
* [http_tracers/](/source/extensions/http_tracers): HTTP tracers which use the
`Envoy::Extensions::HttpTracers` namespace.
* [filters/http/](/source/extensions/filters/http): HTTP L7 filters which use the
`Envoy::Extensions::HttpFilters` namespace.
* [filters/listener/](/source/extensions/filters/listener): Listener filters which use the
`Envoy::Extensions::ListenerFilters` namespace.
* [filters/network/](/source/extensions/filters/network): L4 network filters which use the
`Envoy::Extensions::NetworkFilters` namespace.
* [resolvers/](/source/extensions/resolvers): Network address resolvers which use the
`Envoy::Extensions::Resolvers` namespace.
* [stat_sinks/](/source/extensions/stat_sinks): Stat sink implementations which use the
`Envoy::Extensions::StatSinks` namespace.
* [transport_sockets/](/source/extensions/transport_sockets): Transport socket implementations
which use the `Envoy::Extensions::TransportSockets` namespace.
* Each extension is contained wholly in its own namespace. E.g.,
`Envoy::Extensions::NetworkFilters::Echo`.
* Common code that is used by multiple extensions should be in a `common/` directory as close to
the extensions as possible. E.g., [filters/common/](/source/extensions/filters/common) for common
code that is used by both HTTP and network filters. Common code used only by two HTTP filters
would be found in `filters/http/common/`. Common code should be placed in a common namespace.
E.g., `Envoy::Extensions::Filters::Common`.
4 changes: 4 additions & 0 deletions STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
The following section covers the major areas where we deviate from the Google
guidelines.

# Repository file layout

* Please see [REPO_LAYOUT.md](REPO_LAYOUT.md).

# Deviations from Google C++ style guidelines

* Exceptions are allowed and encouraged where appropriate. When using exceptions, do not add
Expand Down
18 changes: 0 additions & 18 deletions include/envoy/ext_authz/BUILD

This file was deleted.

18 changes: 0 additions & 18 deletions source/common/filter/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,3 @@ envoy_cc_library(
"@envoy_api//envoy/config/filter/network/rate_limit/v2:rate_limit_cc",
],
)

envoy_cc_library(
name = "ext_authz_lib",
srcs = ["ext_authz.cc"],
hdrs = ["ext_authz.h"],
deps = [
"//include/envoy/ext_authz:ext_authz_interface",
"//include/envoy/network:connection_interface",
"//include/envoy/network:filter_interface",
"//include/envoy/runtime:runtime_interface",
"//include/envoy/stats:stats_macros",
"//include/envoy/upstream:cluster_manager_interface",
"//source/common/common:assert_lib",
"//source/common/ext_authz:ext_authz_lib",
"//source/common/tracing:http_tracer_lib",
"@envoy_api//envoy/config/filter/network/ext_authz/v2:ext_authz_cc",
],
)
34 changes: 0 additions & 34 deletions source/common/http/filter/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -151,37 +151,3 @@ envoy_cc_library(
"@envoy_api//envoy/config/filter/http/rate_limit/v2:rate_limit_cc",
],
)

envoy_cc_library(
name = "ext_authz_lib",
srcs = ["ext_authz.cc"],
deps = [
":ext_authz_includes",
"//include/envoy/http:codes_interface",
"//source/common/common:assert_lib",
"//source/common/common:empty_string",
"//source/common/common:enum_to_int",
"//source/common/ext_authz:ext_authz_lib",
"//source/common/http:codes_lib",
"//source/common/router:config_lib",
],
)

envoy_cc_library(
name = "ext_authz_includes",
hdrs = ["ext_authz.h"],
deps = [
"//include/envoy/access_log:access_log_interface",
"//include/envoy/ext_authz:ext_authz_interface",
"//include/envoy/http:filter_interface",
"//include/envoy/local_info:local_info_interface",
"//include/envoy/runtime:runtime_interface",
"//include/envoy/upstream:cluster_manager_interface",
"//source/common/common:assert_lib",
"//source/common/http:header_map_lib",
"//source/common/json:config_schemas_lib",
"//source/common/json:json_loader_lib",
"//source/common/json:json_validator_lib",
"@envoy_api//envoy/config/filter/http/ext_authz/v2:ext_authz_cc",
],
)
2 changes: 0 additions & 2 deletions source/exe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ envoy_cc_library(
"//source/server/config/access_log:grpc_access_log_lib",
"//source/server/config/http:buffer_lib",
"//source/server/config/http:cors_lib",
"//source/server/config/http:ext_authz_lib",
"//source/server/config/http:fault_lib",
"//source/server/config/http:grpc_http1_bridge_lib",
"//source/server/config/http:grpc_json_transcoder_lib",
Expand All @@ -53,7 +52,6 @@ envoy_cc_library(
"//source/server/config/http:router_lib",
"//source/server/config/listener:original_dst_lib",
"//source/server/config/listener:proxy_protocol_lib",
"//source/server/config/network:ext_authz_lib",
"//source/server/config/network:http_connection_manager_lib",
"//source/server/config/network:ratelimit_lib",
"//source/server/config/network:raw_buffer_socket_lib",
Expand Down
2 changes: 2 additions & 0 deletions source/extensions/all_extensions.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
# selection options such as maturity.
def envoy_all_extensions(repository = ""):
return [
repository + "//source/extensions/filters/http/ext_authz:config",
repository + "//source/extensions/filters/network/client_ssl_auth:config",
repository + "//source/extensions/filters/network/echo:config",
repository + "//source/extensions/filters/network/ext_authz:config",
repository + "//source/extensions/filters/network/mongo_proxy:config",
repository + "//source/extensions/filters/network/tcp_proxy:config",
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,21 @@ load(

envoy_package()

envoy_cc_library(
name = "ext_authz_interface",
hdrs = ["ext_authz.h"],
deps = [
"//include/envoy/tracing:http_tracer_interface",
"@envoy_api//envoy/service/auth/v2:external_auth_cc",
],
)

envoy_cc_library(
name = "ext_authz_lib",
srcs = ["ext_authz_impl.cc"],
hdrs = ["ext_authz_impl.h"],
deps = [
"//include/envoy/ext_authz:ext_authz_interface",
":ext_authz_interface",
"//include/envoy/grpc:async_client_interface",
"//include/envoy/grpc:async_client_manager_interface",
"//include/envoy/http:filter_interface",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
#include "envoy/tracing/http_tracer.h"

namespace Envoy {
namespace Extensions {
namespace Filters {
namespace Common {
namespace ExtAuthz {

/**
Expand Down Expand Up @@ -64,4 +67,7 @@ class Client {
typedef std::unique_ptr<Client> ClientPtr;

} // namespace ExtAuthz
} // namespace Common
} // namespace Filters
} // namespace Extensions
} // namespace Envoy
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include "common/ext_authz/ext_authz_impl.h"
#include "extensions/filters/common/ext_authz/ext_authz_impl.h"

#include <chrono>
#include <cstdint>
Expand All @@ -15,9 +15,10 @@
#include "common/network/utility.h"
#include "common/protobuf/protobuf.h"

#include "fmt/format.h"

namespace Envoy {
namespace Extensions {
namespace Filters {
namespace Common {
namespace ExtAuthz {

GrpcClientImpl::GrpcClientImpl(Grpc::AsyncClientPtr&& async_client,
Expand Down Expand Up @@ -191,4 +192,7 @@ void CheckRequestUtils::createTcpCheck(const Network::ReadFilterCallbacks* callb
}

} // namespace ExtAuthz
} // namespace Common
} // namespace Filters
} // namespace Extensions
} // namespace Envoy
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include <string>
#include <vector>

#include "envoy/ext_authz/ext_authz.h"
#include "envoy/grpc/async_client.h"
#include "envoy/grpc/async_client_manager.h"
#include "envoy/http/filter.h"
Expand All @@ -19,7 +18,12 @@

#include "common/singleton/const_singleton.h"

#include "extensions/filters/common/ext_authz/ext_authz.h"

namespace Envoy {
namespace Extensions {
namespace Filters {
namespace Common {
namespace ExtAuthz {

typedef Grpc::TypedAsyncRequestCallbacks<envoy::service::auth::v2::CheckResponse>
Expand Down Expand Up @@ -109,4 +113,7 @@ class CheckRequestUtils {
};

} // namespace ExtAuthz
} // namespace Common
} // namespace Filters
} // namespace Extensions
} // namespace Envoy
38 changes: 38 additions & 0 deletions source/extensions/filters/http/ext_authz/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
licenses(["notice"]) # Apache 2

load(
"//bazel:envoy_build_system.bzl",
"envoy_cc_library",
"envoy_package",
)

envoy_package()

envoy_cc_library(
name = "ext_authz",
srcs = ["ext_authz.cc"],
hdrs = ["ext_authz.h"],
deps = [
"//include/envoy/http:codes_interface",
"//source/common/common:assert_lib",
"//source/common/common:empty_string",
"//source/common/common:enum_to_int",
"//source/common/http:codes_lib",
"//source/common/router:config_lib",
"//source/extensions/filters/common/ext_authz:ext_authz_lib",
"@envoy_api//envoy/config/filter/http/ext_authz/v2:ext_authz_cc",
],
)

envoy_cc_library(
name = "config",
srcs = ["config.cc"],
hdrs = ["config.h"],
deps = [
":ext_authz",
"//include/envoy/registry",
"//include/envoy/server:filter_config_interface",
"//source/common/config:well_known_names",
"//source/common/protobuf:utility_lib",
],
)
Loading