Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions internal/config/json_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,9 @@ func (p *JSONStore) SetWithScope(scope string, key FieldKey, value interface{})
// the fields that appear underneath it. The resulting config will be persisted to
// disk if PersistToDisk has been used.
func (p *JSONStore) RemoveScope(scope string) error {
path := scope
path := escapePathSegment(scope)
if p.scope != "" {
path = fmt.Sprintf("%s.%s", p.scope, scope)
path = fmt.Sprintf("%s.%s", escapePathSegment(p.scope), path)
}

return p.deletePath(path)
Expand Down Expand Up @@ -498,18 +498,30 @@ func (p *JSONStore) GetFieldDefinition(key FieldKey) *FieldDefinition {
}

func (p *JSONStore) getPath(scope string, key FieldKey) string {
path := string(key)
path := escapePathSegment(string(key))
if scope != "" {
path = fmt.Sprintf("%s.%s", scope, key)
path = fmt.Sprintf("%s.%s", escapePathSegment(scope), path)
}

if p.scope != "" {
path = fmt.Sprintf("%s.%s", p.scope, path)
path = fmt.Sprintf("%s.%s", escapePathSegment(p.scope), path)
}

return path
}

// escapePathSegment escapes the gjson/sjson path separator and escape character
// inside a single segment so values like profile names containing a "." are
// stored as a single literal key (e.g. {"first.last": {...}}) rather than as a
// nested object hierarchy.
func escapePathSegment(s string) string {
// Order matters: escape backslashes first so we don't double-escape the
// backslashes we introduce when escaping dots below.
s = strings.ReplaceAll(s, `\`, `\\`)
s = strings.ReplaceAll(s, `.`, `\.`)
return s
}

func (p *JSONStore) deletePath(path string) error {
p.mu.Lock()
defer p.mu.Unlock()
Expand Down
67 changes: 67 additions & 0 deletions internal/config/json_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -770,3 +770,70 @@ func TestStore_GetScopes(t *testing.T) {
s := p.GetScopes()
require.Equal(t, 2, len(s))
}

// The following tests guard against a regression where scope names containing
// a "." (e.g. profile names such as "ava.morgan") were treated by gjson/sjson
// as nested object paths, causing values to be silently lost on read and
// scopes to disappear from list operations.

func TestStore_SetWithScope_DottedScopeName(t *testing.T) {
f, err := ioutil.TempFile("", "newrelic-cli.config_provider_test.*.json")
require.NoError(t, err)
defer f.Close()

p, err := NewJSONStore(PersistToFile(f.Name()))
require.NoError(t, err)

err = p.SetWithScope("ava.morgan", FieldKey("apiKey"), "NRAK-DOTTED-1")
require.NoError(t, err)

v, err := p.GetStringWithScope("ava.morgan", FieldKey("apiKey"))
require.NoError(t, err)
require.Equal(t, "NRAK-DOTTED-1", v)
}

func TestStore_GetScopes_DottedScopeName(t *testing.T) {
f, err := ioutil.TempFile("", "newrelic-cli.config_provider_test.*.json")
require.NoError(t, err)
defer f.Close()

p, err := NewJSONStore(PersistToFile(f.Name()))
require.NoError(t, err)

err = p.SetWithScope("riley.chen", FieldKey("apiKey"), "NRAK-DOTTED-2")
require.NoError(t, err)
err = p.SetWithScope("prod.us-east-1", FieldKey("apiKey"), "NRAK-DOTTED-3")
require.NoError(t, err)
err = p.SetWithScope("staging", FieldKey("apiKey"), "NRAK-PLAIN-4")
require.NoError(t, err)

scopes := p.GetScopes()
require.ElementsMatch(t,
[]string{"riley.chen", "prod.us-east-1", "staging"},
scopes,
)
}

func TestStore_RemoveScope_DottedScopeName(t *testing.T) {
f, err := ioutil.TempFile("", "newrelic-cli.config_provider_test.*.json")
require.NoError(t, err)
defer f.Close()

p, err := NewJSONStore(PersistToFile(f.Name()))
require.NoError(t, err)

err = p.SetWithScope("team.platform", FieldKey("apiKey"), "NRAK-DOTTED-5")
require.NoError(t, err)
err = p.SetWithScope("team.observability", FieldKey("apiKey"), "NRAK-DOTTED-6")
require.NoError(t, err)

err = p.RemoveScope("team.platform")
require.NoError(t, err)

// The removed scope is gone, the sibling that shares the "team" prefix is intact.
require.ElementsMatch(t, []string{"team.observability"}, p.GetScopes())

v, err := p.GetStringWithScope("team.observability", FieldKey("apiKey"))
require.NoError(t, err)
require.Equal(t, "NRAK-DOTTED-6", v)
}
Loading