search

Azure / azure-sdk-for-go

This repository is for active development of the Azure SDK for Go. For consumers of the SDK we recommend visiting our public developer docs at:
https://docs.microsoft.com/azure/developer/go/
MIT License
1.56k stars 802 forks source link

CheckExistenceByID and CheckExistence always returning false with Get or GetByID confirming the resources exists #17809

Closed kabal2010 closed 2 months ago

kabal2010 commented 2 years ago

Bug Report

import ( "context" "log" "testing"

"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources"
"github.com/stretchr/testify/assert"

)

// Set the required variables var subscriptionID = "82d66c37-8d61-4db4-b138-xxxxxxxxxxxx" var apiVersion = "2022-02-01-preview" var resourceID = "/subscriptions/82d66c37-8d61-4db4-b138-xxxxxxxxxxxx/resourceGroups/DEXT-EUN-RGRP01/providers/Microsoft.KeyVault/vaults/dexteunautomationkv01"

// Set the function to test resource existsence func TestAzureKeyVaultExistence(t *testing.T) { // Create a context ctx := context.Background()

// Create a new credential
cred, err := azidentity.NewDefaultAzureCredential(nil)
if err != nil {
    log.Fatal(err)
}

// Create a new resource client
resourceClient, _ := armresources.NewClient(subscriptionID, cred, nil)

// Check if the resource exists
exists, err := resourceClient.CheckExistenceByID(ctx, resourceID, apiVersion, nil)
if err != nil {
    log.Fatal(err)
}
assert.True(t, exists.Success)

}


- When I executed the command `go test -v`, I got the error shown below.
```hcl
=== RUN   TestAzureKeyVaultExistence
    integration_test.go:31:
                Error Trace:    integration_test.go:31    
                Error:          Should be true
                Test:           TestAzureKeyVaultExistence
--- FAIL: TestAzureKeyVaultExistence (2.83s)
FAIL
exit status 1
FAIL    test.com        4.764s
jhendrixMSFT commented 2 years ago

Changing the verb from HEAD to GET the request succeeds, but that shouldn't be necessary. @lirenhe can you please follow up with the service team about this?

ghost commented 2 years ago

Thank you for your feedback. This has been routed to the support team for assistance.

tadelesh commented 2 years ago

@kabal2010 Thanks for your feedback. It seems KeyVault service does not implement the HEAD operation for vaults resources. We'll involve service team to have a look. Could you please use resourceClient.GetById as a temporarily work around?

tadelesh commented 2 years ago

Need KeyVault team support.

kabal2010 commented 2 years ago

@kabal2010 Thanks for your feedback. It seems KeyVault service does not implement the HEAD operation for vaults resources. We'll involve service team to have a look. Could you please use resourceClient.GetById as a temporarily work around?

@tadelesh Yes I can confirm that resourceClient.GetById works and have been using it currently, but will prefer if this can be made to work since it produces a Boolean value which I just need to check to complete my testing. Also, it's worth mentioning that this issue is not just for Azure Key Vault as I tried even a resource group and it came back with a false. It's worth the team checking this against all types of Azure resource, but I just logged this with the Key Vault resource type.

navba-MSFT commented 2 years ago

@kabal2010 Apologies for the late reply. I see that this PR has been filed Azure/autorest.go#819 to address this. As per this comment the Fix is in upcoming preview.41 release. If you have any follow-up questions please feel free to reopen this thread. We would be happy to address it.

tadelesh commented 2 years ago

@navba-MSFT The PR is to fix the wrong handling with 405 response from service in SDK side. Actually, I think the root cause @kabal2010 mentioned is this method has not implemented by service. Could you help to triage this issue to service team to have a look.

navba-MSFT commented 2 years ago

@tadelesh Thanks for clarifying this. I am adding Service Team to have a look into this further.

kabal2010 commented 2 years ago

@navba-MSFT Can I please ask if there has been any update regarding this issue?

Best Regards

navba-MSFT commented 2 years ago

@kabal2010 Unfortunately I don't have any update on this. This is currently pending on the Service Team now.

navba-MSFT commented 2 years ago

@josephkwchan @jennyhunter-msft Could you please review this ask and help us loop in the right owners for this ? Thanks in advance.

kabal2010 commented 2 years ago

@navba-MSFT Can I please ask if this has now been fixed?

navba-MSFT commented 2 years ago

@kabal2010 This is pending with Service team now.

@josephkwchan @jennyhunter-msft Could you please provide an update on this ?

kabal2010 commented 2 years ago

@navba-MSFT Thank you

kabal2010 commented 2 years ago

@navba-MSFT

I think the issue here has to do with using the HEAD method for querying for the resource as seen in https://github.com/Azure/azure-sdk-for-go/blob/main/sdk/resourcemanager/resources/armresources/zz_generated_client.go#L139. I've tested again with another resource and got the error in the snip below.

=== RUN   TestResourceExistence
2022/06/28 21:02:19 HEAD https://management.azure.com/subscriptions/82d66c37-8d61-4db4-b138-xxxxxxxxxxxx/resourceGroups/DEXT-EUN-RGRP01/providers/Microsoft.Network/privateDnsZones/konga.lab/virtualNetworkLinks/dext-eun-pvnet-03
--------------------------------------------------------------------------------
RESPONSE 405: 405 Method Not Allowed
ERROR CODE UNAVAILABLE
--------------------------------------------------------------------------------
Response contained no body
--------------------------------------------------------------------------------
FAIL    test.com        2.439s
FAIL

The test code as as below.

// Set the package
package inegration_test

import (
    "context"
    "log"
    "strings"
    "testing"

    "github.com/Azure/azure-sdk-for-go/sdk/azidentity"
    "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources"
)

// Set function to test for resources
func TestResourceExistence(t *testing.T) {
    // Set the required variables
    resourceId := "/subscriptions/82d66c37-8d61-4db4-b138-xxxxxxxxxxxx/resourceGroups/DEXT-EUN-RGRP01/providers/Microsoft.Network/privateDnsZones/konga.lab/virtualNetworkLinks/dext-eun-pvnet-03"
    apiVersion := "2021-04-01"

    // Create a new context
    ctx := context.Background()

    // Create a new credential
    cred, err := azidentity.NewAzureCLICredential(nil)
    if err != nil {
        log.Fatal(err)
    }

    // Split the resource ID
    resource := strings.Split(resourceId, "/")

    // Create a new client
    client, err := armresources.NewClient(resource[2], cred, nil)
    if err != nil {
        log.Fatal(err)
    }

    // Check existence by ID
    checkExistenceByID, err := client.CheckExistenceByID(ctx, resourceId, apiVersion, nil)
    if err != nil {
        log.Fatal(err)
    }
    log.Println(checkExistenceByID.Success)
}

Changing the method to http.MethodGet should resolve he issue. I've tested this using the Azure Rest API and can only get the resource when the method is GET and not HEAD.

ghost commented 1 year ago

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @darshanhs90, @AshishGargMicrosoft.

Issue Details
### Bug Report - Import path of package: `github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources` - SDK version: `v63.4.0` - go version: `go1.18.1 windows/amd64` - What happened? I created a code shown below. ```hcl // Set the package package main import ( "context" "log" "testing" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources" "github.com/stretchr/testify/assert" ) // Set the required variables var subscriptionID = "82d66c37-8d61-4db4-b138-xxxxxxxxxxxx" var apiVersion = "2022-02-01-preview" var resourceID = "/subscriptions/82d66c37-8d61-4db4-b138-xxxxxxxxxxxx/resourceGroups/DEXT-EUN-RGRP01/providers/Microsoft.KeyVault/vaults/dexteunautomationkv01" // Set the function to test resource existsence func TestAzureKeyVaultExistence(t *testing.T) { // Create a context ctx := context.Background() // Create a new credential cred, err := azidentity.NewDefaultAzureCredential(nil) if err != nil { log.Fatal(err) } // Create a new resource client resourceClient, _ := armresources.NewClient(subscriptionID, cred, nil) // Check if the resource exists exists, err := resourceClient.CheckExistenceByID(ctx, resourceID, apiVersion, nil) if err != nil { log.Fatal(err) } assert.True(t, exists.Success) } ``` - When I executed the command `go test -v`, I got the error shown below. ```hcl === RUN TestAzureKeyVaultExistence integration_test.go:31: Error Trace: integration_test.go:31 Error: Should be true Test: TestAzureKeyVaultExistence --- FAIL: TestAzureKeyVaultExistence (2.83s) FAIL exit status 1 FAIL test.com 4.764s ``` - What did you expect or want to happen? I should have a successful output as shown in the snip below. ```hcl === RUN TestAzureKeyVaultExistence --- PASS: TestAzureKeyVaultExistence (1.98s) PASS ok test.com 3.437s ``` - How can we reproduce it? - Create a similar `go` code as above. - Ensure an Azure resource exists. - Execute the code to check the resource from the `go` code. - Anything we should know about your environment. The `Get-AzKeyVault` command shows the resource exists as shown below ```hcl Vault Name : dexteunautomationkv01 Resource Group Name : DEXT-EUN-RGRP01 Location : northeurope Resource ID : /subscriptions/82d66c37-8d61-4db4-b138-xxxxxxxxxxxx/resourceGroups/DEXT-EUN-RGRP01/providers/Microsoft.KeyVault/vaults/dexteunautomationkv01 Tags : ```
Author: kabal2010
Assignees: tadelesh, lirenhe
Labels: `question`, `Resource Authorization`, `Service Attention`, `Mgmt`, `customer-reported`, `needs-team-attention`
Milestone: -
github-actions[bot] commented 2 months ago

Hi @kabal2010, we deeply appreciate your input into this project. Regrettably, this issue has remained unresolved for over 2 years and inactive for 30 days, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.