From 90097ee9aa81e0c2e0892a9f4134a1512b486ad8 Mon Sep 17 00:00:00 2001 From: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> Date: Tue, 16 Jul 2024 12:33:43 -0400 Subject: [PATCH] [extension/opamp] Rely on Collector APIs for config redaction (#34078) **Description:** Now that https://github.com/open-telemetry/opentelemetry-collector/pull/10139 is merged, we can rely on the Collector APIs to redact sensitive fields in the config for us. I tested this against the latest Collector core commit with the goal that this will land in the v0.105.0 release. --- .chloggen/unredact-effective-config.yaml | 29 +++++++++++ extension/opampextension/opamp_agent.go | 48 ------------------- extension/opampextension/opamp_agent_test.go | 3 +- .../testdata/effective-redacted.yaml | 27 ----------- 4 files changed, 30 insertions(+), 77 deletions(-) create mode 100644 .chloggen/unredact-effective-config.yaml delete mode 100644 extension/opampextension/testdata/effective-redacted.yaml diff --git a/.chloggen/unredact-effective-config.yaml b/.chloggen/unredact-effective-config.yaml new file mode 100644 index 000000000000..553f1aacb5ac --- /dev/null +++ b/.chloggen/unredact-effective-config.yaml @@ -0,0 +1,29 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# 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: Rely on the Collector APIs to do config redaction + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [34078] + +# (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: | + Previously all config fields had to be redacted, now `configopaque.String` is used to determine + which fields should be redacted. As a result, fields that are not sensitive are no longer redacted. + +# 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 122fc68d72fa..9c5a7b6fe8d3 100644 --- a/extension/opampextension/opamp_agent.go +++ b/extension/opampextension/opamp_agent.go @@ -30,13 +30,6 @@ 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 @@ -285,46 +278,6 @@ func (o *opampAgent) updateAgentIdentity(instanceID uuid.UUID) { 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() @@ -334,7 +287,6 @@ func (o *opampAgent) composeEffectiveConfig() *protobufs.EffectiveConfig { } 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)) diff --git a/extension/opampextension/opamp_agent_test.go b/extension/opampextension/opamp_agent_test.go index 1766cadcfedb..49e9c0ef618f 100644 --- a/extension/opampextension/opamp_agent_test.go +++ b/extension/opampextension/opamp_agent_test.go @@ -175,8 +175,7 @@ func TestComposeEffectiveConfig(t *testing.T) { ecFileName := filepath.Join("testdata", "effective.yaml") cm, err := confmaptest.LoadConf(ecFileName) assert.NoError(t, err) - redactedFileName := filepath.Join("testdata", "effective-redacted.yaml") - expected, err := os.ReadFile(redactedFileName) + expected, err := os.ReadFile(ecFileName) assert.NoError(t, err) o.updateEffectiveConfig(cm) diff --git a/extension/opampextension/testdata/effective-redacted.yaml b/extension/opampextension/testdata/effective-redacted.yaml deleted file mode 100644 index 60fa731583b5..000000000000 --- a/extension/opampextension/testdata/effective-redacted.yaml +++ /dev/null @@ -1,27 +0,0 @@ -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]