Skip to content

WIP: Local cert provider#23063

Closed
liverbirdkte wants to merge 22 commits intoenvoyproxy:mainfrom
liverbirdkte:local_cert_provider
Closed

WIP: Local cert provider#23063
liverbirdkte wants to merge 22 commits intoenvoyproxy:mainfrom
liverbirdkte:local_cert_provider

Conversation

@liverbirdkte
Copy link
Copy Markdown

@liverbirdkte liverbirdkte commented Sep 11, 2022

Local certificate provider instance

A certificate provider instance to generate certificates locally. It's based on the certificate provider framework and bumping filter could use this instance for testing.

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Luyao Zhong and others added 21 commits February 15, 2022 13:33
Implement certificate_provider_instances in bootstrap (envoyproxy#19308)
d4eb3cb 
Introduce a CertificateProviderManager to parse the certificate_provider_instances
config and instantiate certificate providers.

Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
Merge branch 'envoyproxy:main' into certificate-provider-instances
4db7266
address comments
18464b7 
* rename default_cert_provider to static_cert_provider
* nest struct Certpair inside of the CertificateProvider class
* avoid the copy of certificate provider config
* update CertificateProvider interfaces
* fix format_pre CI failure

Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
subscription framework code for certificate provider
732fb63 
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
Merge branch 'main' into certificate-provider-instances-pr
3bcbe0a 
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
move certificate_provider.h into envoy/certificate_provider directory
45f9f19 
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
update CertificateProvider interface
3c822c0 
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
address comments and update StaticCertificateProvider
c179df3 
StaticCertificateProvider supports generating identity certificates,
it is supposed to be renamed later.

Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
Merge branch 'main' into certificate-provider-instances
e712dbd 
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
rename StaticCertificateProvider to DefaultCertificateProvider
b052975 
DefaultCertificateProvider supports generating certificates
for handshake which shows how CertificateProvider
asynchronous interfaces work.

Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
Merge remote-tracking branch 'upstream/main' into certificate-provide…
d31b1d7 
…r-instances

Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
update DefaultCertificateProvider and remove CertificateSubscriptionC…
7df7915 
…allbacks

Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
Merge remote-tracking branch 'upstream/main' into certificate-provide…
d87daaa 
…r-instances

Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
Merge remote-tracking branch 'upstream/main' into certificate-provide…
932b282 
…r-instances

Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
Implemet certificate provider framework
0c4e685 
implement ca_certificate_provider_instance in CertificateValidationContext
implement tls_certificate_provider_instance in CommonTlsContext

Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
Merge remote-tracking branch 'upstream/main' into certificate-provide…
797ba3e 
…r-instances

Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
remove test code
9d0ef89 
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
add comments for certificate provider interface
bc4e2d0 
consider current implementation for tls certificate config loading,
certificate provider shoule provide at least one tls certificate,
otherwise the Envoy will complain when loading the config.

Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
Merge branch 'main' into certificate-provider-instances
7bf603a 
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
fix docs format
d0acb6b 
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
WIP: add local certificate provider instance
32cdf03 
Signed-off-by: LeiZhang <lei.a.zhang@intel.com>
@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Sep 11, 2022

Hi @liverbirdkte, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #23063 was opened by liverbirdkte.

see: more, trace.

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Sep 11, 2022

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #23063 was opened by liverbirdkte.

see: more, trace.

@liverbirdkte liverbirdkte marked this pull request as draft September 11, 2022 13:24
Merge branch 'main' into local_cert_provider
6660562 
Signed-off-by: lei zhang <lei.a.zhang@intel.com>
@LuyaoZhong LuyaoZhong mentioned this pull request Sep 14, 2022
Open
@vermajit
Copy link
Copy Markdown

vermajit commented Sep 15, 2022 •
edited
Loading

Hi @liverbirdkte Would you be able to share a sample envoy config that I could use to try this out locally ?

I tried out the following

certificate_provider_instances:
  local_certificate_provider:
    name: envoy.certificate_providers.local_certificate
    typed_config:
      "@type": type.googleapis.com/envoy.extensions.certificate_providers.local_certificate.v3.LocalCertificate
      rootca_cert: {"filename": "./root-ca.pem"}
      rootca_key: {"filename": "./root-ca.key"}
static_resources:
  listeners:
  - name: https_listener
... rest of config ...

But envoy fails to start with following error

[2022-09-14 17:39:52.884][31771][debug][config] [source/server/configuration_impl.cc:99] add certificate provider: local_certificate_provider
[2022-09-14 17:39:52.884][31771][critical][main] [source/server/server.cc:117] error initializing configuration 'config.yaml': Didn't find a registered implementation for 'envoy.certificate_providers.local_certificate' with type URL: 'envoy.extensions.certificate_providers.local_certificate.v3.LocalCertificate'
[2022-09-14 17:39:52.884][31771][info][main] [source/server/server.cc:966] exiting
Didn't find a registered implementation for 'envoy.certificate_providers.local_certificate' with type URL: 'envoy.extensions.certificate_providers.local_certificate.v3.LocalCertificate'

@vermajit
Copy link
Copy Markdown

vermajit commented Sep 15, 2022

Hi @liverbirdkte Would you be able to share a sample envoy config that I could use to try this out locally ?

I tried out the following

certificate_provider_instances:
  local_certificate_provider:
    name: envoy.certificate_providers.local_certificate
    typed_config:
      "@type": type.googleapis.com/envoy.extensions.certificate_providers.local_certificate.v3.LocalCertificate
      rootca_cert: {"filename": "./root-ca.pem"}
      rootca_key: {"filename": "./root-ca.key"}
static_resources:
  listeners:
  - name: https_listener
... rest of config ...

But envoy fails to start with following error

[2022-09-14 17:39:52.884][31771][debug][config] [source/server/configuration_impl.cc:99] add certificate provider: local_certificate_provider
[2022-09-14 17:39:52.884][31771][critical][main] [source/server/server.cc:117] error initializing configuration 'config.yaml': Didn't find a registered implementation for 'envoy.certificate_providers.local_certificate' with type URL: 'envoy.extensions.certificate_providers.local_certificate.v3.LocalCertificate'
[2022-09-14 17:39:52.884][31771][info][main] [source/server/server.cc:966] exiting
Didn't find a registered implementation for 'envoy.certificate_providers.local_certificate' with type URL: 'envoy.extensions.certificate_providers.local_certificate.v3.LocalCertificate'

Found the issue. I was able to load the extension successfully after adding the missing REGISTER_FACTORY(LocalCertificateFactory, CertificateProvider::CertificateProviderFactory); in source/extensions/certificate_providers/local_certificate/config.cc

@liverbirdkte
Copy link
Copy Markdown
Author

liverbirdkte commented Sep 21, 2022

Hi @liverbirdkte Would you be able to share a sample envoy config that I could use to try this out locally ?
I tried out the following

certificate_provider_instances:
  local_certificate_provider:
    name: envoy.certificate_providers.local_certificate
    typed_config:
      "@type": type.googleapis.com/envoy.extensions.certificate_providers.local_certificate.v3.LocalCertificate
      rootca_cert: {"filename": "./root-ca.pem"}
      rootca_key: {"filename": "./root-ca.key"}
static_resources:
  listeners:
  - name: https_listener
... rest of config ...

But envoy fails to start with following error

[2022-09-14 17:39:52.884][31771][debug][config] [source/server/configuration_impl.cc:99] add certificate provider: local_certificate_provider
[2022-09-14 17:39:52.884][31771][critical][main] [source/server/server.cc:117] error initializing configuration 'config.yaml': Didn't find a registered implementation for 'envoy.certificate_providers.local_certificate' with type URL: 'envoy.extensions.certificate_providers.local_certificate.v3.LocalCertificate'
[2022-09-14 17:39:52.884][31771][info][main] [source/server/server.cc:966] exiting
Didn't find a registered implementation for 'envoy.certificate_providers.local_certificate' with type URL: 'envoy.extensions.certificate_providers.local_certificate.v3.LocalCertificate'

Found the issue. I was able to load the extension successfully after adding the missing REGISTER_FACTORY(LocalCertificateFactory, CertificateProvider::CertificateProviderFactory); in source/extensions/certificate_providers/local_certificate/config.cc

Hi vermajit,

Sorry for not seeing this in time, this is a draft of local cert provider instance and not totally ready. You have to integrate with other patches to make it work. We are going to submit a PR to include all the related patches together for testing. I'll ping you when the code is ready. Thanks.

@LuyaoZhong LuyaoZhong mentioned this pull request Sep 21, 2022
Closed
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 21, 2022

This pull request has been automatically marked as stale because it has not had activity in the last 30 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!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Oct 21, 2022
@liverbirdkte
Copy link
Copy Markdown
Author

liverbirdkte commented Oct 28, 2022

Hi @vermajit, we have an integration patch to demo TLS bumping scenario, #23192. You could try it out locally. Any feedback is welcome.

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Oct 28, 2022
@LuyaoZhong LuyaoZhong mentioned this pull request Nov 15, 2022
Merged
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 27, 2022

This pull request has been automatically marked as stale because it has not had activity in the last 30 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!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Nov 27, 2022
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 4, 2022

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot closed this Dec 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lizan lizan Awaiting requested review from lizan

@ggreenway ggreenway Awaiting requested review from ggreenway ggreenway is a code owner

@RyanTheOptimist RyanTheOptimist Awaiting requested review from RyanTheOptimist RyanTheOptimist will be requested when the pull request is marked ready for review RyanTheOptimist is a code owner

@botengyao botengyao Awaiting requested review from botengyao botengyao will be requested when the pull request is marked ready for review botengyao is a code owner

Assignees

@adisuissa adisuissa

Labels

api stale stalebot believes this issue/PR has not been touched recently

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@liverbirdkte @vermajit @adisuissa