Skip to content

[scd] Align constraint and op-intent validation #1377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
RustedBones wants to merge 1 commit into
base: master
Choose a base branch
from
+108 −68
Open

[scd] Align constraint and op-intent validation #1377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 104 additions & 64 deletions pkg/scd/constraints_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package scd

import (
"context"
"time"

"github.com/golang/geo/s2"
"github.com/interuss/dss/pkg/api"
Expand Down Expand Up @@ -260,67 +261,23 @@ func (a *Server) UpdateConstraintReference(ctx context.Context, req *restapi.Upd
// If the ovn argument is empty (""), it will attempt to create a new Constraint.
func (a *Server) PutConstraintReference(ctx context.Context, manager string, entityid restapi.EntityID, ovn restapi.EntityOVN, params *restapi.PutConstraintReferenceParameters,
) (*restapi.ChangeConstraintReferenceResponse, error) {
id, err := dssmodels.IDFromString(string(entityid))

validParams, err := validateAndReturnConstraintUpsertParams(time.Now(), entityid, ovn, params, a.AllowHTTPBaseUrls)
if err != nil {
return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format: `%s`", entityid)
}

var extents = make([]*dssmodels.Volume4D, len(params.Extents))

if len(params.UssBaseUrl) == 0 {
return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing required UssBaseUrl")
}

if !a.AllowHTTPBaseUrls {
err = scdmodels.ValidateUSSBaseURL(string(params.UssBaseUrl))
if err != nil {
return nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate base URL")
}
}

// TODO: factor out logic below into common multi-vol4d parser and reuse with PutOperationReference
for idx, extent := range params.Extents {
cExtent, err := dssmodels.Volume4DFromSCDRest(&extent)
if err != nil {
return nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to parse extent %d", idx)
}
extents[idx] = cExtent
}
uExtent, err := dssmodels.UnionVolumes4D(extents...)
if err != nil {
return nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to union extents")
}

if uExtent.StartTime == nil {
return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing time_start from extents")
}
if uExtent.EndTime == nil {
return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing time_end from extents")
}

if uExtent.StartTime.After(*uExtent.EndTime) {
return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Constraint time_end must be after time_start")
}

cells, err := uExtent.CalculateSpatialCovering()
if err != nil {
return nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid area")
return nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate Constraint upsert parameters")
}

var response *restapi.ChangeConstraintReferenceResponse
action := func(ctx context.Context, r repos.Repository) (err error) {
var version int32 // Version of the Constraint (0 means creation requested).
version := scdmodels.VersionNumber(1)

// Get existing Constraint, if any, and validate request
old, err := r.GetConstraint(ctx, id)
old, err := r.GetConstraint(ctx, validParams.id)
switch {
case err == pgx.ErrNoRows:
// No existing Constraint; verify that creation was requested
if ovn != "" {
return stacktrace.NewErrorWithCode(dsserr.VersionMismatch, "Old version %s does not exist", ovn)
}
version = 0
case err != nil:
return stacktrace.Propagate(err, "Could not get Constraint from repo")
}
Expand All @@ -333,13 +290,13 @@ func (a *Server) PutConstraintReference(ctx context.Context, manager string, ent
return stacktrace.NewErrorWithCode(dsserr.VersionMismatch,
"Current version is %s but client specified version %s", old.OVN, ovn)
}
version = int32(old.Version)
version = old.Version + 1
}

// Compute total affected Volume4D for notification purposes
var notifyVol4 *dssmodels.Volume4D
if old == nil {
notifyVol4 = uExtent
notifyVol4 = validParams.uExtent
} else {
oldVol4 := &dssmodels.Volume4D{
StartTime: old.StartTime,
Expand All @@ -351,26 +308,17 @@ func (a *Server) PutConstraintReference(ctx context.Context, manager string, ent
return old.Cells, nil
}),
}}
notifyVol4, err = dssmodels.UnionVolumes4D(uExtent, oldVol4)
notifyVol4, err = dssmodels.UnionVolumes4D(validParams.uExtent, oldVol4)
if err != nil {
return stacktrace.Propagate(err, "Error constructing 4D volumes union")
}
}

// Construct the new Constraint
constraint := validParams.toConstraint(dssmodels.Manager(manager), version)

// Upsert the Constraint
constraint, err := r.UpsertConstraint(ctx, &scdmodels.Constraint{
ID: id,
Manager: dssmodels.Manager(manager),
Version: scdmodels.VersionNumber(version + 1),

StartTime: uExtent.StartTime,
EndTime: uExtent.EndTime,
AltitudeLower: uExtent.SpatialVolume.AltitudeLo,
AltitudeUpper: uExtent.SpatialVolume.AltitudeHi,

USSBaseURL: string(params.UssBaseUrl),
Cells: cells,
})
constraint, err = r.UpsertConstraint(ctx, constraint)
if err != nil {
return err
}
Expand Down Expand Up @@ -412,6 +360,98 @@ func (a *Server) PutConstraintReference(ctx context.Context, manager string, ent
return response, nil
}

type validConstraintParams struct {
id dssmodels.ID
extents []*dssmodels.Volume4D
uExtent *dssmodels.Volume4D
cells s2.CellUnion
ussBaseURL string
}

func (vp *validConstraintParams) toConstraint(manager dssmodels.Manager, version scdmodels.VersionNumber) *scdmodels.Constraint {
return &scdmodels.Constraint{
ID: vp.id,
Manager: manager,
Version: version,

StartTime: vp.uExtent.StartTime,
EndTime: vp.uExtent.EndTime,
AltitudeLower: vp.uExtent.SpatialVolume.AltitudeLo,
AltitudeUpper: vp.uExtent.SpatialVolume.AltitudeHi,

USSBaseURL: vp.ussBaseURL,
Cells: vp.cells,
}
}

// validateAndReturnConstraintUpsertParams checks that the parameters for an Constraint Reference upsert are valid.
// Note that this does NOT check for anything related to access controls: any error returned should be labeled
// as a dsserr.BadRequest.
func validateAndReturnConstraintUpsertParams(
now time.Time,
entityid restapi.EntityID,
ovn restapi.EntityOVN,
params *restapi.PutConstraintReferenceParameters,
allowHTTPBaseUrls bool,
) (*validConstraintParams, error) {
valid := &validConstraintParams{}
var err error

valid.id, err = dssmodels.IDFromString(string(entityid))
if err != nil {
return nil, stacktrace.NewError("Invalid ID format: `%s`", entityid)
}

if len(params.UssBaseUrl) == 0 {
return nil, stacktrace.NewError("Missing required UssBaseUrl")
}
valid.ussBaseURL = string(params.UssBaseUrl)

if !allowHTTPBaseUrls {
err = scdmodels.ValidateUSSBaseURL(string(params.UssBaseUrl))
if err != nil {
return nil, stacktrace.Propagate(err, "Failed to validate base URL")
}
}

// TODO: factor out logic below into common multi-vol4d parser and reuse with PutOperationReference
valid.extents = make([]*dssmodels.Volume4D, len(params.Extents))

for idx, extent := range params.Extents {
cExtent, err := dssmodels.Volume4DFromSCDRest(&extent)
if err != nil {
return nil, stacktrace.Propagate(err, "Failed to parse extent %d", idx)
}
valid.extents[idx] = cExtent
}
valid.uExtent, err = dssmodels.UnionVolumes4D(valid.extents...)
if err != nil {
return nil, stacktrace.Propagate(err, "Failed to union extents")
}

if valid.uExtent.StartTime == nil {
return nil, stacktrace.NewError("Missing time_start from extents")
}
if valid.uExtent.EndTime == nil {
return nil, stacktrace.NewError("Missing time_end from extents")
}

if now.After(*valid.uExtent.EndTime) {
return nil, stacktrace.NewError("Constraint may not end in the past")
}

if valid.uExtent.StartTime.After(*valid.uExtent.EndTime) {
return nil, stacktrace.NewError("Constraint time_end must be after time_start")
}

valid.cells, err = valid.uExtent.CalculateSpatialCovering()
if err != nil {
return nil, stacktrace.Propagate(err, "Invalid area")
}

return valid, nil
}

// QueryConstraintReferences queries existing contraint refs in the given
// bounds.
func (a *Server) QueryConstraintReferences(ctx context.Context, req *restapi.QueryConstraintReferencesRequest,
Expand Down
8 changes: 4 additions & 4 deletions pkg/scd/operational_intents_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,10 +434,10 @@ func (vp *validOIRParams) toOIR(manager dssmodels.Manager, attachedSub *scdmodel
}
}

// validateAndReturnUpsertParams checks that the parameters for an Operational Intent Reference upsert are valid.
// validateAndReturnOIRUpsertParams checks that the parameters for an Operational Intent Reference upsert are valid.
// Note that this does NOT check for anything related to access controls: any error returned should be labeled
// as a dsserr.BadRequest.
func validateAndReturnUpsertParams(
func validateAndReturnOIRUpsertParams(
now time.Time,
entityid restapi.EntityID,
ovn restapi.EntityOVN,
Expand Down Expand Up @@ -819,9 +819,9 @@ func ensureSubscriptionCoversOIR(ctx context.Context, r repos.Repository, sub *s
// If the ovn argument is empty (""), it will attempt to create a new Operational Intent.
func (a *Server) upsertOperationalIntentReference(ctx context.Context, now time.Time, authorizedManager *api.AuthorizationResult, entityid restapi.EntityID, ovn restapi.EntityOVN, params *restapi.PutOperationalIntentReferenceParameters,
) (*restapi.ChangeOperationalIntentReferenceResponse, *restapi.AirspaceConflictResponse, error) {
// Note: validateAndReturnUpsertParams and checkUpsertPermissionsAndReturnManager could be moved out of this method and only the valid params passed,
// Note: validateAndReturnOIRUpsertParams and checkUpsertPermissionsAndReturnManager could be moved out of this method and only the valid params passed,
// but this requires some changes in the caller that go beyond the immediate scope of #1088 and can be done later.
validParams, err := validateAndReturnUpsertParams(now, entityid, ovn, params, a.AllowHTTPBaseUrls)
validParams, err := validateAndReturnOIRUpsertParams(now, entityid, ovn, params, a.AllowHTTPBaseUrls)
if err != nil {
return nil, nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate Operational Intent Reference upsert parameters")
}
Expand Down
Loading