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

Implementing docker secrets, MQTT TLS connections, MQTT publishing and option to redact lat/long data #49

Merged
merged 4 commits into from
Nov 30, 2023

Conversation

gummigroda
Copy link

This PR will add the following features:

  • Allow connection to MQTT broker with TLS enabled
  • Allow connection to MQTT broker on non standard port (1883)
  • Option to not sending Latitude and Longitude to ABRP
  • Option to output data to a MQTT Topic of choice, including:
    • Status of the script
    • Last successful update
    • Last status
    • Last error
  • Option to use Docker secrets for USER_TOKEN and MQTT_PASSWORD to prevent using sensitive data in environment variables.

gummigroda and others added 4 commits November 26, 2023 22:40
Add possibility to NOT send location
Enable usage of Docker secrets
~ Modified/Rephrased/Reformatted some contents in the README
~ Moved the conditional check for publish_to_mqqt() before calling the function.
@fetzu fetzu changed the title Add some security features Implementing docker secrets, MQTT TLS connections, MQTT publishing and option to redact lat/long data Nov 30, 2023
@fetzu
Copy link
Owner

fetzu commented Nov 30, 2023

Hey @gummigroda, fantastic PR! Thank you very much!
I just had a few changes I wanted to make to the PR, but I'm somehow unable to publish them. Could you please enable the "allow edits" flag in the PR?

I'm also wondering what your reasoning is behind the script also publishing MQTT data itself? Do you have a specific use case in mind? (My initial concern that it would make it too confusing and create feature creep, but your implementation is light and elegant enough.)

@fetzu fetzu added the enhancement New feature or request label Nov 30, 2023
@gummigroda
Copy link
Author

Hi,
Edits are now allowed.

The rationale behind adding data back to MQTT is that I'm using that to trigger events when errors occur and restart the container and just monitor what data does end up in ABPR as I find that easer than parsing the output from the script.
I've no problem if this is removed from the PR, but as a default setting, this is just extra code and not a requirement.

Thanks.

@fetzu
Copy link
Owner

fetzu commented Nov 30, 2023

Thanks for your answer; I will leave it in with the small changes I made.

If it's OK with you, I'll merge this into main and wait 1–2 weeks before making a release; it will be accessible through the :beta tag for testing.

Thanks again for the great PR 🚀 !

@fetzu fetzu merged commit 915e4f4 into fetzu:main Nov 30, 2023
@gummigroda
Copy link
Author

👍 All good.

@gummigroda gummigroda deleted the add-mqtt-tls branch December 1, 2023 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants