From b987f5e7cff3f7a5634e34806788dbeec95fd70d Mon Sep 17 00:00:00 2001 From: Michel Davit Date: Tue, 24 Feb 2026 11:47:02 +0100 Subject: [PATCH] [scd] Align constraint and op-intent validation --- pkg/scd/constraints_handler.go | 168 +++++++++++++++---------- pkg/scd/operational_intents_handler.go | 8 +- 2 files changed, 108 insertions(+), 68 deletions(-) diff --git a/pkg/scd/constraints_handler.go b/pkg/scd/constraints_handler.go index dab14fbc4..203f04a66 100644 --- a/pkg/scd/constraints_handler.go +++ b/pkg/scd/constraints_handler.go @@ -2,6 +2,7 @@ package scd import ( "context" + "time" "github.com/golang/geo/s2" "github.com/interuss/dss/pkg/api" @@ -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") } @@ -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, @@ -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 } @@ -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, diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go index d0b416689..be2aeb4e0 100644 --- a/pkg/scd/operational_intents_handler.go +++ b/pkg/scd/operational_intents_handler.go @@ -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, @@ -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") }