CR fixes

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

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{

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)
 		}

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")
+	})
 }

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.")

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{},
 	}