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 Redis connection parameters full control #8578

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
minor_changes:
- redis cache plugin - add Redis connection parameters full access in order to be able to set advanced socket options, like enabling keep alive (https://github.com/ansible-collections/community.general/pull/8578)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- redis cache plugin - add Redis connection parameters full access in order to be able to set advanced socket options, like enabling keep alive (https://github.com/ansible-collections/community.general/pull/8578)
- redis cache plugin - add Redis connection parameters full access in order to be able to set advanced socket options, like enabling keep alive (https://github.com/ansible-collections/community.general/pull/8578).

225 changes: 208 additions & 17 deletions plugins/cache/redis.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,27 +15,36 @@
requirements:
- redis>=2.4.5 (python lib)
options:
_uri:
description:
- A colon separated string of connection information for Redis.
- The format is V(host:port:db:password), for example V(localhost:6379:0:changeme).
- To use encryption in transit, prefix the connection with V(tls://), as in V(tls://localhost:6379:0:changeme).
- To use redis sentinel, use separator V(;), for example V(localhost:26379;localhost:26379;0:changeme). Requires redis>=2.9.0.
_decode_responses:
description: If set to `true`, returned values from Redis commands get decoded automatically using the client's charset value.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
description: If set to `true`, returned values from Redis commands get decoded automatically using the client's charset value.
description: If set to C(true), returned values from Redis commands get decoded automatically using the client's charset value.

type: bool
default: false
env:
- name: ANSIBLE_CACHE_REDIS_DECODE_RESPONSES
ini:
- key: fact_caching_redis_decode_responses
section: defaults
_encoding:
description: Set the charset to use for facts encoding.
type: string
required: true
default: utf-8
env:
- name: ANSIBLE_CACHE_PLUGIN_CONNECTION
- name: ANSIBLE_CACHE_REDIS_ENCODING
ini:
- key: fact_caching_connection
- key: fact_caching_redis_encoding
section: defaults
_prefix:
description: User defined prefix to use when creating the DB entries
_encoding_errors:
description:
- The error handling scheme to use for encoding errors.
- The default is `strict` meaning that encoding errors raise a `UnicodeEncodeError`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- The default is `strict` meaning that encoding errors raise a `UnicodeEncodeError`.
- The default is V(strict) meaning that encoding errors raise a C(UnicodeEncodeError).

Copy link
Collaborator

Choose a reason for hiding this comment

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

- See https://docs.python.org/fr/3/library/stdtypes.html#str.encode for more details.
type: string
default: ansible_facts
default: strict
choices: [ backslashreplace, ignore, replace, strict, xmlcharrefreplace ]
env:
- name: ANSIBLE_CACHE_PLUGIN_PREFIX
- name: ANSIBLE_CACHE_REDIS_ENCODING_ERRORS
ini:
- key: fact_caching_prefix
- key: fact_caching_redis_encoding_errors
section: defaults
_keyset_name:
description: User defined name for cache keyset name.
Expand All @@ -47,6 +56,27 @@
- key: fact_caching_redis_keyset_name
section: defaults
version_added: 1.3.0
_prefix:
description: User defined prefix to use when creating the DB entries
type: string
default: ansible_facts
env:
- name: ANSIBLE_CACHE_PLUGIN_PREFIX
ini:
- key: fact_caching_prefix
section: defaults
_retry_on_timeout:
description:
- Controls how socket.timeout errors are handled.
- When set to `false` a TimeoutError will be raised anytime a socket.timeout is encountered.
- When set to `true`, it enable retries like other `socket.error`s.
type: bool
default: false
env:
- name: ANSIBLE_CACHE_REDIS_RETRY_ON_TIMEOUT
ini:
- key: fact_caching_redis_retry_on_timeout
section: defaults
_sentinel_service_name:
description: The redis sentinel service name (or referenced as cluster name).
type: string
Expand All @@ -56,16 +86,107 @@
- key: fact_caching_redis_sentinel
section: defaults
version_added: 1.3.0
_socket_connect_timeout:
description:
- Timeout value, in seconds, for Redis socket connection.
- If not set, connection timeout is disabled.
type: integer
env:
- name: ANSIBLE_CACHE_REDIS_SOCKET_CONNECT_TIMEOUT
ini:
- key: fact_caching_redis_socket_connect_timeout
section: defaults
_socket_keepalive:
description:
- Specifies whether to enable keepalive for Redis socket connection.
type: bool
default: false
env:
- name: ANSIBLE_CACHE_REDIS_SOCKET_KEEPALIVE
ini:
- key: fact_caching_redis_socket_keepalive
section: defaults
_socket_keepalive_options:
description:
- Finer grain control keepalive options when `_socket_keepalive` is set to `true`.
- A comma separated socket options string of <key>:<value> pairs, for example V(TCP_KEEPIDLE:600,TCP_KEEPCNT=10,TCP_KEEPINTVL:300).
- Accepted keys are `TCP_KEEPIDLE`, `TCP_KEEPCNT`, and `TCP_KEEPINTVL`.
- Integers are expected for values.
type: string
env:
- name: ANSIBLE_CACHE_REDIS_SOCKET_KEEPALIVE_OPTIONS
ini:
- key: fact_caching_redis_socket_keepalive_options
section: defaults
_socket_timeout:
description:
- Timeout value, in seconds, for Redis socket connection.
- If not set, timeout is disabled.
type: integer
env:
- name: ANSIBLE_CACHE_REDIS_SOCKET_TIMEOUT
ini:
- key: fact_caching_redis_socket_timeout
section: defaults
_ssl_ca_certs_file:
description: When using SSL on Redis connection, specifies the SSL CA file path.
type: string
env:
- name: ANSIBLE_CACHE_REDIS_SSL_CA_CERTS_FILE
ini:
- key: fact_caching_redis_ssl_ca_certs_file
section: defaults
_ssl_cert_file:
description: When using SSL on Redis connection, specifies the SSL certificate file path.
type: string
env:
- name: ANSIBLE_CACHE_REDIS_SSL_CERT_FILE
ini:
- key: fact_caching_redis_ssl_cert_file
section: defaults
_ssl_cert_reqs:
default: none
type: string
description:
- When using SSL on Redis connection, specifies the security mode to use.
- See https://docs.python.org/3/library/ssl.html#ssl.SSLContext.verify_mode for more details.
env:
- name: ANSIBLE_CACHE_REDIS_SSL_CERT_REQ
ini:
- key: fact_caching_redis_ssl_cert_req
section: defaults
choices: [ none, optionnal, required ]
_ssl_key_file:
description: When using SSL on Redis connection, specifies the SSL key file path.
type: string
env:
- name: ANSIBLE_CACHE_REDIS_SSL_KEY_FILE
ini:
- key: fact_caching_redis_ssl_key_file
section: defaults
_timeout:
default: 86400
description: Expiration timeout in seconds for the cache plugin data. Set to 0 to never expire
type: integer
# TODO: determine whether it is OK to change to: type: float
description: Expiration timeout in seconds for the cache plugin data. Set to 0 to never expire
env:
- name: ANSIBLE_CACHE_PLUGIN_TIMEOUT
ini:
- key: fact_caching_timeout
section: defaults
_uri:
description:
- A colon separated string of connection information for Redis.
- The format is V(host:port:db:password), for example V(localhost:6379:0:changeme).
- To use encryption in transit, prefix the connection with V(tls://), as in V(tls://localhost:6379:0:changeme).
- To use redis sentinel, use separator V(;), for example V(localhost:26379;localhost:26379;0:changeme). Requires redis>=2.9.0.
type: string
required: true
env:
- name: ANSIBLE_CACHE_PLUGIN_CONNECTION
ini:
- key: fact_caching_connection
section: defaults
'''

import re
Expand Down Expand Up @@ -97,8 +218,11 @@ class CacheModule(BaseCacheModule):
performance.
"""
_sentinel_service_name = None
_encoding_errors_choices = ['backslashreplace', 'ignore', 'replace', 'strict', 'xmlcharrefreplace']
_socket_keepalive_available_opts = ['TCP_KEEPIDLE', 'TCP_KEEPCNT', 'TCP_KEEPINTVL']
re_url_conn = re.compile(r'^([^:]+|\[[^]]+\]):(\d+):(\d+)(?::(.*))?$')
re_sent_conn = re.compile(r'^(.*):(\d+)$')
re_socket_keepalive_opts = re.compile(r'^(\w+:\d+)(?:,(\w+:\d+))+$')

def __init__(self, *args, **kwargs):
uri = ''
Expand All @@ -110,17 +234,71 @@ def __init__(self, *args, **kwargs):
self._prefix = self.get_option('_prefix')
self._keys_set = self.get_option('_keyset_name')
self._sentinel_service_name = self.get_option('_sentinel_service_name')
self._decode_responses = bool(self.get_option('_decode_responses'))
self._encoding = self.get_option('_encoding')
self._encoding_errors = self.get_option('_encoding_errors')
self._retry_on_timeout = bool(self.get_option('_retry_on_timeout'))
self._socket_keepalive = self.get_option('_socket_keepalive')
self._socket_connect_timeout = int(self.get_option('_socket_connect_timeout')) \
if self._socket_keepalive and self.get_option('_socket_connect_timeout') else None
Comment on lines +242 to +243
Copy link
Collaborator

Choose a reason for hiding this comment

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

As a personal preference for style, I do like using the x = a if b else c construct for smaller expressions. In this case, I think it might be more readable expanding this to a "normal" if block.

self._socket_keepalive_options = self._parse_socket_options(self.get_option('_socket_keepalive_options')) \
if self._socket_keepalive and self.get_option('_socket_keepalive_options') else None
self._socket_timeout = int(self.get_option('_socket_timeout')) \
if self._socket_keepalive and self.get_option('_socket_timeout') else None
self._ssl_ca_certs_file = self.get_option('_ssl_ca_certs_file')
self._ssl_cert_file = self.get_option('_ssl_cert_file')
self._ssl_cert_reqs = self.get_option('_ssl_cert_reqs')
self._ssl_key_file = self.get_option('_ssl_key_file')

if not HAS_REDIS:
raise AnsibleError("The 'redis' python module (version 2.4.5 or newer) is required for the redis fact cache, 'pip install redis'")

if self._encoding_errors not in self._encoding_errors_choices:
raise AnsibleError("Unsupported value '%s' for Redis cache plugin parameter '_encoding_errors'" % (self._encoding_errors))

self._cache = {}
kw = {}
kw = {
'decode_responses': self._decode_responses,
'encoding_errors': self._encoding_errors,
'encoding': self._encoding,
'retry_on_timeout': self._retry_on_timeout,
'socket_connect_timeout': self._socket_connect_timeout,
'socket_keepalive_options': self._socket_keepalive_options,
'socket_keepalive': self._socket_keepalive,
'socket_timeout': self._socket_timeout,
}

# tls connection
tlsprefix = 'tls://'
if uri.startswith(tlsprefix):
kw['ssl'] = True
from os import access, R_OK
from os.path import isfile
import ssl

if not self._ssl_cert_reqs.upper() in list(map(lambda x: x.name.split('_')[1], ssl.VerifyMode)):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Coercing to list is redundant here:

Suggested change
if not self._ssl_cert_reqs.upper() in list(map(lambda x: x.name.split('_')[1], ssl.VerifyMode)):
if not self._ssl_cert_reqs.upper() in map(lambda x: x.name.split('_')[1], ssl.VerifyMode):

raise AnsibleError("Unsupported value '%s' for Redis cache plugin parameter '_ssl_cert_reqs'" % (self._ssl_cert_reqs))

if self._ssl_ca_certs_file:
if not isfile(self._ssl_ca_certs_file) and not access(self._ssl_ca_certs_file, R_OK):
raise AnsibleError("File %s doesn't exist or isn't readable for Redis cache plugin parameter '_ssl_ca_certs_file'" %
self._ssl_ca_certs_file)

if self._ssl_cert_file:
if not isfile(self._ssl_cert_file) and not access(self._ssl_cert_file, R_OK):
raise AnsibleError("File %s doesn't exist or isn't readable for Redis cache plugin parameter '_ssl_cert_file'" % self._ssl_cert_file)

if self._ssl_key_file:
if not isfile(self._ssl_key_file) and not access(self._ssl_key_file, R_OK):
raise AnsibleError("File %s doesn't exist or isn't readable for Redis cache plugin parameter '_ssl_key_file'" % self._ssl_key_file)

kw.update({
'ssl': True,
'ssl_keyfile': self._ssl_key_file,
'ssl_certfile': self._ssl_cert_file,
'ssl_cert_reqs': self._ssl_cert_reqs,
'ssl_ca_certs': self._ssl_ca_certs_file
})

uri = uri[len(tlsprefix):]

# redis sentinel connection
Expand All @@ -132,6 +310,7 @@ def __init__(self, *args, **kwargs):
self._db = StrictRedis(*connection, **kw)

display.vv('Redis connection: %s' % self._db)
display.vvv("Redis connection kwargs: %s" % ({**self._db.get_connection_kwargs(), **{'password': '****'}}))
Copy link
Collaborator

@russoz russoz Aug 6, 2024

Choose a reason for hiding this comment

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

I tried to find documentation related to this method get_connection_kwargs() in the Redis lib, but could not find anything very straightforward. Code also seems to be referencing a dynamic connection class. I am concerned that some sensitive info might be leaked by printing the entire result.


@staticmethod
def _parse_connection(re_patt, uri):
Expand All @@ -140,6 +319,18 @@ def _parse_connection(re_patt, uri):
raise AnsibleError("Unable to parse connection string")
return match.groups()

def _parse_socket_options(self, options):
if not self.re_socket_keepalive_opts.match(options):
raise AnsibleError("Unable to parse Redis cache socket keepalive options string")
import socket
Copy link
Collaborator

Choose a reason for hiding this comment

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

The import statement should be by the top of the file, after the docs.

opts = {}
for opt in options.split(','):
key, value = opt.split(':')
if key not in self._socket_keepalive_available_opts:
raise AnsibleError("Option '%s' is not available for parameter '_socket_keepalive_options' for Redis cache plugin" % (key))
opts[getattr(socket, key)] = int(value)
return opts

def _get_sentinel_connection(self, uri, kw):
"""
get sentinel connection details from _uri
Expand Down
Loading