Add test to ensure package metadata is represented in the JSON schema #1841
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Benchmark Test ResultsBenchmark results from the latest changes vs base branch |
kzantow
reviewed
May 23, 2023
Comment on lines
+21
to
+31
| func AllSyftMetadataTypeNames() ([]string, error) { | ||
| root, err := repoRoot() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| files, err := filepath.Glob(filepath.Join(root, "syft/pkg/*.go")) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return findMetadataDefinitionNames(files...) | ||
| } |
There was a problem hiding this comment.
There is some related data here, could you use any of these exported lists? These need to be kept up to date, used when decoding CDX.
There was a problem hiding this comment.
I thought about using these, but the pkg.MetadataType will be something that is removed closer to syft 1.0 #1735 which means this map would be a format-only concern. So instead I leaned towards the current ast approach, which has the added benefit of not needing to remember to add it to a central list/map, in which case the PR protection would be a little more brittle.
There was a problem hiding this comment.
Right, but the list of all metadata types is something needed by the Syft format decoder once the pkg.MetadataType goes away, which is specifically a concern with the Syft schema, is it not?
There was a problem hiding this comment.
I see -- that is, we should have a single source of truth for these sets. I'll try to account for that in this PR (warning, the scope will creep a bit, but I think it's warranted).
There was a problem hiding this comment.
I still agree with trying to get down to a single source of truth, however, I feel that is better tackled in #1735 and #1844 since there are naming inconsistencies and decoding considerations to make here. Additionally, this PR doesn't add another source of truth, this container struct being created exists today and is now being generated. Hopefully with the other two issues they can expand on this generation to include more things.
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
kzantow
approved these changes
May 25, 2023
spiffcs
added a commit
that referenced
this pull request
Jun 5, 2023
* main: (21 commits) chore(deps): bump github.com/sirupsen/logrus from 1.9.2 to 1.9.3 (#1862) chore(deps): bump modernc.org/sqlite from 1.22.1 to 1.23.0 (#1863) feat: source-version flag (#1859) chore(deps): bump github.com/spf13/viper from 1.15.0 to 1.16.0 (#1851) accept main.version ldflags even without vcs (#1855) feat: add scope to pom properties (#1779) chore(deps): bump github.com/stretchr/testify from 1.8.3 to 1.8.4 (#1852) chore(deps): bump github.com/docker/docker (#1849) Add test to ensure package metadata is represented in the JSON schema (#1841) Fix directory resolver to consider CWD and root path input correctly (#1840) Migrate location-related structs to the file package (#1751) chore(deps): bump github.com/go-git/go-git/v5 from 5.6.1 to 5.7.0 (#1843) fix: add panic recovery for license parse (#1839) chore: return both failures when failed to retrieve an image with a scheme (#1801) Extract go module versions from ldflags for binaries built by go (#1832) fix: duplicate packages, support pnpm lockfile v6 (#1778) chore(deps): update stereoscope to e14bc4437b2eac481c5b6f101890b22df4f33596 (#1834) chore(deps): bump github.com/stretchr/testify from 1.8.2 to 1.8.3 (#1829) chore(deps): bump github.com/docker/docker (#1833) Keep original FileInfo persisted on file.Metadata structs (#1794) ... Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
GijsCalis
pushed a commit
to GijsCalis/syft
that referenced
this pull request
Feb 19, 2024
…anchore#1841) * [wip] try to reflect metadata types... probably wont work Signed-off-by: Alex Goodman <alex.goodman@anchore.com> * refactor to add unit test to ensure there is coverage in the schema Signed-off-by: Alex Goodman <alex.goodman@anchore.com> * [wip] generate metadata container Signed-off-by: Alex Goodman <alex.goodman@anchore.com> * add generation of metadata container struct for JSON schema generation Signed-off-by: Alex Goodman <alex.goodman@anchore.com> * fix linting Signed-off-by: Alex Goodman <alex.goodman@anchore.com> * update linter script to account for code generation Signed-off-by: Alex Goodman <alex.goodman@anchore.com> --------- Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Today we generate our JSON schema from our go types. We create a single
ArtifactMetadataContainertype in theschema/jsonpackage that has allgithub.com/anchore/syft/pkg.*Metadatastructs defined on it, and use this type to infer what can be expressed in thepkg.Package.Metadatafield. This approach has worked well, but there is one shortcoming: when a new type is added someone needs to remember to add this type to theArtifactMetadataContainertype.This PR improves this some by adding a unit test that will fail when there is a new type that appears to be a metadata type within the
pkgpackage that is not represented onArtifactMetadataContainer.Additionally, the
generate-json-schemamake target has been updated to generate theArtifactMetadataContainerstruct based on the latest analysis of the source code in thepkgpackage.All of this will help ensure that we are not introducing data shape changes without updating the schema and are hyper aware of any breaking changes occurring to the schema within the PR making that change.