Skip to content

Commit 82e71c8

Browse files
author
Steve Lee (POWERSHELL HE/HIM) (from Dev Box)
committed
Fix to emit info message if invalid manifest is found
1 parent 2d09a2a commit 82e71c8

6 files changed

Lines changed: 46 additions & 12 deletions

File tree

dsc/tests/dsc_discovery.tests.ps1

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,4 +302,24 @@ Describe 'tests for resource discovery' {
302302
$env:DSC_RESOURCE_PATH = $null
303303
}
304304
}
305+
306+
It 'Invalid resource manifest will generate info message' {
307+
$invalidManifest = @'
308+
{
309+
"$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json",
310+
"type": "Test/InvalidManifest",
311+
"version": "0.1.0",
312+
"get": {
313+
"executable": "dsctest",
314+
"unexpectedField": "This field is not expected in the get section and should be ignored by the discovery process"
315+
},
316+
"newProperty": "This property is not expected in the manifest and should be ignored by the discovery process"
317+
}
318+
'@
319+
Set-Content -Path "$testdrive/test.dsc.resource.json" -Value $invalidManifest
320+
$out = dsc -l info resource list 'Test/InvalidManifest' 2> "$testdrive/error.txt" | ConvertFrom-Json
321+
$LASTEXITCODE | Should -Be 0
322+
$out | Should -BeNullOrEmpty -Because (Get-Content -Raw -Path "$testdrive/error.txt")
323+
Get-Content -Raw -Path "$testdrive/error.txt" | Should -Match "INFO Failed to load manifest for resource '.*test.dsc.resource.json':" -Because (Get-Content -Raw -Path "$testdrive/error.txt")
324+
}
305325
}

dsc_lib/locales/en-us.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ 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+
failedImportManifest = "Failed to import manifest for resource '%{resource}': %{err}"
95+
failedLoadManifest = "Failed to load manifest for resource '%{resource}': %{err}"
9496

9597
[dscresources.commandResource]
9698
invokeGet = "Invoking get for '%{resource}'"
@@ -173,6 +175,7 @@ resourceManifestSchemaDescription = "Defines the JSON Schema the resource manife
173175
discoverNoResults = "No results returned for discovery extension '%{extension}'"
174176
discoverNotAbsolutePath = "Resource path from extension '%{extension}' is not an absolute path: %{path}"
175177
extensionReturned = "Extension '%{extension}' returned line: %{line}"
178+
failedImportManifest = "Failed to import manifest for resource '%{resource}': %{err}"
176179

177180
[extensions.extension_manifest]
178181
extensionManifestSchemaTitle = "Extension manifest schema URI"

dsc_lib/src/discovery/command_discovery.rs

Lines changed: 10 additions & 4 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", resource = path.to_string_lossy(), err = e).to_string());
253253
continue;
254254
},
255255
};
@@ -277,7 +277,13 @@ impl ResourceDiscovery for CommandDiscovery {
277277
ImportedManifest::Resource(resource) => {
278278
if regex.is_match(&resource.type_name) {
279279
if let Some(ref manifest) = resource.manifest {
280-
let manifest = import_manifest(manifest.clone())?;
280+
let manifest = match import_manifest(manifest.clone()) {
281+
Ok(manifest) => manifest,
282+
Err(err) => {
283+
info!("{}", t!("discovery.commandDiscovery.failedImportManifest", resource = resource.type_name, err = err).to_string());
284+
continue;
285+
}
286+
};
281287
if manifest.kind == Some(Kind::Adapter) {
282288
trace!("{}", t!("discovery.commandDiscovery.adapterFound", adapter = resource.type_name));
283289
insert_resource(&mut adapters, &resource, true);

dsc_lib/src/discovery/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::extensions::dscextension::DscExtension;
99
use crate::{dscresources::dscresource::DscResource, dscerror::DscError, progress::ProgressFormat};
1010
use std::collections::BTreeMap;
1111
use command_discovery::ImportedManifest;
12-
use tracing::error;
12+
use tracing::{error, warn};
1313

1414
#[derive(Clone)]
1515
pub struct Discovery {
@@ -54,7 +54,7 @@ impl Discovery {
5454
let discovered_resources = match discovery_type.list_available(kind, type_name_filter, adapter_name_filter) {
5555
Ok(value) => value,
5656
Err(err) => {
57-
error!("{err}");
57+
warn!("{err}");
5858
continue;
5959
}
6060
};

dsc_lib/src/dscresources/resource_manifest.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,6 @@ pub struct ResourceManifest {
3535
pub kind: Option<Kind>,
3636
/// The version of the resource using semantic versioning.
3737
pub version: String,
38-
/// The deprecation message of the resource, if the resource is deprecated.
39-
#[serde(skip_serializing_if = "Option::is_none")]
40-
pub deprecation_message: Option<String>,
4138
/// The description of the resource.
4239
pub description: Option<String>,
4340
/// Tags for the resource.

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.failedImportManifest", resource = manifest_path.to_string_lossy(), err = err).to_string());
120+
continue;
121+
}
122+
};
123+
124+
resources.push(resource);
117125
}
118126
}
119127

0 commit comments

Comments
 (0)