-
Notifications
You must be signed in to change notification settings - Fork 455
CDRIVER-5773 remove code for MONGODB-CR
#1788
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
Conversation
To be used to skip all "MONGODB-OIDC" tests. OIDC is not-yet implemented.
From commit: mongodb/specifications@82be6f2
The driver test server uses the auth source named "mongodb-cr", but uses SCRAM-SHA-1.
src/libmongoc/tests/json-test.h
Outdated
@@ -32,6 +32,7 @@ typedef void (*test_hook) (void *test); | |||
|
|||
typedef struct { | |||
const char *description; | |||
bool check_substring; |
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.
Why was this placed between existing data members instead of after?
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.
To locate it next to description
, since check_substring
applies to description
. But I can move to the end if desired. Added comment to describe the purpose of check_substring
.
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 would normally recommend moving it to the end of the struct so existing code is not impacted as much (i.e. in test-mongoc-retryable-writes.c
), but I think using designated initializers is preferable anyways in these circumstances (+ they can be ordered however is preferable, as already done in test-mongoc-connection-uri.c
), so I do not mind the proposed changes as-is, other than perhaps to better conform to -Wpadded
-friendly layout practices:
warning: padding struct 'test_skip_t' with 7 bytes to align 'reason' [-Wpadded]
36 | const char *reason;
| ^
vs. moving it to the end of the struct (no substantial difference in this case): ¯\_(ツ)_/¯
warning: padding size of 'test_skip_t' with 7 bytes to alignment boundary [-Wpadded]
33 | typedef struct {
| ^
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.
Will keep as-is.
Co-authored-by: Ezra Chung <[email protected]>
src/libmongoc/tests/json-test.h
Outdated
@@ -32,6 +32,7 @@ typedef void (*test_hook) (void *test); | |||
|
|||
typedef struct { | |||
const char *description; | |||
bool check_substring; |
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 would normally recommend moving it to the end of the struct so existing code is not impacted as much (i.e. in test-mongoc-retryable-writes.c
), but I think using designated initializers is preferable anyways in these circumstances (+ they can be ordered however is preferable, as already done in test-mongoc-connection-uri.c
), so I do not mind the proposed changes as-is, other than perhaps to better conform to -Wpadded
-friendly layout practices:
warning: padding struct 'test_skip_t' with 7 bytes to align 'reason' [-Wpadded]
36 | const char *reason;
| ^
vs. moving it to the end of the struct (no substantial difference in this case): ¯\_(ツ)_/¯
warning: padding size of 'test_skip_t' with 7 bytes to alignment boundary [-Wpadded]
33 | typedef struct {
| ^
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 good to me. Re struct field order: I'm happy with optimizing for semantics rather than memory layout in test code, it's neither something that needs to be fast nor a thing we'll be stuck with if we feel like changing it later.
Summary
Remove code for the MONGODB-CR auth mechanism
Verified with this patch build.
Background & Motivation
MONGODB-CR no longer supported
MongoDB server 4.0 dropped support of the authentication mechanism MONGODB-CR. Quoting DRIVERS-441:
As of CDRIVER-4815, the C driver supports the minimum server version 4.0. Attempting to connect to older servers results in error:
Authenticating with MONGODB-CR is not possible on supported servers. Using MONGODB-CR on a newer server results in a protocol error:
With changes in this PR, a client-side error is now returned:
Skipping tests
Auth spec tests are updated to mongodb/specifications@82be6f2 to remove references to MONGODB-CR. Tests requiring unimplemented features or fixes to known bugs are skipped.
test_skip_t
is extended to support skipping from a description substring to ease skipping all MONGODB-OIDC tests.