Skip to content

Commit 1900352

Browse files
authored
try system test (#617)
* try system test * add system test to all sanitizations * fix tsan issue with telemetry
1 parent 544bce9 commit 1900352

11 files changed

+196
-113
lines changed

.github/workflows/clang-format-check.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: clang-format Check
2-
on: [push, pull_request]
2+
on: [pull_request]
33
jobs:
44
formatting-check:
55
name: Formatting Check
@@ -15,6 +15,6 @@ jobs:
1515
- name: Run clang-format style check for C/C++/Protobuf source code.
1616
uses: jidicula/[email protected]
1717
with:
18-
clang-format-version: '13'
18+
clang-format-version: '18'
1919
check-path: ${{ matrix.path }}
2020
fallback-style: 'Google'

.github/workflows/codecov.yml

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
name: Linux CI
2+
3+
on:
4+
push:
5+
branches:
6+
- master
7+
pull_request:
8+
9+
jobs:
10+
codecov:
11+
runs-on: ubuntu-22.04
12+
13+
steps:
14+
- uses: actions/checkout@v4
15+
- name: Setup
16+
shell: bash
17+
run: |
18+
mkdir -p ~/.ssh/
19+
echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config
20+
sudo apt-get update
21+
sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ lcov
22+
if [[ -z "${ACT}" ]]; then auth_header="$(git config --local --get http.https://github.com/.extraheader)"; fi
23+
git submodule sync --recursive
24+
git submodule update --init --force --recursive
25+
26+
# Restore both vcpkg and its artifacts from the GitHub cache service.
27+
- name: Restore vcpkg and its artifacts.
28+
uses: actions/cache@v4
29+
with:
30+
# The first path is where vcpkg generates artifacts while consuming the vcpkg.json manifest file.
31+
# The second path is the location of vcpkg (it contains the vcpkg executable and data files).
32+
# The other paths starting with '!' are exclusions: they contain temporary files generated during the build of the installed packages.
33+
path: |
34+
${{ env.CMAKE_BUILD_DIR }}/vcpkg_installed/
35+
${{ env.VCPKG_ROOT }}
36+
!${{ env.VCPKG_ROOT }}/buildtrees
37+
!${{ env.VCPKG_ROOT }}/packages
38+
!${{ env.VCPKG_ROOT }}/downloads
39+
# The key is composed in a way that it gets properly invalidated: this must happen whenever vcpkg's Git commit id changes, or the list of packages changes. In this case a cache miss must happen and a new entry with a new key with be pushed to GitHub the cache service.
40+
# The key includes: hash of the vcpkg.json file, the hash of the vcpkg Git commit id, and the used vcpkg's triplet. The vcpkg's commit id would suffice, but computing an hash out it does not harm.
41+
# Note: given a key, the cache content is immutable. If a cache entry has been created improperly, in order the recreate the right content the key must be changed as well, and it must be brand new (i.e. not existing already).
42+
key: |
43+
et-vcpkg-${{ hashFiles( 'vcpkg.json' ) }}-${{ hashFiles( '.git/modules/external/vcpkg/HEAD' )}}-linux-codecov
44+
45+
- name: Build with code coverage
46+
run: |
47+
mkdir build
48+
pushd build
49+
cmake -DCODE_COVERAGE=ON ../
50+
make -j`nproc`
51+
./et-test
52+
lcov --capture --directory . --output-file coverage.info
53+
lcov --remove coverage.info '/usr/*' --output-file coverage.info # filter system-files
54+
lcov --list coverage.info # debug info
55+
# Uploading report to CodeCov
56+
bash <(curl -s https://codecov.io/bash) -f coverage.info || echo "Codecov did not collect coverage reports"
57+
popd

.github/workflows/linux_ci.yml

+23-62
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,17 @@ jobs:
2222
echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config
2323
sudo apt-get update
2424
sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++
25-
auth_header="$(git config --local --get http.https://github.com/.extraheader)"
25+
26+
echo "Host localhost\n Port 2222\n\n" >> ~/.ssh/config
27+
28+
sudo /usr/sbin/sshd -p 2222
29+
30+
ssh-keygen -t rsa -f ~/.ssh/id_rsa -P "" -N ""
31+
cat ~/.ssh/id_rsa.pub >> ~/.ssh/authorized_keys
32+
cat ~/.ssh/id_rsa.pub >> ~/.ssh/known_hosts
33+
ssh -vvvvvvv -o "StrictHostKeyChecking no" -o 'PreferredAuthentications=publickey' localhost "echo foobar" # Fails if we can't ssh into localhost without a password
34+
if [[ -z "${ACT}" ]]; then auth_header="$(git config --local --get http.https://github.com/.extraheader)"; fi
35+
2636
git submodule sync --recursive
2737
git submodule update --init --force --recursive
2838
@@ -45,95 +55,46 @@ jobs:
4555
key: |
4656
et-vcpkg-${{ hashFiles( 'vcpkg.json' ) }}-${{ hashFiles( '.git/modules/external/vcpkg/HEAD' )}}-linux-${{ matrix.sanitize }}
4757
48-
- name: Test with ubsan
58+
- name: Build with ubsan
4959
run: |
5060
mkdir build
5161
pushd build
5262
cmake -DSANITIZE_UNDEFINED=ON ../
5363
make -j`nproc`
54-
TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./et-test
5564
popd
56-
rm -Rf build
65+
./test/system_tests/connect_with_jumphost.sh
66+
TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./build/et-test
5767
if: matrix.sanitize == 'ubsan'
5868

59-
- name: Test with asan
69+
- name: Build with asan
6070
run: |
6171
mkdir build
6272
pushd build
6373
cmake -DSANITIZE_ADDRESS=ON ../
6474
make -j`nproc`
65-
TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./et-test
6675
popd
67-
rm -Rf build
76+
./test/system_tests/connect_with_jumphost.sh
77+
TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./build/et-test
6878
if: matrix.sanitize == 'asan'
6979

70-
- name: Test with msan
80+
- name: Build with msan
7181
run: |
7282
mkdir build
7383
pushd build
7484
cmake -DSANITIZE_MEMORY=ON ../
7585
make -j`nproc`
76-
TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./et-test
7786
popd
78-
rm -Rf build
87+
./test/system_tests/connect_with_jumphost.sh
88+
TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./build/et-test
7989
if: matrix.sanitize == 'msan'
8090

81-
- name: Test with tsan
91+
- name: Build with tsan
8292
run: |
8393
mkdir build
8494
pushd build
8595
cmake -DSANITIZE_THREAD=ON -DSANITIZE_LINK_STATIC=ON ../
8696
make -j`nproc`
87-
TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./et-test
8897
popd
89-
rm -Rf build
98+
./test/system_tests/connect_with_jumphost.sh
99+
TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./build/et-test
90100
if: matrix.sanitize == 'tsan'
91-
92-
codecov:
93-
runs-on: ubuntu-22.04
94-
95-
steps:
96-
- uses: actions/checkout@v4
97-
- name: Setup
98-
shell: bash
99-
run: |
100-
mkdir -p ~/.ssh/
101-
echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config
102-
sudo apt-get update
103-
sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ lcov
104-
auth_header="$(git config --local --get http.https://github.com/.extraheader)"
105-
git submodule sync --recursive
106-
git submodule update --init --force --recursive
107-
108-
# Restore both vcpkg and its artifacts from the GitHub cache service.
109-
- name: Restore vcpkg and its artifacts.
110-
uses: actions/cache@v4
111-
with:
112-
# The first path is where vcpkg generates artifacts while consuming the vcpkg.json manifest file.
113-
# The second path is the location of vcpkg (it contains the vcpkg executable and data files).
114-
# The other paths starting with '!' are exclusions: they contain temporary files generated during the build of the installed packages.
115-
path: |
116-
${{ env.CMAKE_BUILD_DIR }}/vcpkg_installed/
117-
${{ env.VCPKG_ROOT }}
118-
!${{ env.VCPKG_ROOT }}/buildtrees
119-
!${{ env.VCPKG_ROOT }}/packages
120-
!${{ env.VCPKG_ROOT }}/downloads
121-
# The key is composed in a way that it gets properly invalidated: this must happen whenever vcpkg's Git commit id changes, or the list of packages changes. In this case a cache miss must happen and a new entry with a new key with be pushed to GitHub the cache service.
122-
# The key includes: hash of the vcpkg.json file, the hash of the vcpkg Git commit id, and the used vcpkg's triplet. The vcpkg's commit id would suffice, but computing an hash out it does not harm.
123-
# Note: given a key, the cache content is immutable. If a cache entry has been created improperly, in order the recreate the right content the key must be changed as well, and it must be brand new (i.e. not existing already).
124-
key: |
125-
et-vcpkg-${{ hashFiles( 'vcpkg.json' ) }}-${{ hashFiles( '.git/modules/external/vcpkg/HEAD' )}}-linux
126-
127-
- name: Test with code coverage
128-
run: |
129-
mkdir build
130-
pushd build
131-
cmake -DCODE_COVERAGE=ON ../
132-
make -j`nproc`
133-
./et-test
134-
lcov --capture --directory . --output-file coverage.info
135-
lcov --remove coverage.info '/usr/*' --output-file coverage.info # filter system-files
136-
lcov --list coverage.info # debug info
137-
# Uploading report to CodeCov
138-
bash <(curl -s https://codecov.io/bash) -f coverage.info || echo "Codecov did not collect coverage reports"
139-
popd

src/terminal/ParseConfigFile.hpp

+1
Original file line numberDiff line numberDiff line change
@@ -1431,6 +1431,7 @@ int parse_ssh_config_file(const char *targethost, struct Options *options,
14311431
}
14321432

14331433
ifstream infile(expandedFilename);
1434+
free(expandedFilename);
14341435
if (!infile.good()) {
14351436
LOG(INFO) << filename << " not found";
14361437
return 0;

src/terminal/SshSetupHandler.cpp

+11-4
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ string genCommand(const string& passkey, const string& id,
2727

2828
string SshSetupHandler::SetupSsh(const string& user, const string& host,
2929
const string& host_alias, int port,
30-
const string& jumphost, int jport, bool kill,
30+
const string& jumphost,
31+
const string& jServerFifo, bool kill,
3132
int vlevel, const string& cmd_prefix,
3233
const string& serverFifo,
3334
const std::vector<std::string>& ssh_options) {
@@ -72,6 +73,9 @@ string SshSetupHandler::SetupSsh(const string& user, const string& host,
7273

7374
ssh_args.push_back(SSH_SCRIPT_DST);
7475

76+
std::string ssh_concat;
77+
for (const auto& piece : ssh_args) ssh_concat += piece + " ";
78+
VLOG(1) << "Trying ssh with args: " << ssh_concat << endl;
7579
auto sshBuffer = SubprocessToStringInteractive("ssh", ssh_args);
7680

7781
try {
@@ -107,9 +111,12 @@ string SshSetupHandler::SetupSsh(const string& user, const string& host,
107111
if (!jumphost.empty()) {
108112
/* If jumphost is set, we need to pass dst host and port to jumphost
109113
* and connect to jumphost here */
110-
string cmdoptions{"--verbose=" + std::to_string(vlevel)};
111-
string jump_cmdoptions = cmdoptions + " --jump --dsthost=" + host +
112-
" --dstport=" + to_string(port);
114+
string jump_cmdoptions{"--verbose=" + std::to_string(vlevel)};
115+
if (!jServerFifo.empty()) {
116+
jump_cmdoptions += " --serverfifo=" + jServerFifo;
117+
}
118+
jump_cmdoptions = jump_cmdoptions + " --jump --dsthost=" + host +
119+
" --dstport=" + to_string(port);
113120
string SSH_SCRIPT_JUMP = genCommand(passkey, id, clientTerm, user, kill,
114121
cmd_prefix, jump_cmdoptions);
115122

src/terminal/SshSetupHandler.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ class SshSetupHandler {
88
public:
99
static string SetupSsh(const string &user, const string &host,
1010
const string &host_alias, int port,
11-
const string &jumphost, int jport, bool kill,
12-
int vlevel, const string &etterminal_path,
11+
const string &jumphost, const string &jServerFifo,
12+
bool kill, int vlevel, const string &etterminal_path,
1313
const string &serverFifo,
1414
const std::vector<std::string> &ssh_options);
1515
static const string ETTERMINAL_BIN;

src/terminal/TelemetryService.cpp

+9-4
Original file line numberDiff line numberDiff line change
@@ -209,11 +209,12 @@ TelemetryService::TelemetryService(const bool _allow,
209209
logSendingThread.reset(new thread([this]() {
210210
auto nextDumpTime = std::chrono::system_clock::now();
211211
while (true) {
212-
bool lastRun = shuttingDown;
212+
bool lastRun;
213213
string payload;
214214
int logBufferSize;
215215
{
216216
lock_guard<recursive_mutex> guard(logMutex);
217+
lastRun = shuttingDown;
217218
logBufferSize = (int)logBuffer.size();
218219
}
219220
if (logBufferSize) {
@@ -253,6 +254,7 @@ TelemetryService::TelemetryService(const bool _allow,
253254
}
254255

255256
TelemetryService::~TelemetryService() {
257+
lock_guard<recursive_mutex> guard(logMutex);
256258
if (!shuttingDown) {
257259
cerr << "Destroyed telemetryService without a shutdown";
258260
}
@@ -283,10 +285,13 @@ void TelemetryService::logToDatadog(const string& logText, el::Level logLevel,
283285
}
284286

285287
void TelemetryService::shutdown() {
286-
if (shuttingDown) {
287-
return;
288+
{
289+
lock_guard<recursive_mutex> guard(logMutex);
290+
if (shuttingDown) {
291+
return;
292+
}
293+
shuttingDown = true;
288294
}
289-
shuttingDown = true;
290295
#ifdef USE_SENTRY
291296
sentry_shutdown();
292297
#endif

0 commit comments

Comments
 (0)