diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cec01490..52ccde5ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `tag_values`. ## Fixed - Fixed a race condition in `lading_signal` that caused lading to hang on shutdown. +- Fixed a tag parsing bug that resulted in tags with hyphenated keys being merged with proceeding tag values. ## [0.30.0] ## Added diff --git a/lading/src/bin/lading.rs b/lading/src/bin/lading.rs index efea36e34..99cd19e80 100644 --- a/lading/src/bin/lading.rs +++ b/lading/src/bin/lading.rs @@ -102,16 +102,22 @@ impl FromStr for CliKeyValues { type Err = String; fn from_str(input: &str) -> Result { - // A key always matches `[[:alpha:]_]+` and a value conforms to - // `[[:alpha:]_:,`. A key is always followed by a '=' and then a - // value. A key and value pair are delimited from other pairs by - // ','. But note that ',' is a valid character in a value, so it's - // ambiguous whether the last member of a value after a ',' is a key. + // A key matches `[[:alnum:]_-]+` (letters, digits, underscores, + // hyphens) and a value conforms to `[[:alpha:]_:,`. A key is always + // followed by a '=' and then a value. A key and value pair are + // delimited from other pairs by ','. But note that ',' is a valid + // character in a value, so it's ambiguous whether the last member of + // a value after a ',' is a key. // // The approach taken here is to use the key notion as delimiter and // then tidy up afterward to find values. + // + // NOTE: the key regex must include hyphens and digits so that keys + // like "example-tag" or "image_tag" are matched as a single token. + // Without this, "example-tag=val" would be misparsed as key="tag" + // and the "example-" fragment would pollute the preceding value. static RE: LazyLock = LazyLock::new(|| { - Regex::new(r"([[:alpha:]_]+)=").expect("Invalid regex pattern provided") + Regex::new(r"([[:alnum:]_-]+)=").expect("Invalid regex pattern provided") }); let mut labels = FxHashMap::default(); @@ -895,6 +901,63 @@ generator: [] ); } + #[test] + fn cli_key_values_parses_hyphenated_keys() { + let val = "purpose=test,example-tag=example-value"; + let deser = + CliKeyValues::from_str(val).expect("String cannot be converted into CliKeyValues"); + + assert_eq!( + deser.get("purpose").expect("purpose key missing"), + "test", + "purpose value should be exactly 'test', not polluted by adjacent keys" + ); + assert_eq!( + deser.get("example-tag").expect("example-tag key missing"), + "example-value", + "hyphenated key 'example-tag' should be parsed as a single key" + ); + } + + #[test] + fn cli_key_values_parses_production_label_set() { + // Construct the comma-separated string the same way the harness does + let labels: Vec<(&str, &str)> = vec![ + ("team_id", "11111111"), + ("experiment", "some_experiment"), + ("example-tag", "example-value"), + ("image_tag", "1.11.1"), + ("target", "datadog-agent"), + ("replicate", "3"), + ("purpose", "testing"), + ]; + + let mut input = String::new(); + for (k, v) in &labels { + input.push_str(k); + input.push('='); + input.push_str(v); + input.push(','); + } + input.pop(); // remove trailing comma + + let deser = + CliKeyValues::from_str(&input).expect("String cannot be converted into CliKeyValues"); + + for (k, v) in &labels { + let actual = deser.get(*k).unwrap_or_else(|| { + panic!( + "key '{k}' missing from parsed result; got keys: {:?}", + deser.inner.keys().collect::>() + ) + }); + assert_eq!( + actual, *v, + "key '{k}': expected '{v}', got '{actual}' — possible cross-label pollution" + ); + } + } + #[tokio::test(flavor = "multi_thread")] async fn yaml_only_telemetry_configuration_works() { // Test that telemetry can be configured in YAML without CLI flags