-
Notifications
You must be signed in to change notification settings - Fork 528
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
Steps for connecting to SQL database #4619
base: main
Are you sure you want to change the base?
Conversation
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
…nal/v-ajaj/123379
ALTER ROLE db_owner ADD MEMBER [[email protected]]; | ||
``` | ||
|
||
Revert the SQL Server Active directory admin user changes once the user has been created, that is, set the `OSS FHIR Server(App service)` user assigned identity back to SQL Server admin. |
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.
@SergeyGaluzo is this the recommended approach?
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.
@brendankowitz What problem are we trying to solve? Is something not working with managed identities?
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.
@SergeyGaluzo We have worked on creating the documentation on the below mentioned details:
"Currently in OSS, we only have a single login enabled for the SQL Server - the User Assigned Managed Identity that the FHIR Service uses to access the server.
This is a problem because customers may need to access the database directly to check data or they may have other applications that need to access the database. This also could be problematic for FHIR Team devs that need to check the database directly like the JobQueue table.
We need to provide instructions or a script to update the SQL Admin of the user's choosing (maybe the current user??) but also create the SQL Login for the User Assigned Managed Identity.
We cannot do this in pipeline because the identity we you add a SQL user via a service principal, the SQL Server needs a managed identity that has graph read all access."
@brendankowitz FYI
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.
@brendankowitz I would suggest different approach. FHIR SQL server does not need FHIR Server UAMI as admin for FHIR server to work, it is just hard to setup by the pipeline. Therefore, I usually "downgrade" UAMI to db owner after FHIR server is setup. This contains similar steps to create login for UAMI and corresponding database user. Setting up user access becomes a separate issue. In certain cases, it could remain admin...
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.
@brendankowitz: Please let us know if you suggest us to update the pipeline and documentation with an alternative approach, and if you'd like us to include instructions on providing user access to the database which will be the separate issue as mentioned.
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.
@brendankowitz Can you please suggest what should be the next step here?
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.
Talked to Brendan, this PR is good to go to review now!
Description
The procedures for connecting to a SQL database are outlined in this PR.
Related issues
Addresses issue #123379.
Testing
Reviewed it in code editor preview window.
FHIR Team Checklist
Semver Change (docs)
Patch|Skip|Feature|Breaking (reason)