Skip to content

Commit becb912

Browse files
SteveL-MSFTSteveL-MSFT
authored
Merge pull request #1446 from SteveL-MSFT/discover-invalid
Backport: Fix discovering incompatible resource manifests to be non-fatal
2 parents f238547 + e62b3dd commit becb912

9 files changed

Lines changed: 110 additions & 17 deletions

File tree

‎build.ps1‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -263,9 +263,9 @@ if ($null -ne $packageType) {
263263
if ($null -eq (Get-Command tree-sitter -ErrorAction Ignore)) {
264264
Write-Verbose -Verbose "tree-sitter not found, installing..."
265265
if ($UseCFS) {
266-
cargo install tree-sitter-cli --config .cargo/config.toml
266+
cargo install tree-sitter-cli --config .cargo/config.toml --version 0.25.10
267267
} else {
268-
cargo install tree-sitter-cli
268+
cargo install tree-sitter-cli --version 0.25.10
269269
}
270270
if ($LASTEXITCODE -ne 0) {
271271
throw "Failed to install tree-sitter-cli"
@@ -349,8 +349,8 @@ if (!$SkipBuild) {
349349
"y2j"
350350
)
351351
$pedantic_unclean_projects = @()
352-
$clippy_unclean_projects = @("grammars/tree-sitter-dscexpression", "grammars/tree-sitter-ssh-server-config")
353-
$skip_test_projects_on_windows = @("grammars/tree-sitter-dscexpression", "grammars/tree-sitter-ssh-server-config")
352+
$clippy_unclean_projects = @("tree-sitter-dscexpression")
353+
$skip_test_projects_on_windows = @("tree-sitter-dscexpression")
354354

355355
if ($IsWindows) {
356356
$projects += $windows_projects

‎dsc/Cargo.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dsc/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "dsc"
3-
version = "3.1.2"
3+
version = "3.1.3"
44
edition = "2021"
55

66
[profile.release]

‎dsc/tests/dsc_discovery.tests.ps1‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ Describe 'tests for resource discovery' {
7474
$resources.Count | Should -Be 0
7575
}
7676

77-
It 'warns on invalid semver' {
77+
It 'info on invalid semver' {
7878
$manifest = @'
7979
{
8080
"$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json",
@@ -94,9 +94,8 @@ Describe 'tests for resource discovery' {
9494
try {
9595
$env:DSC_RESOURCE_PATH = $testdrive
9696
Set-Content -Path "$testdrive/test.dsc.resource.json" -Value $manifest
97-
$out = dsc resource list 2>&1
98-
write-verbose -verbose ($out | Out-String)
99-
$out | Should -Match 'WARN.*?Validation.*?invalid version' -Because ($out | Out-String)
97+
$out = dsc -l info resource list 2>&1 | Out-String
98+
$out | Should -BeLike '*INFO*Validation*invalid version*' -Because ($out | Out-String)
10099
}
101100
finally {
102101
$env:DSC_RESOURCE_PATH = $oldPath
@@ -302,4 +301,32 @@ Describe 'tests for resource discovery' {
302301
$env:DSC_RESOURCE_PATH = $null
303302
}
304303
}
304+
305+
It 'Invalid resource manifest will generate info message' {
306+
$invalidManifest = @'
307+
{
308+
"$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json",
309+
"type": "Test/InvalidManifest",
310+
"version": "0.1.0",
311+
"get": {
312+
"executable": "dsctest",
313+
"unexpectedField": "This field is not expected in the get section and should be ignored by the discovery process"
314+
},
315+
"newProperty": "This property is not expected in the manifest and should be ignored by the discovery process"
316+
}
317+
'@
318+
Set-Content -Path "$testdrive/test.dsc.resource.json" -Value $invalidManifest
319+
$oldPath = $env:DSC_RESOURCE_PATH
320+
try {
321+
$env:DSC_RESOURCE_PATH = $testdrive + [System.IO.Path]::PathSeparator + $env:PATH
322+
323+
$out = dsc -l info resource list 'Test/InvalidManifest' 2> "$testdrive/error.txt" | ConvertFrom-Json
324+
$LASTEXITCODE | Should -Be 0
325+
$out | Should -BeNullOrEmpty -Because (Get-Content -Raw -Path "$testdrive/error.txt")
326+
$errorLog = Get-Content -Raw -Path "$testdrive/error.txt"
327+
$errorLog | Should -BeLike "*INFO Failed to load manifest: Manifest: Invalid manifest for resource '*test.dsc.resource.json'*" -Because $errorLog
328+
} finally {
329+
$env:DSC_RESOURCE_PATH = $oldPath
330+
}
331+
}
305332
}

‎dsc/tests/dsc_extension_discover.tests.ps1‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,4 +101,61 @@ Describe 'Discover extension tests' {
101101
}
102102
$foundWideLine | Should -BeTrue
103103
}
104+
105+
It 'Invalid manifest from extension discovery should not fail overall discovery' {
106+
$invalidManifest = @'
107+
{
108+
"$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json",
109+
"type": "Test/InvalidManifest",
110+
"version": "0.1.0",
111+
"get": {
112+
"executable": "dsctest",
113+
"unexpectedField": "This field is not expected in the get section and should be ignored by the discovery process"
114+
},
115+
"newProperty": "This property is not expected in the manifest and should be ignored by the discovery process"
116+
}
117+
'@
118+
$resourceScript = @'
119+
$resource = @{
120+
manifestPath = "$env:TestDrive" + [System.IO.Path]::DirectorySeparatorChar + 'invalidManifest.dsc.resource.json'
121+
}
122+
$resource | ConvertTo-Json -Compress
123+
'@
124+
$extensionManifest = @'
125+
{
126+
"$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json",
127+
"type": "Test/DiscoverInvalid",
128+
"version": "0.1.0",
129+
"description": "Test discover resource, this is a really long description to test that the table can be rendered without truncating the description text from this extension.",
130+
"discover": {
131+
"executable": "pwsh",
132+
"args": [
133+
"-NoLogo",
134+
"-NonInteractive",
135+
"-NoProfile",
136+
"-Command",
137+
"./discover.ps1"
138+
]
139+
}
140+
}
141+
'@
142+
143+
Set-Content -Path "$TestDrive/invalidManifest.dsc.resource.json" -Value $invalidManifest
144+
Set-Content -Path "$TestDrive/discover.ps1" -Value $resourceScript
145+
Set-Content -Path "$TestDrive/extension.dsc.extension.json" -Value $extensionManifest
146+
try {
147+
$env:DSC_RESOURCE_PATH = $TestDrive + [System.IO.Path]::PathSeparator + $env:PATH
148+
$env:TestDrive = $TestDrive
149+
$out = dsc -l info resource list 2> $TestDrive/error.log | ConvertFrom-Json
150+
$LASTEXITCODE | Should -Be 0 -Because (Get-Content -Path "$TestDrive/error.log" -Raw | Out-String)
151+
# The invalid manifest should be skipped and not included in the discovered resources
152+
foreach ($resource in $out) {
153+
$resource.type | Should -Not -Be 'Test/InvalidManifest'
154+
}
155+
(Get-Content -Path "$TestDrive/error.log" -Raw) | Should -BeLike "*INFO Extension 'Test/DiscoverInvalid' failed: Manifest: Invalid manifest for resource '*invalidManifest.dsc.resource.json'*" -Because (Get-Content -Path "$TestDrive/error.log" -Raw | Out-String)
156+
} finally {
157+
$env:DSC_RESOURCE_PATH = $null
158+
$env:TestDrive = $null
159+
}
160+
}
104161
}

‎dsc_lib/locales/en-us.toml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ extensionResourceFound = "Extension found resource '%{resource}'"
9191
callingExtension = "Calling extension '%{extension}' to discover resources"
9292
extensionFoundResources = "Extension '%{extension}' found %{count} resources"
9393
invalidManifestVersion = "Manifest '%{path}' has invalid version: %{err}"
94+
failedLoadManifest = "Failed to load manifest: %{err}"
9495

9596
[dscresources.commandResource]
9697
invokeGet = "Invoking get for '%{resource}'"
@@ -173,6 +174,7 @@ resourceManifestSchemaDescription = "Defines the JSON Schema the resource manife
173174
discoverNoResults = "No results returned for discovery extension '%{extension}'"
174175
discoverNotAbsolutePath = "Resource path from extension '%{extension}' is not an absolute path: %{path}"
175176
extensionReturned = "Extension '%{extension}' returned line: %{line}"
177+
failedLoadManifest = "Extension '%{extension}' failed: %{err}"
176178

177179
[extensions.extension_manifest]
178180
extensionManifestSchemaTitle = "Extension manifest schema URI"

‎dsc_lib/src/discovery/command_discovery.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,9 @@ impl ResourceDiscovery for CommandDiscovery {
247247
Err(e) => {
248248
// At this point we can't determine whether or not the bad manifest contains
249249
// resource that is requested by resource/config operation
250-
// if it is, then "ResouceNotFound" error will be issued later
251-
// and here we just write as warning
252-
warn!("{e}");
250+
// if it is, then "ResourceNotFound" error will be issued later
251+
// and here we just write as information
252+
info!("{}", t!("discovery.commandDiscovery.failedLoadManifest", err = e));
253253
continue;
254254
},
255255
};

‎dsc_lib/src/dscerror.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use std::str::Utf8Error;
66

77
use indicatif::style::TemplateError;
88
use thiserror::Error;
9-
use tracing::error;
109
use tree_sitter::LanguageError;
1110

1211
#[derive(Error, Debug)]

‎dsc_lib/src/extensions/dscextension.rs‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,17 @@ impl DscExtension {
111111
}
112112
let manifest_path = Path::new(&discover_result.manifest_path);
113113
// Currently we don't support extensions discovering other extensions
114-
if let ImportedManifest::Resource(resource) = load_manifest(manifest_path)? {
115-
resources.push(resource);
116-
}
114+
let resource = match load_manifest(manifest_path) {
115+
Ok(ImportedManifest::Resource(resource)) => resource,
116+
Ok(_) => continue,
117+
Err(err) => {
118+
// For invalid manifest, we write an information and skip it
119+
info!("{}", t!("extensions.dscextension.failedLoadManifest", extension = self.type_name, path = discover_result.manifest_path.clone(), err = err));
120+
continue;
121+
}
122+
};
123+
124+
resources.push(resource);
117125
}
118126
}
119127

0 commit comments

Comments
 (0)