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

Look for keystore under the correct path #13332

Merged
merged 1 commit into from
Aug 28, 2019

Conversation

kvch
Copy link
Contributor

@kvch kvch commented Aug 23, 2019

Previously, Functionbeat put the keystore file under /tmp due to the overrides of the function. But when loading the keystore file before packaging the Beat, data path was not yet initialized, so it looked for the file under the incorrect folder. Furthermore, the file in the compressed package was hardcoded to data/functionbeat.keystore. So there was no way for functionbeat to find the keystore when loaded to the cloud provider.

Closes #13079
Depends on #13345

@kvch kvch changed the title Fix keystore in Functionbeat Look for keystore under the correct path Aug 23, 2019
@kvch kvch requested a review from ph August 23, 2019 12:59
@kvch kvch force-pushed the fix-functionbeat-fix-keystore branch from 4810e26 to b393f3d Compare August 23, 2019 15:01
func (k *FileKeystore) ConfiguredPath() string {
return k.Path
}

Copy link
Contributor

Choose a reason for hiding this comment

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

@kvch I think we should probably force a specific location of the keystore in the zip when we created it. Because, correct me if I am wrong: let's say that the keystore path is /home/ph/info/beats.keystore when AWS tries to unzip the artifact it will try to create it in the same place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. We can force it. But I think we need to add a warning about the config overwrites in Functionbeat. It might be confusing for users when they are looking for their keystore in the configured path and they cannot find it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've seen it's already printed to the logs, so I haven't added anything. Just a new overwrite for keystore path.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was just curious if having a deeply nested path on the keystore when we created the archive would be a problem on the remote system.

Copy link
Contributor

@faec faec left a comment

Choose a reason for hiding this comment

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

looks good!

@kvch kvch force-pushed the fix-functionbeat-fix-keystore branch from b393f3d to 9d18198 Compare August 26, 2019 22:28
@kvch kvch force-pushed the fix-functionbeat-fix-keystore branch from 9d18198 to 379e05c Compare August 27, 2019 11:02
Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

LGTM lets merge the overrides PR and rebase this one.

@kvch kvch force-pushed the fix-functionbeat-fix-keystore branch 3 times, most recently from 0aa8976 to 11d6f1f Compare August 27, 2019 19:15
@kvch kvch force-pushed the fix-functionbeat-fix-keystore branch from 11d6f1f to 31f62ce Compare August 27, 2019 21:38
@kvch
Copy link
Contributor Author

kvch commented Aug 28, 2019

Failing tests are unrelated.

@kvch kvch merged commit 67fe9fd into elastic:master Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Functionbeat keystore is not packaged
4 participants