Skip to content

Commit

Permalink
Fix warning on emitting diagnostics for otel hybrid mode (#6973)
Browse files Browse the repository at this point in the history
If an Otel hybrid mode configuration doesn't exist, we emit a plain text
message into the config file generated by diagnostics. When we try to
redact secrets from it, we fail to unmarshal it into a map and log a
warning about it.

Instead, only run the redaction after confirming the value is actually
a map.
  • Loading branch information
swiatekm authored Feb 26, 2025
1 parent 33d0137 commit 087cfb3
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 8 deletions.
20 changes: 12 additions & 8 deletions internal/pkg/diagnostics/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,19 +327,23 @@ func writeRedacted(errOut, resultWriter io.Writer, fullFilePath string, fileResu

// Should we support json too?
if fileResult.ContentType == "application/yaml" {
unmarshalled := map[string]interface{}{}
var unmarshalled any
err := yaml.Unmarshal(fileResult.Content, &unmarshalled)
if err != nil {
// Best effort, output a warning but still include the file
fmt.Fprintf(errOut, "[WARNING] Could not redact %s due to unmarshalling error: %s\n", fullFilePath, err)
} else {
unmarshalled = Redact(unmarshalled, errOut)
redacted, err := yaml.Marshal(unmarshalled)
if err != nil {
// Best effort, output a warning but still include the file
fmt.Fprintf(errOut, "[WARNING] Could not redact %s due to marshalling error: %s\n", fullFilePath, err)
} else {
out = &redacted
switch t := unmarshalled.(type) { // could be a plain string, we only redact if this is a proper map
case map[string]any:
t = Redact(t, errOut)
redacted, err := yaml.Marshal(t)
if err != nil {
// Best effort, output a warning but still include the file
fmt.Fprintf(errOut, "[WARNING] Could not redact %s due to marshalling error: %s\n", fullFilePath, err)
} else {
out = &redacted
}
default:
}
}
}
Expand Down
13 changes: 13 additions & 0 deletions internal/pkg/diagnostics/diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,19 @@ i4EFZLWrFRsAAAARYWxleGtAZ3JlbWluLm5lc3QBAg==
require.NotContains(t, outWriter.String(), privKey)
}

func TestRedactPlainString(t *testing.T) {
errOut := strings.Builder{}
outWriter := strings.Builder{}
inputString := "Just a string"
res := client.DiagnosticFileResult{Content: []byte(inputString), ContentType: "application/yaml"}

err := writeRedacted(&errOut, &outWriter, "test/path", res)
require.NoError(t, err)

require.Empty(t, errOut.String())
require.Equal(t, outWriter.String(), inputString)
}

func TestRedactComplexKeys(t *testing.T) {
// taken directly from the yaml spec: https://yaml.org/spec/1.1/#c-mapping-key
// This test mostly serves to document that part of the YAML library doesn't work properly
Expand Down

0 comments on commit 087cfb3

Please sign in to comment.