From aadaec09e17d1e3f63346da871ea25af15c60376 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?=
<charles.edouard@nirmata.com>
Date: Wed, 22 Mar 2023 05:46:35 +0100
Subject: [PATCH] fix: remove a couple DeepEqual and fix deletion check bug
(#6640)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
---
pkg/auth/auth.go | 3 +--
pkg/background/common/context.go | 3 +--
pkg/engine/imageVerifyValidate.go | 5 ++---
pkg/engine/internal/imageverifier.go | 6 ++----
pkg/engine/internal/logging.go | 4 +---
pkg/engine/internal/response.go | 6 ++----
pkg/engine/mutation.go | 5 ++---
pkg/engine/validation.go | 5 +----
pkg/webhooks/resource/generation/handler.go | 4 +---
pkg/webhooks/resource/imageverification/handler.go | 7 ++-----
pkg/webhooks/resource/mutation/mutation.go | 8 ++------
pkg/webhooks/resource/validation/validation.go | 7 ++-----
12 files changed, 19 insertions(+), 44 deletions(-)
diff --git a/pkg/auth/auth.go b/pkg/auth/auth.go
index d9ca250bb2..1fb319a4be 100644
--- a/pkg/auth/auth.go
+++ b/pkg/auth/auth.go
@@ -3,7 +3,6 @@ package auth
import (
"context"
"fmt"
- "reflect"
kubeutils "github.com/kyverno/kyverno/pkg/utils/kube"
authorizationv1 "k8s.io/api/authorization/v1"
@@ -68,7 +67,7 @@ func (o *canIOptions) RunAccessCheck(ctx context.Context) (bool, error) {
return false, fmt.Errorf("failed to get GVR for kind %s", o.kind)
}
- if reflect.DeepEqual(gvr, schema.GroupVersionResource{}) {
+ if gvr.Empty() {
// cannot find GVR
return false, fmt.Errorf("failed to get the Group Version Resource for kind %s", o.kind)
}
diff --git a/pkg/background/common/context.go b/pkg/background/common/context.go
index fac5bbd031..a0bce3092d 100644
--- a/pkg/background/common/context.go
+++ b/pkg/background/common/context.go
@@ -2,7 +2,6 @@ package common
import (
"fmt"
- "reflect"
"github.com/go-logr/logr"
kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
@@ -36,7 +35,7 @@ func NewBackgroundContext(dclient dclient.Interface, ur *kyvernov1beta1.UpdateRe
return nil, fmt.Errorf("failed to load request in context: %w", err)
}
- if !reflect.DeepEqual(new, unstructured.Unstructured{}) {
+ if new.Object != nil {
if !check(&new, trigger) {
err := fmt.Errorf("resources don't match")
return nil, fmt.Errorf("resource %v: %w", ur.Spec.GetResource().String(), err)
diff --git a/pkg/engine/imageVerifyValidate.go b/pkg/engine/imageVerifyValidate.go
index f917436929..1da65d5e79 100644
--- a/pkg/engine/imageVerifyValidate.go
+++ b/pkg/engine/imageVerifyValidate.go
@@ -3,7 +3,6 @@ package engine
import (
"context"
"fmt"
- "reflect"
"github.com/go-logr/logr"
gojmespath "github.com/jmespath/go-jmespath"
@@ -86,7 +85,7 @@ func validateImage(ctx engineapi.PolicyContext, imageVerify *kyvernov1.ImageVeri
return fmt.Errorf("missing digest for %s", image)
}
newResource := ctx.NewResource()
- if imageVerify.Required && !reflect.DeepEqual(newResource, unstructured.Unstructured{}) {
+ if imageVerify.Required && newResource.Object != nil {
verified, err := isImageVerified(newResource, image, log)
if err != nil {
return err
@@ -99,7 +98,7 @@ func validateImage(ctx engineapi.PolicyContext, imageVerify *kyvernov1.ImageVeri
}
func isImageVerified(resource unstructured.Unstructured, image string, log logr.Logger) (bool, error) {
- if reflect.DeepEqual(resource, unstructured.Unstructured{}) {
+ if resource.Object == nil {
return false, fmt.Errorf("nil resource")
}
if annotations := resource.GetAnnotations(); len(annotations) == 0 {
diff --git a/pkg/engine/internal/imageverifier.go b/pkg/engine/internal/imageverifier.go
index 8c7420f74a..3a3697a8a3 100644
--- a/pkg/engine/internal/imageverifier.go
+++ b/pkg/engine/internal/imageverifier.go
@@ -6,7 +6,6 @@ import (
"errors"
"fmt"
"net"
- "reflect"
"strings"
"github.com/go-logr/logr"
@@ -53,8 +52,7 @@ func NewImageVerifier(
func HasImageVerifiedAnnotationChanged(ctx engineapi.PolicyContext, log logr.Logger) bool {
newResource := ctx.NewResource()
oldResource := ctx.OldResource()
- if reflect.DeepEqual(newResource, unstructured.Unstructured{}) ||
- reflect.DeepEqual(oldResource, unstructured.Unstructured{}) {
+ if newResource.Object == nil || oldResource.Object == nil {
return false
}
newValue := newResource.GetAnnotations()[engineapi.ImageVerifyAnnotationKey]
@@ -76,7 +74,7 @@ func matchImageReferences(imageReferences []string, image string) bool {
}
func isImageVerified(resource unstructured.Unstructured, image string, log logr.Logger) (bool, error) {
- if reflect.DeepEqual(resource, unstructured.Unstructured{}) {
+ if resource.Object == nil {
return false, fmt.Errorf("nil resource")
}
annotations := resource.GetAnnotations()
diff --git a/pkg/engine/internal/logging.go b/pkg/engine/internal/logging.go
index 0d502cd8c5..f7e8db4dd4 100644
--- a/pkg/engine/internal/logging.go
+++ b/pkg/engine/internal/logging.go
@@ -1,8 +1,6 @@
package internal
import (
- "reflect"
-
"github.com/go-logr/logr"
kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
engineapi "github.com/kyverno/kyverno/pkg/engine/api"
@@ -25,7 +23,7 @@ func LoggerWithPolicy(logger logr.Logger, policy kyvernov1.PolicyInterface) logr
}
func LoggerWithResource(logger logr.Logger, prefix string, resource unstructured.Unstructured) logr.Logger {
- if reflect.DeepEqual(resource, unstructured.Unstructured{}) {
+ if resource.Object == nil {
return logger
}
return logger.WithValues(
diff --git a/pkg/engine/internal/response.go b/pkg/engine/internal/response.go
index e651b95014..7c9a9d8828 100644
--- a/pkg/engine/internal/response.go
+++ b/pkg/engine/internal/response.go
@@ -2,12 +2,10 @@ package internal
import (
"fmt"
- "reflect"
"time"
kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
engineapi "github.com/kyverno/kyverno/pkg/engine/api"
- "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)
func RuleError(rule *kyvernov1.Rule, ruleType engineapi.RuleType, msg string, err error) *engineapi.RuleResponse {
@@ -44,10 +42,10 @@ func AddRuleResponse(resp *engineapi.PolicyResponse, ruleResp *engineapi.RuleRes
}
func BuildResponse(ctx engineapi.PolicyContext, resp *engineapi.EngineResponse, startTime time.Time) *engineapi.EngineResponse {
- if reflect.DeepEqual(resp.PatchedResource, unstructured.Unstructured{}) {
+ if resp.PatchedResource.Object == nil {
// for delete requests patched resource will be oldResource since newResource is empty
resource := ctx.NewResource()
- if reflect.DeepEqual(resource, unstructured.Unstructured{}) {
+ if resource.Object == nil {
resource = ctx.OldResource()
}
resp.PatchedResource = resource
diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go
index 02eb9a2618..b67b1213f1 100644
--- a/pkg/engine/mutation.go
+++ b/pkg/engine/mutation.go
@@ -3,7 +3,6 @@ package engine
import (
"context"
"fmt"
- "reflect"
"time"
"github.com/go-logr/logr"
@@ -108,7 +107,7 @@ func (e *engine) mutate(
}
for _, patchedResource := range patchedResources {
- if reflect.DeepEqual(patchedResource, unstructured.Unstructured{}) {
+ if patchedResource.unstructured.Object == nil {
continue
}
@@ -327,7 +326,7 @@ func buildRuleResponse(rule *kyvernov1.Rule, mutateResp *mutate.Response, info r
}
func buildSuccessMessage(r unstructured.Unstructured) string {
- if reflect.DeepEqual(unstructured.Unstructured{}, r) {
+ if r.Object == nil {
return "mutated resource"
}
diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go
index 0f95867f78..1f32025b69 100644
--- a/pkg/engine/validation.go
+++ b/pkg/engine/validation.go
@@ -4,7 +4,6 @@ import (
"context"
"encoding/json"
"fmt"
- "reflect"
"strings"
"time"
@@ -498,11 +497,9 @@ func isEmptyUnstructured(u *unstructured.Unstructured) bool {
if u == nil {
return true
}
-
- if reflect.DeepEqual(*u, unstructured.Unstructured{}) {
+ if u.Object == nil {
return true
}
-
return false
}
diff --git a/pkg/webhooks/resource/generation/handler.go b/pkg/webhooks/resource/generation/handler.go
index 69ac7582c7..64ea170404 100644
--- a/pkg/webhooks/resource/generation/handler.go
+++ b/pkg/webhooks/resource/generation/handler.go
@@ -3,7 +3,6 @@ package generation
import (
"context"
"fmt"
- "reflect"
"github.com/go-logr/logr"
kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
@@ -22,7 +21,6 @@ import (
webhookgenerate "github.com/kyverno/kyverno/pkg/webhooks/updaterequest"
webhookutils "github.com/kyverno/kyverno/pkg/webhooks/utils"
admissionv1 "k8s.io/api/admission/v1"
- "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
corev1listers "k8s.io/client-go/listers/core/v1"
)
@@ -249,7 +247,7 @@ func (h *generationHandler) createUR(ctx context.Context, policyContext *engine.
managedBy := oldLabels[kyvernov1.LabelAppManagedBy] == kyvernov1.ValueKyvernoApp
deleteDownstream := false
- if reflect.DeepEqual(new, unstructured.Unstructured{}) {
+ if new.Object == nil {
labels = oldLabels
if !managedBy {
deleteDownstream = true
diff --git a/pkg/webhooks/resource/imageverification/handler.go b/pkg/webhooks/resource/imageverification/handler.go
index 69b3e1fbed..43f4a99b3d 100644
--- a/pkg/webhooks/resource/imageverification/handler.go
+++ b/pkg/webhooks/resource/imageverification/handler.go
@@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
- "reflect"
"github.com/go-logr/logr"
kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
@@ -135,11 +134,9 @@ func hasAnnotations(context *engine.PolicyContext) bool {
func isResourceDeleted(policyContext *engine.PolicyContext) bool {
var deletionTimeStamp *metav1.Time
- if reflect.DeepEqual(policyContext.NewResource, unstructured.Unstructured{}) {
- resource := policyContext.NewResource()
+ if resource := policyContext.NewResource(); resource.Object != nil {
deletionTimeStamp = resource.GetDeletionTimestamp()
- } else {
- resource := policyContext.OldResource()
+ } else if resource := policyContext.OldResource(); resource.Object != nil {
deletionTimeStamp = resource.GetDeletionTimestamp()
}
return deletionTimeStamp != nil
diff --git a/pkg/webhooks/resource/mutation/mutation.go b/pkg/webhooks/resource/mutation/mutation.go
index 58830243f9..c017881ef2 100644
--- a/pkg/webhooks/resource/mutation/mutation.go
+++ b/pkg/webhooks/resource/mutation/mutation.go
@@ -3,7 +3,6 @@ package mutation
import (
"context"
"fmt"
- "reflect"
"time"
"github.com/go-logr/logr"
@@ -21,7 +20,6 @@ import (
"go.opentelemetry.io/otel/trace"
admissionv1 "k8s.io/api/admission/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
- "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
corev1listers "k8s.io/client-go/listers/core/v1"
)
@@ -186,11 +184,9 @@ func logMutationResponse(patches [][]byte, engineResponses []*engineapi.EngineRe
func isResourceDeleted(policyContext *engine.PolicyContext) bool {
var deletionTimeStamp *metav1.Time
- if reflect.DeepEqual(policyContext.NewResource, unstructured.Unstructured{}) {
- resource := policyContext.NewResource()
+ if resource := policyContext.NewResource(); resource.Object != nil {
deletionTimeStamp = resource.GetDeletionTimestamp()
- } else {
- resource := policyContext.OldResource()
+ } else if resource := policyContext.OldResource(); resource.Object != nil {
deletionTimeStamp = resource.GetDeletionTimestamp()
}
return deletionTimeStamp != nil
diff --git a/pkg/webhooks/resource/validation/validation.go b/pkg/webhooks/resource/validation/validation.go
index aad5f5b5ce..3d44320c05 100644
--- a/pkg/webhooks/resource/validation/validation.go
+++ b/pkg/webhooks/resource/validation/validation.go
@@ -3,7 +3,6 @@ package validation
import (
"context"
"fmt"
- "reflect"
"time"
"github.com/go-logr/logr"
@@ -80,11 +79,9 @@ func (v *validationHandler) HandleValidation(
logger := v.log.WithValues("action", "validate", "resource", resourceName, "operation", request.Operation, "gvk", request.Kind)
var deletionTimeStamp *metav1.Time
- if reflect.DeepEqual(policyContext.NewResource(), unstructured.Unstructured{}) {
- resource := policyContext.NewResource()
+ if resource := policyContext.NewResource(); resource.Object != nil {
deletionTimeStamp = resource.GetDeletionTimestamp()
- } else {
- resource := policyContext.OldResource()
+ } else if resource := policyContext.OldResource(); resource.Object != nil {
deletionTimeStamp = resource.GetDeletionTimestamp()
}