From 30567be7829eb3cc8a4acee730f4ae42077b7f9a Mon Sep 17 00:00:00 2001 From: Jim Bugwadia Date: Tue, 20 Jul 2021 09:36:12 -0700 Subject: [PATCH] check for changes Signed-off-by: Jim Bugwadia --- pkg/engine/context/context.go | 14 +++++- pkg/engine/context/evaluate.go | 30 ++++++++++++ pkg/engine/context/evaluate_test.go | 66 +++++++++++++++++++++++++++ pkg/engine/context/imageutils.go | 18 ++++---- pkg/engine/context/imageutils_test.go | 12 ++--- pkg/engine/imageVerify.go | 19 +++++--- pkg/engine/utils/utils.go | 28 ++++++++++++ pkg/engine/utils/utils_test.go | 8 ++++ 8 files changed, 173 insertions(+), 22 deletions(-) create mode 100644 pkg/engine/context/evaluate_test.go diff --git a/pkg/engine/context/context.go b/pkg/engine/context/context.go index 45a260e968..4dbdd34544 100644 --- a/pkg/engine/context/context.go +++ b/pkg/engine/context/context.go @@ -17,6 +17,9 @@ import ( //Interface to manage context operations type Interface interface { + // AddRequest marshals and adds the admission request to the context + AddRequest(request *v1beta1.AdmissionRequest) error + // AddJSON merges the json with context AddJSON(dataRaw []byte) error @@ -35,9 +38,17 @@ type Interface interface { EvalInterface } -//EvalInterface ... to evaluate +//EvalInterface is used to query and inspect context data type EvalInterface interface { + + // Query accepts a JMESPath expression and returns matching data Query(query string) (interface{}, error) + + // HasChanged accepts a JMESPath expression and compares matching data in the + // request.object and request.oldObject context fields. If the data has changed + // it return `true`. If the data has not changed it returns false. If either + // request.object or request.oldObject are not found, an error is returned. + HasChanged(jmespath string) (bool, error) } //Context stores the data resources as JSON @@ -100,6 +111,7 @@ func (ctx *Context) AddRequest(request *v1beta1.AdmissionRequest) error { ctx.log.Error(err, "failed to marshal the request") return err } + return ctx.AddJSON(objRaw) } diff --git a/pkg/engine/context/evaluate.go b/pkg/engine/context/evaluate.go index b497bc9eab..7dcfc84540 100644 --- a/pkg/engine/context/evaluate.go +++ b/pkg/engine/context/evaluate.go @@ -3,6 +3,8 @@ package context import ( "encoding/json" "fmt" + "github.com/pkg/errors" + "reflect" "strings" jmespath "github.com/kyverno/kyverno/pkg/engine/jmespath" @@ -63,3 +65,31 @@ func (ctx *Context) isBuiltInVariable(variable string) bool { } return false } + + +func (ctx *Context) HasChanged(jmespath string) (bool, error) { + objData, err := ctx.Query("request.object." + jmespath) + if err != nil { + return false, errors.Wrap(err,"failed to query request.object") + } + + if objData == nil { + return false, fmt.Errorf("request.object.%s not found", jmespath) + } + + oldObjData, err := ctx.Query("request.oldObject." + jmespath) + if err != nil { + return false, errors.Wrap(err,"failed to query request.object") + } + + if oldObjData == nil { + return false, fmt.Errorf("request.oldObject.%s not found", jmespath) + } + + if reflect.DeepEqual(objData, oldObjData) { + return false, nil + } + + return true, nil +} + diff --git a/pkg/engine/context/evaluate_test.go b/pkg/engine/context/evaluate_test.go new file mode 100644 index 0000000000..1892895cf9 --- /dev/null +++ b/pkg/engine/context/evaluate_test.go @@ -0,0 +1,66 @@ +package context + +import ( + "github.com/stretchr/testify/assert" + "k8s.io/api/admission/v1beta1" + "testing" +) + +func TestHasChanged(t *testing.T) { + ctx := createTestContext(`{"a": {"b": 1, "c": 2}, "d": 3}`, `{"a": {"b": 2, "c": 2}, "d": 4}`) + + val, err := ctx.HasChanged("a.b") + assert.NoError(t, err) + assert.True(t, val) + + val, err = ctx.HasChanged("a.c") + assert.NoError(t, err) + assert.False(t, val) + + val, err = ctx.HasChanged("d") + assert.NoError(t, err) + assert.True(t, val) + + val, err = ctx.HasChanged("a.x.y") + assert.Error(t, err) +} + +func TestRequestNotInitialize(t *testing.T) { + request := &v1beta1.AdmissionRequest{} + ctx := NewContext() + ctx.AddRequest(request) + + _, err := ctx.HasChanged("x.y.z") + assert.Error(t, err) +} + +func TestMissingOldObject(t *testing.T) { + request := &v1beta1.AdmissionRequest{} + ctx := NewContext() + ctx.AddRequest(request) + request.Object.Raw = []byte(`{"a": {"b": 1, "c": 2}, "d": 3}`) + + _, err := ctx.HasChanged("a.b") + assert.Error(t, err) +} + +func TestMissingObject(t *testing.T) { + request := &v1beta1.AdmissionRequest{} + ctx := NewContext() + ctx.AddRequest(request) + request.OldObject.Raw = []byte(`{"a": {"b": 1, "c": 2}, "d": 3}`) + + _, err := ctx.HasChanged("a.b") + assert.Error(t, err) +} + +func createTestContext(obj, oldObj string) *Context { + request := &v1beta1.AdmissionRequest{} + request.Operation = "UPDATE" + request.Object.Raw = []byte(obj) + request.OldObject.Raw = []byte(oldObj) + + ctx := NewContext() + ctx.AddRequest(request) + return ctx +} diff --git a/pkg/engine/context/imageutils.go b/pkg/engine/context/imageutils.go index 2dee89469d..6258b39732 100644 --- a/pkg/engine/context/imageutils.go +++ b/pkg/engine/context/imageutils.go @@ -27,8 +27,8 @@ type ImageInfo struct { // Digest is the image digest portion e.g. `sha256:128c6e3534b842a2eec139999b8ce8aa9a2af9907e2b9269550809d18cd832a3` Digest string `json:"digest,omitempty"` - // JSONPath is full JSON path to this image e.g. `/spec/containers/0/image` - JSONPath string `json:"jsonPath,omitempty"` + // JSONPointer is full JSON path to this image e.g. `/spec/containers/0/image` + JSONPointer string `json:"jsonPath,omitempty"` } func (i *ImageInfo) String() string { @@ -144,7 +144,7 @@ func convertToImageInfo(containers []interface{}, jsonPath string) (images []*Co return images, errors.Errorf("%s", strings.Join(errs, ";")) } -func newImageInfo(image, jsonPath string) (*ImageInfo, error) { +func newImageInfo(image, jsonPointer string) (*ImageInfo, error) { image = addDefaultDomain(image) ref, err := reference.Parse(image) if err != nil { @@ -172,12 +172,12 @@ func newImageInfo(image, jsonPath string) (*ImageInfo, error) { } return &ImageInfo{ - Registry: registry, - Name: name, - Path: path, - Tag: tag, - Digest: digest, - JSONPath: jsonPath, + Registry: registry, + Name: name, + Path: path, + Tag: tag, + Digest: digest, + JSONPointer: jsonPointer, }, nil } diff --git a/pkg/engine/context/imageutils_test.go b/pkg/engine/context/imageutils_test.go index 865c6b13f5..33ecc089f3 100644 --- a/pkg/engine/context/imageutils_test.go +++ b/pkg/engine/context/imageutils_test.go @@ -16,21 +16,21 @@ func Test_extractImageInfo(t *testing.T) { }{ { raw: []byte(`{"apiVersion": "v1","kind": "Pod","metadata": {"name": "myapp"},"spec": {"initContainers": [{"name": "init","image": "index.docker.io/busybox:v1.2.3"}],"containers": [{"name": "nginx","image": "nginx:latest"}]}}`), - initContainers: []*ContainerImage{{Name: "init", Image: &ImageInfo{Registry: "index.docker.io", Name: "busybox", Path: "busybox", Tag: "v1.2.3", JSONPath: "/spec/initContainers/0/image"}}}, - containers: []*ContainerImage{{Name: "nginx", Image: &ImageInfo{Registry: "docker.io", Name: "nginx", Path: "nginx", Tag: "latest", JSONPath: "/spec/containers/0/image"}}}, + initContainers: []*ContainerImage{{Name: "init", Image: &ImageInfo{Registry: "index.docker.io", Name: "busybox", Path: "busybox", Tag: "v1.2.3", JSONPointer: "/spec/initContainers/0/image"}}}, + containers: []*ContainerImage{{Name: "nginx", Image: &ImageInfo{Registry: "docker.io", Name: "nginx", Path: "nginx", Tag: "latest", JSONPointer: "/spec/containers/0/image"}}}, }, { raw: []byte(`{"apiVersion": "v1","kind": "Pod","metadata": {"name": "myapp"},"spec": {"containers": [{"name": "nginx","image": "test/nginx:latest"}]}}`), initContainers: []*ContainerImage{}, - containers: []*ContainerImage{{Name: "nginx", Image: &ImageInfo{Registry: "docker.io", Name: "nginx", Path: "test/nginx", Tag: "latest", JSONPath: "/spec/containers/0/image"}}}, + containers: []*ContainerImage{{Name: "nginx", Image: &ImageInfo{Registry: "docker.io", Name: "nginx", Path: "test/nginx", Tag: "latest", JSONPointer: "/spec/containers/0/image"}}}, }, { raw: []byte(`{"apiVersion": "apps/v1","kind": "Deployment","metadata": {"name": "myapp"},"spec": {"selector": {"matchLabels": {"app": "myapp"}},"template": {"metadata": {"labels": {"app": "myapp"}},"spec": {"initContainers": [{"name": "init","image": "fictional.registry.example:10443/imagename:tag@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"}],"containers": [{"name": "myapp","image": "fictional.registry.example:10443/imagename"}]}}}}`), - initContainers: []*ContainerImage{{Name: "init", Image: &ImageInfo{Registry: "fictional.registry.example:10443", Name: "imagename", Path: "imagename", Tag: "tag", JSONPath: "/spec/template/spec/initContainers/0/image", Digest: "sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"}}}, - containers: []*ContainerImage{{Name: "myapp", Image: &ImageInfo{Registry: "fictional.registry.example:10443", Name: "imagename", Path: "imagename", Tag: "latest", JSONPath: "/spec/template/spec/containers/0/image"}}}}, + initContainers: []*ContainerImage{{Name: "init", Image: &ImageInfo{Registry: "fictional.registry.example:10443", Name: "imagename", Path: "imagename", Tag: "tag", JSONPointer: "/spec/template/spec/initContainers/0/image", Digest: "sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"}}}, + containers: []*ContainerImage{{Name: "myapp", Image: &ImageInfo{Registry: "fictional.registry.example:10443", Name: "imagename", Path: "imagename", Tag: "latest", JSONPointer: "/spec/template/spec/containers/0/image"}}}}, { raw: []byte(`{"apiVersion": "batch/v1beta1","kind": "CronJob","metadata": {"name": "hello"},"spec": {"schedule": "*/1 * * * *","jobTemplate": {"spec": {"template": {"spec": {"containers": [{"name": "hello","image": "test.example.com/test/my-app:v2"}]}}}}}}`), - containers: []*ContainerImage{{Name: "hello", Image: &ImageInfo{Registry: "test.example.com", Name: "my-app", Path: "test/my-app", Tag: "v2", JSONPath: "/spec/jobTemplate/spec/template/spec/containers/0/image"}}}, + containers: []*ContainerImage{{Name: "hello", Image: &ImageInfo{Registry: "test.example.com", Name: "my-app", Path: "test/my-app", Tag: "v2", JSONPointer: "/spec/jobTemplate/spec/template/spec/containers/0/image"}}}, }, } diff --git a/pkg/engine/imageVerify.go b/pkg/engine/imageVerify.go index 0645aa5a1a..6ecb1e4b6b 100644 --- a/pkg/engine/imageVerify.go +++ b/pkg/engine/imageVerify.go @@ -27,7 +27,7 @@ func VerifyAndPatchImages(policyContext *PolicyContext) (resp *response.EngineRe "kind", patchedResource.GetKind(), "namespace", patchedResource.GetNamespace(), "name", patchedResource.GetName()) if ManagedPodResource(policy, patchedResource) { - logger.V(4).Info("container images for pods managed by workload controllers are already verified", "policy", policy.GetName()) + logger.V(4).Info("images for resources managed by workload controllers are already verified", "policy", policy.GetName()) resp.PatchedResource = patchedResource return } @@ -53,20 +53,27 @@ func VerifyAndPatchImages(policyContext *PolicyContext) (resp *response.EngineRe policyContext.JSONContext.Restore() for _, imageVerify := range rule.VerifyImages { - verifyAndPatchImages(logger, &rule, imageVerify, images.Containers, resp) - verifyAndPatchImages(logger, &rule, imageVerify, images.InitContainers, resp) + verifyAndPatchImages(logger, policyContext, &rule, imageVerify, images.Containers, resp) + verifyAndPatchImages(logger, policyContext, &rule, imageVerify, images.InitContainers, resp) } } return } -func verifyAndPatchImages(logger logr.Logger, rule *v1.Rule, imageVerify *v1.ImageVerification, images map[string]*context.ImageInfo, resp *response.EngineResponse) { +func verifyAndPatchImages(logger logr.Logger, policyContext *PolicyContext, rule *v1.Rule, imageVerify *v1.ImageVerification, images map[string]*context.ImageInfo, resp *response.EngineResponse) { imagePattern := imageVerify.Image key := imageVerify.Key for _, imageInfo := range images { image := imageInfo.String() + jmespath := utils.JsonPointerToJMESPath(imageInfo.JSONPointer) + changed, err := policyContext.JSONContext.HasChanged(jmespath) + if err == nil && !changed { + logger.V(4).Info("no change in image, skipping check", "image", image) + continue + } + if !wildcard.Match(imagePattern, image) { logger.V(4).Info("image does not match pattern", "image", image, "pattern", imagePattern) continue @@ -95,7 +102,7 @@ func verifyAndPatchImages(logger logr.Logger, rule *v1.Rule, imageVerify *v1.Ima if imageInfo.Digest == "" { patch, err := makeAddDigestPatch(imageInfo, digest) if err != nil { - logger.Error(err, "failed to patch image with digest", "image", imageInfo.String(), "jsonPath", imageInfo.JSONPath) + logger.Error(err, "failed to patch image with digest", "image", imageInfo.String(), "jsonPath", imageInfo.JSONPointer) } else { logger.V(4).Info("patching verified image with digest", "patch", string(patch)) ruleResp.Patches = [][]byte{patch} @@ -110,7 +117,7 @@ func verifyAndPatchImages(logger logr.Logger, rule *v1.Rule, imageVerify *v1.Ima func makeAddDigestPatch(imageInfo *context.ImageInfo, digest string) ([]byte, error) { var patch = make(map[string]interface{}) patch["op"] = "replace" - patch["path"] = imageInfo.JSONPath + patch["path"] = imageInfo.JSONPointer patch["value"] = imageInfo.String() + "@" + digest return json.Marshal(patch) } diff --git a/pkg/engine/utils/utils.go b/pkg/engine/utils/utils.go index 3ba19ab4cf..6d2b0f82f6 100644 --- a/pkg/engine/utils/utils.go +++ b/pkg/engine/utils/utils.go @@ -1,10 +1,13 @@ package utils import ( + "fmt" jsonpatch "github.com/evanphx/json-patch/v5" commonAnchor "github.com/kyverno/kyverno/pkg/engine/anchor/common" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/controller-runtime/pkg/log" + "strconv" + "strings" ) //RuleType defines the type for rule @@ -107,3 +110,28 @@ func GetAnchorsFromMap(anchorsMap map[string]interface{}) map[string]interface{} return result } + +func JsonPointerToJMESPath(jsonPointer string) string { + var sb strings.Builder + tokens := strings.Split(jsonPointer, "/") + i := 0 + for _, t := range tokens { + if t == ""{ + continue + } + + if _, err := strconv.Atoi(t); err == nil { + sb.WriteString(fmt.Sprintf("[%s]", t)) + continue + } + + if i > 0 { + sb.WriteString(".") + } + + sb.WriteString(t) + i++ + } + + return sb.String() +} \ No newline at end of file diff --git a/pkg/engine/utils/utils_test.go b/pkg/engine/utils/utils_test.go index 1732283503..01e7982fd6 100644 --- a/pkg/engine/utils/utils_test.go +++ b/pkg/engine/utils/utils_test.go @@ -27,3 +27,11 @@ func TestGetAnchorsFromMap_ThereAreNoAnchors(t *testing.T) { actualMap := GetAnchorsFromMap(unmarshalled) assert.Equal(t, len(actualMap), 0) } + +func Test_JsonPointerToJMESPath(t *testing.T) { + assert.Equal(t, "a.b.c[1].d", JsonPointerToJMESPath("a/b/c/1//d"), ) + assert.Equal(t, "a.b.c[1].d", JsonPointerToJMESPath("/a/b/c/1/d"), ) + assert.Equal(t, "a.b.c[1].d", JsonPointerToJMESPath("/a/b/c/1/d/"), ) + assert.Equal(t, "a[1].b.c[1].d", JsonPointerToJMESPath("a/1/b/c/1/d"), ) + assert.Equal(t, "a[1].b.c[1].d[2]", JsonPointerToJMESPath("/a/1/b/c/1/d/2/"), ) +} \ No newline at end of file