kube-score icon indicating copy to clipboard operation
kube-score copied to clipboard

Replace Sarif JSON structs with owenrumney/go-sarif/sarif

Open atombrella opened this issue 3 years ago • 5 comments

RELNOTE: Replace Sarif JSON structs with owenrumney/go-sarif/sarif

This addresses #447 It doesn't seem to require more than simply replacing the import, and deleting the go file with the structs. The library in favor contains a more elaborate definition of the structs for the Sarif standard.

Note that I haven't checked the code coverage for the --output-format sarif flag.

atombrella avatar Apr 16 '22 19:04 atombrella

Looks like the code doesn't compile anymore, I'm guessing that we need to make some alterations to our sarif.go to make it fit how the library is supposed to be used.

# github.com/zegl/kube-score/renderer/sarif
renderer/sarif/sarif.go:15: undefined: sarif.Results
renderer/sarif/sarif.go:16: undefined: sarif.Rules
renderer/sarif/sarif.go:25: undefined: sarif.Rules
renderer/sarif/sarif.go:50: undefined: sarif.Results
renderer/sarif/sarif.go:79: undefined: sarif.Driver
renderer/sarif/sarif.go:86: undefined: sarif.Sarif

zegl avatar Apr 20 '22 13:04 zegl

Looks like the code doesn't compile anymore, I'm guessing that we need to make some alterations to our sarif.go to make it fit how the library is supposed to be used.

# github.com/zegl/kube-score/renderer/sarif
renderer/sarif/sarif.go:15: undefined: sarif.Results
renderer/sarif/sarif.go:16: undefined: sarif.Rules
renderer/sarif/sarif.go:25: undefined: sarif.Rules
renderer/sarif/sarif.go:50: undefined: sarif.Results
renderer/sarif/sarif.go:79: undefined: sarif.Driver
renderer/sarif/sarif.go:86: undefined: sarif.Sarif

Yes. I'll have a look during the weekend to fix it. I've been a bit occupied the last few days to fix everything, and should have waited to open the PR.

atombrella avatar Apr 20 '22 20:04 atombrella

Ah, ok, no worries! :-D

zegl avatar Apr 21 '22 07:04 zegl

I have something working locally :tada: https://sarifweb.azurewebsites.net/Validation complains that the output doesn't contain the version property. Is this available somehow? Note the addition of the URL for the tool :+1:

https://gist.github.com/atombrella/8371135f661e48b465d666a01801a31d is the generated output. I used

./kube-score score --output-format=sarif score/testdata/pod-probes-all-missing.yaml > errors.sarif

Looking at this, it'd be nice to include some tests for the generated output, but perhaps it's a bit out of scope for this PR. It also looks a bit funky with the line numbers that appear to be always 1.

atombrella avatar Apr 23 '22 21:04 atombrella

@zegl Do you have time to give some feedback? There's an unanswered question for v.FileLocation.Line. Tests can be added if needed.

Regarding bors, I think it'd be nice to add use_squash_merge = true to the config-file to avoid loads of intermediate commits for this work. I can also squash before you merge.

atombrella avatar Jun 04 '22 06:06 atombrella