Skip to content

Commit

Permalink
arm: Drop azcorearm.ResourceID wrapper
Browse files Browse the repository at this point in the history
The ResourceID text marshalling feature I submitted upstream [1],
and was the reason for the wrapper, got released in azcore 1.17.0.

[1] Azure/azure-sdk-for-go#23381
  • Loading branch information
Matthew Barnes committed Jan 23, 2025
1 parent 8be8ea3 commit 7f3cbfc
Show file tree
Hide file tree
Showing 19 changed files with 70 additions and 98 deletions.
9 changes: 5 additions & 4 deletions backend/operations_scanner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"net/http/httptest"
"testing"

azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
"go.uber.org/mock/gomock"

Expand Down Expand Up @@ -64,7 +65,7 @@ func TestDeleteOperationCompleted(t *testing.T) {
ctrl := gomock.NewController(t)
mockDBClient := mocks.NewMockDBClient(ctrl)

resourceID, err := arm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/testCluster")
resourceID, err := azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/testCluster")
if err != nil {
t.Fatal(err)
}
Expand All @@ -89,7 +90,7 @@ func TestDeleteOperationCompleted(t *testing.T) {

mockDBClient.EXPECT().
DeleteResourceDoc(gomock.Any(), resourceID).
Do(func(ctx context.Context, resourceID *arm.ResourceID) {
Do(func(ctx context.Context, resourceID *azcorearm.ResourceID) {
resourceDocDeleted = tt.resourceDocPresent
})
mockDBClient.EXPECT().
Expand Down Expand Up @@ -212,7 +213,7 @@ func TestUpdateOperationStatus(t *testing.T) {
ctrl := gomock.NewController(t)
mockDBClient := mocks.NewMockDBClient(ctrl)

resourceID, err := arm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/testCluster")
resourceID, err := azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/testCluster")
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -252,7 +253,7 @@ func TestUpdateOperationStatus(t *testing.T) {
})
mockDBClient.EXPECT().
UpdateResourceDoc(gomock.Any(), resourceID, gomock.Any()).
DoAndReturn(func(ctx context.Context, resourceID *arm.ResourceID, callback func(*database.ResourceDocument) bool) (bool, error) {
DoAndReturn(func(ctx context.Context, resourceID *azcorearm.ResourceID, callback func(*database.ResourceDocument) bool) (bool, error) {
if resourceDoc != nil {
return callback(resourceDoc), nil
} else {
Expand Down
8 changes: 5 additions & 3 deletions frontend/pkg/frontend/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"log/slog"

azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"

"github.com/Azure/ARO-HCP/frontend/pkg/config"
"github.com/Azure/ARO-HCP/internal/api"
"github.com/Azure/ARO-HCP/internal/api/arm"
Expand Down Expand Up @@ -114,12 +116,12 @@ func DBClientFromContext(ctx context.Context) (database.DBClient, error) {
return dbClient, nil
}

func ContextWithResourceID(ctx context.Context, resourceID *arm.ResourceID) context.Context {
func ContextWithResourceID(ctx context.Context, resourceID *azcorearm.ResourceID) context.Context {
return context.WithValue(ctx, contextKeyResourceID, resourceID)
}

func ResourceIDFromContext(ctx context.Context) (*arm.ResourceID, error) {
resourceID, ok := ctx.Value(contextKeyResourceID).(*arm.ResourceID)
func ResourceIDFromContext(ctx context.Context) (*azcorearm.ResourceID, error) {
resourceID, ok := ctx.Value(contextKeyResourceID).(*azcorearm.ResourceID)
if !ok {
err := &ContextError{
got: resourceID,
Expand Down
3 changes: 2 additions & 1 deletion frontend/pkg/frontend/frontend.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"strings"
"sync/atomic"

azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
"golang.org/x/sync/errgroup"

Expand Down Expand Up @@ -197,7 +198,7 @@ func (f *Frontend) ArmResourceList(writer http.ResponseWriter, request *http.Req
prefixString += "/providers/" + api.ProviderNamespace
prefixString += "/" + api.ClusterResourceTypeName + "/" + resourceName
}
prefix, err := arm.ParseResourceID(prefixString)
prefix, err := azcorearm.ParseResourceID(prefixString)
if err != nil {
logger.Error(err.Error())
arm.WriteInternalServerError(writer)
Expand Down
5 changes: 3 additions & 2 deletions frontend/pkg/frontend/frontend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"testing"
"time"

azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
"go.uber.org/mock/gomock"
Expand All @@ -35,8 +36,8 @@ func getMockDBDoc[T any](t *T) (*T, error) {
}
}

func equalResourceID(expectResourceID *arm.ResourceID) gomock.Matcher {
return gomock.Cond(func(actualResourceID *arm.ResourceID) bool {
func equalResourceID(expectResourceID *azcorearm.ResourceID) gomock.Matcher {
return gomock.Cond(func(actualResourceID *azcorearm.ResourceID) bool {
return strings.EqualFold(actualResourceID.String(), expectResourceID.String())
})
}
Expand Down
9 changes: 5 additions & 4 deletions frontend/pkg/frontend/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net/http"
"strings"

azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
ocmerrors "github.com/openshift-online/ocm-sdk-go/errors"

Expand Down Expand Up @@ -47,7 +48,7 @@ func (f *Frontend) CheckForProvisioningStateConflict(ctx context.Context, operat
}
}

parent := doc.ResourceId.GetParent()
parent := doc.ResourceId.Parent

// ResourceType casing is preserved for parents in the same namespace.
for parent.ResourceType.Namespace == doc.ResourceId.ResourceType.Namespace {
Expand All @@ -66,7 +67,7 @@ func (f *Frontend) CheckForProvisioningStateConflict(ctx context.Context, operat
strings.ToLower(string(operationRequest)))
}

parent = parent.GetParent()
parent = parent.Parent
}

return nil
Expand All @@ -75,7 +76,7 @@ func (f *Frontend) CheckForProvisioningStateConflict(ctx context.Context, operat
func (f *Frontend) DeleteAllResources(ctx context.Context, subscriptionID string) *arm.CloudError {
logger := LoggerFromContext(ctx)

prefix, err := arm.ParseResourceID("/subscriptions/" + subscriptionID)
prefix, err := azcorearm.ParseResourceID("/subscriptions/" + subscriptionID)
if err != nil {
logger.Error(err.Error())
return arm.NewInternalServerError()
Expand Down Expand Up @@ -229,7 +230,7 @@ func (f *Frontend) DeleteResource(ctx context.Context, resourceDoc *database.Res
return operationDoc.ID, nil
}

func (f *Frontend) MarshalResource(ctx context.Context, resourceID *arm.ResourceID, versionedInterface api.Version) ([]byte, *arm.CloudError) {
func (f *Frontend) MarshalResource(ctx context.Context, resourceID *azcorearm.ResourceID, versionedInterface api.Version) ([]byte, *arm.CloudError) {
var responseBody []byte

logger := LoggerFromContext(ctx)
Expand Down
7 changes: 4 additions & 3 deletions frontend/pkg/frontend/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/http"
"testing"

azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"go.uber.org/mock/gomock"

"github.com/Azure/ARO-HCP/internal/api/arm"
Expand Down Expand Up @@ -143,7 +144,7 @@ func TestCheckForProvisioningStateConflict(t *testing.T) {
for _, tt := range tests {
var name string

resourceID, err := arm.ParseResourceID(tt.resourceID)
resourceID, err := azcorearm.ParseResourceID(tt.resourceID)
if err != nil {
t.Fatal(err)
}
Expand All @@ -162,7 +163,7 @@ func TestCheckForProvisioningStateConflict(t *testing.T) {
doc := database.NewResourceDocument(resourceID)
doc.ProvisioningState = directState

parentResourceID := resourceID.GetParent()
parentResourceID := resourceID.Parent
parentDoc := database.NewResourceDocument(parentResourceID)
// Hold the provisioning state to something benign.
parentDoc.ProvisioningState = arm.ProvisioningStateSucceeded
Expand Down Expand Up @@ -201,7 +202,7 @@ func TestCheckForProvisioningStateConflict(t *testing.T) {
// Hold the provisioning state to something benign.
doc.ProvisioningState = arm.ProvisioningStateSucceeded

parentResourceID := resourceID.GetParent()
parentResourceID := resourceID.Parent
if parentResourceID.ResourceType.Namespace == resourceID.ResourceType.Namespace {
parentDoc := database.NewResourceDocument(parentResourceID)
parentDoc.ProvisioningState = parentState
Expand Down
4 changes: 2 additions & 2 deletions frontend/pkg/frontend/middleware_resourceid.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"fmt"
"net/http"

"github.com/Azure/ARO-HCP/internal/api/arm"
azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
)

// This middleware only applies to endpoints whose path form a valid Azure
Expand All @@ -24,7 +24,7 @@ func MiddlewareResourceID(w http.ResponseWriter, r *http.Request, next http.Hand
originalPath = r.URL.Path
}

resourceID, err := arm.ParseResourceID(originalPath)
resourceID, err := azcorearm.ParseResourceID(originalPath)
if err == nil {
ctx = ContextWithResourceID(ctx, resourceID)
r = r.WithContext(ctx)
Expand Down
2 changes: 1 addition & 1 deletion frontend/pkg/frontend/middleware_resourceid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func TestMiddlewareResourceID(t *testing.T) {
resourceTypes := []string{}
for resourceID != nil {
resourceTypes = append(resourceTypes, resourceID.ResourceType.String())
resourceID = resourceID.GetParent()
resourceID = resourceID.Parent
}

if !reflect.DeepEqual(resourceTypes, tt.resourceTypes) {
Expand Down
3 changes: 2 additions & 1 deletion frontend/pkg/frontend/middleware_validatestatic.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"regexp"
"strings"

azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/google/uuid"

"github.com/Azure/ARO-HCP/internal/api"
Expand All @@ -24,7 +25,7 @@ func MiddlewareValidateStatic(w http.ResponseWriter, r *http.Request, next http.
// in response messages.
//TODO: Inspect the error instead of ignoring it
originalPath, _ := OriginalPathFromContext(r.Context())
resource, _ := arm.ParseResourceID(originalPath)
resource, _ := azcorearm.ParseResourceID(originalPath)

if resource != nil {
if resource.SubscriptionID != "" {
Expand Down
2 changes: 1 addition & 1 deletion frontend/pkg/frontend/node_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func (f *Frontend) CreateOrUpdateNodePool(writer http.ResponseWriter, request *h
}
} else {
logger.Info(fmt.Sprintf("creating resource %s", resourceID))
clusterDoc, err := f.dbClient.GetResourceDoc(ctx, resourceID.GetParent())
clusterDoc, err := f.dbClient.GetResourceDoc(ctx, resourceID.Parent)
if err != nil {
logger.Error(err.Error())
arm.WriteInternalServerError(writer)
Expand Down
5 changes: 3 additions & 2 deletions frontend/pkg/frontend/node_pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"testing"
"time"

azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/prometheus/client_golang/prometheus"
"go.uber.org/mock/gomock"

Expand Down Expand Up @@ -43,11 +44,11 @@ var dummyChannelGroup = "dummyChannelGroup"
var dummyVersionID = "dummy"

func TestCreateNodePool(t *testing.T) {
clusterResourceID, _ := arm.ParseResourceID(dummyClusterID)
clusterResourceID, _ := azcorearm.ParseResourceID(dummyClusterID)
clusterDoc := database.NewResourceDocument(clusterResourceID)
clusterDoc.InternalID, _ = ocm.NewInternalID(dummyClusterHREF)

nodePoolResourceID, _ := arm.ParseResourceID(dummyNodePoolID)
nodePoolResourceID, _ := azcorearm.ParseResourceID(dummyNodePoolID)
nodePoolDoc := database.NewResourceDocument(nodePoolResourceID)
nodePoolDoc.InternalID, _ = ocm.NewInternalID(dummyNodePoolHREF)

Expand Down
10 changes: 5 additions & 5 deletions frontend/pkg/frontend/ocm.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ import (
"fmt"
"net/http"

azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/google/uuid"
cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
configv1 "github.com/openshift/api/config/v1"

"github.com/google/uuid"

"github.com/Azure/ARO-HCP/internal/api"
"github.com/Azure/ARO-HCP/internal/api/arm"
)
Expand Down Expand Up @@ -44,7 +44,7 @@ func convertVisibilityToListening(visibility api.Visibility) (listening cmv1.Lis
}

// ConvertCStoHCPOpenShiftCluster converts a CS Cluster object into HCPOpenShiftCluster object
func ConvertCStoHCPOpenShiftCluster(resourceID *arm.ResourceID, cluster *cmv1.Cluster) *api.HCPOpenShiftCluster {
func ConvertCStoHCPOpenShiftCluster(resourceID *azcorearm.ResourceID, cluster *cmv1.Cluster) *api.HCPOpenShiftCluster {
// A word about ProvisioningState:
// ProvisioningState is stored in Cosmos and is applied to the
// HCPOpenShiftCluster struct along with the ARM metadata that
Expand Down Expand Up @@ -162,7 +162,7 @@ func ensureManagedResourceGroupName(hcpCluster *api.HCPOpenShiftCluster) string
}

// BuildCSCluster creates a CS Cluster object from an HCPOpenShiftCluster object
func (f *Frontend) BuildCSCluster(resourceID *arm.ResourceID, requestHeader http.Header, hcpCluster *api.HCPOpenShiftCluster, updating bool) (*cmv1.Cluster, error) {
func (f *Frontend) BuildCSCluster(resourceID *azcorearm.ResourceID, requestHeader http.Header, hcpCluster *api.HCPOpenShiftCluster, updating bool) (*cmv1.Cluster, error) {

// Ensure required headers are present.
if requestHeader.Get(arm.HeaderNameHomeTenantID) == "" {
Expand Down Expand Up @@ -283,7 +283,7 @@ func (f *Frontend) BuildCSCluster(resourceID *arm.ResourceID, requestHeader http
}

// ConvertCStoNodePool converts a CS Node Pool object into HCPOpenShiftClusterNodePool object
func ConvertCStoNodePool(resourceID *arm.ResourceID, np *cmv1.NodePool) *api.HCPOpenShiftClusterNodePool {
func ConvertCStoNodePool(resourceID *azcorearm.ResourceID, np *cmv1.NodePool) *api.HCPOpenShiftClusterNodePool {
nodePool := &api.HCPOpenShiftClusterNodePool{
TrackedResource: arm.TrackedResource{
Resource: arm.Resource{
Expand Down
4 changes: 3 additions & 1 deletion frontend/pkg/frontend/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"path"
"strings"

azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"

"github.com/Azure/ARO-HCP/internal/api"
"github.com/Azure/ARO-HCP/internal/api/arm"
"github.com/Azure/ARO-HCP/internal/database"
Expand Down Expand Up @@ -91,7 +93,7 @@ func (f *Frontend) ExposeOperation(writer http.ResponseWriter, request *http.Req
_, err := f.dbClient.UpdateOperationDoc(ctx, operationID, func(updateDoc *database.OperationDocument) bool {
// There is no way to propagate a parse error here but it should
// never fail since we are building a trusted resource ID string.
operationID, err := arm.ParseResourceID(path.Join("/",
operationID, err := azcorearm.ParseResourceID(path.Join("/",
"subscriptions", updateDoc.ExternalID.SubscriptionID,
"providers", api.ProviderNamespace,
"locations", f.location,
Expand Down
4 changes: 2 additions & 2 deletions internal/api/arm/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func WriteInternalServerError(w http.ResponseWriter) {
}

// NewResourceNotFoundError creates a CloudError for a nonexistent resource error
func NewResourceNotFoundError(resourceID *ResourceID) *CloudError {
func NewResourceNotFoundError(resourceID *azcorearm.ResourceID) *CloudError {
var code string
var message string

Expand All @@ -149,7 +149,7 @@ func NewResourceNotFoundError(resourceID *ResourceID) *CloudError {
}

// WriteResourceNotFoundError writes a nonexistent resource error to the given ResponseWriter
func WriteResourceNotFoundError(w http.ResponseWriter, resourceID *ResourceID) {
func WriteResourceNotFoundError(w http.ResponseWriter, resourceID *azcorearm.ResourceID) {
WriteCloudError(w, NewResourceNotFoundError(resourceID))
}

Expand Down
20 changes: 11 additions & 9 deletions internal/api/arm/operation.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,19 @@ package arm
import (
"encoding/json"
"time"

azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
)

// Operation is an ARM-defined resource returned by operation status endpoints.
type Operation struct {
ID *ResourceID `json:"id,omitempty"`
Name string `json:"name,omitempty"`
Status ProvisioningState `json:"status"`
StartTime *time.Time `json:"startTime,omitempty"`
EndTime *time.Time `json:"endTime,omitempty"`
PercentComplete float64 `json:"percentComplete,omitempty"`
Properties json.RawMessage `json:"peroperties,omitempty"`
Error *CloudErrorBody `json:"error,omitempty"`
Operations []Operation `json:"operations,omitempty"`
ID *azcorearm.ResourceID `json:"id,omitempty"`
Name string `json:"name,omitempty"`
Status ProvisioningState `json:"status"`
StartTime *time.Time `json:"startTime,omitempty"`
EndTime *time.Time `json:"endTime,omitempty"`
PercentComplete float64 `json:"percentComplete,omitempty"`
Properties json.RawMessage `json:"peroperties,omitempty"`
Error *CloudErrorBody `json:"error,omitempty"`
Operations []Operation `json:"operations,omitempty"`
}
Loading

0 comments on commit 7f3cbfc

Please sign in to comment.