Skip to content

Conversation

Gu1nness
Copy link
Contributor

@Gu1nness Gu1nness commented Sep 18, 2025

The Cast operations are unsafe in case of an incomplete configuration and can end up in SEGFAULT.

This PR fixes that by introducing basic checks (Object is defined) and returning an error if that is not the case.
This PR also adds a test for the S3 case to ensure that this functionality works as expected.

Moreover, as there is a default value for the region, the Region field can be made optional with omitempty and the correct value will be picked automatically.

BEFORE:

root@mongodb-k8s-0:/etc/pbm# pbm config --file ./pbm_config.yaml
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0xcb5383]

goroutine 1 [running]:
github.com/percona/percona-backup-mongodb/pbm/storage/s3.(*Config).Cast(...)
	github.com/percona/percona-backup-mongodb/pbm/storage/s3/s3.go:202
github.com/percona/percona-backup-mongodb/pbm/config.(*StorageConf).Cast(0xc00059f8d8?)
	github.com/percona/percona-backup-mongodb/pbm/config/config.go:262 +0x43
github.com/percona/percona-backup-mongodb/pbm/config.Parse({0x15ccce0, 0xc000526248})
	github.com/percona/percona-backup-mongodb/pbm/config/config.go:93 +0x207
main.readConfigFromFile({0x7ffd5d0984b0?, 0xc000600f90?})
	github.com/percona/percona-backup-mongodb/cmd/pbm/config.go:168 +0x15c
main.runConfig({0x15d5b28, 0xc000600f90}, {0x15dc298, 0xc0004a4870}, 0xc0000a3580, 0xc000037940)
	github.com/percona/percona-backup-mongodb/cmd/pbm/config.go:126 +0x1a5
main.(*pbmApp).buildConfigCmd.func1(0xcee979ce00000000?, {0xc000492660?, 0x0?, 0x0?})
	github.com/percona/percona-backup-mongodb/cmd/pbm/main.go:374 +0x67
main.(*pbmApp).buildConfigCmd.(*pbmApp).wrapRunE.func2(0xc000452300?, {0xc000492660?, 0x4?, 0x1021233?})
	github.com/percona/percona-backup-mongodb/cmd/pbm/main.go:85 +0x29
github.com/spf13/cobra.(*Command).execute(0xc0001c7508, {0xc000492640, 0x2, 0x2})
	github.com/spf13/cobra/command.go:985 +0xaca
github.com/spf13/cobra.(*Command).ExecuteC(0xc0001c6908)
	github.com/spf13/cobra/command.go:1117 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
	github.com/spf13/cobra/command.go:1041
main.main()
	github.com/percona/percona-backup-mongodb/cmd/pbm/main.go:76 +0x1b

AFTER:

root@mongodb-k8s-0:/etc/pbm# ./pbm config --file pbm_config.yaml
Error: unable to get new config: storage cast: Missing S3 configuration with S3 storage type.

Referred in the bug tracker as PBM-1619 here

@Gu1nness Gu1nness force-pushed the fix/dont-null-pointer-exception branch from 3178b34 to aec66c4 Compare September 18, 2025 15:31
Copy link
Member

@boris-ilijic boris-ilijic left a comment

Choose a reason for hiding this comment

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

Hi, PR looks good. Please merge/rebase with dev and fix requested changes and PR will be ready for the merge.
Thank you!

@Gu1nness
Copy link
Contributor Author

Gu1nness commented Oct 8, 2025

Hello @boris-ilijic ,
Thank you for the review !
I have merged dev branch into mine and fixed the two error messages to comply with the styleguide!

Copy link
Member

@boris-ilijic boris-ilijic left a comment

Choose a reason for hiding this comment

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

Hi @Gu1nness, PR looks good!
I've approved this and am merging now. Thanks again for reporting this and for the fix!

@boris-ilijic boris-ilijic merged commit fc11f69 into percona:dev Oct 8, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants