Skip to content
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

import-boss: add inverse import rules #154

Merged
merged 1 commit into from
Oct 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
add inverse import rules
  • Loading branch information
eloyekunle committed Oct 10, 2019
commit 7ccd3e36d2b850d7ba573861a469064cdbfc07fa
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ jobs:
- go test -v ./...
- stage: Verify examples
script:
- go run ./examples/import-boss/main.go -i k8s.io/gengo/... --verify-only
- go run ./examples/import-boss/main.go -i $(go list k8s.io/gengo/... | grep -v import-boss/tests | paste -sd',' -) --verify-only
3 changes: 3 additions & 0 deletions args/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ func (g *GeneratorArgs) InputIncludes(p *types.Package) bool {
if strings.HasSuffix(d, "...") {
d = strings.TrimSuffix(d, "...")
}
if strings.HasPrefix(d, "./vendor/") {
d = strings.TrimPrefix(d, "./vendor/")
}
if strings.HasPrefix(p.Path, d) {
return true
}
Expand Down
28 changes: 28 additions & 0 deletions examples/import-boss/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
TOOL=import-boss

all: test

test: build test_rules test_inverse

build:
@go build -o /tmp/$(TOOL)

test_rules:
/tmp/$(TOOL) --logtostderr --v=4 -i $$(go list ./tests/rules/a)
! /tmp/$(TOOL) --logtostderr --v=4 -i $$(go list ./tests/rules/b)
/tmp/$(TOOL) --logtostderr --v=4 -i $$(go list ./tests/rules/c)
! /tmp/$(TOOL) --logtostderr --v=4 -i $$(go list ./tests/rules/nested)
/tmp/$(TOOL) --logtostderr --v=4 -i $$(go list ./tests/rules/nested/nested)
! /tmp/$(TOOL) --logtostderr --v=4 -i $$(go list ./tests/rules/nested/nested/nested)
! /tmp/$(TOOL) --logtostderr --v=4 -i $$(go list ./tests/rules/nested/nested/nested/inherit)

test_inverse:
/tmp/$(TOOL) --logtostderr --v=4 -i $$(go list ./tests/inverse/a ./tests/inverse/lib/... | grep -v quarantine | paste -sd',' -)
! /tmp/$(TOOL) --logtostderr --v=4 -i $$(go list ./tests/inverse/b ./tests/inverse/lib/... | grep -v quarantine | paste -sd',' -)
/tmp/$(TOOL) --logtostderr --v=4 -i $$(go list ./tests/inverse/c ./tests/inverse/lib/... | grep -v quarantine | paste -sd',' -)
! /tmp/$(TOOL) --logtostderr --v=4 -i $$(go list ./tests/inverse/d ./tests/inverse/lib/... | grep -v quarantine | paste -sd',' -)
! /tmp/$(TOOL) --logtostderr --v=4 -i $$(go list ./tests/inverse/lib/... | paste -sd',' -)

! /tmp/$(TOOL) --logtostderr --v=4 -i $$(go list ./tests/inverse/lib/... | paste -sd',' -)

.PHONY: build test test_rules test_inverse
212 changes: 170 additions & 42 deletions examples/import-boss/generators/import_restrict.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
)

const (
goModFile = "go.mod"
importBossFileType = "import-boss"
)

Expand All @@ -58,7 +59,7 @@ func DefaultNameSystem() string {
func Packages(c *generator.Context, arguments *args.GeneratorArgs) generator.Packages {
pkgs := generator.Packages{}
c.FileTypes = map[string]generator.FileType{
importBossFileType: importRuleFile{},
importBossFileType: importRuleFile{c},
}

for _, p := range c.Universe {
Expand All @@ -70,6 +71,7 @@ func Packages(c *generator.Context, arguments *args.GeneratorArgs) generator.Pac
pkgs = append(pkgs, &generator.DefaultPackage{
PackageName: p.Name,
PackagePath: p.Path,
Source: p.SourcePath,
// GeneratorFunc returns a list of generators. Each generator makes a
// single file.
GeneratorFunc: func(c *generator.Context) (generators []generator.Generator) {
Expand All @@ -96,10 +98,19 @@ type Rule struct {
ForbiddenPrefixes []string
}

type InverseRule struct {
Rule
// True if the rule is to be applied to transitive imports.
Transitive bool
}

type fileFormat struct {
CurrentImports []string

Rules []Rule
Rules []Rule
InverseRules []InverseRule

path string
}

func readFile(path string) (*fileFormat, error) {
Expand All @@ -113,6 +124,7 @@ func readFile(path string) (*fileFormat, error) {
if err != nil {
return nil, fmt.Errorf("couldn't unmarshal %v: %v", path, err)
}
current.path = path
return &current, nil
}

Expand All @@ -131,10 +143,12 @@ func writeFile(path string, ff *fileFormat) error {
}

// This does the actual checking, since it knows the literal destination file.
type importRuleFile struct{}
type importRuleFile struct {
context *generator.Context
}

func (importRuleFile) AssembleFile(f *generator.File, path string) error {
return nil
func (irf importRuleFile) AssembleFile(f *generator.File, path string) error {
return irf.VerifyFile(f, path)
}

// TODO: make a flag to enable this, or expose this information in some other way.
Expand Down Expand Up @@ -169,62 +183,99 @@ func removeLastDir(path string) (newPath, removedDir string) {
return filepath.Join(filepath.Dir(dir), file), filepath.Base(dir)
}

// Keep going up a directory until we find an .import-restrictions file.
func recursiveRead(path string) (*fileFormat, string, error) {
// isGoModRoot checks if a directory is the root directory for a package
// by checking for the existence of a 'go.mod' file in that directory.
func isGoModRoot(path string) bool {
_, err := os.Stat(filepath.Join(filepath.Dir(path), goModFile))
return err == nil
}

// recursiveRead collects all '.import-restriction' files, between the current directory,
// and the package root when Go modules are enabled, or $GOPATH/src when they are not.
func recursiveRead(path string) ([]*fileFormat, error) {
restrictionFiles := make([]*fileFormat, 0)

for {
if _, err := os.Stat(path); err == nil {
ff, err := readFile(path)
return ff, path, err
rules, err := readFile(path)
if err != nil {
return nil, err
}

restrictionFiles = append(restrictionFiles, rules)
}

nextPath, removedDir := removeLastDir(path)
if nextPath == path || removedDir == "src" {
if nextPath == path || isGoModRoot(path) || removedDir == "src" {
break
}

path = nextPath
}
return nil, "", nil

return restrictionFiles, nil
}

func (importRuleFile) VerifyFile(f *generator.File, path string) error {
rules, actualPath, err := recursiveRead(path)
func (irf importRuleFile) VerifyFile(f *generator.File, path string) error {
restrictionFiles, err := recursiveRead(filepath.Join(f.PackageSourcePath, f.Name))
if err != nil {
return fmt.Errorf("error finding rules file: %v", err)
}

if rules == nil {
// No restrictions on this directory.
return nil
if err := irf.verifyRules(restrictionFiles, f); err != nil {
return err
}

return irf.verifyInverseRules(restrictionFiles, f)
}

func (irf importRuleFile) verifyRules(restrictionFiles []*fileFormat, f *generator.File) error {
selectors := make([][]*regexp.Regexp, len(restrictionFiles))
for i, restrictionFile := range restrictionFiles {
for _, r := range restrictionFile.Rules {
re, err := regexp.Compile(r.SelectorRegexp)
if err != nil {
return fmt.Errorf("regexp `%s` in file %q doesn't compile: %v", r.SelectorRegexp, restrictionFile.path, err)
}

selectors[i] = append(selectors[i], re)
}
}

forbiddenImports := map[string]string{}
allowedMismatchedImports := []string{}
for _, r := range rules.Rules {
re, err := regexp.Compile(r.SelectorRegexp)
if err != nil {
return fmt.Errorf("regexp `%s` in file %q doesn't compile: %v", r.SelectorRegexp, actualPath, err)
}
for v := range f.Imports {
klog.V(4).Infof("Checking %v matches %v: %v\n", r.SelectorRegexp, v, re.MatchString(v))
if !re.MatchString(v) {
continue
}
for _, forbidden := range r.ForbiddenPrefixes {
klog.V(4).Infof("Checking %v against %v\n", v, forbidden)
if strings.HasPrefix(v, forbidden) {
forbiddenImports[v] = forbidden

for v := range f.Imports {
explicitlyAllowed := false

NextRestrictionFiles:
for i, rules := range restrictionFiles {
for j, r := range rules.Rules {
matching := selectors[i][j].MatchString(v)
klog.V(5).Infof("Checking %v matches %v: %v\n", r.SelectorRegexp, v, matching)
if !matching {
continue
}
}
found := false
for _, allowed := range r.AllowedPrefixes {
klog.V(4).Infof("Checking %v against %v\n", v, allowed)
if strings.HasPrefix(v, allowed) {
found = true
break
for _, forbidden := range r.ForbiddenPrefixes {
klog.V(4).Infof("Checking %v against %v\n", v, forbidden)
if strings.HasPrefix(v, forbidden) {
forbiddenImports[v] = forbidden
}
}
for _, allowed := range r.AllowedPrefixes {
klog.V(4).Infof("Checking %v against %v\n", v, allowed)
if strings.HasPrefix(v, allowed) {
explicitlyAllowed = true
break
}
}

if !explicitlyAllowed {
allowedMismatchedImports = append(allowedMismatchedImports, v)
} else {
klog.V(2).Infof("%v importing %v allowed by %v\n", f.PackagePath, v, restrictionFiles[i].path)
break NextRestrictionFiles
}
}
if !found {
allowedMismatchedImports = append(allowedMismatchedImports, v)
}
}
}
Expand All @@ -243,8 +294,85 @@ func (importRuleFile) VerifyFile(f *generator.File, path string) error {
}
return errors.New(errorBuilder.String())
}
if len(rules.Rules) > 0 {
klog.V(2).Infof("%v passes rules found in %v\n", path, actualPath)

return nil
}

// verifyInverseRules checks that all packages that import a package are allowed to import it.
func (irf importRuleFile) verifyInverseRules(restrictionFiles []*fileFormat, f *generator.File) error {
// compile all Selector regex in all restriction files
selectors := make([][]*regexp.Regexp, len(restrictionFiles))
for i, restrictionFile := range restrictionFiles {
for _, r := range restrictionFile.InverseRules {
re, err := regexp.Compile(r.SelectorRegexp)
if err != nil {
return fmt.Errorf("regexp `%s` in file %q doesn't compile: %v", r.SelectorRegexp, restrictionFile.path, err)
}

selectors[i] = append(selectors[i], re)
}
}

directImport := map[string]bool{}
for _, imp := range irf.context.IncomingImports()[f.PackagePath] {
directImport[imp] = true
}

forbiddenImports := map[string]string{}
allowedMismatchedImports := []string{}

for _, v := range irf.context.TransitiveIncomingImports()[f.PackagePath] {
explicitlyAllowed := false

NextRestrictionFiles:
for i, rules := range restrictionFiles {
for j, r := range rules.InverseRules {
if !r.Transitive && !directImport[v] {
continue
}

re := selectors[i][j]
matching := re.MatchString(v)
klog.V(4).Infof("Checking %v matches %v (importing %v: %v\n", r.SelectorRegexp, v, f.PackagePath, matching)
if !matching {
continue
}
for _, forbidden := range r.ForbiddenPrefixes {
klog.V(4).Infof("Checking %v against %v\n", v, forbidden)
if strings.HasPrefix(v, forbidden) {
forbiddenImports[v] = forbidden
}
}
for _, allowed := range r.AllowedPrefixes {
klog.V(4).Infof("Checking %v against %v\n", v, allowed)
if strings.HasPrefix(v, allowed) {
explicitlyAllowed = true
break
}
}
if !explicitlyAllowed {
allowedMismatchedImports = append(allowedMismatchedImports, v)
} else {
klog.V(2).Infof("%v importing %v allowed by %v\n", v, f.PackagePath, restrictionFiles[i].path)
break NextRestrictionFiles
}
}
}
}

if len(forbiddenImports) > 0 || len(allowedMismatchedImports) > 0 {
var errorBuilder strings.Builder
for i, f := range forbiddenImports {
fmt.Fprintf(&errorBuilder, "(inverse): import %v has forbidden prefix %v\n", i, f)
}
if len(allowedMismatchedImports) > 0 {
sort.Sort(sort.StringSlice(allowedMismatchedImports))
fmt.Fprintf(&errorBuilder, "(inverse): the following imports did not match any allowed prefix:\n")
for _, i := range allowedMismatchedImports {
fmt.Fprintf(&errorBuilder, " %v\n", i)
}
}
return errors.New(errorBuilder.String())
}

return nil
Expand Down
Loading