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

Update sql server password to MI #3946

Merged
merged 25 commits into from
Jul 12, 2024
Merged

Update sql server password to MI #3946

merged 25 commits into from
Jul 12, 2024

Conversation

aponakampalli
Copy link
Contributor

@aponakampalli aponakampalli commented Jul 2, 2024

Description

This PR introduces the use of Managed Identity for authentication to SQL Server, eliminating the need for a password in OSS.

Related issues

Addresses issue AB#96639.

Testing

Manual test deployment.
mi-sql-server-test

FHIR Team Checklist

  • Update the title of the PR to be succinct and less than 65 characters
  • Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
  • Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
  • Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
  • Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
  • CI is green before merge Build Status
  • Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

@aponakampalli aponakampalli added this to the S144 milestone Jul 2, 2024
@aponakampalli aponakampalli added Open source This change is only relevant to the OSS code or release. Enhancement Enhancement on existing functionality. labels Jul 2, 2024
@brendankowitz
Copy link
Member

We also want to update these settings in the Storage resource
From:
https://github.com/microsoft/fhir-server/blob/aa2e70de8031f2b55177a07a7a522ff6317df2d0/samples/templates/default-azuredeploy-docker.json#L631-L646

to

{
            "name": "[variables('storageAccountName')]",
            "type": "Microsoft.Storage/storageAccounts",
            "apiVersion": "2019-06-01",
            "location": "[resourceGroup().location]",
            "properties": {
                "supportsHttpsTrafficOnly": true,
                "allowBlobPublicAccess": false,
                "allowSharedKeyAccess": false
            },
            "condition": "[variables('enableIntegrationStore')]",
            "dependsOn": [],
            "sku": {
                "name": "Standard_LRS"
            },
            "kind": "Storage",
            "tags": {}
        }

@brendankowitz
Copy link
Member

As part of deployment we'll need to make some minor edits to this file so we can update CI environment when the PR is merged:

- stage: UpdateRandom
displayName: 'Determine Random String'
dependsOn: []
jobs:
- job: Password
pool:
name: '$(DefaultLinuxPool)'
vmImage: '$(LinuxVmImage)'
steps:
- template: ./jobs/update-sqlAdminPassword.yml

build/pr-pipeline.yml Outdated Show resolved Hide resolved
build/pr-pipeline.yml Outdated Show resolved Hide resolved
@aponakampalli aponakampalli force-pushed the t-adithip/mi-sqlserver branch from c270d65 to 0fc1995 Compare July 9, 2024 23:24
@mikaelweave mikaelweave marked this pull request as ready for review July 12, 2024 18:30
@mikaelweave mikaelweave requested a review from a team as a code owner July 12, 2024 18:30
@LTA-Thinking
Copy link
Collaborator

We should also make the update Brendan mentioned above to CI deploy.

LTA-Thinking
LTA-Thinking previously approved these changes Jul 12, 2024
@LTA-Thinking LTA-Thinking merged commit 7d45e4b into main Jul 12, 2024
30 of 43 checks passed
@LTA-Thinking LTA-Thinking deleted the t-adithip/mi-sqlserver branch July 12, 2024 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Enhancement on existing functionality. Open source This change is only relevant to the OSS code or release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants