Skip to content

Config Dump for Secret Discovery Service.#7365

Merged
lizan merged 31 commits intoenvoyproxy:masterfrom
incfly:sds-cfg
Jul 24, 2019
Merged

Config Dump for Secret Discovery Service.#7365
lizan merged 31 commits intoenvoyproxy:masterfrom
incfly:sds-cfg

Conversation

@incfly
Copy link
Contributor

@incfly incfly commented Jun 22, 2019

For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md

Description: Config Dump
Risk Level: Small?
Testing: Unit Test
Docs Changes: N/A(Is config_dump auto gen doc enough?)
Release Notes: config_dump handler prints out Secret Discovery Service information
[Optional Fixes #Issue] #7111

Jianfei Hu added 2 commits June 22, 2019 04:32
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
@incfly
Copy link
Contributor Author

incfly commented Jun 22, 2019

/cc @lizan @JimmyCYJ @PiotrSikora

Still some rough edges and some questions that I'm unsure, but it should be ready to take a look, thanks!

@lizan
Copy link
Member

lizan commented Jun 22, 2019

fix builds? I think this is on the right track.

Jianfei Hu added 7 commits June 24, 2019 21:05
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
@incfly
Copy link
Contributor Author

incfly commented Jun 26, 2019

@lizan all tests pass, PTAL, thanks!

@incfly incfly changed the title [Draft] Config Dump for Secret Discovery Service Config Dump for Secret Discovery Service. Jun 26, 2019
@incfly
Copy link
Contributor Author

incfly commented Jun 27, 2019

Friendly ping :)

Copy link
Member

@lizan lizan left a comment

Choose a reason for hiding this comment

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

This is in right direction, can you clear your own TODOs and questions from code and merge master?

Jianfei Hu added 4 commits July 10, 2019 22:19
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
// TODO(incfly): switch to more generic scrubbing mechanism once
// https://github.com/envoyproxy/envoy/issues/4757 is resolved.
tls_certificate->clear_private_key();
tls_certificate->clear_password();
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps set those to "[redacted]" (for non-empty values) to make it possible to distinguish between value being removed for security reasons vs not being set in the first place?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, while you're at it, could you make similar change to LDS and CDS config dumps? Separate PR is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you mean? The inlined_string typed key/password from tls_certificate used in LDS/CDS dump?

Jianfei Hu added 4 commits July 12, 2019 17:49
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Jianfei Hu added 2 commits July 22, 2019 01:19
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
PiotrSikora
PiotrSikora previously approved these changes Jul 22, 2019
Copy link
Contributor

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

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

LGTM, small nits.

Jianfei Hu added 2 commits July 22, 2019 17:36
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
PiotrSikora
PiotrSikora previously approved these changes Jul 22, 2019
Copy link
Contributor

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

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

Thanks!

@incfly
Copy link
Contributor Author

incfly commented Jul 22, 2019

@lizan @mattklein123 is there anything that I need to merge this PR? Thanks!

lizan
lizan previously approved these changes Jul 22, 2019
@lizan lizan dismissed their stale review July 22, 2019 21:17

mistake

Signed-off-by: Jianfei Hu <jianfeih@google.com>
Jianfei Hu added 2 commits July 22, 2019 23:16
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Signed-off-by: Jianfei Hu <jianfeih@google.com>
Copy link
Member

@lizan lizan left a comment

Choose a reason for hiding this comment

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

@incfly one last thing, can you add a line to version_history?

Signed-off-by: Jianfei Hu <jianfeih@google.com>
* config: changed the default value of :ref:`initial_fetch_timeout <envoy_api_field_core.ConfigSource.initial_fetch_timeout>` from 0s to 15s. This is a change in behaviour in the sense that Envoy will move to the next initialization phase, even if the first config is not delivered in 15s. Refer to :ref:`initialization process <arch_overview_initialization>` for more details.
* listeners: added :ref:`HTTP inspector listener filter <config_listener_filters_http_inspector>`.
* http: added the ability to reject HTTP/1.1 requests with invalid HTTP header values, using the runtime feature `envoy.reloadable_features.strict_header_validation`.
* admin: added config dump support for Secret Discovery Service :ref:`SecretConfigDump <envoy_api_msg_admin.v2alpha.SecretConfigDump>`.
Copy link
Member

Choose a reason for hiding this comment

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

nit: alphabetical order

Signed-off-by: Jianfei Hu <jianfeih@google.com>
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.

6 participants