-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Elastic Agent] Set permissions during installation #26665
Conversation
Pinging @elastic/agent (Team:Agent) |
💚 Build Succeeded
Expand to view the summary
Build stats
Trends 🧪❕ Flaky test reportNo test was executed to be analysed. |
This pull request is now in conflicts. Could you fix it? 🙏
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change looks good it would be nice if somebody with env where problem occurred could test it before merge
7ed564f
to
e22bb58
Compare
@alvarolobato Anyway you could give this a test to see if it fixes the issue for you? |
@blakerouse I gave it a try but this only generates 8.0 artifacts which are not working with my cloud deployment on 7.13, is there a config option to make the agent ignore the fleet server version? or do I need to manually run a 8.0 fleet server to test this?
I called the install command without enrollment token to see how the files looked like and at least now the service generates logs, which didn't before and this is the error:
The only windows I have access to is in Portuguese, the error translates to: |
@alvarolobato There is no way to override the version check. You will need to manually run the whole stack with 8.0.0, or you could normally spin up an 8.0.0-SNAPSHOT in the cloud (but I believe it is broken until a new SNAPSHOT is built). |
@blakerouse ok, I'll start an 8 snapshot once it's working, I'll give it a try. Not sure if that error is helpful. |
This pull request is now in conflicts. Could you fix it? 🙏
|
@alvarolobato Any way you could setup a local stack and try this? 8.0.0-SNAPSHOT still seems broken in Cloud, but running it locally with a docker-compose works. I want to get this in soon as I think it needs to be in 7.14, but don't want it in to late if any issue pop up from the change. |
@blakerouse cloud is working, I was just able to test this and it's still failing. This is what I'm getting on the logs:
|
These are the logs if I start as Administrator:
|
@alvarolobato Based on the logs you provided, it seems that it is running. Installing as an Administrator is required. Other than the log message, are you seeing the Elastic Agent stop running? |
@blakerouse I installed as administrator, but when I run the service as system it dies and the logs are the first ones I shared. So it's not working as expected, it should run properly with the system account. |
@alvarolobato Thanks for explaining I was confused. Being I can't reproduce the behavior that you are seeing it is hard to know what is heppening. I will see if I can figure out why the listener cannot be created. |
@blakerouse let me know if there's anything I can do to help with testing. |
Looking at the permission file you provided (which is very helpful, thank you!) I see the issue. The issue is that being your Windows is not en_US the Administrator account name is different. Elastic Agent does some look ups based on the |
@blakerouse but there should be a way to lookup in a language independent way, I don't believe software will need take that into account. For file names the common files have also a localized name, but the english name usually works. |
Just saw your push. I'll wait for the build and test again. |
@alvarolobato Build should be ready if you can give it a try. |
@blakerouse IT WORKS!!!!!! |
6876162
to
80ff3bb
Compare
@alvarolobato Can you test again? Finally got a good build. |
@alvarolobato I wonder if the build you are using is the correct one, like maybe the build was not published correctly or something? Because just grep-ing through all the code based for |
@blakerouse I thought exactly the same, it's very strange. This is the exact URL I used, copied from the browser: https://storage.googleapis.com/beats-ci-artifacts/pull-requests/pr-26665/elastic-agent/elastic-agent-8.0.0-SNAPSHOT-windows-x86_64.zip Also the build is different from the previous one, it has 13KB more than before and build dates are 2021-07-21 at 14:03:10 (don't know the timezone) so it matches with your PR. Can you launch the job again and see if not all the changes are in? |
/package |
@alvarolobato Here is latest link that shows created time of |
@blakerouse filebeat and metricbeat are working now and reporting data. 🎉 The file was different from the previous one, that's odd. Endpoint still failing, but I guess you want to split this and give it to security.
|
@alvarolobato Yeah let's split it. I will merge this for Elastic Agent and beats, and then we can work on the issue for Endpoint Security. |
* Change permissions on install on Windows * Update go.sum. * Add perms for unix. * Fix format. * Add changelog entry. * Fix imports. * Fix go.sum. * Add comments for access masks. * Fix control listener to use Administrators GUID vs name lookup * Set SecurityDescriptor for starting the Elastic Agent local metrics endpoint * Fix format * Fix lookup in libbeat for Administrators group (cherry picked from commit 48a4703)
* Change permissions on install on Windows * Update go.sum. * Add perms for unix. * Fix format. * Add changelog entry. * Fix imports. * Fix go.sum. * Add comments for access masks. * Fix control listener to use Administrators GUID vs name lookup * Set SecurityDescriptor for starting the Elastic Agent local metrics endpoint * Fix format * Fix lookup in libbeat for Administrators group (cherry picked from commit 48a4703)
@blakerouse thanks for all the work put into these painful bugs, I know it's really hard and frustrating when you can't reproduce it yourself. |
* Change permissions on install on Windows * Update go.sum. * Add perms for unix. * Fix format. * Add changelog entry. * Fix imports. * Fix go.sum. * Add comments for access masks. * Fix control listener to use Administrators GUID vs name lookup * Set SecurityDescriptor for starting the Elastic Agent local metrics endpoint * Fix format * Fix lookup in libbeat for Administrators group (cherry picked from commit 48a4703) Co-authored-by: Blake Rouse <blake.rouse@elastic.co>
* Change permissions on install on Windows * Update go.sum. * Add perms for unix. * Fix format. * Add changelog entry. * Fix imports. * Fix go.sum. * Add comments for access masks. * Fix control listener to use Administrators GUID vs name lookup * Set SecurityDescriptor for starting the Elastic Agent local metrics endpoint * Fix format * Fix lookup in libbeat for Administrators group (cherry picked from commit 48a4703) Co-authored-by: Blake Rouse <blake.rouse@elastic.co>
What does this PR do?
The
install
command now sets the correct permissions for the installation directory on all platforms.Why is it important?
So the started Elastic Agent that is running as a system level service has the required permissions to run correctly and to ensure that only those with the correct level of permissions can access those files.
Checklist
[ ] I have made corresponding changes to the documentation[ ] I have made corresponding change to the default configuration files[ ] I have added tests that prove my fix is effective or that my feature worksCHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Related issues