Skip to content

Commit

Permalink
frontend: Give ArmDeploymentPreflight some overdue love
Browse files Browse the repository at this point in the history 
Dusted off this old method to add node pool support and write some
unit tests for it.
  • Loading branch information
Matthew Barnes committed Feb 27, 2025
1 parent ccd7c03 commit e634f8d
Show file tree
Hide file tree
Showing 3 changed files with 293 additions and 26 deletions.
77 changes: 55 additions & 22 deletions frontend/pkg/frontend/frontend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -785,13 +785,10 @@ func (f *Frontend) ArmSubscriptionPut(writer http.ResponseWriter, request *http.
func (f *Frontend) ArmDeploymentPreflight(writer http.ResponseWriter, request *http.Request) {
var subscriptionID string = request.PathValue(PathSegmentSubscriptionID)
var resourceGroup string = request.PathValue(PathSegmentResourceGroupName)
var apiVersion string = request.URL.Query().Get("api-version")

ctx := request.Context()
logger := LoggerFromContext(ctx)

logger.Info(fmt.Sprintf("%s: ArmDeploymentPreflight", apiVersion))

body, err := BodyFromContext(ctx)
if err != nil {
logger.Error(err.Error())
Expand All @@ -810,6 +807,8 @@ func (f *Frontend) ArmDeploymentPreflight(writer http.ResponseWriter, request *h
preflightErrors := []arm.CloudErrorBody{}

for index, raw := range deploymentPreflight.Resources {
var cloudError *arm.CloudError

resource := &arm.DeploymentPreflightResource{}
err = json.Unmarshal(raw, resource)
if err != nil {
Expand All @@ -818,30 +817,64 @@ func (f *Frontend) ArmDeploymentPreflight(writer http.ResponseWriter, request *h
logger.Warn(cloudError.Message)
}

// This is just "preliminary" validation to ensure all the base resource
// fields are present and the API version is valid.
resourceErrors := api.ValidateRequest(validate, request.Method, resource)
if len(resourceErrors) > 0 {
// Preflight is best-effort: a malformed resource is not a validation failure.
logger.Warn(
fmt.Sprintf("Resource #%d failed preliminary validation (see details)", index+1),
"details", resourceErrors)
continue
}
switch strings.ToLower(resource.Type) {
case strings.ToLower(api.ClusterResourceType.String()):
// This is just "preliminary" validation to ensure all the base resource
// fields are present and the API version is valid.
resourceErrors := api.ValidateRequest(validate, request.Method, resource)
if len(resourceErrors) > 0 {
// Preflight is best-effort: a malformed resource is not a validation failure.
logger.Warn(
fmt.Sprintf("Resource #%d failed preliminary validation (see details)", index+1),
"details", resourceErrors)
continue
}

// API version is already validated by this point.
versionedInterface, _ := api.Lookup(resource.APIVersion)
versionedCluster := versionedInterface.NewHCPOpenShiftCluster(nil)
// API version is already validated by this point.
versionedInterface, _ := api.Lookup(resource.APIVersion)
versionedCluster := versionedInterface.NewHCPOpenShiftCluster(nil)

err = json.Unmarshal(raw, versionedCluster)
if err != nil {
// Preflight is best effort: failure to parse a resource is not a validation failure.
logger.Warn(fmt.Sprintf("Failed to unmarshal %s resource named '%s': %s", resource.Type, resource.Name, err))
err = resource.Convert(versionedCluster)
if err != nil {
// Preflight is best effort: failure to parse a resource is not a validation failure.
logger.Warn(fmt.Sprintf("Failed to unmarshal %s resource named '%s': %s", resource.Type, resource.Name, err))
continue
}

// Perform static validation as if for a cluster creation request.
cloudError = versionedCluster.ValidateStatic(versionedCluster, false, http.MethodPut)

case strings.ToLower(api.NodePoolResourceType.String()):
// This is just "preliminary" validation to ensure all the base resource
// fields are present and the API version is valid.
resourceErrors := api.ValidateRequest(validate, request.Method, resource)
if len(resourceErrors) > 0 {
// Preflight is best-effort: a malformed resource is not a validation failure.
logger.Warn(
fmt.Sprintf("Resource #%d failed preliminary validation (see details)", index+1),
"details", resourceErrors)
continue
}

// API version is already validated by this point.
versionedInterface, _ := api.Lookup(resource.APIVersion)
versionedNodePool := versionedInterface.NewHCPOpenShiftClusterNodePool(nil)

err = resource.Convert(versionedNodePool)
if err != nil {
// Preflight is best effort: failure to parse a resource is not a validation failure.
logger.Warn(fmt.Sprintf("Failed to unmarshal %s resource named '%s': %s", resource.Type, resource.Name, err))
continue
}

// Perform static validation as if for a node pool creation request.
cloudError = versionedNodePool.ValidateStatic(versionedNodePool, false, http.MethodPut)

default:
// Disregard foreign resource types.
continue
}

// Perform static validation as if for a cluster creation request.
cloudError := versionedCluster.ValidateStatic(versionedCluster, false, http.MethodPut)
if cloudError != nil {
var details []arm.CloudErrorBody

Expand Down
213 changes: 213 additions & 0 deletions frontend/pkg/frontend/frontend_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package frontend
import (
"bytes"
"encoding/json"
"fmt"
"io"
"log/slog"
"maps"
Expand Down Expand Up @@ -306,6 +307,218 @@ func TestSubscriptionsPUT(t *testing.T) {
}
}

func TestDeploymentPreflight(t *testing.T) {
tests := []struct {
name string
resource map[string]any
expectStatus arm.DeploymentPreflightStatus
expectErrors int
}{
{
name: "Unhandled resource type returns no error",
resource: map[string]any{
"name": "virtual-machine",
"type": "Microsoft.Compute/virtualMachines",
"location": "eastus",
"apiVersion": "2024-07-01",
},
expectStatus: arm.DeploymentPreflightStatusSucceeded,
},
{
name: "Unrecognized API version returns no error",
resource: map[string]any{
"name": "my-hcp-cluster",
"type": api.ClusterResourceType.String(),
"location": "eastus",
"apiVersion": "1980-01-01",
},
expectStatus: arm.DeploymentPreflightStatusSucceeded,
},
{
name: "Well-formed cluster resource returns no error",
resource: map[string]any{
"name": "my-hcp-cluster",
"type": api.ClusterResourceType.String(),
"location": "eastus",
"apiVersion": "2024-06-10-preview",
"properties": map[string]any{
"version": map[string]any{
"id": "4.0.0",
"channelGroup": "stable",
},
"network": map[string]any{
"podCidr": "10.128.0.0/14",
"serviceCidr": "172.30.0.0/16",
"machineCidr": "10.0.0.0/16",
},
"api": map[string]any{
"visibility": "public",
},
"platform": map[string]any{
"subnetId": "/something/something/virtualNetworks/subnets",
},
},
},
expectStatus: arm.DeploymentPreflightStatusSucceeded,
},
{
name: "Preflight catches cluster resource with invalid fields",
resource: map[string]any{
"name": "my-hcp-cluster",
"type": api.ClusterResourceType.String(),
"location": "eastus",
"apiVersion": "2024-06-10-preview",
"properties": map[string]any{
"version": map[string]any{
"id": "openshift-v4.0.0",
"channelGroup": "stable",
},
"network": map[string]any{
// 1 invalid + 2 missing required fields
"podCidr": "invalidCidr",
},
"api": map[string]any{
// 1 invalid field
"visibility": "invisible",
},
"platform": map[string]any{
// 1 missing required field
},
},
},
expectStatus: arm.DeploymentPreflightStatusFailed,
expectErrors: 5,
},
{
name: "Well-formed node pool resource returns no error",
resource: map[string]any{
"name": "my-node-pool",
"type": api.NodePoolResourceType.String(),
"location": "eastus",
"apiVersion": "2024-06-10-preview",
"properties": map[string]any{
"version": map[string]any{
"id": "openshift-v4.0.0",
"channelGroup": "stable",
},
"platform": map[string]any{
"vmSize": "Standard_D8s_v3",
},
},
},
expectStatus: arm.DeploymentPreflightStatusSucceeded,
},
{
name: "Preflight catches node pool resource with invalid fields",
resource: map[string]any{
"name": "my-node-pool",
"type": api.NodePoolResourceType.String(),
"location": "eastus",
"apiVersion": "2024-06-10-preview",
"properties": map[string]any{
"version": map[string]any{
"id": "openshift-v4.0.0",
"channelGroup": "stable",
},
"platform": map[string]any{
// 1 missing required field
},
"autoScaling": map[string]any{
// 1 invalid field
"min": 3,
"max": 1,
},
"taints": []map[string]any{
{
// 1 invalid + 1 missing required field
"effect": "NoTouchy",
},
},
},
},
expectStatus: arm.DeploymentPreflightStatusFailed,
expectErrors: 4,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
preflightPath := fmt.Sprintf("/subscriptions/%s/resourceGroups/myRG/providers/%s/deployments/myDeployment/preflight", subscriptionID, api.ProviderNamespace)

ctrl := gomock.NewController(t)
mockDBClient := mocks.NewMockDBClient(ctrl)
reg := prometheus.NewRegistry()

f := NewFrontend(
testLogger,
nil,
nil,
reg,
mockDBClient,
"",
nil,
)

// MiddlewareValidateSubscriptionState and MetricsMiddleware
mockDBClient.EXPECT().
GetSubscriptionDoc(gomock.Any(), subscriptionID).
Return(&arm.Subscription{
State: arm.SubscriptionStateRegistered,
}, nil).
MaxTimes(2)

subs := map[string]*arm.Subscription{
subscriptionID: &arm.Subscription{
State: arm.SubscriptionStateRegistered,
},
}
ts := newHTTPServer(f, ctrl, mockDBClient, subs)

resource, err := json.Marshal(&test.resource)
require.NoError(t, err)
preflightReq := arm.DeploymentPreflight{
Resources: []json.RawMessage{resource},
}
body, err := json.Marshal(&preflightReq)
require.NoError(t, err)

req, err := http.NewRequest(http.MethodPost, ts.URL+preflightPath, bytes.NewReader(body))
require.NoError(t, err)
req.Header.Set("Content-Type", "application/json")

resp, err := ts.Client().Do(req)
require.NoError(t, err)

assert.Equal(t, http.StatusOK, resp.StatusCode)

defer resp.Body.Close()
body, err = io.ReadAll(resp.Body)
require.NoError(t, err)

var preflightResp arm.DeploymentPreflightResponse
err = json.Unmarshal(body, &preflightResp)
require.NoError(t, err)

assert.Equal(t, test.expectStatus, preflightResp.Status)
switch test.expectErrors {
case 0:
assert.Nil(t, preflightResp.Error)
case 1:
if assert.NotNil(t, preflightResp.Error) {
assert.Nil(t, preflightResp.Error.Details)
assert.NotEmpty(t, preflightResp.Error.Code)
assert.NotEmpty(t, preflightResp.Error.Message)
assert.NotEmpty(t, preflightResp.Error.Target)
}
default:
if assert.NotNil(t, preflightResp.Error) {
assert.Equal(t, test.expectErrors, len(preflightResp.Error.Details))
}
}
})
}
}

func lintMetrics(t *testing.T, r prometheus.Gatherer) {
t.Helper()

Expand Down
29 changes: 25 additions & 4 deletions internal/api/arm/preflight.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,31 @@ func UnmarshalDeploymentPreflight(data []byte) (*DeploymentPreflight, *CloudErro

// DeploymentPreflightResource represents a desired resource in a deployment preflight request.
type DeploymentPreflightResource struct {
Name string `json:"name" validate:"required"`
Type string `json:"type" validate:"required"`
Location string `json:"location" validate:"required"`
APIVersion string `json:"apiVersion" validate:"required,api_version"`
Name string `json:"name" validate:"required"`
Type string `json:"type" validate:"required"`
Location string `json:"location" validate:"required"`
APIVersion string `json:"apiVersion,omitempty" validate:"required,api_version"`

// Preserve other tracked resource fields as raw data.
Properties json.RawMessage `json:"properties,omitempty"`
SystemData json.RawMessage `json:"systemData,omitempty"`
Tags json.RawMessage `json:"tags,omitempty"`
}

// Convert discards the APIVersion, marshals itself back to raw JSON,
// and then unmarshals the raw JSON to the given value, which should
// be an extension of the TrackedResource type.
func (r *DeploymentPreflightResource) Convert(v any) error {
var clone DeploymentPreflightResource = *r

// Omit APIVersion from the clone.
clone.APIVersion = ""

data, err := json.Marshal(&clone)
if err != nil {
return err
}
return json.Unmarshal(data, v)
}

// ResourceID returns a resource ID string for the resource.
Expand Down

0 comments on commit e634f8d

Please sign in to comment.