Merge pull request #160 from Kuadrant/78-production-ready-configure-p…

…ossible-different-image

pull an image from a private registry

Makefile

@@ -234,14 +234,16 @@ clean-cov: ## Remove coverage reports
 .PHONY: test
 test: test-unit test-integration ## Run all tests
 
+ifdef VERBOSE
+test-integration: VERBOSE_FLAG = -v
+endif
 test-integration: clean-cov generate fmt vet ginkgo ## Run Integration tests.
 	mkdir -p $(PROJECT_PATH)/coverage/integration
 #	Check `ginkgo help run` for command line options. For example to filtering tests.
-	$(GINKGO) \
+	$(GINKGO) $(VERBOSE_FLAG) \
 		--coverpkg $(INTEGRATION_COVER_PKGS) \
 		--output-dir $(PROJECT_PATH)/coverage/integration \
 		--coverprofile cover.out \
-		-v \
 		--compilers=$(INTEGRATION_TEST_NUM_CORES) \
 		--procs=$(INTEGRATION_TEST_NUM_PROCESSES) \
 		--randomize-all \
@@ -255,9 +257,12 @@ test-integration: clean-cov generate fmt vet ginkgo ## Run Integration tests.
 ifdef TEST_NAME
 test-unit: TEST_PATTERN := --run $(TEST_NAME)
 endif
+ifdef VERBOSE
+test-unit: VERBOSE_FLAG = -v
+endif
 test-unit: clean-cov generate fmt vet ## Run Unit tests.
 	mkdir -p $(PROJECT_PATH)/coverage/unit
-	go test $(UNIT_DIRS) -coverprofile $(PROJECT_PATH)/coverage/unit/cover.out -v -timeout 0 $(TEST_PATTERN)
+	go test $(UNIT_DIRS) -coverprofile $(PROJECT_PATH)/coverage/unit/cover.out $(VERBOSE_FLAG) -timeout 0 $(TEST_PATTERN)
 
 ##@ Build
 build: GIT_SHA=$(shell git rev-parse HEAD || echo "unknown")

README.md

@@ -44,6 +44,7 @@ spec:
 * [Rate Limit Headers](./doc/rate-limit-headers.md)
 * [Logging](./doc/logging.md)
 * [Tracing](./doc/tracing.md)
+* [Custom Image](./doc/custom-image.md)
 
 ## Contributing
 
@@ -60,4 +61,4 @@ This software is licensed under the [Apache 2.0 license](https://www.apache.org/
 See the LICENSE and NOTICE files that should have been provided along with this software for details.
 
 
-[![FOSSA Status](https://app.fossa.com/api/projects/git%2Bgithub.com%2FKuadrant%2Flimitador-operator.svg?type=large)](https://app.fossa.com/projects/git%2Bgithub.com%2FKuadrant%2Flimitador-operator?ref=badge_large)
+[![FOSSA Status](https://app.fossa.com/api/projects/git%2Bgithub.com%2FKuadrant%2Flimitador-operator.svg?type=large)](https://app.fossa.com/projects/git%2Bgithub.com%2FKuadrant%2Flimitador-operator?ref=badge_large)

api/v1alpha1/limitador_types.go

@@ -107,6 +107,9 @@ type LimitadorSpec struct {
 
 	// +optional
 	Image *string `json:"image,omitempty"`
+
+	// +optional
+	ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty" patchStrategy:"merge" patchMergeKey:"name" protobuf:"bytes,15,rep,name=imagePullSecrets"`
 }
 
 //+kubebuilder:object:root=true

bundle/manifests/limitador-operator.clusterserviceversion.yaml

@@ -37,7 +37,7 @@ metadata:
     capabilities: Basic Install
     categories: Integration & Delivery
     containerImage: quay.io/kuadrant/limitador-operator:latest
-    createdAt: "2024-09-24T13:57:26Z"
+    createdAt: "2024-09-27T10:13:26Z"
     operators.operatorframework.io/builder: operator-sdk-v1.32.0
     operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
     repository: https://github.com/Kuadrant/limitador-operator

bundle/manifests/limitador.kuadrant.io_limitadors.yaml

@@ -803,6 +803,21 @@ spec:
                 type: object
               image:
                 type: string
+              imagePullSecrets:
+                items:
+                  description: |-
+                    LocalObjectReference contains enough information to let you locate the
+                    referenced object inside the same namespace.
+                  properties:
+                    name:
+                      description: |-
+                        Name of the referent.
+                        More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                        TODO: Add other useful fields. apiVersion, kind, uid?
+                      type: string
+                  type: object
+                  x-kubernetes-map-type: atomic
+                type: array
               limits:
                 items:
                   description: RateLimit defines the desired Limitador limit

config/crd/bases/limitador.kuadrant.io_limitadors.yaml

@@ -803,6 +803,21 @@ spec:
                 type: object
               image:
                 type: string
+              imagePullSecrets:
+                items:
+                  description: |-
+                    LocalObjectReference contains enough information to let you locate the
+                    referenced object inside the same namespace.
+                  properties:
+                    name:
+                      description: |-
+                        Name of the referent.
+                        More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                        TODO: Add other useful fields. apiVersion, kind, uid?
+                      type: string
+                  type: object
+                  x-kubernetes-map-type: atomic
+                type: array
               limits:
                 items:
                   description: RateLimit defines the desired Limitador limit

controllers/limitador_controller.go

@@ -24,7 +24,7 @@ import (
 
 	"github.com/go-logr/logr"
 	appsv1 "k8s.io/api/apps/v1"
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	policyv1 "k8s.io/api/policy/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/labels"
@@ -137,7 +137,7 @@ func (r *LimitadorReconciler) reconcileSpec(ctx context.Context, limitadorObj *l
 }
 
 func (r *LimitadorReconciler) reconcilePodLimitsHashAnnotation(ctx context.Context, limitadorObj *limitadorv1alpha1.Limitador) (ctrl.Result, error) {
-	podList := &v1.PodList{}
+	podList := &corev1.PodList{}
 	options := &client.ListOptions{
 		LabelSelector: labels.SelectorFromSet(limitador.Labels(limitadorObj)),
 		Namespace:     limitadorObj.Namespace,
@@ -156,7 +156,7 @@ func (r *LimitadorReconciler) reconcilePodLimitsHashAnnotation(ctx context.Conte
 	}
 
 	// Use CM resource version to track limits changes
-	cm := &v1.ConfigMap{}
+	cm := &corev1.ConfigMap{}
 	if err := r.Client().Get(ctx, types.NamespacedName{Name: limitador.LimitsConfigMapName(limitadorObj), Namespace: limitadorObj.Namespace}, cm); err != nil {
 		if apierrors.IsNotFound(err) {
 			return ctrl.Result{Requeue: true}, nil
@@ -236,6 +236,13 @@ func (r *LimitadorReconciler) reconcileDeployment(ctx context.Context, limitador
 		reconcilers.DeploymentReadinessProbeMutator,
 	)
 
+	// reconcile imagepullsecrets only when set in limitador CR
+	// if not set in limitador CR, the user will be able to add them manually and the operator
+	// will not revert the changes.
+	if len(deploymentOptions.ImagePullSecrets) > 0 {
+		deploymentMutators = append(deploymentMutators, reconcilers.DeploymentImagePullSecretsMutator)
+	}
+
 	deployment := limitador.Deployment(limitadorObj, deploymentOptions)
 	// controller reference
 	if err := r.SetOwnerReference(limitadorObj, deployment); err != nil {
@@ -327,13 +334,13 @@ func (r *LimitadorReconciler) reconcileLimitsConfigMap(ctx context.Context, limi
 }
 
 func mutateLimitsConfigMap(existingObj, desiredObj client.Object) (bool, error) {
-	existing, ok := existingObj.(*v1.ConfigMap)
+	existing, ok := existingObj.(*corev1.ConfigMap)
 	if !ok {
-		return false, fmt.Errorf("%T is not a *v1.ConfigMap", existingObj)
+		return false, fmt.Errorf("%T is not a *corev1.ConfigMap", existingObj)
 	}
-	desired, ok := desiredObj.(*v1.ConfigMap)
+	desired, ok := desiredObj.(*corev1.ConfigMap)
 	if !ok {
-		return false, fmt.Errorf("%T is not a *v1.ConfigMap", desiredObj)
+		return false, fmt.Errorf("%T is not a *corev1.ConfigMap", desiredObj)
 	}
 
 	updated := false
@@ -378,6 +385,7 @@ func (r *LimitadorReconciler) getDeploymentOptions(ctx context.Context, limObj *
 	if err != nil {
 		return deploymentOptions, err
 	}
+	deploymentOptions.ImagePullSecrets = r.getDeploymentImagePullSecrets(limObj)
 
 	return deploymentOptions, nil
 }
@@ -402,7 +410,7 @@ func (r *LimitadorReconciler) getDeploymentStorageOptions(ctx context.Context, l
 	return limitador.InMemoryDeploymentOptions()
 }
 
-func (r *LimitadorReconciler) getDeploymentEnvVar(limObj *limitadorv1alpha1.Limitador) ([]v1.EnvVar, error) {
+func (r *LimitadorReconciler) getDeploymentEnvVar(limObj *limitadorv1alpha1.Limitador) ([]corev1.EnvVar, error) {
 	if limObj.Spec.Storage != nil {
 		if limObj.Spec.Storage.Redis != nil {
 			return limitador.DeploymentEnvVar(limObj.Spec.Storage.Redis.ConfigSecretRef)
@@ -416,12 +424,16 @@ func (r *LimitadorReconciler) getDeploymentEnvVar(limObj *limitadorv1alpha1.Limi
 	return nil, nil
 }
 
+func (r *LimitadorReconciler) getDeploymentImagePullSecrets(limObj *limitadorv1alpha1.Limitador) []corev1.LocalObjectReference {
+	return limObj.Spec.ImagePullSecrets
+}
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *LimitadorReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&limitadorv1alpha1.Limitador{}).
 		Owns(&appsv1.Deployment{}).
-		Owns(&v1.ConfigMap{}).
+		Owns(&corev1.ConfigMap{}).
 		Owns(&policyv1.PodDisruptionBudget{}).
 		Complete(r)
 }

controllers/limitador_controller_test.go

@@ -715,6 +715,33 @@ var _ = Describe("Limitador controller", func() {
 			Expect(pvc.GetOwnerReferences()).To(HaveLen(1))
 		}, specTimeOut)
 	})
+
+	Context("Creating a new Limitador object with imagePullSecrets", func() {
+		var limitadorObj *limitadorv1alpha1.Limitador
+
+		BeforeEach(func(ctx SpecContext) {
+			limitadorObj = basicLimitador(testNamespace)
+			limitadorObj.Spec.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "regcred"}}
+
+			Expect(k8sClient.Create(ctx, limitadorObj)).Should(Succeed())
+			Eventually(testLimitadorIsReady(ctx, limitadorObj)).WithContext(ctx).Should(Succeed())
+		})
+
+		It("Should create a new deployment with imagepullsecrets", func(ctx SpecContext) {
+			deployment := &appsv1.Deployment{}
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx,
+					types.NamespacedName{
+						Namespace: testNamespace,
+						Name:      limitador.DeploymentName(limitadorObj),
+					}, deployment)).To(Succeed())
+			}).WithContext(ctx).Should(Succeed())
+
+			Expect(deployment.Spec.Template.Spec.ImagePullSecrets).To(
+				HaveExactElements(corev1.LocalObjectReference{Name: "regcred"}),
+			)
+		}, specTimeOut)
+	})
 })
 
 func basicLimitador(ns string) *limitadorv1alpha1.Limitador {

doc/custom-image.md

@@ -0,0 +1,53 @@
+# Custom Image
+
+Currently, the limitador image being used in the deployment is read from different sources with some order of precedence:
+* If Limtador CR's `spec.image` is set -> image = `${spec.image}`
+* If Limtador CR's `spec.version` is set -> image = `quay.io/kuadrant/limitador:${spec.version}` (note the repo is hardcoded)
+* if `RELATED_IMAGE_LIMITADOR` env var is set -> image = `$RELATED_IMAGE_LIMITADOR`
+* else: hardcoded to `quay.io/kuadrant/limitador:latest`
+
+The `spec.image` field is not meant to be used in production environments.
+It is meant to be used for dev/testing purposes.
+The main drawback of the `spec.image` usage is that upgrades cannot be supported as the
+limitador operator cannot ensure the operation to be safe.
+
+
+```yaml
+---
+apiVersion: limitador.kuadrant.io/v1alpha1
+kind: Limitador
+metadata:
+  name: limitador-instance-1
+spec:
+  image: example.com/myorg/limitador-repo:custom-image-v1
+EOF
+```
+
+## Pull an Image from a Private Registry
+
+To pull an image from a private container image registry or repository, you need to provide credentials.
+
+Create a Secret of type `kubernetes.io/dockerconfigjson` by providing credentials.
+For example, using `kubectl` tool with the following command line:
+
+```
+kubectl create secret docker-registry regcred --docker-server=<your-registry-server> --docker-username=<your-name> --docker-password=<your-pword>
+```
+
+That will create a secret named `regcred`.
+
+Deploy limitador instance with the `imagePullSecrets` field having a reference to the `regcred`.
+
+```yaml
+---
+apiVersion: limitador.kuadrant.io/v1alpha1
+kind: Limitador
+metadata:
+  name: limitador-instance-1
+spec:
+  image: example.com/myorg/limitador-repo:custom-image-v1
+  imagePullSecrets:
+  - name: regcred
+```
+
+> **NOTE**: It is mandatory that the secret and limitador CR are created in the same namespace.

pkg/limitador/deployment_options.go

@@ -7,23 +7,24 @@ import (
 	"strings"
 
 	appsv1 "k8s.io/api/apps/v1"
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 
 	limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1"
 )
 
 type DeploymentOptions struct {
 	Command            []string
-	VolumeMounts       []v1.VolumeMount
-	Volumes            []v1.Volume
+	VolumeMounts       []corev1.VolumeMount
+	Volumes            []corev1.Volume
 	DeploymentStrategy appsv1.DeploymentStrategy
-	EnvVar             []v1.EnvVar
+	EnvVar             []corev1.EnvVar
+	ImagePullSecrets   []corev1.LocalObjectReference
 }
 
 type DeploymentStorageOptions struct {
 	Command            []string
-	VolumeMounts       []v1.VolumeMount
-	Volumes            []v1.Volume
+	VolumeMounts       []corev1.VolumeMount
+	Volumes            []corev1.Volume
 	DeploymentStrategy appsv1.DeploymentStrategy
 }
 
@@ -67,8 +68,8 @@ func DeploymentCommand(limObj *limitadorv1alpha1.Limitador, storageOptions Deplo
 	return command
 }
 
-func DeploymentVolumeMounts(storageOptions DeploymentStorageOptions) []v1.VolumeMount {
-	volumeMounts := []v1.VolumeMount{
+func DeploymentVolumeMounts(storageOptions DeploymentStorageOptions) []corev1.VolumeMount {
+	volumeMounts := []corev1.VolumeMount{
 		{
 			Name:      LimitsCMVolumeName,
 			MountPath: LimitadorCMMountPath,
@@ -78,13 +79,13 @@ func DeploymentVolumeMounts(storageOptions DeploymentStorageOptions) []v1.Volume
 	return volumeMounts
 }
 
-func DeploymentVolumes(limObj *limitadorv1alpha1.Limitador, storageOptions DeploymentStorageOptions) []v1.Volume {
-	volumes := []v1.Volume{
+func DeploymentVolumes(limObj *limitadorv1alpha1.Limitador, storageOptions DeploymentStorageOptions) []corev1.Volume {
+	volumes := []corev1.Volume{
 		{
 			Name: LimitsCMVolumeName,
-			VolumeSource: v1.VolumeSource{
-				ConfigMap: &v1.ConfigMapVolumeSource{
-					LocalObjectReference: v1.LocalObjectReference{
+			VolumeSource: corev1.VolumeSource{
+				ConfigMap: &corev1.ConfigMapVolumeSource{
+					LocalObjectReference: corev1.LocalObjectReference{
 						Name: LimitsConfigMapName(limObj),
 					},
 				},

pkg/limitador/k8s_objects.go

@@ -87,7 +87,8 @@ func Deployment(limitador *limitadorv1alpha1.Limitador, deploymentOptions Deploy
 					Labels: Labels(limitador),
 				},
 				Spec: v1.PodSpec{
-					Affinity: limitador.Spec.Affinity,
+					Affinity:         limitador.Spec.Affinity,
+					ImagePullSecrets: deploymentOptions.ImagePullSecrets,
 					Containers: []v1.Container{
 						{
 							Name:    "limitador",