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

Added possibility to set remote base path to get logs from #154

Merged
merged 2 commits into from
May 7, 2021

Conversation

fastlorenzo
Copy link
Collaborator

Added a 5th argument to getremotelogs.sh to be able to set the remote base path to get the logs from the C2.

@github-actions github-actions bot added docker Related to docker container builds elkserver Related to RedELK server components labels Apr 20, 2021
@xychix
Copy link
Collaborator

xychix commented Apr 29, 2021

Interesting but rather have this for V2.0.1
Holding back

@xychix xychix added this to the V2.0.1 milestone Apr 29, 2021
@fastlorenzo
Copy link
Collaborator Author

@xychix it would be great to have it before that as it's a retro-compatible change with no side effect, and I need to currently patch the container before using it in prod as I'm using a different folder 😄

@MarcOverIP
Copy link
Member

Very small fix that Im happy to approve now.

However, @fastlorenzo why are you using different bash operators :- and : for $4 and $5?

@fastlorenzo
Copy link
Collaborator Author

$4 was your change I think, for $5 that worked for me, not sure if this is an issue
Happy to fix if needed

@MarcOverIP
Copy link
Member

No, $4 is from PR #135 by @yamakadi . Im not sure on the exact difference between both bash statements. But keeping them consistent has my preference. Could you please check what would be the best statement to use for both?

@yamakadi
Copy link

yamakadi commented May 5, 2021

Hi @MarcOverIP @fastlorenzo

You can check here for a reference on “:-“ → http://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html

The correct usage should be “:-“. I’m not sure why “:” works but it might not behave the same way across different shells.

@MarcOverIP MarcOverIP merged commit 90eeda3 into outflanknl:master May 7, 2021
@MarcOverIP
Copy link
Member

Thanks for the quick reply @yamakadi :-)

Changed to :- and merged.

@fastlorenzo fastlorenzo deleted the cron-variable branch May 8, 2021 20:00
@fastlorenzo fastlorenzo removed this from the V2.0.1 milestone Feb 17, 2023
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.

5 participants