From 1cea6363f60421d4faed7469ccdcae25f82231b2 Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Mon, 15 Jan 2024 19:36:57 +0100 Subject: [PATCH] [configopaque] Implement fmt.GoStringer interface for String (#9271) **Description:** Follow up to #9214. To make sure the output when using `%#v` is redacted, we must also implement the `fmt.GoStringer` interface. **Link to tracking Issue:** Relates to #9213 **Testing:** Adds some tests with YAML and JSON as well as more thorough testing of all `fmt` verbs **Documentation:** Clarify that printing this is also redacted. --- .chloggen/configopaque_stringer.yaml | 2 +- config/configopaque/opaque.go | 16 +++++-- config/configopaque/opaque_test.go | 62 +++++++++++++++++++++++++--- 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/.chloggen/configopaque_stringer.yaml b/.chloggen/configopaque_stringer.yaml index 08b2f55612a..f2f1e0c70c9 100755 --- a/.chloggen/configopaque_stringer.yaml +++ b/.chloggen/configopaque_stringer.yaml @@ -7,7 +7,7 @@ change_type: breaking component: configopaque # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: configopaque.String implements `fmt.Stringer`, outputting [REDACTED] when used as `fmt.Sprintf("%s", opaquestring)` +note: configopaque.String implements `fmt.Stringer` and `fmt.GoStringer`, outputting [REDACTED] when formatted with the %s, %q or %#v verbs` # One or more tracking issues or pull requests related to the change issues: [9213] diff --git a/config/configopaque/opaque.go b/config/configopaque/opaque.go index f66c2829bb0..332bc0e0bde 100644 --- a/config/configopaque/opaque.go +++ b/config/configopaque/opaque.go @@ -8,11 +8,9 @@ import ( "fmt" ) -// String alias that is marshaled in an opaque way. +// String alias that is marshaled and printed in an opaque way. type String string -var _ fmt.Stringer = String("") - const maskedString = "[REDACTED]" var _ encoding.TextMarshaler = String("") @@ -22,6 +20,18 @@ func (s String) MarshalText() ([]byte, error) { return []byte(maskedString), nil } +var _ fmt.Stringer = String("") + +// String formats the string as `[REDACTED]`. +// This is used for the %s and %q verbs. func (s String) String() string { return maskedString } + +var _ fmt.GoStringer = String("") + +// GoString formats the string as `[REDACTED]`. +// This is used for the %#v verb. +func (s String) GoString() string { + return fmt.Sprintf("%#v", maskedString) +} diff --git a/config/configopaque/opaque_test.go b/config/configopaque/opaque_test.go index 49aa3d6763a..34a2c232467 100644 --- a/config/configopaque/opaque_test.go +++ b/config/configopaque/opaque_test.go @@ -4,11 +4,13 @@ package configopaque // import "go.opentelemetry.io/collector/config/configopaque" import ( + "encoding/json" "fmt" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" ) func TestStringMarshalText(t *testing.T) { @@ -16,17 +18,65 @@ func TestStringMarshalText(t *testing.T) { for _, example := range examples { opaque, err := example.MarshalText() require.NoError(t, err) - assert.Equal(t, "[REDACTED]", string(opaque)) + assert.Equal(t, maskedString, string(opaque)) } } +type TestStruct struct { + Opaque String `json:"opaque" yaml:"opaque"` + Plain string `json:"plain" yaml:"plain"` +} + +var example = TestStruct{ + Opaque: "opaque", + Plain: "plain", +} + +func TestStringJSON(t *testing.T) { + bytes, err := json.Marshal(example) + require.NoError(t, err) + assert.Equal(t, `{"opaque":"[REDACTED]","plain":"plain"}`, string(bytes)) +} + +func TestStringYAML(t *testing.T) { + bytes, err := yaml.Marshal(example) + require.NoError(t, err) + assert.Equal(t, "opaque: '[REDACTED]'\nplain: plain\n", string(bytes)) +} + func TestStringFmt(t *testing.T) { examples := []String{"opaque", "s", "veryveryveryveryveryveryveryveryveryverylong"} + verbs := []string{"%s", "%q", "%v", "%#v", "%+v", "%x"} for _, example := range examples { - // nolint S1025 - assert.Equal(t, "[REDACTED]", fmt.Sprintf("%s", example)) - // converting to a string allows to output as an unredacted string still: - // nolint S1025 - assert.Equal(t, string(example), fmt.Sprintf("%s", string(example))) + for _, verb := range verbs { + t.Run(fmt.Sprintf("%s/%s", string(example), verb), func(t *testing.T) { + assert.Equal(t, + fmt.Sprintf(verb, maskedString), + fmt.Sprintf(verb, example), + ) + }) + } + + for _, verb := range verbs { + t.Run(fmt.Sprintf("string(%s)/%s", string(example), verb), func(t *testing.T) { + // converting to a string allows to output as an unredacted string still: + var expected string + switch verb { + case "%s", "%v", "%+v": + expected = string(example) + case "%q", "%#v": + expected = "\"" + string(example) + "\"" + case "%x": + expected = fmt.Sprintf("%x", []byte(example)) + default: + t.Errorf("unexpected verb %q", verb) + } + + assert.Equal(t, + expected, + fmt.Sprintf(verb, string(example)), + ) + }) + } } }