fix(HMS-2757): Do not cascade on pubkey delete

RHEnVision · Oct 20, 2023 · f9724b4 · f9724b4
1 parent 0479f94
commit f9724b4
Show file tree

Hide file tree

Showing 16 changed files with 70 additions and 28 deletions.
diff --git a/cmd/spec/example_reservation.go b/cmd/spec/example_reservation.go
@@ -86,7 +86,7 @@ var AwsReservationRequestPayloadExample = payloads.AWSReservationRequest{
 }
 
 var AwsReservationResponsePayloadPendingExample = payloads.AWSReservationResponse{
-	PubkeyID:         42,
+	PubkeyID:         ptr.ToInt64(42),
 	SourceID:         "654321",
 	Region:           "us-east-1",
 	InstanceType:     "t3.small",
@@ -99,7 +99,7 @@ var AwsReservationResponsePayloadPendingExample = payloads.AWSReservationRespons
 
 var AwsReservationResponsePayloadDoneExample = payloads.AWSReservationResponse{
 	ID:               1305,
-	PubkeyID:         42,
+	PubkeyID:         ptr.ToInt64(42),
 	SourceID:         "654321",
 	Region:           "us-east-1",
 	InstanceType:     "t3.small",
@@ -133,7 +133,7 @@ var AzureReservationRequestPayloadExample = payloads.AzureReservationRequest{
 
 var AzureReservationResponsePayloadPendingExample = payloads.AzureReservationResponse{
 	ID:            1310,
-	PubkeyID:      42,
+	PubkeyID:      ptr.ToInt64(42),
 	SourceID:      "654321",
 	ResourceGroup: "myCustom Azure RG",
 	Location:      "useast",
@@ -147,7 +147,7 @@ var AzureReservationResponsePayloadPendingExample = payloads.AzureReservationRes
 
 var AzureReservationResponsePayloadDoneExample = payloads.AzureReservationResponse{
 	ID:            1310,
-	PubkeyID:      42,
+	PubkeyID:      ptr.ToInt64(42),
 	SourceID:      "654321",
 	ResourceGroup: "myCustom Azure RG",
 	Location:      "useast",
@@ -179,7 +179,7 @@ var GCPReservationRequestPayloadExample = payloads.GCPReservationRequest{
 
 var GCPReservationResponsePayloadPendingExample = payloads.GCPReservationResponse{
 	ID:               1305,
-	PubkeyID:         42,
+	PubkeyID:         ptr.ToInt64(42),
 	SourceID:         "654321",
 	Zone:             "us-east-4",
 	MachineType:      "e2-micro",
@@ -193,7 +193,7 @@ var GCPReservationResponsePayloadPendingExample = payloads.GCPReservationRespons
 
 var GCPReservationResponsePayloadDoneExample = payloads.GCPReservationResponse{
 	ID:               1305,
-	PubkeyID:         42,
+	PubkeyID:         ptr.ToInt64(42),
 	SourceID:         "654321",
 	Zone:             "us-east-4",
 	MachineType:      "e2-micro",

diff --git a/internal/dao/dao_errors.go b/internal/dao/dao_errors.go
@@ -38,4 +38,7 @@ var (
 
 	// ErrReservationRateExceeded is returned when SQL constraint does not allow to insert more reservations
 	ErrReservationRateExceeded = usrerr.New(429, "rate limit exceeded", "too many reservations, wait and retry")
+
+	// ErrPubkeyNotFound is returned when a nil pointer to a pubkey is used for reservation detail
+	ErrPubkeyNotFound = usrerr.New(404, "pubkey not found", "no pubkey found, it may have been already deleted")
 )
diff --git a/internal/dao/pgx/reservation_pgx.go b/internal/dao/pgx/reservation_pgx.go
@@ -49,11 +49,15 @@ func (x *reservationDao) CreateAWS(ctx context.Context, reservation *models.AWSR
 			return err
 		}
 
+		if reservation.PubkeyID == nil {
+			return fmt.Errorf("pgx error: %w", dao.ErrPubkeyNotFound)
+		}
+
 		awsQuery := `INSERT INTO aws_reservation_details (reservation_id, pubkey_id, source_id, image_id, detail)
 		VALUES ($1, $2, $3, $4, $5)`
 		tag, err := tx.Exec(ctx, awsQuery,
 			reservation.ID,
-			reservation.PubkeyID,
+			&reservation.PubkeyID,
 			reservation.SourceID,
 			reservation.ImageID,
 			reservation.Detail)
@@ -80,6 +84,10 @@ func (x *reservationDao) CreateAzure(ctx context.Context, reservation *models.Az
 			return err
 		}
 
+		if reservation.PubkeyID == nil {
+			return fmt.Errorf("pgx error: %w", dao.ErrPubkeyNotFound)
+		}
+
 		azureQuery := `INSERT INTO azure_reservation_details (reservation_id, pubkey_id, source_id, image_id, detail)
 			VALUES ($1, $2, $3, $4, $5)`
 		tag, err := tx.Exec(ctx, azureQuery,
@@ -111,6 +119,10 @@ func (x *reservationDao) CreateGCP(ctx context.Context, reservation *models.GCPR
 			return err
 		}
 
+		if reservation.PubkeyID == nil {
+			return fmt.Errorf("pgx error: %w", dao.ErrPubkeyNotFound)
+		}
+
 		gcpQuery := `INSERT INTO gcp_reservation_details (reservation_id, pubkey_id, source_id, image_id, detail)
 			VALUES ($1, $2, $3, $4, $5)`
 		tag, err := tx.Exec(ctx, gcpQuery,

diff --git a/internal/dao/tests/reservation_test.go b/internal/dao/tests/reservation_test.go
@@ -12,6 +12,7 @@ import (
 	"github.com/RHEnVision/provisioning-backend/internal/dao"
 	"github.com/RHEnVision/provisioning-backend/internal/db"
 	"github.com/RHEnVision/provisioning-backend/internal/models"
+	"github.com/RHEnVision/provisioning-backend/internal/ptr"
 	"github.com/RHEnVision/provisioning-backend/internal/testing/identity"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -47,7 +48,7 @@ func newAWSReservation() *models.AWSReservation {
 			AccountID: 1,
 			Status:    "Created",
 		},
-		PubkeyID: 1,
+		PubkeyID: ptr.ToInt64(1),
 	}
 }
 
@@ -58,7 +59,7 @@ func newGCPReservation() *models.GCPReservation {
 			AccountID: 1,
 			Status:    "Created",
 		},
-		PubkeyID: 1,
+		PubkeyID: ptr.ToInt64(1),
 	}
 }
 

diff --git a/internal/jobs/launch_instance_aws_test.go b/internal/jobs/launch_instance_aws_test.go
@@ -39,7 +39,7 @@ func prepareAWSReservation(t *testing.T, ctx context.Context, pk *models.Pubkey)
 		PowerOff:     false,
 	}
 	reservation := &models.AWSReservation{
-		PubkeyID: pk.ID,
+		PubkeyID: &pk.ID,
 		SourceID: "irrelevant",
 		ImageID:  "irrelevant",
 		Detail:   detail,

diff --git a/internal/jobs/launch_instance_azure_test.go b/internal/jobs/launch_instance_azure_test.go
@@ -38,7 +38,7 @@ func prepareAzureReservation(t *testing.T, ctx context.Context, pk *models.Pubke
 		PowerOff:     false,
 	}
 	reservation := &models.AzureReservation{
-		PubkeyID: pk.ID,
+		PubkeyID: &pk.ID,
 		SourceID: "irrelevant",
 		ImageID:  "irrelevant",
 		Detail:   detail,

diff --git a/internal/jobs/launch_instance_gcp_test.go b/internal/jobs/launch_instance_gcp_test.go
@@ -38,7 +38,7 @@ func prepareGCPReservation(t *testing.T, ctx context.Context, pk *models.Pubkey)
 		PowerOff:    false,
 	}
 	reservation := &models.GCPReservation{
-		PubkeyID: pk.ID,
+		PubkeyID: &pk.ID,
 		SourceID: "irrelevant",
 		ImageID:  "irrelevant",
 		Detail:   detail,

diff --git a/internal/migrations/sql/021_remove_delete_cascade.sql b/internal/migrations/sql/021_remove_delete_cascade.sql
@@ -0,0 +1,14 @@
+ALTER TABLE aws_reservation_details
+DROP CONSTRAINT aws_reservation_details_pubkey_id_fkey,
+ADD CONSTRAINT aws_reservation_details_pubkey_id_fkey
+FOREIGN key (pubkey_id) REFERENCES pubkeys(id) ON DELETE SET NULL;
+
+ALTER TABLE gcp_reservation_details
+DROP CONSTRAINT gcp_reservation_details_pubkey_id_fkey,
+ADD CONSTRAINT gcp_reservation_details_pubkey_id_fkey
+FOREIGN key (pubkey_id) REFERENCES pubkeys(id) ON DELETE SET NULL;
+
+ALTER TABLE azure_reservation_details
+DROP CONSTRAINT azure_reservation_details_pubkey_id_fkey,
+ADD CONSTRAINT azure_reservation_details_pubkey_id_fkey
+FOREIGN key (pubkey_id) REFERENCES pubkeys(id) ON DELETE SET NULL;
diff --git a/internal/migrations/sql/022_pubkey_nullable.sql b/internal/migrations/sql/022_pubkey_nullable.sql
@@ -0,0 +1,3 @@
+ALTER TABLE aws_reservation_details ALTER COLUMN pubkey_id DROP NOT NULL;
+ALTER TABLE gcp_reservation_details ALTER COLUMN pubkey_id DROP NOT NULL;
+ALTER TABLE azure_reservation_details ALTER COLUMN pubkey_id DROP NOT NULL;
diff --git a/internal/models/reservation_model.go b/internal/models/reservation_model.go
@@ -76,7 +76,7 @@ type AWSReservation struct {
 	Reservation
 
 	// Pubkey ID.
-	PubkeyID int64 `db:"pubkey_id" json:"pubkey_id"`
+	PubkeyID *int64 `db:"pubkey_id" json:"pubkey_id,omitempty"`
 
 	// Source ID.
 	SourceID string `db:"source_id" json:"source_id"`
@@ -118,7 +118,7 @@ type GCPReservation struct {
 	Reservation
 
 	// Pubkey ID.
-	PubkeyID int64 `db:"pubkey_id" json:"pubkey_id"`
+	PubkeyID *int64 `db:"pubkey_id" json:"pubkey_id,omitempty"`
 
 	// Source ID.
 	SourceID string `db:"source_id" json:"source_id"`
@@ -158,7 +158,7 @@ type AzureReservation struct {
 	Reservation
 
 	// Pubkey ID.
-	PubkeyID int64 `db:"pubkey_id" json:"pubkey_id"`
+	PubkeyID *int64 `db:"pubkey_id" json:"pubkey_id,omitempty"`
 
 	// Source ID.
 	SourceID string `db:"source_id" json:"source_id"`

diff --git a/internal/payloads/reservation_payload.go b/internal/payloads/reservation_payload.go
@@ -55,7 +55,7 @@ type AWSReservationResponse struct {
 	ID int64 `json:"reservation_id" yaml:"reservation_id"`
 
 	// Pubkey ID.
-	PubkeyID int64 `json:"pubkey_id" yaml:"pubkey_id"`
+	PubkeyID *int64 `json:"pubkey_id,omitempty" yaml:"pubkey_id,omitempty"`
 
 	// Source ID.
 	SourceID string `json:"source_id" yaml:"source_id"`
@@ -91,7 +91,7 @@ type AWSReservationResponse struct {
 type AzureReservationResponse struct {
 	ID int64 `json:"reservation_id" yaml:"reservation_id"`
 
-	PubkeyID int64 `json:"pubkey_id" yaml:"pubkey_id"`
+	PubkeyID *int64 `json:"pubkey_id,omitempty" yaml:"pubkey_id,omitempty"`
 
 	SourceID string `json:"source_id" yaml:"source_id"`
 
@@ -123,7 +123,7 @@ type GCPReservationResponse struct {
 	ID int64 `json:"reservation_id" yaml:"reservation_id"`
 
 	// Pubkey ID.
-	PubkeyID int64 `json:"pubkey_id" yaml:"pubkey_id"`
+	PubkeyID *int64 `json:"pubkey_id,omitempty" yaml:"pubkey_id,omitempty"`
 
 	// Source ID.
 	SourceID string `json:"source_id" yaml:"source_id"`

diff --git a/internal/services/aws_reservation_service.go b/internal/services/aws_reservation_service.go
@@ -75,7 +75,7 @@ func CreateAWSReservation(w http.ResponseWriter, r *http.Request) {
 		PowerOff:         payload.PowerOff,
 	}
 	reservation := &models.AWSReservation{
-		PubkeyID: payload.PubkeyID,
+		PubkeyID: &payload.PubkeyID,
 		SourceID: payload.SourceID,
 		ImageID:  payload.ImageID,
 		Detail:   detail,
@@ -89,8 +89,12 @@ func CreateAWSReservation(w http.ResponseWriter, r *http.Request) {
 	reservation.Detail.Name = newName
 
 	// validate pubkey - must be always present because of data integrity (foreign keys)
-	logger.Debug().Msgf("Validating existence of pubkey %d for this account", reservation.PubkeyID)
-	pk, err := pkDao.GetById(r.Context(), reservation.PubkeyID)
+	if reservation.PubkeyID == nil {
+		renderError(w, r, payloads.NewNotFoundError(r.Context(), "could not create AWS reservation", ErrPubkeyNotFound))
+	}
+
+	logger.Debug().Msgf("Validating existence of pubkey %d for this account", *reservation.PubkeyID)
+	pk, err := pkDao.GetById(r.Context(), *reservation.PubkeyID)
 	if err != nil {
 		message := fmt.Sprintf("get pubkey with id %d", reservation.PubkeyID)
 		renderNotFoundOrDAOError(w, r, err, message)

diff --git a/internal/services/azure_reservation_service.go b/internal/services/azure_reservation_service.go
@@ -125,7 +125,7 @@ func CreateAzureReservation(w http.ResponseWriter, r *http.Request) {
 		Name:          name,
 	}
 	reservation := &models.AzureReservation{
-		PubkeyID: payload.PubkeyID,
+		PubkeyID: &payload.PubkeyID,
 		SourceID: payload.SourceID,
 		ImageID:  payload.ImageID,
 		Detail:   detail,

diff --git a/internal/services/gcp_reservation_service.go b/internal/services/gcp_reservation_service.go
@@ -67,7 +67,7 @@ func CreateGCPReservation(w http.ResponseWriter, r *http.Request) {
 		LaunchTemplateID: payload.LaunchTemplateID,
 	}
 	reservation := &models.GCPReservation{
-		PubkeyID: payload.PubkeyID,
+		PubkeyID: &payload.PubkeyID,
 		ImageID:  payload.ImageID,
 		SourceID: payload.SourceID,
 		Detail:   detail,
@@ -79,10 +79,14 @@ func CreateGCPReservation(w http.ResponseWriter, r *http.Request) {
 	reservation.Steps = 2
 	reservation.StepTitles = jobs.LaunchInstanceGCPSteps
 
-	logger.Debug().Msgf("Validating existence of pubkey %d for this account", reservation.PubkeyID)
-	pk, err := pkDao.GetById(r.Context(), reservation.PubkeyID)
+	if reservation.PubkeyID == nil {
+		renderError(w, r, payloads.NewNotFoundError(r.Context(), "could not create AWS reservation", ErrPubkeyNotFound))
+	}
+
+	logger.Debug().Msgf("Validating existence of pubkey %d for this account", *reservation.PubkeyID)
+	pk, err := pkDao.GetById(r.Context(), *reservation.PubkeyID)
 	if err != nil {
-		message := fmt.Sprintf("get pubkey with id %d", reservation.PubkeyID)
+		message := fmt.Sprintf("get pubkey with id %d", *reservation.PubkeyID)
 		renderNotFoundOrDAOError(w, r, err, message)
 		return
 	}
@@ -147,7 +151,7 @@ func CreateGCPReservation(w http.ResponseWriter, r *http.Request) {
 		Args: jobs.LaunchInstanceGCPTaskArgs{
 			ReservationID:    reservation.ID,
 			Zone:             reservation.Detail.Zone,
-			PubkeyID:         reservation.PubkeyID,
+			PubkeyID:         *reservation.PubkeyID,
 			Detail:           reservation.Detail,
 			ImageName:        name,
 			ProjectID:        authentication,

diff --git a/internal/services/reservations_service.go b/internal/services/reservations_service.go
@@ -23,6 +23,7 @@ var (
 	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")
+	ErrPubkeyNotFound             = errors.New("no pubkey found")
 )
 
 // CreateReservation dispatches requests to type provider specific handlers

diff --git a/internal/services/reservations_service_test.go b/internal/services/reservations_service_test.go
@@ -43,7 +43,7 @@ func TestGetReservationDetail(t *testing.T) {
 			PowerOff:     true,
 		}
 		reservation := &models.AWSReservation{
-			PubkeyID: pk.ID,
+			PubkeyID: &pk.ID,
 			SourceID: "1",
 			ImageID:  "ami-random",
 			Detail:   detail,