sds: certificate hot-reload for xDS gRPC connection by tsaarni · Pull Request #10163 · envoyproxy/envoy

tsaarni · 2020-02-25T18:56:16Z

Create watches for secrets pointed by path based SDS resources and trigger
hot-reload when the files change. This allows rotation of TLS certificate
and key for the xDS gRPC connection without restart.

Signed-off-by: Tero Saarni tero.saarni@est.tech

Description:
This PR adds support for certificate rotation for xDS gRPC interface. The implementation approach has been discussed in #9359.

The PR consists of following parts:

Add capability to SDS to make filesystem subscriptions for the certificate and key files referred to from SDS resources.
Adds capability to inotify WatcherImpl to make subscriptions for directories instead of only files. The change is backwards compatible.

The latter is needed to support commonly used pattern of updating files with atomic directory swap. Watching individual files is not possible, since in this pattern the files are symbolic links which point to a directory. The target of the symbolic links is a directory, which is atomically renamed (replaced) at update, not the files. Kubernetes uses this approach for secret volume mount. The details are covered in: #9359 (comment) and by the test case introduced in this PR.

The feature introduced by this PR is needed by Contour projectcontour/contour#2143 but it can be beneficial to any user of Envoy.

Risk Level: Medium
Testing: unittest
Docs Changes: To be defined: add information about the feature somewhere in the documentation
Release Notes:

Fixes #9359

tsaarni · 2020-02-25T19:04:57Z

I have added the new behavior to only inotify file watcher for Linux, but should this be added to kqueue watcher too - I guess the test case I added may fail on MacOS?

Edit: yes it did. I'll arrange compilation environment on my macbook and fix it!

tsaarni · 2020-03-04T09:01:00Z

After adding kqueue watcher implementation for macos, a different test case failed. I cannot find direct link between my change and the new failure. I see it occasionally failing on my machine too, ranging from 1 failure out of 1000 runs, to 3 out of 1000:

bazel test --runs_per_test 1000 //test/extensions/common/redis:cluster_refresh_manager_test`
...
//test/extensions/common/redis:cluster_refresh_manager_test              FAILED in 3 out of 1000 in 2.6s

This happens on master branch as well.

stale · 2020-03-11T09:42:11Z

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

tsaarni · 2020-03-11T09:45:34Z

I'd appreciate review comments on this PR, thank you!

lizan

Sorry for the delay, I'm back from OOO.

source/common/secret/sds_api.h

stale · 2020-03-23T21:26:12Z

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

lizan · 2020-03-23T21:58:41Z

Can you merge master? thanks

Create watches for secrets pointed by path based SDS resources and trigger hot-reload when the files change. This allows rotation of TLS certificates and key for the xDS gRPC connection without restart. Signed-off-by: Tero Saarni <tero.saarni@est.tech>

tsaarni · 2020-03-24T14:17:20Z

I've rebased the PR.
There was some flakiness in test environment this time...

stale · 2020-03-31T15:14:14Z

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

lizan · 2020-03-31T16:45:45Z

Sorry for the delay, can you merge master again? not rebase but just merge.

Signed-off-by: Tero Saarni <tero.saarni@est.tech>

tsaarni · 2020-04-01T15:40:41Z

I merged but got some new errors with the CI https://dev.azure.com/cncf/envoy/_build/results?buildId=36546&view=logs&j=3bf5ec37-bb6f-5fb6-2cf1-5134dc6106d6&t=41d571fe-3811-578d-07de-68468f467dae&l=1567

variable generic_secret should be generic_secret_: this code existed there before
sys/event.h file missing: this include is in macos specific code and should not exist on linux

lizan · 2020-04-01T19:04:39Z

@tsaarni I merged this ahead and will fix clang-tidy later. Thanks.

mattklein123 · 2020-04-01T20:29:14Z

@lizan @tsaarni this change should have a release note and documentation updates about the new behavior. @tsaarni can you please add some in a follow up? Thank you!

tsaarni · 2020-04-02T15:55:53Z

I propose to:

Add new example in Security / Secret discovery service (SDS) to explain how to use SDS with filesystem based config resources to achieve hot-reload for xDS gRPC.
Mention that filesystem subscriptions are also set up for referred certificate and key files in documentation of v2 core.ConfigSource and config.core.v3.ConfigSource

Though I'm bit unsure if the 2) is the best place in the API documentation for this information - at least those are the places where it already mentions about filesystem subscriptions in general.

I'm preparing documentation and will be submitting a PR soon for commenting.

mattklein123 · 2020-04-02T17:57:04Z

Thank you @tsaarni we can discuss the details in the PR!

Adds documentation for envoyproxy#10163. Signed-off-by: Tero Saarni <tero.saarni@est.tech>

Adds documentation for #10163. Signed-off-by: Tero Saarni <tero.saarni@est.tech>

tsaarni force-pushed the issue-9359 branch 3 times, most recently from dfc8b89 to 223fda7 Compare February 25, 2020 23:48

tsaarni requested review from PiotrSikora and lizan as code owners February 25, 2020 23:48

tsaarni requested a review from mattklein123 February 25, 2020 23:56

mattklein123 assigned lizan Feb 29, 2020

tsaarni force-pushed the issue-9359 branch from 223fda7 to 09c6e9f Compare March 2, 2020 07:28

stale bot added the stale stalebot believes this issue/PR has not been touched recently label Mar 11, 2020

stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Mar 11, 2020

This was referenced Mar 11, 2020

bootstrap: support for Envoy xDS certificate rotation projectcontour/contour#2333

Merged

Contour should document how to use alternatives to certgen for securing traffic between contour and envoy projectcontour/contour#2149

Closed

lizan reviewed Mar 12, 2020

View reviewed changes

source/common/secret/sds_api.h Outdated Show resolved Hide resolved

source/common/secret/sds_api.h Outdated Show resolved Hide resolved

source/common/secret/sds_api.h Outdated Show resolved Hide resolved

tsaarni force-pushed the issue-9359 branch from 09c6e9f to e1756b8 Compare March 16, 2020 18:05

stale bot added the stale stalebot believes this issue/PR has not been touched recently label Mar 23, 2020

stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Mar 23, 2020

tsaarni force-pushed the issue-9359 branch from e1756b8 to 6da98a3 Compare March 24, 2020 11:20

stale bot added the stale stalebot believes this issue/PR has not been touched recently label Mar 31, 2020

tsaarni mentioned this pull request Mar 31, 2020

Provide support for certificate rotation for xDS connection in Envoy container images #9359

Closed

Merge branch 'master' into issue-9359

1681590

Signed-off-by: Tero Saarni <tero.saarni@est.tech>

lizan approved these changes Apr 1, 2020

View reviewed changes

lizan merged commit 5105d9d into envoyproxy:master Apr 1, 2020

tsaarni mentioned this pull request Apr 2, 2020

doc: certificate hot-reload for xDS gRPC connection #10628

Merged

tsaarni added a commit to Nordix/envoy that referenced this pull request Apr 2, 2020

doc: certificate hot-reload for xDS gRPC connection

a5c483b

Adds documentation for envoyproxy#10163. Signed-off-by: Tero Saarni <tero.saarni@est.tech>

mattklein123 pushed a commit that referenced this pull request Apr 2, 2020

doc: certificate hot-reload for xDS gRPC connection (#10628)

f5d2cc6

Adds documentation for #10163. Signed-off-by: Tero Saarni <tero.saarni@est.tech>

mattklein123 mentioned this pull request Jun 11, 2020

Fix runtime config hot reload envoyproxy/ratelimit#150

Closed

freddygv mentioned this pull request Jun 27, 2020

Dynamic certificate reloading for Terminating Gateways hashicorp/consul#8198

Open

tsaarni mentioned this pull request Jul 10, 2020

Improve SDS file watching docs #10979

Closed

htuch mentioned this pull request Oct 1, 2020

Symlink-atomic reads for private key/certificate pairs #13370

Closed

htuch mentioned this pull request Oct 20, 2020

SDS filesystem watches operate on file parent rather than grandparent #13663

Closed

ggreenway mentioned this pull request May 13, 2021

Support reloading TlsCertificate files #10387

Open

Conversation

tsaarni commented Feb 25, 2020

Uh oh!

tsaarni commented Feb 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tsaarni commented Mar 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stale bot commented Mar 11, 2020

Uh oh!

tsaarni commented Mar 11, 2020

Uh oh!

lizan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stale bot commented Mar 23, 2020

Uh oh!

lizan commented Mar 23, 2020

Uh oh!

tsaarni commented Mar 24, 2020

Uh oh!

stale bot commented Mar 31, 2020

Uh oh!

lizan commented Mar 31, 2020

Uh oh!

tsaarni commented Apr 1, 2020

Uh oh!

lizan commented Apr 1, 2020

Uh oh!

mattklein123 commented Apr 1, 2020

Uh oh!

tsaarni commented Apr 2, 2020

Uh oh!

mattklein123 commented Apr 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tsaarni commented Feb 25, 2020 •

edited

Loading

tsaarni commented Mar 4, 2020 •

edited

Loading