From 10e5af2d4b289206f0b90613f6aed3af36c80217 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Mon, 2 Jun 2025 11:49:56 -0400 Subject: [PATCH 1/8] Support serviceaccount pull secrets Serviceaccounts reference pull secrets! * Determine our serviceaccount (via the new internal/shared/util/sa package). * Use a common pull_secret_controller * Update the pull_secret_controller to know about the service account * Update the pull_secret_controller to watch the namespace-local secrets * Update caching to include sa, and use filters for additional secrets * Add RBAC to access these secrets and sa * Update writing the auth.json file to handle dockercfg and dockerconfigjson * Update writing the auth.json file to include multiple secrets Signed-off-by: Todd Short --- cmd/catalogd/main.go | 65 +++-- cmd/operator-controller/main.go | 65 +++-- config/base/catalogd/rbac/role.yaml | 23 ++ config/base/catalogd/rbac/role_binding.yaml | 17 ++ .../base/operator-controller/rbac/role.yaml | 8 + go.mod | 1 + go.sum | 2 + .../core/clustercatalog_controller.go | 2 + .../core/pull_secret_controller.go | 111 --------- .../clusterextension_controller.go | 1 + .../controllers/pull_secret_controller.go | 111 --------- .../pull_secret_controller_test.go | 98 -------- .../controllers/pull_secret_controller.go | 228 ++++++++++++++++++ .../pull_secret_controller_test.go | 30 ++- internal/shared/util/sa/serviceaccount.go | 51 ++++ .../shared/util/sa/serviceaccount_test.go | 49 ++++ 16 files changed, 493 insertions(+), 369 deletions(-) delete mode 100644 internal/catalogd/controllers/core/pull_secret_controller.go delete mode 100644 internal/operator-controller/controllers/pull_secret_controller.go delete mode 100644 internal/operator-controller/controllers/pull_secret_controller_test.go create mode 100644 internal/shared/controllers/pull_secret_controller.go rename internal/{catalogd/controllers/core => shared/controllers}/pull_secret_controller_test.go (73%) create mode 100644 internal/shared/util/sa/serviceaccount.go create mode 100644 internal/shared/util/sa/serviceaccount_test.go diff --git a/cmd/catalogd/main.go b/cmd/catalogd/main.go index 9499a7006f..bfa42a26c5 100644 --- a/cmd/catalogd/main.go +++ b/cmd/catalogd/main.go @@ -61,8 +61,10 @@ import ( "github.com/operator-framework/operator-controller/internal/catalogd/serverutil" "github.com/operator-framework/operator-controller/internal/catalogd/storage" "github.com/operator-framework/operator-controller/internal/catalogd/webhook" + sharedcontrollers "github.com/operator-framework/operator-controller/internal/shared/controllers" fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs" imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" + sautil "github.com/operator-framework/operator-controller/internal/shared/util/sa" "github.com/operator-framework/operator-controller/internal/shared/version" ) @@ -246,18 +248,40 @@ func run(ctx context.Context) error { cacheOptions := crcache.Options{ ByObject: map[client.Object]crcache.ByObject{}, } - if cfg.globalPullSecretKey != nil { - cacheOptions.ByObject[&corev1.Secret{}] = crcache.ByObject{ - Namespaces: map[string]crcache.Config{ - cfg.globalPullSecretKey.Namespace: { - LabelSelector: k8slabels.Everything(), - FieldSelector: fields.SelectorFromSet(map[string]string{ - "metadata.name": cfg.globalPullSecretKey.Name, - }), - }, + + saKey, err := sautil.GetServiceAccount() + if err != nil { + setupLog.Error(err, "Unable to get pod namesapce and serviceaccount") + return err + } + + setupLog.Info("Read token", "serviceaccount", saKey) + cacheOptions.ByObject[&corev1.ServiceAccount{}] = crcache.ByObject{ + Namespaces: map[string]crcache.Config{ + saKey.Namespace: { + LabelSelector: k8slabels.Everything(), + FieldSelector: fields.SelectorFromSet(map[string]string{ + "metadata.name": saKey.Name, + }), }, + }, + } + + secretCache := crcache.ByObject{} + secretCache.Namespaces = make(map[string]crcache.Config, 2) + secretCache.Namespaces[saKey.Namespace] = crcache.Config{ + LabelSelector: k8slabels.Everything(), + FieldSelector: fields.Everything(), + } + if cfg.globalPullSecretKey != nil { + secretCache.Namespaces[cfg.globalPullSecretKey.Namespace] = crcache.Config{ + LabelSelector: k8slabels.Everything(), + FieldSelector: fields.SelectorFromSet(map[string]string{ + "metadata.name": cfg.globalPullSecretKey.Name, + }), } } + cacheOptions.ByObject[&corev1.Secret{}] = secretCache // Create manager mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ @@ -312,7 +336,7 @@ func run(ctx context.Context) error { DockerCertPath: cfg.pullCasDir, OCICertPath: cfg.pullCasDir, } - if _, err := os.Stat(authFilePath); err == nil && cfg.globalPullSecretKey != nil { + if _, err := os.Stat(authFilePath); err == nil { logger.Info("using available authentication information for pulling image") srcContext.AuthFilePath = authFilePath } else if os.IsNotExist(err) { @@ -370,17 +394,16 @@ func run(ctx context.Context) error { return err } - if cfg.globalPullSecretKey != nil { - setupLog.Info("creating SecretSyncer controller for watching secret", "Secret", cfg.globalPullSecret) - err := (&corecontrollers.PullSecretReconciler{ - Client: mgr.GetClient(), - AuthFilePath: authFilePath, - SecretKey: *cfg.globalPullSecretKey, - }).SetupWithManager(mgr) - if err != nil { - setupLog.Error(err, "unable to create controller", "controller", "SecretSyncer") - return err - } + setupLog.Info("creating SecretSyncer controller for watching secret", "Secret", cfg.globalPullSecret) + err = (&sharedcontrollers.PullSecretReconciler{ + Client: mgr.GetClient(), + AuthFilePath: authFilePath, + SecretKey: cfg.globalPullSecretKey, + ServiceAccountKey: saKey, + }).SetupWithManager(mgr) + if err != nil { + setupLog.Error(err, "unable to create controller", "controller", "SecretSyncer") + return err } //+kubebuilder:scaffold:builder diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 6125472484..480e92d569 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -71,9 +71,11 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/certproviders" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1" "github.com/operator-framework/operator-controller/internal/operator-controller/scheme" + sharedcontrollers "github.com/operator-framework/operator-controller/internal/shared/controllers" fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs" httputil "github.com/operator-framework/operator-controller/internal/shared/util/http" imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" + sautil "github.com/operator-framework/operator-controller/internal/shared/util/sa" "github.com/operator-framework/operator-controller/internal/shared/version" ) @@ -217,18 +219,40 @@ func run() error { }, DefaultLabelSelector: k8slabels.Nothing(), } - if globalPullSecretKey != nil { - cacheOptions.ByObject[&corev1.Secret{}] = crcache.ByObject{ - Namespaces: map[string]crcache.Config{ - globalPullSecretKey.Namespace: { - LabelSelector: k8slabels.Everything(), - FieldSelector: fields.SelectorFromSet(map[string]string{ - "metadata.name": globalPullSecretKey.Name, - }), - }, + + saKey, err := sautil.GetServiceAccount() + if err != nil { + setupLog.Error(err, "Unable to get pod namesapce and serviceaccount") + return err + } + + setupLog.Info("Read token", "serviceaccount", saKey) + cacheOptions.ByObject[&corev1.ServiceAccount{}] = crcache.ByObject{ + Namespaces: map[string]crcache.Config{ + saKey.Namespace: { + LabelSelector: k8slabels.Everything(), + FieldSelector: fields.SelectorFromSet(map[string]string{ + "metadata.name": saKey.Name, + }), }, + }, + } + + secretCache := crcache.ByObject{} + secretCache.Namespaces = make(map[string]crcache.Config, 2) + secretCache.Namespaces[saKey.Namespace] = crcache.Config{ + LabelSelector: k8slabels.Everything(), + FieldSelector: fields.Everything(), + } + if globalPullSecretKey != nil { + secretCache.Namespaces[globalPullSecretKey.Namespace] = crcache.Config{ + LabelSelector: k8slabels.Everything(), + FieldSelector: fields.SelectorFromSet(map[string]string{ + "metadata.name": globalPullSecretKey.Name, + }), } } + cacheOptions.ByObject[&corev1.Secret{}] = secretCache metricsServerOptions := server.Options{} if len(cfg.certFile) > 0 && len(cfg.keyFile) > 0 { @@ -360,7 +384,7 @@ func run() error { OCICertPath: cfg.pullCasDir, } logger := log.FromContext(ctx) - if _, err := os.Stat(authFilePath); err == nil && globalPullSecretKey != nil { + if _, err := os.Stat(authFilePath); err == nil { logger.Info("using available authentication information for pulling image") srcContext.AuthFilePath = authFilePath } else if os.IsNotExist(err) { @@ -482,17 +506,16 @@ func run() error { return err } - if globalPullSecretKey != nil { - setupLog.Info("creating SecretSyncer controller for watching secret", "Secret", cfg.globalPullSecret) - err := (&controllers.PullSecretReconciler{ - Client: mgr.GetClient(), - AuthFilePath: authFilePath, - SecretKey: *globalPullSecretKey, - }).SetupWithManager(mgr) - if err != nil { - setupLog.Error(err, "unable to create controller", "controller", "SecretSyncer") - return err - } + setupLog.Info("creating SecretSyncer controller for watching secret", "Secret", cfg.globalPullSecret) + err = (&sharedcontrollers.PullSecretReconciler{ + Client: mgr.GetClient(), + AuthFilePath: authFilePath, + SecretKey: globalPullSecretKey, + ServiceAccountKey: saKey, + }).SetupWithManager(mgr) + if err != nil { + setupLog.Error(err, "unable to create controller", "controller", "SecretSyncer") + return err } //+kubebuilder:scaffold:builder diff --git a/config/base/catalogd/rbac/role.yaml b/config/base/catalogd/rbac/role.yaml index 40f4095c62..ec3fe89a6a 100644 --- a/config/base/catalogd/rbac/role.yaml +++ b/config/base/catalogd/rbac/role.yaml @@ -4,6 +4,14 @@ kind: ClusterRole metadata: name: manager-role rules: +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch - apiGroups: - olm.operatorframework.io resources: @@ -30,3 +38,18 @@ rules: - get - patch - update +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: manager-role + namespace: system +rules: +- apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - get + - list + - watch diff --git a/config/base/catalogd/rbac/role_binding.yaml b/config/base/catalogd/rbac/role_binding.yaml index a618c0e474..41dc229bc7 100644 --- a/config/base/catalogd/rbac/role_binding.yaml +++ b/config/base/catalogd/rbac/role_binding.yaml @@ -13,3 +13,20 @@ subjects: - kind: ServiceAccount name: controller-manager namespace: system +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + labels: + app.kubernetes.io/part-of: olm + app.kubernetes.io/name: catalogd + name: manager-rolebinding + namespace: system +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: manager-role +subjects: + - kind: ServiceAccount + name: controller-manager + namespace: system diff --git a/config/base/operator-controller/rbac/role.yaml b/config/base/operator-controller/rbac/role.yaml index be89deec1d..d18eb4c6c7 100644 --- a/config/base/operator-controller/rbac/role.yaml +++ b/config/base/operator-controller/rbac/role.yaml @@ -77,3 +77,11 @@ rules: - patch - update - watch +- apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - get + - list + - watch diff --git a/go.mod b/go.mod index 6cbe9dfef9..ece8105b18 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/containers/image/v5 v5.35.0 github.com/fsnotify/fsnotify v1.9.0 github.com/go-logr/logr v1.4.3 + github.com/golang-jwt/jwt/v5 v5.2.2 github.com/google/go-cmp v0.7.0 github.com/google/go-containerregistry v0.20.3 github.com/gorilla/handlers v1.5.2 diff --git a/go.sum b/go.sum index bdfddc1814..ab8a4cb0d6 100644 --- a/go.sum +++ b/go.sum @@ -209,6 +209,8 @@ github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJA github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= +github.com/golang-jwt/jwt/v5 v5.2.2 h1:Rl4B7itRWVtYIHFrSNd7vhTiz9UpLdi6gZhZ3wEeDy8= +github.com/golang-jwt/jwt/v5 v5.2.2/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang-migrate/migrate/v4 v4.18.3 h1:EYGkoOsvgHHfm5U/naS1RP/6PL/Xv3S4B/swMiAmDLs= github.com/golang-migrate/migrate/v4 v4.18.3/go.mod h1:99BKpIi6ruaaXRM1A77eqZ+FWPQ3cfRa+ZVy5bmWMaY= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= diff --git a/internal/catalogd/controllers/core/clustercatalog_controller.go b/internal/catalogd/controllers/core/clustercatalog_controller.go index ce16362660..12a67dc0fe 100644 --- a/internal/catalogd/controllers/core/clustercatalog_controller.go +++ b/internal/catalogd/controllers/core/clustercatalog_controller.go @@ -79,6 +79,8 @@ type storedCatalogData struct { //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs/status,verbs=get;update;patch //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs/finalizers,verbs=update +//+kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch +//+kubebuilder:rbac:namespace=system,groups=core,resources=serviceaccounts,verbs=get;list;watch // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. diff --git a/internal/catalogd/controllers/core/pull_secret_controller.go b/internal/catalogd/controllers/core/pull_secret_controller.go deleted file mode 100644 index 8105810474..0000000000 --- a/internal/catalogd/controllers/core/pull_secret_controller.go +++ /dev/null @@ -1,111 +0,0 @@ -/* -Copyright 2024. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package core - -import ( - "context" - "fmt" - "os" - - "github.com/go-logr/logr" - corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/types" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/predicate" -) - -// PullSecretReconciler reconciles a specific Secret object -// that contains global pull secrets for pulling Catalog images -type PullSecretReconciler struct { - client.Client - SecretKey types.NamespacedName - AuthFilePath string -} - -func (r *PullSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - logger := log.FromContext(ctx) - if req.Name != r.SecretKey.Name || req.Namespace != r.SecretKey.Namespace { - logger.Error(fmt.Errorf("received unexpected request for Secret %v/%v", req.Namespace, req.Name), "reconciliation error") - return ctrl.Result{}, nil - } - - secret := &corev1.Secret{} - err := r.Get(ctx, req.NamespacedName, secret) - if err != nil { - if apierrors.IsNotFound(err) { - logger.Info("secret not found") - return r.deleteSecretFile(logger) - } - logger.Error(err, "failed to get Secret") - return ctrl.Result{}, err - } - - return r.writeSecretToFile(logger, secret) -} - -// SetupWithManager sets up the controller with the Manager. -func (r *PullSecretReconciler) SetupWithManager(mgr ctrl.Manager) error { - _, err := ctrl.NewControllerManagedBy(mgr). - For(&corev1.Secret{}). - Named("catalogd-pull-secret-controller"). - WithEventFilter(newSecretPredicate(r.SecretKey)). - Build(r) - - return err -} - -func newSecretPredicate(key types.NamespacedName) predicate.Predicate { - return predicate.NewPredicateFuncs(func(obj client.Object) bool { - return obj.GetName() == key.Name && obj.GetNamespace() == key.Namespace - }) -} - -// writeSecretToFile writes the secret data to the specified file -func (r *PullSecretReconciler) writeSecretToFile(logger logr.Logger, secret *corev1.Secret) (ctrl.Result, error) { - // image registry secrets are always stored with the key .dockerconfigjson - // ref: https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/#registry-secret-existing-credentials - dockerConfigJSON, ok := secret.Data[".dockerconfigjson"] - if !ok { - logger.Error(fmt.Errorf("expected secret.Data key not found"), "expected secret Data to contain key .dockerconfigjson") - return ctrl.Result{}, nil - } - // expected format for auth.json - // https://github.com/containers/image/blob/main/docs/containers-auth.json.5.md - err := os.WriteFile(r.AuthFilePath, dockerConfigJSON, 0600) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to write secret data to file: %w", err) - } - logger.Info("saved global pull secret data locally") - return ctrl.Result{}, nil -} - -// deleteSecretFile deletes the auth file if the secret is deleted -func (r *PullSecretReconciler) deleteSecretFile(logger logr.Logger) (ctrl.Result, error) { - logger.Info("deleting local auth file", "file", r.AuthFilePath) - if err := os.Remove(r.AuthFilePath); err != nil { - if os.IsNotExist(err) { - logger.Info("auth file does not exist, nothing to delete") - return ctrl.Result{}, nil - } - return ctrl.Result{}, fmt.Errorf("failed to delete secret file: %w", err) - } - logger.Info("auth file deleted successfully") - return ctrl.Result{}, nil -} diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index 9a79e8c75b..7d268df05a 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -95,6 +95,7 @@ type InstalledBundleGetter interface { //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions/finalizers,verbs=update //+kubebuilder:rbac:namespace=system,groups=core,resources=secrets,verbs=create;update;patch;delete;deletecollection;get;list;watch //+kubebuilder:rbac:groups=core,resources=serviceaccounts/token,verbs=create +//+kubebuilder:rbac:namespace=system,groups=core,resources=serviceaccounts,verbs=get;list;watch //+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles;clusterrolebindings;roles;rolebindings,verbs=list;watch diff --git a/internal/operator-controller/controllers/pull_secret_controller.go b/internal/operator-controller/controllers/pull_secret_controller.go deleted file mode 100644 index d73ccddb3b..0000000000 --- a/internal/operator-controller/controllers/pull_secret_controller.go +++ /dev/null @@ -1,111 +0,0 @@ -/* -Copyright 2024. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package controllers - -import ( - "context" - "fmt" - "os" - - "github.com/go-logr/logr" - corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/types" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/predicate" -) - -// PullSecretReconciler reconciles a specific Secret object -// that contains global pull secrets for pulling bundle images -type PullSecretReconciler struct { - client.Client - SecretKey types.NamespacedName - AuthFilePath string -} - -func (r *PullSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - logger := log.FromContext(ctx) - if req.Name != r.SecretKey.Name || req.Namespace != r.SecretKey.Namespace { - logger.Error(fmt.Errorf("received unexpected request for Secret %v/%v", req.Namespace, req.Name), "reconciliation error") - return ctrl.Result{}, nil - } - - secret := &corev1.Secret{} - err := r.Get(ctx, req.NamespacedName, secret) - if err != nil { - if apierrors.IsNotFound(err) { - logger.Info("secret not found") - return r.deleteSecretFile(logger) - } - logger.Error(err, "failed to get Secret") - return ctrl.Result{}, err - } - - return r.writeSecretToFile(logger, secret) -} - -// SetupWithManager sets up the controller with the Manager. -func (r *PullSecretReconciler) SetupWithManager(mgr ctrl.Manager) error { - _, err := ctrl.NewControllerManagedBy(mgr). - Named("controller-operator-pull-secret-controller"). - For(&corev1.Secret{}). - WithEventFilter(newSecretPredicate(r.SecretKey)). - Build(r) - - return err -} - -func newSecretPredicate(key types.NamespacedName) predicate.Predicate { - return predicate.NewPredicateFuncs(func(obj client.Object) bool { - return obj.GetName() == key.Name && obj.GetNamespace() == key.Namespace - }) -} - -// writeSecretToFile writes the secret data to the specified file -func (r *PullSecretReconciler) writeSecretToFile(logger logr.Logger, secret *corev1.Secret) (ctrl.Result, error) { - // image registry secrets are always stored with the key .dockerconfigjson - // ref: https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/#registry-secret-existing-credentials - dockerConfigJSON, ok := secret.Data[".dockerconfigjson"] - if !ok { - logger.Error(fmt.Errorf("expected secret.Data key not found"), "expected secret Data to contain key .dockerconfigjson") - return ctrl.Result{}, nil - } - // expected format for auth.json - // https://github.com/containers/image/blob/main/docs/containers-auth.json.5.md - err := os.WriteFile(r.AuthFilePath, dockerConfigJSON, 0600) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to write secret data to file: %w", err) - } - logger.Info("saved global pull secret data locally") - return ctrl.Result{}, nil -} - -// deleteSecretFile deletes the auth file if the secret is deleted -func (r *PullSecretReconciler) deleteSecretFile(logger logr.Logger) (ctrl.Result, error) { - logger.Info("deleting local auth file", "file", r.AuthFilePath) - if err := os.Remove(r.AuthFilePath); err != nil { - if os.IsNotExist(err) { - logger.Info("auth file does not exist, nothing to delete") - return ctrl.Result{}, nil - } - return ctrl.Result{}, fmt.Errorf("failed to delete secret file: %w", err) - } - logger.Info("auth file deleted successfully") - return ctrl.Result{}, nil -} diff --git a/internal/operator-controller/controllers/pull_secret_controller_test.go b/internal/operator-controller/controllers/pull_secret_controller_test.go deleted file mode 100644 index 4406ffa2f6..0000000000 --- a/internal/operator-controller/controllers/pull_secret_controller_test.go +++ /dev/null @@ -1,98 +0,0 @@ -package controllers_test - -import ( - "context" - "os" - "path/filepath" - "testing" - - "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - - "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" - "github.com/operator-framework/operator-controller/internal/operator-controller/scheme" -) - -func TestSecretSyncerReconciler(t *testing.T) { - secretData := []byte(`{"auths":{"exampleRegistry": "exampledata"}}`) - authFileName := "test-auth.json" - for _, tt := range []struct { - name string - secret *corev1.Secret - addSecret bool - wantErr string - fileShouldExistBefore bool - fileShouldExistAfter bool - }{ - { - name: "secret exists, content gets saved to authFile", - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-secret", - Namespace: "test-secret-namespace", - }, - Data: map[string][]byte{ - ".dockerconfigjson": secretData, - }, - }, - addSecret: true, - fileShouldExistBefore: false, - fileShouldExistAfter: true, - }, - { - name: "secret does not exist, file exists previously, file should get deleted", - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-secret", - Namespace: "test-secret-namespace", - }, - Data: map[string][]byte{ - ".dockerconfigjson": secretData, - }, - }, - addSecret: false, - fileShouldExistBefore: true, - fileShouldExistAfter: false, - }, - } { - t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - tempAuthFile := filepath.Join(t.TempDir(), authFileName) - clientBuilder := fake.NewClientBuilder().WithScheme(scheme.Scheme) - if tt.addSecret { - clientBuilder = clientBuilder.WithObjects(tt.secret) - } - cl := clientBuilder.Build() - - secretKey := types.NamespacedName{Namespace: tt.secret.Namespace, Name: tt.secret.Name} - r := &controllers.PullSecretReconciler{ - Client: cl, - SecretKey: secretKey, - AuthFilePath: tempAuthFile, - } - if tt.fileShouldExistBefore { - err := os.WriteFile(tempAuthFile, secretData, 0600) - require.NoError(t, err) - } - res, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: secretKey}) - if tt.wantErr == "" { - require.NoError(t, err) - } else { - require.ErrorContains(t, err, tt.wantErr) - } - require.Equal(t, ctrl.Result{}, res) - - if tt.fileShouldExistAfter { - _, err := os.Stat(tempAuthFile) - require.NoError(t, err) - } else { - _, err := os.Stat(tempAuthFile) - require.True(t, os.IsNotExist(err)) - } - }) - } -} diff --git a/internal/shared/controllers/pull_secret_controller.go b/internal/shared/controllers/pull_secret_controller.go new file mode 100644 index 0000000000..8bf2bdd799 --- /dev/null +++ b/internal/shared/controllers/pull_secret_controller.go @@ -0,0 +1,228 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "encoding/json" + "fmt" + "os" + + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +// PullSecretReconciler reconciles a specific Secret object +// that contains global pull secrets for pulling Catalog images +type PullSecretReconciler struct { + client.Client + SecretKey *types.NamespacedName + ServiceAccountKey types.NamespacedName + ServiceAccountPullSecrets []types.NamespacedName + AuthFilePath string +} + +func (r *PullSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + logger := log.FromContext(ctx).WithName("pull-secret-reconciler") + + logger.Info("processing event", "name", req.NamespacedName) + defer logger.Info("processed event", "name", req.NamespacedName) + + secrets := []*corev1.Secret{} + secret := &corev1.Secret{} + + if r.SecretKey != nil { //nolint:nestif + // Add the configured pull secret to the list of secrets + if err := r.Get(ctx, *r.SecretKey, secret); err != nil { + if apierrors.IsNotFound(err) { + logger.Info("secret not found", "name", r.SecretKey) + } else { + logger.Error(err, "failed to get Secret", "name", r.SecretKey) + return ctrl.Result{}, err + } + } else { + logger.Info("global pull secret", "name", *r.SecretKey) + secrets = append(secrets, secret) + } + } + + // Grab all the pull secrets from the serviceaccount and add them to the list of secrets + sa := &corev1.ServiceAccount{} + logger.Info("serviceaccount", "name", r.ServiceAccountKey) + if err := r.Get(ctx, r.ServiceAccountKey, sa); err != nil { //nolint:nestif + if apierrors.IsNotFound(err) { + logger.Info("serviceaccount not found", "serviceaccount", r.ServiceAccountKey) + } else { + logger.Error(err, "failed to get serviceaccount", "serviceaccount", r.ServiceAccountKey) + return ctrl.Result{}, err + } + } else { + nn := types.NamespacedName{Namespace: r.ServiceAccountKey.Namespace} + pullSecrets := []types.NamespacedName{} + for _, ips := range sa.ImagePullSecrets { + nn.Name = ips.Name + // This is to update the list of secrets that we are filtering on + // Add all secrets regardless if they exist or not + pullSecrets = append(pullSecrets, nn) + secret := &corev1.Secret{} + err = r.Get(ctx, nn, secret) + if err != nil { + if apierrors.IsNotFound(err) { + logger.Info("serviceaccount pull secret not found", "secret", nn) + } else { + logger.Error(err, "failed to get serviceaccount pull secret", "secret", nn) + return ctrl.Result{}, err + } + } else { + secrets = append(secrets, secret) + } + } + // update list of pull secrets from service account + logger.Info("updating list of pull secrets", "list", pullSecrets) + r.ServiceAccountPullSecrets = pullSecrets + } + + if len(secrets) == 0 { + return ctrl.Result{}, r.deleteSecretFile(logger) + } + return ctrl.Result{}, r.writeSecretToFile(logger, secrets) +} + +// SetupWithManager sets up the controller with the Manager. +func (r *PullSecretReconciler) SetupWithManager(mgr ctrl.Manager) error { + _, err := ctrl.NewControllerManagedBy(mgr). + For(&corev1.Secret{}). + Named("pull-secret-controller"). + WithEventFilter(newSecretPredicate(r)). + Build(r) + if err != nil { + return err + } + + _, err = ctrl.NewControllerManagedBy(mgr). + For(&corev1.ServiceAccount{}). + Named("service-account-controller"). + WithEventFilter(newNamespacedPredicate(r.ServiceAccountKey)). + Build(r) + + return err +} + +// Filters based on the global SecretKey, or any pull secret from the serviceaccount +func newSecretPredicate(r *PullSecretReconciler) predicate.Predicate { + return predicate.NewPredicateFuncs(func(obj client.Object) bool { + nn := types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()} + if r.SecretKey != nil && nn == *r.SecretKey { + return true + } + for _, ps := range r.ServiceAccountPullSecrets { + if nn == ps { + return true + } + } + return false + }) +} + +func newNamespacedPredicate(key types.NamespacedName) predicate.Predicate { + return predicate.NewPredicateFuncs(func(obj client.Object) bool { + return obj.GetName() == key.Name && obj.GetNamespace() == key.Namespace + }) +} + +// Golang representation of the docker configuration - either dockerconfigjson or dockercfg formats. +// This allows us to merge the two formats together, regardless of type, and dump it out as a +// dockerconfigjson for use my contaners/images +type dockerConfigJSON struct { + Auths dockerCfg `json:"auths"` +} + +type dockerCfg map[string]authEntries + +type authEntries struct { + Auth string `json:"auth"` + Email string `json:"email,omitempty"` +} + +// writeSecretToFile writes the secret data to the specified file +func (r *PullSecretReconciler) writeSecretToFile(logger logr.Logger, secrets []*corev1.Secret) error { + // image registry secrets are always stored with the key .dockerconfigjson or .dockercfg + // ref: https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/#registry-secret-existing-credentials + // expected format for auth.json + // ref: https://github.com/containers/image/blob/main/docs/containers-auth.json.5.md + + jsonData := dockerConfigJSON{} + jsonData.Auths = make(dockerCfg) + + for _, s := range secrets { + if secretData, ok := s.Data[".dockerconfigjson"]; ok { + // process as dockerconfigjson + dcj := &dockerConfigJSON{} + if err := json.Unmarshal(secretData, dcj); err != nil { + return err + } + for n, v := range dcj.Auths { + jsonData.Auths[n] = v + } + continue + } + if secretData, ok := s.Data[".dockercfg"]; ok { + // process as dockercfg, despite being a map, this has to be Unmarshal'd as a pointer + dc := &dockerCfg{} + if err := json.Unmarshal(secretData, dc); err != nil { + return err + } + for n, v := range *dc { + jsonData.Auths[n] = v + } + continue + } + // Ignore the unknown secret + logger.Info("expected secret.Data key not found", "secret", types.NamespacedName{Name: s.Name, Namespace: s.Namespace}) + } + + data, err := json.Marshal(jsonData) + if err != nil { + return fmt.Errorf("failed to marshal secret data: %w", err) + } + err = os.WriteFile(r.AuthFilePath, data, 0600) + if err != nil { + return fmt.Errorf("failed to write secret data to file: %w", err) + } + logger.Info("saved global pull secret data locally") + return nil +} + +// deleteSecretFile deletes the auth file if the secret is deleted +func (r *PullSecretReconciler) deleteSecretFile(logger logr.Logger) error { + logger.Info("deleting local auth file", "file", r.AuthFilePath) + if err := os.Remove(r.AuthFilePath); err != nil { + if os.IsNotExist(err) { + logger.Info("auth file does not exist, nothing to delete") + return nil + } + return fmt.Errorf("failed to delete secret file: %w", err) + } + logger.Info("auth file deleted successfully") + return nil +} diff --git a/internal/catalogd/controllers/core/pull_secret_controller_test.go b/internal/shared/controllers/pull_secret_controller_test.go similarity index 73% rename from internal/catalogd/controllers/core/pull_secret_controller_test.go rename to internal/shared/controllers/pull_secret_controller_test.go index 8b91da340f..03f46c0078 100644 --- a/internal/catalogd/controllers/core/pull_secret_controller_test.go +++ b/internal/shared/controllers/pull_secret_controller_test.go @@ -1,4 +1,4 @@ -package core +package controllers import ( "context" @@ -15,7 +15,8 @@ import ( ) func TestSecretSyncerReconciler(t *testing.T) { - secretData := []byte(`{"auths":{"exampleRegistry": "exampledata"}}`) + secretFullData := []byte(`{"auths":{"exampleRegistry": {"auth": "exampledata"}}}`) + secretPartData := []byte(`{"exampleRegistry": {"auth": "exampledata"}}`) authFileName := "test-auth.json" for _, tt := range []struct { name string @@ -26,14 +27,29 @@ func TestSecretSyncerReconciler(t *testing.T) { fileShouldExistAfter bool }{ { - name: "secret exists, content gets saved to authFile", + name: "secret exists, dockerconfigjson content gets saved to authFile", secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "test-secret", Namespace: "test-secret-namespace", }, Data: map[string][]byte{ - ".dockerconfigjson": secretData, + ".dockerconfigjson": secretFullData, + }, + }, + addSecret: true, + fileShouldExistBefore: false, + fileShouldExistAfter: true, + }, + { + name: "secret exists, dockercfg content gets saved to authFile", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-secret-namespace", + }, + Data: map[string][]byte{ + ".dockercfg": secretPartData, }, }, addSecret: true, @@ -48,7 +64,7 @@ func TestSecretSyncerReconciler(t *testing.T) { Namespace: "test-secret-namespace", }, Data: map[string][]byte{ - ".dockerconfigjson": secretData, + ".dockerconfigjson": secretFullData, }, }, addSecret: false, @@ -68,11 +84,11 @@ func TestSecretSyncerReconciler(t *testing.T) { secretKey := types.NamespacedName{Namespace: tt.secret.Namespace, Name: tt.secret.Name} r := &PullSecretReconciler{ Client: cl, - SecretKey: secretKey, + SecretKey: &secretKey, AuthFilePath: tempAuthFile, } if tt.fileShouldExistBefore { - err := os.WriteFile(tempAuthFile, secretData, 0600) + err := os.WriteFile(tempAuthFile, secretFullData, 0600) require.NoError(t, err) } res, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: secretKey}) diff --git a/internal/shared/util/sa/serviceaccount.go b/internal/shared/util/sa/serviceaccount.go new file mode 100644 index 0000000000..0b275f498e --- /dev/null +++ b/internal/shared/util/sa/serviceaccount.go @@ -0,0 +1,51 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sa + +import ( + "fmt" + "os" + "strings" + + "github.com/golang-jwt/jwt/v5" + k8stypes "k8s.io/apimachinery/pkg/types" +) + +// Returns nameaspce/serviceaccount name +func GetServiceAccount() (k8stypes.NamespacedName, error) { + return getServiceAccountInternal(os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/token")) +} + +func getServiceAccountInternal(data []byte, err error) (k8stypes.NamespacedName, error) { + if err != nil { + return k8stypes.NamespacedName{}, err + } + // Not verifying the token, we just want to extract the subject + token, _, err := jwt.NewParser([]jwt.ParserOption{}...).ParseUnverified(string(data), jwt.MapClaims{}) + if err != nil { + return k8stypes.NamespacedName{}, err + } + subject, err := token.Claims.GetSubject() + if err != nil { + return k8stypes.NamespacedName{}, err + } + subjects := strings.Split(subject, ":") + if len(subjects) != 4 { + return k8stypes.NamespacedName{}, fmt.Errorf("badly formatted subject: %s", subject) + } + return k8stypes.NamespacedName{Namespace: subjects[2], Name: subjects[3]}, nil +} diff --git a/internal/shared/util/sa/serviceaccount_test.go b/internal/shared/util/sa/serviceaccount_test.go new file mode 100644 index 0000000000..b18663e662 --- /dev/null +++ b/internal/shared/util/sa/serviceaccount_test.go @@ -0,0 +1,49 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sa + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" +) + +const ( + // taken from a kind run + goodSa = "eyJhbGciOiJSUzI1NiIsImtpZCI6IjdyM3VIbkJ0VlRQVy1uWWlsSVFCV2pfQmdTS0RIdjZHNDBVT1hDSVFtZmcifQ.eyJhdWQiOlsiaHR0cHM6Ly9rdWJlcm5ldGVzLmRlZmF1bHQuc3ZjLmNsdXN0ZXIubG9jYWwiXSwiZXhwIjoxNzgwNTEwMjAwLCJpYXQiOjE3NDg5NzQyMDAsImlzcyI6Imh0dHBzOi8va3ViZXJuZXRlcy5kZWZhdWx0LnN2Yy5jbHVzdGVyLmxvY2FsIiwianRpIjoiNTQ2OThmZGYtNzg4NC00YzhkLWI5NzctYTg4YThiYmY3ODQxIiwia3ViZXJuZXRlcy5pbyI6eyJuYW1lc3BhY2UiOiJvbG12MS1zeXN0ZW0iLCJub2RlIjp7Im5hbWUiOiJvcGVyYXRvci1jb250cm9sbGVyLWUyZS1jb250cm9sLXBsYW5lIiwidWlkIjoiZWY0YjdkNGQtZmUxZi00MThkLWIyZDAtM2ZmYWJmMWQ0ZDI3In0sInBvZCI6eyJuYW1lIjoib3BlcmF0b3ItY29udHJvbGxlci1jb250cm9sbGVyLW1hbmFnZXItNjU3Njg1ZGNkYy01cTZ0dCIsInVpZCI6IjE4MmFkNTkxLWUzYTktNDMyNC1hMjk4LTg0NzIxY2Q0OTAzYSJ9LCJzZXJ2aWNlYWNjb3VudCI6eyJuYW1lIjoib3BlcmF0b3ItY29udHJvbGxlci1jb250cm9sbGVyLW1hbmFnZXIiLCJ1aWQiOiI3MDliZTA4OS00OTI1LTQ2NjYtYjA1Ny1iYWMyNmVmYWJjMGIifSwid2FybmFmdGVyIjoxNzQ4OTc3ODA3fSwibmJmIjoxNzQ4OTc0MjAwLCJzdWIiOiJzeXN0ZW06c2VydmljZWFjY291bnQ6b2xtdjEtc3lzdGVtOm9wZXJhdG9yLWNvbnRyb2xsZXItY29udHJvbGxlci1tYW5hZ2VyIn0.OjExhuNHdMZjdGwDXM0bWQnJKcfLNpEJ2S47BzlAa560uNw8EwMItlfpG970umQBbVPWhyhUBFimUD5XmXWAlrNvhFwpOLXw2W978Obs1mna5JWcHliC6IkwrOMCh5k9XReQ9-KBdw36QY1G2om77-7mNtPNPg9lg5TQaLuNGrIhX9EC_tucbflXSvB-SA243J_X004W4HkJirt6vVH5FoRg-MDohXm0C4bhTeaXfOtTW6fwsnpomCKso7apu_eOG9E2h8CXXYKhZg4Jrank_Ata8J1lANh06FuxRQK-vwqFrW3_9rscGxweM5CbeicZFOc6MDIuYtgR515YTHPbUA" // notsecret + badSa1 = "eyJhbGciOiJSUzI1NiIsImtpZCI6IjdyM3VIbkJ0VlRQVy1uWWlsSVFCV2pfQmdTS0RIdjZHNDBVT1hDSVFtZmcifQ.eyJhdWQiOlsiaHR0cHM6Ly9rdWJlcm5ldGVzLmRlZmF1bHQuc3ZjLmNsdXN0ZXIubG9jYWwiXSwiZXhwIjoxNzgwNTEwMjAwLCJpYXQiOjE3NDg5NzQyMDAsImlzcyI6Imh0dHBzOi8va3ViZXJuZXRlcy5kZWZhdWx0LnN2Yy5jbHVzdGVyLmxvY2FsIiwianRpIjoiNTQ2OThmZGYtNzg4NC00YzhkLWI5NzctYTg4YThiYmY3ODQxIiwia3ViZXJuZXRlcy5pbyI6eyJuYW1lc3BhY2UiOiJvbG12MS1zeXN0ZW0iLCJub2RlIjp7Im5hbWUiOiJvcGVyYXRvci1jb250cm9sbGVyLWUyZS1jb250cm9sLXBsYW5lIiwidWlkIjoiZWY0YjdkNGQtZmUxZi00MThkLWIyZDAtM2ZmYWJmMWQ0ZDI3In0sInBvZCI6eyJuYW1lIjoib3BlcmF0b3ItY29udHJvbGxlci1jb250cm9sbGVyLW1hbmFnZXItNjU3Njg1ZGNkYy01cTZ0dCIsInVpZCI6IjE4MmFkNTkxLWUzYTktNDMyNC1hMjk4LTg0NzIxY2Q0OTAzYSJ9LCJzZXJ2aWNlYWNjb3VudCI6eyJuYW1lIjoib3BlcmF0b3ItY29udHJvbGxlci1jb250cm9sbGVyLW1hbmFnZXIiLCJ1aWQiOiI3MDliZTA4OS00OTI1LTQ2NjYtYjA1Ny1iYWMyNmVmYWJjMGIifSwid2FybmFmdGVyIjoxNzQ4OTc3ODA3fSwibmJmIjoxNzQ4OTc0MjAwLCJzdWIiOiJzeXN0ZW06c2VydmljZWFjY291bnRzOm9sbXYxLXN5c3RlbSJ9.OjExhuNHdMZjdGwDXM0bWQnJKcfLNpEJ2S47BzlAa560uNw8EwMItlfpG970umQBbVPWhyhUBFimUD5XmXWAlrNvhFwpOLXw2W978Obs1mna5JWcHliC6IkwrOMCh5k9XReQ9-KBdw36QY1G2om77-7mNtPNPg9lg5TQaLuNGrIhX9EC_tucbflXSvB-SA243J_X004W4HkJirt6vVH5FoRg-MDohXm0C4bhTeaXfOtTW6fwsnpomCKso7apu_eOG9E2h8CXXYKhZg4Jrank_Ata8J1lANh06FuxRQK-vwqFrW3_9rscGxweM5CbeicZFOc6MDIuYtgR515YTHPbUA" // notsecret + badSa2 = "eyJhbGciOiJSUzI1NiIsImtpZCI6IjdyM3VIbkJ0VlRQVy1uWWlsSVFCV2pfQmdTS0RIdjZHNDBVT1hDSVFtZmcifQ" // notsecret +) + +func TestGetServiceAccount(t *testing.T) { + nn, err := getServiceAccountInternal([]byte(goodSa), nil) + require.NoError(t, err) + require.Equal(t, "olmv1-system", nn.Namespace) + require.Equal(t, "operator-controller-controller-manager", nn.Name) + + _, err = getServiceAccountInternal([]byte{}, fmt.Errorf("this is a test error")) + require.ErrorContains(t, err, "this is a test") + + // Modified the subject to be invalid + _, err = getServiceAccountInternal([]byte(badSa1), nil) + require.ErrorContains(t, err, "badly formatted subject") + + // Only includes a header + _, err = getServiceAccountInternal([]byte(badSa2), nil) + require.ErrorContains(t, err, "token is malformed") +} From 8ba76132f79141140fdb75ba1a3208ea85a25142 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Fri, 6 Jun 2025 13:56:20 -0400 Subject: [PATCH 2/8] fixup! Support serviceaccount pull secrets Signed-off-by: Todd Short --- config/base/catalogd/rbac/role.yaml | 9 +-- go.mod | 1 + go.sum | 2 + .../core/clustercatalog_controller.go | 2 +- .../controllers/pull_secret_controller.go | 59 +++++++++++-------- 5 files changed, 40 insertions(+), 33 deletions(-) diff --git a/config/base/catalogd/rbac/role.yaml b/config/base/catalogd/rbac/role.yaml index ec3fe89a6a..0b15af0c6c 100644 --- a/config/base/catalogd/rbac/role.yaml +++ b/config/base/catalogd/rbac/role.yaml @@ -4,14 +4,6 @@ kind: ClusterRole metadata: name: manager-role rules: -- apiGroups: - - "" - resources: - - secrets - verbs: - - get - - list - - watch - apiGroups: - olm.operatorframework.io resources: @@ -48,6 +40,7 @@ rules: - apiGroups: - "" resources: + - secrets - serviceaccounts verbs: - get diff --git a/go.mod b/go.mod index ece8105b18..7cb2e43a4f 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/golang-jwt/jwt/v5 v5.2.2 github.com/google/go-cmp v0.7.0 github.com/google/go-containerregistry v0.20.3 + github.com/google/renameio/v2 v2.0.0 github.com/gorilla/handlers v1.5.2 github.com/klauspost/compress v1.18.0 github.com/opencontainers/go-digest v1.0.0 diff --git a/go.sum b/go.sum index ab8a4cb0d6..22805e8582 100644 --- a/go.sum +++ b/go.sum @@ -255,6 +255,8 @@ github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0= github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/pprof v0.0.0-20250423184734-337e5dd93bb4 h1:gD0vax+4I+mAj+jEChEf25Ia07Jq7kYOFO5PPhAxFl4= github.com/google/pprof v0.0.0-20250423184734-337e5dd93bb4/go.mod h1:5hDyRhoBCxViHszMt12TnOpEI4VVi+U8Gm9iphldiMA= +github.com/google/renameio/v2 v2.0.0 h1:UifI23ZTGY8Tt29JbYFiuyIU3eX+RNFtUwefq9qAhxg= +github.com/google/renameio/v2 v2.0.0/go.mod h1:BtmJXm5YlszgC+TD4HOEEUFgkJP3nLxehU6hfe7jRt4= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= diff --git a/internal/catalogd/controllers/core/clustercatalog_controller.go b/internal/catalogd/controllers/core/clustercatalog_controller.go index 12a67dc0fe..7a5db11f02 100644 --- a/internal/catalogd/controllers/core/clustercatalog_controller.go +++ b/internal/catalogd/controllers/core/clustercatalog_controller.go @@ -79,7 +79,7 @@ type storedCatalogData struct { //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs/status,verbs=get;update;patch //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs/finalizers,verbs=update -//+kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch +//+kubebuilder:rbac:namespace=system,groups=core,resources=secrets,verbs=get;list;watch //+kubebuilder:rbac:namespace=system,groups=core,resources=serviceaccounts,verbs=get;list;watch // Reconcile is part of the main kubernetes reconciliation loop which aims to diff --git a/internal/shared/controllers/pull_secret_controller.go b/internal/shared/controllers/pull_secret_controller.go index 8bf2bdd799..93e8faab7a 100644 --- a/internal/shared/controllers/pull_secret_controller.go +++ b/internal/shared/controllers/pull_secret_controller.go @@ -23,6 +23,7 @@ import ( "os" "github.com/go-logr/logr" + "github.com/google/renameio/v2" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" @@ -45,23 +46,18 @@ type PullSecretReconciler struct { func (r *PullSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { logger := log.FromContext(ctx).WithName("pull-secret-reconciler") - logger.Info("processing event", "name", req.NamespacedName) - defer logger.Info("processed event", "name", req.NamespacedName) + logger.Info("processing event", logName(req.NamespacedName)...) + defer logger.Info("processed event", logName(req.NamespacedName)...) secrets := []*corev1.Secret{} - secret := &corev1.Secret{} if r.SecretKey != nil { //nolint:nestif + secret, err := r.getSecret(ctx, logger, *r.SecretKey) + if err != nil { + return ctrl.Result{}, err + } // Add the configured pull secret to the list of secrets - if err := r.Get(ctx, *r.SecretKey, secret); err != nil { - if apierrors.IsNotFound(err) { - logger.Info("secret not found", "name", r.SecretKey) - } else { - logger.Error(err, "failed to get Secret", "name", r.SecretKey) - return ctrl.Result{}, err - } - } else { - logger.Info("global pull secret", "name", *r.SecretKey) + if secret != nil { secrets = append(secrets, secret) } } @@ -71,9 +67,9 @@ func (r *PullSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) logger.Info("serviceaccount", "name", r.ServiceAccountKey) if err := r.Get(ctx, r.ServiceAccountKey, sa); err != nil { //nolint:nestif if apierrors.IsNotFound(err) { - logger.Info("serviceaccount not found", "serviceaccount", r.ServiceAccountKey) + logger.Info("serviceaccount not found", logName(r.ServiceAccountKey)...) } else { - logger.Error(err, "failed to get serviceaccount", "serviceaccount", r.ServiceAccountKey) + logger.Error(err, "failed to get serviceaccount", logName(r.ServiceAccountKey)...) return ctrl.Result{}, err } } else { @@ -84,16 +80,12 @@ func (r *PullSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) // This is to update the list of secrets that we are filtering on // Add all secrets regardless if they exist or not pullSecrets = append(pullSecrets, nn) - secret := &corev1.Secret{} - err = r.Get(ctx, nn, secret) + + secret, err := r.getSecret(ctx, logger, nn) if err != nil { - if apierrors.IsNotFound(err) { - logger.Info("serviceaccount pull secret not found", "secret", nn) - } else { - logger.Error(err, "failed to get serviceaccount pull secret", "secret", nn) - return ctrl.Result{}, err - } - } else { + return ctrl.Result{}, err + } + if secret != nil { secrets = append(secrets, secret) } } @@ -108,6 +100,25 @@ func (r *PullSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, r.writeSecretToFile(logger, secrets) } +func (r *PullSecretReconciler) getSecret(ctx context.Context, logger logr.Logger, nn types.NamespacedName) (*corev1.Secret, error) { + secret := &corev1.Secret{} + if err := r.Get(ctx, *r.SecretKey, secret); err != nil { + if apierrors.IsNotFound(err) { + logger.Info("secret not found", logName(nn)...) + return nil, nil + } + logger.Error(err, "failed to get secret", logName(nn)...) + return nil, err + } + logger.Info("found secret", logName(nn)...) + return secret, nil +} + +// Helper function to log NamespacedNames +func logName(nn types.NamespacedName) []any { + return []any{"name", nn.Name, "namespace", nn.Namespace} +} + // SetupWithManager sets up the controller with the Manager. func (r *PullSecretReconciler) SetupWithManager(mgr ctrl.Manager) error { _, err := ctrl.NewControllerManagedBy(mgr). @@ -205,7 +216,7 @@ func (r *PullSecretReconciler) writeSecretToFile(logger logr.Logger, secrets []* if err != nil { return fmt.Errorf("failed to marshal secret data: %w", err) } - err = os.WriteFile(r.AuthFilePath, data, 0600) + err = renameio.WriteFile(r.AuthFilePath, data, 0600) if err != nil { return fmt.Errorf("failed to write secret data to file: %w", err) } From 10514a4435a47bafb1717a27e108945f52e3771f Mon Sep 17 00:00:00 2001 From: Todd Short Date: Fri, 6 Jun 2025 14:53:03 -0400 Subject: [PATCH 3/8] fixup! Support serviceaccount pull secrets Signed-off-by: Todd Short --- .../controllers/pull_secret_controller.go | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/internal/shared/controllers/pull_secret_controller.go b/internal/shared/controllers/pull_secret_controller.go index 93e8faab7a..3a1f1f4dbf 100644 --- a/internal/shared/controllers/pull_secret_controller.go +++ b/internal/shared/controllers/pull_secret_controller.go @@ -46,8 +46,8 @@ type PullSecretReconciler struct { func (r *PullSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { logger := log.FromContext(ctx).WithName("pull-secret-reconciler") - logger.Info("processing event", logName(req.NamespacedName)...) - defer logger.Info("processed event", logName(req.NamespacedName)...) + logger.Info("processing event", "name", req.Name, "namespace", req.Namespace) + defer logger.Info("processed event", "name", req.Name, "namespace", req.Namespace) secrets := []*corev1.Secret{} @@ -67,9 +67,9 @@ func (r *PullSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) logger.Info("serviceaccount", "name", r.ServiceAccountKey) if err := r.Get(ctx, r.ServiceAccountKey, sa); err != nil { //nolint:nestif if apierrors.IsNotFound(err) { - logger.Info("serviceaccount not found", logName(r.ServiceAccountKey)...) + logger.Info("serviceaccount not found", "name", r.ServiceAccountKey.Name, "namespace", r.ServiceAccountKey.Namespace) } else { - logger.Error(err, "failed to get serviceaccount", logName(r.ServiceAccountKey)...) + logger.Error(err, "failed to get serviceaccount", "name", r.ServiceAccountKey.Name, "namespace", r.ServiceAccountKey.Namespace) return ctrl.Result{}, err } } else { @@ -102,23 +102,18 @@ func (r *PullSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) func (r *PullSecretReconciler) getSecret(ctx context.Context, logger logr.Logger, nn types.NamespacedName) (*corev1.Secret, error) { secret := &corev1.Secret{} - if err := r.Get(ctx, *r.SecretKey, secret); err != nil { + if err := r.Get(ctx, nn, secret); err != nil { if apierrors.IsNotFound(err) { - logger.Info("secret not found", logName(nn)...) + logger.Info("secret not found", "name", nn.Name, "namespace", nn.Namespace) return nil, nil } - logger.Error(err, "failed to get secret", logName(nn)...) + logger.Error(err, "failed to get secret", "name", nn.Name, "namespace", nn.Namespace) return nil, err } - logger.Info("found secret", logName(nn)...) + logger.Info("found secret", "name", nn.Name, "namespace", nn.Namespace) return secret, nil } -// Helper function to log NamespacedNames -func logName(nn types.NamespacedName) []any { - return []any{"name", nn.Name, "namespace", nn.Namespace} -} - // SetupWithManager sets up the controller with the Manager. func (r *PullSecretReconciler) SetupWithManager(mgr ctrl.Manager) error { _, err := ctrl.NewControllerManagedBy(mgr). From 760af3ccbd63e619fff9fcc232fea2daed7c7714 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Fri, 6 Jun 2025 16:43:27 -0400 Subject: [PATCH 4/8] fixup! Support serviceaccount pull secrets Signed-off-by: Todd Short --- .../controllers/pull_secret_controller.go | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/internal/shared/controllers/pull_secret_controller.go b/internal/shared/controllers/pull_secret_controller.go index 3a1f1f4dbf..916b707c3d 100644 --- a/internal/shared/controllers/pull_secret_controller.go +++ b/internal/shared/controllers/pull_secret_controller.go @@ -43,11 +43,11 @@ type PullSecretReconciler struct { AuthFilePath string } -func (r *PullSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +func (r *PullSecretReconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result, error) { logger := log.FromContext(ctx).WithName("pull-secret-reconciler") - logger.Info("processing event", "name", req.Name, "namespace", req.Namespace) - defer logger.Info("processed event", "name", req.Name, "namespace", req.Namespace) + logger.Info("starting reconciliation") + defer logger.Info("finishing reconciliation") secrets := []*corev1.Secret{} @@ -64,15 +64,15 @@ func (r *PullSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) // Grab all the pull secrets from the serviceaccount and add them to the list of secrets sa := &corev1.ServiceAccount{} - logger.Info("serviceaccount", "name", r.ServiceAccountKey) if err := r.Get(ctx, r.ServiceAccountKey, sa); err != nil { //nolint:nestif if apierrors.IsNotFound(err) { - logger.Info("serviceaccount not found", "name", r.ServiceAccountKey.Name, "namespace", r.ServiceAccountKey.Namespace) + logger.Info("serviceaccount not found", "pod-sa", logNamespacedName(r.ServiceAccountKey)) } else { - logger.Error(err, "failed to get serviceaccount", "name", r.ServiceAccountKey.Name, "namespace", r.ServiceAccountKey.Namespace) + logger.Error(err, "failed to get serviceaccount", "pod-sa", logNamespacedName(r.ServiceAccountKey)) return ctrl.Result{}, err } } else { + logger.Info("found serviceaccount", "pod-sa", logNamespacedName(r.ServiceAccountKey)) nn := types.NamespacedName{Namespace: r.ServiceAccountKey.Namespace} pullSecrets := []types.NamespacedName{} for _, ips := range sa.ImagePullSecrets { @@ -90,8 +90,13 @@ func (r *PullSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) } } // update list of pull secrets from service account - logger.Info("updating list of pull secrets", "list", pullSecrets) r.ServiceAccountPullSecrets = pullSecrets + // Log ever-so slightly nicer + names := []string{} + for _, ps := range pullSecrets { + names = append(names, logNamespacedName(ps)) + } + logger.Info("updating list of pull-secrets", "pull-secrets", names) } if len(secrets) == 0 { @@ -104,16 +109,20 @@ func (r *PullSecretReconciler) getSecret(ctx context.Context, logger logr.Logger secret := &corev1.Secret{} if err := r.Get(ctx, nn, secret); err != nil { if apierrors.IsNotFound(err) { - logger.Info("secret not found", "name", nn.Name, "namespace", nn.Namespace) + logger.Info("pull-secret not found", "pull-secret", logNamespacedName(nn)) return nil, nil } - logger.Error(err, "failed to get secret", "name", nn.Name, "namespace", nn.Namespace) + logger.Error(err, "failed to get pull-secret", "pull-secret", logNamespacedName(nn)) return nil, err } - logger.Info("found secret", "name", nn.Name, "namespace", nn.Namespace) + logger.Info("found pull-secret", "pull-secret", logNamespacedName(nn)) return secret, nil } +func logNamespacedName(nn types.NamespacedName) string { + return fmt.Sprintf("%s/%s", nn.Namespace, nn.Name) +} + // SetupWithManager sets up the controller with the Manager. func (r *PullSecretReconciler) SetupWithManager(mgr ctrl.Manager) error { _, err := ctrl.NewControllerManagedBy(mgr). @@ -204,7 +213,7 @@ func (r *PullSecretReconciler) writeSecretToFile(logger logr.Logger, secrets []* continue } // Ignore the unknown secret - logger.Info("expected secret.Data key not found", "secret", types.NamespacedName{Name: s.Name, Namespace: s.Namespace}) + logger.Info("expected secret.Data key not found", "pull-secret", logNamespacedName(types.NamespacedName{Name: s.Name, Namespace: s.Namespace})) } data, err := json.Marshal(jsonData) From a4c4504b6ddae0a3ad2948a7081b5d89ff00ee73 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Wed, 11 Jun 2025 00:01:59 -0400 Subject: [PATCH 5/8] fixup! Support serviceaccount pull secrets --- cmd/catalogd/main.go | 38 ++++-------------- cmd/operator-controller/main.go | 37 ++++------------- .../controllers/pull_secret_controller.go | 2 +- .../util/pullsecretcache/pullsecretcache.go | 40 +++++++++++++++++++ internal/shared/util/sa/serviceaccount.go | 2 +- 5 files changed, 58 insertions(+), 61 deletions(-) create mode 100644 internal/shared/util/pullsecretcache/pullsecretcache.go diff --git a/cmd/catalogd/main.go b/cmd/catalogd/main.go index bfa42a26c5..ec7c4946ff 100644 --- a/cmd/catalogd/main.go +++ b/cmd/catalogd/main.go @@ -30,9 +30,6 @@ import ( "github.com/containers/image/v5/types" "github.com/spf13/cobra" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/fields" - k8slabels "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" k8stypes "k8s.io/apimachinery/pkg/types" apimachineryrand "k8s.io/apimachinery/pkg/util/rand" @@ -64,6 +61,7 @@ import ( sharedcontrollers "github.com/operator-framework/operator-controller/internal/shared/controllers" fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs" imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" + "github.com/operator-framework/operator-controller/internal/shared/util/pullsecretcache" sautil "github.com/operator-framework/operator-controller/internal/shared/util/sa" "github.com/operator-framework/operator-controller/internal/shared/version" ) @@ -251,37 +249,17 @@ func run(ctx context.Context) error { saKey, err := sautil.GetServiceAccount() if err != nil { - setupLog.Error(err, "Unable to get pod namesapce and serviceaccount") + setupLog.Error(err, "Failed to extract serviceaccount from JWT") return err } + setupLog.Info("Successfully extracted serviceaccount from JWT", "serviceaccount", + fmt.Sprintf("%s/%s", saKey.Namespace, saKey.Name)) - setupLog.Info("Read token", "serviceaccount", saKey) - cacheOptions.ByObject[&corev1.ServiceAccount{}] = crcache.ByObject{ - Namespaces: map[string]crcache.Config{ - saKey.Namespace: { - LabelSelector: k8slabels.Everything(), - FieldSelector: fields.SelectorFromSet(map[string]string{ - "metadata.name": saKey.Name, - }), - }, - }, - } - - secretCache := crcache.ByObject{} - secretCache.Namespaces = make(map[string]crcache.Config, 2) - secretCache.Namespaces[saKey.Namespace] = crcache.Config{ - LabelSelector: k8slabels.Everything(), - FieldSelector: fields.Everything(), - } - if cfg.globalPullSecretKey != nil { - secretCache.Namespaces[cfg.globalPullSecretKey.Namespace] = crcache.Config{ - LabelSelector: k8slabels.Everything(), - FieldSelector: fields.SelectorFromSet(map[string]string{ - "metadata.name": cfg.globalPullSecretKey.Name, - }), - } + err = pullsecretcache.SetupPullSecretCache(&cacheOptions, cfg.globalPullSecretKey, saKey) + if err != nil { + setupLog.Error(err, "Unable to setup pull-secret cache") + return err } - cacheOptions.ByObject[&corev1.Secret{}] = secretCache // Create manager mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 480e92d569..d426793d46 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -30,10 +30,8 @@ import ( "github.com/containers/image/v5/types" "github.com/spf13/cobra" - corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" apiextensionsv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1" - "k8s.io/apimachinery/pkg/fields" k8slabels "k8s.io/apimachinery/pkg/labels" k8stypes "k8s.io/apimachinery/pkg/types" apimachineryrand "k8s.io/apimachinery/pkg/util/rand" @@ -75,6 +73,7 @@ import ( fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs" httputil "github.com/operator-framework/operator-controller/internal/shared/util/http" imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" + "github.com/operator-framework/operator-controller/internal/shared/util/pullsecretcache" sautil "github.com/operator-framework/operator-controller/internal/shared/util/sa" "github.com/operator-framework/operator-controller/internal/shared/version" ) @@ -222,37 +221,17 @@ func run() error { saKey, err := sautil.GetServiceAccount() if err != nil { - setupLog.Error(err, "Unable to get pod namesapce and serviceaccount") + setupLog.Error(err, "Failed to extract serviceaccount from JWT") return err } + setupLog.Info("Successfully extracted serviceaccount from JWT", "serviceaccount", + fmt.Sprintf("%s/%s", saKey.Namespace, saKey.Name)) - setupLog.Info("Read token", "serviceaccount", saKey) - cacheOptions.ByObject[&corev1.ServiceAccount{}] = crcache.ByObject{ - Namespaces: map[string]crcache.Config{ - saKey.Namespace: { - LabelSelector: k8slabels.Everything(), - FieldSelector: fields.SelectorFromSet(map[string]string{ - "metadata.name": saKey.Name, - }), - }, - }, - } - - secretCache := crcache.ByObject{} - secretCache.Namespaces = make(map[string]crcache.Config, 2) - secretCache.Namespaces[saKey.Namespace] = crcache.Config{ - LabelSelector: k8slabels.Everything(), - FieldSelector: fields.Everything(), - } - if globalPullSecretKey != nil { - secretCache.Namespaces[globalPullSecretKey.Namespace] = crcache.Config{ - LabelSelector: k8slabels.Everything(), - FieldSelector: fields.SelectorFromSet(map[string]string{ - "metadata.name": globalPullSecretKey.Name, - }), - } + err = pullsecretcache.SetupPullSecretCache(&cacheOptions, globalPullSecretKey, saKey) + if err != nil { + setupLog.Error(err, "Unable to setup pull-secret cache") + return err } - cacheOptions.ByObject[&corev1.Secret{}] = secretCache metricsServerOptions := server.Options{} if len(cfg.certFile) > 0 && len(cfg.keyFile) > 0 { diff --git a/internal/shared/controllers/pull_secret_controller.go b/internal/shared/controllers/pull_secret_controller.go index 916b707c3d..43a2ceec80 100644 --- a/internal/shared/controllers/pull_secret_controller.go +++ b/internal/shared/controllers/pull_secret_controller.go @@ -51,7 +51,7 @@ func (r *PullSecretReconciler) Reconcile(ctx context.Context, _ ctrl.Request) (c secrets := []*corev1.Secret{} - if r.SecretKey != nil { //nolint:nestif + if r.SecretKey != nil { secret, err := r.getSecret(ctx, logger, *r.SecretKey) if err != nil { return ctrl.Result{}, err diff --git a/internal/shared/util/pullsecretcache/pullsecretcache.go b/internal/shared/util/pullsecretcache/pullsecretcache.go new file mode 100644 index 0000000000..90db9540f3 --- /dev/null +++ b/internal/shared/util/pullsecretcache/pullsecretcache.go @@ -0,0 +1,40 @@ +package pullsecretcache + +import ( + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/cache" +) + +func SetupPullSecretCache(cacheOptions *cache.Options, globalPullSecretKey *types.NamespacedName, saKey types.NamespacedName) error { + cacheOptions.ByObject[&corev1.ServiceAccount{}] = cache.ByObject{ + Namespaces: map[string]cache.Config{ + saKey.Namespace: { + LabelSelector: labels.Everything(), + FieldSelector: fields.SelectorFromSet(map[string]string{ + "metadata.name": saKey.Name, + }), + }, + }, + } + + secretCache := cache.ByObject{} + secretCache.Namespaces = make(map[string]cache.Config, 2) + secretCache.Namespaces[saKey.Namespace] = cache.Config{ + LabelSelector: labels.Everything(), + FieldSelector: fields.Everything(), + } + if globalPullSecretKey != nil { + secretCache.Namespaces[globalPullSecretKey.Namespace] = cache.Config{ + LabelSelector: labels.Everything(), + FieldSelector: fields.SelectorFromSet(map[string]string{ + "metadata.name": globalPullSecretKey.Name, + }), + } + } + cacheOptions.ByObject[&corev1.Secret{}] = secretCache + + return nil +} diff --git a/internal/shared/util/sa/serviceaccount.go b/internal/shared/util/sa/serviceaccount.go index 0b275f498e..f668c859e8 100644 --- a/internal/shared/util/sa/serviceaccount.go +++ b/internal/shared/util/sa/serviceaccount.go @@ -44,7 +44,7 @@ func getServiceAccountInternal(data []byte, err error) (k8stypes.NamespacedName, return k8stypes.NamespacedName{}, err } subjects := strings.Split(subject, ":") - if len(subjects) != 4 { + if len(subjects) != 4 || subjects[2] == "" || subjects[3] == "" { return k8stypes.NamespacedName{}, fmt.Errorf("badly formatted subject: %s", subject) } return k8stypes.NamespacedName{Namespace: subjects[2], Name: subjects[3]}, nil From c80fae05da09c5a731f3032c526f9d4f2a2f240f Mon Sep 17 00:00:00 2001 From: Todd Short Date: Wed, 11 Jun 2025 00:09:56 -0400 Subject: [PATCH 6/8] fixup! Support serviceaccount pull secrets --- internal/shared/util/pullsecretcache/pullsecretcache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/shared/util/pullsecretcache/pullsecretcache.go b/internal/shared/util/pullsecretcache/pullsecretcache.go index 90db9540f3..910b4b50b8 100644 --- a/internal/shared/util/pullsecretcache/pullsecretcache.go +++ b/internal/shared/util/pullsecretcache/pullsecretcache.go @@ -26,7 +26,7 @@ func SetupPullSecretCache(cacheOptions *cache.Options, globalPullSecretKey *type LabelSelector: labels.Everything(), FieldSelector: fields.Everything(), } - if globalPullSecretKey != nil { + if globalPullSecretKey != nil && globalPullSecretKey.Namespace != saKey.Namespace { secretCache.Namespaces[globalPullSecretKey.Namespace] = cache.Config{ LabelSelector: labels.Everything(), FieldSelector: fields.SelectorFromSet(map[string]string{ From 2144fad13872d90deafe2f393c9b1578c9c02cf7 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Thu, 12 Jun 2025 17:05:33 -0400 Subject: [PATCH 7/8] fixup! Support serviceaccount pull secrets Signed-off-by: Todd Short --- .../pull_secret_controller_test.go | 113 ++++++++++++------ 1 file changed, 78 insertions(+), 35 deletions(-) diff --git a/internal/shared/controllers/pull_secret_controller_test.go b/internal/shared/controllers/pull_secret_controller_test.go index 03f46c0078..1619267551 100644 --- a/internal/shared/controllers/pull_secret_controller_test.go +++ b/internal/shared/controllers/pull_secret_controller_test.go @@ -20,78 +20,121 @@ func TestSecretSyncerReconciler(t *testing.T) { authFileName := "test-auth.json" for _, tt := range []struct { name string - secret *corev1.Secret - addSecret bool + secretKey *types.NamespacedName + sa *corev1.ServiceAccount + secrets []corev1.Secret wantErr string fileShouldExistBefore bool fileShouldExistAfter bool }{ { - name: "secret exists, dockerconfigjson content gets saved to authFile", - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-secret", - Namespace: "test-secret-namespace", - }, - Data: map[string][]byte{ - ".dockerconfigjson": secretFullData, + name: "secret exists, dockerconfigjson content gets saved to authFile", + secretKey: &types.NamespacedName{Namespace: "test-secret-namespace", Name: "test-secret"}, + secrets: []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-secret-namespace", + }, + Data: map[string][]byte{ + ".dockerconfigjson": secretFullData, + }, }, }, - addSecret: true, fileShouldExistBefore: false, fileShouldExistAfter: true, }, { - name: "secret exists, dockercfg content gets saved to authFile", - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-secret", - Namespace: "test-secret-namespace", - }, - Data: map[string][]byte{ - ".dockercfg": secretPartData, + name: "secret exists, dockercfg content gets saved to authFile", + secretKey: &types.NamespacedName{Namespace: "test-secret-namespace", Name: "test-secret"}, + secrets: []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-secret-namespace", + }, + Data: map[string][]byte{ + ".dockercfg": secretPartData, + }, }, }, - addSecret: true, fileShouldExistBefore: false, fileShouldExistAfter: true, }, { - name: "secret does not exist, file exists previously, file should get deleted", - secret: &corev1.Secret{ + name: "secret does not exist, file exists previously, file should get deleted", + secretKey: &types.NamespacedName{Namespace: "test-secret-namespace", Name: "test-secret"}, + fileShouldExistBefore: true, + fileShouldExistAfter: false, + }, + { + name: "serviceaccount secrets, both dockerconfigjson and dockercfg content gets saved to authFile", + sa: &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-secret", + Name: "test-sa", Namespace: "test-secret-namespace", }, - Data: map[string][]byte{ - ".dockerconfigjson": secretFullData, + ImagePullSecrets: []corev1.LocalObjectReference{ + {Name: "test-secret1"}, + {Name: "test-secret2"}, }, }, - addSecret: false, - fileShouldExistBefore: true, - fileShouldExistAfter: false, + secrets: []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret1", + Namespace: "test-secret-namespace", + }, + Data: map[string][]byte{ + ".dockerconfigjson": secretFullData, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret2", + Namespace: "test-secret-namespace", + }, + Data: map[string][]byte{ + ".dockerconfigjson": secretFullData, + }, + }, + }, + fileShouldExistBefore: false, + fileShouldExistAfter: true, }, } { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() tempAuthFile := filepath.Join(t.TempDir(), authFileName) clientBuilder := fake.NewClientBuilder() - if tt.addSecret { - clientBuilder = clientBuilder.WithObjects(tt.secret) + for _, ps := range tt.secrets { + clientBuilder = clientBuilder.WithObjects(ps.DeepCopy()) + } + if tt.sa != nil { + clientBuilder = clientBuilder.WithObjects(tt.sa) } cl := clientBuilder.Build() - secretKey := types.NamespacedName{Namespace: tt.secret.Namespace, Name: tt.secret.Name} + var triggerKey types.NamespacedName + if tt.secretKey != nil { + triggerKey = *tt.secretKey + } + var saKey types.NamespacedName + if tt.sa != nil { + saKey = types.NamespacedName{Namespace: tt.sa.Namespace, Name: tt.sa.Name} + triggerKey = saKey + } r := &PullSecretReconciler{ - Client: cl, - SecretKey: &secretKey, - AuthFilePath: tempAuthFile, + Client: cl, + SecretKey: tt.secretKey, + ServiceAccountKey: saKey, + AuthFilePath: tempAuthFile, } if tt.fileShouldExistBefore { err := os.WriteFile(tempAuthFile, secretFullData, 0600) require.NoError(t, err) } - res, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: secretKey}) + res, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: triggerKey}) if tt.wantErr == "" { require.NoError(t, err) } else { From f53a858a827874bdd4663725b84ad29f803e7950 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Thu, 12 Jun 2025 17:10:04 -0400 Subject: [PATCH 8/8] fixup! Support serviceaccount pull secrets Signed-off-by: Todd Short --- manifests/standard.yaml | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/manifests/standard.yaml b/manifests/standard.yaml index 669684d93e..da08382a70 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -1095,6 +1095,22 @@ rules: --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role +metadata: + name: catalogd-manager-role + namespace: olmv1-system +rules: +- apiGroups: + - "" + resources: + - secrets + - serviceaccounts + verbs: + - get + - list + - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role metadata: name: operator-controller-leader-election-role namespace: olmv1-system @@ -1150,6 +1166,14 @@ rules: - patch - update - watch +- apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - get + - list + - watch --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole @@ -1355,6 +1379,23 @@ subjects: --- apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding +metadata: + labels: + app.kubernetes.io/name: catalogd + app.kubernetes.io/part-of: olm + name: catalogd-manager-rolebinding + namespace: olmv1-system +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: catalogd-manager-role +subjects: +- kind: ServiceAccount + name: catalogd-controller-manager + namespace: olmv1-system +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding metadata: name: operator-controller-leader-election-rolebinding namespace: olmv1-system