Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Dockerfile, README and add maxmind example #53

Merged
merged 2 commits into from
Apr 23, 2023
Merged

Update Dockerfile, README and add maxmind example #53

merged 2 commits into from
Apr 23, 2023

Conversation

olofvndrhr
Copy link
Contributor

Hi, this PR does the following:

  • Move to a alpine based image and add basic shell and tools for troubleshooting, as the google image is less commonly used.
  • Add an example docker-compose file for the max-minds db.
  • Remove the not implemented location api: "freegeoip"
  • Add a section in the README for privileged ports.
  • Fix small formatting issues
  • Add default UID/GID in README

If you have any questions or comments, just tell me.

Best regards,
Ivan

Signed-off-by: Ivan Schaller <ivan@schaller.sh>
Dockerfile Outdated
&& go build -o "endlessh-go"


FROM alpine:3.17
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would better not to use apline in the release.

There are some article on the internet telling why.
e.g. https://martinheinz.dev/blog/92

README.md Outdated
@@ -33,6 +33,8 @@ Then you can try to connect to the endlessh server. Your SSH client should hang
ssh -p 2222 localhost
```

The default container user has uid/gid 2000.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this is irreverent here.
Why we need it here?
If we have to keep it, it should go to line 28.

README.md Outdated
@@ -83,26 +85,34 @@ Usage of ./endlessh-go
comma-separated list of pattern=N settings for file-filtered logging
```

## Using privileged ports (<1024)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a docker problem not the endlessh-go problem.
The only place I mention docker in this main README.md is line 28.

This could go to the docker example.

You cannot assume every one is using container.
You cannot assume who is using container also uses docker.

README.md Outdated
| endlessh_trapped_time_seconds_total | count | Total seconds clients spent on endlessh. |
| endlessh_client_open_count | count | Number of connections of clients. <br> Labels: <br> <ul><li> `ip`: IP of the client </li> <li> `country`: Country of the IP </li> <li> `location`: Country, Region, and City </li> <li> `geohash`: Geohash of the location </li></ul> |
| endlessh_client_trapped_time_seconds | count | Seconds a client spends on endlessh. <br> Labels: <br> <ul><li> `ip`: IP of the client </li></ul> |
| Metric | Type | Description |
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which editor are you using?

I don't thin we need the extra space at the end to align each line.

@@ -0,0 +1,36 @@
version: "3"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This goes to a new folder.

Signed-off-by: Ivan Schaller <ivan@schaller.sh>
@olofvndrhr
Copy link
Contributor Author

Updated the PR

@shizunge shizunge merged commit 27ad3fa into shizunge:main Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants