Skip to content

Conversation

@nikpivkin
Copy link
Contributor

@nikpivkin nikpivkin commented Oct 16, 2025

Description

This PR updates ManifestNode parsing to correctly capture the start line of map nodes using the key's line. Previously, snippets for map nodes would start at the first value, omitting the key itself, which could make diagnostic messages and snippet highlighting less accurate.

Example before:

For this YAML snippet:

metadata:
  name: hello-host-ports

The snippet in diagnostics showed only the value:

4 [   name: hello-host-ports

This does not give any indication that the value belongs to metadata. The user only sees the line name: hello-host-ports without knowing its level or context.

Example after:

Now the snippet correctly includes the key, and the range references the metadata map:

3 ┌ metadata:
4 └   name: hello-host-ports

This provides more precise context for misconfigurations in YAML manifests and improves the clarity of diagnostic messages.

Additional improvement:

  • EndLine is now correctly calculated for maps and slices that end with a null node.

Related issues:

Related PRs

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@nikpivkin nikpivkin added the autoready Automatically mark PR as ready for review when all checks pass label Oct 16, 2025
@github-actions github-actions bot added the apidiff Indicates Go API changes relevant to library consumers (CLI compatibility may be unaffected) label Oct 16, 2025
@github-actions
Copy link

github-actions bot commented Oct 16, 2025

📊 API Changes Detected

Semver impact: major

github.com/aquasecurity/trivy/pkg/iac/scanners/kubernetes/parser
  Incompatible changes:
  - (*Manifest).UnmarshalYAML: removed
  - Manifest.Path: removed
  - ManifestNode.Path: removed
  - NewManifest: changed from func(string, *ManifestNode) *Manifest to func(string, *ManifestNode) (*Manifest, error)
  Compatible changes:
  - Manifest.FilePath: added
  - ManifestFromYAML: added
  - ManifestNode.FilePath: added

@aqua-bot aqua-bot requested a review from a team October 16, 2025 21:28
@nikpivkin nikpivkin force-pushed the feat/k8s-manifest-map-start-line branch from 54515f9 to b02712b Compare October 17, 2025 11:12
@github-actions github-actions bot marked this pull request as ready for review October 17, 2025 11:32
@github-actions github-actions bot removed the autoready Automatically mark PR as ready for review when all checks pass label Oct 17, 2025
@nikpivkin nikpivkin requested a review from Copilot October 17, 2025 11:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves YAML manifest diagnostics by ensuring map node snippets start at the map key line and by handling line ranges correctly when maps/slices end with null nodes.

  • Include map key line in snippets for map nodes by adjusting StartLine.
  • Correct EndLine calculation for maps and slices that end with a null node.
  • Refactor parsing to use ManifestFromYAML/NewManifest and propagate FilePath; update tests and goldens accordingly.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/iac/scanners/kubernetes/parser/parser.go Refactors parsing to use ManifestFromYAML; tracks per-document line offset.
pkg/iac/scanners/kubernetes/parser/manifest_node.go Renames Path to FilePath; adds newManifestNodeFromYaml; adjusts map handling to start at key’s line; handles null nodes.
pkg/iac/scanners/kubernetes/parser/manifest.go Introduces ManifestFromYAML; updates Manifest structure and NewManifest behavior; improves error wrapping.
pkg/iac/scanners/kubernetes/parser/manifest_test.go Adds YAML test covering nulls and map key start line; updates tests to use ManifestFromYAML and FilePath metadata.
integration/testdata/*.golden Updates expected diagnostics to include map keys and corrected line ranges.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@nikpivkin nikpivkin force-pushed the feat/k8s-manifest-map-start-line branch from dab80da to 306eec9 Compare October 20, 2025 09:45
@nikpivkin nikpivkin force-pushed the feat/k8s-manifest-map-start-line branch from 306eec9 to 003e7ae Compare October 21, 2025 17:30
Signed-off-by: nikpivkin <[email protected]>
@simar7
Copy link
Member

simar7 commented Oct 21, 2025

Was there any associated GitHub issue or discussion for it?

@nikpivkin
Copy link
Contributor Author

@simar7 I found that this could be improved while working on #9668. I will create a discussion.

@nikpivkin nikpivkin added this pull request to the merge queue Oct 21, 2025
Merged via the queue into aquasecurity:main with commit 197c9e1 Oct 21, 2025
15 checks passed
@nikpivkin nikpivkin deleted the feat/k8s-manifest-map-start-line branch October 21, 2025 18:44
knqyf263 pushed a commit to knqyf263/trivy that referenced this pull request Oct 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

apidiff Indicates Go API changes relevant to library consumers (CLI compatibility may be unaffected)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(misconf): Improve k8s manifest parsing to include map keys in snippet diagnostics

2 participants