fritzbox-telegraf-plugin icon indicating copy to clipboard operation
fritzbox-telegraf-plugin copied to clipboard

Add docker option for building and running

Open vlorian-de opened this issue 1 year ago • 2 comments
trafficstars

Hallo, vielen lieben Dank, dass du dieses nützliche Plugin geschrieben hast. Da ich die meisten Services inzwischen containerisiert laufen lasse, habe ich 2 Skripte geschrieben um dein Plugin sowohl in einem Container zu bauen und auch in einem Container auszuführen.

Das Bauen im Container ist für die Leute interessant, die noch keine Golang-Umgebung aufgesetzt haben. So müssen sie zum Bauen nichts installieren und der Container löscht sich nach erfolgreichem Bauen auch wieder ganz von alleine.

Das Ausführen im Container ist für Leute interessant, die die Fritzbox-Daten mit einem eigenen Hostname wegloggen möchten. Das Docker-Run-Skript macht dein Plugin quasi standalone lauffähig und so kann man fürs Loggen einen eigenen Hostname wählen und das Plugin auch auf Maschinen nutzen auf denen noch keine Telegraf installiert ist.

Es wäre mir eine Ehre, wenn meine Skripte es in dein Repo schaffen würden.

Viele Grüße Vlorian

vlorian-de avatar Mar 10 '24 12:03 vlorian-de

@CodiumAI-Agent /review

vlorian-de avatar Dec 18 '24 11:12 vlorian-de

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 Security concerns

Sensitive information exposure:
The docker/config/fritzbox.conf file contains placeholders for sensitive data like usernames and passwords. Ensure that these placeholders are replaced securely and that users are warned against committing sensitive information to the repository.

⚡ Recommended focus areas for review

Documentation Clarity
The new section on running/building via Docker is a useful addition, but it could benefit from more detailed instructions or examples, especially for users unfamiliar with Docker. For instance, specifying the expected directory structure or providing a sample command for customizing the configuration files might help.

Sensitive Information Placeholder
The placeholders <username> and <password> in the devices array might lead to confusion or accidental inclusion of sensitive information in the configuration file. Consider adding a comment to remind users not to commit sensitive data.

Hardcoded Repository URL
The script clones a specific repository URL (https://github.com/vlorian-de/fritzbox-telegraf-plugin.git). This could limit flexibility if the repository changes or if users want to use a fork. Consider making the repository URL configurable.

Configuration Adaptation Reminder
The script includes a reminder to adapt configuration files, but it does not validate or check if the configurations are properly set. This could lead to runtime errors. Consider adding a validation step or more explicit instructions.

CodiumAI-Agent avatar Dec 18 '24 11:12 CodiumAI-Agent