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

Allow azure:// prefix when parsing resource IDs #3616

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions azure/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ package azure
import (
"fmt"
"net/http"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/Azure/go-autorest/autorest"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
"sigs.k8s.io/cluster-api-provider-azure/version"
Expand Down Expand Up @@ -361,3 +363,8 @@ func msCorrelationIDSendDecorator(snd autorest.Sender) autorest.Sender {
return snd.Do(r)
})
}

// ParseResourceID parses a string to an *arm.ResourceID, first removing any "azure://" prefix.
func ParseResourceID(id string) (*arm.ResourceID, error) {
return arm.ParseResourceID(strings.TrimPrefix(id, ProviderIDPrefix))
}
52 changes: 52 additions & 0 deletions azure/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,55 @@ func TestMSCorrelationIDSendDecorator(t *testing.T) {
receivedReq.Header.Get(string(tele.CorrIDKeyVal)),
).To(Equal(string(corrID)))
}

func TestParseResourceID(t *testing.T) {
g := NewWithT(t)

tests := []struct {
name string
id string
expectedName string
errExpected bool
}{
{
name: "invalid",
id: "invalid",
expectedName: "",
errExpected: true,
},
{
name: "invalid: must start with slash",
id: "subscriptions/123/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm",
expectedName: "",
errExpected: true,
},
{
name: "invalid: must start with subscriptions or providers",
id: "/prescriptions/123/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm",
expectedName: "",
errExpected: true,
},
{
name: "valid",
id: "/subscriptions/123/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm",
expectedName: "vm",
},
{
name: "valid with provider prefix",
id: "azure:///subscriptions/123/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm",
expectedName: "vm",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
resourceID, err := ParseResourceID(tt.id)
if tt.errExpected {
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(err).NotTo(HaveOccurred())
g.Expect(resourceID.Name).To(Equal(tt.expectedName))
}
})
}
}
3 changes: 1 addition & 2 deletions azure/scope/machinepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"io"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -378,7 +377,7 @@ func (m *MachinePoolScope) createMachine(ctx context.Context, machine azure.VMSS
ctx, _, done := tele.StartSpanWithLogger(ctx, "scope.MachinePoolScope.createMachine")
defer done()

parsed, err := arm.ParseResourceID(machine.ID)
parsed, err := azure.ParseResourceID(machine.ID)
if err != nil {
return errors.Wrap(err, fmt.Sprintf("failed to parse resource id %q", machine.ID))
}
Expand Down
3 changes: 1 addition & 2 deletions azure/services/identities/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package identities
import (
"context"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/Azure/azure-sdk-for-go/services/msi/mgmt/2018-11-30/msi"
"github.com/Azure/go-autorest/autorest"
"sigs.k8s.io/cluster-api-provider-azure/azure"
Expand Down Expand Up @@ -63,7 +62,7 @@ func (ac *AzureClient) GetClientID(ctx context.Context, providerID string) (stri
ctx, _, done := tele.StartSpanWithLogger(ctx, "identities.GetClientID")
defer done()

parsed, err := arm.ParseResourceID(providerID)
parsed, err := azure.ParseResourceID(providerID)
if err != nil {
return "", err
}
Expand Down
3 changes: 1 addition & 2 deletions azure/services/natgateways/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package natgateways
import (
"context"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-08-01/network"
"github.com/pkg/errors"
"k8s.io/utils/pointer"
Expand Down Expand Up @@ -97,7 +96,7 @@ func hasPublicIP(natGateway network.NatGateway, publicIPName string) bool {
}

for _, publicIP := range *natGateway.PublicIPAddresses {
resource, err := arm.ParseResourceID(*publicIP.ID)
resource, err := azure.ParseResourceID(*publicIP.ID)
if err != nil {
continue
}
Expand Down
3 changes: 1 addition & 2 deletions azure/services/scalesetvms/scalesetvms.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/go-logr/logr"
"github.com/pkg/errors"
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
Expand Down Expand Up @@ -147,7 +146,7 @@ func (s *Service) deleteVMSSFlexVM(ctx context.Context, resourceID string) error
}
}()

parsed, err := arm.ParseResourceID(resourceID)
parsed, err := azure.ParseResourceID(resourceID)
if err != nil {
return errors.Wrap(err, fmt.Sprintf("failed to parse resource id %q", resourceID))
}
Expand Down
3 changes: 1 addition & 2 deletions azure/services/virtualmachines/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"fmt"
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-11-01/compute"
"github.com/Azure/go-autorest/autorest"
azureautorest "github.com/Azure/go-autorest/autorest/azure"
Expand Down Expand Up @@ -90,7 +89,7 @@ func (ac *AzureClient) GetByID(ctx context.Context, resourceID string) (compute.
ctx, log, done := tele.StartSpanWithLogger(ctx, "virtualmachines.AzureClient.GetByID")
defer done()

parsed, err := arm.ParseResourceID(resourceID)
parsed, err := azure.ParseResourceID(resourceID)
if err != nil {
return compute.VirtualMachine{}, errors.Wrap(err, fmt.Sprintf("failed parsing the VM resource id %q", resourceID))
}
Expand Down
5 changes: 1 addition & 4 deletions test/e2e/azure_edgezone.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ package e2e

import (
"context"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-11-01/compute"
"github.com/Azure/go-autorest/autorest/azure/auth"
. "github.com/onsi/ginkgo/v2"
Expand Down Expand Up @@ -87,8 +85,7 @@ func AzureEdgeZoneClusterSpec(ctx context.Context, inputGetter func() AzureEdgeZ
vmClient.Authorizer = auth

// get the resource group name
resourceID := strings.TrimPrefix(*machineList.Items[0].Spec.ProviderID, azure.ProviderIDPrefix)
resource, err := arm.ParseResourceID(resourceID)
resource, err := azure.ParseResourceID(*machineList.Items[0].Spec.ProviderID)
Expect(err).NotTo(HaveOccurred())

vmListResults, err := vmClient.List(ctx, resource.ResourceGroupName, "")
Expand Down
6 changes: 2 additions & 4 deletions test/e2e/azure_logcollector.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-11-01/compute"
"github.com/Azure/go-autorest/autorest/azure/auth"
"github.com/pkg/errors"
Expand Down Expand Up @@ -401,8 +400,7 @@ func collectVMBootLog(ctx context.Context, am *infrav1.AzureMachine, outputPath
return errors.New("AzureMachine provider ID is nil")
}

resourceID := strings.TrimPrefix(*am.Spec.ProviderID, azure.ProviderIDPrefix)
resource, err := arm.ParseResourceID(resourceID)
resource, err := azure.ParseResourceID(*am.Spec.ProviderID)
if err != nil {
return errors.Wrap(err, "failed to parse resource id")
}
Expand Down Expand Up @@ -432,7 +430,7 @@ func collectVMSSBootLog(ctx context.Context, providerID string, outputPath strin
v := strings.Split(resourceID, "/")
instanceID := v[len(v)-1]
resourceID = strings.TrimSuffix(resourceID, "/virtualMachines/"+instanceID)
resource, err := arm.ParseResourceID(resourceID)
resource, err := azure.ParseResourceID(resourceID)
if err != nil {
return errors.Wrap(err, "failed to parse resource id")
}
Expand Down
5 changes: 2 additions & 3 deletions test/e2e/azure_privatecluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"path/filepath"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-11-01/compute"
"github.com/Azure/azure-sdk-for-go/services/msi/mgmt/2018-11-30/msi"
"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-08-01/network"
Expand Down Expand Up @@ -421,7 +420,7 @@ func SetupExistingVNet(ctx context.Context, vnetCidr string, cpSubnetCidrs, node
}

func getAPIVersion(resourceID string) (string, error) {
parsed, err := arm.ParseResourceID(resourceID)
parsed, err := azure.ParseResourceID(resourceID)
if err != nil {
return "", errors.Wrap(err, fmt.Sprintf("unable to parse resource ID %q", resourceID))
}
Expand Down Expand Up @@ -455,7 +454,7 @@ func getClientIDforMSI(resourceID string) string {
msiClient := msi.NewUserAssignedIdentitiesClient(subscriptionID)
msiClient.Authorizer = authorizer

parsed, err := arm.ParseResourceID(resourceID)
parsed, err := azure.ParseResourceID(resourceID)
Expect(err).NotTo(HaveOccurred())

id, err := msiClient.Get(context.TODO(), parsed.ResourceGroupName, parsed.Name)
Expand Down
8 changes: 2 additions & 6 deletions test/e2e/azure_vmextensions.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ package e2e

import (
"context"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-11-01/compute"
"github.com/Azure/go-autorest/autorest/azure/auth"
. "github.com/onsi/ginkgo/v2"
Expand Down Expand Up @@ -98,8 +96,7 @@ func AzureVMExtensionsSpec(ctx context.Context, inputGetter func() AzureVMExtens
vmExtensionsClient.Authorizer = auth

// get the resource group name
resourceID := strings.TrimPrefix(*machineList.Items[0].Spec.ProviderID, azure.ProviderIDPrefix)
resource, err := arm.ParseResourceID(resourceID)
resource, err := azure.ParseResourceID(*machineList.Items[0].Spec.ProviderID)
Expect(err).NotTo(HaveOccurred())

vmListResults, err := vmClient.List(ctx, resource.ResourceGroupName, "")
Expand Down Expand Up @@ -146,8 +143,7 @@ func AzureVMExtensionsSpec(ctx context.Context, inputGetter func() AzureVMExtens
vmssExtensionsClient.Authorizer = auth

// get the resource group name
resourceID := strings.TrimPrefix(machinePoolList.Items[0].Spec.ProviderID, azure.ProviderIDPrefix)
resource, err := arm.ParseResourceID(resourceID)
resource, err := azure.ParseResourceID(machinePoolList.Items[0].Spec.ProviderID)
Expect(err).NotTo(HaveOccurred())

vmssListResults, err := vmssClient.List(ctx, resource.ResourceGroupName)
Expand Down