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 getremotelogs.sh to accept custom a SSH port #135

Merged
merged 5 commits into from
Jan 14, 2021

Conversation

yamakadi
Copy link

Hi!

Before anything else, thank you for RedELK! It's been a much welcome addition to and an essential part of our workflow.

This PR adds an SSH port argument to the getremotelogs.sh script that defaults to 22.

Due to how our lab is structured, we have been using a custom version of RedELK where the teamserver pushes the logs. We finally decided to update our team server setup to match RedELK instead, but we can't use a standard SSH port for the getremotelogs.sh script.

I can undo the style changes if necessary. They were mostly to quiet down shellcheck.

Regarding the argument count check, I didn't see an issue with passing too many arguments, so opted for a less than check to make the SSH port argument optional. Please correct me if I'm wrong.

@github-actions github-actions bot added docker Related to docker container builds elkserver Related to RedELK server components labels Jan 13, 2021
Copy link
Member

@MarcOverIP MarcOverIP left a comment

Choose a reason for hiding this comment

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

Please change to:

echo "[X] Optional - need the remote system's ssh port to get logs as 4th parameter. Defaults to '22'

@MarcOverIP
Copy link
Member

Thanks for this PR, and thanks for letting us know you appreciate RedELK.

PR looks good, and I think it is a great addition! Just one small text change before Im merging.

@yamakadi
Copy link
Author

Thank you! Made the change!
I was hoping you'd suggest something for that to be honest. :D

@MarcOverIP MarcOverIP merged commit 7638b53 into outflanknl:master Jan 14, 2021
@MarcOverIP
Copy link
Member

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker Related to docker container builds elkserver Related to RedELK server components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants