-
Notifications
You must be signed in to change notification settings - Fork 587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add test to ensure package metadata is represented in the JSON schema #1841
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
844ce86
[wip] try to reflect metadata types... probably wont work
wagoodman f16db96
refactor to add unit test to ensure there is coverage in the schema
wagoodman f664535
[wip] generate metadata container
wagoodman 9e70e5f
add generation of metadata container struct for JSON schema generation
wagoodman ddf2253
Merge remote-tracking branch 'origin/main' into reflect-metadata-schema
wagoodman ab4d82e
fix linting
wagoodman e99d968
update linter script to account for code generation
wagoodman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,27 +1,17 @@ | ||
| #!/usr/bin/env bash | ||
| set -u | ||
|
|
||
| if ! git diff-index --quiet HEAD --; then | ||
| git diff-index HEAD -- | ||
| git --no-pager diff | ||
| echo "there are uncommitted changes, please commit them before running this check" | ||
| if [ "$(git status --porcelain | wc -l)" -ne "0" ]; then | ||
| echo " 🔴 there are uncommitted changes, please commit them before running this check" | ||
| exit 1 | ||
| fi | ||
|
|
||
| success=true | ||
|
|
||
| if ! make generate-json-schema; then | ||
| echo "Generating json schema failed" | ||
| success=false | ||
| fi | ||
|
|
||
| if ! git diff-index --quiet HEAD --; then | ||
| git diff-index HEAD -- | ||
| git --no-pager diff | ||
| echo "JSON schema drift detected!" | ||
| success=false | ||
| exit 1 | ||
| fi | ||
|
|
||
| if ! $success; then | ||
| if [ "$(git status --porcelain | wc -l)" -ne "0" ]; then | ||
| echo " 🔴 there are uncommitted changes, please commit them before running this check" | ||
| exit 1 | ||
| fi |
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "os" | ||
|
|
||
| "github.com/dave/jennifer/jen" | ||
|
|
||
| "github.com/anchore/syft/schema/json/internal" | ||
| ) | ||
|
|
||
| // This program generates internal/generated.go. | ||
|
|
||
| const ( | ||
| pkgImport = "github.com/anchore/syft/syft/pkg" | ||
| path = "internal/generated.go" | ||
| ) | ||
|
|
||
| func main() { | ||
| typeNames, err := internal.AllSyftMetadataTypeNames() | ||
| if err != nil { | ||
| panic(fmt.Errorf("unable to get all metadata type names: %w", err)) | ||
| } | ||
|
|
||
| fmt.Printf("updating metadata container object with %+v types\n", len(typeNames)) | ||
|
|
||
| f := jen.NewFile("internal") | ||
| f.HeaderComment("DO NOT EDIT: generated by schema/json/generate/main.go") | ||
| f.ImportName(pkgImport, "pkg") | ||
| f.Comment("ArtifactMetadataContainer is a struct that contains all the metadata types for a package, as represented in the pkg.Package.Metadata field.") | ||
| f.Type().Id("ArtifactMetadataContainer").StructFunc(func(g *jen.Group) { | ||
| for _, typeName := range typeNames { | ||
| g.Id(typeName).Qual(pkgImport, typeName) | ||
| } | ||
| }) | ||
|
|
||
| rendered := fmt.Sprintf("%#v", f) | ||
|
|
||
| fh, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644) | ||
| if err != nil { | ||
| panic(fmt.Errorf("unable to open file: %w", err)) | ||
| } | ||
| _, err = fh.WriteString(rendered) | ||
| if err != nil { | ||
| panic(fmt.Errorf("unable to write file: %w", err)) | ||
| } | ||
| if err := fh.Close(); err != nil { | ||
| panic(fmt.Errorf("unable to close file: %w", err)) | ||
| } | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| // DO NOT EDIT: generated by schema/json/generate/main.go | ||
|
|
||
| package internal | ||
|
|
||
| import "github.com/anchore/syft/syft/pkg" | ||
|
|
||
| // ArtifactMetadataContainer is a struct that contains all the metadata types for a package, as represented in the pkg.Package.Metadata field. | ||
| type ArtifactMetadataContainer struct { | ||
| AlpmMetadata pkg.AlpmMetadata | ||
| ApkMetadata pkg.ApkMetadata | ||
| BinaryMetadata pkg.BinaryMetadata | ||
| CargoPackageMetadata pkg.CargoPackageMetadata | ||
| CocoapodsMetadata pkg.CocoapodsMetadata | ||
| ConanLockMetadata pkg.ConanLockMetadata | ||
| ConanMetadata pkg.ConanMetadata | ||
| DartPubMetadata pkg.DartPubMetadata | ||
| DotnetDepsMetadata pkg.DotnetDepsMetadata | ||
| DpkgMetadata pkg.DpkgMetadata | ||
| GemMetadata pkg.GemMetadata | ||
| GolangBinMetadata pkg.GolangBinMetadata | ||
| GolangModMetadata pkg.GolangModMetadata | ||
| HackageMetadata pkg.HackageMetadata | ||
| JavaMetadata pkg.JavaMetadata | ||
| KbPackageMetadata pkg.KbPackageMetadata | ||
| LinuxKernelMetadata pkg.LinuxKernelMetadata | ||
| LinuxKernelModuleMetadata pkg.LinuxKernelModuleMetadata | ||
| MixLockMetadata pkg.MixLockMetadata | ||
| NixStoreMetadata pkg.NixStoreMetadata | ||
| NpmPackageJSONMetadata pkg.NpmPackageJSONMetadata | ||
| NpmPackageLockJSONMetadata pkg.NpmPackageLockJSONMetadata | ||
| PhpComposerJSONMetadata pkg.PhpComposerJSONMetadata | ||
| PortageMetadata pkg.PortageMetadata | ||
| PythonPackageMetadata pkg.PythonPackageMetadata | ||
| PythonPipfileLockMetadata pkg.PythonPipfileLockMetadata | ||
| PythonRequirementsMetadata pkg.PythonRequirementsMetadata | ||
| RDescriptionFileMetadata pkg.RDescriptionFileMetadata | ||
| RebarLockMetadata pkg.RebarLockMetadata | ||
| RpmMetadata pkg.RpmMetadata | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| package internal | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "go/ast" | ||
| "go/parser" | ||
| "go/token" | ||
| "os/exec" | ||
| "path/filepath" | ||
| "sort" | ||
| "strings" | ||
| "unicode" | ||
|
|
||
| "github.com/scylladb/go-set/strset" | ||
| ) | ||
|
|
||
| var metadataExceptions = strset.New( | ||
| "FileMetadata", | ||
| ) | ||
|
|
||
| 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...) | ||
| } | ||
|
|
||
| func repoRoot() (string, error) { | ||
| root, err := exec.Command("git", "rev-parse", "--show-toplevel").Output() | ||
| if err != nil { | ||
| return "", fmt.Errorf("unable to find repo root dir: %+v", err) | ||
| } | ||
| absRepoRoot, err := filepath.Abs(strings.TrimSpace(string(root))) | ||
| if err != nil { | ||
| return "", fmt.Errorf("unable to get abs path to repo root: %w", err) | ||
| } | ||
| return absRepoRoot, nil | ||
| } | ||
|
|
||
| func findMetadataDefinitionNames(paths ...string) ([]string, error) { | ||
| names := strset.New() | ||
| usedNames := strset.New() | ||
| for _, path := range paths { | ||
| metadataDefinitions, usedTypeNames, err := findMetadataDefinitionNamesInFile(path) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| // useful for debugging... | ||
| // fmt.Println(path) | ||
| // fmt.Println("Defs:", metadataDefinitions) | ||
| // fmt.Println("Used Types:", usedTypeNames) | ||
| // fmt.Println() | ||
|
|
||
| names.Add(metadataDefinitions...) | ||
| usedNames.Add(usedTypeNames...) | ||
| } | ||
|
|
||
| // any definition that is used within another struct should not be considered a top-level metadata definition | ||
| names.Remove(usedNames.List()...) | ||
|
|
||
| strNames := names.List() | ||
| sort.Strings(strNames) | ||
|
|
||
| // note: 30 is a point-in-time gut check. This number could be updated if new metadata definitions are added, but is not required. | ||
| // it is really intended to catch any major issues with the generation process that would generate, say, 0 definitions. | ||
| if len(strNames) < 30 { | ||
| return nil, fmt.Errorf("not enough metadata definitions found (discovered: " + fmt.Sprintf("%d", len(strNames)) + ")") | ||
| } | ||
|
|
||
| return strNames, nil | ||
| } | ||
|
|
||
| func findMetadataDefinitionNamesInFile(path string) ([]string, []string, error) { | ||
| // set up the parser | ||
| fs := token.NewFileSet() | ||
| f, err := parser.ParseFile(fs, path, nil, parser.ParseComments) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
|
|
||
| var metadataDefinitions []string | ||
| var usedTypeNames []string | ||
| for _, decl := range f.Decls { | ||
| // check if the declaration is a type declaration | ||
| spec, ok := decl.(*ast.GenDecl) | ||
| if !ok || spec.Tok != token.TYPE { | ||
| continue | ||
| } | ||
|
|
||
| // loop over all types declared in the type declaration | ||
| for _, typ := range spec.Specs { | ||
| // check if the type is a struct type | ||
| spec, ok := typ.(*ast.TypeSpec) | ||
| if !ok || spec.Type == nil { | ||
| continue | ||
| } | ||
|
|
||
| structType, ok := spec.Type.(*ast.StructType) | ||
| if !ok { | ||
| continue | ||
| } | ||
|
|
||
| // check if the struct type ends with "Metadata" | ||
| name := spec.Name.String() | ||
|
|
||
| // only look for exported types that end with "Metadata" | ||
| if isMetadataTypeCandidate(name) { | ||
| // print the full declaration of the struct type | ||
| metadataDefinitions = append(metadataDefinitions, name) | ||
| usedTypeNames = append(usedTypeNames, typeNamesUsedInStruct(structType)...) | ||
| } | ||
| } | ||
| } | ||
| return metadataDefinitions, usedTypeNames, nil | ||
| } | ||
|
|
||
| func typeNamesUsedInStruct(structType *ast.StructType) []string { | ||
| // recursively find all type names used in the struct type | ||
| var names []string | ||
| for i := range structType.Fields.List { | ||
| // capture names of all of the types (not field names) | ||
| ast.Inspect(structType.Fields.List[i].Type, func(n ast.Node) bool { | ||
| ident, ok := n.(*ast.Ident) | ||
| if !ok { | ||
| return true | ||
| } | ||
|
|
||
| // add the type name to the list | ||
| names = append(names, ident.Name) | ||
|
|
||
| // continue inspecting | ||
| return true | ||
| }) | ||
| } | ||
|
|
||
| return names | ||
| } | ||
|
|
||
| func isMetadataTypeCandidate(name string) bool { | ||
| return len(name) > 0 && | ||
| strings.HasSuffix(name, "Metadata") && | ||
| unicode.IsUpper(rune(name[0])) && // must be exported | ||
| !metadataExceptions.Has(name) | ||
| } | ||
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about using these, but the
pkg.MetadataTypewill 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.