Skip to content

Commit 2c37fd5

Browse files
authored
Rename engineVersion to version, generate reports, cleanup wrong apis (#159)
1 parent 884492d commit 2c37fd5

16 files changed

Lines changed: 1121 additions & 685 deletions

File tree

scripts/compare_cfnlint.py

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -245,12 +245,14 @@ def _load_cfnlint_result_file(f, prefix, results):
245245
return
246246
if not isinstance(data, list):
247247
return
248-
# Derive key from the template filename inside the JSON, not the results filename.
249-
# cfn-lint results filenames may differ from the actual template filename
250-
# (e.g., results file "metadata.json" for template "metdata.yaml"). The file
251-
# extension is kept as part of the key (".yaml" -> "_yaml") so a template
252-
# authored in both JSON and YAML maps to two distinct keys, matching the
253-
# engine report names.
248+
249+
# The default key comes from the result filename, whose stem now embeds the
250+
# source extension as a suffix (template "metdata.yaml" -> result
251+
# "metadata_yaml.json")
252+
# Prefer the template filename read from the JSON's `Filename` field: the
253+
# result filename's base may differ from the real template name (e.g. result
254+
# "metadata_yaml.json" for template "metdata.yaml"), and the engine report is
255+
# keyed off the true template path.
254256
key = f"{prefix}_{f.stem}"
255257
if data and isinstance(data[0], dict) and data[0].get("Filename"):
256258
tpl = data[0]["Filename"].replace("test/fixtures/templates/", "")
@@ -259,25 +261,29 @@ def _load_cfnlint_result_file(f, prefix, results):
259261
key = derived
260262
else:
261263
# An empty result list (cfn-lint found nothing) carries no `Filename`, so
262-
# the extension cannot be read from the diagnostics. Recover it by mirroring
263-
# the result path into the templates tree and adopting the real template
264-
# extension; otherwise the key stays suffix-less and never matches the
265-
# extension-suffixed engine report, silently dropping the template from the
266-
# comparison.
264+
# the true template extension cannot be read from the diagnostics. Confirm
265+
# it instead by locating the mirror template under the templates tree; if
266+
# none exists the default key (from the result filename) is kept as-is.
267267
derived = _derive_key_from_template_path(f)
268268
if derived:
269269
key = derived
270270
results[key] = normalize_cfnlint_diags(data)
271271

272272

273273
def _derive_key_from_template_path(result_file):
274-
"""Recover the extension-suffixed key for a result file by locating the mirror
275-
template under CFN_LINT_TEMPLATES. Returns None if no matching template exists."""
274+
"""Recover the extension-suffixed key for an empty-result file by locating the
275+
mirror template under CFN_LINT_TEMPLATES. The result stem now embeds the source
276+
extension as a suffix (e.g. "foo_yaml.json"), so split that suffix back off, find
277+
the matching template, and rebuild the key from its real extension. Returns None
278+
if no matching template exists (leaving the caller's default key in place)."""
276279
relative = result_file.relative_to(CFN_LINT_RESULTS)
277-
stem_path = str(relative.with_suffix("")).replace(os.sep, "_")
278-
for ext in (".yaml", ".yml", ".json"):
279-
if (CFN_LINT_TEMPLATES / relative.with_suffix(ext)).exists():
280-
return f"{stem_path}_{ext.lstrip('.')}"
280+
stem = relative.stem # drops ".json"; still carries the "_yaml"/"_yml"/"_json" suffix
281+
for ext in ("yaml", "yml", "json"):
282+
base = stem[: -(len(ext) + 1)] if stem.endswith(f"_{ext}") else stem
283+
template = relative.with_name(f"{base}.{ext}")
284+
if (CFN_LINT_TEMPLATES / template).exists():
285+
key_path = str(template.with_suffix("")).replace(os.sep, "_")
286+
return f"{key_path}_{ext}"
281287
return None
282288

283289

scripts/snapshots/report_cel_detailed.md

Lines changed: 102 additions & 138 deletions
Large diffs are not rendered by default.

scripts/snapshots/report_rego_detailed.md

Lines changed: 102 additions & 138 deletions
Large diffs are not rendered by default.

src/bindings-jvm/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ val diagnostics = validator.validate(File("template.yaml"))
173173
data class StandardReport(
174174
val filePath: String,
175175
val status: ReportStatus, // OK or ERROR (ERROR when template fails to parse)
176-
val engineVersion: String,
176+
val version: String,
177177
val metadata: ReportMetadata,
178178
val performance: PerformanceMetrics,
179179
val diagnostics: List<StandardDiagnostic>,

src/bindings-jvm/bench/src/main/kotlin/Benchmark.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,6 @@ fun validateConfig() = ValidateConfig(
466466
parameterOverrides = mapOf(),
467467
pseudoParameterOverrides = PseudoParameterOverrides(),
468468
strict = false,
469-
includeEngineRules = true,
470469
)
471470

472471
private data class PendingReport(val dest: File, val rel: String, val report: DetailedReport, val metrics: JsonObject)

src/bindings-jvm/tests/kotlin/src/test/kotlin/SmokeTest.kt

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ class SmokeTest {
423423
private fun stripGoldenExcludedFields(report: Map<String, Any?>, filePath: String? = null): Map<String, Any?> {
424424
val out = LinkedHashMap(report)
425425
if (filePath != null) out["filePath"] = filePath
426-
out.remove("engineVersion")
426+
out.remove("version")
427427
out.remove("performance")
428428
val metadata = out["metadata"] as? Map<String, Any?>
429429
if (metadata != null) {
@@ -434,21 +434,6 @@ class SmokeTest {
434434
return out
435435
}
436436

437-
@Test
438-
fun rulesEvaluatedIsFullRuleCount() {
439-
val expected = 280u
440-
assertEquals(expected, CEL.validateDetailed(templateFile("good/generic.yaml"), defaultConfig()).metadata.rulesEvaluated, "cel: rulesEvaluated")
441-
assertEquals(expected, REGO.validateDetailed(templateFile("good/generic.yaml"), defaultConfig()).metadata.rulesEvaluated, "rego: rulesEvaluated")
442-
}
443-
444-
@Test
445-
fun engineVersionMatchesWorkspaceVersion() {
446-
val expected = "1.4.0"
447-
assertEquals(expected, readWorkspaceVersion(), "expected version must match workspace Cargo.toml")
448-
assertEquals(expected, CEL.validateDetailed(templateFile("good/generic.yaml"), defaultConfig()).engineVersion, "cel: engineVersion")
449-
assertEquals(expected, REGO.validateDetailed(templateFile("good/generic.yaml"), defaultConfig()).engineVersion, "rego: engineVersion")
450-
}
451-
452437
@Test
453438
fun performanceIsPresentWithTimingPerPhase() {
454439
val performance = REGO.validateDetailed(templateFile("good/generic.yaml"), defaultConfig()).performance

src/bindings-wasm/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ validator.free();
177177
interface StandardReport {
178178
filePath: string;
179179
status: "OK" | "ERROR"; // ERROR when the template fails to parse
180-
engineVersion: string;
180+
version: string;
181181
metadata: ReportMetadata;
182182
performance: PerformanceMetrics;
183183
diagnostics: StandardDiagnostic[];

src/bindings-wasm/bench/benchmark.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import type { DetailedReport, EngineConfig, ValidateConfig } from '@aws/cloudfor
1313
import type {
1414
WasmCelEngine as WasmCelEngineType,
1515
WasmRegoEngine as WasmRegoEngineType,
16-
} from 'bindings-wasm/bindings_wasm';
16+
} from '@aws/cloudformation-validate/bindings_wasm';
1717
type WasmEngine = WasmRegoEngineType | WasmCelEngineType;
1818

1919
const { SchemaValidator } = wasmBindings;
@@ -243,7 +243,6 @@ fs.mkdirSync(jsonDir, { recursive: true });
243243
const validateConfig: ValidateConfig = {
244244
severityLevel: 'DEBUG',
245245
strict: false,
246-
includeEngineRules: true,
247246
};
248247

249248
if (templates.length > 0) {

0 commit comments

Comments
 (0)