Skip to content

Enforce Name not being a valid UUID #1424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
just-be-dev merged 12 commits into from
Jul 15, 2022
Merged

Enforce Name not being a valid UUID #1424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5266e9d
Ensure names don't overlap with UUIDs
just-be-dev Jul 13, 2022
e42f6fa
Fix formatting
just-be-dev Jul 13, 2022
a2a6dfa
Merge branch 'main' into no-name-id-overlap
just-be-dev Jul 14, 2022
8174c31
Note Name cannot be a UUID
just-be-dev Jul 14, 2022
89956ed
Update Name regex
just-be-dev Jul 14, 2022
60aae26
Add extra test for a UUID Name w/ a suffix
just-be-dev Jul 14, 2022
9a6231f
Update regex to match upper case chars
just-be-dev Jul 14, 2022
a284138
Update api spec
just-be-dev Jul 14, 2022
83f7ffe
Update common/src/api/external/mod.rs
just-be-dev Jul 14, 2022
34bd0ee
Last tweak to the regex to remove unnecessary slash
just-be-dev Jul 15, 2022
0506515
Fix tests, add space in docs
just-be-dev Jul 15, 2022
707448a
Out of date test snapshots, corret a test using a UUID as name
just-be-dev Jul 15, 2022
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
34 changes: 30 additions & 4 deletions common/src/api/external/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,12 @@ impl TryFrom<String> for Name {
return Err(String::from("name cannot end with \"-\""));
}

if Uuid::parse_str(&value).is_ok() {
return Err(String::from(
"name cannot be a UUID to avoid ambiguity with IDs",
));
}

Ok(Name(value))
}
}
Expand Down Expand Up @@ -216,7 +222,8 @@ impl JsonSchema for Name {
description: Some(
"Names must begin with a lower case ASCII letter, be \
composed exclusively of lowercase ASCII, uppercase \
ASCII, numbers, and '-', and may not end with a '-'."
ASCII, numbers, and '-', and may not end with a '-'. \
Names cannot be a UUID though they may contain a UUID."
.to_string(),
),
..Default::default()
Expand All @@ -225,7 +232,16 @@ impl JsonSchema for Name {
string: Some(Box::new(schemars::schema::StringValidation {
max_length: Some(63),
min_length: None,
pattern: Some("^[a-z](|[a-zA-Z0-9-]*[a-zA-Z0-9])$".to_string()),
pattern: Some(
concat!(
r#"^"#,
// Cannot match a UUID
r#"(?![0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$)"#,
r#"^[a-z][a-z0-9-]*[a-zA-Z0-9]"#,
r#"$"#,
)
.to_string(),
)
})),
..Default::default()
}
Expand Down Expand Up @@ -1976,6 +1992,10 @@ mod test {
"name contains invalid character: \"\u{00e9}\" (allowed \
characters are lowercase ASCII, digits, and \"-\")",
),
(
"a7e55044-10b1-426f-9247-bb680e5fe0c8",
"name cannot be a UUID to avoid ambiguity with IDs",
),
];

for (input, expected_message) in error_cases {
Expand All @@ -1984,8 +2004,14 @@ mod test {
}

// Success cases
let valid_names: Vec<&str> =
vec!["abc", "abc-123", "a123", &long_name[0..63]];
let valid_names: Vec<&str> = vec![
"abc",
"abc-123",
"a123",
"ok-a7e55044-10b1-426f-9247-bb680e5fe0c8",
"a7e55044-10b1-426f-9247-bb680e5fe0c8-ok",
&long_name[0..63],
];

for name in valid_names {
eprintln!("check name \"{}\" (should be valid)", name);
Expand Down
8 changes: 4 additions & 4 deletions common/tests/output/pagination-schema.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,10 @@ schema for pagination parameters: page selector, scan by name only
"definitions": {
"Name": {
"title": "A name unique within the parent collection",
"description": "Names must begin with a lower case ASCII letter, be composed exclusively of lowercase ASCII, uppercase ASCII, numbers, and '-', and may not end with a '-'.",
"description": "Names must begin with a lower case ASCII letter, be composed exclusively of lowercase ASCII, uppercase ASCII, numbers, and '-', and may not end with a '-'. Names cannot be a UUID though they may contain a UUID.",
"type": "string",
"maxLength": 63,
"pattern": "^[a-z](|[a-zA-Z0-9-]*[a-zA-Z0-9])$"
"pattern": "^(?![0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$)^[a-z][a-z0-9-]*[a-zA-Z0-9]$"
},
"NameSortMode": {
"description": "Supported set of sort modes for scanning by name only\n\nCurrently, we only support scanning in ascending order.",
Expand Down Expand Up @@ -186,10 +186,10 @@ schema for pagination parameters: page selector, scan by name or id
"definitions": {
"Name": {
"title": "A name unique within the parent collection",
"description": "Names must begin with a lower case ASCII letter, be composed exclusively of lowercase ASCII, uppercase ASCII, numbers, and '-', and may not end with a '-'.",
"description": "Names must begin with a lower case ASCII letter, be composed exclusively of lowercase ASCII, uppercase ASCII, numbers, and '-', and may not end with a '-'. Names cannot be a UUID though they may contain a UUID.",
"type": "string",
"maxLength": 63,
"pattern": "^[a-z](|[a-zA-Z0-9-]*[a-zA-Z0-9])$"
"pattern": "^(?![0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$)^[a-z][a-z0-9-]*[a-zA-Z0-9]$"
},
"NameOrIdMarker": {
"oneOf": [
Expand Down
6 changes: 3 additions & 3 deletions nexus/tests/integration_tests/basic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,10 +476,10 @@ async fn test_projects_list(cptestctx: &ControlPlaneTestContext) {
for _ in 0..projects_total {
// We'll use uuids for the names to make sure that works, and that we
// can paginate through by _name_ even though the names happen to be
// uuids. Names have to start with a letter, though, so we've got to
// make sure our uuid has one.
// uuids. Names have to start with a letter and can't exactly match

@davepacheco davepacheco Jul 15, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't believe we actually had a case doing exactly this! How was it working before?

@just-be-dev just-be-dev Jul 15, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the test? The first character of the UUID was replaced with a lowercase ascii character so it was a bit artificial.

@davepacheco davepacheco Jul 16, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh of course 🤦

// a uuid though, so we'll use a prefix.
let mut name = Uuid::new_v4().to_string();
name.replace_range(0..1, "a");
name.insert_str(0, "project-");
let project = create_project(&client, org_name, &name).await;
projects_created.push(project.identity);
}
Expand Down
4 changes: 2 additions & 2 deletions openapi/nexus.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8290,9 +8290,9 @@
},
"Name": {
"title": "A name unique within the parent collection",
"description": "Names must begin with a lower case ASCII letter, be composed exclusively of lowercase ASCII, uppercase ASCII, numbers, and '-', and may not end with a '-'.",
"description": "Names must begin with a lower case ASCII letter, be composed exclusively of lowercase ASCII, uppercase ASCII, numbers, and '-', and may not end with a '-'. Names cannot be a UUID though they may contain a UUID.",
"type": "string",
"pattern": "^[a-z](|[a-zA-Z0-9-]*[a-zA-Z0-9])$",
"pattern": "^(?![0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$)^[a-z][a-z0-9-]*[a-zA-Z0-9]$",
"maxLength": 63
},
"NetworkInterface": {
Expand Down
4 changes: 2 additions & 2 deletions openapi/sled-agent.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1053,9 +1053,9 @@
},
"Name": {
"title": "A name unique within the parent collection",
"description": "Names must begin with a lower case ASCII letter, be composed exclusively of lowercase ASCII, uppercase ASCII, numbers, and '-', and may not end with a '-'.",
"description": "Names must begin with a lower case ASCII letter, be composed exclusively of lowercase ASCII, uppercase ASCII, numbers, and '-', and may not end with a '-'. Names cannot be a UUID though they may contain a UUID.",
"type": "string",
"pattern": "^[a-z](|[a-zA-Z0-9-]*[a-zA-Z0-9])$",
"pattern": "^(?![0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$)^[a-z][a-z0-9-]*[a-zA-Z0-9]$",
"maxLength": 63
},
"NetworkInterface": {
Expand Down