From 11a3e8f64f671209f82bf6d70888542ef78da423 Mon Sep 17 00:00:00 2001 From: Yann Hamon Date: Tue, 15 Dec 2020 16:27:25 +0100 Subject: [PATCH 1/4] add cpu profiling --- cmd/kubeconform/main.go | 14 ++++++++++++++ pkg/config/config.go | 2 ++ 2 files changed, 16 insertions(+) diff --git a/cmd/kubeconform/main.go b/cmd/kubeconform/main.go index 4a6367f..c9564c0 100644 --- a/cmd/kubeconform/main.go +++ b/cmd/kubeconform/main.go @@ -3,7 +3,9 @@ package main import ( "context" "fmt" + "log" "os" + "runtime/pprof" "sync" "github.com/yannh/kubeconform/pkg/config" @@ -51,6 +53,18 @@ func realMain() int { return 1 } + if cfg.CPUProfileFile != "" { + f, err := os.Create(cfg.CPUProfileFile) + if err != nil { + log.Fatal("could not create CPU profile: ", err) + } + defer f.Close() + if err := pprof.StartCPUProfile(f); err != nil { + log.Fatal("could not start CPU profile: ", err) + } + defer pprof.StopCPUProfile() + } + // Detect whether we have data being piped through stdin stat, _ := os.Stdin.Stat() isStdin := (stat.Mode() & os.ModeCharDevice) == 0 diff --git a/pkg/config/config.go b/pkg/config/config.go index 6b1232b..09ca557 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -9,6 +9,7 @@ import ( ) type Config struct { + CPUProfileFile string ExitOnError bool Files []string SchemaLocations []string @@ -74,6 +75,7 @@ func FromFlags(progName string, args []string) (Config, string, error) { flags.StringVar(&c.OutputFormat, "output", "text", "output format - json, tap, text") flags.BoolVar(&c.Verbose, "verbose", false, "print results for all resources") flags.BoolVar(&c.SkipTLS, "insecure-skip-tls-verify", false, "disable verification of the server's SSL certificate. This will make your HTTPS connections insecure") + flags.StringVar(&c.CPUProfileFile, "cpu-prof", "", "debug - log CPU profiling to file") flags.BoolVar(&c.Help, "h", false, "show help information") flags.Usage = func() { fmt.Fprintf(os.Stderr, "Usage: %s [OPTION]... [FILE OR FOLDER]...\n", progName) From 4afe9b1977843c09b9130bad33f9874f106e9e30 Mon Sep 17 00:00:00 2001 From: Yann Hamon Date: Tue, 15 Dec 2020 17:04:44 +0100 Subject: [PATCH 2/4] reduce allocations done when splitting --- pkg/config/config.go | 2 +- pkg/resource/files.go | 52 ++++++++++++++++++++++++++----------------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 09ca557..fbac538 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -75,7 +75,7 @@ func FromFlags(progName string, args []string) (Config, string, error) { flags.StringVar(&c.OutputFormat, "output", "text", "output format - json, tap, text") flags.BoolVar(&c.Verbose, "verbose", false, "print results for all resources") flags.BoolVar(&c.SkipTLS, "insecure-skip-tls-verify", false, "disable verification of the server's SSL certificate. This will make your HTTPS connections insecure") - flags.StringVar(&c.CPUProfileFile, "cpu-prof", "", "debug - log CPU profiling to file") + //flags.StringVar(&c.CPUProfileFile, "cpu-prof", "", "debug - log CPU profiling to file") flags.BoolVar(&c.Help, "h", false, "show help information") flags.Usage = func() { fmt.Fprintf(os.Stderr, "Usage: %s [OPTION]... [FILE OR FOLDER]...\n", progName) diff --git a/pkg/resource/files.go b/pkg/resource/files.go index 6b443ac..81cf272 100644 --- a/pkg/resource/files.go +++ b/pkg/resource/files.go @@ -42,6 +42,7 @@ func isIgnored(path string, ignoreFilePatterns []string) (bool, error) { func FromFiles(ctx context.Context, ignoreFilePatterns []string, paths ...string) (<-chan Resource, <-chan error) { resources := make(chan Resource) + files := make(chan string) errors := make(chan error) go func() { @@ -71,27 +72,7 @@ func FromFiles(ctx context.Context, ignoreFilePatterns []string, paths ...string return nil } - f, err := os.Open(p) - if err != nil { - return err - } - - scanner := bufio.NewScanner(f) - const maxResourceSize = 4 * 1024 * 1024 // 4MB ought to be enough for everybody - buf := make([]byte, maxResourceSize) - scanner.Buffer(buf, maxResourceSize) - scanner.Split(SplitYAMLDocument) - nRes := 0 - for res := scanner.Scan(); res != false; res = scanner.Scan() { - resources <- Resource{Path: p, Bytes: scanner.Bytes()} - nRes++ - } - if err := scanner.Err(); err != nil { - errors <- DiscoveryError{p, err} - } - if nRes == 0 { - resources <- Resource{Path: p, Bytes: []byte{}} - } + files <- p return nil }) @@ -101,6 +82,35 @@ func FromFiles(ctx context.Context, ignoreFilePatterns []string, paths ...string } } + close(files) + }() + + go func() { + maxResourceSize := 4 * 1024 * 1024 + buf := make([]byte, maxResourceSize) + + for p := range files { + f, err := os.Open(p) + if err != nil { + errors <- DiscoveryError{p, err} + } + + scanner := bufio.NewScanner(f) + scanner.Buffer(buf, maxResourceSize) + scanner.Split(SplitYAMLDocument) + nRes := 0 + for res := scanner.Scan(); res != false; res = scanner.Scan() { + resources <- Resource{Path: p, Bytes: []byte(scanner.Text())} + nRes++ + } + if err := scanner.Err(); err != nil { + errors <- DiscoveryError{p, err} + } + if nRes == 0 { + resources <- Resource{Path: p, Bytes: []byte{}} + } + } + close(resources) close(errors) }() From 29a8f4c09e8a921488953a1184b85bea900784eb Mon Sep 17 00:00:00 2001 From: Yann Hamon Date: Tue, 15 Dec 2020 18:35:33 +0100 Subject: [PATCH 3/4] avoid double unmarshalling --- cmd/kubeconform/main.go | 3 +++ pkg/config/config.go | 2 +- pkg/resource/files.go | 2 +- pkg/resource/resource.go | 23 +++++++++++++++++++++++ pkg/resource/resource_test.go | 29 +++++++++++++++++++++++++++++ pkg/validator/validator.go | 12 ++++++------ pkg/validator/validator_test.go | 18 +++++++++--------- 7 files changed, 72 insertions(+), 17 deletions(-) diff --git a/cmd/kubeconform/main.go b/cmd/kubeconform/main.go index c9564c0..894cdc5 100644 --- a/cmd/kubeconform/main.go +++ b/cmd/kubeconform/main.go @@ -5,6 +5,7 @@ import ( "fmt" "log" "os" + "runtime" "runtime/pprof" "sync" @@ -62,6 +63,8 @@ func realMain() int { if err := pprof.StartCPUProfile(f); err != nil { log.Fatal("could not start CPU profile: ", err) } + runtime.SetBlockProfileRate(1) + defer pprof.StopCPUProfile() } diff --git a/pkg/config/config.go b/pkg/config/config.go index fbac538..09ca557 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -75,7 +75,7 @@ func FromFlags(progName string, args []string) (Config, string, error) { flags.StringVar(&c.OutputFormat, "output", "text", "output format - json, tap, text") flags.BoolVar(&c.Verbose, "verbose", false, "print results for all resources") flags.BoolVar(&c.SkipTLS, "insecure-skip-tls-verify", false, "disable verification of the server's SSL certificate. This will make your HTTPS connections insecure") - //flags.StringVar(&c.CPUProfileFile, "cpu-prof", "", "debug - log CPU profiling to file") + flags.StringVar(&c.CPUProfileFile, "cpu-prof", "", "debug - log CPU profiling to file") flags.BoolVar(&c.Help, "h", false, "show help information") flags.Usage = func() { fmt.Fprintf(os.Stderr, "Usage: %s [OPTION]... [FILE OR FOLDER]...\n", progName) diff --git a/pkg/resource/files.go b/pkg/resource/files.go index 81cf272..b13f4bf 100644 --- a/pkg/resource/files.go +++ b/pkg/resource/files.go @@ -111,8 +111,8 @@ func FromFiles(ctx context.Context, ignoreFilePatterns []string, paths ...string } } - close(resources) close(errors) + close(resources) }() return resources, errors diff --git a/pkg/resource/resource.go b/pkg/resource/resource.go index a764fb4..96a46ef 100644 --- a/pkg/resource/resource.go +++ b/pkg/resource/resource.go @@ -42,3 +42,26 @@ func (res *Resource) Signature() (*Signature, error) { res.sig = &Signature{Kind: resource.Kind, Version: resource.APIVersion, Namespace: resource.Metadata.Namespace, Name: name} return res.sig, err } + +func (res *Resource) SignatureFromMap(m map[string]interface{}) (*Signature, error) { + if res.sig != nil { + return res.sig, nil + } + + APIVersion, _ := m["apiVersion"].(string) + Kind, _ := m["kind"].(string) + + var name, ns string + Metadata, ok := m["metadata"].(map[string]interface{}) + if ok { + name, _ = Metadata["name"].(string) + ns, _ = Metadata["namespace"].(string) + if _, ok := Metadata["generateName"].(string); ok { + name = Metadata["generateName"].(string) + "{{ generateName }}" + } + } + + // We cache the result to not unmarshall every time we want to access the signature + res.sig = &Signature{Kind: Kind, Version: APIVersion, Namespace: ns, Name: name} + return res.sig, nil +} diff --git a/pkg/resource/resource_test.go b/pkg/resource/resource_test.go index 50144e8..6093e89 100644 --- a/pkg/resource/resource_test.go +++ b/pkg/resource/resource_test.go @@ -1,9 +1,12 @@ package resource_test import ( + "fmt" + "log" "testing" "github.com/yannh/kubeconform/pkg/resource" + "sigs.k8s.io/yaml" ) func TestSignatureFromBytes(t *testing.T) { @@ -47,3 +50,29 @@ spec: } } } + +func TestSignatureFromMap(t *testing.T) { + testCases := []struct { + b string + }{ + { + "apiVersion: v1\nkind: ReplicationController\nmetadata:\n name: \"bob\"\nspec:\n replicas: 2\n", + }, + } + + for _, testCase := range testCases { + res := resource.Resource{ + Path: "foo", + Bytes: []byte(testCase.b), + } + + var r map[string]interface{} + if err := yaml.Unmarshal(res.Bytes, &r); err != nil { + log.Fatal(err) + } + + res.SignatureFromMap(r) + sig, _ := res.Signature() + fmt.Printf("%+v", sig) + } +} diff --git a/pkg/validator/validator.go b/pkg/validator/validator.go index 71343ea..c2c2984 100644 --- a/pkg/validator/validator.go +++ b/pkg/validator/validator.go @@ -109,7 +109,12 @@ func (val *v) ValidateResource(res resource.Resource) Result { return Result{Resource: res, Err: nil, Status: Empty} } - sig, err := res.Signature() + var r map[string]interface{} + if err := yaml.Unmarshal(res.Bytes, &r); err != nil { + return Result{Resource: res, Status: Error, Err: fmt.Errorf("error unmarshalling resource: %s", err)} + } + + sig, err := res.SignatureFromMap(r) if err != nil { return Result{Resource: res, Err: fmt.Errorf("error while parsing: %s", err), Status: Error} } @@ -122,11 +127,6 @@ func (val *v) ValidateResource(res resource.Resource) Result { return Result{Resource: res, Err: fmt.Errorf("prohibited resource kind %s", sig.Kind), Status: Error} } - var r map[string]interface{} - if err := yaml.Unmarshal(res.Bytes, &r); err != nil { - return Result{Resource: res, Status: Error, Err: fmt.Errorf("error unmarshalling resource: %s", err)} - } - if r == nil { // Resource is empty return Result{Resource: res, Err: nil, Status: Empty} } diff --git a/pkg/validator/validator_test.go b/pkg/validator/validator_test.go index 168c7fd..3184936 100644 --- a/pkg/validator/validator_test.go +++ b/pkg/validator/validator_test.go @@ -18,7 +18,7 @@ func TestValidate(t *testing.T) { { "valid resource", []byte(` -Kind: name +kind: name firstName: foo lastName: bar `), @@ -26,7 +26,7 @@ lastName: bar "title": "Example Schema", "type": "object", "properties": { - "Kind": { + "kind": { "type": "string" }, "firstName": { @@ -48,7 +48,7 @@ lastName: bar { "invalid resource", []byte(` -Kind: name +kind: name firstName: foo lastName: bar `), @@ -56,7 +56,7 @@ lastName: bar "title": "Example Schema", "type": "object", "properties": { - "Kind": { + "kind": { "type": "string" }, "firstName": { @@ -78,14 +78,14 @@ lastName: bar { "missing required field", []byte(` -Kind: name +kind: name firstName: foo `), []byte(`{ "title": "Example Schema", "type": "object", "properties": { - "Kind": { + "kind": { "type": "string" }, "firstName": { @@ -107,7 +107,7 @@ firstName: foo { "resource has invalid yaml", []byte(` -Kind: name +kind: name firstName foo lastName: bar `), @@ -115,7 +115,7 @@ lastName: bar "title": "Example Schema", "type": "object", "properties": { - "Kind": { + "kind": { "type": "string" }, "firstName": { @@ -151,7 +151,7 @@ lastName: bar regs: nil, } if got := val.ValidateResource(resource.Resource{Bytes: testCase.rawResource}); got.Status != testCase.expect { - t.Errorf("%d - expected %d, got %d", i, testCase.expect, got.Status) + t.Errorf("%d - expected %d, got %d: %s", i, testCase.expect, got.Status, got.Err.Error()) } } } From c5aa8e6da39370d106d23bf9208e898169e789b7 Mon Sep 17 00:00:00 2001 From: Yann Hamon Date: Tue, 15 Dec 2020 19:36:22 +0100 Subject: [PATCH 4/4] fix closing files --- pkg/resource/files.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/resource/files.go b/pkg/resource/files.go index b13f4bf..a3b5e16 100644 --- a/pkg/resource/files.go +++ b/pkg/resource/files.go @@ -93,6 +93,7 @@ func FromFiles(ctx context.Context, ignoreFilePatterns []string, paths ...string f, err := os.Open(p) if err != nil { errors <- DiscoveryError{p, err} + continue } scanner := bufio.NewScanner(f) @@ -109,6 +110,8 @@ func FromFiles(ctx context.Context, ignoreFilePatterns []string, paths ...string if nRes == 0 { resources <- Resource{Path: p, Bytes: []byte{}} } + + f.Close() } close(errors)