From 46d12b185eb637bd61f4b16f9beaa79183aca7aa Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Tue, 26 Jan 2016 16:42:41 -0800 Subject: [PATCH] Add 'import-boss': enforce go import restrictions. --- cmd/libs/go2idl/.import-restrictions | 11 + cmd/libs/go2idl/args/args.go | 26 +- .../go2idl/client-gen/.import-restrictions | 1 + cmd/libs/go2idl/import-boss/.gitignore | 1 + .../import-boss/generators/import_restrict.go | 269 ++++++++++++++++++ .../generators/import_restrict_test.go | 36 +++ cmd/libs/go2idl/import-boss/main.go | 90 ++++++ cmd/libs/go2idl/parser/parse.go | 46 ++- hack/after-build/run-import-boss.sh | 26 ++ hack/verify-import-boss.sh | 30 ++ 10 files changed, 529 insertions(+), 7 deletions(-) create mode 100644 cmd/libs/go2idl/.import-restrictions create mode 100644 cmd/libs/go2idl/client-gen/.import-restrictions create mode 100644 cmd/libs/go2idl/import-boss/.gitignore create mode 100644 cmd/libs/go2idl/import-boss/generators/import_restrict.go create mode 100644 cmd/libs/go2idl/import-boss/generators/import_restrict_test.go create mode 100644 cmd/libs/go2idl/import-boss/main.go create mode 100755 hack/after-build/run-import-boss.sh create mode 100755 hack/verify-import-boss.sh diff --git a/cmd/libs/go2idl/.import-restrictions b/cmd/libs/go2idl/.import-restrictions new file mode 100644 index 0000000000..937fec69fe --- /dev/null +++ b/cmd/libs/go2idl/.import-restrictions @@ -0,0 +1,11 @@ +{ + "Rules": [ + { + "SelectorRegexp": "k8s[.]io", + "AllowedPrefixes": [ + "k8s.io/kubernetes/cmd/libs/go2idl", + "k8s.io/kubernetes/third_party" + ] + } + ] +} diff --git a/cmd/libs/go2idl/args/args.go b/cmd/libs/go2idl/args/args.go index 61543edc89..4e6db325d5 100644 --- a/cmd/libs/go2idl/args/args.go +++ b/cmd/libs/go2idl/args/args.go @@ -30,6 +30,7 @@ import ( "k8s.io/kubernetes/cmd/libs/go2idl/generator" "k8s.io/kubernetes/cmd/libs/go2idl/namer" "k8s.io/kubernetes/cmd/libs/go2idl/parser" + "k8s.io/kubernetes/cmd/libs/go2idl/types" "github.com/spf13/pflag" ) @@ -50,6 +51,9 @@ type GeneratorArgs struct { // Which directories to parse. InputDirs []string + // If true, recurse into all children of InputDirs + Recursive bool + // Source tree to write results to. OutputBase string @@ -72,6 +76,7 @@ func (g *GeneratorArgs) AddFlags(fs *pflag.FlagSet) { fs.StringVarP(&g.OutputPackagePath, "output-package", "p", g.OutputPackagePath, "Base package path.") fs.StringVarP(&g.GoHeaderFilePath, "go-header-file", "h", g.GoHeaderFilePath, "File containing boilerplate header text. The string YEAR will be replaced with the current 4-digit year.") fs.BoolVar(&g.VerifyOnly, "verify-only", g.VerifyOnly, "If true, only verify existing output, do not write anything.") + fs.BoolVar(&g.Recursive, "recursive", g.VerifyOnly, "If true, recurse into all children of input directories.") } // LoadGoBoilerplate loads the boilerplate file passed to --go-header-file. @@ -89,13 +94,30 @@ func (g *GeneratorArgs) LoadGoBoilerplate() ([]byte, error) { func (g *GeneratorArgs) NewBuilder() (*parser.Builder, error) { b := parser.New() for _, d := range g.InputDirs { - if err := b.AddDir(d); err != nil { - return nil, fmt.Errorf("unable to add directory %q: %v", d, err) + if g.Recursive { + if err := b.AddDirRecursive(d); err != nil { + return nil, fmt.Errorf("unable to add directory %q: %v", d, err) + } + } else { + if err := b.AddDir(d); err != nil { + return nil, fmt.Errorf("unable to add directory %q: %v", d, err) + } } } return b, nil } +// InputIncludes returns true if the given package is a (sub) package of one of +// the InputDirs. +func (g *GeneratorArgs) InputIncludes(p *types.Package) bool { + for _, dir := range g.InputDirs { + if strings.HasPrefix(p.Path, dir) { + return true + } + } + return false +} + // DefaultSourceTree returns the /src directory of the first entry in $GOPATH. // If $GOPATH is empty, it returns "./". Useful as a default output location. func DefaultSourceTree() string { diff --git a/cmd/libs/go2idl/client-gen/.import-restrictions b/cmd/libs/go2idl/client-gen/.import-restrictions new file mode 100644 index 0000000000..0967ef424b --- /dev/null +++ b/cmd/libs/go2idl/client-gen/.import-restrictions @@ -0,0 +1 @@ +{} diff --git a/cmd/libs/go2idl/import-boss/.gitignore b/cmd/libs/go2idl/import-boss/.gitignore new file mode 100644 index 0000000000..a5c47b66f8 --- /dev/null +++ b/cmd/libs/go2idl/import-boss/.gitignore @@ -0,0 +1 @@ +import-boss diff --git a/cmd/libs/go2idl/import-boss/generators/import_restrict.go b/cmd/libs/go2idl/import-boss/generators/import_restrict.go new file mode 100644 index 0000000000..db5ba517c8 --- /dev/null +++ b/cmd/libs/go2idl/import-boss/generators/import_restrict.go @@ -0,0 +1,269 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package generators has the generators for the import-boss utility. +package generators + +import ( + "encoding/json" + "fmt" + "io" + "io/ioutil" + "os" + "path/filepath" + "regexp" + "sort" + "strings" + + "k8s.io/kubernetes/cmd/libs/go2idl/args" + "k8s.io/kubernetes/cmd/libs/go2idl/generator" + "k8s.io/kubernetes/cmd/libs/go2idl/namer" + "k8s.io/kubernetes/cmd/libs/go2idl/types" + + //"github.com/golang/glog" +) + +const ( + importBossFileType = "import-boss" +) + +// NameSystems returns the name system used by the generators in this package. +func NameSystems() namer.NameSystems { + return namer.NameSystems{ + "raw": namer.NewRawNamer("", nil), + } +} + +// DefaultNameSystem returns the default name system for ordering the types to be +// processed by the generators in this package. +func DefaultNameSystem() string { + return "raw" +} + +// Packages makes the sets package definition. +func Packages(c *generator.Context, arguments *args.GeneratorArgs) generator.Packages { + pkgs := generator.Packages{} + c.FileTypes = map[string]generator.FileType{ + importBossFileType: importRuleFile{}, + } + + for _, p := range c.Universe { + if !arguments.InputIncludes(p) { + // Don't run on e.g. third party dependencies. + continue + } + savedPackage := p + pkgs = append(pkgs, &generator.DefaultPackage{ + PackageName: p.Name, + PackagePath: p.Path, + // GeneratorFunc returns a list of generators. Each generator makes a + // single file. + GeneratorFunc: func(c *generator.Context) (generators []generator.Generator) { + return []generator.Generator{&importRules{ + myPackage: savedPackage, + }} + }, + FilterFunc: func(c *generator.Context, t *types.Type) bool { + return false + }, + }) + } + + return pkgs +} + +// A single import restriction rule. +type Rule struct { + // All import paths that match this regexp... + SelectorRegexp string + // ... must have one of these prefixes ... + AllowedPrefixes []string + // ... and must not have one of these prefixes. + ForbiddenPrefixes []string +} + +type fileFormat struct { + CurrentImports []string + + Rules []Rule +} + +func readFile(path string) (*fileFormat, error) { + currentBytes, err := ioutil.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("couldn't read %v: %v", path, err) + } + + var current fileFormat + err = json.Unmarshal(currentBytes, ¤t) + if err != nil { + return nil, fmt.Errorf("couldn't unmarshal %v: %v", path, err) + } + return ¤t, nil +} + +func writeFile(path string, ff *fileFormat) error { + raw, err := json.MarshalIndent(ff, "", "\t") + if err != nil { + return fmt.Errorf("couldn't format data for file %v.\n%#v", path, ff) + } + f, err := os.Create(path) + if err != nil { + return fmt.Errorf("couldn't open %v for writing: %v", path, err) + } + defer f.Close() + _, err = f.Write(raw) + return err +} + +// This does the actual checking, since it knows the literal destination file. +type importRuleFile struct{} + +func (importRuleFile) AssembleFile(f *generator.File, path string) error { + return nil + + // If the file exists, populate its current imports. This is mostly to help + // humans figure out what they need to fix. + // TODO: add a command line flag to enable this? Or require that it always stay up-to-date? + if _, err := os.Stat(path); err != nil { + // Ignore packages which haven't opted in by adding an .import-restrictions file. + return nil + } + + current, err := readFile(path) + if err != nil { + return err + } + + current.CurrentImports = []string{} + for v := range f.Imports { + current.CurrentImports = append(current.CurrentImports, v) + } + sort.Strings(current.CurrentImports) + + return writeFile(path, current) +} + +// removeLastDir removes the last directory, but leaves the file name +// unchanged. It returns the new path and the removed directory. So: +// "a/b/c/file" -> ("a/b/file", "c") +func removeLastDir(path string) (newPath, removedDir string) { + dir, file := filepath.Split(path) + dir = strings.TrimSuffix(dir, string(filepath.Separator)) + 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) { + for { + if _, err := os.Stat(path); err == nil { + ff, err := readFile(path) + return ff, path, err + } + + nextPath, removedDir := removeLastDir(path) + if nextPath == path || removedDir == "src" { + break + } + path = nextPath + } + return nil, "", nil +} + +func (importRuleFile) VerifyFile(f *generator.File, path string) error { + rules, actualPath, err := recursiveRead(path) + if err != nil { + return fmt.Errorf("error finding rules file: %v", err) + } + + if rules == nil { + // No restrictions on this directory. + return nil + } + + 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 { + // fmt.Printf("Checking %v matches %v: %v\n", r.SelectorRegexp, v, re.MatchString(v)) + if !re.MatchString(v) { + continue + } + for _, forbidden := range r.ForbiddenPrefixes { + // fmt.Printf("Checking %v against %v\n", v, forbidden) + if strings.HasPrefix(v, forbidden) { + return fmt.Errorf("import %v has forbidden prefix %v", v, forbidden) + } + } + found := false + for _, allowed := range r.AllowedPrefixes { + fmt.Printf("Checking %v against %v\n", v, allowed) + if strings.HasPrefix(v, allowed) { + found = true + break + } + } + if !found { + return fmt.Errorf("import %v did not match any allowed prefix", v) + } + } + } + if len(rules.Rules) > 0 { + fmt.Printf("%v passes rules found in %v\n", path, actualPath) + } + + return nil +} + +// importRules produces a file with a set for a single type. +type importRules struct { + myPackage *types.Package + imports *generator.ImportTracker +} + +var ( + _ = generator.Generator(&importRules{}) + _ = generator.FileType(importRuleFile{}) +) + +func (r *importRules) Name() string { return "import rules" } +func (r *importRules) Filter(*generator.Context, *types.Type) bool { return false } +func (r *importRules) Namers(*generator.Context) namer.NameSystems { return nil } +func (r *importRules) PackageVars(*generator.Context) []string { return []string{} } +func (r *importRules) PackageConsts(*generator.Context) []string { return []string{} } +func (r *importRules) GenerateType(*generator.Context, *types.Type, io.Writer) error { return nil } +func (r *importRules) Filename() string { return ".import-restrictions" } +func (r *importRules) FileType() string { return importBossFileType } +func (r *importRules) Init(c *generator.Context, w io.Writer) error { return nil } + +func dfsImports(dest *[]string, seen map[string]bool, p *types.Package) { + for _, p2 := range p.Imports { + if seen[p2.Path] { + continue + } + seen[p2.Path] = true + dfsImports(dest, seen, p2) + *dest = append(*dest, p2.Path) + } +} + +func (r *importRules) Imports(*generator.Context) []string { + all := []string{} + dfsImports(&all, map[string]bool{}, r.myPackage) + return all +} diff --git a/cmd/libs/go2idl/import-boss/generators/import_restrict_test.go b/cmd/libs/go2idl/import-boss/generators/import_restrict_test.go new file mode 100644 index 0000000000..a70cc38160 --- /dev/null +++ b/cmd/libs/go2idl/import-boss/generators/import_restrict_test.go @@ -0,0 +1,36 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package generators + +import ( + "testing" +) + +func TestRemoveLastDir(t *testing.T) { + table := map[string]struct{ newPath, removedDir string }{ + "a/b/c": {"a/c", "b"}, + } + for input, expect := range table { + gotPath, gotRemoved := removeLastDir(input) + if e, a := expect.newPath, gotPath; e != a { + t.Errorf("%v: wanted %v, got %v", input, e, a) + } + if e, a := expect.removedDir, gotRemoved; e != a { + t.Errorf("%v: wanted %v, got %v", input, e, a) + } + } +} diff --git a/cmd/libs/go2idl/import-boss/main.go b/cmd/libs/go2idl/import-boss/main.go new file mode 100644 index 0000000000..7d10171ebc --- /dev/null +++ b/cmd/libs/go2idl/import-boss/main.go @@ -0,0 +1,90 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// import-boss enforces import restrictions in a given repository. +// +// When a directory is verified, import-boss looks for a file called +// ".import-restrictions". If this file is not found, parent directories will be +// recursively searched. +// +// If an ".import-restrictions" file is found, then all imports of the package +// are checked against each "rule" in the file. A rule consists of three parts: +// * A SelectorRegexp, to select the import paths that the rule applies to. +// * A list of AllowedPrefixes +// * A list of ForbiddenPrefixes +// An import is allowed if it matches at least one allowed prefix and does not +// match any forbidden prefix. An example file looks like this: +// +// { +// "Rules": [ +// { +// "SelectorRegexp": "k8s[.]io", +// "AllowedPrefixes": [ +// "k8s.io/kubernetes/cmd/libs/go2idl", +// "k8s.io/kubernetes/third_party" +// ], +// "ForbiddenPrefixes": [ +// "k8s.io/kubernetes/pkg/third_party/deprecated" +// ] +// }, +// { +// "SelectorRegexp": "^unsafe$", +// "AllowedPrefixes": [ +// ], +// "ForbiddenPrefixes": [ +// "" +// ] +// } +// ] +// } +// +// Note the secound block explicitly matches the unsafe package, and forbids it +// ("" is a prefix of everything). +package main + +import ( + "os" + + "k8s.io/kubernetes/cmd/libs/go2idl/args" + "k8s.io/kubernetes/cmd/libs/go2idl/import-boss/generators" + + "github.com/golang/glog" +) + +func main() { + arguments := args.Default() + + // Override defaults. These are Kubernetes specific input and output + // locations. + arguments.InputDirs = []string{ + "k8s.io/kubernetes/pkg/", + "k8s.io/kubernetes/cmd/", + "k8s.io/kubernetes/plugin/", + } + arguments.OutputBase = "" + arguments.Recursive = true + // arguments.VerifyOnly = true + + if err := arguments.Execute( + generators.NameSystems(), + generators.DefaultNameSystem(), + generators.Packages, + ); err != nil { + glog.Errorf("Error: %v", err) + os.Exit(1) + } + glog.Info("Completed successfully.") +} diff --git a/cmd/libs/go2idl/parser/parse.go b/cmd/libs/go2idl/parser/parse.go index 44454c2dc1..7addec2b45 100644 --- a/cmd/libs/go2idl/parser/parse.go +++ b/cmd/libs/go2idl/parser/parse.go @@ -19,6 +19,7 @@ package parser import ( "fmt" "io/ioutil" + "os" "os/exec" "path/filepath" "strings" @@ -29,6 +30,8 @@ import ( "k8s.io/kubernetes/third_party/golang/go/parser" "k8s.io/kubernetes/third_party/golang/go/token" tc "k8s.io/kubernetes/third_party/golang/go/types" + + "github.com/golang/glog" ) // Builder lets you add all the go files in all the packages that you care @@ -158,6 +161,35 @@ func (b *Builder) AddDir(dir string) error { return b.addDir(dir, true) } +// AddDirRecursive is just like AddDir, but it also recursively adds +// subdirectories; it returns an error only if the path couldn't be resolved; +// any directories recursed into without go source are ignored. +func (b *Builder) AddDirRecursive(dir string) error { + // First, find it, so we know what path to use. + pkg, err := b.context.Import(dir, ".", build.FindOnly) + if err != nil { + return fmt.Errorf("unable to *find* %q: %v", dir, err) + } + + if err := b.addDir(dir, true); err != nil { + glog.Warningf("Ignoring directory %v: %v", dir, err) + } + + prefix := strings.TrimSuffix(pkg.Dir, strings.TrimSuffix(dir, "/")) + filepath.Walk(pkg.Dir, func(path string, info os.FileInfo, err error) error { + if info != nil && info.IsDir() { + trimmed := strings.TrimPrefix(path, prefix) + if trimmed != "" { + if err := b.addDir(trimmed, true); err != nil { + glog.Warningf("Ignoring child directory %v: %v", trimmed, err) + } + } + } + return nil + }) + return nil +} + // The implementation of AddDir. A flag indicates whether this directory was // user-requested or just from following the import graph. func (b *Builder) addDir(dir string, userRequested bool) error { @@ -202,7 +234,6 @@ func (b *Builder) importer(imports map[string]*tc.Package, path string) (*tc.Pac // Ignore errors in paths that we're importing solely because // they're referenced by other packages. ignoreError = true - // fmt.Printf("trying to import %q\n", path) if err := b.addDir(path, false); err != nil { return nil, err } @@ -286,8 +317,8 @@ func (b *Builder) FindTypes() (types.Universe, error) { u := types.Universe{} - for pkgName, pkg := range b.pkgs { - if !b.userRequested[pkgName] { + for pkgPath, pkg := range b.pkgs { + if !b.userRequested[pkgPath] { // Since walkType is recursive, all types that the // packages they asked for depend on will be included. // But we don't need to include all types in all @@ -318,9 +349,10 @@ func (b *Builder) FindTypes() (types.Universe, error) { b.addVariable(u, nil, tv) } } - for p := range b.importGraph[pkgName] { - u.AddImports(pkgName, p) + for p := range b.importGraph[pkgPath] { + u.AddImports(pkgPath, p) } + u.Package(pkgPath).Name = pkg.Name() } return u, nil } @@ -349,6 +381,10 @@ func tcNameToName(in string) types.Name { // Detect anonymous type names. (These may have '.' characters because // embedded types may have packages, so we detect them specially.) if strings.HasPrefix(in, "struct{") || + strings.HasPrefix(in, "<-chan") || + strings.HasPrefix(in, "chan<-") || + strings.HasPrefix(in, "chan ") || + strings.HasPrefix(in, "func(") || strings.HasPrefix(in, "*") || strings.HasPrefix(in, "map[") || strings.HasPrefix(in, "[") { diff --git a/hack/after-build/run-import-boss.sh b/hack/after-build/run-import-boss.sh new file mode 100755 index 0000000000..d57b273039 --- /dev/null +++ b/hack/after-build/run-import-boss.sh @@ -0,0 +1,26 @@ +#!/bin/bash + +# Copyright 2015 The Kubernetes Authors All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -o errexit +set -o nounset +set -o pipefail + +KUBE_ROOT=$(dirname "${BASH_SOURCE}")/../.. +source "${KUBE_ROOT}/hack/lib/init.sh" + +kube::golang::setup_env + +$(kube::util::find-binary "import-boss") "$@" diff --git a/hack/verify-import-boss.sh b/hack/verify-import-boss.sh new file mode 100755 index 0000000000..619fe6d9c6 --- /dev/null +++ b/hack/verify-import-boss.sh @@ -0,0 +1,30 @@ +#!/bin/bash + +# Copyright 2014 The Kubernetes Authors All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -o errexit +set -o nounset +set -o pipefail + +KUBE_ROOT=$(dirname "${BASH_SOURCE}")/.. + +KUBE_ROOT=$(dirname "${BASH_SOURCE}")/.. +source "${KUBE_ROOT}/hack/lib/init.sh" + +kube::golang::setup_env + +"${KUBE_ROOT}/hack/build-go.sh" cmd/libs/go2idl/import-boss + +"${KUBE_ROOT}/hack/after-build/run-import-boss.sh" --verify-only