Merge pull request #1188 from SteveL-MSFT/resourceid-encode

Encode name part of `resourceId()` function to allow name to accept any characters

Author: SteveL-MSFT

dsc/tests/dsc_functions.tests.ps1

@@ -123,7 +123,7 @@ Describe 'tests for function expressions' {
     @{ expression = "[intersection(parameters('nestedObject1'), parameters('nestedObject2'))]"; expected = [pscustomobject]@{
         shared = [pscustomobject]@{ value = 42; flag = $true }
         level  = 1
-      } 
+      }
     }
     @{ expression = "[intersection(parameters('nestedObject1'), parameters('nestedObject3'))]"; expected = [pscustomobject]@{ level = 1 } }
     @{ expression = "[intersection(parameters('nestedObject1'), parameters('nestedObject2'), parameters('nestedObject4'))]"; expected = [pscustomobject]@{ level = 1 } }
@@ -948,10 +948,10 @@ Describe 'tests for function expressions' {
     @{ testInput = ' ' }
   ) {
     param($testInput)
-    
+
     $expected = [Uri]::EscapeDataString($testInput)
     $expression = "[uriComponent('$($testInput -replace "'", "''")')]"
-    
+
     $config_yaml = @"
             `$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
             resources:
@@ -963,13 +963,13 @@ Describe 'tests for function expressions' {
     $out = $config_yaml | dsc config get -f - | ConvertFrom-Json
     $out.results[0].result.actualState.output | Should -BeExactly $expected
   }
-  
+
   It 'uriComponent function works with concat' {
     $input1 = 'hello'
     $input2 = ' '
     $input3 = 'world'
     $expected = [Uri]::EscapeDataString($input1 + $input2 + $input3)
-    
+
     $config_yaml = @"
             `$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
             resources:
@@ -995,10 +995,10 @@ Describe 'tests for function expressions' {
     @{ testInput = '100%25' }
   ) {
     param($testInput)
-    
+
     $expected = [Uri]::UnescapeDataString($testInput)
     $expression = "[uriComponentToString('$($testInput -replace "'", "''")')]"
-    
+
     $config_yaml = @"
             `$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
             resources:
@@ -1010,11 +1010,11 @@ Describe 'tests for function expressions' {
     $out = $config_yaml | dsc config get -f - | ConvertFrom-Json
     $out.results[0].result.actualState.output | Should -BeExactly $expected
   }
-  
+
   It 'uriComponentToString function works with round-trip encoding' {
     $original = 'hello world'
     $expected = $original
-    
+
     $config_yaml = @"
             `$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
             resources:
@@ -1026,11 +1026,11 @@ Describe 'tests for function expressions' {
     $out = $config_yaml | dsc config get -f - | ConvertFrom-Json
     $out.results[0].result.actualState.output | Should -BeExactly $expected
   }
-  
+
   It 'uriComponentToString function works with nested round-trip' {
     $original = '[email protected]'
     $expected = $original
-    
+
     $config_yaml = @"
             `$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
             resources:
@@ -1042,13 +1042,13 @@ Describe 'tests for function expressions' {
     $out = $config_yaml | dsc config get -f - | ConvertFrom-Json
     $out.results[0].result.actualState.output | Should -BeExactly $expected
   }
-  
+
   It 'uriComponentToString function works with concat' {
     $input1 = 'hello'
     $input2 = '%20'
     $input3 = 'world'
     $expected = [Uri]::UnescapeDataString($input1 + $input2 + $input3)
-    
+
     $config_yaml = @"
             `$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
             resources:
@@ -1077,7 +1077,7 @@ Describe 'tests for function expressions' {
     @{ data = @{ nested = @{ value = 123 } }; accessor = '.nested.value'; expected = 123 }
   ) {
     param($data, $accessor, $expected)
-    
+
     $jsonString = ConvertTo-Json -Compress -InputObject $data
     $expression = "[json(''$($jsonString)'')$accessor]"
 
@@ -1152,7 +1152,7 @@ Describe 'tests for function expressions' {
     @{ base = 'http://192.168.1.1/'; relative = 'api/v1'; expected = 'http://192.168.1.1/api/v1' }
   ) {
     param($base, $relative, $expected)
-    
+
     $expression = "[uri('$($base -replace "'", "''")','$($relative -replace "'", "''")')]"
     $config_yaml = @"
             `$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
@@ -1188,4 +1188,20 @@ Describe 'tests for function expressions' {
     $errorContent = Get-Content $TestDrive/error.log -Raw
     $errorContent | Should -Match ([regex]::Escape($expectedError))
   }
+
+  It 'resourceId allows for arbitrary characters in names including unicode' {
+    $name = 'My Resource @123/!#$%^&*()[]{}-+=;`~'
+    $config_yaml = @"
+            `$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
+            resources:
+            - name: "$name"
+              type: Microsoft.DSC.Debug/Echo
+              properties:
+                output: "[resourceId('Microsoft.DSC.Debug/Echo', '$name')]"
+"@
+    $out = dsc config get -i $config_yaml | ConvertFrom-Json
+    $LASTEXITCODE | Should -Be 0
+    $expected = "Microsoft.DSC.Debug/Echo:$([Uri]::EscapeDataString($name))"
+    $out.results[0].result.actualState.output | Should -BeExactly $expected
+  }
 }

lib/dsc-lib/locales/en-us.toml

@@ -476,9 +476,6 @@ unavailableInUserFunction = "The 'reference()' function is not available in user
 [functions.resourceId]
 description = "Constructs a resource ID from the given type and name"
 incorrectTypeFormat = "Type argument must contain exactly one slash"
-invalidFirstArgType = "Invalid argument type for first parameter"
-incorrectNameFormat = "Name argument cannot contain a slash"
-invalidSecondArgType = "Invalid argument type for second parameter"
 
 [functions.secret]
 description = "Retrieves a secret from a vault"

lib/dsc-lib/src/configure/depends_on.rs

@@ -42,7 +42,7 @@ pub fn get_resource_invocation_order(config: &Configuration, parser: &mut Statem
                 let (resource_type, resource_name) = get_type_and_name(string_result)?;
 
                 // find the resource by name
-                let Some(dependency_resource) = config.resources.iter().find(|r| r.name.eq(resource_name)) else {
+                let Some(dependency_resource) = config.resources.iter().find(|r| r.name.eq(&resource_name)) else {
                     return Err(DscError::Validation(t!("configure.dependsOn.dependencyNotFound", dependency_name = resource_name, resource_name = resource.name).to_string()));
                 };
                 // validate the type matches
@@ -91,12 +91,14 @@ pub fn get_resource_invocation_order(config: &Configuration, parser: &mut Statem
     Ok(order)
 }
 
-fn get_type_and_name(statement: &str) -> Result<(&str, &str), DscError> {
+fn get_type_and_name(statement: &str) -> Result<(&str, String), DscError> {
     let parts: Vec<&str> = statement.split(':').collect();
     if parts.len() != 2 {
         return Err(DscError::Validation(t!("configure.dependsOn.syntaxIncorrect", dependency = statement).to_string()));
     }
-    Ok((parts[0], parts[1]))
+    // the name is url encoded so we need to decode it
+    let decoded_name = urlencoding::decode(parts[1]).map_err(|_| DscError::Validation(t!("configure.dependsOn.syntaxIncorrect", dependency = statement).to_string()))?;
+    Ok((parts[0], decoded_name.into_owned()))
 }
 
 #[cfg(test)]

lib/dsc-lib/src/configure/mod.rs

@@ -15,6 +15,7 @@ use crate::DscResource;
 use crate::discovery::Discovery;
 use crate::parser::Statement;
 use crate::progress::{Failure, ProgressBar, ProgressFormat};
+use crate::util::resource_id;
 use self::config_doc::{Configuration, DataType, MicrosoftDscMetadata, Operation, SecurityContextKind};
 use self::depends_on::get_resource_invocation_order;
 use self::config_result::{ConfigurationExportResult, ConfigurationGetResult, ConfigurationSetResult, ConfigurationTestResult};
@@ -388,15 +389,15 @@ impl Configurator {
 
             match &mut get_result {
                 GetResult::Resource(ref mut resource_result) => {
-                    self.context.references.insert(format!("{}:{}", resource.resource_type, evaluated_name), serde_json::to_value(&resource_result.actual_state)?);
+                    self.context.references.insert(resource_id(&resource.resource_type, &evaluated_name), serde_json::to_value(&resource_result.actual_state)?);
                     get_metadata_from_result(Some(&mut self.context), &mut resource_result.actual_state, &mut metadata)?;
                 },
                 GetResult::Group(group) => {
                     let mut results = Vec::<Value>::new();
                     for result in group {
                         results.push(serde_json::to_value(&result.result)?);
                     }
-                    self.context.references.insert(format!("{}:{}", resource.resource_type, evaluated_name), Value::Array(results.clone()));
+                    self.context.references.insert(resource_id(&resource.resource_type, &evaluated_name), Value::Array(results.clone()));
                 },
             }
             let resource_result = config_result::ResourceGetResult {
@@ -559,15 +560,15 @@ impl Configurator {
             };
             match &mut set_result {
                 SetResult::Resource(resource_result) => {
-                    self.context.references.insert(format!("{}:{}", resource.resource_type, evaluated_name), serde_json::to_value(&resource_result.after_state)?);
+                    self.context.references.insert(resource_id(&resource.resource_type, &evaluated_name), serde_json::to_value(&resource_result.after_state)?);
                     get_metadata_from_result(Some(&mut self.context), &mut resource_result.after_state, &mut metadata)?;
                 },
                 SetResult::Group(group) => {
                     let mut results = Vec::<Value>::new();
                     for result in group {
                         results.push(serde_json::to_value(&result.result)?);
                     }
-                    self.context.references.insert(format!("{}:{}", resource.resource_type, evaluated_name), Value::Array(results.clone()));
+                    self.context.references.insert(resource_id(&resource.resource_type, &evaluated_name), Value::Array(results.clone()));
                 },
             }
             let resource_result = config_result::ResourceSetResult {
@@ -637,15 +638,15 @@ impl Configurator {
             };
             match &mut test_result {
                 TestResult::Resource(resource_test_result) => {
-                    self.context.references.insert(format!("{}:{}", resource.resource_type, evaluated_name), serde_json::to_value(&resource_test_result.actual_state)?);
+                    self.context.references.insert(resource_id(&resource.resource_type, &evaluated_name), serde_json::to_value(&resource_test_result.actual_state)?);
                     get_metadata_from_result(Some(&mut self.context), &mut resource_test_result.actual_state, &mut metadata)?;
                 },
                 TestResult::Group(group) => {
                     let mut results = Vec::<Value>::new();
                     for result in group {
                         results.push(serde_json::to_value(&result.result)?);
                     }
-                    self.context.references.insert(format!("{}:{}", resource.resource_type, evaluated_name), Value::Array(results.clone()));
+                    self.context.references.insert(resource_id(&resource.resource_type, &evaluated_name), Value::Array(results.clone()));
                 },
             }
             let resource_result = config_result::ResourceTestResult {
@@ -707,7 +708,7 @@ impl Configurator {
                     return Err(e);
                 },
             };
-            self.context.references.insert(format!("{}:{}", resource.resource_type, evaluated_name), serde_json::to_value(&export_result.actual_state)?);
+            self.context.references.insert(resource_id(&resource.resource_type, &evaluated_name), serde_json::to_value(&export_result.actual_state)?);
             progress.set_result(&serde_json::to_value(export_result)?);
             progress.write_increment(1);
         }

lib/dsc-lib/src/functions/resource_id.rs

@@ -4,6 +4,7 @@
 use crate::DscError;
 use crate::configure::context::Context;
 use crate::functions::{FunctionArgKind, Function, FunctionCategory, FunctionMetadata};
+use crate::util::resource_id;
 use rust_i18n::t;
 use serde_json::Value;
 
@@ -28,32 +29,15 @@ impl Function for ResourceId {
     }
 
     fn invoke(&self, args: &[Value], _context: &Context) -> Result<Value, DscError> {
-        let mut result = String::new();
         // first argument is the type and must contain only 1 slash
-        let resource_type = &args[0];
-        if let Some(value) = resource_type.as_str() {
-            let slash_count = value.chars().filter(|c| *c == '/').count();
-            if slash_count != 1 {
-                return Err(DscError::Function("resourceId".to_string(), t!("functions.resourceId.incorrectTypeFormat").to_string()));
-            }
-            result.push_str(value);
-        } else {
-            return Err(DscError::Parser(t!("functions.resourceId.invalidFirstArgType").to_string()));
-        }
-        // ARM uses a slash separator, but here we use a colon which is not allowed for the type nor name
-        result.push(':');
-        // second argument is the name and must contain no slashes
-        let resource_name = &args[1];
-        if let Some(value) = resource_name.as_str() {
-            if value.contains('/') {
-                return Err(DscError::Function("resourceId".to_string(), t!("functions.resourceId.incorrectNameFormat").to_string()));
-            }
-
-            result.push_str(value);
-        } else {
-            return Err(DscError::Parser(t!("functions.resourceId.invalidSecondArgType").to_string()));
+        let resource_type = &args[0].as_str().unwrap();
+        let slash_count = resource_type.chars().filter(|c| *c == '/').count();
+        if slash_count != 1 {
+            return Err(DscError::Function("resourceId".to_string(), t!("functions.resourceId.incorrectTypeFormat").to_string()));
         }
 
+        let resource_name = &args[1].as_str().unwrap();
+        let result = resource_id(resource_type, resource_name);
         Ok(Value::String(result))
     }
 }
@@ -85,10 +69,10 @@ mod tests {
     }
 
     #[test]
-    fn invalid_name() {
+    fn valid_name_with_slashes() {
         let mut parser = Statement::new().unwrap();
-        let result = parser.parse_and_execute("[resourceId('a','b/c')]", &Context::new());
-        assert!(result.is_err());
+        let result = parser.parse_and_execute("[resourceId('a/a','b/c/d')]", &Context::new()).unwrap();
+        assert_eq!(result, "a/a:b%2Fc%2Fd");
     }
 
     #[test]

lib/dsc-lib/src/util.rs

@@ -213,6 +213,24 @@ fn get_settings_policy_file_path() -> String
     Path::new("/etc").join("dsc").join("dsc.settings.json").display().to_string()
 }
 
+/// Generates a resource ID from the specified type and name.
+///
+/// # Arguments
+/// * `type_name` - The resource type in the format "namespace/type".
+/// * `name` - The resource name.
+///
+/// # Returns
+/// A string that holds the resource ID in the format "namespace/type:name".
+#[must_use]
+pub fn resource_id(type_name: &str, name: &str) -> String {
+    let mut result = String::new();
+    result.push_str(type_name);
+    result.push(':');
+    let encoded = urlencoding::encode(name);
+    result.push_str(&encoded);
+    result
+}
+
 #[macro_export]
 macro_rules! locked_is_empty {
     ($lockable:expr) => {{