Skip to content

Commit

Permalink
fix(HMS-2674): Error when name pattern is invalid
Browse files Browse the repository at this point in the history
  • Loading branch information
adiabramovitch authored and akhil-jha committed Oct 3, 2023
1 parent 88f4954 commit 3a5cf3f
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 10 deletions.
2 changes: 1 addition & 1 deletion api/openapi.gen.json
Original file line number Diff line number Diff line change
Expand Up @@ -1796,7 +1796,7 @@
},
"/reservations/gcp": {
"post": {
"description": "A reservation is a way to activate a job, keeps all data needed for a job to start. A GCP reservation is a reservation created for a GCP job. Image Builder UUID image is required and needs to be shared with the service account. Furthermore, by specifying the name pattern for example as \"instance\", instances names will be created in the format: \"instance-#####\". A single account can create maximum of 2 reservations per second.\n",
"description": "A reservation is a way to activate a job, keeps all data needed for a job to start. A GCP reservation is a reservation created for a GCP job. Image Builder UUID image is required and needs to be shared with the service account. Furthermore, by specifying the RFC-1035 compatible name pattern for example as \"instance\", instances names will be created in the format: \"instance-#####\". A single account can create maximum of 2 reservations per second.\n",
"operationId": "createGCPReservation",
"requestBody": {
"content": {
Expand Down
2 changes: 1 addition & 1 deletion api/openapi.gen.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1262,7 +1262,7 @@ paths:
tags:
- Reservation
description: |
A reservation is a way to activate a job, keeps all data needed for a job to start. A GCP reservation is a reservation created for a GCP job. Image Builder UUID image is required and needs to be shared with the service account. Furthermore, by specifying the name pattern for example as "instance", instances names will be created in the format: "instance-#####". A single account can create maximum of 2 reservations per second.
A reservation is a way to activate a job, keeps all data needed for a job to start. A GCP reservation is a reservation created for a GCP job. Image Builder UUID image is required and needs to be shared with the service account. Furthermore, by specifying the RFC-1035 compatible name pattern for example as "instance", instances names will be created in the format: "instance-#####". A single account can create maximum of 2 reservations per second.
operationId: createGCPReservation
requestBody:
description: gcp request body
Expand Down
2 changes: 1 addition & 1 deletion cmd/spec/path.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ paths:
A reservation is a way to activate a job, keeps all data needed for a job to start.
A GCP reservation is a reservation created for a GCP job. Image Builder UUID image
is required and needs to be shared with the service account.
Furthermore, by specifying the name pattern for example as "instance",
Furthermore, by specifying the RFC-1035 compatible name pattern for example as "instance",
instances names will be created in the format: "instance-#####".
A single account can create maximum of 2 reservations per second.
requestBody:
Expand Down
6 changes: 1 addition & 5 deletions internal/jobs/launch_instance_gcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ func DoLaunchInstanceGCP(ctx context.Context, args *LaunchInstanceGCPTaskArgs) e
// status updates before and after the code logic
updateStatusBefore(ctx, args.ReservationID, "Launching instance(s)")
defer updateStatusAfter(ctx, args.ReservationID, "Launched instance(s)", 1)
name := "inst-####"
pkD := dao.GetPubkeyDao(ctx)

pk, err := pkD.GetById(ctx, args.PubkeyID)
Expand All @@ -120,11 +119,8 @@ func DoLaunchInstanceGCP(ctx context.Context, args *LaunchInstanceGCPTaskArgs) e
return fmt.Errorf("cannot generate user data: %w", err)
}

if args.Detail.NamePattern != nil {
name = fmt.Sprintf("%s-#####", *args.Detail.NamePattern)
}
params := &clients.GCPInstanceParams{
NamePattern: ptr.To(name),
NamePattern: args.Detail.NamePattern,
ImageName: args.ImageName,
MachineType: args.Detail.MachineType,
Zone: args.Zone,
Expand Down
2 changes: 1 addition & 1 deletion internal/jobs/launch_instance_gcp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func prepareGCPReservation(t *testing.T, ctx context.Context, pk *models.Pubkey)
detail := &models.GCPDetail{
Zone: "europe-west8-c",
MachineType: "e2-micro",
NamePattern: ptr.To("instance"),
NamePattern: ptr.To("instance-#####"),
Amount: 1,
PowerOff: false,
}
Expand Down
25 changes: 24 additions & 1 deletion internal/services/gcp_reservation_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package services
import (
"fmt"
"net/http"
"regexp"
"strings"

"github.com/RHEnVision/provisioning-backend/internal/preload"
"github.com/google/uuid"
Expand Down Expand Up @@ -40,9 +42,21 @@ func CreateGCPReservation(w http.ResponseWriter, r *http.Request) {
return
}

namePattern := "inst-####"
// Verify name pattern is lower cased and add #####
if payload.NamePattern != "" {
ok := isValidNamePattern(payload.NamePattern)
if ok {
namePattern = fmt.Sprintf("%s-#####", payload.NamePattern)
} else {
renderError(w, r, payloads.NewInvalidRequestError(r.Context(), "Invalid name pattern", ErrInvalidNamePattern))
return
}
}

resUUID := uuid.New().String()
detail := &models.GCPDetail{
NamePattern: &payload.NamePattern,
NamePattern: &namePattern,
Zone: payload.Zone,
MachineType: payload.MachineType,
Amount: payload.Amount,
Expand Down Expand Up @@ -152,3 +166,12 @@ func CreateGCPReservation(w http.ResponseWriter, r *http.Request) {
return
}
}

func isValidNamePattern(namePattern string) bool {
if strings.ToLower(namePattern) != namePattern {
return false
}
// Define a regular expression pattern to match valid RFC-1035-compatible names
pattern := regexp.MustCompile(`^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$`)
return pattern.MatchString(namePattern)
}
26 changes: 26 additions & 0 deletions internal/services/gcp_reservation_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,30 @@ func TestCreateGCPReservationHandler(t *testing.T) {
assert.Contains(t, rr.Body.String(), "Unsupported zone")
require.Equal(t, http.StatusBadRequest, rr.Code, "Handler returned wrong status code")
})

t.Run("failed reservation with invalid name pattern", func(t *testing.T) {
var err error
values := map[string]interface{}{
"name_pattern": "Envision",
"source_id": source.ID,
"image_id": "80967e7f-efef-4eee-85b0-bd4cef4c455d",
"amount": 1,
"zone": "us-central1-a",
"machine_type": "n1-standard-1",
"pubkey_id": pk.ID,
}
if json_data, err = json.Marshal(values); err != nil {
t.Fatalf("unable to marshal values to json: %v", err)
}

req, err := http.NewRequestWithContext(ctx, "POST", "/api/provisioning/reservations/gcp", bytes.NewBuffer(json_data))
require.NoError(t, err, "failed to create request")
req.Header.Add("Content-Type", "application/json")

rr := httptest.NewRecorder()
handler := http.HandlerFunc(services.CreateGCPReservation)
handler.ServeHTTP(rr, req)
assert.Contains(t, rr.Body.String(), "Invalid name pattern")
require.Equal(t, http.StatusBadRequest, rr.Code, "Handler returned wrong status code")
})
}
1 change: 1 addition & 0 deletions internal/services/reservations_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ var (
ErrArchitectureMismatch = errors.New("instance type and image architecture mismatch")
ErrBothTypeAndTemplateMissing = errors.New("instance type or launch template not set")
ErrUnsupportedRegion = errors.New("unknown region/location/zone")
ErrInvalidNamePattern = errors.New("name pattern is not RFC-1035 compatible")
)

// CreateReservation dispatches requests to type provider specific handlers
Expand Down

0 comments on commit 3a5cf3f

Please sign in to comment.