feat: grant permissions to read secrets (#25)

Signed-off-by: Leonardo Cecchi <leonardo.cecchi@enterprisedb.com>

config/rbac/role.yaml

@@ -4,6 +4,16 @@ kind: ClusterRole
 metadata:
   name: plugin-barman-cloud
 rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - watch
 - apiGroups:
   - barmancloud.cnpg.io
   resources:

internal/cnpgi/instance/manager.go

@@ -7,9 +7,11 @@ import (
 
 	cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
 	"github.com/spf13/viper"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -23,6 +25,7 @@ var scheme = runtime.NewScheme()
 func init() {
 	utilruntime.Must(barmancloudv1.AddToScheme(scheme))
 	utilruntime.Must(cnpgv1.AddToScheme(scheme))
+	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
 }
 
 // Start starts the sidecar informers and CNPG-i server
@@ -52,6 +55,13 @@ func Start(ctx context.Context) error {
 				},
 			},
 		},
+		Client: client.Options{
+			Cache: &client.CacheOptions{
+				DisableFor: []client.Object{
+					&corev1.Secret{},
+				},
+			},
+		},
 	})
 	if err != nil {
 		setupLog.Error(err, "unable to start manager")

internal/cnpgi/instance/wal.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"path"
 	"strings"
 	"time"
 
@@ -22,6 +23,13 @@ import (
 	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
 )
 
+const (
+	// CheckEmptyWalArchiveFile is the name of the file in the PGDATA that,
+	// if present, requires the WAL archiver to check that the backup object
+	// store is empty.
+	CheckEmptyWalArchiveFile = ".check-empty-wal-archive"
+)
+
 // WALServiceImplementation is the implementation of the WAL Service
 type WALServiceImplementation struct {
 	BarmanObjectKey  client.ObjectKey
@@ -75,11 +83,20 @@ func (w WALServiceImplementation) Archive(
 		objectStore.Namespace,
 		&objectStore.Spec.Configuration,
 		os.Environ())
-	if apierrors.IsForbidden(err) {
-		return nil, errors.New("backup credentials don't yet have access permissions. Will retry reconciliation loop")
+	if err != nil {
+		if apierrors.IsForbidden(err) {
+			return nil, errors.New("backup credentials don't yet have access permissions. Will retry reconciliation loop")
+		}
+		return nil, err
 	}
 
-	arch, err := archiver.New(ctx, envArchive, w.SpoolDirectory, w.PGDataPath, w.PGWALPath)
+	arch, err := archiver.New(
+		ctx,
+		envArchive,
+		w.SpoolDirectory,
+		w.PGDataPath,
+		path.Join(w.PGDataPath, CheckEmptyWalArchiveFile),
+	)
 	if err != nil {
 		return nil, err
 	}

internal/cnpgi/operator/reconciler.go

@@ -2,19 +2,21 @@ package operator
 
 import (
 	"context"
-	"fmt"
 
 	cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
 	"github.com/cloudnative-pg/cnpg-i-machinery/pkg/pluginhelper/decoder"
 	"github.com/cloudnative-pg/cnpg-i-machinery/pkg/pluginhelper/object"
 	"github.com/cloudnative-pg/cnpg-i/pkg/reconciler"
 	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 
+	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
 	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/config"
+	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/specs"
 )
 
 // ReconcilerImplementation implements the Reconciler capability
@@ -45,6 +47,8 @@ func (r ReconcilerImplementation) Pre(
 	ctx context.Context,
 	request *reconciler.ReconcilerHooksRequest,
 ) (*reconciler.ReconcilerHooksResult, error) {
+	contextLogger := log.FromContext(ctx)
+
 	reconciledKind, err := object.GetKind(request.GetResourceDefinition())
 	if err != nil {
 		return nil, err
@@ -60,12 +64,28 @@ func (r ReconcilerImplementation) Pre(
 		return nil, err
 	}
 
+	contextLogger = contextLogger.WithValues("name", cluster.Name, "namespace", cluster.Namespace)
+	ctx = log.IntoContext(ctx, contextLogger)
+
 	pluginConfiguration, err := config.NewFromCluster(cluster)
 	if err != nil {
 		return nil, err
 	}
 
-	if err := r.ensureRole(ctx, cluster, pluginConfiguration.BarmanObjectName); err != nil {
+	var barmanObject barmancloudv1.ObjectStore
+	if err := r.Client.Get(ctx, client.ObjectKey{
+		Namespace: cluster.Namespace,
+		Name:      pluginConfiguration.BarmanObjectName,
+	}, &barmanObject); err != nil {
+		if apierrs.IsNotFound(err) {
+			contextLogger.Info("Not found barman object configuration, requeuing")
+			return &reconciler.ReconcilerHooksResult{
+				Behavior: reconciler.ReconcilerHooksResult_BEHAVIOR_REQUEUE,
+			}, nil
+		}
+	}
+
+	if err := r.ensureRole(ctx, cluster, &barmanObject); err != nil {
 		return nil, err
 	}
 
@@ -91,21 +111,50 @@ func (r ReconcilerImplementation) Post(
 func (r ReconcilerImplementation) ensureRole(
 	ctx context.Context,
 	cluster *cnpgv1.Cluster,
-	barmanObjectName string,
+	barmanObject *barmancloudv1.ObjectStore,
 ) error {
+	contextLogger := log.FromContext(ctx)
+	newRole := specs.BuildRole(cluster, barmanObject)
+
 	var role rbacv1.Role
 	if err := r.Client.Get(ctx, client.ObjectKey{
-		Namespace: cluster.Namespace,
-		Name:      getRBACName(cluster.Name),
+		Namespace: newRole.Namespace,
+		Name:      newRole.Name,
 	}, &role); err != nil {
-		if apierrs.IsNotFound(err) {
-			return r.createRole(ctx, cluster, barmanObjectName)
+		if !apierrs.IsNotFound(err) {
+			return err
 		}
-		return err
+
+		contextLogger.Info(
+			"Creating role",
+			"name", newRole.Name,
+			"namespace", newRole.Namespace,
+		)
+
+		if err := ctrl.SetControllerReference(
+			cluster,
+			newRole,
+			r.Client.Scheme(),
+		); err != nil {
+			return err
+		}
+
+		return r.Client.Create(ctx, newRole)
 	}
 
-	// TODO: patch existing role
-	return nil
+	if equality.Semantic.DeepEqual(newRole.Rules, role.Rules) {
+		// There's no need to hit the API server again
+		return nil
+	}
+
+	contextLogger.Info(
+		"Patching role",
+		"name", newRole.Name,
+		"namespace", newRole.Namespace,
+		"rules", newRole.Rules,
+	)
+
+	return r.Client.Patch(ctx, newRole, client.MergeFrom(&role))
 }
 
 func (r ReconcilerImplementation) ensureRoleBinding(
@@ -115,7 +164,7 @@ func (r ReconcilerImplementation) ensureRoleBinding(
 	var role rbacv1.RoleBinding
 	if err := r.Client.Get(ctx, client.ObjectKey{
 		Namespace: cluster.Namespace,
-		Name:      getRBACName(cluster.Name),
+		Name:      specs.GetRBACName(cluster.Name),
 	}, &role); err != nil {
 		if apierrs.IsNotFound(err) {
 			return r.createRoleBinding(ctx, cluster)
@@ -123,90 +172,18 @@ func (r ReconcilerImplementation) ensureRoleBinding(
 		return err
 	}
 
-	// TODO: patch existing role binding
+	// TODO: this assumes role bindings never change.
+	// Is that true? Should we relax this assumption?
 	return nil
 }
 
-func (r ReconcilerImplementation) createRole(
-	ctx context.Context,
-	cluster *cnpgv1.Cluster,
-	barmanObjectName string,
-) error {
-	role := buildRole(cluster, barmanObjectName)
-	if err := ctrl.SetControllerReference(cluster, role, r.Client.Scheme()); err != nil {
-		return err
-	}
-	return r.Client.Create(ctx, role)
-}
-
 func (r ReconcilerImplementation) createRoleBinding(
 	ctx context.Context,
 	cluster *cnpgv1.Cluster,
 ) error {
-	roleBinding := buildRoleBinding(cluster)
+	roleBinding := specs.BuildRoleBinding(cluster)
 	if err := ctrl.SetControllerReference(cluster, roleBinding, r.Client.Scheme()); err != nil {
 		return err
 	}
 	return r.Client.Create(ctx, roleBinding)
 }
-
-func buildRole(
-	cluster *cnpgv1.Cluster,
-	barmanObjectName string,
-) *rbacv1.Role {
-	return &rbacv1.Role{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: cluster.Namespace,
-			Name:      getRBACName(cluster.Name),
-		},
-
-		Rules: []rbacv1.PolicyRule{
-			{
-				APIGroups: []string{
-					"barmancloud.cnpg.io",
-				},
-				Verbs: []string{
-					"get",
-					"watch",
-					"list",
-				},
-				Resources: []string{
-					"objectstores",
-				},
-				ResourceNames: []string{
-					barmanObjectName,
-				},
-			},
-		},
-	}
-}
-
-func buildRoleBinding(
-	cluster *cnpgv1.Cluster,
-) *rbacv1.RoleBinding {
-	return &rbacv1.RoleBinding{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: cluster.Namespace,
-			Name:      getRBACName(cluster.Name),
-		},
-		Subjects: []rbacv1.Subject{
-			{
-				Kind:      "ServiceAccount",
-				APIGroup:  "",
-				Name:      cluster.Name,
-				Namespace: cluster.Namespace,
-			},
-		},
-		RoleRef: rbacv1.RoleRef{
-			APIGroup: "rbac.authorization.k8s.io",
-			Kind:     "Role",
-			Name:     getRBACName(cluster.Name),
-		},
-	}
-}
-
-// getRBACName returns the name of the RBAC entities for the
-// barman cloud plugin
-func getRBACName(clusterName string) string {
-	return fmt.Sprintf("%s-barman", clusterName)
-}

internal/cnpgi/operator/specs/role.go

@@ -0,0 +1,88 @@
+package specs
+
+import (
+	"fmt"
+
+	cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
+)
+
+// BuildRole builds the Role object for this cluster
+func BuildRole(
+	cluster *cnpgv1.Cluster,
+	barmanObject *barmancloudv1.ObjectStore,
+) *rbacv1.Role {
+	return &rbacv1.Role{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: cluster.Namespace,
+			Name:      GetRBACName(cluster.Name),
+		},
+
+		Rules: []rbacv1.PolicyRule{
+			{
+				APIGroups: []string{
+					"barmancloud.cnpg.io",
+				},
+				Verbs: []string{
+					"get",
+					"watch",
+					"list",
+				},
+				Resources: []string{
+					"objectstores",
+				},
+				ResourceNames: []string{
+					barmanObject.Name,
+				},
+			},
+			{
+				APIGroups: []string{
+					"",
+				},
+				Resources: []string{
+					"secrets",
+				},
+				Verbs: []string{
+					"get",
+					"watch",
+					"list",
+				},
+				ResourceNames: collectSecretNames(barmanObject),
+			},
+		},
+	}
+}
+
+// BuildRoleBinding builds the role binding object for this cluster
+func BuildRoleBinding(
+	cluster *cnpgv1.Cluster,
+) *rbacv1.RoleBinding {
+	return &rbacv1.RoleBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: cluster.Namespace,
+			Name:      GetRBACName(cluster.Name),
+		},
+		Subjects: []rbacv1.Subject{
+			{
+				Kind:      "ServiceAccount",
+				APIGroup:  "",
+				Name:      cluster.Name,
+				Namespace: cluster.Namespace,
+			},
+		},
+		RoleRef: rbacv1.RoleRef{
+			APIGroup: "rbac.authorization.k8s.io",
+			Kind:     "Role",
+			Name:     GetRBACName(cluster.Name),
+		},
+	}
+}
+
+// GetRBACName returns the name of the RBAC entities for the
+// barman cloud plugin
+func GetRBACName(clusterName string) string {
+	return fmt.Sprintf("%s-barman-cloud", clusterName)
+}

internal/cnpgi/operator/specs/secrets.go

@@ -0,0 +1,51 @@
+package specs
+
+import (
+	machineryapi "github.com/cloudnative-pg/machinery/pkg/api"
+
+	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
+)
+
+func collectSecretNames(object *barmancloudv1.ObjectStore) []string {
+	if object == nil {
+		return nil
+	}
+
+	var references []*machineryapi.SecretKeySelector
+	if object.Spec.Configuration.AWS != nil {
+		references = append(
+			references,
+			object.Spec.Configuration.AWS.AccessKeyIDReference,
+			object.Spec.Configuration.AWS.SecretAccessKeyReference,
+			object.Spec.Configuration.AWS.RegionReference,
+			object.Spec.Configuration.AWS.SessionToken,
+		)
+	}
+	if object.Spec.Configuration.Azure != nil {
+		references = append(
+			references,
+			object.Spec.Configuration.Azure.ConnectionString,
+			object.Spec.Configuration.Azure.StorageAccount,
+			object.Spec.Configuration.Azure.StorageKey,
+			object.Spec.Configuration.Azure.StorageSasToken,
+		)
+	}
+	if object.Spec.Configuration.Google != nil {
+		references = append(
+			references,
+			object.Spec.Configuration.Google.ApplicationCredentials,
+		)
+	}
+
+	result := make([]string, 0, len(references))
+	for _, reference := range references {
+		if reference == nil {
+			continue
+		}
+		result = append(result, reference.Name)
+	}
+
+	// TODO: stringset belongs to machinery :(
+
+	return result
+}

internal/cnpgi/operator/specs/specs.go

@@ -0,0 +1,3 @@
+// Package specs contains the specification of the kubernetes objects
+// that are created by the plugin
+package specs

internal/controller/objectstore_controller.go

@@ -36,6 +36,7 @@ type ObjectStoreReconciler struct {
 
 // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=create;patch;update;get;list;watch
 // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles,verbs=create;patch;update;get;list;watch
+// +kubebuilder:rbac:groups="",resources=secrets,verbs=create;list;get;watch;delete
 // +kubebuilder:rbac:groups=barmancloud.cnpg.io,resources=objectstores,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=barmancloud.cnpg.io,resources=objectstores/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=barmancloud.cnpg.io,resources=objectstores/finalizers,verbs=update

scripts/run.sh

@@ -10,7 +10,7 @@ fi
 
 current_context=$(kubectl config view --raw -o json | jq -r '."current-context"' | sed "s/kind-//")
 operator_image=$(KIND_CLUSTER_NAME="$current_context" KO_DOCKER_REPO=kind.local ko build -BP ./cmd/operator)
-instance_image=$(KIND_CLUSTER_NAME="$current_context" KO_DOCKER_REPO=kind.local ko build -BP ./cmd/instance)
+instance_image=$(KIND_CLUSTER_NAME="$current_context" KO_DOCKER_REPO=kind.local KO_DEFAULTBASEIMAGE="ghcr.io/cloudnative-pg/postgresql:17.0" ko build -BP ./cmd/instance)
 
 (
   cd kubernetes;