-
Notifications
You must be signed in to change notification settings - Fork 148
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
Add support for pre existing Active Directory user #5988
Conversation
Co-authored-by: Blake Rouse <blake.rouse@elastic.co>
Co-authored-by: Blake Rouse <blake.rouse@elastic.co>
…ic-agent into feat/unprivileged/ad
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
username = ElasticUsername | ||
groupName = ElasticGroupName | ||
ownership, err = EnsureUserAndGroup(username, groupName, pt) | ||
username, password = UnprivilegedUser(customUser, userPassword) |
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.
Why only allow custom user for unprivileged mode? Do we not allow to specify custom user when installing in privileged mode?
return false, nil | ||
} | ||
|
||
match, err := regexp.MatchString(`^.*(\\)(.*)$`, username) |
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.
Isn't this regex match equivalent to the strings.Contains
?
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.
yeah basically. but i'd rather change regexp to be more strict and changing regexp and not allow multiple \
s instead of using contains
need to check windows reqs for domain and user names
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.
This looks good. Thanks for the fixes on checking the domain name in the user.
Thank you for the update. @ycombinator Thanks! |
Thanks for the summary @ycombinator! |
…ic-agent into feat/unprivileged/ad
…ic-agent into feat/unprivileged/ad
Quality Gate failedFailed conditions |
Not sure why SonarQube is reporting 0% coverage when there is a unit test added in this PR. Perhaps it's because it's a Windows-only test file? The remaining CI checks are green. Force merging as we're trying to get this feature into |
* unpriviledged ad works' * Ensure rights for AD users * changelog * support for mac and unix * Update internal/pkg/agent/cmd/install.go Co-authored-by: Blake Rouse <blake.rouse@elastic.co> * Update internal/pkg/agent/cmd/unprivileged.go Co-authored-by: Blake Rouse <blake.rouse@elastic.co> * added e2e tests for darwin and linux * reverted sample_test * mage fmt * do not require password * more strict regex * fix description in changelog * resolved review comments * linter * handle g115 in user_windows.go * more test coverage * fix broken windows UT * coverage * fixed logic for UnprivilegedUser * fixed windows tests --------- Co-authored-by: Blake Rouse <blake.rouse@elastic.co> (cherry picked from commit dccfb70)
* unpriviledged ad works' * Ensure rights for AD users * changelog * support for mac and unix * Update internal/pkg/agent/cmd/install.go Co-authored-by: Blake Rouse <blake.rouse@elastic.co> * Update internal/pkg/agent/cmd/unprivileged.go Co-authored-by: Blake Rouse <blake.rouse@elastic.co> * added e2e tests for darwin and linux * reverted sample_test * mage fmt * do not require password * more strict regex * fix description in changelog * resolved review comments * linter * handle g115 in user_windows.go * more test coverage * fix broken windows UT * coverage * fixed logic for UnprivilegedUser * fixed windows tests --------- Co-authored-by: Blake Rouse <blake.rouse@elastic.co> (cherry picked from commit dccfb70)
* unpriviledged ad works' * Ensure rights for AD users * changelog * support for mac and unix * Update internal/pkg/agent/cmd/install.go Co-authored-by: Blake Rouse <blake.rouse@elastic.co> * Update internal/pkg/agent/cmd/unprivileged.go Co-authored-by: Blake Rouse <blake.rouse@elastic.co> * added e2e tests for darwin and linux * reverted sample_test * mage fmt * do not require password * more strict regex * fix description in changelog * resolved review comments * linter * handle g115 in user_windows.go * more test coverage * fix broken windows UT * coverage * fixed logic for UnprivilegedUser * fixed windows tests --------- Co-authored-by: Blake Rouse <blake.rouse@elastic.co> (cherry picked from commit dccfb70) Co-authored-by: Michal Pristas <michal.pristas@gmail.com>
…user (#6201) * Add support for pre existing Active Directory user (#5988) * unpriviledged ad works' * Ensure rights for AD users * changelog * support for mac and unix * Update internal/pkg/agent/cmd/install.go Co-authored-by: Blake Rouse <blake.rouse@elastic.co> * Update internal/pkg/agent/cmd/unprivileged.go Co-authored-by: Blake Rouse <blake.rouse@elastic.co> * added e2e tests for darwin and linux * reverted sample_test * mage fmt * do not require password * more strict regex * fix description in changelog * resolved review comments * linter * handle g115 in user_windows.go * more test coverage * fix broken windows UT * coverage * fixed logic for UnprivilegedUser * fixed windows tests --------- Co-authored-by: Blake Rouse <blake.rouse@elastic.co> (cherry picked from commit dccfb70) * lint, remove G115 directives --------- Co-authored-by: Michal Pristas <michal.pristas@gmail.com>
Hi @ycombinator We have revalidated this on latest 8.17.0 BC6 kibana cloud environment and had below observations: Observations:
Scenario 2:
Logs: Build details: Hence we are marking this ticket as QA:Validated. Please let us know if we are missing anything here. cc: @michalpristas |
Waiting on custom windows image for testing with AD e2e.
New flags are introduced
These flags are taken into account only when
--unprivileged
is used.New user is added same permissions as elastic-agent user when created in order to be able to log on as a service (otherwise agent won't start)
Custom user won't be created and needs to be present
Testing steps
Create a windows VM (windows server 2022)
Run script 1 that will prepare Active Directory
Script will also wait for AD services to start
Reboot to make changes effective
Run script that will prepare user
Scenario 1
elastic-agent install --unprivileged --user="testing.local\TestUser" --password=Changeme123+
Scenario 2
elastic-agent unprivileged --user="testing.local\TestUser" --password=Changeme123+
Resolves #4585