Skip to content

Commit

Permalink
frontend: Generalize ArmResourceList to also handle node pools
Browse files Browse the repository at this point in the history 
I was surprised to discover the collection endpoint for node pools
is a valid resource ID (according to ParseResourceID), whereas the
collection endpoints for clusters (by subscription and by resource
group) are not.

This required tweaking the static validation middleware, which was
blocking node pool collection requests because the parsed resource
ID had no resource name.
  • Loading branch information
Matthew Barnes committed Nov 23, 2024
1 parent 53de71c commit 807f48a
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 48 deletions.
84 changes: 61 additions & 23 deletions frontend/pkg/frontend/frontend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"net"
"net/http"
"os"
"path"
"strconv"
"strings"
"sync/atomic"
Expand Down Expand Up @@ -175,6 +176,8 @@ func (f *Frontend) ArmResourceList(writer http.ResponseWriter, request *http.Req

subscriptionID := request.PathValue(PathSegmentSubscriptionID)
resourceGroupName := request.PathValue(PathSegmentResourceGroupName)
resourceName := request.PathValue(PathSegmentResourceName)
resourceTypeName := path.Base(request.URL.Path)

// Even though the bulk of the list content comes from Cluster Service,
// we start by querying Cosmos DB because its continuation token meets
Expand All @@ -185,18 +188,25 @@ func (f *Frontend) ArmResourceList(writer http.ResponseWriter, request *http.Req
if resourceGroupName != "" {
prefixString += "/resourceGroups/" + resourceGroupName
}
if resourceName != "" {
// This is a nested resource request. Build a resource ID for
// the parent cluster. We use this below to get the cluster's
// ResourceDocument from Cosmos DB.
prefixString += "/providers/" + api.ProviderNamespace
prefixString += "/" + api.ClusterResourceTypeName + "/" + resourceName
}
prefix, err := arm.ParseResourceID(prefixString)
if err != nil {
f.logger.Error(err.Error())
arm.WriteInternalServerError(writer)
return
}

iterator := f.dbClient.ListResourceDocs(ctx, prefix, pageSizeHint, continuationToken)
dbIterator := f.dbClient.ListResourceDocs(ctx, prefix, pageSizeHint, continuationToken)

// Build a map of cluster documents by Cluster Service cluster ID.
documentMap := make(map[string]*database.ResourceDocument)
for item := range iterator.Items(ctx) {
for item := range dbIterator.Items(ctx) {
var doc database.ResourceDocument

err = json.Unmarshal(item, &doc)
Expand All @@ -206,12 +216,12 @@ func (f *Frontend) ArmResourceList(writer http.ResponseWriter, request *http.Req
return
}

if strings.EqualFold(doc.Key.ResourceType.String(), api.ClusterResourceType.String()) {
if strings.HasSuffix(strings.ToLower(doc.Key.ResourceType.Type), resourceTypeName) {
documentMap[doc.InternalID.ID()] = &doc
}
}

err = iterator.GetError()
err = dbIterator.GetError()
if err != nil {
f.logger.Error(err.Error())
arm.WriteInternalServerError(writer)
Expand All @@ -226,33 +236,61 @@ func (f *Frontend) ArmResourceList(writer http.ResponseWriter, request *http.Req
query := fmt.Sprintf("id in (%s)", strings.Join(queryIDs, ", "))
f.logger.Info(fmt.Sprintf("Searching Cluster Service for %q", query))

listRequest := f.clusterServiceClient.GetConn().ClustersMgmt().V1().Clusters().List().Search(query)
switch resourceTypeName {
case strings.ToLower(api.ClusterResourceTypeName):
csIterator := f.clusterServiceClient.ListCSClusters(query)

for csCluster := range csIterator.Items(ctx) {
if doc, ok := documentMap[csCluster.ID()]; ok {
value, err := marshalCSCluster(csCluster, doc, versionedInterface)
if err != nil {
f.logger.Error(err.Error())
arm.WriteInternalServerError(writer)
return
}
pagedResponse.AddValue(value)
}
}
err = csIterator.GetError()

case strings.ToLower(api.NodePoolResourceTypeName):
var resourceDoc *database.ResourceDocument

// Fetch the cluster document for the Cluster Service ID.
resourceDoc, err = f.dbClient.GetResourceDoc(ctx, prefix)
if err != nil {
f.logger.Error(err.Error())
arm.WriteInternalServerError(writer)
return
}

csIterator := f.clusterServiceClient.ListCSNodePools(resourceDoc.InternalID, query)

// XXX This SHOULD avoid dealing with pagination from Cluster Service.
// As far I can tell, uhc-cluster-service does not impose its own
// limit on the page size. Further testing is needed to verify.
listRequest.Size(len(documentMap))
for csNodePool := range csIterator.Items(ctx) {
if doc, ok := documentMap[csNodePool.ID()]; ok {
value, err := marshalCSNodePool(csNodePool, doc, versionedInterface)
if err != nil {
f.logger.Error(err.Error())
arm.WriteInternalServerError(writer)
return
}
pagedResponse.AddValue(value)
}
}
err = csIterator.GetError()

listResponse, err := listRequest.SendContext(ctx)
default:
err = fmt.Errorf("unsupported resource type: %s", resourceTypeName)
}

// Check for iteration error.
if err != nil {
f.logger.Error(err.Error())
arm.WriteInternalServerError(writer)
return
}

for _, csCluster := range listResponse.Items().Slice() {
if doc, ok := documentMap[csCluster.ID()]; ok {
value, err := marshalCSCluster(csCluster, doc, versionedInterface)
if err != nil {
f.logger.Error(err.Error())
arm.WriteInternalServerError(writer)
return
}
pagedResponse.AddValue(value)
}
}

err = pagedResponse.SetNextLink(request.Referer(), iterator.GetContinuationToken())
err = pagedResponse.SetNextLink(request.Referer(), dbIterator.GetContinuationToken())
if err != nil {
f.logger.Error(err.Error())
arm.WriteInternalServerError(writer)
Expand Down
11 changes: 11 additions & 0 deletions frontend/pkg/frontend/middleware_resourceid_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,17 @@ func TestMiddlewareResourceID(t *testing.T) {
"Microsoft.Resources/tenants",
},
},
{
name: "node pool collection",
path: "/SUBSCRIPTIONS/00000000-0000-0000-0000-000000000000/RESOURCEGROUPS/MyResourceGroup/PROVIDERS/MICROSOFT.REDHATOPENSHIFT/HCPOPENSHIFTCLUSTERS/myCluster/NODEPOOLS",
resourceTypes: []string{
"MICROSOFT.REDHATOPENSHIFT/HCPOPENSHIFTCLUSTERS/NODEPOOLS",
"MICROSOFT.REDHATOPENSHIFT/HCPOPENSHIFTCLUSTERS",
"Microsoft.Resources/resourceGroups",
"Microsoft.Resources/subscriptions",
"Microsoft.Resources/tenants",
},
},
{
name: "preflight deployment",
path: "/SUBSCRIPTIONS/00000000-0000-0000-0000-000000000000/RESOURCEGROUPS/MyResourceGroup/PROVIDERS/MICROSOFT.REDHATOPENSHIFT/DEPLOYMENTS/preflight",
Expand Down
47 changes: 22 additions & 25 deletions frontend/pkg/frontend/middleware_validatestatic.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import (
var rxHCPOpenShiftClusterResourceName = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9-]{2,53}$`)
var rxNodePoolResourceName = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9-]{2,14}$`)

var resourceTypeSubscription = "Microsoft.Resources/subscriptions"

func MiddlewareValidateStatic(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
// To conform with "OAPI012: Resource IDs must not be case sensitive"
// we need to use the original, non-lowercased resource ID components
Expand All @@ -40,29 +38,28 @@ func MiddlewareValidateStatic(w http.ResponseWriter, r *http.Request, next http.
}
}

// Skip static validation for subscription resources
if !strings.EqualFold(resource.ResourceType.String(), resourceTypeSubscription) {
switch strings.ToLower(resource.ResourceType.Type) {
case strings.ToLower(api.ClusterResourceType.Type):
if !rxHCPOpenShiftClusterResourceName.MatchString(resource.Name) {
arm.WriteError(w, http.StatusBadRequest,
arm.CloudErrorCodeInvalidResourceName,
resource.String(),
"The Resource '%s/%s' under resource group '%s' does not conform to the naming restriction.",
resource.ResourceType, resource.Name,
resource.ResourceGroupName)
return
}
case strings.ToLower(api.NodePoolResourceType.Type):
if !rxNodePoolResourceName.MatchString(resource.Name) {
arm.WriteError(w, http.StatusBadRequest,
arm.CloudErrorCodeInvalidResourceName,
resource.String(),
"The Resource '%s/%s' under resource group '%s' does not conform to the naming restriction.",
resource.ResourceType, resource.Name,
resource.ResourceGroupName)
return
}
switch strings.ToLower(resource.ResourceType.Type) {
case strings.ToLower(api.ClusterResourceType.Type):
if !rxHCPOpenShiftClusterResourceName.MatchString(resource.Name) {
arm.WriteError(w, http.StatusBadRequest,
arm.CloudErrorCodeInvalidResourceName,
resource.String(),
"The Resource '%s/%s' under resource group '%s' does not conform to the naming restriction.",
resource.ResourceType, resource.Name,
resource.ResourceGroupName)
return
}
case strings.ToLower(api.NodePoolResourceType.Type):
// The collection GET endpoint for nested resources
// parses into a ResourceID with an empty Name field.
if resource.Name != "" && !rxNodePoolResourceName.MatchString(resource.Name) {
arm.WriteError(w, http.StatusBadRequest,
arm.CloudErrorCodeInvalidResourceName,
resource.String(),
"The Resource '%s/%s' under resource group '%s' does not conform to the naming restriction.",
resource.ResourceType, resource.Name,
resource.ResourceGroupName)
return
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions frontend/pkg/frontend/routes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ func (f *Frontend) routes() *MiddlewareMux {
mux.Handle(
MuxPattern(http.MethodGet, PatternSubscriptions, PatternResourceGroups, PatternProviders, api.ClusterResourceTypeName),
postMuxMiddleware.HandlerFunc(f.ArmResourceList))
mux.Handle(
MuxPattern(http.MethodGet, PatternSubscriptions, PatternResourceGroups, PatternProviders, PatternClusters, api.NodePoolResourceTypeName),
postMuxMiddleware.HandlerFunc(f.ArmResourceList))

// Resource ID endpoints
// Request context holds an azcorearm.ResourceID
Expand Down

0 comments on commit 807f48a

Please sign in to comment.