From 71778b2feafa68d57d0a484163966360ebf1db37 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 | 5 +++- 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 +++++++++++++------ pkg/limitador/deployment_options.go | 8 +++-- pkg/limitador/k8s_objects.go | 3 +- pkg/reconcilers/deployment.go | 20 +++++++++++++ 9 files changed, 88 insertions(+), 15 deletions(-) diff --git a/Makefile b/Makefile index 28bf0081..a1e0ccbe 100644 --- a/Makefile +++ b/Makefile @@ -255,9 +255,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/pkg/limitador/deployment_options.go b/pkg/limitador/deployment_options.go index 07427a86..c8a9f66c 100644 --- a/pkg/limitador/deployment_options.go +++ b/pkg/limitador/deployment_options.go @@ -7,6 +7,7 @@ import ( "strings" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" @@ -14,10 +15,11 @@ import ( 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 { 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/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 +}