dird: director tries to connect to client when connection is disabled#1099
Conversation
45ab2ab to
b148169
Compare
pstorz
left a comment
There was a problem hiding this comment.
I think it would make sense to create a gtest instead of a system test.
We only need to check if the connection request is denied when the configuration is configured correctly.
core/src/dird/fd_cmds.cc
Outdated
|
|
||
| if (!IsConnectingToClientAllowed(jcr)) { | ||
| Dmsg1(120, "connecting to client \"%s\" is not allowed.\n", | ||
| Dmsg1(120, "Connection from director to client \"%s\" is not allowed.\n", |
There was a problem hiding this comment.
| Dmsg1(120, "Connection from director to client \"%s\" is not allowed.\n", | |
| Dmsg1(120, "Connecting to client \"%s\" is not allowed.\n", |
There was a problem hiding this comment.
I would suggest to keep the wording unified (You added "Connecting to %s is not allowed" in line 231, so I would suggest to call it like this here also. Or we would need to update all other occurrences to contain the word "director" also.
There was a problem hiding this comment.
That's the only occurrence of IsConnectingToClientAllowed(jcr) where there is a message returned. Adding a from director to client makes sense in my opinion to specify the exact reason on that occasion. But IsConnectFromClientAllowed(jcr) has a few occurrences that I can easily update.
For line 231, I did not add a from since both types of connection would be closed. If the check is verified, there is no way to connect to the client at all.
There was a problem hiding this comment.
The "from director" is not needed as dmesg log entries are prefixed witht he daemon name that logs.
b148169 to
0b64ae5
Compare
6fe30c5 to
6d8698c
Compare
6d8698c to
2d912cf
Compare
core/src/dird/fd_cmds.cc
Outdated
|
|
||
| if (!IsConnectingToClientAllowed(jcr)) { | ||
| Dmsg1(120, "connecting to client \"%s\" is not allowed.\n", | ||
| Dmsg1(120, "Connection from director to client \"%s\" is not allowed.\n", |
There was a problem hiding this comment.
The "from director" is not needed as dmesg log entries are prefixed witht he daemon name that logs.
2d912cf to
68a9290
Compare
452bb90 to
8c8bea6
Compare
pstorz
left a comment
There was a problem hiding this comment.
I think printing out the logfiles in case of error when running inside of jenkins will help us to find problems faster.
8fc55f3 to
1bbb2fc
Compare
Test crashes in case the checks do not work properly
introduced testing function that uses grep to check expected strings in log files
1bbb2fc to
f4d68c7
Compare
Description
When
Connection From Director To Client = no, the director tries anyway to connect but fails. Connection should not be even tried in the first place. This PR fixes the issue.Please check
If you have any questions or problems, please give a comment in the PR.
Helpful documentation and best practices
Checklist for the reviewer of the PR (will be processed by the Bareos team)
General
Source code quality
bareos-check-sources --since-mergedoes not report any problemsgit statusshould not report modifications in the source tree after building and testingTests