[dss] allow accepted op-intent references to not have a subscription (#…

…1005)

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")

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 (

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(),
 	}

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()

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 \