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

fix/centralize ag 'create any database' perms #8182

Merged
merged 1 commit into from
Feb 20, 2022
Merged

fix/centralize ag 'create any database' perms #8182

merged 1 commit into from
Feb 20, 2022

Conversation

lowlydba
Copy link
Contributor

@lowlydba lowlydba commented Feb 16, 2022

  • Not needed in New-DbaAvailabilityGroup since it is handled in both Replica and Database AG Adders
  • Utilize Grant-DbaAgPermission as the route to do this perm via SMO, and only if automatic seeding
  • Fix Grant-DbaAgPermission parameter parsing so it actually works

Type of Change

  • Bug fix (non-breaking change, fixes Add-DbaAgReplica doesn't add req 'NT AUTHORITY\SYSTEM' perms #8178
  • New feature (non-breaking change, adds functionality, fixes # )
  • Breaking change (effects multiple commands or functionality, fixes # )
  • Ran manual Pester test and has passed (.\tests\manual.pester.ps1)
  • Adding code coverage to existing functionality
  • Pester test is included
  • If new file reference added for test, has is been added to github.com/dataplat/appveyor-lab ?
  • Unit test is included
  • Documentation
  • Build system

Purpose

Followup from #8181
Fixes & standardizes the create database permission required by AGs for automatic seeding.

Approach

  • Make Grant-DbaAgPermission use SMO
  • Make all other functions use Grant-DbaAgPermission
  • Remove the granting from New-DbaAgAvailabilityGroup since it calls both the Add-Replica and Add-Database functions already

Commands to test

  • Add-DbaAgDatabase
  • New-DbaAvailabilityGroup
  • Add-DbaAgReplica
  • Grant-DbaAgPermission

- Not needed in New-DbaAvailabilityGroup since it is handled in both Replica and Database AG Adders
- Utilize Grant-DbaAgPermission as the route to do this perm via SMO, and only if automatic seeding
- Fix Grant-DbaAgPermission parameter parsing so it actually works
Copy link
Contributor

@andreasjordan andreasjordan left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request, looks good. Will do some more testing when both AG related PRs are merged into development.

@potatoqualitee potatoqualitee merged commit d2e03da into dataplat:development Feb 20, 2022
@potatoqualitee
Copy link
Member

Thank you both 🙇🏼

@andreasjordan
Copy link
Contributor

Sorry, we have an issue with this:

image

I will have a look, please do not release yet, @potatoqualitee

@andreasjordan
Copy link
Contributor

When Add-DbaAgReplica is started from New-DbaAvailabilityGroup, then the AG is still in State Creating - and thus it can not be altered. So we have to set this inside of Add-DbaAgReplica only if the AG is already created to support adding new nodes to the AG. But this has also be set inside of New-DbaAvailabilityGroup after creating the AG.

Will code this now...

@andreasjordan
Copy link
Contributor

Ok, fixed the issue, but will do more tests...

@andreasjordan
Copy link
Contributor

Opened #8185 for that. Tested in my Windows Cluster with Automatic Seeding. Created a two node AG with New-DbaAvailabilityGroup and one database, then used Add-DbaAgReplica to add a third replica. Database was automatically created on the third replica because of Automatic Seeding. So I think this work now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add-DbaAgReplica doesn't add req 'NT AUTHORITY\SYSTEM' perms
3 participants