Azure / Synapse-workspace-deployment

MIT License
27 stars 34 forks source link

DeleteArtifactsNotInTemplate also deletes Managed Private Endpoints #93

Open jj-obrien opened 10 months ago

jj-obrien commented 10 months ago

DeleteArtifactsNotInTemplate: 'true' also deletes Managed Private Endpoints which are never in the template as each workspace has a unique set. Each Managed Private Endpoint then needs to be recreated manually and approved.

Additionally, if DeleteArtifactsNotInTemplate runs on a Managed Private Endpoint in Resource Group with Delete Lock on the RG, the Managed Private Endpoints do not get deleted but change provisioning status to "Failed" instead. The result is such that testing Linked Service connection that uses such endpoint succeeds, but authorization fails.

Please provide an option to specify which Artifact types are to be included or excluded from the DeleteArtifactsNotInTemplate toggle, or better yet, exclude any such Artifact types which are never included in the template.

This is last tested using Azure/Synapse-workspace-deployment@V1.7.0 on 23/09/2023

JohannesMoersch commented 9 months ago

I'm having the same problem. Even when DeployManagedPrivateEndpoints is set to false, it still deletes my managed private endpoints. I expected that flag to completely leave my private endpoints alone. This is forcing us to recreate our PEs after every deployment.

SebastiaanOudeGroeniger commented 8 months ago

Same problem here. This is really becoming an issue for us trying to have a clean workspace where obsolete artifacts get deleted, while still maintaining MPE's. There is also no proper workaround for this!

ltutar commented 8 months ago

Same problem here. This is really becoming an issue for us trying to have a clean workspace where obsolete artifacts get deleted, while still maintaining MPE's. There is also no proper workaround for this!

FYI. Azure DevOps does have this option. We have no plans yet to migrate to Azure DevOps from GitHub Actions.

I am also having this issue. A solution would really help us also.

ArthurSteijn commented 7 months ago

So I agree that the behavior is not as we might expect. But deleting and redeploying the MPE's with override params works fine for me. Am I missing something?

jj-obrien commented 7 months ago

deleting and redeploying the MPE's with override params works fine for me

If you have a solution that works for you please do share. MPEs are not in the template, so I don't see how it would be possible to redeploy them with override params; furthermore MPEs have to be approved in the portal when they're created, so I don't know how it would be possible to deploy them automatically without further manual intervention?

SebastiaanOudeGroeniger commented 7 months ago

deleting and redeploying the MPE's with override params works fine for me

If you have a solution that works for you please do share. MPEs are not in the template, so I don't see how it would be possible to redeploy them with override params; furthermore MPEs have to be approved in the portal when they're created, so I don't know how it would be possible to deploy them automatically without further manual intervention?

Agreed, not sure how to do this. It is supported in Azure DevOps, but as MPEs are not in the template (and have different names per environment) overriding seems not possible? Also, a deletion and recreation is still not solving our issues because approvals are not done automatically for all endpoints. And since some of the approvals are done by members of different teams, it's impossible to align on this with each deployment. But if I am missing something here @ArthurSteijn, I really like to know!

amritakasaundhan commented 7 months ago

I guess we will need to rename the private end point to synapse-ws-sql for it to skip the private endpoint deletion as it checks for name and type before marking resources to default https://github.com/Azure/Synapse-workspace-deployment/blob/master/build_and_deploy/utils/common_utils.ts#L26C36-L26C36.

ltutar commented 7 months ago

@amritakasaundhan For my understanding. Does this mean that we can expect a new version soon where this issue is solved.

amritakasaundhan commented 7 months ago

Its not understanding looking at the code, I am not the contributor in the repo. I would appreciate if any of the contributors can confirm that

ArthurSteijn commented 7 months ago

@jj-obrien / @SebastiaanOudeGroeniger So for the record, we are using Azure DevOps. But this is the order that we follow:

  1. Create MPE within the Synapse Studio on the Dev environment, connected to a git repo
  2. Approve the Link once in the resource like Azure SQL or Storage Account
  3. Add a custom parameter file to your repo And make sure to add the MPE's like this:
    "Microsoft.Synapse/workspaces/managedVirtualNetworks/managedPrivateEndpoints": {
        "properties": {
            "privateLinkResourceId": "="
        }
    }
  4. Have a build stage in you pipeline that validates the workspace artifacts and creates the 'TemplateForWorkspace.json'. The MPE should be in the parameters section of the file:
    "pe_blob_bronze_properties_privateLinkResourceId": {
            "type": "string",
            "defaultValue": "/subscriptions/<xxxx>/resourceGroups/<xxx-rg>/providers/Microsoft.Storage/storageAccounts/<saname>"
        }
  5. Deploy the 'TemplateForWorkspace.json' with a custom parameters file that references the right resource.
  6. Approve the link on the Storage Account or SQL or other Manually - Once.

After this, even with the 'DeleteArtifactsNotInTemplate' the MPE are still in place. Just make sure the naming of the MPE is consistent over all environments.

image
chau0 commented 4 months ago

Another approach is to maintain the name of the Managed Private Endpoint (MPE) unchanged. By doing so, during the deployment process, the system will not attempt to delete the existing MPE. Modify the TemplateForWorkspace.json file to adjust the endpoint it points to from development to production. This is accomplished by updating the file as follows:

Initially, you might have:

{
    "name": "[concat(parameters('workspaceName'), '/default/pe-azure-sql-dev')]",
    "type": "Microsoft.Synapse/workspaces/managedVirtualNetworks/managedPrivateEndpoints"
}

This should be changed to:

{
    "name": "[concat(parameters('workspaceName'), '/default/pe-azure-sql-prod')]",
    "type": "Microsoft.Synapse/workspaces/managedVirtualNetworks/managedPrivateEndpoints"
}

By keeping the MPE name consistent, the deployment process recognizes it as an existing resource, thereby avoiding the unnecessary deletion and re-creation of the MPE, which might otherwise interrupt service continuity or access permissions.

JMAN-Devops commented 3 weeks ago

Any update on this issue?