From ce090710312ce579c77286a49cd8448e4ecca350 Mon Sep 17 00:00:00 2001 From: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> Date: Mon, 3 Jun 2024 11:02:22 -0400 Subject: [PATCH] [extension/opamp] Redact values in effective config (#33267) **Description:** Redacts primitive values in the effective config reported by the extension. Necessary for https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/31641 until https://github.com/open-telemetry/opentelemetry-collector/pull/10139 is resolved. Relates to https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/32983 --- .chloggen/redact-effective-config.yaml | 33 ++++++++++++ extension/opampextension/opamp_agent.go | 51 ++++++++++++++++++- extension/opampextension/opamp_agent_test.go | 3 +- .../testdata/effective-redacted.yaml | 27 ++++++++++ .../opampextension/testdata/effective.yaml | 13 +++++ 5 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 .chloggen/redact-effective-config.yaml create mode 100644 extension/opampextension/testdata/effective-redacted.yaml diff --git a/.chloggen/redact-effective-config.yaml b/.chloggen/redact-effective-config.yaml new file mode 100644 index 000000000000..239c8b026091 --- /dev/null +++ b/.chloggen/redact-effective-config.yaml @@ -0,0 +1,33 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: extension/opamp + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Redact all values in the effective config + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [33267] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: | + All values will be treated as if they are a `configopaque.String` type. This will + be changed once the Collector APIs are updated to unmarshal the config while + only redacting actual `configopaque.String`-typed values. + + The exception to redaction is the `service::pipelines` section, which is useful + for debugging and does not contain any `configopaque.String` values. + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/extension/opampextension/opamp_agent.go b/extension/opampextension/opamp_agent.go index b495bc62273b..f699005851ce 100644 --- a/extension/opampextension/opamp_agent.go +++ b/extension/opampextension/opamp_agent.go @@ -28,6 +28,13 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/extension/opampcustommessages" ) +const redactedVal = "[REDACTED]" + +// Paths that will not have values redacted when reporting the effective config. +var unredactedPaths = []string{ + "service::pipelines", +} + type opampAgent struct { cfg *Config logger *zap.Logger @@ -261,6 +268,46 @@ func (o *opampAgent) updateAgentIdentity(instanceID ulid.ULID) { o.instanceID = instanceID } +func redactConfig(cfg any, parentPath string) { + switch val := cfg.(type) { + case map[string]any: + for k, v := range val { + path := parentPath + if path == "" { + path = k + } else { + path += "::" + k + } + // We don't want to redact certain parts of the config + // that are known not to contain secrets, e.g. pipelines. + for _, p := range unredactedPaths { + if p == path { + return + } + } + switch x := v.(type) { + case map[string]any: + redactConfig(x, path) + case []any: + redactConfig(x, path) + default: + val[k] = redactedVal + } + } + case []any: + for i, v := range val { + switch x := v.(type) { + case map[string]any: + redactConfig(x, parentPath) + case []any: + redactConfig(x, parentPath) + default: + val[i] = redactedVal + } + } + } +} + func (o *opampAgent) composeEffectiveConfig() *protobufs.EffectiveConfig { o.eclk.RLock() defer o.eclk.RUnlock() @@ -269,7 +316,9 @@ func (o *opampAgent) composeEffectiveConfig() *protobufs.EffectiveConfig { return nil } - conf, err := yaml.Marshal(o.effectiveConfig.ToStringMap()) + m := o.effectiveConfig.ToStringMap() + redactConfig(m, "") + conf, err := yaml.Marshal(m) if err != nil { o.logger.Error("cannot unmarshal effectiveConfig", zap.Any("conf", o.effectiveConfig), zap.Error(err)) return nil diff --git a/extension/opampextension/opamp_agent_test.go b/extension/opampextension/opamp_agent_test.go index ceb9f912f766..48f76548ff9f 100644 --- a/extension/opampextension/opamp_agent_test.go +++ b/extension/opampextension/opamp_agent_test.go @@ -176,7 +176,8 @@ func TestComposeEffectiveConfig(t *testing.T) { ecFileName := filepath.Join("testdata", "effective.yaml") cm, err := confmaptest.LoadConf(ecFileName) assert.NoError(t, err) - expected, err := os.ReadFile(ecFileName) + redactedFileName := filepath.Join("testdata", "effective-redacted.yaml") + expected, err := os.ReadFile(redactedFileName) assert.NoError(t, err) o.updateEffectiveConfig(cm) diff --git a/extension/opampextension/testdata/effective-redacted.yaml b/extension/opampextension/testdata/effective-redacted.yaml new file mode 100644 index 000000000000..60fa731583b5 --- /dev/null +++ b/extension/opampextension/testdata/effective-redacted.yaml @@ -0,0 +1,27 @@ +exporters: + otlp: + endpoint: "[REDACTED]" +receivers: + otlp: + protocols: + grpc: {} + http: {} +processors: + transform: + error_mode: "[REDACTED]" + metric_statements: + - context: "[REDACTED]" + conditions: + - "[REDACTED]" + statements: + - "[REDACTED]" +service: + pipelines: + traces: + receivers: [otlp] + processors: [] + exporters: [otlp] + metrics: + receivers: [otlp] + processors: [transform] + exporters: [otlp] diff --git a/extension/opampextension/testdata/effective.yaml b/extension/opampextension/testdata/effective.yaml index 69261ddf6dd3..fb19f758661e 100644 --- a/extension/opampextension/testdata/effective.yaml +++ b/extension/opampextension/testdata/effective.yaml @@ -6,9 +6,22 @@ receivers: protocols: grpc: {} http: {} +processors: + transform: + error_mode: ignore + metric_statements: + - context: metric + conditions: + - type == METRIC_DATA_TYPE_SUM + statements: + - set(description, "Sum") service: pipelines: traces: receivers: [otlp] processors: [] exporters: [otlp] + metrics: + receivers: [otlp] + processors: [transform] + exporters: [otlp]