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 missing redis ssl params #826

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
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
28 changes: 26 additions & 2 deletions manifests/integrations/redis.pp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,20 @@
# The main redis port.
# $ports
# Array of redis ports: overrides port (optional)
# $ssl
# Enable SSL/TLS encryption for the check (optional)
# $ssl_keyfile
# The path to the client-side private keyfile (optional)
# $ssl_certfile
# The path to the client-side certificate file (optional)
# $ssl_ca_certs
# The path to the ca_certs file (optional)
# $ssl_cert_reqs
# Specifies whether a certificate is required from the
# other side of the connection, and whether it's validated if provided (optional)
# * 0 for ssl.CERT_NONE (certificates ignored)
# * 1 for ssl.CERT_OPTIONAL (not required, but validated if provided)
# * 2 for ssl.CERT_REQUIRED (required and validated)
# $slowlog_max_len
# The max length of the slow-query log (optional)
# $tags
Expand Down Expand Up @@ -40,11 +54,16 @@
# warn_on_missing_keys: true
# command_stats: false
#
class datadog_agent::integrations::redis(
class datadog_agent::integrations::redis (
String $host = 'localhost',
String $password = '',
Variant[String, Integer] $port = '6379',
Optional[Array] $ports = undef,
Boolean $ssl = false,
String $ssl_keyfile = '',
String $ssl_certfile = '',
String $ssl_ca_certs = '',
Variant[String, Integer] $ssl_cert_reqs = '',
Variant[String, Integer] $slowlog_max_len = '',
Array $tags = [],
Array $keys = [],
Expand All @@ -66,6 +85,11 @@
'host' => $host,
'password' => $password,
'port' => $instance_port,
'ssl' => $ssl,
'ssl_keyfile' => $ssl_keyfile,
'ssl_certfile' => $ssl_certfile,
'ssl_ca_certs' => $ssl_ca_certs,
'ssl_cert_reqs' => $ssl_cert_reqs,
'slowlog_max_len' => $slowlog_max_len,
'tags' => $tags,
'keys' => $keys,
Expand Down Expand Up @@ -96,7 +120,7 @@

if !$instances and $host {
$_instances = $_port_instances
} elsif !$instances{
} elsif !$instances {
$_instances = []
} else {
$_instances = $instances
Expand Down
30 changes: 30 additions & 0 deletions spec/classes/datadog_agent_integrations_redis_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,36 @@
it { is_expected.to contain_file(conf_file).without_content(%r{command_stats: true}) }
end

context 'with instances set and port nil' do
let(:params) do
{
instances: [
{
'host' => 'redis1',
'password' => 'hunter2',
'tags' => ['foo', 'bar'],
'keys' => ['baz', 'bat'],
},
Comment on lines +147 to +152

Choose a reason for hiding this comment

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

Code Quality Violation

Consider using symbols instead of string hash keys (...read more)

In Ruby, it is a best practice to use symbols instead of strings as hash keys. This rule emphasizes that it's more efficient and idiomatic to use symbols for this purpose. Symbols are immutable and unique, which makes them ideal for identifying things, whereas strings are mutable and can create multiple objects for the same sequence of characters.

The importance of this rule lies in the performance and memory usage of your Ruby application. Using symbols as hash keys reduces memory usage because they are stored in memory only once during a Ruby process. This can make a significant difference in the efficiency of your application, especially when dealing with large data sets.

To ensure you're following good coding practices, always use symbols for hash keys unless there's a specific reason to use a string. A simple refactoring from values = { 'foo' => 42, 'bar' => 99, 'baz' => 123 } to values = { foo: 42, bar: 99, baz: 123 } will make your code compliant with this rule. This not only improves your code's performance but also makes it more readable and consistent with Ruby's conventions.

View in Datadog  Leave us feedback  Documentation

{
'host' => 'redis1',
'password' => 'hunter2',
'tags' => ['foo', 'bar'],
'keys' => ['baz', 'bat'],
},
],
}
end

it { is_expected.to contain_file(conf_file).with_content(%r{host: redis1}) }
it { is_expected.to contain_file(conf_file).with_content(%r{^[^#]*password: hunter2}) }
it { is_expected.to contain_file(conf_file).with_content(%r{port: 6379}) }
it { is_expected.to contain_file(conf_file).with_content(%r{tags:.*\s+- foo\s+- bar}) }
it { is_expected.to contain_file(conf_file).with_content(%r{keys:.*\s+- baz\s+- bat}) }
it { is_expected.to contain_file(conf_file).without_content(%r{^[^#]*slowlog-max-len: 5309}) }
it { is_expected.to contain_file(conf_file).without_content(%r{warn_on_missing_keys: false}) }
it { is_expected.to contain_file(conf_file).without_content(%r{command_stats: true}) }
end

context 'with only keys' do
let(:params) do
{
Expand Down
19 changes: 17 additions & 2 deletions templates/agent-conf.d/redisdb.yaml.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,27 @@ init_config:
instances:
<%- (Array(@_instances)).each do |instance| -%>
- host: <%= instance['host'] %>
port: <%= instance['port'] %>
port: <%= instance['port'] %>
# unix_socket_path: /var/run/redis/redis.sock # optional, can be used in lieu of host/port
<% if instance['password'] and ! instance['password'].empty? -%>
password: <%= instance['password'] %>
<% end -%>
<% if !instance['ssl'].nil? -%>
ssl: <%= instance['ssl'] %>
<% end -%>
<% if instance['ssl_keyfile'] and ! instance['ssl_keyfile'].empty? -%>
ssl_keyfile: <%= instance['ssl_keyfile'] %>
<% end -%>
<% if instance['ssl_certfile'] and ! instance['ssl_certfile'].empty? -%>
ssl_keyfile: <%= instance['ssl_keyfile'] %>
<% end -%>
<% if instance['ssl_ca_certs'] and ! instance['ssl_ca_certs'].empty? -%>
ssl_keyfile: <%= instance['ssl_keyfile'] %>
<% end -%>
<% if instance['ssl_cert_reqs'] and ! instance['ssl_cert_reqs'].to_s.empty? -%>
ssl_cert_reqs: <%= instance['ssl_cert_reqs'] %>
<% end -%>
<% if instance['slowlog_max_len'] and ! instance['slowlog_max_len'].to_s.empty? -%>
# unix_socket_path: /var/run/redis/redis.sock # optional, can be used in lieu of host/port
slowlog-max-len: <%= instance['slowlog_max_len'] %>
<% end -%>
<% if !instance['warn_on_missing_keys'].nil? -%>
Expand Down
Loading