microsoft / vscode-cosmosdb

Azure Databases extension for VS Code
https://marketplace.visualstudio.com/items?itemName=ms-azuretools.vscode-cosmosdb
MIT License
155 stars 68 forks source link

Feature to add IP to Postgres firewall rule has stopped working #2204

Closed dhpalan closed 11 months ago

dhpalan commented 11 months ago

I was using this nice feature to add my IP address to the Postgres firewall rule straight from VSCode. Recently this has stopped working.

Past

image

Now

Azure Resources v0.7.5 Azure Databases v0.20.0

Code: https://github.com/microsoft/vscode-cosmosdb/blob/1ffeba81343d374294e84eb7d0b7c85cf99e0d23/src/postgres/commands/configurePostgresFirewall.ts#L18-L36 I believe that some updates made here has caused this -> https://github.com/microsoft/vscode-cosmosdb/releases

image


VSCode version

Version: 1.83.1 (system setup)
Commit: f1b07bd25dfad64b0167beb15359ae573aecd2cc
Date: 2023-10-10T23:48:05.904Z
Electron: 25.8.4
ElectronBuildId: 24154031
Chromium: 114.0.5735.289
Node.js: 18.15.0
V8: 11.4.183.29-electron.0
OS: Windows_NT x64 10.0.19044
alexweininger commented 11 months ago

Since the issue is surfacing in the Databases extension, I'm going to transfer it there. If the underlying issue is in this codebase we can file a new issue, but I think it's unlikely.

alexweininger commented 11 months ago

@dhpalan please share the version of the Azure Databases extension you're using. You can install the older version and see if that fixes your issue. That will prove that it's an issue with the latest release.

The Azure Resources extension is mostly a UI layer that the other extensions (ex: Azure Databases) use to surface their features. So in this case, I think the bug might be caused by our latest release of the Azure Databases extension where this feature lives.

dhpalan commented 11 months ago

@alexweininger Of course. You are right. The issue definitely belongs to Azure Databases.

The Azure Databases extension I am using is the latest version v0.20.0 . After my short exchange with @JasonYeMSFT, I leant that I can still manually add my IP to the firewall from the command palette. This solves my problem :)

Since I am not sure if this is bug or if it is an expected behavior of the recent changes, Let Jason decide to keep this issue Open or closed. Happy to help, if you need more information.

alexweininger commented 11 months ago

@dhpalan, ok that makes sense. Thanks for the information!

JasonYeMSFT commented 11 months ago

@dhpalan I added a manual check in the code PostgresDatabaseTreeItem#L55 to see if the IP address we obtained is allowed in the current firewall settings and only show the "configure firewall" option if the IP isn't already included. If the IP is already included in the firewall settings but still get a firewall related error, we show the new "must configure firewall" message. This is an intended change. I'll explain the reasons below.

Prior to this change, the "auto configure firewall" works by waiting for an firewall related error, call some remote service to get the public IP address, and then add that public IP address to the firewall settings. However, as you have noticed, the IP address we get doesn't always represent the IP address that should be added to the firewall settings. In the worst case, we would have added the IP address we peeked but still get the same error and enter a loop always setting the wrong IP in the firewall settings and potentially give people false hope that it would eventually work by keep retrying it. With the new change, we will attempt to set the peeked IP at most once and when it doesn't get you pass the firewall, it halts and ask you to do something in Azure Portal instead. That's because we have already added the only IP that we are aware of in the firewall settings and we cannot make any further progress in resolving the issue.

I think what I need to do is consult the Portal team on how we can reliably get the correct IP to add to the firewall settings. I will reopen https://github.com/microsoft/vscode-cosmosdb/issues/2179, add a comment to track doing that and close this issue as by design. Thanks for the detailed investigation.