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

Generate proto files for XDS filter configs #159

Merged
merged 3 commits into from
Dec 15, 2020
Merged

Generate proto files for XDS filter configs #159

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d3f307
Generate proto files for XDS filter configs
iffyio Dec 10, 2020
2f00e7a
Remove go option
iffyio Dec 14, 2020
b295030
Merge branch 'master' into iu/filter-proto
markmandel Dec 15, 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
7 changes: 7 additions & 0 deletions build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,17 @@
// we need for XDS GRPC communication.
fn main() -> Result<(), Box<dyn std::error::Error>> {
let proto_files = vec![
"proto/data-plane-api/envoy/config/accesslog/v3/accesslog.proto",
"proto/data-plane-api/envoy/config/cluster/v3/cluster.proto",
"proto/data-plane-api/envoy/config/listener/v3/listener.proto",
"proto/data-plane-api/envoy/config/route/v3/route.proto",
"proto/data-plane-api/envoy/service/cluster/v3/cds.proto",
"proto/data-plane-api/envoy/service/discovery/v3/ads.proto",
"proto/data-plane-api/envoy/service/discovery/v3/discovery.proto",
"proto/data-plane-api/envoy/type/metadata/v3/metadata.proto",
"proto/data-plane-api/envoy/type/tracing/v3/custom_tag.proto",
"proto/udpa/udpa/core/v1/resource_name.proto",
"proto/quilkin/extensions/filters/debug/v1alpha1/debug.proto",
]
.iter()
.map(|name| std::env::current_dir().unwrap().join(name))
Expand All @@ -33,6 +39,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
"proto/udpa",
"proto/googleapis",
"proto/protoc-gen-validate",
"proto/quilkin",
]
.iter()
.map(|i| std::env::current_dir().unwrap().join(i))
Expand Down
12 changes: 12 additions & 0 deletions proto/quilkin/extensions/filters/debug/v1alpha1/debug.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
syntax = "proto3";
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why a proto for each Filter is necessary? I'm assuming this is for being able to pull config info from the XDS proto values? Mostly just want to make sure I understand what is happening here. (Also leads me into that fact we should write some docs on "writing your own filters" 😄)

Looking at Envoy, I assume this matches up to something like:
https://github.com/envoyproxy/envoy/blob/master/api/envoy/extensions/filters/http/gzip/v3/gzip.proto

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes that's the case it matches envoy's filter proto files. XDS has a filter chain proto message that's a list of these config messages one per filter. Then on our end after receiving we transform that XDS filter chain into our own e32fb6d#diff-07a585ddcc9ea470eb22742d985007bb1483e42bafd05d660ed3ec5294cdb1d0R142-R154


package quilkin.extensions.filters.debug.v1alpha1;

import "google/protobuf/wrappers.proto";

option go_package = "example";
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are going to have a go_package, probably makes sense to have it be a package other than "example"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will fix, it was left over from local testing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed


message Debug {
google.protobuf.StringValue id = 1;
}

15 changes: 15 additions & 0 deletions src/extensions/filters/debug.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,21 @@ use crate::extensions::filter_registry::{
};
use crate::extensions::Filter;

/// Protobuf config for this filter.
mod quilkin {
pub(crate) mod extensions {
pub(crate) mod filters {
pub(crate) mod debug {
pub(crate) mod v1alpha1 {
#![cfg(not(doctest))]
#![doc(hidden)]
tonic::include_proto!("quilkin.extensions.filters.debug.v1alpha1");
}
}
}
}
}

/// Debug logs all incoming and outgoing packets
///
/// # Configuration
Expand Down
35 changes: 35 additions & 0 deletions src/xds/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,34 @@ mod envoy {
tonic::include_proto!("envoy.r#type.matcher.v3");
}
}
pub mod metadata {
Copy link
Contributor

Choose a reason for hiding this comment

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

Random thought - this seems like a lot of boilerplate. Macro time?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That could be the case, not sure what a macro here would look like and I don't think it'll be trivial to write if possible, since the heirarchies are arbitrarily nested and the macro would need to figure out where each include goes and how to split the raw string to figure out the module components

pub mod v3 {
#![cfg(not(doctest))]
#![doc(hidden)]
tonic::include_proto!("envoy.r#type.metadata.v3");
}
}
pub mod tracing {
pub mod v3 {
#![cfg(not(doctest))]
#![doc(hidden)]
tonic::include_proto!("envoy.r#type.tracing.v3");
}
}
pub mod v3 {
#![cfg(not(doctest))]
#![doc(hidden)]
tonic::include_proto!("envoy.r#type.v3");
}
}
pub mod config {
pub mod accesslog {
pub mod v3 {
#![cfg(not(doctest))]
#![doc(hidden)]
tonic::include_proto!("envoy.config.accesslog.v3");
}
}
pub mod cluster {
pub mod v3 {
#![cfg(not(doctest))]
Expand All @@ -62,6 +83,20 @@ mod envoy {
tonic::include_proto!("envoy.config.endpoint.v3");
}
}
pub mod listener {
pub mod v3 {
#![cfg(not(doctest))]
#![doc(hidden)]
tonic::include_proto!("envoy.config.listener.v3");
}
}
pub mod route {
pub mod v3 {
#![cfg(not(doctest))]
#![doc(hidden)]
tonic::include_proto!("envoy.config.route.v3");
}
}
}
pub mod service {
pub mod discovery {
Expand Down