Skip to content

Commit b9ebfb8

Browse files
binarinbinarin
committed
Fix ssl port handling in prometheus plugin
All ssl options were stored in the same proplist, and the code was then trying to determine whether an option actually belongs to ranch ssl options or not. Some keys landed in the wrong place, like it did happen in #2975 - different ports were mentioned in listener config (default at top-level, and non-default in `ssl_opts`). Then `ranch` and `rabbitmq_web_dispatch` were treating this differently. This change just moves all ranch ssl opts into proper place using schema, removing any need for guessing in code. The only downside is that advanced config compatibility is broken.
1 parent 9cf18e8 commit b9ebfb8

File tree

3 files changed

+53
-54
lines changed

3 files changed

+53
-54
lines changed

‎deps/rabbitmq_prometheus/priv/schema/rabbitmq_prometheus.schema

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -62,52 +62,53 @@
6262
[{datatype, integer}]}.
6363
{mapping, "prometheus.ssl.ip", "rabbitmq_prometheus.ssl_config.ip",
6464
[{datatype, string}, {validators, ["is_ip"]}]}.
65-
{mapping, "prometheus.ssl.certfile", "rabbitmq_prometheus.ssl_config.certfile",
65+
66+
{mapping, "prometheus.ssl.certfile", "rabbitmq_prometheus.ssl_config.ssl_opts.certfile",
6667
[{datatype, string}, {validators, ["file_accessible"]}]}.
67-
{mapping, "prometheus.ssl.keyfile", "rabbitmq_prometheus.ssl_config.keyfile",
68+
{mapping, "prometheus.ssl.keyfile", "rabbitmq_prometheus.ssl_config.ssl_opts.keyfile",
6869
[{datatype, string}, {validators, ["file_accessible"]}]}.
69-
{mapping, "prometheus.ssl.cacertfile", "rabbitmq_prometheus.ssl_config.cacertfile",
70+
{mapping, "prometheus.ssl.cacertfile", "rabbitmq_prometheus.ssl_config.ssl_opts.cacertfile",
7071
[{datatype, string}, {validators, ["file_accessible"]}]}.
71-
{mapping, "prometheus.ssl.password", "rabbitmq_prometheus.ssl_config.password",
72+
{mapping, "prometheus.ssl.password", "rabbitmq_prometheus.ssl_config.ssl_opts.password",
7273
[{datatype, string}]}.
7374

74-
{mapping, "prometheus.ssl.verify", "rabbitmq_prometheus.ssl_config.verify", [
75+
{mapping, "prometheus.ssl.verify", "rabbitmq_prometheus.ssl_config.ssl_opts.verify", [
7576
{datatype, {enum, [verify_peer, verify_none]}}]}.
7677

77-
{mapping, "prometheus.ssl.fail_if_no_peer_cert", "rabbitmq_prometheus.ssl_config.fail_if_no_peer_cert", [
78+
{mapping, "prometheus.ssl.fail_if_no_peer_cert", "rabbitmq_prometheus.ssl_config.ssl_opts.fail_if_no_peer_cert", [
7879
{datatype, {enum, [true, false]}}]}.
7980

80-
{mapping, "prometheus.ssl.honor_cipher_order", "rabbitmq_prometheus.ssl_config.honor_cipher_order",
81+
{mapping, "prometheus.ssl.honor_cipher_order", "rabbitmq_prometheus.ssl_config.ssl_opts.honor_cipher_order",
8182
[{datatype, {enum, [true, false]}}]}.
8283

83-
{mapping, "prometheus.ssl.honor_ecc_order", "rabbitmq_prometheus.ssl_config.honor_ecc_order",
84+
{mapping, "prometheus.ssl.honor_ecc_order", "rabbitmq_prometheus.ssl_config.ssl_opts.honor_ecc_order",
8485
[{datatype, {enum, [true, false]}}]}.
8586

86-
{mapping, "prometheus.ssl.reuse_sessions", "rabbitmq_prometheus.ssl_config.reuse_sessions",
87+
{mapping, "prometheus.ssl.reuse_sessions", "rabbitmq_prometheus.ssl_config.ssl_opts.reuse_sessions",
8788
[{datatype, {enum, [true, false]}}]}.
8889

89-
{mapping, "prometheus.ssl.secure_renegotiate", "rabbitmq_prometheus.ssl_config.secure_renegotiate",
90+
{mapping, "prometheus.ssl.secure_renegotiate", "rabbitmq_prometheus.ssl_config.ssl_opts.secure_renegotiate",
9091
[{datatype, {enum, [true, false]}}]}.
9192

92-
{mapping, "prometheus.ssl.client_renegotiation", "rabbitmq_prometheus.ssl_config.client_renegotiation",
93+
{mapping, "prometheus.ssl.client_renegotiation", "rabbitmq_prometheus.ssl_config.ssl_opts.client_renegotiation",
9394
[{datatype, {enum, [true, false]}}]}.
9495

95-
{mapping, "prometheus.ssl.depth", "rabbitmq_prometheus.ssl_config.depth",
96+
{mapping, "prometheus.ssl.depth", "rabbitmq_prometheus.ssl_config.ssl_opts.depth",
9697
[{datatype, integer}, {validators, ["byte"]}]}.
9798

98-
{mapping, "prometheus.ssl.versions.$version", "rabbitmq_prometheus.ssl_config.versions",
99+
{mapping, "prometheus.ssl.versions.$version", "rabbitmq_prometheus.ssl_config.ssl_opts.versions",
99100
[{datatype, atom}]}.
100101

101-
{translation, "rabbitmq_prometheus.ssl_config.versions",
102+
{translation, "rabbitmq_prometheus.ssl_config.ssl_opts.versions",
102103
fun(Conf) ->
103104
Settings = cuttlefish_variable:filter_by_prefix("prometheus.ssl.versions", Conf),
104105
[V || {_, V} <- Settings]
105106
end}.
106107

107-
{mapping, "prometheus.ssl.ciphers.$cipher", "rabbitmq_prometheus.ssl_config.ciphers",
108+
{mapping, "prometheus.ssl.ciphers.$cipher", "rabbitmq_prometheus.ssl_config.ssl_opts.ciphers",
108109
[{datatype, string}]}.
109110

110-
{translation, "rabbitmq_prometheus.ssl_config.ciphers",
111+
{translation, "rabbitmq_prometheus.ssl_config.ssl_opts.ciphers",
111112
fun(Conf) ->
112113
Settings = cuttlefish_variable:filter_by_prefix("prometheus.ssl.ciphers", Conf),
113114
lists:reverse([V || {_, V} <- Settings])

‎deps/rabbitmq_prometheus/src/rabbit_prometheus_app.erl

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,7 @@ has_configured_listener(Key) ->
7474
end.
7575

7676
get_tls_listener() ->
77-
{ok, Listener0} = application:get_env(rabbitmq_prometheus, ssl_config),
78-
case proplists:get_value(cowboy_opts, Listener0) of
79-
undefined ->
80-
[{ssl, true}, {ssl_opts, Listener0}];
81-
CowboyOpts ->
82-
Listener1 = lists:keydelete(cowboy_opts, 1, Listener0),
83-
[{ssl, true}, {ssl_opts, Listener1}, {cowboy_opts, CowboyOpts}]
84-
end.
77+
[{ssl, true} | application:get_env(rabbitmq_prometheus, ssl_config, [])].
8578

8679
get_tcp_listener() ->
8780
application:get_env(rabbitmq_prometheus, tcp_config, []).
@@ -111,8 +104,9 @@ ensure_port_and_protocol(tcp, Protocol, Listener) ->
111104
do_ensure_port_and_protocol(?DEFAULT_PORT, Protocol, Listener).
112105

113106
do_ensure_port_and_protocol(Port, Protocol, Listener) ->
114-
%% include default port if it's not provided in the config
115-
%% as Cowboy won't start if the port is missing
107+
%% Include default port if it's not provided in the config
108+
%% as Cowboy won't start if the port is missing.
109+
%% Protocol is displayed in mgmt UI and CLI output.
116110
M0 = maps:from_list(Listener),
117111
M1 = maps:merge(#{port => Port, protocol => Protocol}, M0),
118112
{ok, maps:to_list(M1)}.

‎deps/rabbitmq_prometheus/test/config_schema_SUITE_data/rabbitmq_prometheus.snippets

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,14 @@
144144
{ssl_config,[
145145
{ip, "192.168.1.2"},
146146
{port,15691},
147-
{cacertfile,"test/config_schema_SUITE_data/certs/cacert.pem"},
148-
{certfile,"test/config_schema_SUITE_data/certs/cert.pem"},
149-
{keyfile,"test/config_schema_SUITE_data/certs/key.pem"},
150-
{verify, verify_none},
151-
{fail_if_no_peer_cert, false}
152-
]}
147+
{ssl_opts, [
148+
{cacertfile,"test/config_schema_SUITE_data/certs/cacert.pem"},
149+
{certfile,"test/config_schema_SUITE_data/certs/cert.pem"},
150+
{keyfile,"test/config_schema_SUITE_data/certs/key.pem"},
151+
{verify, verify_none},
152+
{fail_if_no_peer_cert, false}
153+
]}
154+
]}
153155
]}],
154156
[rabbitmq_prometheus]},
155157

@@ -184,31 +186,33 @@
184186
{ssl_config,[
185187
{ip, "192.168.1.2"},
186188
{port,15691},
187-
{cacertfile,"test/config_schema_SUITE_data/certs/cacert.pem"},
188-
{certfile,"test/config_schema_SUITE_data/certs/cert.pem"},
189-
{keyfile,"test/config_schema_SUITE_data/certs/key.pem"},
189+
{ssl_opts, [
190+
{cacertfile,"test/config_schema_SUITE_data/certs/cacert.pem"},
191+
{certfile,"test/config_schema_SUITE_data/certs/cert.pem"},
192+
{keyfile,"test/config_schema_SUITE_data/certs/key.pem"},
190193

191-
{verify, verify_peer},
192-
{fail_if_no_peer_cert, false},
194+
{verify, verify_peer},
195+
{fail_if_no_peer_cert, false},
193196

194-
{honor_cipher_order, true},
195-
{honor_ecc_order, true},
196-
{client_renegotiation, false},
197-
{secure_renegotiate, true},
197+
{honor_cipher_order, true},
198+
{honor_ecc_order, true},
199+
{client_renegotiation, false},
200+
{secure_renegotiate, true},
198201

199-
{versions,['tlsv1.2','tlsv1.1']},
200-
{ciphers, [
201-
"ECDHE-ECDSA-AES256-GCM-SHA384",
202-
"ECDHE-RSA-AES256-GCM-SHA384",
203-
"ECDHE-ECDSA-AES256-SHA384",
204-
"ECDHE-RSA-AES256-SHA384",
205-
"ECDH-ECDSA-AES256-GCM-SHA384",
206-
"ECDH-RSA-AES256-GCM-SHA384",
207-
"ECDH-ECDSA-AES256-SHA384",
208-
"ECDH-RSA-AES256-SHA384",
209-
"DHE-RSA-AES256-GCM-SHA384"
210-
]}
211-
]}
202+
{versions,['tlsv1.2','tlsv1.1']},
203+
{ciphers, [
204+
"ECDHE-ECDSA-AES256-GCM-SHA384",
205+
"ECDHE-RSA-AES256-GCM-SHA384",
206+
"ECDHE-ECDSA-AES256-SHA384",
207+
"ECDHE-RSA-AES256-SHA384",
208+
"ECDH-ECDSA-AES256-GCM-SHA384",
209+
"ECDH-RSA-AES256-GCM-SHA384",
210+
"ECDH-ECDSA-AES256-SHA384",
211+
"ECDH-RSA-AES256-SHA384",
212+
"DHE-RSA-AES256-GCM-SHA384"
213+
]}
214+
]}
215+
]}
212216
]}],
213217
[rabbitmq_prometheus]},
214218

0 commit comments

Comments
 (0)