-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(NODE-7046): remove AWS uri/options support #4689
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
base: main
Are you sure you want to change the base?
Changes from all commits
9a47fca
5c6d9de
28539cb
c6545f3
f68d8ef
813cac4
1c45bb2
e6c0bfd
cd6e6f7
b5aaf29
b842edc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
[submodule "drivers-evergreen-tools"] | ||
path = drivers-evergreen-tools | ||
url = https://github.com/mongodb-labs/drivers-evergreen-tools.git | ||
url = https://github.com/mongodb-labs/drivers-evergreen-tools.git |
+14 −5 | .evergreen/auth_aws/aws_tester.py | |
+22 −0 | .evergreen/tests/test-aws.sh |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -423,6 +423,18 @@ export function parseOptions( | |
); | ||
} | ||
|
||
if (isAws) { | ||
const { username, password } = mongoOptions.credentials; | ||
if (username || password) { | ||
throw new MongoParseError( | ||
'username and password cannot be provided when using MONGODB-AWS' | ||
); | ||
} | ||
if (mongoOptions.credentials.mechanismProperties.AWS_SESSION_TOKEN) { | ||
throw new MongoParseError('AWS_SESSION_TOKEN cannot be provided when using MONGODB-AWS'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we update both of these messages to hint at the correct way to provide the credentials? |
||
} | ||
} | ||
|
||
mongoOptions.credentials.validate(); | ||
|
||
// Check if the only auth related option provided was authSource, if so we can remove credentials | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -440,6 +440,21 @@ | |
} | ||
} | ||
}, | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bookmark for corresponding spec update |
||
"description": "should throw an exception if username provided (MONGODB-AWS) implies default mechanism)", | ||
"uri": "mongodb://user:localhost.com/", | ||
"valid": false | ||
}, | ||
{ | ||
"description": "should throw an exception if username and password provided (MONGODB-AWS) implies default mechanism)", | ||
"uri": "mongodb://user@pass:localhost.com/", | ||
"valid": false | ||
}, | ||
{ | ||
"description": "should throw an exception if AWS_SESSION_TOKEN provided (MONGODB-AWS) implies default mechanism)", | ||
"uri": "mongodb://localhost/?authMechanism=MONGODB-AWS&authMechanismProperties=AWS_SESSION_TOKEN:token", | ||
"valid": false | ||
}, | ||
{ | ||
"description": "should recognise the mechanism with test environment (MONGODB-OIDC)", | ||
"uri": "mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:test", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -320,6 +320,18 @@ tests: | |
mechanism: MONGODB-AWS | ||
mechanism_properties: | ||
AWS_SESSION_TOKEN: token!@#$%^&*()_+ | ||
- description: should throw an exception if username provided (MONGODB-AWS) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bookmark for corresponding spec update |
||
implies default mechanism) | ||
uri: mongodb://user:localhost.com/ | ||
valid: false | ||
- description: should throw an exception if username and password provided (MONGODB-AWS) | ||
implies default mechanism) | ||
uri: mongodb://user@pass:localhost.com/ | ||
valid: false | ||
- description: should throw an exception if AWS_SESSION_TOKEN provided (MONGODB-AWS) | ||
implies default mechanism) | ||
uri: mongodb://localhost/?authMechanism=MONGODB-AWS&authMechanismProperties=AWS_SESSION_TOKEN:token | ||
valid: false | ||
- description: should recognise the mechanism with test environment (MONGODB-OIDC) | ||
uri: mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:test | ||
valid: true | ||
|
@@ -468,4 +480,4 @@ tests: | |
(MONGODB-OIDC) | ||
uri: mongodb://user:pass@localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ENVIRONMENT:k8s | ||
valid: false | ||
credential: null | ||
credential: null |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,23 @@ | ||
import { loadSpecTests } from '../../spec'; | ||
import { executeUriValidationTest } from '../../tools/uri_spec_runner'; | ||
|
||
const SKIP = [ | ||
'should throw an exception if username and no password (MONGODB-AWS)', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suspect we can leave this one unskipped, since it should continue to fail (albeit for a different reason), but no strong preference since we should get the same coverage from the newly added test |
||
'should use username and password if specified (MONGODB-AWS)', | ||
'should use username, password and session token if specified (MONGODB-AWS)' | ||
]; | ||
|
||
describe('Auth option spec tests (legacy)', function () { | ||
const suites = loadSpecTests('auth', 'legacy'); | ||
|
||
for (const suite of suites) { | ||
describe(suite.name, function () { | ||
for (const test of suite.tests) { | ||
it(`${test.description}`, function () { | ||
if (SKIP.includes(test.description)) { | ||
this.test.skipReason = `NODE-7046: ${test.description}`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bookmark for the deletion ticket (once the DRIVERS ticket is filed, we can generate the language tickets and then add the NODE ticket reference here) |
||
this.test.skip(); | ||
} | ||
executeUriValidationTest(test); | ||
}); | ||
} | ||
|
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.
Since we're trying to move away from using
MongoParseError
for options that we are able to read but that happen to be invalid (see NODE-4015), can we go withMongoAPIError
for both of the new instances here?