From a727ffca4266de63dcc4ae3c75c48d191ff158cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Wed, 14 Jun 2023 12:06:52 +0200 Subject: [PATCH] refactor: introduce engine image data client interface (#7529) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- .../kubectl-kyverno/utils/common/common.go | 1 + .../utils/common/kyverno_policies_types.go | 4 +- .../utils/store/contextloader.go | 6 +- cmd/internal/engine.go | 1 + pkg/engine/adapters/rclient.go | 59 +++++++++++++++++++ pkg/engine/api/client.go | 14 +++++ pkg/engine/api/context.go | 50 ++++------------ pkg/engine/api/contextloader.go | 12 ++-- pkg/engine/context/evaluate.go | 1 - pkg/engine/engine.go | 5 +- pkg/engine/image_verify_test.go | 2 + pkg/engine/mutation_test.go | 1 + pkg/engine/test/contextloader.go | 7 +-- pkg/engine/validation_test.go | 2 + pkg/webhooks/resource/fake.go | 1 + pkg/webhooks/resource/validation_test.go | 10 +++- 16 files changed, 119 insertions(+), 57 deletions(-) create mode 100644 pkg/engine/adapters/rclient.go diff --git a/cmd/cli/kubectl-kyverno/utils/common/common.go b/cmd/cli/kubectl-kyverno/utils/common/common.go index 544b369722..2784f666f4 100644 --- a/cmd/cli/kubectl-kyverno/utils/common/common.go +++ b/cmd/cli/kubectl-kyverno/utils/common/common.go @@ -857,6 +857,7 @@ func initializeMockController(objects []runtime.Object) (*generate.GenerateContr jmespath.New(cfg), adapters.Client(client), nil, + nil, store.ContextLoaderFactory(nil), nil, "", diff --git a/cmd/cli/kubectl-kyverno/utils/common/kyverno_policies_types.go b/cmd/cli/kubectl-kyverno/utils/common/kyverno_policies_types.go index d5909a22e4..674631569b 100644 --- a/cmd/cli/kubectl-kyverno/utils/common/kyverno_policies_types.go +++ b/cmd/cli/kubectl-kyverno/utils/common/kyverno_policies_types.go @@ -111,12 +111,14 @@ OuterLoop: } } } + rclient := registryclient.NewOrDie() eng := engine.NewEngine( cfg, config.NewDefaultMetricsConfiguration(), jmespath.New(cfg), adapters.Client(c.Client), - registryclient.NewOrDie(), + adapters.ImageDataClient(rclient), + rclient, store.ContextLoaderFactory(nil), nil, "", diff --git a/cmd/cli/kubectl-kyverno/utils/store/contextloader.go b/cmd/cli/kubectl-kyverno/utils/store/contextloader.go index 54b401be3c..49282ededb 100644 --- a/cmd/cli/kubectl-kyverno/utils/store/contextloader.go +++ b/cmd/cli/kubectl-kyverno/utils/store/contextloader.go @@ -5,11 +5,11 @@ import ( "github.com/go-logr/logr" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" + "github.com/kyverno/kyverno/pkg/engine/adapters" engineapi "github.com/kyverno/kyverno/pkg/engine/api" enginecontext "github.com/kyverno/kyverno/pkg/engine/context" "github.com/kyverno/kyverno/pkg/engine/jmespath" "github.com/kyverno/kyverno/pkg/logging" - "github.com/kyverno/kyverno/pkg/registryclient" ) func ContextLoaderFactory( @@ -39,7 +39,7 @@ func (l *mockContextLoader) Load( ctx context.Context, jp jmespath.Interface, client engineapi.RawClient, - _ registryclient.Client, + _ engineapi.ImageDataClient, contextEntries []kyvernov1.ContextEntry, jsonContext enginecontext.Interface, ) error { @@ -57,7 +57,7 @@ func (l *mockContextLoader) Load( for _, entry := range contextEntries { if entry.ImageRegistry != nil && hasRegistryAccess { rclient := GetRegistryClient() - if err := engineapi.LoadImageData(ctx, jp, rclient, l.logger, entry, jsonContext); err != nil { + if err := engineapi.LoadImageData(ctx, jp, adapters.ImageDataClient(rclient), l.logger, entry, jsonContext); err != nil { return err } } else if entry.Variable != nil { diff --git a/cmd/internal/engine.go b/cmd/internal/engine.go index 709338de56..96ff1711db 100644 --- a/cmd/internal/engine.go +++ b/cmd/internal/engine.go @@ -39,6 +39,7 @@ func NewEngine( metricsConfiguration, jp, adapters.Client(client), + adapters.ImageDataClient(rclient), rclient, engineapi.DefaultContextLoaderFactory(configMapResolver), exceptionsSelector, diff --git a/pkg/engine/adapters/rclient.go b/pkg/engine/adapters/rclient.go new file mode 100644 index 0000000000..ea7be288a4 --- /dev/null +++ b/pkg/engine/adapters/rclient.go @@ -0,0 +1,59 @@ +package adapters + +import ( + "context" + "fmt" + + "github.com/google/go-containerregistry/pkg/name" + engineapi "github.com/kyverno/kyverno/pkg/engine/api" + "github.com/kyverno/kyverno/pkg/registryclient" +) + +type rclientAdapter struct { + client registryclient.Client +} + +func ImageDataClient(client registryclient.Client) engineapi.ImageDataClient { + if client == nil { + return nil + } + return &rclientAdapter{client} +} + +func (a *rclientAdapter) ForRef(ctx context.Context, ref string) (*engineapi.ImageData, error) { + desc, err := a.client.FetchImageDescriptor(ctx, ref) + if err != nil { + return nil, fmt.Errorf("failed to fetch image descriptor: %s, error: %v", ref, err) + } + parsedRef, err := name.ParseReference(ref) + if err != nil { + return nil, fmt.Errorf("failed to parse image reference: %s, error: %v", ref, err) + } + if err != nil { + return nil, err + } + image, err := desc.Image() + if err != nil { + return nil, fmt.Errorf("failed to resolve image reference: %s, error: %v", ref, err) + } + // We need to use the raw config and manifest to avoid dropping unknown keys + // which are not defined in GGCR structs. + rawManifest, err := image.RawManifest() + if err != nil { + return nil, fmt.Errorf("failed to fetch manifest for image reference: %s, error: %v", ref, err) + } + rawConfig, err := image.RawConfigFile() + if err != nil { + return nil, fmt.Errorf("failed to fetch config for image reference: %s, error: %v", ref, err) + } + data := engineapi.ImageData{ + Image: ref, + ResolvedImage: fmt.Sprintf("%s@%s", parsedRef.Context().Name(), desc.Digest.String()), + Registry: parsedRef.Context().RegistryStr(), + Repository: parsedRef.Context().RepositoryStr(), + Identifier: parsedRef.Identifier(), + Manifest: rawManifest, + Config: rawConfig, + } + return &data, nil +} diff --git a/pkg/engine/api/client.go b/pkg/engine/api/client.go index 5a23cbcaa8..d508b1f377 100644 --- a/pkg/engine/api/client.go +++ b/pkg/engine/api/client.go @@ -33,3 +33,17 @@ type Client interface { AuthClient ResourceClient } + +type ImageData struct { + Image string + ResolvedImage string + Registry string + Repository string + Identifier string + Manifest []byte + Config []byte +} + +type ImageDataClient interface { + ForRef(ctx context.Context, ref string) (*ImageData, error) +} diff --git a/pkg/engine/api/context.go b/pkg/engine/api/context.go index 2fac43b519..fe75c0e251 100644 --- a/pkg/engine/api/context.go +++ b/pkg/engine/api/context.go @@ -6,13 +6,11 @@ import ( "fmt" "github.com/go-logr/logr" - "github.com/google/go-containerregistry/pkg/name" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/engine/apicall" enginecontext "github.com/kyverno/kyverno/pkg/engine/context" "github.com/kyverno/kyverno/pkg/engine/jmespath" "github.com/kyverno/kyverno/pkg/engine/variables" - "github.com/kyverno/kyverno/pkg/registryclient" ) func LoadVariable(logger logr.Logger, jp jmespath.Interface, entry kyvernov1.ContextEntry, ctx enginecontext.Interface) (err error) { @@ -77,8 +75,8 @@ func LoadVariable(logger logr.Logger, jp jmespath.Interface, entry kyvernov1.Con } } -func LoadImageData(ctx context.Context, jp jmespath.Interface, rclient registryclient.Client, logger logr.Logger, entry kyvernov1.ContextEntry, enginectx enginecontext.Interface) error { - imageData, err := fetchImageData(ctx, jp, rclient, logger, entry, enginectx) +func LoadImageData(ctx context.Context, jp jmespath.Interface, client ImageDataClient, logger logr.Logger, entry kyvernov1.ContextEntry, enginectx enginecontext.Interface) error { + imageData, err := fetchImageData(ctx, jp, client, logger, entry, enginectx) if err != nil { return err } @@ -115,7 +113,7 @@ func LoadConfigMap(ctx context.Context, logger logr.Logger, entry kyvernov1.Cont return nil } -func fetchImageData(ctx context.Context, jp jmespath.Interface, rclient registryclient.Client, logger logr.Logger, entry kyvernov1.ContextEntry, enginectx enginecontext.Interface) (interface{}, error) { +func fetchImageData(ctx context.Context, jp jmespath.Interface, client ImageDataClient, logger logr.Logger, entry kyvernov1.ContextEntry, enginectx enginecontext.Interface) (interface{}, error) { ref, err := variables.SubstituteAll(logger, enginectx, entry.ImageRegistry.Reference) if err != nil { return nil, fmt.Errorf("ailed to substitute variables in context entry %s %s: %v", entry.Name, entry.ImageRegistry.Reference, err) @@ -128,7 +126,7 @@ func fetchImageData(ctx context.Context, jp jmespath.Interface, rclient registry if err != nil { return nil, fmt.Errorf("failed to substitute variables in context entry %s %s: %v", entry.Name, entry.ImageRegistry.JMESPath, err) } - imageData, err := fetchImageDataMap(ctx, rclient, refString) + imageData, err := fetchImageDataMap(ctx, client, refString) if err != nil { return nil, err } @@ -142,47 +140,25 @@ func fetchImageData(ctx context.Context, jp jmespath.Interface, rclient registry } // FetchImageDataMap fetches image information from the remote registry. -func fetchImageDataMap(ctx context.Context, rclient registryclient.Client, ref string) (interface{}, error) { - desc, err := rclient.FetchImageDescriptor(ctx, ref) +func fetchImageDataMap(ctx context.Context, client ImageDataClient, ref string) (interface{}, error) { + desc, err := client.ForRef(ctx, ref) if err != nil { return nil, fmt.Errorf("failed to fetch image descriptor: %s, error: %v", ref, err) } - parsedRef, err := name.ParseReference(ref) - if err != nil { - return nil, fmt.Errorf("failed to parse image reference: %s, error: %v", ref, err) - } - if err != nil { - return nil, err - } - image, err := desc.Image() - if err != nil { - return nil, fmt.Errorf("failed to resolve image reference: %s, error: %v", ref, err) - } - // We need to use the raw config and manifest to avoid dropping unknown keys - // which are not defined in GGCR structs. - rawManifest, err := image.RawManifest() - if err != nil { - return nil, fmt.Errorf("failed to fetch manifest for image reference: %s, error: %v", ref, err) - } var manifest interface{} - if err := json.Unmarshal(rawManifest, &manifest); err != nil { + if err := json.Unmarshal(desc.Manifest, &manifest); err != nil { return nil, fmt.Errorf("failed to decode manifest for image reference: %s, error: %v", ref, err) } - rawConfig, err := image.RawConfigFile() - if err != nil { - return nil, fmt.Errorf("failed to fetch config for image reference: %s, error: %v", ref, err) - } var configData interface{} - if err := json.Unmarshal(rawConfig, &configData); err != nil { + if err := json.Unmarshal(desc.Config, &configData); err != nil { return nil, fmt.Errorf("failed to decode config for image reference: %s, error: %v", ref, err) } - data := map[string]interface{}{ - "image": ref, - "resolvedImage": fmt.Sprintf("%s@%s", parsedRef.Context().Name(), desc.Digest.String()), - "registry": parsedRef.Context().RegistryStr(), - "repository": parsedRef.Context().RepositoryStr(), - "identifier": parsedRef.Identifier(), + "image": desc.Image, + "resolvedImage": desc.ResolvedImage, + "registry": desc.Registry, + "repository": desc.Repository, + "identifier": desc.Identifier, "manifest": manifest, "configData": configData, } diff --git a/pkg/engine/api/contextloader.go b/pkg/engine/api/contextloader.go index be944cc79e..59b6e37536 100644 --- a/pkg/engine/api/contextloader.go +++ b/pkg/engine/api/contextloader.go @@ -9,7 +9,6 @@ import ( enginecontext "github.com/kyverno/kyverno/pkg/engine/context" "github.com/kyverno/kyverno/pkg/engine/jmespath" "github.com/kyverno/kyverno/pkg/logging" - "github.com/kyverno/kyverno/pkg/registryclient" ) // ContextLoaderFactory provides a ContextLoader given a policy context and rule name @@ -21,7 +20,7 @@ type ContextLoader interface { ctx context.Context, jp jmespath.Interface, client RawClient, - rclient registryclient.Client, + imgClient ImageDataClient, contextEntries []kyvernov1.ContextEntry, jsonContext enginecontext.Interface, ) error @@ -47,16 +46,15 @@ func (l *contextLoader) Load( ctx context.Context, jp jmespath.Interface, client RawClient, - rclient registryclient.Client, + imgClient ImageDataClient, contextEntries []kyvernov1.ContextEntry, jsonContext enginecontext.Interface, ) error { for _, entry := range contextEntries { - deferredLoader := l.newDeferredLoader(ctx, jp, client, rclient, entry, jsonContext) + deferredLoader := l.newDeferredLoader(ctx, jp, client, imgClient, entry, jsonContext) if deferredLoader == nil { return fmt.Errorf("invalid context entry %s", entry.Name) } - jsonContext.AddDeferredLoader(entry.Name, deferredLoader) } return nil @@ -66,7 +64,7 @@ func (l *contextLoader) newDeferredLoader( ctx context.Context, jp jmespath.Interface, client RawClient, - rclient registryclient.Client, + imgClient ImageDataClient, entry kyvernov1.ContextEntry, jsonContext enginecontext.Interface, ) enginecontext.DeferredLoader { @@ -86,7 +84,7 @@ func (l *contextLoader) newDeferredLoader( } } else if entry.ImageRegistry != nil { return func() error { - if err := LoadImageData(ctx, jp, rclient, l.logger, entry, jsonContext); err != nil { + if err := LoadImageData(ctx, jp, imgClient, l.logger, entry, jsonContext); err != nil { return err } return nil diff --git a/pkg/engine/context/evaluate.go b/pkg/engine/context/evaluate.go index 6839011bfb..c0106371bf 100644 --- a/pkg/engine/context/evaluate.go +++ b/pkg/engine/context/evaluate.go @@ -52,7 +52,6 @@ func (ctx *context) loadDeferred(query string) error { func (ctx *context) getMatchingLoaders(query string) []DeferredLoader { ctx.deferred.mutex.Lock() defer ctx.deferred.mutex.Unlock() - var matchingLoaders []DeferredLoader for name, deferredLoader := range ctx.deferred.loaders { if strings.Contains(query, name) { diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 8c891d3925..728bd97ad9 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -31,6 +31,7 @@ type engine struct { metricsConfiguration config.MetricsConfiguration jp jmespath.Interface client engineapi.Client + imgClient engineapi.ImageDataClient rclient registryclient.Client contextLoader engineapi.ContextLoaderFactory exceptionSelector engineapi.PolicyExceptionSelector @@ -47,6 +48,7 @@ func NewEngine( metricsConfiguration config.MetricsConfiguration, jp jmespath.Interface, client engineapi.Client, + imgClient engineapi.ImageDataClient, rclient registryclient.Client, contextLoader engineapi.ContextLoaderFactory, exceptionSelector engineapi.PolicyExceptionSelector, @@ -72,6 +74,7 @@ func NewEngine( metricsConfiguration: metricsConfiguration, jp: jp, client: client, + imgClient: imgClient, rclient: rclient, contextLoader: contextLoader, exceptionSelector: exceptionSelector, @@ -176,7 +179,7 @@ func (e *engine) ContextLoader( ctx, e.jp, e.client, - e.rclient, + e.imgClient, contextEntries, jsonContext, ) diff --git a/pkg/engine/image_verify_test.go b/pkg/engine/image_verify_test.go index ff0acc8497..f63d23e7a1 100644 --- a/pkg/engine/image_verify_test.go +++ b/pkg/engine/image_verify_test.go @@ -11,6 +11,7 @@ import ( kyverno "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/config" "github.com/kyverno/kyverno/pkg/cosign" + "github.com/kyverno/kyverno/pkg/engine/adapters" engineapi "github.com/kyverno/kyverno/pkg/engine/api" enginecontext "github.com/kyverno/kyverno/pkg/engine/context" "github.com/kyverno/kyverno/pkg/engine/context/resolvers" @@ -181,6 +182,7 @@ func testVerifyAndPatchImages( metricsCfg, jp, nil, + adapters.ImageDataClient(rclient), rclient, engineapi.DefaultContextLoaderFactory(cmResolver), nil, diff --git a/pkg/engine/mutation_test.go b/pkg/engine/mutation_test.go index 424aa48477..3b929c096a 100644 --- a/pkg/engine/mutation_test.go +++ b/pkg/engine/mutation_test.go @@ -36,6 +36,7 @@ func testMutate( config.NewDefaultMetricsConfiguration(), jp, adapters.Client(client), + adapters.ImageDataClient(rclient), rclient, contextLoader, nil, diff --git a/pkg/engine/test/contextloader.go b/pkg/engine/test/contextloader.go index f2cc3e7470..34bc5c62e3 100644 --- a/pkg/engine/test/contextloader.go +++ b/pkg/engine/test/contextloader.go @@ -9,7 +9,6 @@ import ( enginecontext "github.com/kyverno/kyverno/pkg/engine/context" "github.com/kyverno/kyverno/pkg/engine/jmespath" "github.com/kyverno/kyverno/pkg/logging" - "github.com/kyverno/kyverno/pkg/registryclient" ) type Policy struct { @@ -46,7 +45,7 @@ func (l *mockContextLoader) Load( ctx context.Context, jp jmespath.Interface, client engineapi.RawClient, - rclient registryclient.Client, + imgClient engineapi.ImageDataClient, contextEntries []kyvernov1.ContextEntry, jsonContext enginecontext.Interface, ) error { @@ -63,8 +62,8 @@ func (l *mockContextLoader) Load( } // Context Variable should be loaded after the values loaded from values file for _, entry := range contextEntries { - if entry.ImageRegistry != nil && rclient != nil { - if err := engineapi.LoadImageData(ctx, jp, rclient, l.logger, entry, jsonContext); err != nil { + if entry.ImageRegistry != nil && imgClient != nil { + if err := engineapi.LoadImageData(ctx, jp, imgClient, l.logger, entry, jsonContext); err != nil { return err } } else if entry.Variable != nil { diff --git a/pkg/engine/validation_test.go b/pkg/engine/validation_test.go index cdbdbef3a5..9601b5bb15 100644 --- a/pkg/engine/validation_test.go +++ b/pkg/engine/validation_test.go @@ -11,6 +11,7 @@ import ( kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" urkyverno "github.com/kyverno/kyverno/api/kyverno/v1beta1" "github.com/kyverno/kyverno/pkg/config" + "github.com/kyverno/kyverno/pkg/engine/adapters" engineapi "github.com/kyverno/kyverno/pkg/engine/api" enginetest "github.com/kyverno/kyverno/pkg/engine/test" "github.com/kyverno/kyverno/pkg/registryclient" @@ -36,6 +37,7 @@ func testValidate( config.NewDefaultMetricsConfiguration(), jp, nil, + adapters.ImageDataClient(rclient), rclient, contextLoader, nil, diff --git a/pkg/webhooks/resource/fake.go b/pkg/webhooks/resource/fake.go index cefe0d58d8..436616cbfe 100644 --- a/pkg/webhooks/resource/fake.go +++ b/pkg/webhooks/resource/fake.go @@ -60,6 +60,7 @@ func NewFakeHandlers(ctx context.Context, policyCache policycache.Cache) webhook config.NewDefaultMetricsConfiguration(), jp, adapters.Client(dclient), + adapters.ImageDataClient(rclient), rclient, engineapi.DefaultContextLoaderFactory(configMapResolver), peLister, diff --git a/pkg/webhooks/resource/validation_test.go b/pkg/webhooks/resource/validation_test.go index 2afc117260..a4f35fbc21 100644 --- a/pkg/webhooks/resource/validation_test.go +++ b/pkg/webhooks/resource/validation_test.go @@ -9,6 +9,7 @@ import ( kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/config" "github.com/kyverno/kyverno/pkg/engine" + "github.com/kyverno/kyverno/pkg/engine/adapters" engineapi "github.com/kyverno/kyverno/pkg/engine/api" "github.com/kyverno/kyverno/pkg/engine/jmespath" log "github.com/kyverno/kyverno/pkg/logging" @@ -1051,12 +1052,14 @@ func TestValidate_failure_action_overrides(t *testing.T) { } cfg := config.NewDefaultConfiguration(false) jp := jmespath.New(cfg) + rclient := registryclient.NewOrDie() eng := engine.NewEngine( cfg, config.NewDefaultMetricsConfiguration(), jp, nil, - registryclient.NewOrDie(), + adapters.ImageDataClient(rclient), + rclient, engineapi.DefaultContextLoaderFactory(nil), nil, "", @@ -1152,13 +1155,14 @@ func Test_RuleSelector(t *testing.T) { assert.NilError(t, err) ctx = ctx.WithPolicy(&policy) - + rclient := registryclient.NewOrDie() eng := engine.NewEngine( cfg, config.NewDefaultMetricsConfiguration(), jp, nil, - registryclient.NewOrDie(), + adapters.ImageDataClient(rclient), + rclient, engineapi.DefaultContextLoaderFactory(nil), nil, "",