jasonacox / Powerwall-Dashboard

Grafana Monitoring Dashboard for Tesla Solar and Powerwall Systems
MIT License
296 stars 63 forks source link

Minor bug in setup.sh on docker group test if docker is the FIRST group returned by "id -Gn" #476

Open hulkster opened 4 months ago

hulkster commented 4 months ago

Problem If docker is the first group returned via "id -Gn", setup.sh says "WARNING: You do not appear to be in the docker group"

The issue is with from line 39 of 4.3.1 setup.sh that does the following test: if ! $(id -Gn 2>/dev/null | grep -qE " docker( |$)"); then A possible fix it to also use "^docker" so the expression returns true if docker is the first (or only) group listed. But I don't know the full reasoning for the regex, so someone should make sure whatever fix is done is "correct" ... since regex can be "maze of twisty little passages" ;-)

To Reproduce $ id -Gn docker $ ./setup.sh Powerwall Dashboard (v4.3.1) - SETUP

WARNING: You do not appear to be in the docker group.

Please ensure your local user is in the docker group and run without sudo. sudo usermod -aG docker $USER ./setup.sh

Screenshots N/A

Host System AlmaLinux release 9.4

Additional context The reason I stumbled across this bug is because I have a captive account (named solar!) that only runs powerwall ... so I listed the group ID for docker in the /etc/passwd entry for solar. Note the problem ALSO occurs if the user belongs to multiple groups, but docker is the first one returned by id -Gn

So yea, I could make the "primary" group something else and then add solar to the /etc/group docker entry ... but I don't think this should be necessary.

Note that setup.sh does allow you to proceed ... so it's not really a show-stopper ... but a bit confusing because I was like WTF, the user IS in the docker group! ;-)

jasonacox commented 4 months ago

Can you run this script to see what you get?

if id -Gn 2>/dev/null | grep -qw "docker"; then
   echo "good"
else
   echo "bad"
fi
hulkster commented 4 months ago

I tested using various "group settings" and substrings and that works great @jasonacox.

My situation was a real corner case ... but I think using "-w" for the regex is the way to go.

You can close out this bug report when pushed into production.