Skip to content

Commit

Permalink
Improved error handling (#70)
Browse files Browse the repository at this point in the history
* discoverConnections has no errors to return

* error if parsing k8s object fails

* Types for reporting file-processing errors

* Functions to return multiple error objects

* unit testing

* not exporting parse functions

* ScanK8sDeployObject -> ScanK8sWorkloadObject

* Moving regexs to vars at the top of the file

* Moving error type to controller package

* discard FileProcessingErrorList

* error should not start with a capital letter

* create all error objects in error_types.go

* DocumentID() returns an error + Location() method

* Work with externally-provided logger

* A missing not

* implemented stop-on-first-error option

* Fix lint issue

* report line number when marshaling fails

* PoliciesFromFolderPath() -> PoliciesSynthesizer

* Logger.Errorf() takes error as an arg

* A missing check to stop on first error

* Always stop processing on fatal error

* Not assuming a severe error is always last

* separate logging from custom error interface

* not relying on fatal error being the last error

* Do not rely on fatal errors to be last

Signed-off-by: Ziv Nevo <[email protected]>
  • Loading branch information
zivnevo committed Sep 7, 2022
1 parent cf82fb4 commit 82fcc36
Show file tree
Hide file tree
Showing 21 changed files with 1,543 additions and 247 deletions.
31 changes: 6 additions & 25 deletions cmd/nettop/main.go
Original file line number Diff line number Diff line change
@@ -1,43 +1,24 @@
package main

import (
"errors"
"fmt"
"os"
"syscall"

"go.uber.org/zap"

"github.com/np-guard/cluster-topology-analyzer/pkg/common"
"github.com/np-guard/cluster-topology-analyzer/pkg/controller"
)

func runAnalysis() int {
logger := common.SetupLogger()
defer func() {
err := logger.Sync()
// If stderr is a TTY we might not be able to sync.
// See https://github.com/uber-go/zap/issues/991#issuecomment-962098428 for
// why we ignore ENOTTY. On OSX, we must ignore EBADF.
if err != nil && !errors.Is(err, syscall.ENOTTY) && !errors.Is(err, syscall.EBADF) {
panic(err)
}
}()

func main() {
var inArgs common.InArgs
err := common.ParseInArgs(&inArgs)
if err != nil {
zap.S().Debug("error parsing arguments, exiting...")
return 1
fmt.Fprintf(os.Stderr, "error parsing arguments: %v. exiting...\n", err)
os.Exit(1)
}

err = controller.Start(inArgs)
if err != nil {
zap.S().Debug("error running topology analysis exiting...")
return 1
fmt.Fprintf(os.Stderr, "error running topology analysis: %v. exiting...", err)
os.Exit(1)
}
return 0
}

func main() {
os.Exit(runAnalysis())
}
3 changes: 0 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ module github.com/np-guard/cluster-topology-analyzer
go 1.17

require (
go.uber.org/zap v1.23.0
gopkg.in/yaml.v3 v3.0.1
k8s.io/api v0.23.8
k8s.io/apimachinery v0.23.8
Expand All @@ -18,8 +17,6 @@ require (
github.com/json-iterator/go v1.1.12 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
go.uber.org/atomic v1.7.0 // indirect
go.uber.org/multierr v1.6.0 // indirect
golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd // indirect
golang.org/x/text v0.3.7 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
Expand Down
17 changes: 1 addition & 16 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ github.com/NYTimes/gziphandler v0.0.0-20170623195520-56545f4a5d46/go.mod h1:3wb0
github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0=
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE=
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8=
github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI=
github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI=
Expand Down Expand Up @@ -205,8 +203,6 @@ github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGV
github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY=
github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
Expand All @@ -217,15 +213,12 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Expand All @@ -238,14 +231,6 @@ go.opencensus.io v0.22.3/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw=
go.opencensus.io v0.22.4/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw=
go.opencensus.io v0.22.5/go.mod h1:5pWMHQbX5EPX2/62yrJeAkowc+lfs/XD7Uxpq3pI6kk=
go.opencensus.io v0.23.0/go.mod h1:XItmlyltB5F7CS4xOC1DcqMoFqwtC6OG2xF7mCv7P7E=
go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw=
go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI=
go.uber.org/goleak v1.1.11/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ=
go.uber.org/multierr v1.6.0 h1:y6IPFStTAIT5Ytl7/XYmHvzXQ7S3g/IeZW9hyZ5thw4=
go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU=
go.uber.org/zap v1.23.0 h1:OjGQ5KQDEUawVHxNwQgPpiypGHOxo2mNZsOqTak4fFY=
go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
Expand Down
58 changes: 16 additions & 42 deletions pkg/analyzer/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,8 @@ import (

const yamlParseBufferSize = 200

// ParsePod parses replicationController
func ParsePod(r io.Reader) *v1.Pod {
if r == nil {
return nil
}
rc := v1.Pod{}
err := yaml.NewYAMLOrJSONDecoder(r, yamlParseBufferSize).Decode(&rc)
if err != nil {
return nil
}
return &rc
}

// ParseDeployment parses deployment
func ParseDeployment(r io.Reader) *appsv1.Deployment {
// parseDeployment parses deployment
func parseDeployment(r io.Reader) *appsv1.Deployment {
if r == nil {
return nil
}
Expand All @@ -38,8 +25,8 @@ func ParseDeployment(r io.Reader) *appsv1.Deployment {
return &rc
}

// ParseReplicaSet parses replicaset
func ParseReplicaSet(r io.Reader) *appsv1.ReplicaSet {
// parseReplicaSet parses replicaset
func parseReplicaSet(r io.Reader) *appsv1.ReplicaSet {
if r == nil {
return nil
}
Expand All @@ -51,8 +38,8 @@ func ParseReplicaSet(r io.Reader) *appsv1.ReplicaSet {
return &rc
}

// ParseReplicationController parses replicationController
func ParseReplicationController(r io.Reader) *v1.ReplicationController {
// parseReplicationController parses replicationController
func parseReplicationController(r io.Reader) *v1.ReplicationController {
if r == nil {
return nil
}
Expand All @@ -65,8 +52,8 @@ func ParseReplicationController(r io.Reader) *v1.ReplicationController {
return &rc
}

// ParseDaemonSet parses replicationController
func ParseDaemonSet(r io.Reader) *appsv1.DaemonSet {
// parseDaemonSet parses a DaemonSet resource
func parseDaemonSet(r io.Reader) *appsv1.DaemonSet {
if r == nil {
return nil
}
Expand All @@ -79,8 +66,8 @@ func ParseDaemonSet(r io.Reader) *appsv1.DaemonSet {
return &rc
}

// ParseStatefulSet parses replicationController
func ParseStatefulSet(r io.Reader) *appsv1.StatefulSet {
// parseStatefulSet parses a StatefulSet resource
func parseStatefulSet(r io.Reader) *appsv1.StatefulSet {
if r == nil {
return nil
}
Expand All @@ -93,8 +80,8 @@ func ParseStatefulSet(r io.Reader) *appsv1.StatefulSet {
return &rc
}

// ParseJob parses replicationController
func ParseJob(r io.Reader) *batchv1.Job {
// parseJob parses a Job resource
func parseJob(r io.Reader) *batchv1.Job {
if r == nil {
return nil
}
Expand All @@ -107,8 +94,8 @@ func ParseJob(r io.Reader) *batchv1.Job {
return &rc
}

// ParseService parses replicationController
func ParseService(r io.Reader) *v1.Service {
// parseService parses a Service resource
func parseService(r io.Reader) *v1.Service {
if r == nil {
return nil
}
Expand All @@ -120,21 +107,8 @@ func ParseService(r io.Reader) *v1.Service {
return &rc
}

// ParseServiceAccount parses replicationController
func ParseServiceAccount(r io.Reader) *v1.ServiceAccount {
if r == nil {
return nil
}
rc := v1.ServiceAccount{}
err := yaml.NewYAMLOrJSONDecoder(r, yamlParseBufferSize).Decode(&rc)
if err != nil {
return nil
}
return &rc
}

// ParseConfigMap parses ConfigMap
func ParseConfigMap(r io.Reader) *v1.ConfigMap {
// parseConfigMap parses a ConfigMap resource
func parseConfigMap(r io.Reader) *v1.ConfigMap {
if r == nil {
return nil
}
Expand Down
32 changes: 17 additions & 15 deletions pkg/analyzer/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,54 +6,51 @@ import (
"net/url"
"strconv"

"go.uber.org/zap"
v1 "k8s.io/api/core/v1"
metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/np-guard/cluster-topology-analyzer/pkg/common"
)

// Create a common.Resource object from a k8s Workload object
func ScanK8sDeployObject(kind string, objDataBuf []byte) (common.Resource, error) {
func ScanK8sWorkloadObject(kind string, objDataBuf []byte) (common.Resource, error) {
var podSpecV1 v1.PodTemplateSpec
var resourceCtx common.Resource
var metaObj metaV1.Object
resourceCtx.Resource.Kind = kind
switch kind {
case "Pod":
zap.S().Info("evaluating pod")
switch kind { // TODO: handle Pod
case "ReplicaSet":
obj := ParseReplicaSet(bytes.NewReader(objDataBuf))
obj := parseReplicaSet(bytes.NewReader(objDataBuf))
resourceCtx.Resource.Labels = obj.GetLabels()
resourceCtx.Resource.Selectors = matchLabelSelectorToStrLabels(obj.Spec.Selector.MatchLabels)
podSpecV1 = obj.Spec.Template
metaObj = obj
case "ReplicationController":
obj := ParseReplicationController(bytes.NewReader(objDataBuf))
obj := parseReplicationController(bytes.NewReader(objDataBuf))
resourceCtx.Resource.Labels = obj.Spec.Template.Labels
resourceCtx.Resource.Selectors = matchLabelSelectorToStrLabels(obj.Spec.Selector)
podSpecV1 = *obj.Spec.Template
metaObj = obj
case "Deployment":
obj := ParseDeployment(bytes.NewReader(objDataBuf))
obj := parseDeployment(bytes.NewReader(objDataBuf))
resourceCtx.Resource.Labels = obj.Spec.Template.Labels
resourceCtx.Resource.Selectors = matchLabelSelectorToStrLabels(obj.Spec.Selector.MatchLabels)
podSpecV1 = obj.Spec.Template
metaObj = obj
case "DaemonSet":
obj := ParseDaemonSet(bytes.NewReader(objDataBuf))
obj := parseDaemonSet(bytes.NewReader(objDataBuf))
resourceCtx.Resource.Labels = obj.Spec.Template.Labels
resourceCtx.Resource.Selectors = matchLabelSelectorToStrLabels(obj.Spec.Selector.MatchLabels)
podSpecV1 = obj.Spec.Template
metaObj = obj
case "StatefulSet":
obj := ParseStatefulSet(bytes.NewReader(objDataBuf))
obj := parseStatefulSet(bytes.NewReader(objDataBuf))
resourceCtx.Resource.Labels = obj.Spec.Template.Labels
resourceCtx.Resource.Selectors = matchLabelSelectorToStrLabels(obj.Spec.Selector.MatchLabels)
podSpecV1 = obj.Spec.Template
metaObj = obj
case "Job":
obj := ParseJob(bytes.NewReader(objDataBuf))
obj := parseJob(bytes.NewReader(objDataBuf))
resourceCtx.Resource.Labels = obj.Spec.Template.Labels
resourceCtx.Resource.Selectors = matchLabelSelectorToStrLabels(obj.Spec.Selector.MatchLabels)
podSpecV1 = obj.Spec.Template
Expand All @@ -75,7 +72,10 @@ func matchLabelSelectorToStrLabels(labels map[string]string) []string {
}

func ScanK8sConfigmapObject(kind string, objDataBuf []byte) (common.CfgMap, error) {
obj := ParseConfigMap(bytes.NewReader(objDataBuf))
obj := parseConfigMap(bytes.NewReader(objDataBuf))
if obj == nil {
return common.CfgMap{}, fmt.Errorf("unable to parse configmap")
}

fullName := obj.ObjectMeta.Namespace + "/" + obj.ObjectMeta.Name
data := map[string]string{}
Expand All @@ -95,7 +95,10 @@ func ScanK8sServiceObject(kind string, objDataBuf []byte) (common.Service, error
}

var serviceCtx common.Service
svcObj := ParseService(bytes.NewReader(objDataBuf))
svcObj := parseService(bytes.NewReader(objDataBuf))
if svcObj == nil {
return common.Service{}, fmt.Errorf("failed to parse Service resource")
}
serviceCtx.Resource.Name = svcObj.GetName()
serviceCtx.Resource.Namespace = svcObj.Namespace
serviceCtx.Resource.Kind = kind
Expand Down Expand Up @@ -129,8 +132,7 @@ func parseDeployResource(podSpec *v1.PodTemplateSpec, obj metaV1.Object, resourc
}
for _, e := range container.Env {
if e.Value != "" {
isPotentialAddress := IsNetworkAddressValue(e.Value)
if isPotentialAddress {
if IsNetworkAddressValue(e.Value) {
resourceCtx.Resource.Envs = append(resourceCtx.Resource.Envs, e.Value)
}
} else if e.ValueFrom != nil && e.ValueFrom.ConfigMapKeyRef != nil {
Expand Down
7 changes: 2 additions & 5 deletions pkg/common/params.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package common

import (
"errors"
"flag"

"go.uber.org/zap"
"fmt"
)

func ParseInArgs(args *InArgs) error {
Expand All @@ -20,9 +18,8 @@ func ParseInArgs(args *InArgs) error {
*args.GitBranch == "" ||
*args.CommitID == "" ||
*args.GitURL == "" {
zap.S().Debugf("missing parameters: [%s %s %s %s]", *args.DirPath, *args.GitURL, *args.GitBranch, *args.CommitID)
flag.PrintDefaults()
return errors.New("missing params")
return fmt.Errorf("missing parameters: [%s %s %s %s]", *args.DirPath, *args.GitURL, *args.GitBranch, *args.CommitID)
}

return nil
Expand Down
17 changes: 0 additions & 17 deletions pkg/common/utils.go

This file was deleted.

Loading

0 comments on commit 82fcc36

Please sign in to comment.