From eb6f6c485229711b729c10a9a342666bbdda755f Mon Sep 17 00:00:00 2001 From: Jakub Warczarek Date: Mon, 12 Aug 2024 13:17:21 +0200 Subject: [PATCH] chore: change format of image for KongPluginInstallation (#474) --- CHANGELOG.md | 2 +- Dockerfile | 4 +- .../kongplugininstallation/controller.go | 10 +-- .../kongplugininstallation/image/image.go | 82 ++++++++++++++----- .../image/image_test.go | 70 ++++++++++++---- .../test_kongplugininstallation.go | 26 ++++-- 6 files changed, 135 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6790f08f..6494490c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,7 @@ [#387](https://github.com/Kong/gateway-operator/pull/387) - Introduce `KongPluginInstallation` CRD to allow installing custom Kong plugins distributed as container images. - [#400](https://github.com/Kong/gateway-operator/pull/400), [#424](https://github.com/Kong/gateway-operator/pull/424) + [#400](https://github.com/Kong/gateway-operator/pull/400), [#424](https://github.com/Kong/gateway-operator/pull/424), [#474](https://github.com/Kong/gateway-operator/pull/474) - Extended `DataPlane` API with a possibility to specify `PodDisruptionBudget` to be created for the `DataPlane` deployments via `spec.resources.podDisruptionBudget`. [#464](https://github.com/Kong/gateway-operator/pull/464) diff --git a/Dockerfile b/Dockerfile index 09510e772..d425cede1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,7 @@ # Builder # ------------------------------------------------------------------------------ -FROM --platform=$BUILDPLATFORM golang:1.22.6 as builder +FROM --platform=$BUILDPLATFORM golang:1.22.6 AS builder WORKDIR /workspace ARG GOPATH @@ -55,7 +55,7 @@ RUN --mount=type=cache,target=$GOPATH/pkg/mod \ # Use distroless as minimal base image to package the operator binary # Refer to https://github.com/GoogleContainerTools/distroless for more details -FROM gcr.io/distroless/static:nonroot as distroless +FROM gcr.io/distroless/static:nonroot AS distroless ARG TAG ARG NAME="Kong Gateway Operator" diff --git a/controller/kongplugininstallation/controller.go b/controller/kongplugininstallation/controller.go index d2d280913..8ba5dad65 100644 --- a/controller/kongplugininstallation/controller.go +++ b/controller/kongplugininstallation/controller.go @@ -112,7 +112,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } log.Trace(logger, "fetch plugin for KongPluginInstallation resource", kpi) - plugin, err := image.FetchPluginContent(ctx, kpi.Spec.Image, credentialsStore) + plugin, err := image.FetchPlugin(ctx, kpi.Spec.Image, credentialsStore) if err != nil { return ctrl.Result{}, setStatusConditionFailedForKongPluginInstallation(ctx, r.Client, &kpi, fmt.Sprintf("problem with the image: %q error: %s", kpi.Spec.Image, err)) } @@ -130,9 +130,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu cm.GenerateName = kpi.Name } cm.Namespace = kpi.Namespace - cm.Data = map[string]string{ - fmt.Sprintf("%s.lua", kpi.Name): string(plugin), - } + cm.Data = plugin if err := ctrl.SetControllerReference(&kpi, &cm, r.Scheme); err != nil { return ctrl.Result{}, err } @@ -142,9 +140,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu kpi.Status.UnderlyingConfigMapName = cm.Name case 1: cm = cms[0] - cm.Data = map[string]string{ - fmt.Sprintf("%s.lua", kpi.Name): string(plugin), - } + cm.Data = plugin if err := r.Client.Update(ctx, &cm); err != nil { return ctrl.Result{}, err } diff --git a/controller/kongplugininstallation/image/image.go b/controller/kongplugininstallation/image/image.go index cfad77da6..3372c1c9d 100644 --- a/controller/kongplugininstallation/image/image.go +++ b/controller/kongplugininstallation/image/image.go @@ -9,11 +9,13 @@ import ( "io" "os" "path/filepath" + "strings" "sync" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/types" ociv1 "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/samber/lo" "oras.land/oras-go/v2" "oras.land/oras-go/v2/content/memory" "oras.land/oras-go/v2/registry/remote" @@ -23,8 +25,37 @@ import ( "github.com/kong/gateway-operator/modules/manager/metadata" ) -// FetchPluginContent fetches the content of the plugin from the image URL. When authentication is not needed pass nil. -func FetchPluginContent(ctx context.Context, imageURL string, credentialsStore credentials.Store) ([]byte, error) { +// The target files' names expected in an image with a custom Kong plugin. +const ( + kongPluginHandler = "handler.lua" + kongPluginSchema = "schema.lua" +) + +// PluginFiles maps a plugin's file names to their content. +// It's expected that each plugin consists of `schema.lua` and `handler.lua` files. +type PluginFiles map[string]string + +// newPluginFilesFromMap creates PluginFiles from a map of files with content. +// It ensures that the required files handler.lua and schema.lua are only present +// in the map. +func newPluginFilesFromMap(pluginFiles map[string]string) (PluginFiles, error) { + var missingFiles []string + for _, f := range []string{kongPluginHandler, kongPluginSchema} { + if _, ok := pluginFiles[f]; !ok { + missingFiles = append(missingFiles, f) + } + } + if len(missingFiles) > 0 { + return nil, fmt.Errorf("required files not found in the image: %s", strings.Join(missingFiles, ", ")) + } + if len(pluginFiles) != 2 { + return nil, fmt.Errorf("expected exactly 2 files, got %d: %s", len(pluginFiles), strings.Join(lo.Keys(pluginFiles), ",")) + } + return PluginFiles(pluginFiles), nil +} + +// FetchPlugin fetches the content of the plugin from the image URL. When authentication is not needed pass nil. +func FetchPlugin(ctx context.Context, imageURL string, credentialsStore credentials.Store) (PluginFiles, error) { ref, err := name.ParseReference(imageURL) if err != nil { return nil, fmt.Errorf("unexpected format of image url: %w", err) @@ -117,34 +148,41 @@ func (sl sizeLimitBytes) String() string { return fmt.Sprintf("%.2f MiB", float64(sl)/(1024*1024)) } -func extractKongPluginFromLayer(r io.Reader) ([]byte, error) { +func extractKongPluginFromLayer(r io.Reader) (PluginFiles, error) { + // Search for the files walking through the archive. + // The size of a plugin is limited to the size of a ConfigMap in Kubernetes. + const sizeLimit_1MiB sizeLimitBytes = 1024 * 1024 + gr, err := gzip.NewReader(r) if err != nil { return nil, fmt.Errorf("failed to parse layer as tar.gz: %w", err) } - - // The target file name for custom Kong plugin. - const kongPluginName = "plugin.lua" - // Search for the file walking through the archive. - // Size of plugin is limited to size of a ConfigMap in Kubernetes. - const sizeLimit_1MiB sizeLimitBytes = 1024 * 1024 + pluginFiles := make(map[string]string) for tr := tar.NewReader(io.LimitReader(gr, sizeLimit_1MiB.int64())); ; { - switch h, err := tr.Next(); { - case err == nil: - if filepath.Base(h.Name) == kongPluginName { - plugin := make([]byte, h.Size) - if _, err := io.ReadFull(tr, plugin); err != nil { - if errors.Is(err, io.ErrUnexpectedEOF) { - return nil, fmt.Errorf("plugin size exceed %s", sizeLimit_1MiB) - } - return nil, fmt.Errorf("failed to read %s from image: %w", kongPluginName, err) + h, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + return nil, fmt.Errorf("unexpected error during looking for plugin: %w", err) + } + + switch fileName := filepath.Base(h.Name); fileName { + case kongPluginHandler, kongPluginSchema: + file := make([]byte, h.Size) + if _, err := io.ReadFull(tr, file); err != nil { + if errors.Is(err, io.ErrUnexpectedEOF) { + return nil, fmt.Errorf("plugin size limit of %s exceeded", sizeLimit_1MiB) } - return plugin, nil + return nil, fmt.Errorf("failed to read %s from image: %w", fileName, err) } - case errors.Is(err, io.EOF): - return nil, fmt.Errorf("file %q not found in the image", kongPluginName) + pluginFiles[fileName] = string(file) default: - return nil, fmt.Errorf("unexpected error during looking for plugin: %w", err) + return nil, fmt.Errorf( + "file %q is unexpected, required files are %s and %s", fileName, kongPluginHandler, kongPluginSchema, + ) } } + + return newPluginFilesFromMap(pluginFiles) } diff --git a/controller/kongplugininstallation/image/image_test.go b/controller/kongplugininstallation/image/image_test.go index 6da649dc3..91b910d02 100644 --- a/controller/kongplugininstallation/image/image_test.go +++ b/controller/kongplugininstallation/image/image_test.go @@ -59,26 +59,26 @@ func TestCredentialsStoreFromString(t *testing.T) { func TestFetchPluginContent(t *testing.T) { t.Run("invalid image URL", func(t *testing.T) { - _, err := image.FetchPluginContent(context.Background(), "foo bar", nil) + _, err := image.FetchPlugin(context.Background(), "foo bar", nil) require.ErrorContains(t, err, "unexpected format of image url: could not parse reference: foo bar") }) const registryUrl = "northamerica-northeast1-docker.pkg.dev/k8s-team-playground/" t.Run("valid image (Docker format)", func(t *testing.T) { - plugin, err := image.FetchPluginContent( - context.Background(), registryUrl+"plugin-example/valid", nil, + plugin, err := image.FetchPlugin( + context.Background(), registryUrl+"plugin-example/valid:0.1.0", nil, ) require.NoError(t, err) - require.Equal(t, string(plugin), "plugin-content\n") + requireExpectedContent(t, plugin) }) t.Run("valid image (OCI format)", func(t *testing.T) { - plugin, err := image.FetchPluginContent( - context.Background(), registryUrl+"plugin-example/valid-oci", nil, + plugin, err := image.FetchPlugin( + context.Background(), registryUrl+"plugin-example/valid-oci:0.1.0", nil, ) require.NoError(t, err) - require.Equal(t, string(plugin), "plugin-content\n") + requireExpectedContent(t, plugin) }) t.Run("valid image from private registry", func(t *testing.T) { @@ -90,31 +90,65 @@ func TestFetchPluginContent(t *testing.T) { credsStore, err := image.CredentialsStoreFromString(credentials) require.NoError(t, err) - plugin, err := image.FetchPluginContent( - context.Background(), registryUrl+"plugin-example-private/valid:v1.0", credsStore, + plugin, err := image.FetchPlugin( + context.Background(), registryUrl+"plugin-example-private/valid:0.1.0", credsStore, ) require.NoError(t, err) - require.Equal(t, string(plugin), "plugin-content-private\n") + requireExpectedContentPrivate(t, plugin) }) t.Run("invalid image - too many layers", func(t *testing.T) { - _, err := image.FetchPluginContent( + _, err := image.FetchPlugin( context.Background(), registryUrl+"plugin-example/invalid-layers", nil, ) require.ErrorContains(t, err, "expected exactly one layer with plugin, found 2 layers") }) - t.Run("invalid image - invalid name of plugin inside of it", func(t *testing.T) { - _, err := image.FetchPluginContent( + t.Run("invalid image - invalid names of files", func(t *testing.T) { + _, err := image.FetchPlugin( context.Background(), registryUrl+"plugin-example/invalid-name", nil, ) - require.ErrorContains(t, err, `file "plugin.lua" not found in the image`) + require.ErrorContains(t, err, `file "add-header.lua" is unexpected, required files are handler.lua and schema.lua`) }) - t.Run("invalid image - invalid too big plugin", func(t *testing.T) { - _, err := image.FetchPluginContent( - context.Background(), registryUrl+"plugin-example/invalid-size", nil, + t.Run("invalid image - missing file", func(t *testing.T) { + _, err := image.FetchPlugin( + context.Background(), registryUrl+"plugin-example/missing-file", nil, ) - require.ErrorContains(t, err, "plugin size exceed 1.00 MiB") + require.ErrorContains(t, err, `required files not found in the image: schema.lua`) }) + + // Single file - handler.lua is over 1 MiB. + t.Run("invalid image - invalid too big plugin (size of single file)", func(t *testing.T) { + _, err := image.FetchPlugin( + context.Background(), registryUrl+"plugin-example/invalid-size-one", nil, + ) + require.ErrorContains(t, err, "plugin size limit of 1.00 MiB exceeded") + }) + + // Each file is 512 KiB so together they are 1 MiB. + t.Run("invalid image - invalid too big plugin (size of files combined)", func(t *testing.T) { + _, err := image.FetchPlugin( + context.Background(), registryUrl+"plugin-example/invalid-size-combined", nil, + ) + require.ErrorContains(t, err, "plugin size limit of 1.00 MiB exceeded") + }) +} + +func requireExpectedContent(t *testing.T, actual map[string]string) { + t.Helper() + require.Len(t, actual, 2) + require.Equal(t, map[string]string{ + "handler.lua": "handler-content\n", + "schema.lua": "schema-content\n", + }, actual) +} + +func requireExpectedContentPrivate(t *testing.T, actual map[string]string) { + t.Helper() + require.Len(t, actual, 2) + require.Equal(t, map[string]string{ + "handler.lua": "handler-content-private\n", + "schema.lua": "schema-content-private\n", + }, actual) } diff --git a/test/integration/test_kongplugininstallation.go b/test/integration/test_kongplugininstallation.go index 09751eb0f..3b3245f1a 100644 --- a/test/integration/test_kongplugininstallation.go +++ b/test/integration/test_kongplugininstallation.go @@ -49,7 +49,7 @@ func TestKongPluginInstallationEssentials(t *testing.T) { t.Log("updating KongPluginInstallation resource to a valid image") kpi, err = GetClients().OperatorClient.ApisV1alpha1().KongPluginInstallations(kpiNN.Namespace).Get(GetCtx(), kpiNN.Name, metav1.GetOptions{}) - kpi.Spec.Image = registryUrl + "plugin-example/valid" + kpi.Spec.Image = registryUrl + "plugin-example/valid:0.1.0" require.NoError(t, err) _, err = GetClients().OperatorClient.ApisV1alpha1().KongPluginInstallations(kpiNN.Namespace).Update(GetCtx(), kpi, metav1.UpdateOptions{}) require.NoError(t, err) @@ -71,7 +71,7 @@ func TestKongPluginInstallationEssentials(t *testing.T) { return } }, 15*time.Second, time.Second) - checkContentOfRespectiveCM(t, respectiveCM, kpiNN.Name, "plugin-content\n") + require.Equal(t, pluginExpectedContent(), respectiveCM.Data) t.Log("delete respective ConfigMap to check if it will be recreated") var respectiveCMName = respectiveCM.Name @@ -83,12 +83,12 @@ func TestKongPluginInstallationEssentials(t *testing.T) { recreatedCM, err = GetClients().K8sClient.CoreV1().ConfigMaps(kpiNN.Namespace).Get(GetCtx(), respectiveCMName, metav1.GetOptions{}) assert.NoError(c, err) }, 15*time.Second, time.Second) - checkContentOfRespectiveCM(t, *recreatedCM, kpiNN.Name, "plugin-content\n") + require.Equal(t, pluginExpectedContent(), recreatedCM.Data) if registryCreds := GetKongPluginImageRegistryCredentialsForTests(); registryCreds != "" { t.Log("update KongPluginInstallation resource to a private image") kpi, err = GetClients().OperatorClient.ApisV1alpha1().KongPluginInstallations(kpiNN.Namespace).Get(GetCtx(), kpiNN.Name, metav1.GetOptions{}) - kpi.Spec.Image = registryUrl + "plugin-example-private/valid:v1.0" + kpi.Spec.Image = registryUrl + "plugin-example-private/valid:0.1.0" require.NoError(t, err) _, err = GetClients().OperatorClient.ApisV1alpha1().KongPluginInstallations(kpiNN.Namespace).Update(GetCtx(), kpi, metav1.UpdateOptions{}) require.NoError(t, err) @@ -128,7 +128,7 @@ func TestKongPluginInstallationEssentials(t *testing.T) { require.EventuallyWithT(t, func(c *assert.CollectT) { updatedCM, err = GetClients().K8sClient.CoreV1().ConfigMaps(kpiNN.Namespace).Get(GetCtx(), respectiveCMName, metav1.GetOptions{}) assert.NoError(c, err) - checkContentOfRespectiveCM(t, *updatedCM, kpiNN.Name, "plugin-content-private\n") + assert.Equal(c, privatePluginExpectedContent(), updatedCM.Data) }, 15*time.Second, time.Second) } else { t.Log("skipping private image test - no credentials provided") @@ -172,8 +172,16 @@ func checkKongPluginInstallationConditions( }, 15*time.Second, time.Second) } -func checkContentOfRespectiveCM(t *testing.T, respectiveCM corev1.ConfigMap, kpiName, expectedPluginContent string) { - pluginContent, ok := respectiveCM.Data[kpiName+".lua"] - require.True(t, ok, "plugin.lua not found in ConfigMap") - require.Equal(t, expectedPluginContent, pluginContent) +func pluginExpectedContent() map[string]string { + return map[string]string{ + "handler.lua": "handler-content\n", + "schema.lua": "schema-content\n", + } +} + +func privatePluginExpectedContent() map[string]string { + return map[string]string{ + "handler.lua": "handler-content-private\n", + "schema.lua": "schema-content-private\n", + } }