Skip to content

configs: add support for using dynamic forward proxy#320

Merged
rebello95 merged 16 commits intomasterfrom
dynamic-config
Aug 13, 2019
Merged

configs: add support for using dynamic forward proxy#320
rebello95 merged 16 commits intomasterfrom
dynamic-config

Conversation

@rebello95
Copy link
Copy Markdown
Contributor

@rebello95 rebello95 commented Aug 9, 2019

We'll be using the dynamic forward proxy configuration as the defacto configuration that will ship with Envoy Mobile. This will allow us to avoid making consumers register the DNS/domains they need to support when starting up the Envoy instance.

This PR:

Future PRs will:

Part of #169.

Signed-off-by: Michael Rebello me@michaelrebello.com

goaway added 2 commits August 9, 2019 13:59
Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: Mike Schore <mike.schore@gmail.com>
Copy link
Copy Markdown
Contributor

@goaway goaway left a comment

Choose a reason for hiding this comment

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

This looks great, nice work!

Can the Android example apps be updated here too to keep CI green?

goaway and others added 5 commits August 9, 2019 15:34
Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Nice. Note that this is not going to work via the direct call mechanism that @junr03 and @goaway are working on because that only has the router filter. We will need to build the real "API listener" to get this functionality.

# tls_context:
# common_tls_context:
# validation_context:
# trusted_ca: {filename: /etc/ssl/certs/ca-certificates.crt}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We definitely need/want this. Do we know what the correct paths to load the platform trusted CA files are? Or is it more difficult than that on mobile?

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.

Totally agree. We're planning on merging with this temporarily disabled, as we'll likely need to compile our own with the library. Tracking here, and linked within the config: #322

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.

It's a little scary to merge this as is, in case anyone's actually playing around with this library. Do you want to add logic to manually reject non-https traffic for now?

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.

Talked offline - opted to leave a WARNING! here since this is in examples/ and we are still in demo stage.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>

Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
@rebello95 rebello95 marked this pull request as ready for review August 12, 2019 22:32
@rebello95 rebello95 requested a review from goaway August 12, 2019 22:32
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: Michael Rebello <me@michaelrebello.com>
@goaway
Copy link
Copy Markdown
Contributor

goaway commented Aug 12, 2019

nice work

@rebello95
Copy link
Copy Markdown
Contributor Author

Thanks for the reviews!
@mattklein123 just to close the loop here - we plan to make this work properly with the direct API when/before switching over the demos to use it instead of the native networking calls they use today.

@rebello95 rebello95 merged commit e325273 into master Aug 13, 2019
@rebello95 rebello95 deleted the dynamic-config branch August 13, 2019 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants