From 21864616eab12e2a1b5c46958df97ef7b6a669f7 Mon Sep 17 00:00:00 2001 From: Jakub Warczarek Date: Tue, 6 Aug 2024 13:36:28 +0200 Subject: [PATCH] CR fixes --- ...or.konghq.com_kongplugininstallations.yaml | 2 +- config/debug/manager_debug.yaml | 4 +++ config/dev/manager_dev.yaml | 1 + .../kongplugininstallation/controller.go | 23 ++++++++------- .../kongplugininstallation/image/image.go | 29 ++++++++++++------- .../image/image_test.go | 16 +++++----- modules/cli/cli.go | 2 +- modules/cli/cli_test.go | 2 +- 8 files changed, 48 insertions(+), 31 deletions(-) diff --git a/config/crd/bases/gateway-operator.konghq.com_kongplugininstallations.yaml b/config/crd/bases/gateway-operator.konghq.com_kongplugininstallations.yaml index db811691f..27b6746dd 100644 --- a/config/crd/bases/gateway-operator.konghq.com_kongplugininstallations.yaml +++ b/config/crd/bases/gateway-operator.konghq.com_kongplugininstallations.yaml @@ -20,7 +20,7 @@ spec: scope: Namespaced versions: - additionalPrinterColumns: - - description: The Resource is ready + - description: The Resource is accepted jsonPath: .status.conditions[?(@.type=='Accepted')].status name: Accepted type: string diff --git a/config/debug/manager_debug.yaml b/config/debug/manager_debug.yaml index b7fffd7d1..50fc24435 100644 --- a/config/debug/manager_debug.yaml +++ b/config/debug/manager_debug.yaml @@ -31,6 +31,10 @@ spec: - -zap-time-encoding=iso8601 - -cluster-ca-secret-namespace=kong-system - -zap-log-level=debug + - -enable-controller-gateway=true + - -enable-controller-controlplane=true + - -enable-controller-kongplugininstallation=true + - -enable-validating-webhook=true name: manager env: - name: GATEWAY_OPERATOR_DEVELOPMENT_MODE diff --git a/config/dev/manager_dev.yaml b/config/dev/manager_dev.yaml index 00fa640cb..cc2667cb6 100644 --- a/config/dev/manager_dev.yaml +++ b/config/dev/manager_dev.yaml @@ -24,6 +24,7 @@ spec: - -zap-devel=true - -enable-controller-gateway=true - -enable-controller-controlplane=true + - -enable-controller-kongplugininstallation=true - -enable-validating-webhook=true name: manager env: diff --git a/controller/kongplugininstallation/controller.go b/controller/kongplugininstallation/controller.go index bb7cf31f7..720c9848e 100644 --- a/controller/kongplugininstallation/controller.go +++ b/controller/kongplugininstallation/controller.go @@ -46,7 +46,7 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { return false }, UpdateFunc: func(e event.UpdateEvent) bool { - return false + return true }, }, )). @@ -79,6 +79,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu log.Trace(logger, "managing KongPluginInstallation resource", kpi) var credentialsStore credentials.Store if kpi.Spec.ImagePullSecretRef != nil { + log.Trace(logger, "getting secret for KongPluginInstallation resource", kpi) secretNN := client.ObjectKey{ Namespace: kpi.Spec.ImagePullSecretRef.Namespace, Name: kpi.Spec.ImagePullSecretRef.Name, @@ -93,26 +94,27 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu secretNN, &secret, ); err != nil { - return ctrl.Result{}, setStatusConditionFailed(ctx, r.Client, &kpi, fmt.Sprintf("cannot retrieve secret %q, because: %s", secretNN, err)) + return ctrl.Result{}, setStatusConditionFailedForKongPluginInstallation(ctx, r.Client, &kpi, fmt.Sprintf("cannot retrieve secret %q, because: %s", secretNN, err)) } const requiredKey = ".dockerconfigjson" secretData, ok := secret.Data[requiredKey] if !ok { - return ctrl.Result{}, setStatusConditionFailed( + return ctrl.Result{}, setStatusConditionFailedForKongPluginInstallation( ctx, r.Client, &kpi, fmt.Sprintf("can't parse secret %q - unexpected type, it should follow 'kubernetes.io/dockerconfigjson'", secretNN), ) } var err error credentialsStore, err = image.CredentialsStoreFromString(string(secretData)) if err != nil { - return ctrl.Result{}, setStatusConditionFailed(ctx, r.Client, &kpi, fmt.Sprintf("can't parse secret: %q data: %s", secretNN, err)) + return ctrl.Result{}, setStatusConditionFailedForKongPluginInstallation(ctx, r.Client, &kpi, fmt.Sprintf("can't parse secret: %q data: %s", secretNN, err)) } } + log.Trace(logger, "fetch plugin for KongPluginInstallation resource", kpi) plugin, err := image.FetchPluginContent(ctx, kpi.Spec.Image, credentialsStore) if err != nil { - return ctrl.Result{}, setStatusConditionFailed(ctx, r.Client, &kpi, fmt.Sprintf("problem with the image: %q error: %s", kpi.Spec.Image, err)) + return ctrl.Result{}, setStatusConditionFailedForKongPluginInstallation(ctx, r.Client, &kpi, fmt.Sprintf("problem with the image: %q error: %s", kpi.Spec.Image, err)) } cms, err := kubernetes.ListConfigMapsForOwner(ctx, r.Client, kpi.GetUID()) @@ -140,7 +142,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu kpi.Status.UnderlyingConfigMapName = cm.Name case 1: cm = cms[0] - cm.Name = kpi.Status.UnderlyingConfigMapName cm.Data = map[string]string{ fmt.Sprintf("%s.lua", kpi.Name): string(plugin), } @@ -152,7 +153,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, errors.New("unexpected error happened - more than one ConfigMap found") } - return ctrl.Result{}, setStatusCondition( + return ctrl.Result{}, setStatusConditionForKongPluginInstallation( ctx, r.Client, &kpi, metav1.ConditionTrue, v1alpha1.KongPluginInstallationReasonReady, "plugin successfully saved in cluster as ConfigMap", ) } @@ -189,11 +190,13 @@ func (r *Reconciler) listKongPluginInstallationsForSecret(ctx context.Context, o return recs } -func setStatusConditionFailed(ctx context.Context, client client.Client, kpi *v1alpha1.KongPluginInstallation, msg string) error { - return setStatusCondition(ctx, client, kpi, metav1.ConditionFalse, v1alpha1.KongPluginInstallationReasonFailed, msg) +func setStatusConditionFailedForKongPluginInstallation( + ctx context.Context, client client.Client, kpi *v1alpha1.KongPluginInstallation, msg string, +) error { + return setStatusConditionForKongPluginInstallation(ctx, client, kpi, metav1.ConditionFalse, v1alpha1.KongPluginInstallationReasonFailed, msg) } -func setStatusCondition( +func setStatusConditionForKongPluginInstallation( ctx context.Context, client client.Client, kpi *v1alpha1.KongPluginInstallation, conditionStatus metav1.ConditionStatus, reason v1alpha1.KongPluginInstallationConditionReason, msg string, ) error { status := metav1.Condition{ diff --git a/controller/kongplugininstallation/image/image.go b/controller/kongplugininstallation/image/image.go index e8e730061..cfad77da6 100644 --- a/controller/kongplugininstallation/image/image.go +++ b/controller/kongplugininstallation/image/image.go @@ -73,8 +73,8 @@ func FetchPluginContent(ctx context.Context, imageURL string, credentialsStore c }); err != nil { return nil, fmt.Errorf("can't fetch image: %s, because: %w", imageURL, err) } - // How to build such image is described in details, - // following manual results in the image with exactly one layer that contains a plugin. + // Image with plugin should have exactly one layer that contains a plugin with the name plugin.lua. + // This is requirement described in details in the documentation. Any mismatch is treated as invalid image. if numOfLayers := len(layersThatMayContainPlugin); numOfLayers != 1 { return nil, fmt.Errorf("expected exactly one layer with plugin, found %d layers", numOfLayers) } @@ -99,14 +99,24 @@ func CredentialsStoreFromString(s string) (credentials.Store, error) { if err != nil { return nil, fmt.Errorf("failed to create temporary file: %w", err) } - defer tmpFile.Close() defer os.Remove(tmpFile.Name()) + defer tmpFile.Close() if _, err = tmpFile.WriteString(s); err != nil { return nil, fmt.Errorf("failed to write credentials to file: %w", err) } return credentials.NewFileStore(tmpFile.Name()) } +type sizeLimitBytes int64 + +func (sl sizeLimitBytes) int64() int64 { + return int64(sl) +} + +func (sl sizeLimitBytes) String() string { + return fmt.Sprintf("%.2f MiB", float64(sl)/(1024*1024)) +} + func extractKongPluginFromLayer(r io.Reader) ([]byte, error) { gr, err := gzip.NewReader(r) if err != nil { @@ -115,25 +125,24 @@ func extractKongPluginFromLayer(r io.Reader) ([]byte, error) { // The target file name for custom Kong plugin. const kongPluginName = "plugin.lua" - // Search for the file walking through the archive. - // Limit plugin to 1MB the same as ConfigMap in Kubernetes. - const sizeLimit_1MiB = 1024 * 1024 - - for tr := tar.NewReader(io.LimitReader(gr, sizeLimit_1MiB)); ; { + // Size of plugin is limited to size of a ConfigMap in Kubernetes. + const sizeLimit_1MiB sizeLimitBytes = 1024 * 1024 + 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) } return plugin, nil } case errors.Is(err, io.EOF): return nil, fmt.Errorf("file %q not found in the image", kongPluginName) - case errors.Is(err, io.ErrUnexpectedEOF): - return nil, fmt.Errorf("plugin size exceed %d bytes", sizeLimit_1MiB) default: return nil, fmt.Errorf("unexpected error during looking for plugin: %w", err) } diff --git a/controller/kongplugininstallation/image/image_test.go b/controller/kongplugininstallation/image/image_test.go index e09007756..6da649dc3 100644 --- a/controller/kongplugininstallation/image/image_test.go +++ b/controller/kongplugininstallation/image/image_test.go @@ -81,13 +81,6 @@ func TestFetchPluginContent(t *testing.T) { require.Equal(t, string(plugin), "plugin-content\n") }) - t.Run("invalid image - to many layers", func(t *testing.T) { - _, err := image.FetchPluginContent( - context.Background(), registryUrl+"plugin-example/invalid-layers", nil, - ) - require.ErrorContains(t, err, "expected exactly one layer with plugin, found 2 layers") - }) - t.Run("valid image from private registry", func(t *testing.T) { credentials := integration.GetKongPluginImageRegistryCredentialsForTests() if credentials == "" { @@ -104,7 +97,7 @@ func TestFetchPluginContent(t *testing.T) { require.Equal(t, string(plugin), "plugin-content-private\n") }) - t.Run("invalid image - to many layers", func(t *testing.T) { + t.Run("invalid image - too many layers", func(t *testing.T) { _, err := image.FetchPluginContent( context.Background(), registryUrl+"plugin-example/invalid-layers", nil, ) @@ -117,4 +110,11 @@ func TestFetchPluginContent(t *testing.T) { ) require.ErrorContains(t, err, `file "plugin.lua" not found in the image`) }) + + t.Run("invalid image - invalid too big plugin", func(t *testing.T) { + _, err := image.FetchPluginContent( + context.Background(), registryUrl+"plugin-example/invalid-size", nil, + ) + require.ErrorContains(t, err, "plugin size exceed 1.00 MiB") + }) } diff --git a/modules/cli/cli.go b/modules/cli/cli.go index 5b71a7997..1b4c5a7e5 100644 --- a/modules/cli/cli.go +++ b/modules/cli/cli.go @@ -43,7 +43,7 @@ func New(m metadata.Info) *CLI { // controllers for specialized APIs and features flagSet.BoolVar(&cfg.AIGatewayControllerEnabled, "enable-controller-aigateway", false, "Enable the AIGateway controller. (Experimental).") - flagSet.BoolVar(&cfg.KongPluginInstallationControllerEnabled, "enable-controller-kongplugininstallation", true, "Enable the KongPluginInstallation controller.") + flagSet.BoolVar(&cfg.KongPluginInstallationControllerEnabled, "enable-controller-kongplugininstallation", false, "Enable the KongPluginInstallation controller.") // controllers for Konnect APIs flagSet.BoolVar(&cfg.KonnectControllersEnabled, "enable-controller-konnect", false, "Enable the Konnect controllers.") diff --git a/modules/cli/cli_test.go b/modules/cli/cli_test.go index 29b2e9d20..38ae229ff 100644 --- a/modules/cli/cli_test.go +++ b/modules/cli/cli_test.go @@ -149,7 +149,7 @@ func expectedDefaultCfg() manager.Config { DataPlaneControllerEnabled: true, DataPlaneBlueGreenControllerEnabled: true, KonnectControllersEnabled: false, - KongPluginInstallationControllerEnabled: true, + KongPluginInstallationControllerEnabled: false, ValidatingWebhookEnabled: true, LoggerOpts: &zap.Options{}, }