feat(cli): Print report on failure

otdfctl/cmd/migrate/namespaced_policy.go

@@ -93,6 +93,7 @@ func executeNamespacedPolicyCommit(cmd *cobra.Command, h namespacedpolicy.Execut
 	}
 
 	if err := executor.Execute(cmd.Context(), plan); err != nil {
+		writeNamespacedPolicySummary(plan, true, "failure")
 		cli.ExitWithError("could not execute namespaced-policy commit", err)
 	}
 }

otdfctl/docs/man/migrate/namespaced-policy.md

@@ -10,10 +10,14 @@ command:
       default: ''
 ---
 
+## General Information
+
 `namespaced-policy` is the migration entrypoint for moving legacy policy objects into namespaced policy.
 
 The command prints a human-readable migration summary to stdout. Dry runs show the plan summary; `--commit` shows the committed summary with created target IDs.
 
+Commit mode can partially apply changes before an error occurs. When that happens, the command prints a committed summary with `Result: failure` and per-scope `Created`, `Will Create`, and `Failed` sections so you can see what was applied and what still remains.
+
 `--scope` is required and selects any subset of `actions`, `subject-condition-sets`, `subject-mappings`, `registered-resources`, and `obligation-triggers`.
 
 The parent `migrate` command provides the shared `--commit` and `--interactive` flags.
@@ -22,9 +26,40 @@ The parent `migrate` command provides the shared `--commit` and `--interactive`
 
 All target namespaces must already exist before the command runs. Planning should fail before any writes if a required namespace is missing.
 
+## Pre-requisites
+
+1. Run at least `v0.14.0` of the OpenTDF platform before using this migration.
+
+2. Standard actions are seeded per namespace in that platform version. The migration expects those namespaced standard actions to exist when migrating references to `create`, `read`, `update`, and `delete`.
+
+## Best practices
+
+1. Before running any migration commands you should take a backup of your database to avoid any potential issues.
+
+2. Turn on the `namespaced_policy` feature flag within your deployed service yaml to avoid creating any accidental non-namespaced policy objects.
+
+3. While we allow you to perform a full migration of policy at once with the multiple scopes, we recommend that you migrate one-by-one in the following order:
+   - Actions
+   - Subject-Condition-Sets
+   - Subject-Mappings
+   - Obligation-Triggers
+   - Registered-Resources
+
+4. Use `--interactive` mode when migrating policy, this will give you the best chance of success for handling any issues that might occur. In addition, we ask for confirmation for each
+object before creating it.
+
 ## Examples
 
 ```shell
 otdfctl migrate namespaced-policy --scope=registered-resources
 otdfctl migrate namespaced-policy --scope=actions,subject-mappings,registered-resources --commit
+otdfctl migrate namespaced-policy --scope=actions,subject-mappings --interactive --commit
 ```
+
+## Other information
+
+1. For subject-condition-sets and actions, if they are not used by any other policy object they will not be migrated or considered for migration.
+2. If you provide a parent scope, the dependent scopes will also be added. For example, given scope `subject-mappings` will also add scope: `actions`, `subject-condition-sets`
+since those are required by `subject-mappings` to be migrated to a new namespace.
+3. In `--interactive --commit` mode, declining the backup confirmation or aborting review prints an `aborted` summary and exits without applying the remaining changes.
+4. In `--interactive` mode only create operations are reviewed. Existing standard objects and already migrated objects are not prompted.

otdfctl/migrations/namespacedpolicy/interactive_commit.go

@@ -35,16 +35,14 @@ const (
 	obligationValueText = "Obligation value: "
 	valuesText          = "Values: "
 	actionBindingsText  = "Action bindings: "
-	existingTargetText  = "Existing target resource: "
 	subjectSetsTextFmt  = "Subject sets: %d"
 	scsSourceText       = "Subject condition set source: "
 
-	createActionDescription            = "This will create a new namespaced action."
-	createSubjectConditionSetDesc      = "This will create a new namespaced subject condition set."
-	createSubjectMappingDescription    = "This will create a new namespaced subject mapping."
-	reuseRegisteredResourceDescription = "This will reuse the existing parent registered resource and create any missing values."
-	createRegisteredResourceDesc       = "This will create a new namespaced registered resource and its values."
-	createObligationTriggerDesc        = "This will create a new namespaced obligation trigger."
+	createActionDescription         = "This will create a new namespaced action."
+	createSubjectConditionSetDesc   = "This will create a new namespaced subject condition set."
+	createSubjectMappingDescription = "This will create a new namespaced subject mapping."
+	createRegisteredResourceDesc    = "This will create a new namespaced registered resource and its values."
+	createObligationTriggerDesc     = "This will create a new namespaced obligation trigger."
 
 	confirmMigrationLabel       = "Confirm migration"
 	confirmMigrationDescription = "apply this create operation"
@@ -358,14 +356,7 @@ func registeredResourcePrompt(plan *Plan, resourcePlan *RegisteredResourcePlan)
 		targetNamespaceText + namespaceDisplay(resourcePlan.Target.Namespace),
 		valuesText + plainRegisteredResourceValueFQNsSummary(resourcePlan),
 		actionBindingsText + plainRegisteredResourceActionBindingsSummary(plan, resourcePlan),
-	}
-	if strings.TrimSpace(resourcePlan.Target.ExistingID) != "" {
-		description = append(description,
-			existingTargetText+resourcePlan.Target.ExistingID,
-			reuseRegisteredResourceDescription,
-		)
-	} else {
-		description = append(description, createRegisteredResourceDesc)
+		createRegisteredResourceDesc,
 	}
 
 	return SelectPrompt{

otdfctl/migrations/namespacedpolicy/plan.go

@@ -128,11 +128,8 @@ type RegisteredResourcePlan struct {
 }
 
 type RegisteredResourceTargetPlan struct {
-	Namespace *policy.Namespace `json:"namespace"`
-	Status    TargetStatus      `json:"status"`
-	// For registered resources, ExistingID is also used on create targets to mean
-	// "reuse this parent RR and reconcile missing values under it" rather than
-	// creating a new top-level RR.
+	Namespace  *policy.Namespace              `json:"namespace"`
+	Status     TargetStatus                   `json:"status"`
 	ExistingID string                         `json:"existing_id,omitempty"`
 	Execution  *ExecutionResult               `json:"execution,omitempty"`
 	Reason     string                         `json:"reason,omitempty"`

otdfctl/migrations/namespacedpolicy/registered_resources_execute.go

@@ -43,8 +43,6 @@ func (e *Executor) executeRegisteredResourceTarget(ctx context.Context, plan *Re
 		return nil
 	case TargetStatusCreate:
 		return e.createRegisteredResourceTarget(ctx, plan, target)
-	case TargetStatusExistingStandard:
-		return fmt.Errorf("%w: registered resource %q target %q has unsupported status %q", ErrUnsupportedStatus, plan.Source.GetId(), namespaceLabel(target.Namespace), target.Status)
 	case TargetStatusUnresolved:
 		return nil
 	default:
@@ -58,36 +56,23 @@ func (e *Executor) createRegisteredResourceTarget(ctx context.Context, plan *Reg
 		return fmt.Errorf("%w: registered resource %q", ErrTargetNamespaceRequired, plan.Source.GetId())
 	}
 
-	// Create the parent RR only when the plan did not already select an existing
-	// target RR to reuse for this namespace.
-	created, hasExistingParent, err := e.existingRegisteredResource(ctx, target)
+	created, err := e.handler.CreateRegisteredResource(
+		ctx,
+		namespace,
+		plan.Source.GetName(),
+		nil,
+		metadataForCreate(
+			plan.Source.GetId(),
+			metadataLabels(plan.Source.GetMetadata()),
+			e.runID,
+		),
+	)
 	if err != nil {
 		target.Execution = &ExecutionResult{
 			RunID:   e.runID,
 			Failure: err.Error(),
 		}
-		return fmt.Errorf("load registered resource %q target %q: %w", plan.Source.GetId(), namespaceLabel(target.Namespace), err)
-	}
-	if !hasExistingParent {
-		var err error
-		created, err = e.handler.CreateRegisteredResource(
-			ctx,
-			namespace,
-			plan.Source.GetName(),
-			nil,
-			metadataForCreate(
-				plan.Source.GetId(),
-				metadataLabels(plan.Source.GetMetadata()),
-				e.runID,
-			),
-		)
-		if err != nil {
-			target.Execution = &ExecutionResult{
-				RunID:   e.runID,
-				Failure: err.Error(),
-			}
-			return fmt.Errorf("%w: create registered resource %q in namespace %q", err, plan.Source.GetId(), namespaceLabel(target.Namespace))
-		}
+		return fmt.Errorf("%w: create registered resource %q in namespace %q", err, plan.Source.GetId(), namespaceLabel(target.Namespace))
 	}
 	if created == nil {
 		target.Execution = &ExecutionResult{
@@ -135,18 +120,6 @@ func (e *Executor) createRegisteredResourceTarget(ctx context.Context, plan *Reg
 	return nil
 }
 
-func (e *Executor) existingRegisteredResource(ctx context.Context, target *RegisteredResourceTargetPlan) (*policy.RegisteredResource, bool, error) {
-	if target == nil || target.ExistingID == "" {
-		return nil, false, nil
-	}
-
-	resource, err := e.handler.GetRegisteredResource(ctx, target.ExistingID, "", "")
-	if err != nil {
-		return nil, true, err
-	}
-	return resource, true, nil
-}
-
 func (e *Executor) createRegisteredResourceValue(ctx context.Context, target *RegisteredResourceTargetPlan, valuePlan *RegisteredResourceValuePlan) error {
 	actionAttributeValues, err := e.registeredResourceActionAttributeValues(target.Namespace, valuePlan)
 	if err != nil {

otdfctl/migrations/namespacedpolicy/registered_resources_execute_test.go

@@ -271,99 +271,6 @@ func TestExecuteRegisteredResources(t *testing.T) {
 				assert.Nil(t, handler.createdRegisteredResourceValues)
 			},
 		},
-		{
-			name: "reuses existing registered resource parent on create targets",
-			plan: &Plan{
-				Scopes: []Scope{ScopeActions, ScopeRegisteredResources},
-				Actions: []*ActionPlan{
-					{
-						Source: &policy.Action{Id: "action-1", Name: "read"},
-						Targets: []*ActionTargetPlan{
-							{
-								Namespace: namespace1,
-								Status:    TargetStatusCreate,
-							},
-						},
-					},
-				},
-				RegisteredResources: []*RegisteredResourcePlan{
-					{
-						Source: &policy.RegisteredResource{Id: "rr-1", Name: "repo"},
-						Target: &RegisteredResourceTargetPlan{
-							Namespace:  namespace1,
-							Status:     TargetStatusCreate,
-							ExistingID: "existing-rr-1",
-							Values: []*RegisteredResourceValuePlan{
-								{
-									Source: &policy.RegisteredResourceValue{Id: "rrv-1", Value: "repo-a"},
-									ActionBindings: []*RegisteredResourceActionBinding{
-										{
-											SourceActionID: "action-1",
-											AttributeValue: &policy.Value{Id: "attribute-value-id-1"},
-										},
-									},
-								},
-								{
-									Source: &policy.RegisteredResourceValue{Id: "rrv-2", Value: "repo-b"},
-									ActionBindings: []*RegisteredResourceActionBinding{
-										{
-											SourceActionID: "action-1",
-											AttributeValue: &policy.Value{Id: "attribute-value-id-2"},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			handler: &mockExecutorHandler{
-				results: map[string]map[string]*policy.Action{
-					"read": {
-						"ns-1": {Id: "created-action-1", Name: "read"},
-					},
-				},
-				registeredResourceValueResult: map[string]map[string]*policy.RegisteredResourceValue{
-					"rrv-2": {
-						"existing-rr-1": {Id: "created-rrv-2", Value: "repo-b"},
-					},
-				},
-				registeredResourcesByID: map[string]*policy.RegisteredResource{
-					"existing-rr-1": {
-						Id:   "existing-rr-1",
-						Name: "repo",
-						Values: []*policy.RegisteredResourceValue{
-							{Id: "existing-rrv-1", Value: "repo-a"},
-						},
-					},
-				},
-			},
-			assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) {
-				t.Helper()
-
-				require.NoError(t, err)
-				assert.Nil(t, handler.createdRegisteredResources)
-				require.Contains(t, handler.createdRegisteredResourceValues, "rrv-2")
-				require.Contains(t, handler.createdRegisteredResourceValues["rrv-2"], "existing-rr-1")
-				assert.NotContains(t, handler.createdRegisteredResourceValues, "rrv-1")
-
-				target := plan.RegisteredResources[0].Target
-				require.NotNil(t, target.Execution)
-				assert.True(t, target.Execution.Applied)
-				assert.Equal(t, "existing-rr-1", target.Execution.CreatedTargetID)
-				assert.Equal(t, "existing-rr-1", target.TargetID())
-
-				existingValue := target.Values[0]
-				require.NotNil(t, existingValue.Execution)
-				assert.True(t, existingValue.Execution.Applied)
-				assert.Equal(t, "existing-rrv-1", existingValue.Execution.CreatedTargetID)
-
-				createdValue := target.Values[1]
-				require.NotNil(t, createdValue.Execution)
-				assert.True(t, createdValue.Execution.Applied)
-				assert.Equal(t, "created-rrv-2", createdValue.Execution.CreatedTargetID)
-			},
-		},
 		{
 			name: "records shell creation failures on the target",
 			plan: &Plan{