Add common proxy solution for all the supported transport methods that works transparently in the network() and the syslog() sources
Resolves #4388
A new logtransport class for proxied socket has been implemented based on code from the proxied text server.
If everything is good then theoretically the new logtransport could allow deleting the proxied text server code in favor of configuring a regular text server with this new transport, however nothing has been deleted yet in this PR.
First of all, sorry for this taking this long. I find I have less than the ideal amount of time to do code level changes to syslog-ng these days.
This looks so MUCH better than the previous iteration, thanks for doing it.
One more ask: can you please also remove the entire old implementation as well? The check for transport(proxied-tcp) will shadow the old implementation anyway, so it's going to be just dead code. If anything wrong comes up with this implementation we would fix it instead of going back to the old one.
First of all, sorry for this taking this long. I find I have less than the ideal amount of time to do code level changes to syslog-ng these days.
This looks so MUCH better than the previous iteration, thanks for doing it.
One more ask: can you please also remove the entire old implementation as well? The check for transport(proxied-tcp) will shadow the old implementation anyway, so it's going to be just dead code. If anything wrong comes up with this implementation we would fix it instead of going back to the old one.
Thank you for the review!
I have added commits to remove the proxied text server. It was used by the network driver originally, not the syslog driver, so I had to make a few tweaks in order to keep the network driver functioning correctly.
I am still doing the review, currently halfway through at commit 937bb62, but wanted to give some feedback already.
The overall big picture is nice and clean so far.
Thank you for your reply! Just to avoid any misunderstandings, I did not write the PROXY implementation, my PR is a refactor to move proxy processing into the logtransport layer so that it can be enabled for the syslog source driver. Because of this, I do not know what the magic numbers stand for. However, I can try to address smaller issues like adding typedefs and such.
Thanks for the clarification, I started the review by going through each commit and did not realize until now that it was a simple move (lib/logproto/logproto-proxied-text-server.c -> lib/transport/transport-proxied-socket.c). I will check the diff first and then go through the commits, and close my notes where I think it is not really relevant in this PR. Sorry for the confusion!
I see that the CI is failing due to the light test case test_pp_tls failing, maybe a followup is needed here as well? (I haven't checked it thoroughly yet, just guessing).
I see that the CI is failing due to the light test case
test_pp_tlsfailing, maybe a followup is needed here as well? (I haven't checked it thoroughly yet, just guessing).
I've tried to replicate the TLS functionality in the new logtransport as it was in the ProxiedTextServer. Unfortunately I don't have much experience with tls in syslog, so I would need some help fixing it if it's not working.
I see that the CI is failing due to the light test case
test_pp_tlsfailing, maybe a followup is needed here as well? (I haven't checked it thoroughly yet, just guessing).I've tried to replicate the TLS functionality in the new logtransport as it was in the
ProxiedTextServer. Unfortunately I don't have much experience with tls in syslog, so I would need some help fixing it if it's not working.
@alexb271 would you still work on this or should/may I try to finish?
I see that the CI is failing due to the light test case
test_pp_tlsfailing, maybe a followup is needed here as well? (I haven't checked it thoroughly yet, just guessing).I've tried to replicate the TLS functionality in the new logtransport as it was in the
ProxiedTextServer. Unfortunately I don't have much experience with tls in syslog, so I would need some help fixing it if it's not working.@alexb271 would you still work on this or should/may I try to finish?
I'm not actively troubleshooting this right now, so of course you would be welcome to investigate or finish it. If the solution turns out to be something very simple I've missed I can fix that too if required.
@alexb271 would you still work on this or should/may I try to finish?
I'm not actively troubleshooting this right now, so of course you would be welcome to investigate or finish it. If the solution turns out to be something very simple I've missed I can fix that too if required.
I do not know if it's possible correctly,(or what is the simplest solution here), but could you please give me rights to this fork (it seems github allows forking of a fork only if I do not yet have my own fork of the same origin), I do not want to move this branch or create a new PR later on, the best would be if I could just simply edit here anything
@alexb271 would you still work on this or should/may I try to finish?
I'm not actively troubleshooting this right now, so of course you would be welcome to investigate or finish it. If the solution turns out to be something very simple I've missed I can fix that too if required.
I do not know if it's possible correctly,(or what is the simplest solution here), but could you please give me rights to this fork (it seems github allows forking of a fork only if I do not yet have my own fork of the same origin), I do not want to move this branch or create a new PR later on, the best would be if I could just simply edit here anything
The Allow edits by maintainers checkbox is already ticked on this PR, not sure what else would be needed for you to be able to add commits.
The
Allow edits by maintainerscheckbox is already ticked on this PR, not sure what else would be needed for you to be able to add commits.
Seems I need more rights, tried both https/ssh
Pushing to https://github.com/alexb271/syslog-ng.git remote: Permission to alexb271/syslog-ng.git denied to HofiOne. fatal: unable to access 'https://github.com/alexb271/syslog-ng.git/': The requested URL returned error: 403
and
Pushing to github.com:alexb271/syslog-ng.git [email protected]: Permission denied (publickey). fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists.
How do the settings look like for you here?
How do the settings look like for you here?
the invite worked, thanks for fixing the rights
How do the settings look like for you here?
the invite worked, thanks for fixing the rights
I thought the 'allow edits' checkbox was enough, but apparently not. Glad the invite worked though, thanks for finishing this PR.
Build FAILURE
Build FAILURE
Build FAILURE
Build FAILURE
@OverOrion , @bazsi
I've added the first, draft version of how I'd like to solve this. I've rewritten the proxy handling part again, it is a kind of proxy class that is not inherited from LogTransport anymore.
Could you please double-check this solution prior I start to finalize the code, and fix/add the tests as well, just to avoid unnecessary later rounds?
Thank you in advance!
Build FAILURE
Build FAILURE
Build FAILURE
Build FAILURE
Please also fix all the copyrights too
The copyright is incorrect in most of the files, it needs a separate run that corrects all of them, what more, something automatic if we want to follow the actual date in each year.
Build FAILURE
Build FAILURE
@kira-syslogng test this please;