Skip to content

K8SPXC-1851: Fix potential nil pointer dereference in pkg/pxc/backup/restore.go#2423

Open
DimitriosLisenko wants to merge 5 commits intopercona:mainfrom
DimitriosLisenko:feature/fix-potential-nil-crash
Open

K8SPXC-1851: Fix potential nil pointer dereference in pkg/pxc/backup/restore.go#2423
DimitriosLisenko wants to merge 5 commits intopercona:mainfrom
DimitriosLisenko:feature/fix-potential-nil-crash

Conversation

@DimitriosLisenko
Copy link
Copy Markdown

@DimitriosLisenko DimitriosLisenko commented Apr 2, 2026

Just a quick thing I noticed while implementing another feature. There is a logic bug in pkg/pxc/backup/restore.go that checks if cluster.Spec.Backup is is nil and if so, calls cluster.Spec.Backup.Storages.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 2, 2026

CLA assistant check
All committers have signed the CLA.

@egegunes egegunes changed the title Fix potential nil pointer dereference in pkg/pxc/backup/restore.go K8SPXC-1851: Fix potential nil pointer dereference in pkg/pxc/backup/restore.go Apr 3, 2026
@egegunes egegunes added this to the v1.20.0 milestone Apr 3, 2026
egegunes
egegunes previously approved these changes Apr 3, 2026
Copy link
Copy Markdown
Contributor

@egegunes egegunes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why we have this check here, maybe @pooknull remembers.

Comment thread pkg/pxc/backup/restore.go Outdated

if pitr {
if cluster.Spec.Backup == nil && len(cluster.Spec.Backup.Storages) == 0 {
if cluster.Spec.Backup == nil || len(cluster.Spec.Backup.Storages) == 0 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's cover this with a unit test cause these kind of errors are easy to surface without testing, like 2 scenarios, nil backup spec returns error and empty storages map returns error

@pooknull
Copy link
Copy Markdown
Contributor

I don't remember why I added this check in the #1521. It no longer makes sense to me, so I removed it

@pooknull pooknull requested review from egegunes and gkech April 14, 2026 13:17
@JNKPercona
Copy link
Copy Markdown
Collaborator

Test Name Result Time
auto-tuning-8-0 passed 00:21:59
allocator-8-0 passed 00:14:21
allocator-8-4 passed 00:14:45
backup-storage-tls-8-0 passed 00:25:03
cross-site-8-0 passed 00:40:06
cross-site-proxysql-8-0 passed 00:47:13
cross-site-proxysql-8-4 passed 00:44:57
custom-users-8-0 passed 00:15:29
demand-backup-cloud-8-0 passed 01:07:29
demand-backup-cloud-8-4 passed 01:05:02
demand-backup-cloud-pxb-8-0 failure 00:40:08
demand-backup-encrypted-with-tls-5-7 passed 00:51:41
demand-backup-encrypted-with-tls-8-0 passed 00:51:14
demand-backup-encrypted-with-tls-8-4 passed 00:51:06
demand-backup-encrypted-with-tls-pxb-5-7 passed 00:19:36
demand-backup-encrypted-with-tls-pxb-8-0 passed 00:19:54
demand-backup-encrypted-with-tls-pxb-8-4 passed 00:20:24
demand-backup-8-0 passed 00:48:59
demand-backup-flow-control-8-0 passed 00:12:00
demand-backup-flow-control-8-4 passed 00:12:40
demand-backup-parallel-8-0 passed 00:10:53
demand-backup-parallel-8-4 passed 00:10:44
demand-backup-without-passwords-8-0 passed 00:17:59
demand-backup-without-passwords-8-4 passed 00:18:35
extra-pvc-8-0 passed 00:28:37
haproxy-5-7 passed 00:16:56
haproxy-8-0 passed 00:16:44
haproxy-8-4 passed 00:16:52
init-deploy-5-7 passed 00:20:58
init-deploy-8-0 passed 00:20:26
limits-8-0 passed 00:13:59
monitoring-2-0-8-0 passed 00:24:49
monitoring-pmm3-8-0 passed 00:20:19
monitoring-pmm3-8-4 passed 00:19:55
one-pod-5-7 passed 00:14:51
one-pod-8-0 passed 00:14:40
pitr-8-0 passed 00:52:17
pitr-8-4 passed 01:03:40
pitr-pxb-8-0 failure 00:41:06
pitr-pxb-8-4 failure 00:47:45
pitr-gap-errors-8-0 passed 00:54:37
pitr-gap-errors-8-4 passed 00:54:00
proxy-protocol-8-0 passed 00:20:56
proxy-switch-8-0 passed 00:14:42
proxysql-sidecar-res-limits-8-0 passed 00:09:14
proxysql-scheduler-8-0 passed 00:28:35
pvc-resize-5-7 passed 00:20:46
pvc-resize-8-0 passed 00:18:26
recreate-8-0 passed 00:20:23
restore-to-encrypted-cluster-8-0 passed 00:37:20
restore-to-encrypted-cluster-8-4 passed 00:29:47
restore-to-encrypted-cluster-pxb-8-0 passed 00:18:28
restore-to-encrypted-cluster-pxb-8-4 passed 00:18:16
scaling-proxysql-8-0 passed 00:10:16
scaling-8-0 passed 00:12:52
scheduled-backup-5-7 passed 01:12:41
scheduled-backup-8-0 passed 01:10:47
scheduled-backup-8-4 passed 01:09:52
security-context-8-0 passed 00:27:43
smart-update1-8-0 passed 00:39:39
smart-update1-8-4 passed 00:38:47
smart-update2-8-0 passed 00:44:36
smart-update2-8-4 passed 00:44:27
smart-update3-8-0 passed 00:18:33
sst-retry-limit-8-0 passed 00:15:14
sst-retry-limit-8-4 passed 00:15:55
storage-8-0 passed 00:12:14
tls-issue-cert-manager-ref-8-0 passed 00:10:54
tls-issue-cert-manager-8-0 passed 00:11:06
tls-issue-self-8-0 passed 00:15:31
upgrade-consistency-8-0 passed 00:13:14
upgrade-consistency-8-4 passed 00:16:56
upgrade-haproxy-5-7 passed 00:28:54
upgrade-haproxy-8-0 passed 00:28:09
upgrade-proxysql-5-7 passed 00:18:29
upgrade-proxysql-8-0 passed 00:17:21
users-5-7 passed 00:32:23
users-8-0 passed 00:31:24
users-scheduler-8-4 passed 00:29:24
validation-hook-8-0 passed 00:02:07
Summary Value
Tests Run 80/80
Job Duration 04:05:29
Total Test Time 37:51:49

commit: edfc6c2
image: perconalab/percona-xtradb-cluster-operator:PR-2423-edfc6c27

@hors hors added the community label Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants