Skip to content
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

Update MongoDB tests to not fail in Go 1.16 #11533

Merged
merged 4 commits into from
May 12, 2021
Merged

Conversation

pcman312
Copy link
Contributor

@pcman312 pcman312 commented May 4, 2021

Go 1.16 introduced changes to the x509.CertPool type that make it incompatible with reflect.DeepEqual (golang/go#45891). This caused some of our tests to start failing in MongoDB. This PR changes the comparison to use github.com/google/go-cmp for the equality check which allows us to ignore certain fields and types.

t.Helper()

if diff := cmp.Diff(a, b, cmpClientOptionsOpts); diff != "" {
t.Fatalf("assertion failed: values are not equal\n--- expected\n+++ actual\n%v", diff)

Choose a reason for hiding this comment

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

Could we see an output of this error message? Is it expecting no differences and then lists out the differences?

Copy link
Contributor Author

@pcman312 pcman312 May 4, 2021

Choose a reason for hiding this comment

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

Note: I flipped the assertion so it has the expected value on the left and the actual on the right.

Here's a failure if I change the expectedOpts in the TestGetTLSAuth/good_ca test to not include the provided cert.Pem:
Updated test case:

"good ca": {
	tlsCAData: cert.Pem,

	expectOpts: options.Client().
		SetTLSConfig(
			&tls.Config{
				RootCAs: x509.NewCertPool(), // <-- Note the missing CA
			},
		),
	expectErr: false,
},

Test output:

$ go test -v -run TestGetTLSAuth/good_ca
=== RUN   TestGetTLSAuth
=== RUN   TestGetTLSAuth/good_ca
    mongodb_test.go:353: assertion failed: values are not equal
        --- expected
        +++ actual
          &options.ClientOptions{
          	... // 22 identical fields
          	ServerSelectionTimeout: nil,
          	SocketTimeout:          nil,
          	TLSConfig: &tls.Config{
          		... // 7 identical fields
          		VerifyPeerCertificate: ⟪0x00⟫,
          		VerifyConnection:      ⟪0x00⟫,
        - 		RootCAs:               nil,
        + 		RootCAs: &x509.CertPool{
        + 			byName:    map[string][]int{"0\x141\x120\x10\x06\x03U\x04\x03\x13\ttest cert": {0}},
        + 			lazyCerts: []x509.lazyCert{{rawSubject: []uint8{...}, getCert: ⟪0x0124c580⟫}},
        + 			haveSum: map[x509.sum224]bool{
        + 				{0xbd, 0x0c, 0xeb, 0xa8, 0xfe, 0xaa, 0xe8, 0xaf, 0x6d, 0xdf, 0xb9, 0x82, 0x58, 0xff, 0x0c, 0x27, 0x3a, 0xbd, 0xd4, 0xba, 0x24, 0x8b, 0x47, 0xef, 0xbc, 0xbc, 0x78, 0xf0}: true,
        + 			},
        + 		},
          		NextProtos: nil,
          		ServerName: "",
          		... // 1 ignored and 16 identical fields
          	},
          	WriteConcern: nil,
          	ZlibLevel:    nil,
          	... // 6 identical fields
          }
--- FAIL: TestGetTLSAuth (0.44s)
    --- FAIL: TestGetTLSAuth/good_ca (0.00s)
FAIL
exit status 1
FAIL	github.com/hashicorp/vault/plugins/database/mongodb	0.881s

Same thing, but messing with the good key test:

"good key": {
	username:   "unittest",
	tlsKeyData: cert.CombinedPEM(),

	expectOpts: options.Client().
		SetTLSConfig(
			&tls.Config{}, // <-- Note the missing cert
		).
		SetAuth(options.Credential{
			AuthMechanism: "MONGODB-X509",
			Username:      "unittest",
		}),
	expectErr: false,
},

Test Output:

$ go test -v -run TestGetTLSAuth/good_key
=== RUN   TestGetTLSAuth
=== RUN   TestGetTLSAuth/good_key
    mongodb_test.go:353: assertion failed: values are not equal
        --- expected
        +++ actual
          &options.ClientOptions{
          	... // 22 identical fields
          	ServerSelectionTimeout: nil,
          	SocketTimeout:          nil,
          	TLSConfig: &tls.Config{
          		Rand:         nil,
          		Time:         ⟪0x00⟫,
        - 		Certificates: nil,
        + 		Certificates: []tls.Certificate{
        + 			{
        + 				Certificate: [][]uint8{{...}},
        + 				PrivateKey: &rsa.PrivateKey{
        + 					PublicKey:   rsa.PublicKey{...},
        + 					D:           s"99111349761763075119885005748638"...,
        + 					Primes:      []*big.Int{...},
        + 					Precomputed: rsa.PrecomputedValues{...},
        + 				},
        + 			},
        + 		},
          		NameToCertificate: nil,
          		GetCertificate:    ⟪0x00⟫,
          		... // 1 ignored and 23 identical fields
          	},
          	WriteConcern: nil,
          	ZlibLevel:    nil,
          	... // 6 identical fields
          }
--- FAIL: TestGetTLSAuth (0.30s)
    --- FAIL: TestGetTLSAuth/good_key (0.00s)
FAIL
exit status 1
FAIL	github.com/hashicorp/vault/plugins/database/mongodb	0.730s

Copy link

@Valarissa Valarissa left a comment

Choose a reason for hiding this comment

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

Looks good to me, but I'd love to see an example of the error message output!

@vercel vercel bot temporarily deployed to Preview – vault-storybook May 4, 2021 21:52 Inactive
@vercel vercel bot temporarily deployed to Preview – vault May 4, 2021 21:52 Inactive
@Valarissa
Copy link

Thanks for that update! Looks great!

@vercel vercel bot temporarily deployed to Preview – vault-storybook May 4, 2021 22:21 Inactive
@vercel vercel bot temporarily deployed to Preview – vault May 4, 2021 22:21 Inactive
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove the extra newline here?

@pcman312 pcman312 merged commit c191081 into master May 12, 2021
@pcman312 pcman312 deleted the fix-mongodb-tests branch May 12, 2021 21:22
ncabatoff pushed a commit that referenced this pull request Dec 14, 2021
# Conflicts:
#	.circleci/config.yml
#	.circleci/config/commands/go_test.yml
ncabatoff added a commit that referenced this pull request Dec 14, 2021
* Move to Go 1.16.12 and newer cimg docker images for CI. (#13422)
* Update MongoDB tests to not fail in Go 1.16 (#11533)
* Upgrade snappy to fix panic with identity/packer on Go 1.16+arm64. (#12371) (#12375)
* creds/aws: Add support for DSA signature verification for EC2 (#12340)
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.

4 participants