local api unix socket support

[cyberb]

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[bot]]

@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[bot]]

@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]

/kind feature /area local-api

[codecov-commenter]

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%.

:exclamation: 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 :warning:
pkg/apiclient/client.go	85.41%	4 Missing and 3 partials :warning:
pkg/apiserver/apiserver.go	80.95%	2 Missing and 2 partials :warning:

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%)	:arrow_down:
unit-linux	37.28% <58.03%> (-17.95%)	:arrow_down:
unit-windows	?

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[LaurenceJJones]

Link to feature request #2197

[cyberb]

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

[cyberb]

@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:

1. bouncers
2. prometheus, I will have to beg them here https://github.com/prometheus/prometheus/issues/12024
3. Metabase, they seem to use ring-jetty which is being worked on to add unix support (https://github.com/ring-clojure/ring/pull/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]

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]

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]

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]

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

[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)

#!/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]

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

[cyberb]

@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]

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

sure, whatever is the easiest

[cyberb]

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

[LaurenceJJones]

Hello, sorry for lack of reviews.

Could you bring the branch up to date with latest.

[cyberb]

Sure np

[cyberb]

@LaurenceJJones done

[cyberb]

@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]

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

[LaurenceJJones]

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]

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]

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]

Sorry it took so long @cyberb, what do you think of https://github.com/crowdsecurity/crowdsec/pull/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]

Done in 1.6.1

[cyberb]

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