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

Add support for Mongo TLS & Atlas #21

Merged
merged 1 commit into from
Oct 17, 2023
Merged

Conversation

mkrle
Copy link
Contributor

@mkrle mkrle commented Sep 27, 2023

  • I have read the contributing guideline and understand that I have made the correct modifications

Description:

I have added flags for TLS (tls) and authSource to the MongoDB URLs. The TLS flag is set to true depending on the MONGO_TLS variable. authSource is always set to admin.

Benefits of this PR and context:

This PR will do the following:

  • Optionally enable TLS for MongoDB (required for Atlas)
  • Fix authentication issues when using Atlas as MongoDB backend due to database name being passed as defaultauthdb parameter

How Has This Been Tested?

I have tested with not defining MONGO_TLS variable and haven't noticed any change in behaviour. It would be good if we could get more volunteers to test.
My test environment was linux (arm64) docker with Atlas as MongoDB backend.

Source / References:

https://www.mongodb.com/docs/v4.4/reference/connection-string/
https://stackoverflow.com/questions/38237663/mongo-atlas-connection-authentication-failed-with-custom-databases

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thanks for opening this pull request! Be sure to follow the pull request template!

@LinuxServer-CI
Copy link
Contributor

I am a bot, here is the pushed image/manifest for this PR:

ghcr.io/linuxserver/lspipepr-unifi-network-application:7.5.176-pkg-3f48ad74-dev-76b26b72cdf462d75beb14f8af9cad331e6953a1-pr-21

@tobiasfaust
Copy link

can please somebody approve and merge this PR. This PR fixes #9 and lots of people are waiting for that ;)

@trunet
Copy link

trunet commented Oct 12, 2023

there's no rush I guess... people waiting can just point to the image mentioned on this comment: #21 (comment)

@thespad thespad self-assigned this Oct 12, 2023
@LinuxServer-CI
Copy link
Contributor

I am a bot, here is the pushed image/manifest for this PR:

ghcr.io/linuxserver/lspipepr-unifi-network-application:7.5.176-pkg-22c0bf17-dev-874e2dfe0459454c38d5695bd67361d9eb59e6a9-pr-21

@tobiasfaust
Copy link

hi,
unfortunally it doesn´t work without modifications.
By using authSource=unifi it works.
I will describe it in details at #9

@mkrle
Copy link
Contributor Author

mkrle commented Oct 12, 2023

Got your point @tobiasfaust. I've updated the PR to set authSource to MONGO_DBNAME by default, and then allow others (e.g. Atlas users) to set it to admin using MONGO_AUTHSOURCE variable. Please let me know if it works in your case.

@LinuxServer-CI
Copy link
Contributor

I am a bot, here is the pushed image/manifest for this PR:

ghcr.io/linuxserver/lspipepr-unifi-network-application:7.5.176-pkg-22c0bf17-dev-525cb05f23fad0e830b2ce9198954279fdcf102d-pr-21

@mkrle mkrle mentioned this pull request Oct 12, 2023
1 task
@tobiasfaust
Copy link

hi @mkrle, it works perfectly right now.
I hope this PR would be merged as soon as possible into the master branch.

  • Add this 2 new variables to README description

@mkrle
Copy link
Contributor Author

mkrle commented Oct 13, 2023

That README section should be generated automatically from readme-vars.yml so should be taken care of.

readme-vars.yml Outdated Show resolved Hide resolved
Copy link
Member

@thespad thespad left a comment

Choose a reason for hiding this comment

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

Please add an entry to the changelog in the readme summarising the changes.

@mkrle
Copy link
Contributor Author

mkrle commented Oct 17, 2023

Please add an entry to the changelog in the readme summarising the changes.

Done. Also squashed the commits as they were growing in number. Thanks a lot for the feedback!

Copy link
Member

@thespad thespad left a comment

Choose a reason for hiding this comment

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

Other than the changelog fix looks fine and I'll be happy to approve it.

readme-vars.yml Outdated Show resolved Hide resolved
@LinuxServer-CI
Copy link
Contributor

I am a bot, here is the pushed image/manifest for this PR:

ghcr.io/linuxserver/lspipepr-unifi-network-application:7.5.187-pkg-5959d60e-dev-6492326b25ff789d4f76fa023627442ca925bd33-pr-21

@LinuxServer-CI
Copy link
Contributor

I am a bot, here is the pushed image/manifest for this PR:

ghcr.io/linuxserver/lspipepr-unifi-network-application:7.5.187-pkg-5959d60e-dev-6109b5698168a728503fdca3aa4955e1fff660f9-pr-21

@thespad thespad merged commit a87ec4e into linuxserver:main Oct 17, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants