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

Feature request: Decouple using managed identity from updating sql network settings #186

Closed
audunsolemdal opened this issue Aug 8, 2023 · 8 comments · Fixed by #231
Closed
Labels
idle Inactive for 14 days waiting-for-user Waiting for inputs from user
Milestone

Comments

@audunsolemdal
Copy link

I am trying to apply migrations from a self hosted runner. The runner has network access to the sql server via private endpoint and does not need any new network openings. The SQL server denies public access.

It seems as the sql-action is hard coded to attempt adding network rules if azure/login is found

(...)
  deployToDev:
    needs: [buildandtest]
    runs-on: [self-hosted, my-runner] # Deployment must happen through a self-hosted agent with network access.
    environment: "dev"
    permissions:
      id-token: write
      contents: read
      packages: read
    steps:
      #Run migrations

      - uses: azure/login@v1
        with:
          client-id: ${{ vars.AZURE_SQL_CLIENT_ID }}
          tenant-id: ${{ vars.TENANT_ID }}
          subscription-id: ${{ vars.AZURE_SUBSCRIPTION_ID }}

      - name: "Download migrations script"
        uses: actions/download-artifact@v3
        with:
          name: "migrations"
          path: "./SQLScripts"

      - name: Run migrations script
        uses: azure/sql-action@v2
        with:
          connection-string: "Server=xxxxl;Initial Catalog=yyyyy;Authentication=Active Directory Default; Encrypt=True;TrustServerCertificate=False;Connection Timeout=30;"
          path: "./SQLScripts/migrations.sql"

I end up with a

The client 'zzzzzzzz' with object id 'zzzzzzzzzzzzzzzz' does not have authorization to perform action 'Microsoft.Sql/servers/firewallRules/write' over scope (..)

I would like there to be an input var to skip this step entirely, as I have no interest in ad-hoc openings towards the sql server.

I also tried not granting the managed identity access to any subscriptions, setting this

      - uses: azure/login@v1
        with:
          client-id: ${{ vars.AZURE_SQL_CLIENT_ID }}
          tenant-id: ${{ vars.TENANT_ID }}
          allow-no-subscriptions: true

this however failed with this error:

Error: The subscription 'MY-TENANT-ID could not be found.
Error: {"statusCode":404,"message":"The subscription 'MY-TENANT-ID' could not be found.","code":"SubscriptionNotFound"}
@audunsolemdal audunsolemdal added the need-to-triage Requires investigation label Aug 8, 2023
@maskati
Copy link

maskati commented Aug 11, 2023

This is definitely unexpected. Any changes to Azure resource state should be explicitly opt-in rather than implicit as a result of some earlier run action.

@github-actions
Copy link

This issue is idle because it has been open for 14 days with no activity.

@github-actions github-actions bot added the idle Inactive for 14 days label Aug 25, 2023
@audunsolemdal
Copy link
Author

bump.

@github-actions github-actions bot removed the idle Inactive for 14 days label Sep 4, 2023
@github-actions
Copy link

This issue is idle because it has been open for 14 days with no activity.

@github-actions github-actions bot added the idle Inactive for 14 days label Sep 18, 2023
@dzsquared
Copy link
Collaborator

The action attempts to add a firewall rule if it cannot login to the SQL instance. It first tries the master db, then the user db from the connection string.
https://github.com/Azure/sql-action/blob/master/src/SqlUtils.ts#L26

You may get more insights to the initial login failures with debug logging enabled: https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/enabling-debug-logging

@dzsquared dzsquared added waiting-for-user Waiting for inputs from user and removed need-to-triage Requires investigation idle Inactive for 14 days labels Nov 1, 2023
Copy link

This issue is idle because it has been open for 14 days with no activity.

@github-actions github-actions bot added the idle Inactive for 14 days label Nov 15, 2023
@benjamin-hodgson
Copy link

The action attempts to add a firewall rule if it cannot login to the SQL instance

It appears this is not functioning correctly though - in my build I can successfully deploy the database (as a managed identity) but I still get the warning about the firewall.

I'd like to be able to opt out of the firewall behaviour altogether (per #205 (comment)) - I don't like that the action silently attempts to change a security-sensitive azure config with no way to opt out

@github-actions github-actions bot removed the idle Inactive for 14 days label May 17, 2024
Copy link

This issue is idle because it has been open for 14 days with no activity.

@github-actions github-actions bot added the idle Inactive for 14 days label May 31, 2024
@zijchen zijchen linked a pull request Jun 26, 2024 that will close this issue
@zijchen zijchen added this to the v2.3 milestone Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idle Inactive for 14 days waiting-for-user Waiting for inputs from user
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants