local api unix socket support #2213

cyberb · 2023-05-20T19:26:56Z

Initial unix socket support, tested on my running instance.

config.yaml

api:
  server:
    listen_uri: /var/snap/crowdsec/current/crowdsec.sock

Also
local_api_credentials.yaml

url: /var/snap/crowdsec/current/crowdsec.sock

Will check if there are http test server tests so I can add a unix socket unit test.

Some comments:

pkg/csconfig/api.go contains too many structs in one file, it is better to split into multiple files.

github-actions · 2023-05-20T19:27:09Z

@cyberb: There are no 'kind' label on this PR. You need a 'kind' label to generate the release automatically.

/kind feature
/kind enhancement
/kind fix
/kind chore
/kind dependencies

Details

I am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository.

github-actions · 2023-05-20T19:27:11Z

@cyberb: There are no area labels on this PR. You can add as many areas as you see fit.

/area agent
/area local-api
/area cscli
/area security
/area configuration

Details

I am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository.

LaurenceJJones · 2023-05-20T20:20:56Z

/kind feature
/area local-api

codecov-commenter · 2023-05-20T20:27:25Z

Codecov Report

Attention: 22 lines in your changes are missing coverage. Please review.

Comparison is base (7e5ab34) 58.22% compared to head (24154ae) 56.20%.

❗ Current head 24154ae differs from pull request most recent head ef9fb8c. Consider uploading reports for the commit ef9fb8c to get more accurate results

Files	Patch %	Lines
pkg/csconfig/api.go	47.61%	10 Missing and 1 partial ⚠️
pkg/apiclient/client.go	85.41%	4 Missing and 3 partials ⚠️
pkg/apiserver/apiserver.go	80.95%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2213      +/-   ##
==========================================
- Coverage   58.22%   56.20%   -2.03%     
==========================================
  Files         201      182      -19     
  Lines       27048    25433    -1615     
==========================================
- Hits        15750    14295    -1455     
+ Misses       9753     9613     -140     
+ Partials     1545     1525      -20

Flag	Coverage Δ
bats	`37.72% <70.53%> (-0.68%)`	⬇️
unit-linux	`37.28% <58.03%> (-17.95%)`	⬇️
unit-windows	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

LaurenceJJones · 2023-05-22T09:21:39Z

Link to feature request #2197

cyberb · 2023-05-23T08:37:17Z

I do not think I changed anything which may affect this test on windows pkg/acquisition/modules/file/file_test.go did I?

cyberb · 2023-05-23T08:47:10Z

@LaurenceJJones I tested this on my running instance seems ok (no tcp involved in local api). Do you have any comments?

This is just a step towards total no tcp mode which involves future PRs in:

bouncers
prometheus, I will have to beg them here Support Unix socket for metrics address prometheus/prometheus#12024
Metabase, they seem to use ring-jetty which is being worked on to add unix support (Add support for serving on Unix domain socket ring-clojure/ring#478). While I must say metabase being a java heavy product is a bit over kill for a simple dashboard but some webui has to exist for a complete user experience on a simplified self-hosted platform.

LaurenceJJones · 2023-05-23T08:50:53Z

I do not think I changed anything which may affect this test on windows pkg/acquisition/modules/file/file_test.go did I?

No, the current windows tests are not the best. I will rerun them now.

LaurenceJJones · 2023-05-23T08:52:11Z

I tested this on my running instance seems ok (no tcp involved in local api). Do you have any comments?

I am going to test the PR on my system now and get back to you with any feedback. I do note currently it will all be scoped to CrowdSec as there is no bouncer integrated but it a massive first step thank you!

🎆

mmetc · 2023-06-09T09:12:00Z

diff --git a/pkg/apiclient/client_test.go b/pkg/apiclient/client_test.go
index 27210962..cea87bb0 100644
--- a/pkg/apiclient/client_test.go
+++ b/pkg/apiclient/client_test.go
@@ -115,7 +115,7 @@ func TestNewClientOk_UnixSocket(t *testing.T) {
 	client, err := NewClient(&Config{
 		MachineID:     "test_login",
 		Password:      "test_password",
-		UserAgent:     fmt.Sprintf("crowdsec/%s", cwversion.VersionStr()),
+		UserAgent:     fmt.Sprintf("crowdsec/%s", version.String()),
 		URL:           apiURL,
 		VersionPrefix: "v1",
 	})
@@ -292,7 +292,7 @@ func TestNewClientRegisterOK_UnixSocket(t *testing.T) {
 	client, err := RegisterClient(&Config{
 		MachineID:     "test_login",
 		Password:      "test_password",
-		UserAgent:     fmt.Sprintf("crowdsec/%s", cwversion.VersionStr()),
+		UserAgent:     fmt.Sprintf("crowdsec/%s", version.String()),
 		URL:           apiURL,
 		VersionPrefix: "v1",
 	}, &http.Client{})

cyberb · 2023-06-09T09:43:17Z

Is there any news or do you want me to help?

mmetc · 2023-06-09T10:15:26Z

I am reviewing, the above snippet is to fix the current version after merging to master. And here is a functional test (09_socket.bats)

#!/usr/bin/env bats
# vim: ft=bats:list:ts=8:sts=4:sw=4:et:ai:si:

set -u

setup_file() {
    load "../lib/setup_file.sh"
}

teardown_file() {
    load "../lib/teardown_file.sh"
}

setup() {
    load "../lib/setup.sh"
    load "../lib/bats-file/load.bash"
    ./instance-data load
}

teardown() {
    ./instance-crowdsec stop
}

#----------

@test "cscli - connects with socket" {
    sockdir=$(TMPDIR="${BATS_TEST_TMPDIR}" mktemp -u)
    mkdir -p "${sockdir}"
    export socket="${sockdir}/crowdsec_api.sock"

    config_set ".api.server.listen_uri=strenv(socket)"

    LOCAL_API_CREDENTIALS=$(config_get '.api.client.credentials_path')
    config_set "${LOCAL_API_CREDENTIALS}" ".url=strenv(socket)"

    ./instance-crowdsec start
    rune -0 cscli lapi status
    assert_stderr --partial "You can successfully interact with Local API (LAPI)"
}

Can I prepare a PR for your branch with my suggestions?

mmetc · 2023-06-09T10:17:50Z

@cyberb I'd rather have unix:///path/to/socket instead of a plain path. What do you think?

cyberb · 2023-06-09T14:46:53Z

@cyberb I'd rather have unix:///path/to/socket instead of a plain path. What do you think?

sure, you will still need a pure path later in the code, but I am fine, I do not actually know what is the standard here, looking at other servers they all I think do pure path but in a separate config field

mongo:

net:
  bindIp: 127.0.0.1
  port: 27017
  unixDomainSocket:
    enabled: true
    pathPrefix: /var/snap/rocketchat/current

postgres:

listen_addresses = '' # what IP address(es) to listen on;
unix_socket_directories = '' # comma-separated list of directories

cyberb · 2023-06-09T14:47:19Z

Can I prepare a PR for your branch with my suggestions?

sure, whatever is the easiest

cyberb · 2023-06-09T14:50:51Z

mmetc I think I saw somewhere that it is possible to add commits to someone else's PR (never done this myself)

LaurenceJJones · 2023-07-10T10:38:46Z

Hello, sorry for lack of reviews.

Could you bring the branch up to date with latest.

cyberb · 2023-07-10T10:49:27Z

Sure np

cyberb · 2023-07-13T09:15:14Z

@LaurenceJJones done

cyberb · 2023-07-13T09:20:02Z

@mmetc

I am reviewing, the above snippet is to fix the current version after merging to master. And here is a functional test (09_socket.bats)

added functional test and version fixes

mmetc · 2023-10-03T08:22:52Z

Sorry for the delay, this will surely be merged but we'll review again and do more tests

LaurenceJJones · 2023-12-01T16:43:56Z

Merged master and solved conflicts, tested working fine. I want to add support to lua-cs-bouncer as I use i personally and want to switch across to sockets. Little bit of lua refactoring is needed but the build works, we could merge and then add support for bouncers without it hindering master.

LaurenceJJones · 2023-12-03T09:14:00Z

And to @mmetc comment about adding unix:// I agree in the context of local_api_credentials.yaml as we need to know if http[s] in standard cases so to keep the standard we should ask for it.

However, I always have a problem with add unix: to a property could we add a helper to the library between crowdsec and the bouncer to parse for these cases?

unix:/var/run/crowdsec.sock
unix://var/run/crowdsec.sock
unix:///var/run/crowdsec.sock

The basic principle is no matter how many slashes you put between unix: and the start of the path it will correctly set it.

mmetc · 2024-01-16T08:11:11Z

I have ported this on top of 1.5.6 changes and allow both TCP and socket at the same time, I'm preparing the PR but will be merged after release to allow some more testing

mmetc · 2024-01-22T13:14:14Z

Sorry it took so long @cyberb, what do you think of #2770

Unfortunately I cannot merge your PR directly since the code needed some refactoring first, which had already started when I looked into it, but it's more or less the same -- except with two listeners

mmetc · 2024-04-30T19:57:54Z

Done in 1.6.1

cyberb · 2024-05-01T08:11:10Z

Sorry it took so long @cyberb, what do you think of #2770

Unfortunately I cannot merge your PR directly since the code needed some refactoring first, which had already started when I looked into it, but it's more or less the same -- except with two listeners

All good thank you very much!
I will test when I migrate to 1.6.1

github-actions bot added the needs/kind label May 20, 2023

github-actions bot added the needs/area label May 20, 2023

github-actions bot added kind/feature area/local-api and removed needs/kind needs/area labels May 20, 2023

local api unix socket support

c9d383d

cyberb force-pushed the master branch from bfd8902 to c9d383d Compare July 13, 2023 08:40

functional test, test version fix

24154ae

mmetc self-assigned this Jul 24, 2023

mmetc added this to the 1.5.4 milestone Sep 7, 2023

mmetc modified the milestones: 1.5.4, 1.5.5 Sep 19, 2023

mmetc removed this from the 1.5.5 milestone Oct 3, 2023

Merge branch 'master' into master

ef9fb8c

mmetc mentioned this pull request Jan 22, 2024

LAPI: local api unix socket support #2770

Merged

mmetc closed this Apr 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

local api unix socket support #2213

local api unix socket support #2213

cyberb commented May 20, 2023

github-actions bot commented May 20, 2023

github-actions bot commented May 20, 2023

LaurenceJJones commented May 20, 2023

codecov-commenter commented May 20, 2023 •

edited by codecov bot

Loading

LaurenceJJones commented May 22, 2023

cyberb commented May 23, 2023

cyberb commented May 23, 2023

LaurenceJJones commented May 23, 2023

LaurenceJJones commented May 23, 2023

mmetc commented Jun 9, 2023

cyberb commented Jun 9, 2023

mmetc commented Jun 9, 2023

mmetc commented Jun 9, 2023

cyberb commented Jun 9, 2023

cyberb commented Jun 9, 2023

cyberb commented Jun 9, 2023

LaurenceJJones commented Jul 10, 2023

cyberb commented Jul 10, 2023

cyberb commented Jul 13, 2023

cyberb commented Jul 13, 2023 •

edited

Loading

mmetc commented Oct 3, 2023 •

edited

Loading

LaurenceJJones commented Dec 1, 2023

LaurenceJJones commented Dec 3, 2023

mmetc commented Jan 16, 2024

mmetc commented Jan 22, 2024

mmetc commented Apr 30, 2024

cyberb commented May 1, 2024

local api unix socket support #2213

local api unix socket support #2213

Conversation

cyberb commented May 20, 2023

github-actions bot commented May 20, 2023

github-actions bot commented May 20, 2023

LaurenceJJones commented May 20, 2023

codecov-commenter commented May 20, 2023 • edited by codecov bot Loading

Codecov Report

LaurenceJJones commented May 22, 2023

cyberb commented May 23, 2023

cyberb commented May 23, 2023

LaurenceJJones commented May 23, 2023

LaurenceJJones commented May 23, 2023

mmetc commented Jun 9, 2023

cyberb commented Jun 9, 2023

mmetc commented Jun 9, 2023

mmetc commented Jun 9, 2023

cyberb commented Jun 9, 2023

cyberb commented Jun 9, 2023

cyberb commented Jun 9, 2023

LaurenceJJones commented Jul 10, 2023

cyberb commented Jul 10, 2023

cyberb commented Jul 13, 2023

cyberb commented Jul 13, 2023 • edited Loading

mmetc commented Oct 3, 2023 • edited Loading

LaurenceJJones commented Dec 1, 2023

LaurenceJJones commented Dec 3, 2023

mmetc commented Jan 16, 2024

mmetc commented Jan 22, 2024

mmetc commented Apr 30, 2024

cyberb commented May 1, 2024

codecov-commenter commented May 20, 2023 •

edited by codecov bot

Loading

cyberb commented Jul 13, 2023 •

edited

Loading

mmetc commented Oct 3, 2023 •

edited

Loading