Skip to content

[Vectra XDR] Python version updated from 3.9 to 3.12#12637

Merged
v-atulyadav merged 6 commits into
Azure:masterfrom
fenil-savani:Python-Version-Update
Oct 1, 2025
Merged

[Vectra XDR] Python version updated from 3.9 to 3.12#12637
v-atulyadav merged 6 commits into
Azure:masterfrom
fenil-savani:Python-Version-Update

Conversation

@fenil-savani
Copy link
Copy Markdown
Contributor

Change(s):
  • Python version 3.9 to 3.12

Reason for Change(s):

  • Version soon too be deprecated

Version Updated:

  • N/A

Testing Completed:

  • Yes

@fenil-savani fenil-savani marked this pull request as ready for review August 11, 2025 13:15
@fenil-savani fenil-savani requested review from a team as code owners August 11, 2025 13:15
@v-atulyadav v-atulyadav self-assigned this Aug 12, 2025
@v-atulyadav v-atulyadav added Connector Connector specialty review needed Solution Solution specialty review needed labels Aug 12, 2025
@rahul0216
Copy link
Copy Markdown
Collaborator

rahul0216 commented Aug 26, 2025

@fenil-savani, In file Data%20Connectors/VectraDataConnector/SharedCode/keyvault_secrets_management.py, DefaultAzureCredential has been used for authentication. DefaultAzureCredential should only be used in dev setup. For production environments, Use deterministic credential. Common examples of such credentials include ManagedIdentityCredential, WorkloadIdentityCredential, and ClientAssertionCredential.

@niralishah-crest
Copy link
Copy Markdown
Contributor

Hi @rahul0216

From the deterministic credentials can we use any for Azure Gov Cloud support?

I'm trying to determine whether ManagedIdentityCredential, WorkloadIdentityCredential, and ClientAssertionCredential can be configured to support Azure Government Cloud. So far, I haven’t found any parameters in these credential types that explicitly allow targeting Azure Gov Cloud endpoints.
In particular:
WorkloadIdentityCredential includes a parameter called func, which appears to be a callable function.
ClientAssertionCredential includes a parameter named token_file_path.
Could you clarify the purpose of these parameters and whether they can be leveraged to enable support for Azure Government Cloud?

As we know how to to use Azure Gov Support in DefaultAzureCredential, Could you guide us how to give support on above Credentials?

@jayeshprajapaticrest
Copy link
Copy Markdown
Contributor

@rahul0216 @v-atulyadav
Nirali has reverted to your question. So can you please proceed with the reviewing and merge the PR as it has been raised since 3 weeks ago.

@rahul0216
Copy link
Copy Markdown
Collaborator

rahul0216 commented Sep 3, 2025

@niralishah-crest @jayeshprajapaticrest DefaultAzureCredential is not recommended for Prod env. It might put the user data at risk. Ask is simple avoid using DefaultAzureCredential. You can use any other credential type. And yes, those are supported in Azure Gov. For example, using ClientSecretCredential ,

from azure.identity import ClientSecretCredential

def get_client(self):
    """To obtain AzureKeyVault client using ClientSecretCredential."""
    tenant_id = consts.AZURE_TENANT_ID
    client_id = consts.AZURE_CLIENT_ID
    client_secret = consts.AZURE_CLIENT_SECRET

    credential = ClientSecretCredential(tenant_id=tenant_id, client_id=client_id, client_secret=client_secret)
    client = SecretClient(vault_url=self.keyvault_uri, credential=credential)

@jayeshprajapaticrest Please get the Tenable App Data Connector updated. DefaultAzureCredential is being used there as well.

@jayeshprajapaticrest
Copy link
Copy Markdown
Contributor

@rahul0216 @v-atulyadav
We’ve updated the code by removing DefaultAzureCredential and adding support for Managed Identity. Could you please proceed with the review and merge this PR at the earliest convenience?

@rahul0216
Copy link
Copy Markdown
Collaborator

@rahul0216 @v-atulyadav We’ve updated the code by removing DefaultAzureCredential and adding support for Managed Identity. Could you please proceed with the review and merge this PR at the earliest convenience?

@jayeshprajapaticrest There is another review comment. Please look into that.

@jayeshprajapaticrest
Copy link
Copy Markdown
Contributor

@rahul0216 We are currently working on Tenable, but since this is a separate project and already includes the changes mentioned above, could you please proceed with the review and merge of this PR? Our customer has requested that these changes be merged at the earliest.

@rahul0216
Copy link
Copy Markdown
Collaborator

rahul0216 commented Sep 15, 2025

@rahul0216 We are currently working on Tenable, but since this is a separate project and already includes the changes mentioned above, could you please proceed with the review and merge of this PR? Our customer has requested that these changes be merged at the earliest.

Hi @jayeshprajapaticrest, I'm not talking about Tenable change. You have updated "WEBSITE_RUN_FROM_PACKAGE" parameter. That change will cause function app run to fail. This parameter points to the location of zip file. So, I'm suggesting to correct it. Update it to https://github.com/Azure/Azure-Sentinel/raw/master/Solutions/Vectra%20XDR/Data%20Connectors/VectraDataConnector/VectraXDR321.zip

@jayeshprajapaticrest
Copy link
Copy Markdown
Contributor

@rahul0216 As per the standard approach, we need to provide the shorthand URL to WEBSITE_RUN_FROM_PACKAGE rather than hardcoding the direct ZIP file URL. We have followed the same process for all other integrations, and I have already raised PRs for those, which have been merged.

As I understand it, the reviewer is responsible for mapping the actual ZIP file URL to the provided shorthand URL in the Microsoft side.

@v-atulyadav
Could you kindly confirm this?

@jayeshprajapaticrest
Copy link
Copy Markdown
Contributor

@v-atulyadav @rahul0216
can you please provide PR review status update?

@v-atulyadav
Copy link
Copy Markdown
Collaborator

Hi @jayeshprajapaticrest, @fenil-savani,

The point here is not about mapping the URL to the ZIP file, the concern is that when a ZIP file already exists, a new one should not be created unnecessarily. Ideally, all required changes should be incorporated into the existing ZIP to avoid redundant files.

image

@niralishah-crest
Copy link
Copy Markdown
Contributor

@v-atulyadav @rahul0216 As per the comment, we've removed the new zip file and updated the code in existing zip only. Can you please review it and merge it if no other changes are required?

"Azure_Resource_Group_Name": "[resourceGroup().name]",
"Azure_Subscription_Id": "[subscription().subscriptionId]",
"WEBSITE_RUN_FROM_PACKAGE": "https://aka.ms/sentinel-VectraXDR320-functionapp"
"WEBSITE_RUN_FROM_PACKAGE": "https://aka.ms/sentinel-VectraXDR321-functionapp"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This will fail. Add the absolute path of updated zip file.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@v-atulyadav v-atulyadav merged commit f644956 into Azure:master Oct 1, 2025
30 checks passed
@jayeshprajapaticrest
Copy link
Copy Markdown
Contributor

@rahul0216
As discussed earlier, when updating the Python version to 3.12 along with the managed identity changes, updating the existing ZIP file directly can cause issues for customers who have already configured their Function App, since the runtime ZIP is replaced on every execution. To avoid any disruption for end users, I had initially proposed adding a new ZIP so that existing configurations would remain unaffected. Additionally, Microsoft does not currently allow versioning or upgrades on already configured data connectors, which further adds to this limitation.

In this case, since the existing ZIP was updated, we later encountered escalation cases where customers experienced failures after the PR was merged and made available in the repository. To address the situation, we supported the customer immediately, even at late hours, and resolved the issue.

In previous cases, we have added a new ZIP file for other integrations when significant changes were introduced, and this approach was reviewed and accepted. Going forward, it would be great if we can align on the most suitable approach for such scenarios to minimize risks and ensure a smoother experience for both customers and the team.
Cc: @v-atulyadav

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Connector Connector specialty review needed Solution Solution specialty review needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants