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

PYTHON-3920 - Migrate AWS Auth Tests to use AWS Secrets #1367

Merged
merged 36 commits into from
Sep 5, 2023

Conversation

NoahStapp
Copy link
Contributor

No description provided.

@NoahStapp NoahStapp requested a review from a team as a code owner August 30, 2023 19:59
@NoahStapp NoahStapp requested review from Jibola and blink1073 and removed request for a team August 30, 2023 19:59
- command: shell.exec
type: test
params:
add_expansions_to_env: true
Copy link
Member

Choose a reason for hiding this comment

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

These can be simplified to include_expansions_in_env: ["AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY", "AWS_SESSION_TOKEN"]. I tried it out in mongodb/mongo-go-driver#1365

Copy link
Member

Choose a reason for hiding this comment

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

This particular instance still needs to be updated. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for get_aws_secrets it fails without expansions_to_env, test-atlas has been updated to use include_expansions_in_env

Copy link
Member

Choose a reason for hiding this comment

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

Wait so there are things we're missing from the new vault?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

get_aws_secrets is what fetches the secrets from the vault.

Copy link
Member

Choose a reason for hiding this comment

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

Right, okay, so now I'm confused, because that's what I'm doing in the go driver ticket.


cd -

# Write an empty prepare_aws_env so no auth environment variables are set.
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't need to do this anymore (perhaps something still needs to change in D-E-T?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see how it is used now. I think we can also absorb this logic into run-mongodb-aws-test.sh. I do like the idea of adding the script in D-E-T as you've done.

if [ "${skip_EC2_auth_test}" = "true" ]; then
echo "This platform does not support the web identity auth test, skipping..."
exit 0
fi
set -ex

# Try to source exported AWS Secrets
Copy link
Member

Choose a reason for hiding this comment

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

We can probably move this logic into run-mongodb-aws-test.sh and pass in the test to run (web-identity in this case).

.evergreen/run-tests.sh Outdated Show resolved Hide resolved
@blink1073
Copy link
Member

Nice, I like the direction this is heading. Looks like you still need to convert the last few tests?

@NoahStapp
Copy link
Contributor Author

Nice, I like the direction this is heading. Looks like you still need to convert the last few tests?

Should all be converted and good to go now!

Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Beautiful!

@NoahStapp
Copy link
Contributor Author

Beautiful!

Are these failures expected?

Screen Shot 2023-09-05 at 11 20 28 AM

@blink1073
Copy link
Member

No, I something changed on those tests, I'll open a ticket.

@NoahStapp
Copy link
Contributor Author

No, I something changed on those tests, I'll open a ticket.

Is this good to merge then?

@blink1073
Copy link
Member

Is this good to merge then?

Yes, I opened https://jira.mongodb.org/browse/PYTHON-3951

@NoahStapp NoahStapp merged commit b67ca68 into mongodb:master Sep 5, 2023
8 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