Skip to content

Commit

Permalink
[component] Restrict character set for component.ID name (#10674)
Browse files Browse the repository at this point in the history
#### Description
While working on
#10495 we
discovered that there are not any restrictions on the `name` field of a
`component.ID`. There are restrictions on the `type` field introduced in
#9208. This PR adds similar restrictions to `name`.

A type must
- have at least one character,
- start with an ASCII alphabetic character and
- can only contain ASCII alphanumeric characters and '_'.

I found that we need a slightly different set of rules for name as some
tests use a digit and others use a uuid as a name. A name is still
optional, but if it's provided it must:
- have at least one character,
- start with an ASCII alphanumeric character and
- can only contain ASCII alphanumeric characters, '_', and '-'.

I'd be willing to adjust these restrictions if anyone has any opinions
on what should or should not be allowed.

<!-- Issue number if applicable -->
#### Link to tracking issue
Fixes #10673

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Unit tests

<!--Describe the documentation added.-->
#### Documentation
Code comments

<!--Please delete paragraphs that you did not use before submitting.-->
  • Loading branch information
mwear committed Jul 25, 2024
1 parent 192bfe3 commit 95902c1
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 0 deletions.
25 changes: 25 additions & 0 deletions .chloggen/nameval_refactor.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# 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. otlpreceiver)
component: component

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Adds restrictions on the character set for component.ID name.

# One or more tracking issues or pull requests related to the change
issues: [10673]

# (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:

# 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: []
12 changes: 12 additions & 0 deletions component/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,3 +164,15 @@ var (
// DataTypeLogs is the data type tag for logs.
DataTypeLogs = mustNewDataType("logs")
)

// nameRegexp is used to validate the name of a component. A name can consist of
// 1 to 63 unicode characters excluding whitespace, control characters, and
// symbols.
var nameRegexp = regexp.MustCompile(`^[^\pZ\pC\pS]{1,63}$`)

func validateName(nameStr string) error {
if !nameRegexp.MatchString(nameStr) {
return fmt.Errorf("invalid character(s) in name %q", nameStr)
}
return nil
}
3 changes: 3 additions & 0 deletions component/identifiable.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ func (id *ID) UnmarshalText(text []byte) error {
if nameStr == "" {
return fmt.Errorf("in %q id: the part after %s should not be empty", idStr, typeAndNameSeparator)
}
if err := validateName(nameStr); err != nil {
return fmt.Errorf("in %q id: %w", nameStr, err)
}
}

var err error
Expand Down
16 changes: 16 additions & 0 deletions component/identifiable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ func TestUnmarshalText(t *testing.T) {
idStr: " valid_type / valid_name ",
expectedID: ID{typeVal: validType, nameVal: "valid_name"},
},
{
idStr: "valid_type/中文好",
expectedID: ID{typeVal: validType, nameVal: "中文好"},
},
{
idStr: "valid_type/name-with-dashes",
expectedID: ID{typeVal: validType, nameVal: "name-with-dashes"},
},
{
idStr: "valid_type/1",
expectedID: ID{typeVal: validType, nameVal: "1"},
},
{
idStr: "/valid_name",
expectedErr: true,
Expand All @@ -55,6 +67,10 @@ func TestUnmarshalText(t *testing.T) {
idStr: " ",
expectedErr: true,
},
{
idStr: "valid_type/invalid name",
expectedErr: true,
},
}

for _, test := range testCases {
Expand Down

0 comments on commit 95902c1

Please sign in to comment.