Skip to content

[release-1.7] extension config server#2960

Closed
kyessenov wants to merge 14 commits intoistio:release-1.7from
kyessenov:ecds
Closed

[release-1.7] extension config server#2960
kyessenov wants to merge 14 commits intoistio:release-1.7from
kyessenov:ecds

Conversation

@kyessenov
Copy link
Copy Markdown
Contributor

This implements an xDS server to serve extension configs (Wasm primarily).
There is no UI so open to suggestions how to make it usable :)
The idea is to deploy this in the cluster as a federated control plane server, and point to it via EnvoyFilters.
/cc @mandarjog
/cc @douglas-reid

Signed-off-by: Kuat Yessenov <kuat@google.com>
Signed-off-by: Kuat Yessenov <kuat@google.com>
Signed-off-by: Kuat Yessenov <kuat@google.com>
Signed-off-by: Kuat Yessenov <kuat@google.com>
@kyessenov kyessenov requested a review from a team August 3, 2020 22:15
@istio-testing istio-testing added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 3, 2020
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Aug 3, 2020
Signed-off-by: Kuat Yessenov <kuat@google.com>
Signed-off-by: Kuat Yessenov <kuat@google.com>
@jwendell jwendell changed the title extension config server [release-1.7] extension config server Aug 4, 2020
@JimmyCYJ
Copy link
Copy Markdown
Member

JimmyCYJ commented Aug 4, 2020

@kyessenov @mandarjog @douglas-reid is this PR ready? Could you append a GitHub issue?

Signed-off-by: Kuat Yessenov <kuat@google.com>
Signed-off-by: Kuat Yessenov <kuat@google.com>
@kyessenov
Copy link
Copy Markdown
Contributor Author

This is a draft. I'm using it to test against 1.7 proxy since it diverged from upstream envoy.

Signed-off-by: Kuat Yessenov <kuat@google.com>
Signed-off-by: Kuat Yessenov <kuat@google.com>
Copy link
Copy Markdown
Member

@JimmyCYJ JimmyCYJ left a comment

Choose a reason for hiding this comment

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

Didn't know that this PR is not ready. Will review again.

@jwendell
Copy link
Copy Markdown
Member

@kyessenov So, is this intended to be in 1.7.0? We have a code freeze tomorrow.

@kyessenov
Copy link
Copy Markdown
Contributor Author

This PR doesn't affect the release 1.7 binary in any way. I'm mostly interested in testing 1.7 proxy, so you don't have to merge this. If it doesn't, I'll just move it to master.

@jwendell
Copy link
Copy Markdown
Member

Awesome, thanks for the info.

Signed-off-by: Kuat Yessenov <kuat@google.com>
Signed-off-by: Kuat Yessenov <kuat@google.com>
params.Vars["ServerMetadata"] = params.LoadTestData("testdata/server_node_metadata.json.tmpl")
params.Vars["ServerHTTPFilters"] = params.LoadTestData("testdata/filters/stackdriver_inbound.yaml.tmpl")
params.Vars["ClientHTTPFilters"] = params.LoadTestData("testdata/filters/stackdriver_outbound.yaml.tmpl")
params.Vars["ServerHTTPFilters"] = params.LoadTestData("testdata/filters/mx_inbound.yaml.tmpl") + "\n" +
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Params setup seems repetitive, consider factoring it out.

)

// Convert to an envoy config.
// It so happens that 1.7 and 1.8 match in terms protobuf bytes, but not JSON.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Which part is different? The additional StringValue wrapping?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

vm_config got renamed to inline_vm_config.

log.Fatal(err)
}
server = extensionserver.New(context.Background())
discoveryservice.RegisterAggregatedDiscoveryServiceServer(grpcServer, server)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is ADS being used anywhere?
I only see ECDS in the example above with otherwise static config.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not here, but it could be used. envoy doesn't support multiple ADS streams though.

@kyessenov
Copy link
Copy Markdown
Contributor Author

Moving to master.

@kyessenov kyessenov closed this Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. do-not-merge Block automatic merging of a PR. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants