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

Fix for SQL Server Private Endpoints listing #62

Merged
merged 1 commit into from
May 12, 2022
Merged

Fix for SQL Server Private Endpoints listing #62

merged 1 commit into from
May 12, 2022

Conversation

EdneiMonteiro
Copy link
Contributor

SQL servers with only one private endpoint are not being listed. This is due to the erratic behavior of the [String]::IsNullOrEmpty method being used with array of objects. When the object (privateEndpointConnections) has more than one element it returns $false, but when the object has only one element it returns $true. That way, ignoring unique private endpoints. I'm proposing to fix the object test and add a new loop for listing multiple private endpoints on the same SQL Server.

I understand that all modules need review. The use of the [string]::IsNullOrEmpty method can generate unwanted effects, as the method expects a String and not a collection or array of objects. When passing an array of objects, it is converted to a string. This test will only work in scenarios where there is no value or in scenarios with more than one value. For example, consider two arrays pvt1 and pvt2 containing private Endpoint Connections:

pvt1
image

pvt2
image

Checking the values with [String]::IsNullOrEmpty will return:

[String]::IsNullOrEmpty($pvt1) => $false
[String]::IsNullOrEmpty($pvt2) => $true

Both have data, but for one of them the function returned $true, ie null or empty.

This inappropriate use of the method can cause inaccuracy when reporting some resources or properties.

@ClaudioMerola ClaudioMerola merged commit 64800e7 into microsoft:main May 12, 2022
@EdneiMonteiro EdneiMonteiro deleted the sql-private-endpoint branch May 15, 2022 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants