From fe0947dcb3212448883e8ca1b5979eb150bc808e Mon Sep 17 00:00:00 2001 From: Bricktop Date: Mon, 11 Oct 2021 23:57:43 +0200 Subject: [PATCH] Add error handling where missing (#2516) Signed-off-by: Marcel Mueller --- pkg/engine/mutate/utils.go | 5 ++++- pkg/generate/cleanup/controller.go | 5 ++++- pkg/kyverno/apply/apply_command.go | 2 -- pkg/kyverno/apply/report_test.go | 4 ---- pkg/kyverno/common/common.go | 5 ++++- pkg/policyreport/changerequestcreator.go | 20 ++++++++++++++------ pkg/webhooks/server.go | 5 ++++- 7 files changed, 30 insertions(+), 16 deletions(-) diff --git a/pkg/engine/mutate/utils.go b/pkg/engine/mutate/utils.go index b29ccf8183..d909207842 100644 --- a/pkg/engine/mutate/utils.go +++ b/pkg/engine/mutate/utils.go @@ -12,7 +12,10 @@ type buffer struct { func (buff buffer) UnmarshalJSON(b []byte) error { buff.Reset() - buff.Write(b) + _, err := buff.Write(b) + if err != nil { + return err + } return nil } diff --git a/pkg/generate/cleanup/controller.go b/pkg/generate/cleanup/controller.go index e2c089d02f..673712a9ed 100644 --- a/pkg/generate/cleanup/controller.go +++ b/pkg/generate/cleanup/controller.go @@ -342,7 +342,10 @@ func (c *Controller) syncGenerateRequest(key string) error { if !apierrors.IsNotFound(err) { return err } - c.control.Delete(gr.Name) + err = c.control.Delete(gr.Name) + if err != nil { + return err + } return nil } return c.processGR(*gr) diff --git a/pkg/kyverno/apply/apply_command.go b/pkg/kyverno/apply/apply_command.go index c86f96f6fb..3ffa91325c 100644 --- a/pkg/kyverno/apply/apply_command.go +++ b/pkg/kyverno/apply/apply_command.go @@ -8,7 +8,6 @@ import ( "time" "github.com/go-git/go-billy/v5/memfs" - pkgCommon "github.com/kyverno/kyverno/pkg/common" client "github.com/kyverno/kyverno/pkg/dclient" "github.com/kyverno/kyverno/pkg/kyverno/common" sanitizederror "github.com/kyverno/kyverno/pkg/kyverno/sanitizedError" @@ -331,7 +330,6 @@ func printReportOrViolation(policyReport bool, rc *common.ResultCounts, resource } if policyReport { - os.Setenv("POLICY-TYPE", pkgCommon.PolicyReport) resps := buildPolicyReports(pvInfos) if len(resps) > 0 || resourcesLen == 0 { fmt.Println("\n----------------------------------------------------------------------\nPOLICY REPORT:\n----------------------------------------------------------------------") diff --git a/pkg/kyverno/apply/report_test.go b/pkg/kyverno/apply/report_test.go index a3a899463d..ee02d75f8e 100644 --- a/pkg/kyverno/apply/report_test.go +++ b/pkg/kyverno/apply/report_test.go @@ -2,13 +2,11 @@ package apply import ( "encoding/json" - "os" "testing" kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1" preport "github.com/kyverno/kyverno/pkg/api/policyreport/v1alpha2" report "github.com/kyverno/kyverno/pkg/api/policyreport/v1alpha2" - "github.com/kyverno/kyverno/pkg/common" "github.com/kyverno/kyverno/pkg/engine/response" kyvCommon "github.com/kyverno/kyverno/pkg/kyverno/common" "github.com/kyverno/kyverno/pkg/policyreport" @@ -89,7 +87,6 @@ var rawPolicy = []byte(` var rawEngRes = []byte(`{"PatchedResource":{"apiVersion":"v1","kind":"Pod","metadata":{"name":"nginx1","namespace":"default"},"spec":{"containers":[{"image":"nginx","imagePullPolicy":"IfNotPresent","name":"nginx","resources":{"limits":{"cpu":"200m","memory":"100Mi"},"requests":{"cpu":"100m","memory":"50Mi"}}}]}},"PolicyResponse":{"policy":{"name":"pod-requirements","namespace":""},"resource":{"kind":"Pod","apiVersion":"v1","namespace":"default","name":"nginx1","uid":""},"processingTime":974958,"rulesAppliedCount":2,"policyExecutionTimestamp":1630527712,"rules":[{"name":"pods-require-account","type":"Validation","message":"validation error: User pods must include an account for charging. Rule pods-require-account failed at path /metadata/labels/","status":"fail","processingTime":28833,"ruleExecutionTimestamp":1630527712},{"name":"pods-require-limits","type":"Validation","message":"validation rule 'pods-require-limits' passed.","status":"pass","processingTime":578625,"ruleExecutionTimestamp":1630527712}],"ValidationFailureAction":"audit"}}`) func Test_buildPolicyReports(t *testing.T) { - os.Setenv("POLICY-TYPE", common.PolicyReport) rc := &kyvCommon.ResultCounts{} var pvInfos []policyreport.Info var policy kyverno.ClusterPolicy @@ -126,7 +123,6 @@ func Test_buildPolicyReports(t *testing.T) { } func Test_buildPolicyResults(t *testing.T) { - os.Setenv("POLICY-TYPE", common.PolicyReport) rc := &kyvCommon.ResultCounts{} var pvInfos []policyreport.Info var policy kyverno.ClusterPolicy diff --git a/pkg/kyverno/common/common.go b/pkg/kyverno/common/common.go index 4de66c038d..18b54bc555 100644 --- a/pkg/kyverno/common/common.go +++ b/pkg/kyverno/common/common.go @@ -571,7 +571,10 @@ func ApplyPolicyOnResource(policy *v1.ClusterPolicy, resource *unstructured.Unst for key, value := range variables { jsonData := pkgcommon.VariableToJSON(key, value) - ctx.AddJSON(jsonData) + err = ctx.AddJSON(jsonData) + if err != nil { + log.Log.Error(err, "failed to add variable to context") + } } mutateResponse := engine.Mutate(&engine.PolicyContext{Policy: *policy, NewResource: *resource, JSONContext: ctx, NamespaceLabels: namespaceLabels}) diff --git a/pkg/policyreport/changerequestcreator.go b/pkg/policyreport/changerequestcreator.go index 4d380c892c..3e1053f9b2 100644 --- a/pkg/policyreport/changerequestcreator.go +++ b/pkg/policyreport/changerequestcreator.go @@ -52,12 +52,19 @@ func newChangeRequestCreator(client *dclient.Client, tickerInterval time.Duratio func (c *changeRequestCreator) add(request *unstructured.Unstructured) { uid, _ := rand.Int(rand.Reader, big.NewInt(100000)) + var err error switch request.GetKind() { case "ClusterReportChangeRequest": - c.CRCRCache.Add(uid.String(), request, cache.NoExpiration) + err = c.CRCRCache.Add(uid.String(), request, cache.NoExpiration) + if err != nil { + c.log.Error(err, "failed to add ClusterReportChangeRequest to cache") + } case "ReportChangeRequest": - c.RCRCache.Add(uid.String(), request, cache.NoExpiration) + err = c.RCRCache.Add(uid.String(), request, cache.NoExpiration) + if err != nil { + c.log.Error(err, "failed to add ReportChangeRequest to cache") + } default: return } @@ -204,15 +211,15 @@ func merge(dst, src *unstructured.Unstructured) bool { dstResults = append(dstResults, srcResults...) if err := unstructured.SetNestedSlice(dst.UnstructuredContent(), dstResults, "results"); err == nil { - addSummary(dst, src) - return true + err = addSummary(dst, src) + return err == nil } } } return false } -func addSummary(dst, src *unstructured.Unstructured) { +func addSummary(dst, src *unstructured.Unstructured) error { if dstSum, ok, _ := unstructured.NestedMap(dst.UnstructuredContent(), "summary"); ok { if srcSum, ok, _ := unstructured.NestedMap(src.UnstructuredContent(), "summary"); ok { for key, dstVal := range dstSum { @@ -223,8 +230,9 @@ func addSummary(dst, src *unstructured.Unstructured) { } } } - unstructured.SetNestedMap(dst.UnstructuredContent(), dstSum, "summary") + return unstructured.SetNestedMap(dst.UnstructuredContent(), dstSum, "summary") } + return nil } func isDeleteRequest(request *unstructured.Unstructured) bool { diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index c189b8e7f1..c6180f5d30 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -595,7 +595,10 @@ func (ws *WebhookServer) Stop(ctx context.Context) { if err != nil { // Error from closing listeners, or context timeout: logger.Error(err, "shutting down server") - ws.server.Close() + err = ws.server.Close() + if err != nil { + logger.Error(err, "server shut down failed") + } } }