Commit Graph

129 Commits

Author SHA1 Message Date
Amaury Denoyelle
e4a617c931 MINOR: action: replace match_pfx by a keyword flags field
Define a new keyword flag KWF_MATCH_PREFIX. This is used to replace the
match_pfx field of action struct.

This has the benefit to have more explicit action declaration, and now
it is possible to quickly implement experimental actions.
2021-05-07 14:35:01 +02:00
Willy Tarreau
b205bfdab7 CLEANUP: cli/tree-wide: properly re-align the CLI commands' help messages
There were 102 CLI commands whose help were zig-zagging all along the dump
making them unreadable. This patch realigns all these messages so that the
command now uses up to 40 characters before the delimiting colon. About a
third of the commands did not correctly list their arguments which were
added after the first version, so they were all updated. Some abuses of
the term "id" were fixed to use a more explanatory term. The
"set ssl ocsp-response" command was not listed because it lacked a help
message, this was fixed as well. The deprecated enable/disable commands
for agent/health/server were prominently written as deprecated. Whenever
possible, clearer explanations were provided.
2021-05-07 11:51:26 +02:00
Willy Tarreau
2b71810cb3 CLEANUP: lists/tree-wide: rename some list operations to avoid some confusion
The current "ADD" vs "ADDQ" is confusing because when thinking in terms
of appending at the end of a list, "ADD" naturally comes to mind, but
here it does the opposite, it inserts. Several times already it's been
incorrectly used where ADDQ was expected, the latest of which was a
fortunate accident explained in 6fa922562 ("CLEANUP: stream: explain
why we queue the stream at the head of the server list").

Let's use more explicit (but slightly longer) names now:

   LIST_ADD        ->       LIST_INSERT
   LIST_ADDQ       ->       LIST_APPEND
   LIST_ADDED      ->       LIST_INLIST
   LIST_DEL        ->       LIST_DELETE

The same is true for MT_LISTs, including their "TRY" variant.
LIST_DEL_INIT keeps its short name to encourage to use it instead of the
lazier LIST_DELETE which is often less safe.

The change is large (~674 non-comment entries) but is mechanical enough
to remain safe. No permutation was performed, so any out-of-tree code
can easily map older names to new ones.

The list doc was updated.
2021-04-21 09:20:17 +02:00
Emeric Brun
c8f3e45c6a MEDIUM: resolvers: add support of tcp address on nameserver line.
This patch re-works configuration parsing, it removes the "server"
lines from "resolvers" sections introduced in commit 56fc5d9eb:
MEDIUM: resolvers: add supports of TCP nameservers in resolvers.

It also extends the nameserver lines to support stream server
addresses such as:

resolvers
  nameserver localhost tcp@127.0.0.1:53

Doing so, a part of nameserver's init code was factorized in
function 'parse_resolvers' and removed from 'post_parse_resolvers'.
2021-04-08 14:20:40 +02:00
Amaury Denoyelle
ce44482fe5 REORG: global: move initcall register code in a dedicated file
Create a new module init which contains code related to REGISTER_*
macros for initcalls. init.h is included in api.h to make init code
available to all modules.

It's a step to clean up a bit haproxy.c/global.h.
2021-03-26 15:28:33 +01:00
Willy Tarreau
70490ebb12 CLEANUP: resolvers: use pool_zalloc() in resolv_link_resolution()
This one used to alloc then zero the area, let's have the allocator do it.
2021-03-22 23:19:28 +01:00
Amaury Denoyelle
30c0537f5a REORG: server: use flags for parse_server
Modify the API of parse_server function. Use flags to describe the type
of the parsed server instead of discrete arguments. These flags can be
used to specify if a server/default-server/server-template is parsed.
Additional parameters are also specified (parsing of the address
required, resolve of a name must be done immediately).

It is now unneeded to use strcmp on args[0] in parse_server. Also, the
calls to parse_server are more explicit thanks to the flags.
2021-03-18 15:37:05 +01:00
Christopher Faulet
db31b4486c CLEANUP: resolvers: Perform unsafe loop on requester list when possible
When answer list of a response is checked, it is useless to perform a safe
loop on the requester list.
2021-03-12 17:42:47 +01:00
Christopher Faulet
e8674c7184 MINOR: resolvers: Don't try to match immediatly renewed ADD items
The loop looking for existing ADD items to renew their last_seen must ignore
the items already renewed in the same loop. To do so, we rely on the
last_seen time. because it is now based on now_ms, it is safe.

Doing so avoid to match several time the same ADD item when the same IP
address is found in several ADD item. This reduces the number of extra DNS
resolutions.

This patch depends on "MINOR: resolvers: Use milliseconds for cached items
in resolver responses". Both may be backported as far as 2.2 if necessary.
2021-03-12 17:42:45 +01:00
Christopher Faulet
55c1c4053f MINOR: resolvers: Use milliseconds for cached items in resolver responses
The last time when an item was seen in a resolver responses is now stored in
milliseconds instead of seconds. This avoid some corner-cases at the
edges. This also simplifies time comparisons.
2021-03-12 17:41:28 +01:00
Christopher Faulet
d83a6df5cd BUG/MEDIUM: resolvers: Skip DNS resolution at startup if SRV resolution is set
At startup, if a SRV resolution is set for a server, no DNS resolution is
created. We must wait the first SRV resolution to know if it must be
triggered. It is important to do so for two reasons.

First, during a "classical" startup, a server based on a SRV resolution has
no hostname. Thus the created DNS resolution is useless. Best waiting the
first SRV resolution. It is not really a bug at this stage, it is just
useless.

Second, in the same situation, if the server state is loaded from a file,
its hosname will be set a bit later. Thus, if there is no additionnal record
for this server, because there is already a DNS resolution, it inhibits any
new DNS resolution. But there is no hostname attached to the existing DNS
resolution. So no resolution is performed at all for this server.

To avoid any problem, it is fairly easier to handle this special case during
startup. But this means we must be prepared to have no "resolv_requester"
field for a server at runtime.

This patch must be backported as far as 2.2.
2021-03-12 17:41:28 +01:00
Christopher Faulet
0efc0993ec BUG/MEDIUM: resolvers: Don't release resolution from a requester callbacks
Another way to say it: "Safely unlink requester from a requester callbacks".

Requester callbacks must never try to unlink a requester from a resolution, for
the current requester or another one. First, these callback functions are called
in a loop on a request list, not necessarily safe. Thus unlink resolution at
this place, may be unsafe. And it is useless to try to make these loops safe
because, all this stuff is placed in a loop on a resolution list. Unlink a
requester may lead to release a resolution if it is the last requester.

However, the unkink is necessary because we cannot reset the server state
(hostname and IP) with some pending DNS resolution on it. So, to workaround
this issue, we introduce the "safe" unlink. It is only performed from a
requester callback. In this case, the unlink function never releases the
resolution, it only reset it if necessary. And when a resolution is found
with an empty requester list, it is released.

This patch depends on the following commits :

 * MINOR: resolvers: Purge answer items when a SRV resolution triggers an error
 * MINOR: resolvers: Use a function to remove answers attached to a resolution
 * MINOR: resolvers: Directly call srvrq_update_srv_state() when possible
 * MINOR: resolvers: Add function to change the srv status based on SRV resolution

All the series must be backported as far as 2.2. It fixes a regression
introduced by the commit b4badf720 ("BUG/MINOR: resolvers: new callback to
properly handle SRV record errors").

don't release resolution from requester cb
2021-03-12 17:41:28 +01:00
Christopher Faulet
6b117aed49 MINOR: resolvers: Directly call srvrq_update_srv_state() when possible
When the server status must be updated from the result of a SRV resolution,
we can directly call srvrq_update_srv_state(). It is simpler and this avoid
a test on the server DNS resolution.

This patch is mandatory for the next commit. It also rely on "MINOR:
resolvers: Directly call srvrq_update_srv_state() when possible".
2021-03-12 17:41:28 +01:00
Christopher Faulet
1dec5c7934 MINOR: resolvers: Use a function to remove answers attached to a resolution
resolv_purge_resolution_answer_records() must be used to removed all answers
attached to a resolution. For now, it is only used when a resolution is
released.
2021-03-12 17:41:28 +01:00
Christopher Faulet
3e0600fbbf BUG/MEDIUM: resolvers: Trigger a DNS resolution if an ADD item is obsolete
When a ADD item attached to a SRV item is removed because it is obsolete, we
must trigger a DNS resolution to be sure the hostname still resolves or
not. There is no other way to be the entry is still valid. And we cannot set
the server in RMAINT immediatly, because a DNS server may be inconsitent and
may stop to add some additionnal records.

The opposite is also true. If a valid ADD item is still attached to a SRV
item, any DNS resolution must be stopped. There is no reason to perform
extra resolution in this case.

This patch must be backported as far as 2.2.
2021-03-12 17:41:28 +01:00
Baptiste Assmann
6a8d11dc80 MINOR: resolvers: new function find_srvrq_answer_record()
This function search for a SRV answer item associated to a requester
whose type is server.
This is mainly useful to "link" a server to its SRV record when no
additional record were found to configure the IP address.

This patch is required by a bug fix.
2021-03-12 17:41:28 +01:00
Christopher Faulet
77f860699c BUG/MEDIUM: resolvers: Fix the loop looking for an existing ADD item
For each ADD item found in a SRV response, we try to find a corresponding
ADD item already attached to an existing SRV item. If found, the ADD
last_seen time is updated, otherwise we try to find a SRV item with no ADD
to attached the new one.

However, the loop is buggy. Instead of comparing 2 ADD items, it compares
the new ADD item with the SRV item. Because of this bug, we are unable to
renew last_seen time of existing ADD.

This patch must be backported as far as 2.2.
2021-03-12 17:41:24 +01:00
Christopher Faulet
ab177ac1f3 BUG/MEDIUM: resolvers: Don't set an address-less server as UP
when a server status is updated based on a SRV item, it is always set to UP,
regardless it has an IP address defined or not. For instance, if only a SRV
item is received, with no additional record, only the server hostname is
defined. We must wait to have an IP address to set the server as UP.

This patch must be backported as far as 2.2.
2021-03-12 16:43:37 +01:00
Christopher Faulet
bca680ba90 BUG/MINOR: resolvers: Unlink DNS resolution to set RMAINT on SRV resolution
When a server is set in RMAINT becaues of a SRV resolution failure, the
server DNS resolution, if any, must be unlink first. It is mandatory to
handle the change in the context of a SRV resolution.

This patch must be backported as far as 2.2.
2021-03-12 16:43:37 +01:00
Christopher Faulet
5037c06d91 Revert "BUG/MINOR: resolvers: Only renew TTL for SRV records with an additional record"
This reverts commit a331a1e8eb.

This commit fixes a real bug, but it also reveals some hidden bugs, mostly
because of some design issues. Thus, in itself, it create more problem than
it solves. So revert it for now. All known bugs will be addressed in next
commits.

This patch should be backported as far as 2.2.
2021-03-12 16:43:37 +01:00
Willy Tarreau
144f84a09d MEDIUM: task: extend the state field to 32 bits
It's been too short for quite a while now and is now full. It's still
time to extend it to 32-bits since we have room for this without
wasting any space, so we now gained 16 new bits for future flags.

The values were not reassigned just in case there would be a few
hidden u16 or short somewhere in which these flags are placed (as
it used to be the case with stream->pending_events).

The patch is tagged MEDIUM because this required to update the task's
process() prototype to use an int instead of a short, that's quite a
bunch of places.
2021-03-05 08:30:08 +01:00
Willy Tarreau
61cfdf4fd8 CLEANUP: tree-wide: replace free(x);x=NULL with ha_free(&x)
This makes the code more readable and less prone to copy-paste errors.
In addition, it allows to place some __builtin_constant_p() predicates
to trigger a link-time error in case the compiler knows that the freed
area is constant. It will also produce compile-time error if trying to
free something that is not a regular pointer (e.g. a function).

The DEBUG_MEM_STATS macro now also defines an instance for ha_free()
so that all these calls can be checked.

178 occurrences were converted. The vast majority of them were handled
by the following Coccinelle script, some slightly refined to better deal
with "&*x" or with long lines:

  @ rule @
  expression E;
  @@
  - free(E);
  - E = NULL;
  + ha_free(&E);

It was verified that the resulting code is the same, more or less a
handful of cases where the compiler optimized slightly differently
the temporary variable that holds the copy of the pointer.

A non-negligible amount of {free(str);str=NULL;str_len=0;} are still
present in the config part (mostly header names in proxies). These
ones should also be cleaned for the same reasons, and probably be
turned into ist strings.
2021-02-26 21:21:09 +01:00
Christopher Faulet
69beaa91d5 REORG: server: Export and rename some functions updating server info
Some static functions are now exported and renamed to follow the same
pattern of other exported functions. Here is the list :

 * update_server_fqdn: Renamed to srv_update_fqdn and exported
 * update_server_check_addr_port: renamed to srv_update_check_addr_port and exported
 * update_server_agent_addr_port: renamed to srv_update_agent_addr_port and exported
 * update_server_addr: renamed to srv_update_addr
 * update_server_addr_potr: renamed to srv_update_addr_port
 * srv_prepare_for_resolution: exported

This change is mandatory to move all functions dealing with the server-state
files in a separate file.
2021-02-25 10:02:39 +01:00
Christopher Faulet
52d4d30109 BUG/MEDIUM: resolvers: Reset server address and port for obselete SRV records
When a SRV record expires, the ip/port assigned to the associated server are
now removed. Otherwise, the server is stopped but keeps its ip/port while
the server hostname is removed. It is confusing when the servers state are
retrieve on the CLI and may be a problem if saved in a server-state
file. Because the reload may fail because of this inconsistency.

Here is an example:

 * Declare a server template in a backend, using the resolver <dns>

server-template test 2 _http._tcp.example.com resolvers dns check

 * 2 SRV records are announced with the corresponding additional
   records. Thus, 2 servers are filled. Here is the "show servers state"
   output :

2 frt 1 test1 192.168.1.1 2 64 0 1 2 15 3 4 6 0 0 0 http1.example.com 8001 _http._tcp.example.com 0 0 - - 0
2 frt 2 test2 192.168.1.2 2 64 0 1 1 15 3 4 6 0 0 0 http2.example.com 8002 _http._tcp.example.com 0 0 - - 0

 * Then, one additional record is removed (or a SRV record is removed, the
   result is the same). Here is the new "show servers state" output :

2 frt 1 test1 192.168.1.1 2 64 0 1 38 15 3 4 6 0 0 0 http1.example.com 8001 _http._tcp.example.com 0 0 - - 0
2 frt 2 test2 192.168.1.2 0 96 0 1 19 15 3 0 14 0 0 0 - 8002 _http._tcp.example.com 0 0 - - 0

On reload, if a server-state file is used, this leads to undefined behaviors
depending on the configuration.

This patch should be backported as far as 2.0.
2021-02-24 21:58:45 +01:00
Baptiste Assmann
b4badf720c BUG/MINOR: resolvers: new callback to properly handle SRV record errors
When a SRV record was created, it used to register the regular server name
resolution callbacks. That said, SRV records and regular server name
resolution don't work the same way, furthermore on error management.

This patch introduces a new call back to manage DNS errors related to
the SRV queries.

this fixes github issue #50.

Backport status: 2.3, 2.2, 2.1, 2.0
2021-02-24 21:58:45 +01:00
Christopher Faulet
a331a1e8eb BUG/MINOR: resolvers: Only renew TTL for SRV records with an additional record
If no additional record is associated to a SRV record, its TTL must not be
renewed. Otherwise the entry never expires. Thus once announced a first
time, the entry remains blocked on the same IP/port except if a new announce
replaces the old one.

Now, the TTL is updated if a SRV record is received while a matching
existing one is found with an additional record or when an new additional
record is assigned to an existing SRV record.

This patch should be backported as far as 2.2.
2021-02-24 21:58:45 +01:00
Christopher Faulet
9c246a4b6c BUG/MINOR: resolvers: Fix condition to release received ARs if not assigned
At the end of resolv_validate_dns_response(), if a received additionnal
record is not assigned to an existing server record, it is released. But the
condition to do so is buggy. If "answer_record" (the received AR) is not
assigned, "tmp_record" is not a valid record object. It is just a dummy
record "representing" the head of the record list.

Now, the condition is far cleaner. This patch must be backported as far as
2.2.
2021-02-24 21:58:45 +01:00
Emeric Brun
56fc5d9ebc MEDIUM: resolvers: add supports of TCP nameservers in resolvers.
This patch introduce the new line "server" to set a TCP
nameserver in a "resolvers" section:

server <name> <address> [param*]
  Used to configure a DNS TCP or stream server. This supports for all
  "server" parameters found in 5.2 paragraph. Some of these parameters
  are irrelevant for DNS resolving. Note: currently 4 queries are pipelined
  on the same connections. A batch of idle connections are removed every
  5 seconds. "maxconn" can be configured to limit the amount of those
  concurrent connections and TLS should also usable if the server supports
. The current implementation limits to 4 pipelined

The name of the line in configuration is open to discussion
and could be changed before the next release.
2021-02-13 10:03:46 +01:00
Emeric Brun
c943799c86 MEDIUM: resolvers/dns: split dns.c into dns.c and resolvers.c
This patch splits current dns.c into two files:

The first dns.c contains code related to DNS message exchange over UDP
and in future other TCP. We try to remove depencies to resolving
to make it usable by other stuff as DNS load balancing.

The new resolvers.c inherit of the code specific to the actual
resolvers.

Note:
It was really difficult to obtain a clean diff dur to the amount
of moved code.

Note2:
Counters and stuff related to stats is not cleany separated because
currently counters for both layers are merged and hard to separate
for now.
2021-02-13 10:03:46 +01:00