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

ADManagedServiceAccount: Fixed KeberosEncryptionType and TrustedForDelegation on account creation #722

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dasy77
Copy link

@dasy77 dasy77 commented Feb 17, 2025

Pull Request (PR) description

ADManagedServiceAccount: Set properties KeberosEncryptionType and TrustedForDelegation properly on account creation.
Both properties were not considered previously when the account was created.

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry to the change log under the Unreleased section of the file CHANGELOG.md.
    Entry should say what was changed and how that affects users (if applicable), and
    reference the issue being resolved (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof and comment-based
    help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Community Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
  • New/changed code adheres to DSC Community Style Guidelines.

This change is Reviewable

Copy link

codecov bot commented Feb 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98%. Comparing base (cb46d63) to head (7841405).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@         Coverage Diff         @@
##           main   #722   +/-   ##
===================================
  Coverage    98%    98%           
===================================
  Files        26     26           
  Lines      3720   3727    +7     
===================================
+ Hits       3651   3658    +7     
  Misses       69     69           
Files with missing lines Coverage Δ
...edServiceAccount/MSFT_ADManagedServiceAccount.psm1 100% <100%> (ø)

@dasy77 dasy77 changed the title MSFT_ADManagedServiceAccount: Fixed KeberosEncryptionType and TrustedForDelegation on account creation ADManagedServiceAccount: Fixed KeberosEncryptionType and TrustedForDelegation on account creation Feb 17, 2025
Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dasy77)


a discussion (no related file):
Can we try to get the second line covered in unit tests as well? Maybe it is enough to add TrustedForDelegation here as well?

KerberosEncryptionType = $mockAdServiceAccountStandAlone.KerberosEncryptionType

@johlju johlju added the waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. label Feb 17, 2025
@dasy77 dasy77 requested a review from johlju February 20, 2025 13:26
@dasy77
Copy link
Author

dasy77 commented Feb 21, 2025

@johlju
I have updated the unit test as requested.

@dasy77
Copy link
Author

dasy77 commented Feb 28, 2025

@johlju
Can you please review this pull request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ADManagedServiceAccount: KeberosEncryptionType doesn't work on creation
2 participants