Repository: cloudnative-pg/cloudnative-pg

Issue:
[Bug]: Segfault due to missing resource validation for bootstrap.recovery credentials
### Is there an existing issue already for this bug?

- [x] I have searched for an existing issue, and could not find anything. I believe this is a new bug.

### I have read the troubleshooting guide

- [x] I have read the troubleshooting guide and I think this is a new bug.

### I am running a supported version of CloudNativePG

- [x] I have read the troubleshooting guide and I think this is a new bug.

### Contact Details

_No response_

### Version

1.24.0

### What version of Kubernetes are you using?

1.31

### What is your Kubernetes environment?

Self-managed: kind (evaluation)

### How did you install the operator?

YAML manifest

### What happened?

I know the cluster resource is not properly done, but while trying to make a replication work I came up with this invalid configuration that rather than failing resulted in a panic.  
Unfortunately I'm not extremely familiar with the codebase so I cannot pinpoint exactly where the missing validation is

This is also a low priority bug of course

### Cluster resource

```shell
apiVersion: postgresql.cnpg.io/v1
kind: Cluster
metadata:
  annotations:
    meta.helm.sh/release-name: supabase
    meta.helm.sh/release-namespace: test
  creationTimestamp: "2024-11-15T15:26:09Z"
  generation: 1
  labels:
    app.kubernetes.io/managed-by: Helm
  name: supabase-postgresql
  namespace: test
spec:
  affinity:
    podAntiAffinityType: preferred
  bootstrap:
    recovery:
      database: app
      owner: app
      source: source-db
  enablePDB: true
  enableSuperuserAccess: true
  externalClusters:
  - connectionParameters:
      dbname: postgres
      host: supabase-postgresql
      user: postgres
    name: source-db
    password:
      key: password
      name: supabase-database-postgres-password
    sslCert:
      key: tls.crt
      name: supabase-postgresql-replication
    sslKey:
      key: tls.key
      name: supabase-postgresql-replication
  failoverDelay: 0
  imageName: psqlc:15.8
  instances: 1
  logLevel: info
  maxSyncReplicas: 0
  minSyncReplicas: 0
  monitoring:
    customQueriesConfigMap:
    - key: queries
      name: cnpg-default-monitoring
    disableDefaultQueries: false
    enablePodMonitor: true
  postgresGID: 26
  postgresUID: 26
  postgresql:
    parameters:
      archive_mode: always
      archive_timeout: 5min
      cron.database_name: postgres
      cron.use_background_workers: "on"
      dynamic_shared_memory_type: posix
      effective_cache_size: 128MB
      full_page_writes: "on"
      log_destination: csvlog
      log_directory: /controller/log
      log_filename: postgres
      log_rotation_age: "0"
      log_rotation_size: "0"
      log_statement: none
      log_truncate_on_rotation: "false"
      logging_collector: "on"
      max_parallel_workers: "32"
      max_replication_slots: "32"
      max_worker_processes: "32"
      shared_buffers: 128MB
      shared_memory_type: mmap
      shared_preload_libraries: ""
      ssl_max_protocol_version: TLSv1.3
      ssl_min_protocol_version: TLSv1.3
      wal_keep_size: 512MB
      wal_level: logical
      wal_log_hints: "on"
      wal_receiver_timeout: 5s
      wal_sender_timeout: 5s
    shared_preload_libraries:
    - pg_stat_statements
    - pgaudit
    - plpgsql
    - plpgsql_check
    - pg_cron
    - pg_net
    - pgsodium
    syncReplicaElectionConstraint:
      enabled: false
  primaryUpdateMethod: restart
  primaryUpdateStrategy: unsupervised
  replica:
    enabled: true
    source: source-db
  replicationSlots:
    highAvailability:
      enabled: true
      slotPrefix: _cnpg_
    synchronizeReplicas:
      enabled: true
    updateInterval: 30
  smartShutdownTimeout: 180
  startDelay: 3600
  stopDelay: 1800
  storage:
    resizeInUseVolumes: true
    size: 1Gi
  superuserSecret:
    name: supabase-database-postgres-password
  switchoverDelay: 3600
```

### Relevant log output

```shell
{"level":"info","ts":"2024-11-15T15:26:24.107419416Z","msg":"Recovering from external cluster","logging_pod":"supabase-supabase-postgresql-1-full-recovery","sourceName":"source-db"}
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x16beac0]

goroutine 1 [running]:
github.com/cloudnative-pg/barman-cloud/pkg/credentials.EnvSetRestoreCloudCredentials({0x22d4f48?, 0x3679fe0?}, {0xffff71e1ee18?, 0x4000482148?}, {0x40004f6088?, 0x2?}, 0x40005fc288?, {0x4000654908?, 0x0?, 0x400000ea78?})
        pkg/mod/github.com/cloudnative-pg/barman-cloud@v0.0.0-20240924124724-92831d48562a/pkg/credentials/env.go:86 +0x30
github.com/cloudnative-pg/cloudnative-pg/pkg/management/postgres.InitInfo.loadBackupObjectFromExternalCluster({{0x400005a007, 0x1f}, {0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}, ...}, ...)
        pkg/management/postgres/restore.go:479 +0x2ac
github.com/cloudnative-pg/cloudnative-pg/pkg/management/postgres.InitInfo.loadBackup({{0x400005a007, 0x1f}, {0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}, ...}, ...)
        pkg/management/postgres/restore.go:454 +0x9c
github.com/cloudnative-pg/cloudnative-pg/pkg/management/postgres.InitInfo.Restore({{0x400005a007, 0x1f}, {0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}, ...}, ...)
        pkg/management/postgres/restore.go:259 +0x2e0
github.com/cloudnative-pg/cloudnative-pg/internal/cmd/manager/instance/restore.restoreSubCommand({_, _}, {{0x400005a007, 0x1f}, {0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}, ...})
        internal/cmd/manager/instance/restore/cmd.go:91 +0x98
github.com/cloudnative-pg/cloudnative-pg/internal/cmd/manager/instance/restore.NewCmd.func2(0x40005284f0?, {0x1f43b0c?, 0x4?, 0x1f43b10?})
        internal/cmd/manager/instance/restore/cmd.go:63 +0xc4
github.com/spf13/cobra.(*Command).execute(0x4000479208, {0x400013d5b0, 0x1, 0x1})
        pkg/mod/github.com/spf13/cobra@v1.8.1/command.go:985 +0x834
github.com/spf13/cobra.(*Command).ExecuteC(0x40002a3208)
        pkg/mod/github.com/spf13/cobra@v1.8.1/command.go:1117 +0x344
github.com/spf13/cobra.(*Command).Execute(...)
        pkg/mod/github.com/spf13/cobra@v1.8.1/command.go:1041
main.main()
        cmd/manager/main.go:68 +0x394
```

### Code of Conduct

- [x] I agree to follow this project's Code of Conduct

Hints from issue comments:
@gbartolini: Not blocking 1.25.0-RC1. Definitely to be addressed for the official release.@anjuls: We are still having an issue.  I am not very confident if it is related to this. we are using CNPG 1.25 and barman cloud plugin v0.3.0 

<img width="1064" alt="Image" src="https://github.com/user-attachments/assets/c852788f-8661-4d28-9bdf-df054f809926" />


Solution patch:
diff --git a/api/v1/cluster_webhook.go b/api/v1/cluster_webhook.go
index 0d6da62c..eec7a01f 100644
--- a/api/v1/cluster_webhook.go
+++ b/api/v1/cluster_webhook.go
@@ -866,7 +866,9 @@ func (r *Cluster) validateBootstrapRecoverySource() field.ErrorList {
 		return result
 	}
 
-	_, found := r.ExternalCluster(r.Spec.Bootstrap.Recovery.Source)
+	externalCluster, found := r.ExternalCluster(r.Spec.Bootstrap.Recovery.Source)
+
+	// Ensure the existence of the external cluster
 	if !found {
 		result = append(
 			result,
@@ -876,6 +878,18 @@ func (r *Cluster) validateBootstrapRecoverySource() field.ErrorList {
 				fmt.Sprintf("External cluster %v not found", r.Spec.Bootstrap.Recovery.Source)))
 	}
 
+	// Ensure the external cluster definition has enough information
+	// to be used to recover a data directory
+	if externalCluster.BarmanObjectStore == nil && externalCluster.PluginConfiguration == nil {
+		result = append(
+			result,
+			field.Invalid(
+				field.NewPath("spec", "bootstrap", "recovery", "source"),
+				r.Spec.Bootstrap.Recovery.Source,
+				fmt.Sprintf("External cluster %v cannot be used for recovery: "+
+					"both Barman and CNPG-i plugin configurations are missing", r.Spec.Bootstrap.Recovery.Source)))
+	}
+
 	return result
 }
 
diff --git a/pkg/management/postgres/restore.go b/pkg/management/postgres/restore.go
index 2cdad8c8..979ed57f 100644
--- a/pkg/management/postgres/restore.go
+++ b/pkg/management/postgres/restore.go
@@ -511,6 +511,11 @@ func (info InitInfo) loadBackupObjectFromExternalCluster(
 	if !found {
 		return nil, nil, fmt.Errorf("missing external cluster: %v", sourceName)
 	}
+
+	if server.BarmanObjectStore == nil {
+		return nil, nil, fmt.Errorf("missing barman object store configuration for source: %v", sourceName)
+	}
+
 	serverName := server.GetServerName()
 
 	env, err := barmanCredentials.EnvSetRestoreCloudCredentials(



Test patch:
diff --git a/api/v1/cluster_webhook_test.go b/api/v1/cluster_webhook_test.go
index cdbf5585..bd08a265 100644
--- a/api/v1/cluster_webhook_test.go
+++ b/api/v1/cluster_webhook_test.go
@@ -23,6 +23,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/cloudnative-pg/barman-cloud/pkg/api"
 	"github.com/cloudnative-pg/machinery/pkg/image/reference"
 	pgversion "github.com/cloudnative-pg/machinery/pkg/postgres/version"
 	storagesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1"
@@ -2328,7 +2329,49 @@ var _ = Describe("bootstrap recovery validation", func() {
 		Expect(result).To(BeEmpty())
 	})
 
-	It("does not complain when bootstrap recovery source matches one of the names of external clusters", func() {
+	Context("does not complain when bootstrap recovery source matches one of the names of external clusters", func() {
+		When("using a barman object store configuration", func() {
+			recoveryCluster := &Cluster{
+				Spec: ClusterSpec{
+					Bootstrap: &BootstrapConfiguration{
+						Recovery: &BootstrapRecovery{
+							Source: "test",
+						},
+					},
+					ExternalClusters: []ExternalCluster{
+						{
+							Name:              "test",
+							BarmanObjectStore: &api.BarmanObjectStoreConfiguration{},
+						},
+					},
+				},
+			}
+			errorsList := recoveryCluster.validateBootstrapRecoverySource()
+			Expect(errorsList).To(BeEmpty())
+		})
+
+		When("using a plugin configuration", func() {
+			recoveryCluster := &Cluster{
+				Spec: ClusterSpec{
+					Bootstrap: &BootstrapConfiguration{
+						Recovery: &BootstrapRecovery{
+							Source: "test",
+						},
+					},
+					ExternalClusters: []ExternalCluster{
+						{
+							Name:                "test",
+							PluginConfiguration: &PluginConfiguration{},
+						},
+					},
+				},
+			}
+			errorsList := recoveryCluster.validateBootstrapRecoverySource()
+			Expect(errorsList).To(BeEmpty())
+		})
+	})
+
+	It("complains when bootstrap recovery source does not match one of the names of external clusters", func() {
 		recoveryCluster := &Cluster{
 			Spec: ClusterSpec{
 				Bootstrap: &BootstrapConfiguration{
@@ -2338,16 +2381,16 @@ var _ = Describe("bootstrap recovery validation", func() {
 				},
 				ExternalClusters: []ExternalCluster{
 					{
-						Name: "test",
+						Name: "another-test",
 					},
 				},
 			},
 		}
 		errorsList := recoveryCluster.validateBootstrapRecoverySource()
-		Expect(errorsList).To(BeEmpty())
+		Expect(errorsList).ToNot(BeEmpty())
 	})
 
-	It("complains when bootstrap recovery source does not match one of the names of external clusters", func() {
+	It("complains when bootstrap recovery source have no BarmanObjectStore nor plugin configuration", func() {
 		recoveryCluster := &Cluster{
 			Spec: ClusterSpec{
 				Bootstrap: &BootstrapConfiguration{
@@ -2357,13 +2400,13 @@ var _ = Describe("bootstrap recovery validation", func() {
 				},
 				ExternalClusters: []ExternalCluster{
 					{
-						Name: "another-test",
+						Name: "test",
 					},
 				},
 			},
 		}
 		errorsList := recoveryCluster.validateBootstrapRecoverySource()
-		Expect(errorsList).ToNot(BeEmpty())
+		Expect(errorsList).To(HaveLen(1))
 	})
 })