Skip to content

Add ssl argument to Mongo connection#59927

Merged
dwoz merged 19 commits intosaltstack:masterfrom
babs:mongodb-ssl-arg
Jan 15, 2024
Merged

Add ssl argument to Mongo connection#59927
dwoz merged 19 commits intosaltstack:masterfrom
babs:mongodb-ssl-arg

Conversation

@babs
Copy link
Copy Markdown
Contributor

@babs babs commented Mar 29, 2021

What does this PR do?

Add ssl argument to Mongo connection

@babs babs requested a review from a team as a code owner March 29, 2021 17:26
@babs babs requested review from waynew and removed request for a team March 29, 2021 17:26
Copy link
Copy Markdown
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

I really like this addition, for obvious reasons 😄

We do require test coverage for code that's changed, and we don't really have that here. Is that something that you'd be wiling to add?

If not, no big deal - we'll just add the help-wanted label and will be able to merge when we can write tests for it.

Also this will need a changelog entry

Comment thread salt/states/mongodb_user.py Outdated
@babs
Copy link
Copy Markdown
Contributor Author

babs commented Nov 22, 2021

I'll have to call for help on that :/

@waynew waynew added the help-wanted Community help is needed to resolve this label Nov 22, 2021
@waynew
Copy link
Copy Markdown
Contributor

waynew commented Mar 30, 2022

I went ahead and implemented the changes+tests. Just re-merged the latest changes from master in, so 🤞 hopefully all the tests pass now.

@babs
Copy link
Copy Markdown
Contributor Author

babs commented Sep 12, 2022

@waynew done :)
sorry for the delay.

@dwoz
Copy link
Copy Markdown
Contributor

dwoz commented Dec 10, 2023

Closing this due to inactivity. Anyone should feel free to re-open it if they want to see it through to the end in one release cycle.

@dwoz dwoz closed this Dec 10, 2023
@dwoz dwoz added the Abandoned label Dec 10, 2023
@babs
Copy link
Copy Markdown
Contributor Author

babs commented Dec 11, 2023

It's still relevant unless another PR has been merged that adds ssl to mongodb...

@dwoz dwoz reopened this Dec 18, 2023
@dwoz
Copy link
Copy Markdown
Contributor

dwoz commented Dec 18, 2023

@babs Please rebase and fix the conflicts.

@babs
Copy link
Copy Markdown
Contributor Author

babs commented Dec 20, 2023

@waynew might need a second pair of eyes and possibly hands to check the rebase :)

@babs
Copy link
Copy Markdown
Contributor Author

babs commented Dec 21, 2023

@waynew might need your help on mongo ssl tests

@babs
Copy link
Copy Markdown
Contributor Author

babs commented Jan 6, 2024

@dwoz do you know somebody that could give me a hand on fixing the instace tests ?

@babs
Copy link
Copy Markdown
Contributor Author

babs commented Jan 7, 2024

@dwoz do you know somebody that could give me a hand on fixing the instance tests ?

waynew and others added 19 commits January 10, 2024 21:41
These tests are not *super* meaningful but they do at least ensure that
ssl the arg correctly gets passed into the MongoClient. Still left to do
is add tests + ssl arg for the mongodb_database state, the mongodb
module, as well as updating all of the documentation for these modules.

We also want to add a few functional tests that ensure that the SSL
argument is correctly used with an actual mongodb server.
... as well as the necessary modules, natch.
Would still like to add functional tests against an actual mongodb.
Without verify_ssl it's a bit complicated to test, and if someone wants
to use self-signed certs in their environment without actually
installing the certs or making a CA, that should also be up to them.
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