From 42236a8141f81d5eca857eaaa60bb3c24ee21628 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Fri, 27 Sep 2024 12:48:58 +0200 Subject: [PATCH] imagepullSecrets to access private repos Signed-off-by: Eguzki Astiz Lezaun --- Makefile | 11 +++++-- api/v1alpha1/zz_generated.deepcopy.go | 5 ++++ ...itador-operator.clusterserviceversion.yaml | 2 +- .../limitador.kuadrant.io_limitadors.yaml | 15 ++++++++++ .../limitador.kuadrant.io_limitadors.yaml | 15 ++++++++++ controllers/limitador_controller.go | 30 +++++++++++++------ controllers/limitador_controller_test.go | 27 +++++++++++++++++ doc/custom-image.md | 10 +++++-- pkg/limitador/deployment_options.go | 27 +++++++++-------- pkg/limitador/k8s_objects.go | 3 +- pkg/limitador/k8s_objects_test.go | 12 ++++++++ pkg/reconcilers/deployment.go | 20 +++++++++++++ 12 files changed, 147 insertions(+), 30 deletions(-) diff --git a/Makefile b/Makefile index 28bf0081..152ef862 100644 --- a/Makefile +++ b/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") diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 49a380ea..34573d0e 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -197,6 +197,11 @@ func (in *LimitadorSpec) DeepCopyInto(out *LimitadorSpec) { *out = new(string) **out = **in } + if in.ImagePullSecrets != nil { + in, out := &in.ImagePullSecrets, &out.ImagePullSecrets + *out = make([]v1.LocalObjectReference, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LimitadorSpec. diff --git a/bundle/manifests/limitador-operator.clusterserviceversion.yaml b/bundle/manifests/limitador-operator.clusterserviceversion.yaml index 90633ad6..fec73e00 100644 --- a/bundle/manifests/limitador-operator.clusterserviceversion.yaml +++ b/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 diff --git a/bundle/manifests/limitador.kuadrant.io_limitadors.yaml b/bundle/manifests/limitador.kuadrant.io_limitadors.yaml index e9bc7011..07357473 100644 --- a/bundle/manifests/limitador.kuadrant.io_limitadors.yaml +++ b/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 diff --git a/config/crd/bases/limitador.kuadrant.io_limitadors.yaml b/config/crd/bases/limitador.kuadrant.io_limitadors.yaml index cc1b6be6..19b39804 100644 --- a/config/crd/bases/limitador.kuadrant.io_limitadors.yaml +++ b/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 diff --git a/controllers/limitador_controller.go b/controllers/limitador_controller.go index 3d81ab4a..4ee11325 100644 --- a/controllers/limitador_controller.go +++ b/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) } diff --git a/controllers/limitador_controller_test.go b/controllers/limitador_controller_test.go index 93d0b4c4..2d6a18b5 100644 --- a/controllers/limitador_controller_test.go +++ b/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 { diff --git a/doc/custom-image.md b/doc/custom-image.md index 0a0e9b4d..1d828f21 100644 --- a/doc/custom-image.md +++ b/doc/custom-image.md @@ -27,14 +27,16 @@ EOF To pull an image from a private container image registry or repository, you need to provide credentials. -Create a Secret by providing credentials on the command line +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= --docker-username= --docker-password= --docker-email= +kubectl create secret docker-registry regcred --docker-server= --docker-username= --docker-password= ``` -Deploy limitador instance that uses your secret +That will create a secret named `regcred`. +Deploy limitador instance with the `imagePullSecrets` field having a reference to the `regcred`. ```yaml --- @@ -47,3 +49,5 @@ spec: imagePullSecrets: - name: regcred ``` + +> **NOTE**: It is mandatory that the secret and limitador CR are created in the same namespace. diff --git a/pkg/limitador/deployment_options.go b/pkg/limitador/deployment_options.go index 07427a86..b6e7abc3 100644 --- a/pkg/limitador/deployment_options.go +++ b/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), }, }, diff --git a/pkg/limitador/k8s_objects.go b/pkg/limitador/k8s_objects.go index ff1a4758..57eda6ba 100644 --- a/pkg/limitador/k8s_objects.go +++ b/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", diff --git a/pkg/limitador/k8s_objects_test.go b/pkg/limitador/k8s_objects_test.go index 2aa2ce26..0e8dfe15 100644 --- a/pkg/limitador/k8s_objects_test.go +++ b/pkg/limitador/k8s_objects_test.go @@ -4,6 +4,7 @@ import ( "testing" "gotest.tools/assert" + corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" policyv1 "k8s.io/api/policy/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -192,6 +193,17 @@ func TestDeployment(t *testing.T) { }, ) }) + + t.Run("imagePullSecrets", func(subT *testing.T) { + limObj := newTestLimitadorObj("some-name", "some-ns", nil) + deployment := Deployment(limObj, DeploymentOptions{ + ImagePullSecrets: []corev1.LocalObjectReference{{Name: "regcred"}}, + }) + + assert.DeepEqual(subT, deployment.Spec.Template.Spec.ImagePullSecrets, + []corev1.LocalObjectReference{{Name: "regcred"}}, + ) + }) } func TestPodDisruptionBudgetName(t *testing.T) { diff --git a/pkg/reconcilers/deployment.go b/pkg/reconcilers/deployment.go index 7bddf8ff..863ef446 100644 --- a/pkg/reconcilers/deployment.go +++ b/pkg/reconcilers/deployment.go @@ -4,8 +4,11 @@ import ( "fmt" "reflect" + "github.com/google/go-cmp/cmp" appsv1 "k8s.io/api/apps/v1" "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kuadrant/limitador-operator/pkg/log" ) // DeploymentMutateFn is a function which mutates the existing Deployment into it's desired state. @@ -189,3 +192,20 @@ func DeploymentReadinessProbeMutator(desired, existing *appsv1.Deployment) bool return update } + +func DeploymentImagePullSecretsMutator(desired, existing *appsv1.Deployment) bool { + logger := log.Log + + if cmp.Equal(desired.Spec.Template.Spec.ImagePullSecrets, existing.Spec.Template.Spec.ImagePullSecrets) { + return false + } + + if logger.V(1).Enabled() { + diff := cmp.Diff(desired.Spec.Template.Spec.ImagePullSecrets, existing.Spec.Template.Spec.ImagePullSecrets) + logger.V(1).Info("imagepullsecrets not equal", "difference", diff) + } + + // indeed, update the deployment + existing.Spec.Template.Spec.ImagePullSecrets = desired.Spec.Template.Spec.ImagePullSecrets + return true +}