-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test/plugin/mongodb: skip tests on 32bit systems #19873
Conversation
@@ -709,6 +709,10 @@ func TestBackend_StaticRole_Rotations_PostgreSQL(t *testing.T) { | |||
} | |||
|
|||
func TestBackend_StaticRole_Rotations_MongoDB(t *testing.T) { | |||
if v := os.Getenv("GOARCH"); v == "386" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can use runtime.GOARCH
more reliably.
I am guessing the only 32-bit platform we test on is 386, but Go supports others, notably arm
.
Would it be safer to disable the plugin altogether when on a 32-bit arch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the input!
Would it be safer to disable the plugin altogether when on a 32-bit arch?
Yes, I think so. Would that entail adding a check for runtime.GOARCH
in the plugin and exiting if it is 32bit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That, or maybe checking the size of uintptr
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like this may have been fixed in 1.8 version of the driver https://jira.mongodb.org/browse/GODRIVER-2236. Maybe I will try upgrading
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
It would definitely be preferable to upgrade.
But if we go this route, we might want to mention it in the Vault docs?
Closing in favor of #19954 |
Example of the panics we see:
My understanding is that we would need to add some padding to a struct in that package to ensure proper alignment on 32-bit architectures. See golang/go#36606. However, since this is a third party package we will just skip the test on 32bit systems.