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

[Windows] Create service utils #15114

Merged
merged 11 commits into from
Jan 19, 2023
Merged

[Windows] Create service utils #15114

merged 11 commits into from
Jan 19, 2023

Conversation

richl-dd
Copy link
Contributor

@richl-dd richl-dd commented Jan 17, 2023

What does this PR do?

This PR creates a file that exposes utilities to interact with Windows services via the Service Control Manager (SCM). It wraps OpenSCManager and OpenService to allow the caller to specify the desiredAccess rights (as opposed to the Go package that uses ALL_ACCESS). This will allow agent code to use least-privilege accesses when working with Windows services.

Motivation

The motivation is to enable least-privileges SCM interactions from one place in the agent repo.

Additional Notes

Possible Drawbacks / Trade-offs

  • None I was able to identify at this time.

Describe how to test/QA your changes

Reviewer's Checklist

  • If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
  • Use the major_change label if your change either has a major impact on the code base, is impacting multiple teams or is changing important well-established internals of the Agent. This label will be use during QA to make sure each team pay extra attention to the changed behavior. For any customer facing change use a releasenote.
  • A release note has been added or the changelog/no-changelog label has been applied.
  • Changed code has automated tests for its functionality.
  • Adequate QA/testing plan information is provided if the qa/skip-qa label is not applied.
  • At least one team/.. label has been applied, indicating the team(s) that should QA this change.
  • If applicable, docs team has been notified or an issue has been opened on the documentation repo.
  • If applicable, the need-change/operator and need-change/helm labels have been applied.
  • If applicable, the k8s/<min-version> label, indicating the lowest Kubernetes version compatible with this feature.
  • If applicable, the config template has been updated.

- enumDependentServices now returns an empty slice and no error if there
  are no dependent services

- add tests that use least priviledge to:
  - open SC manager
  - create a service
  - open a service handle
  - list dependent services
  - delete services
@richl-dd richl-dd requested a review from a team as a code owner January 17, 2023 17:29
@richl-dd richl-dd changed the title [Windows [Windows] Create service utils Jan 17, 2023
@richl-dd richl-dd marked this pull request as draft January 17, 2023 17:31
pkg/util/winutil/service.go Outdated Show resolved Hide resolved
pkg/util/winutil/service.go Show resolved Hide resolved
pkg/util/winutil/service.go Outdated Show resolved Hide resolved
pkg/util/winutil/service.go Show resolved Hide resolved
pkg/util/winutil/service.go Show resolved Hide resolved
@richl-dd richl-dd marked this pull request as ready for review January 17, 2023 21:07
@richl-dd richl-dd added this to the 7.43.0 milestone Jan 17, 2023
- Add another test verification step for the order in which services are
  returned in ListDependentServices
Copy link
Contributor

@iglendd iglendd left a comment

Choose a reason for hiding this comment

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

I wonder if we have other places in Agent where SCM is used and we probably can switch to this new code.

pkg/util/winutil/service.go Show resolved Hide resolved
pkg/util/winutil/service.go Outdated Show resolved Hide resolved
pkg/util/winutil/service.go Outdated Show resolved Hide resolved
pkg/util/winutil/service.go Outdated Show resolved Hide resolved
pkg/util/winutil/service.go Outdated Show resolved Hide resolved
pkg/util/winutil/service.go Outdated Show resolved Hide resolved
- Include service name in more logging statements
- Remove unneeded "else"
- as I was re-reading, I found that the "m" and "s" variables were confusing to
  keep track of, so I updated them to better reflect what they held
- include service name in logging
- use Go return conventions
@richl-dd richl-dd merged commit ff01f49 into main Jan 19, 2023
@richl-dd richl-dd deleted the richl/windows-service-utils branch January 19, 2023 00:00
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.

3 participants