From 123a501f802c54066e8242fb88175c2941428bf2 Mon Sep 17 00:00:00 2001 From: Julien Perrochet Date: Fri, 15 Mar 2024 08:30:02 +0100 Subject: [PATCH] [dss] allow accepted op-intent references to not have a subscription (#1005) --- pkg/models/models.go | 5 + pkg/scd/models/models.go | 7 ++ pkg/scd/models/operational_intents.go | 14 ++- pkg/scd/operational_intents_handler.go | 117 +++++++++++-------- test/migrations/rid_db_post_migration_e2e.sh | 2 +- 5 files changed, 95 insertions(+), 50 deletions(-) diff --git a/pkg/models/models.go b/pkg/models/models.go index 1d07bbd7b..7d9c1da2c 100644 --- a/pkg/models/models.go +++ b/pkg/models/models.go @@ -38,8 +38,13 @@ const ( MaxResultLimit = 10000 ) +// PgUUID converts an ID to a pgtype.UUID. +// If the ID this is called on is nil, nil will be returned func (id *ID) PgUUID() (*pgtype.UUID, error) { var pgUUID pgtype.UUID + if id == nil { + return nil, nil + } err := pgUUID.Set(id.String()) if err != nil { return nil, stacktrace.Propagate(err, "Error converting ID to PgUUID format") diff --git a/pkg/scd/models/models.go b/pkg/scd/models/models.go index b18493ca8..e9d188961 100644 --- a/pkg/scd/models/models.go +++ b/pkg/scd/models/models.go @@ -7,12 +7,19 @@ import ( "strings" "time" + restapi "github.com/interuss/dss/pkg/api/scdv1" "github.com/interuss/stacktrace" ) const ( // Value for OVN that should be returned for entities not owned by the client NoOvnPhrase = "Available from USS" + + // NullV4UUID is a valid V4 UUID to be used as a placeholder where no UUID is available + // but one needs to be specified, as in certain API return values. + // Note that this UUID is not meant to be persisted to the database: it should only be used + // to populate required API fields for which a proper value does not exist. + NullV4UUID = restapi.SubscriptionID("00000000-0000-4000-8000-000000000000") ) type ( diff --git a/pkg/scd/models/operational_intents.go b/pkg/scd/models/operational_intents.go index b14b137e4..45771d1b3 100644 --- a/pkg/scd/models/operational_intents.go +++ b/pkg/scd/models/operational_intents.go @@ -22,6 +22,12 @@ const ( // OperationState models the state of an operation. type OperationalIntentState string +// RequiresSubscription indicates whether transitioning an OperationalIntent +// to this state requires a subscription. +func (s OperationalIntentState) RequiresSubscription() bool { + return s != OperationalIntentStateAccepted +} + // RequiresKey indicates whether transitioning an OperationalIntent to this // OperationalIntentState requires a valid key. func (s OperationalIntentState) RequiresKey() bool { @@ -62,7 +68,7 @@ type OperationalIntent struct { StartTime *time.Time EndTime *time.Time USSBaseURL string - SubscriptionID dssmodels.ID + SubscriptionID *dssmodels.ID AltitudeLower *float32 AltitudeUpper *float32 Cells s2.CellUnion @@ -79,13 +85,17 @@ func (s OperationalIntentState) ToRest() restapi.OperationalIntentState { // ToRest converts the OperationalIntent to its SCD v1 REST model API format func (o *OperationalIntent) ToRest() *restapi.OperationalIntentReference { ovn := restapi.EntityOVN(o.OVN.String()) + subID := NullV4UUID + if o.SubscriptionID != nil { + subID = restapi.SubscriptionID(o.SubscriptionID.String()) + } result := &restapi.OperationalIntentReference{ Id: restapi.EntityID(o.ID.String()), Ovn: &ovn, Manager: o.Manager.String(), Version: int32(o.Version), UssBaseUrl: restapi.OperationalIntentUssBaseURL(o.USSBaseURL), - SubscriptionId: restapi.SubscriptionID(o.SubscriptionID.String()), + SubscriptionId: subID, State: o.State.ToRest(), UssAvailability: o.UssAvailability.ToRest(), } diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go index 80c6cd24b..f00c1e72c 100644 --- a/pkg/scd/operational_intents_handler.go +++ b/pkg/scd/operational_intents_handler.go @@ -55,26 +55,29 @@ func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *rest "OperationalIntent owned by %s, but %s attempted to delete", old.Manager, *req.Auth.ClientID) } - // Get the Subscription supporting the OperationalIntent - sub, err := r.GetSubscription(ctx, old.SubscriptionID) - if err != nil { - return stacktrace.Propagate(err, "Unable to get OperationalIntent's Subscription from repo") - } - if sub == nil { - return stacktrace.NewError("OperationalIntent's Subscription missing from repo") - } - + // Get the Subscription supporting the OperationalIntent, if one is defined + var sub *scdmodels.Subscription removeImplicitSubscription := false - if sub.ImplicitSubscription { - // Get the Subscription's dependent OperationalIntents - dependentOps, err := r.GetDependentOperationalIntents(ctx, sub.ID) + if old.SubscriptionID != nil { + sub, err = r.GetSubscription(ctx, *old.SubscriptionID) if err != nil { - return stacktrace.Propagate(err, "Could not find dependent OperationalIntents") + return stacktrace.Propagate(err, "Unable to get OperationalIntent's Subscription from repo") + } + if sub == nil { + return stacktrace.NewError("OperationalIntent's Subscription missing from repo") } - if len(dependentOps) == 0 { - return stacktrace.NewError("An implicit Subscription had no dependent OperationalIntents") - } else if len(dependentOps) == 1 { - removeImplicitSubscription = true + + if sub.ImplicitSubscription { + // Get the Subscription's dependent OperationalIntents + dependentOps, err := r.GetDependentOperationalIntents(ctx, sub.ID) + if err != nil { + return stacktrace.Propagate(err, "Could not find dependent OperationalIntents") + } + if len(dependentOps) == 0 { + return stacktrace.NewError("An implicit Subscription had no dependent OperationalIntents") + } else if len(dependentOps) == 1 { + removeImplicitSubscription = true + } } } @@ -111,6 +114,7 @@ func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *rest return stacktrace.Propagate(err, "Unable to delete OperationalIntent from repo") } + // removeImplicitSubscription is only true if the OIR had a subscription defined if removeImplicitSubscription { // Automatically remove a now-unused implicit Subscription err = r.DeleteSubscription(ctx, sub.ID) @@ -291,7 +295,7 @@ func (a *Server) CreateOperationalIntentReference(ctx context.Context, req *rest respOK, respConflict, err := a.PutOperationalIntentReference(ctx, *req.Auth.ClientID, req.Entityid, "", req.Body) if err != nil { - err = stacktrace.Propagate(err, "Could not put subscription") + err = stacktrace.Propagate(err, "Could not put Operational Intent Reference") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} switch stacktrace.GetCode(err) { case dsserr.PermissionDenied: @@ -425,6 +429,15 @@ func (a *Server) PutOperationalIntentReference(ctx context.Context, manager stri } } + // Check if a subscription is required for this request: + // OIRs in an accepted state do not need a subscription. + if state.RequiresSubscription() && + subscriptionID.Empty() && + (params.NewSubscription == nil || + params.NewSubscription.UssBaseUrl == "") { + return nil, nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Provided Operational Intent Reference state `%s` requires either a subscription ID or information to create an implicit subscription", state) + } + var responseOK *restapi.ChangeOperationalIntentReferenceResponse var responseConflict *restapi.AirspaceConflictResponse action := func(ctx context.Context, r repos.Repository) (err error) { @@ -463,36 +476,38 @@ func (a *Server) PutOperationalIntentReference(ctx context.Context, manager stri var sub *scdmodels.Subscription if subscriptionID.Empty() { - // Create implicit Subscription - if params.NewSubscription == nil || params.NewSubscription.UssBaseUrl == "" { - return stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing new_subscription or uss_base_url in new_subscription") - } - if !a.EnableHTTP { - err := scdmodels.ValidateUSSBaseURL(string(params.NewSubscription.UssBaseUrl)) - if err != nil { - return stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate USS base URL") + // Create an implicit subscription if the implicit subscription params are set: + // for situations where these params are required but have not been set, + // an error will have been returned earlier. + // If they are not set at this point, continue without creating an implicit subscription. + if params.NewSubscription != nil && params.NewSubscription.UssBaseUrl != "" { + if !a.EnableHTTP { + err := scdmodels.ValidateUSSBaseURL(string(params.NewSubscription.UssBaseUrl)) + if err != nil { + return stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate USS base URL") + } } - } - subToUpsert := scdmodels.Subscription{ - ID: dssmodels.ID(uuid.New().String()), - Manager: dssmodels.Manager(manager), - StartTime: uExtent.StartTime, - EndTime: uExtent.EndTime, - AltitudeLo: uExtent.SpatialVolume.AltitudeLo, - AltitudeHi: uExtent.SpatialVolume.AltitudeHi, - Cells: cells, - USSBaseURL: string(params.NewSubscription.UssBaseUrl), - NotifyForOperationalIntents: true, - ImplicitSubscription: true, - } - if params.NewSubscription.NotifyForConstraints != nil { - subToUpsert.NotifyForConstraints = *params.NewSubscription.NotifyForConstraints - } + subToUpsert := scdmodels.Subscription{ + ID: dssmodels.ID(uuid.New().String()), + Manager: dssmodels.Manager(manager), + StartTime: uExtent.StartTime, + EndTime: uExtent.EndTime, + AltitudeLo: uExtent.SpatialVolume.AltitudeLo, + AltitudeHi: uExtent.SpatialVolume.AltitudeHi, + Cells: cells, + USSBaseURL: string(params.NewSubscription.UssBaseUrl), + NotifyForOperationalIntents: true, + ImplicitSubscription: true, + } + if params.NewSubscription.NotifyForConstraints != nil { + subToUpsert.NotifyForConstraints = *params.NewSubscription.NotifyForConstraints + } - sub, err = r.UpsertSubscription(ctx, &subToUpsert) - if err != nil { - return stacktrace.Propagate(err, "Failed to create implicit subscription") + sub, err = r.UpsertSubscription(ctx, &subToUpsert) + if err != nil { + return stacktrace.Propagate(err, "Failed to create implicit subscription") + } } } else { @@ -570,7 +585,7 @@ func (a *Server) PutOperationalIntentReference(ctx context.Context, manager stri // Identify Constraints missing from the key var missingConstraints []*scdmodels.Constraint - if sub.NotifyForConstraints { + if sub != nil && sub.NotifyForConstraints { constraints, err := r.SearchConstraints(ctx, uExtent) if err != nil { return stacktrace.Propagate(err, "Unable to SearchConstraints") @@ -609,6 +624,14 @@ func (a *Server) PutOperationalIntentReference(ctx context.Context, manager stri } } + // For OIR's in the accepted state, we may not have a subscription available, + // in such cases the subscription ID on scdmodels.OperationalIntent will be nil + // and will be replaced with the 'NullV4UUID' when sent over to a client. + var subID *dssmodels.ID = nil + if sub != nil { + subID = &sub.ID + } + // Construct the new OperationalIntent op := &scdmodels.OperationalIntent{ ID: id, @@ -622,7 +645,7 @@ func (a *Server) PutOperationalIntentReference(ctx context.Context, manager stri Cells: cells, USSBaseURL: string(params.UssBaseUrl), - SubscriptionID: sub.ID, + SubscriptionID: subID, State: state, } err = op.ValidateTimeRange() diff --git a/test/migrations/rid_db_post_migration_e2e.sh b/test/migrations/rid_db_post_migration_e2e.sh index e50204956..1a3039726 100755 --- a/test/migrations/rid_db_post_migration_e2e.sh +++ b/test/migrations/rid_db_post_migration_e2e.sh @@ -82,7 +82,7 @@ docker run --link dummy-oauth-for-testing:oauth \ --link core-service-for-testing:core-service \ -v "${RESULTFILE}:/app/test_result" \ -w /app/monitoring/prober \ - interuss/monitoring:v0.3.0 \ + interuss/monitoring:v0.5.0 \ pytest \ "${1:-.}" \ -rsx \