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

[redis] Add redis Group And Grant Read/Write Access to Members #5289

Merged
merged 3 commits into from
Sep 3, 2020

Conversation

tahmed-dev
Copy link
Contributor

@tahmed-dev tahmed-dev commented Aug 31, 2020

sonic-cfggen is now using Unix Domain Socket for Redis DB. The socket
is created using root account. Subsequently, services that are started
as admin fail to start. This PR creates redis group and add admin
user to redis group. It also grants read/write access on redis.sock
for redis group members.

closes #5277
resolves #5277

signed-off-by: Tamer Ahmed tamer.ahmed@microsoft.com

- Why I did it
Access to redis.sock fails when using admin account

- How I did it
Added redis group
Changed redis.sock group to the new group redis
Gave read/write access to redis group on redis.sock

- How to verify it
without this change

admin@str-s6000-acs-14:~$ groups
admin sudo docker

admin@str-s6000-acs-14:~$ ls -alrt /var/run/redis/redis.sock 
srwx------ 1 root root 0 Aug 31 18:48 /var/run/redis/redis.sock

admin@str-s6000-acs-14:~$ sonic-cfggen -d -v 'DEVICE_METADATA["localhost"]["hwsku"]'
Traceback (most recent call last):
  File "/usr/local/bin/sonic-cfggen", line 420, in <module>
    main()
  File "/usr/local/bin/sonic-cfggen", line 347, in main
    configdb.connect()
  File "/usr/local/lib/python2.7/dist-packages/swsssdk/configdb.py", line 74, in connect
    self.db_connect('CONFIG_DB', wait_for_init, retry_on)
  File "/usr/local/lib/python2.7/dist-packages/swsssdk/configdb.py", line 69, in db_connect
    SonicV2Connector.connect(self, self.db_name, retry_on)
  File "/usr/local/lib/python2.7/dist-packages/swsssdk/dbconnector.py", line 250, in connect
    self.dbintf.connect(db_id, retry_on)
  File "/usr/local/lib/python2.7/dist-packages/swsssdk/interface.py", line 171, in connect
    self._onetime_connect(db_id)
  File "/usr/local/lib/python2.7/dist-packages/swsssdk/interface.py", line 183, in _onetime_connect
    client.config_set('notify-keyspace-events', self.KEYSPACE_EVENTS)
  File "/usr/local/lib/python2.7/dist-packages/redis/client.py", line 1243, in config_set
    return self.execute_command('CONFIG SET', name, value)
  File "/usr/local/lib/python2.7/dist-packages/redis/client.py", line 898, in execute_command
    conn = self.connection or pool.get_connection(command_name, **options)
  File "/usr/local/lib/python2.7/dist-packages/redis/connection.py", line 1192, in get_connection
    connection.connect()
  File "/usr/local/lib/python2.7/dist-packages/redis/connection.py", line 563, in connect
    raise ConnectionError(self._error_message(e))
redis.exceptions.ConnectionError: Error 13 connecting to unix socket: /var/run/redis/redis.sock. Permission denied.

with this change

admin@str-s6000-acs-14:~$ groups
admin sudo docker redis

admin@str-s6000-acs-14:~$ ls -alrt /var/run/redis/redis.sock 
srwxrw---- 1 root redis 0 Aug 31 23:00 /var/run/redis/redis.sock

admin@str-s6000-acs-14:~$ sonic-cfggen -d -v 'DEVICE_METADATA["localhost"]["hwsku"]'
Force10-S6000

- Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

sonic-cfggen is now using Unix Domain Socket for Redis DB. The socket
is created using root account. Subsequently, services that are started
as admin fails to start. This PR creates redis group and add admin
user to redis group. It also grants read/write access on redis.sock
for redis group members.

signed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
build_debian.sh Outdated Show resolved Hide resolved
@jleveque jleveque self-requested a review August 31, 2020 23:32
jleveque
jleveque previously approved these changes Aug 31, 2020
Copy link
Contributor

@jleveque jleveque left a comment

Choose a reason for hiding this comment

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

LGTM. Please wait for other reviewers.

pavel-shirshov
pavel-shirshov previously approved these changes Sep 1, 2020
Copy link
Contributor

@pavel-shirshov pavel-shirshov left a comment

Choose a reason for hiding this comment

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

LGTM but wait for others

Copy link
Contributor

@judyjoseph judyjoseph left a comment

Choose a reason for hiding this comment

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

Looks ok with multi-asic platform. One observation though I don't see this new redis group inside the docker. But I feel it is ok as long as the owner is root.

admin@str--acs-1:~$ docker exec -it database bash
root@str--acs-1:/# ls -la /var/run/redis/redis.sock
srwxrw---- 1 root 1000 0 Sep 3 05:29 /var/run/redis/redis.sock

@tahmed-dev
Copy link
Contributor Author

Looks ok with multi-asic platform. One observation though I don't see this new redis group inside the docker. But I feel it is ok as long as the owner is root.

admin@str--acs-1:~$ docker exec -it database bash
root@str--acs-1:/# ls -la /var/run/redis/redis.sock
srwxrw---- 1 root 1000 0 Sep 3 05:29 /var/run/redis/redis.sock

docker has user root AFAIKT. user admin is not available inside docker

@tahmed-dev tahmed-dev merged commit fdb9d02 into sonic-net:master Sep 3, 2020
abdosi pushed a commit that referenced this pull request Sep 6, 2020
sonic-cfggen is now using Unix Domain Socket for Redis DB. The socket
is created using root account. Subsequently, services that are started
as admin fails to start. This PR creates redis group and add admin
user to redis group. It also grants read/write access on redis.sock
for redis group members.

signed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
santhosh-kt pushed a commit to santhosh-kt/sonic-buildimage that referenced this pull request Feb 25, 2021
…-net#5289)

sonic-cfggen is now using Unix Domain Socket for Redis DB. The socket
is created using root account. Subsequently, services that are started
as admin fails to start. This PR creates redis group and add admin
user to redis group. It also grants read/write access on redis.sock
for redis group members.

signed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
abdosi added a commit to abdosi/sonic-py-swsssdk that referenced this pull request Apr 1, 2022
…s unix socket is given to the redis group members.

Many of sonic-util commands (especially in multi-asic) case use redis
unix socket to connect to DB and thus those comamnd fails without
providing sudo. This PR is continuation  of PR:
sonic-net/sonic-buildimage#7002 where we default to
use TCP for Redis if user is not root

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
abdosi added a commit to sonic-net/sonic-py-swsssdk that referenced this pull request Apr 1, 2022
What I did:
Changes to Use Redis Unix Socket if the user is root else default to TCP

Why I did:
With the changes in PR:sonic-net/sonic-buildimage#5289 access to redis unix socket is given to the redis group members or to the root. Many of sonic-util commands (especially in multi-asic) case use redis unix socket to connect to DB and thus those comamnd fails without providing sudo. This PR is continuation of PR: sonic-net/sonic-buildimage#7002 where we default to use TCP for Redis if user is not root in sonic-cfggen.

This should fix: sonic-net/sonic-buildimage#8501
abdosi added a commit to sonic-net/sonic-py-swsssdk that referenced this pull request Apr 1, 2022
…s unix socket is given to the redis group members.

Many of sonic-util commands (especially in multi-asic) case use redis
unix socket to connect to DB and thus those comamnd fails without
providing sudo. This PR is continuation  of PR:
sonic-net/sonic-buildimage#7002 where we default to
use TCP for Redis if user is not root

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
abdosi added a commit that referenced this pull request Apr 1, 2022
9ce4d19d5a199cffe2933d80e343a80ded398b4a (HEAD -> 201911, origin/201911) With the changes in PR:#5289 access to redis unix socket is given to the redis group members. Many of sonic-util commands (especially in multi-asic) case use redis unix socket to connect to DB and thus those comamnd fails without providing sudo. This PR is continuation  of PR: #7002 where we default to use TCP for Redis if user is not root

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
abdosi added a commit to abdosi/sonic-buildimage that referenced this pull request Apr 1, 2022
78f167e4728f939712b3f3ea550949e2ea675fec With the changes in PR:sonic-net#5289 access to redis unix socket is given to the redis group members. Many of sonic-util commands (especially in multi-asic) case use redis unix socket to connect to DB and thus those comamnd fails without providing sudo. This PR is continuation  of PR: sonic-net#7002 where we default to use TCP for Redis if user is not root

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
abdosi added a commit that referenced this pull request Apr 2, 2022
*[Submodule update] sonic-py-swssdk

78f167e4728f939712b3f3ea550949e2ea675fec With the changes in PR:#5289 access to redis unix socket is given to the redis group members. Many of sonic-util commands (especially in multi-asic) case use redis unix socket to connect to DB and thus those comamnd fails without providing sudo. This PR is continuation  of PR: #7002 where we default to use TCP for Redis if user is not root

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sonic-cfggen fails to connect to /var/run/redis/redis.sock
6 participants