diff --git a/otdfctl/cmd/migrate/namespaced_policy.go b/otdfctl/cmd/migrate/namespaced_policy.go index efec1c66ed..1a852ba855 100644 --- a/otdfctl/cmd/migrate/namespaced_policy.go +++ b/otdfctl/cmd/migrate/namespaced_policy.go @@ -13,7 +13,6 @@ import ( func migrateNamespacedPolicyCmd() *cobra.Command { doc := man.Docs.GetCommand("migrate/namespaced-policy", man.WithRun(migrateNamespacedPolicy)) doc.Args = cobra.NoArgs - doc.Hidden = true doc.Flags().StringP( doc.GetDocFlag("scope").Name, doc.GetDocFlag("scope").Shorthand, @@ -46,7 +45,7 @@ func migrateNamespacedPolicy(cmd *cobra.Command, args []string) { plannerOpts = append(plannerOpts, namespacedpolicy.WithInteractiveReviewer(namespacedpolicy.NewHuhInteractiveReviewer(&h, prompter))) } - planner, err := namespacedpolicy.NewPlanner(&h, scopeCSV, plannerOpts...) + planner, err := namespacedpolicy.NewMigrationPlanner(&h, scopeCSV, plannerOpts...) if err != nil { cli.ExitWithError("could not create namespaced-policy planner", err) } @@ -65,20 +64,20 @@ func migrateNamespacedPolicy(cmd *cobra.Command, args []string) { } } -func confirmNamespacedPolicyCommit(cmd *cobra.Command, plan *namespacedpolicy.Plan, interactive bool, prompter namespacedpolicy.InteractivePrompter) error { +func confirmNamespacedPolicyCommit(cmd *cobra.Command, plan *namespacedpolicy.MigrationPlan, interactive bool, prompter namespacedpolicy.InteractivePrompter) error { if !interactive { return nil } if err := namespacedpolicy.ConfirmNamespacedPolicyBackup(cmd.Context(), prompter); err != nil { return err } - if err := namespacedpolicy.ReviewNamespacedPolicyInteractiveCommit(cmd.Context(), plan, prompter); err != nil { + if err := namespacedpolicy.ConfirmMigrationPlan(cmd.Context(), plan, prompter); err != nil { return err } return nil } -func executeNamespacedPolicyCommit(cmd *cobra.Command, h namespacedpolicy.ExecutorHandler, plan *namespacedpolicy.Plan, interactive bool, prompter namespacedpolicy.InteractivePrompter) { +func executeNamespacedPolicyCommit(cmd *cobra.Command, h namespacedpolicy.ExecutorHandler, plan *namespacedpolicy.MigrationPlan, interactive bool, prompter namespacedpolicy.InteractivePrompter) { if err := confirmNamespacedPolicyCommit(cmd, plan, interactive, prompter); err != nil { if errors.Is(err, namespacedpolicy.ErrNamespacedPolicyBackupNotConfirmed) || errors.Is(err, namespacedpolicy.ErrInteractiveReviewAborted) { writeNamespacedPolicySummary(cmd, plan, false, "aborted") @@ -86,18 +85,18 @@ func executeNamespacedPolicyCommit(cmd *cobra.Command, h namespacedpolicy.Execut cli.ExitWithError("could not review namespaced-policy commit", err) } - executor, err := namespacedpolicy.NewExecutor(h) + executor, err := namespacedpolicy.NewMigrationExecutor(h) if err != nil { cli.ExitWithError("could not create namespaced-policy executor", err) } - if err := executor.Execute(cmd.Context(), plan); err != nil { + if err := executor.ExecuteMigration(cmd.Context(), plan); err != nil { writeNamespacedPolicySummary(cmd, plan, true, "failure") cli.ExitWithError("could not execute namespaced-policy commit", err) } } -func writeNamespacedPolicySummary(cmd *cobra.Command, plan *namespacedpolicy.Plan, commit bool, result string) { +func writeNamespacedPolicySummary(cmd *cobra.Command, plan *namespacedpolicy.MigrationPlan, commit bool, result string) { if _, err := cmd.OutOrStdout().Write([]byte(namespacedpolicy.RenderNamespacedPolicySummaryWithResult(plan, commit, result) + "\n")); err != nil { cli.ExitWithError("could not write namespaced-policy summary", err) } diff --git a/otdfctl/cmd/migrate/prune/namespaced_policy.go b/otdfctl/cmd/migrate/prune/namespaced_policy.go index a543a3b584..086483abdc 100644 --- a/otdfctl/cmd/migrate/prune/namespaced_policy.go +++ b/otdfctl/cmd/migrate/prune/namespaced_policy.go @@ -13,7 +13,6 @@ import ( func pruneNamespacedPolicyCmd() *cobra.Command { doc := man.Docs.GetCommand("migrate/prune/namespaced-policy", man.WithRun(pruneNamespacedPolicy)) doc.Args = cobra.NoArgs - doc.Hidden = true doc.Flags().StringP( doc.GetDocFlag("scope").Name, doc.GetDocFlag("scope").Shorthand, @@ -79,7 +78,7 @@ func executeNamespacedPolicyPruneCommit(cmd *cobra.Command, h namespacedpolicy.E } } - executor, err := namespacedpolicy.NewExecutor(h) + executor, err := namespacedpolicy.NewPruneExecutor(h) if err != nil { cli.ExitWithError("could not create namespaced-policy prune executor", err) } diff --git a/otdfctl/cmd/migrate/prune/prune.go b/otdfctl/cmd/migrate/prune/prune.go index be2b45dc74..e5214f821e 100644 --- a/otdfctl/cmd/migrate/prune/prune.go +++ b/otdfctl/cmd/migrate/prune/prune.go @@ -11,6 +11,5 @@ var ( ) func InitCommands() { - Cmd.Hidden = true Cmd.AddCommand(pruneNamespacedPolicyCmd()) } diff --git a/otdfctl/docs/man/migrate/_index.md b/otdfctl/docs/man/migrate/_index.md index 6d93047e98..273ddb080c 100644 --- a/otdfctl/docs/man/migrate/_index.md +++ b/otdfctl/docs/man/migrate/_index.md @@ -17,20 +17,11 @@ command: default: false --- -`migrate` groups commands used to migrate policy resources and related state. +`migrate` groups migration and migration-related cleanup workflows. -The end-to-end workflow is not implemented yet, but the command surface is in place. +Use this command family when you want to inspect a migration plan, review changes interactively, or apply migration-related updates. -Available subcommands currently include `namespaced-policy` for migration planning and execution, and `prune` for cleanup flows. +The parent `migrate` command owns flags shared by its subcommands: -The parent `migrate` command owns the shared `--commit` and `--interactive` flags. - -`migrate prune` is separate from the existing destructive `otdfctl policy subject-condition-sets prune` command. - -## Planned examples - -```shell -otdfctl migrate namespaced-policy --scope=registered-resources --output=policy-migration.json -otdfctl migrate prune namespaced-policy --scope=registered-resources -otdfctl migrate namespaced-policy --scope=actions,subject-mappings,registered-resources --output=policy-migration.json --commit -``` +- `--commit`, `-c`: apply the planned changes instead of only rendering the plan +- `--interactive`, `-i`: walk through the plan interactively before continuing diff --git a/otdfctl/e2e/migrate-namespaced-policy.bats b/otdfctl/e2e/migrate-namespaced-policy.bats index 858310f27a..78e907a4de 100644 --- a/otdfctl/e2e/migrate-namespaced-policy.bats +++ b/otdfctl/e2e/migrate-namespaced-policy.bats @@ -632,7 +632,6 @@ assert_subject_mapping_created_in_namespace() { assert_equal "$(echo "$created_mapping_json" | jq -r '.subject_condition_set.id')" "$expected_scs_target_id" assert_metadata_labels_preserved "$source_mapping_json" "$created_mapping_json" assert_equal "$(echo "$created_mapping_json" | jq -r '.metadata.labels.migrated_from')" "$source_mapping_id" - assert_not_equal "$(echo "$created_mapping_json" | jq -r '.metadata.labels.migration_run // empty')" "" } assert_subject_mapping_already_migrated_in_namespace() { @@ -748,7 +747,7 @@ assert_metadata_labels_preserved() { assert_not_equal "$source_labels" "{}" local target_labels - target_labels=$(echo "$target_json" | jq -c '(.metadata.labels // {}) | del(.migrated_from, .migration_run)') + target_labels=$(echo "$target_json" | jq -c '(.metadata.labels // {}) | del(.migrated_from)') assert_equal "$target_labels" "$source_labels" } @@ -971,7 +970,6 @@ assert_custom_action_created_in_namespace() { assert_equal "$(echo "$created_action_json" | jq -r '.namespace.id')" "$namespace_id" assert_metadata_labels_preserved "$source_action_json" "$created_action_json" assert_equal "$(echo "$created_action_json" | jq -r '.metadata.labels.migrated_from')" "$source_action_id" - assert_not_equal "$(echo "$created_action_json" | jq -r '.metadata.labels.migration_run // empty')" "" } assert_legacy_custom_action_still_exists() { @@ -1018,7 +1016,6 @@ assert_scs_created_in_namespace() { assert_equal "$(subject_sets_signature "$created_scs_json")" "$(subject_sets_signature "$source_scs_json")" assert_metadata_labels_preserved "$source_scs_json" "$created_scs_json" assert_equal "$(echo "$created_scs_json" | jq -r '.metadata.labels.migrated_from')" "$source_scs_id" - assert_not_equal "$(echo "$created_scs_json" | jq -r '.metadata.labels.migration_run // empty')" "" } assert_registered_resource_created_in_namespace() { @@ -1064,7 +1061,6 @@ assert_registered_resource_created_in_namespace() { assert_equal "$(echo "$created_resource_json" | jq -r '.namespace.id')" "$namespace_id" assert_metadata_labels_preserved "$source_resource_json" "$created_resource_json" assert_equal "$(echo "$created_resource_json" | jq -r '.metadata.labels.migrated_from')" "$source_resource_id" - assert_not_equal "$(echo "$created_resource_json" | jq -r '.metadata.labels.migration_run // empty')" "" local source_resource_value_json run_otdfctl_registered_resource_values get --id "$source_value_id" --json @@ -1085,7 +1081,6 @@ assert_registered_resource_created_in_namespace() { assert_equal "$(echo "$created_resource_value_json" | jq -r '.action_attribute_values[0].attribute_value.id')" "$attribute_value_id" assert_metadata_labels_preserved "$source_resource_value_json" "$created_resource_value_json" assert_equal "$(echo "$created_resource_value_json" | jq -r '.metadata.labels.migrated_from')" "$source_value_id" - assert_not_equal "$(echo "$created_resource_value_json" | jq -r '.metadata.labels.migration_run // empty')" "" } assert_registered_resource_already_migrated_in_namespace() { @@ -1251,7 +1246,6 @@ assert_obligation_trigger_created_in_namespace() { assert_equal "$(echo "$created_trigger_json" | jq -r '.context[0].pep.client_id')" "$client_id" assert_metadata_labels_preserved "$source_trigger_json" "$created_trigger_json" assert_equal "$(echo "$created_trigger_json" | jq -r '.metadata.labels.migrated_from')" "$source_trigger_id" - assert_not_equal "$(echo "$created_trigger_json" | jq -r '.metadata.labels.migration_run // empty')" "" } assert_obligation_trigger_already_migrated_in_namespace() { @@ -2122,7 +2116,7 @@ teardown_file() { local ns_a_state_after create_global_action action_id "$action_name" --label "test_case=prune-empty-scope" --label "fixture=${TEST_PREFIX}-source" - create_namespaced_action action_target_id "$NS_A_ID" "$action_name" --label "test_case=prune-empty-scope" --label "fixture=${TEST_PREFIX}-target" --label "migrated_from=$action_id" --label "migration_run=${TEST_PREFIX}-manual" + create_namespaced_action action_target_id "$NS_A_ID" "$action_name" --label "test_case=prune-empty-scope" --label "fixture=${TEST_PREFIX}-target" --label "migrated_from=$action_id" ns_a_state_before=$(namespace_state_json "$NS_A_ID") @@ -2148,7 +2142,7 @@ teardown_file() { local ns_a_state_after create_global_action action_id "$action_name" --label "test_case=prune-invalid-scope" --label "fixture=${TEST_PREFIX}-source" - create_namespaced_action action_target_id "$NS_A_ID" "$action_name" --label "test_case=prune-invalid-scope" --label "fixture=${TEST_PREFIX}-target" --label "migrated_from=$action_id" --label "migration_run=${TEST_PREFIX}-manual" + create_namespaced_action action_target_id "$NS_A_ID" "$action_name" --label "test_case=prune-invalid-scope" --label "fixture=${TEST_PREFIX}-target" --label "migrated_from=$action_id" ns_a_state_before=$(namespace_state_json "$NS_A_ID") @@ -2174,7 +2168,7 @@ teardown_file() { local ns_a_state_after create_global_action action_id "$action_name" --label "test_case=prune-multiple-scopes" --label "fixture=${TEST_PREFIX}-source" - create_namespaced_action action_target_id "$NS_A_ID" "$action_name" --label "test_case=prune-multiple-scopes" --label "fixture=${TEST_PREFIX}-target" --label "migrated_from=$action_id" --label "migration_run=${TEST_PREFIX}-manual" + create_namespaced_action action_target_id "$NS_A_ID" "$action_name" --label "test_case=prune-multiple-scopes" --label "fixture=${TEST_PREFIX}-target" --label "migrated_from=$action_id" ns_a_state_before=$(namespace_state_json "$NS_A_ID") @@ -2230,8 +2224,8 @@ teardown_file() { create_global_action used_by_trigger_id "$used_by_trigger_name" --label "test_case=prune-actions" --label "fixture=${TEST_PREFIX}-used-by-trigger-source" create_global_action not_migrated_id "$not_migrated_name" --label "test_case=prune-actions" --label "fixture=${TEST_PREFIX}-not-migrated-source" - create_namespaced_action delete_a_target_id "$NS_A_ID" "$delete_a_name" --label "test_case=prune-actions" --label "fixture=${TEST_PREFIX}-delete-a-target" --label "migrated_from=$delete_a_id" --label "migration_run=${TEST_PREFIX}-manual" - create_namespaced_action delete_b_target_id "$NS_A_ID" "$delete_b_name" --label "test_case=prune-actions" --label "fixture=${TEST_PREFIX}-delete-b-target" --label "migrated_from=$delete_b_id" --label "migration_run=${TEST_PREFIX}-manual" + create_namespaced_action delete_a_target_id "$NS_A_ID" "$delete_a_name" --label "test_case=prune-actions" --label "fixture=${TEST_PREFIX}-delete-a-target" --label "migrated_from=$delete_a_id" + create_namespaced_action delete_b_target_id "$NS_A_ID" "$delete_b_name" --label "test_case=prune-actions" --label "fixture=${TEST_PREFIX}-delete-b-target" --label "migrated_from=$delete_b_id" create_global_scs shared_scs_id "$shared_scs" --label "test_case=prune-actions" --label "fixture=${TEST_PREFIX}-shared-scs" create_legacy_subject_mapping mapping_id "$ATTR_A_VAL_1_ID" "$used_by_mapping_id" "$shared_scs_id" --label "test_case=prune-actions" --label "fixture=${TEST_PREFIX}-mapping-reference" @@ -2310,9 +2304,9 @@ teardown_file() { create_global_scs not_migrated_id "$not_migrated_sets" --label "test_case=prune-scs" --label "fixture=${TEST_PREFIX}-not-migrated-source" create_global_scs unlabeled_id "$unlabeled_target_sets" --label "test_case=prune-scs" --label "fixture=${TEST_PREFIX}-unlabeled-target-source" - create_namespaced_scs delete_a_target_id "$NS_A_ID" "$delete_a_sets" --label "test_case=prune-scs" --label "fixture=${TEST_PREFIX}-delete-a-target" --label "migrated_from=$delete_a_id" --label "migration_run=${TEST_PREFIX}-manual" - create_namespaced_scs delete_b_target_id "$NS_A_ID" "$delete_b_sets" --label "test_case=prune-scs" --label "fixture=${TEST_PREFIX}-delete-b-target" --label "migrated_from=$delete_b_id" --label "migration_run=${TEST_PREFIX}-manual" - create_namespaced_scs used_target_id "$NS_A_ID" "$used_sets" --label "test_case=prune-scs" --label "fixture=${TEST_PREFIX}-used-target" --label "migrated_from=$used_id" --label "migration_run=${TEST_PREFIX}-manual" + create_namespaced_scs delete_a_target_id "$NS_A_ID" "$delete_a_sets" --label "test_case=prune-scs" --label "fixture=${TEST_PREFIX}-delete-a-target" --label "migrated_from=$delete_a_id" + create_namespaced_scs delete_b_target_id "$NS_A_ID" "$delete_b_sets" --label "test_case=prune-scs" --label "fixture=${TEST_PREFIX}-delete-b-target" --label "migrated_from=$delete_b_id" + create_namespaced_scs used_target_id "$NS_A_ID" "$used_sets" --label "test_case=prune-scs" --label "fixture=${TEST_PREFIX}-used-target" --label "migrated_from=$used_id" create_namespaced_scs unlabeled_target_id "$NS_A_ID" "$unlabeled_target_sets" --label "test_case=prune-scs" --label "fixture=${TEST_PREFIX}-unlabeled-target" create_global_action action_id "${TEST_PREFIX}-prune-scs-action" --label "test_case=prune-scs" --label "fixture=${TEST_PREFIX}-action-reference" @@ -2421,8 +2415,8 @@ teardown_file() { create_global_action unlabeled_global_action_id "$unlabeled_global_action_name" --label "test_case=prune-subject-mappings" --label "fixture=${TEST_PREFIX}-unlabeled-target-action" create_global_scs unlabeled_global_scs_id "$unlabeled_global_sets" --label "test_case=prune-subject-mappings" --label "fixture=${TEST_PREFIX}-unlabeled-target-scs" create_legacy_subject_mapping unlabeled_global_mapping_id "$ATTR_A_VAL_2_ID" "$unlabeled_global_action_id" "$unlabeled_global_scs_id" --label "test_case=prune-subject-mappings" --label "fixture=${TEST_PREFIX}-unlabeled-target-mapping" - create_namespaced_action unlabeled_action_target_id "$NS_A_ID" "$unlabeled_global_action_name" --label "test_case=prune-subject-mappings" --label "fixture=${TEST_PREFIX}-unlabeled-target-action-target" --label "migrated_from=$unlabeled_global_action_id" --label "migration_run=${TEST_PREFIX}-manual" - create_namespaced_scs unlabeled_scs_target_id "$NS_A_ID" "$unlabeled_global_sets" --label "test_case=prune-subject-mappings" --label "fixture=${TEST_PREFIX}-unlabeled-target-scs-target" --label "migrated_from=$unlabeled_global_scs_id" --label "migration_run=${TEST_PREFIX}-manual" + create_namespaced_action unlabeled_action_target_id "$NS_A_ID" "$unlabeled_global_action_name" --label "test_case=prune-subject-mappings" --label "fixture=${TEST_PREFIX}-unlabeled-target-action-target" --label "migrated_from=$unlabeled_global_action_id" + create_namespaced_scs unlabeled_scs_target_id "$NS_A_ID" "$unlabeled_global_sets" --label "test_case=prune-subject-mappings" --label "fixture=${TEST_PREFIX}-unlabeled-target-scs-target" --label "migrated_from=$unlabeled_global_scs_id" create_namespaced_subject_mapping unlabeled_mapping_target_id "$NS_A_ID" "$ATTR_A_VAL_2_ID" "$unlabeled_action_target_id" "$unlabeled_scs_target_id" --label "test_case=prune-subject-mappings" --label "fixture=${TEST_PREFIX}-unlabeled-target-mapping-target" ns_a_state_before=$(namespace_state_json "$NS_A_ID") @@ -2548,7 +2542,7 @@ teardown_file() { create_global_action unlabeled_global_action_id "$unlabeled_global_action_name" --label "test_case=prune-registered-resources" --label "fixture=${TEST_PREFIX}-unlabeled-target-action" create_global_registered_resource unlabeled_global_rr_id "${TEST_PREFIX}-prune-rr-unlabeled-target" --label "test_case=prune-registered-resources" --label "fixture=${TEST_PREFIX}-unlabeled-target-rr" create_registered_resource_value unlabeled_global_value_id "$unlabeled_global_rr_id" "${TEST_PREFIX}-unlabeled-target-value" --action-attribute-value "$unlabeled_global_action_id;$ATTR_A_VAL_2_ID" --label "test_case=prune-registered-resources" --label "fixture=${TEST_PREFIX}-unlabeled-target-value" - create_namespaced_action unlabeled_action_target_id "$NS_A_ID" "$unlabeled_global_action_name" --label "test_case=prune-registered-resources" --label "fixture=${TEST_PREFIX}-unlabeled-target-action-target" --label "migrated_from=$unlabeled_global_action_id" --label "migration_run=${TEST_PREFIX}-manual" + create_namespaced_action unlabeled_action_target_id "$NS_A_ID" "$unlabeled_global_action_name" --label "test_case=prune-registered-resources" --label "fixture=${TEST_PREFIX}-unlabeled-target-action-target" --label "migrated_from=$unlabeled_global_action_id" create_namespaced_registered_resource unlabeled_rr_target_id "$NS_A_ID" "${TEST_PREFIX}-prune-rr-unlabeled-target" --label "test_case=prune-registered-resources" --label "fixture=${TEST_PREFIX}-unlabeled-target-rr-target" create_registered_resource_value unlabeled_value_target_id "$unlabeled_rr_target_id" "${TEST_PREFIX}-unlabeled-target-value" --action-attribute-value "$unlabeled_action_target_id;$ATTR_A_VAL_2_ID" --label "test_case=prune-registered-resources" --label "fixture=${TEST_PREFIX}-unlabeled-target-value-target" @@ -2556,8 +2550,8 @@ teardown_file() { create_global_registered_resource multi_namespace_rr_id "${TEST_PREFIX}-prune-rr-multi-namespace" --label "test_case=prune-registered-resources" --label "fixture=${TEST_PREFIX}-multi-namespace-rr" create_registered_resource_value multi_namespace_value_a_id "$multi_namespace_rr_id" "${TEST_PREFIX}-multi-namespace-a" --action-attribute-value "$GLOBAL_READ_ID;$ATTR_A_VAL_1_ID" --label "test_case=prune-registered-resources" --label "fixture=${TEST_PREFIX}-multi-namespace-value-a" create_registered_resource_value multi_namespace_value_b_id "$multi_namespace_rr_id" "${TEST_PREFIX}-multi-namespace-b" --action-attribute-value "$GLOBAL_READ_ID;$ATTR_B_VAL_1_ID" --label "test_case=prune-registered-resources" --label "fixture=${TEST_PREFIX}-multi-namespace-value-b" - create_namespaced_registered_resource multi_namespace_rr_target_id "$NS_A_ID" "${TEST_PREFIX}-prune-rr-multi-namespace" --label "test_case=prune-registered-resources" --label "fixture=${TEST_PREFIX}-multi-namespace-rr-target" --label "migrated_from=$multi_namespace_rr_id" --label "migration_run=${TEST_PREFIX}-manual" - create_registered_resource_value multi_namespace_value_target_id "$multi_namespace_rr_target_id" "${TEST_PREFIX}-multi-namespace-a" --action-attribute-value "$ns_a_read_action_id;$ATTR_A_VAL_1_ID" --label "test_case=prune-registered-resources" --label "fixture=${TEST_PREFIX}-multi-namespace-value-target" --label "migrated_from=$multi_namespace_value_a_id" --label "migration_run=${TEST_PREFIX}-manual" + create_namespaced_registered_resource multi_namespace_rr_target_id "$NS_A_ID" "${TEST_PREFIX}-prune-rr-multi-namespace" --label "test_case=prune-registered-resources" --label "fixture=${TEST_PREFIX}-multi-namespace-rr-target" --label "migrated_from=$multi_namespace_rr_id" + create_registered_resource_value multi_namespace_value_target_id "$multi_namespace_rr_target_id" "${TEST_PREFIX}-multi-namespace-a" --action-attribute-value "$ns_a_read_action_id;$ATTR_A_VAL_1_ID" --label "test_case=prune-registered-resources" --label "fixture=${TEST_PREFIX}-multi-namespace-value-target" --label "migrated_from=$multi_namespace_value_a_id" ns_a_state_before=$(namespace_state_json "$NS_A_ID") @@ -2659,7 +2653,7 @@ teardown_file() { create_namespaced_obligation unlabeled_source_obligation_id "$NS_A_ID" "${TEST_PREFIX}-prune-trigger-unlabeled-target" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-unlabeled-target-obligation" create_obligation_value unlabeled_source_value_id "$unlabeled_source_obligation_id" "${TEST_PREFIX}-unlabeled-target-value" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-unlabeled-target-value" create_legacy_obligation_trigger unlabeled_source_trigger_id "$ATTR_A_VAL_2_ID" "$unlabeled_global_action_id" "$unlabeled_source_value_id" --client-id "${TEST_PREFIX}-unlabeled-target-client" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-unlabeled-target-trigger" - create_namespaced_action unlabeled_action_target_id "$NS_A_ID" "$unlabeled_global_action_name" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-unlabeled-target-action-target" --label "migrated_from=$unlabeled_global_action_id" --label "migration_run=${TEST_PREFIX}-manual" + create_namespaced_action unlabeled_action_target_id "$NS_A_ID" "$unlabeled_global_action_name" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-unlabeled-target-action-target" --label "migrated_from=$unlabeled_global_action_id" run_otdfctl_obligation_triggers create --attribute-value "$ATTR_A_VAL_2_ID" --action "$unlabeled_action_target_id" --obligation-value "$unlabeled_source_value_id" --client-id "${TEST_PREFIX}-unlabeled-target-client" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-unlabeled-target-trigger-target" --json unlabeled_trigger_target_id=$(echo "$output" | jq -r '.id // empty') assert_not_equal "$unlabeled_trigger_target_id" "" diff --git a/otdfctl/migrations/namespacedpolicy/actions_execute.go b/otdfctl/migrations/namespacedpolicy/actions_execute.go index 80005293c0..cae9099756 100644 --- a/otdfctl/migrations/namespacedpolicy/actions_execute.go +++ b/otdfctl/migrations/namespacedpolicy/actions_execute.go @@ -7,7 +7,7 @@ import ( "github.com/opentdf/platform/protocol/go/policy" ) -func (e *Executor) rememberActionTarget(sourceID string, target *ActionTargetPlan) { +func (e *MigrationExecutor) rememberActionTarget(sourceID string, target *ActionTargetPlan) { if e == nil || sourceID == "" || target == nil { return } @@ -27,7 +27,7 @@ func (e *Executor) rememberActionTarget(sourceID string, target *ActionTargetPla e.actionTargets[sourceID][namespaceKey] = target } -func (e *Executor) cachedActionTargetID(sourceID string, namespace *policy.Namespace) string { +func (e *MigrationExecutor) cachedActionTargetID(sourceID string, namespace *policy.Namespace) string { if e == nil || sourceID == "" { return "" } @@ -50,7 +50,7 @@ func (e *Executor) cachedActionTargetID(sourceID string, namespace *policy.Names return target.TargetID() } -func (e *Executor) executeActions(ctx context.Context, actionPlans []*ActionPlan) error { +func (e *MigrationExecutor) executeActions(ctx context.Context, actionPlans []*ActionPlan) error { for _, actionPlan := range actionPlans { if actionPlan == nil || actionPlan.Source == nil { continue @@ -70,7 +70,7 @@ func (e *Executor) executeActions(ctx context.Context, actionPlans []*ActionPlan return nil } -func (e *Executor) executeActionTarget(ctx context.Context, actionPlan *ActionPlan, target *ActionTargetPlan) error { +func (e *MigrationExecutor) executeActionTarget(ctx context.Context, actionPlan *ActionPlan, target *ActionTargetPlan) error { switch target.Status { case TargetStatusExistingStandard, TargetStatusAlreadyMigrated: if target.TargetID() == "" { @@ -93,7 +93,7 @@ func (e *Executor) executeActionTarget(ctx context.Context, actionPlan *ActionPl } } -func (e *Executor) createActionTarget(ctx context.Context, actionPlan *ActionPlan, target *ActionTargetPlan) error { +func (e *MigrationExecutor) createActionTarget(ctx context.Context, actionPlan *ActionPlan, target *ActionTargetPlan) error { namespace := namespaceIdentifier(target.Namespace) if namespace == "" { return fmt.Errorf("%w: action %q", ErrTargetNamespaceRequired, actionPlan.Source.GetId()) @@ -106,26 +106,22 @@ func (e *Executor) createActionTarget(ctx context.Context, actionPlan *ActionPla metadataForCreate( actionPlan.Source.GetId(), metadataLabels(actionPlan.Source.GetMetadata()), - e.runID, ), ) if err != nil { target.Execution = &ExecutionResult{ - RunID: e.runID, Failure: err.Error(), } return fmt.Errorf("create action %q in namespace %q: %w", actionPlan.Source.GetId(), namespaceLabel(target.Namespace), err) } if created.GetId() == "" { target.Execution = &ExecutionResult{ - RunID: e.runID, Failure: ErrMissingCreatedTargetID.Error(), } return fmt.Errorf("%w: action %q target %q", ErrMissingCreatedTargetID, actionPlan.Source.GetId(), namespaceLabel(target.Namespace)) } target.Execution = &ExecutionResult{ - RunID: e.runID, Applied: true, CreatedTargetID: created.GetId(), } diff --git a/otdfctl/migrations/namespacedpolicy/actions_execute_test.go b/otdfctl/migrations/namespacedpolicy/actions_execute_test.go index 630f474014..c6a3e25489 100644 --- a/otdfctl/migrations/namespacedpolicy/actions_execute_test.go +++ b/otdfctl/migrations/namespacedpolicy/actions_execute_test.go @@ -18,15 +18,14 @@ func TestExecuteActions(t *testing.T) { tests := []struct { name string - plan *Plan + plan *MigrationPlan handler *mockExecutorHandler - runID string wantErr *expectedError - assert func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, plan *Plan) + assert func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) }{ { name: "handles created, existing, and already migrated action targets", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions}, Actions: []*ActionPlan{ { @@ -66,8 +65,7 @@ func TestExecuteActions(t *testing.T) { }, }, }, - runID: "run-123", - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -80,7 +78,6 @@ func TestExecuteActions(t *testing.T) { "owner": "policy-team", "env": "dev", migrationLabelMigratedFrom: "action-1", - migrationLabelRun: "run-123", }, handler.created["decrypt"]["ns-1"].Metadata.GetLabels()) createdTarget := plan.Actions[0].Targets[0] @@ -89,7 +86,6 @@ func TestExecuteActions(t *testing.T) { require.NotNil(t, createdTarget.Execution) assert.True(t, createdTarget.Execution.Applied) assert.Equal(t, "created-action-1", createdTarget.Execution.CreatedTargetID) - assert.Equal(t, "run-123", createdTarget.Execution.RunID) assert.Equal(t, "created-action-1", createdTarget.TargetID()) existingTarget := plan.Actions[0].Targets[1] @@ -106,7 +102,7 @@ func TestExecuteActions(t *testing.T) { }, { name: "ignores unresolved target status", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions}, Actions: []*ActionPlan{ { @@ -122,7 +118,7 @@ func TestExecuteActions(t *testing.T) { }, }, handler: &mockExecutorHandler{}, - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -132,7 +128,7 @@ func TestExecuteActions(t *testing.T) { }, { name: "returns error for missing existing standard target id", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions}, Actions: []*ActionPlan{ { @@ -148,7 +144,7 @@ func TestExecuteActions(t *testing.T) { }, handler: &mockExecutorHandler{}, wantErr: wantError(ErrMissingExistingTarget, `action %q target %q`, "action-1", namespace1.GetFqn()), - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.Error(t, err) @@ -158,7 +154,7 @@ func TestExecuteActions(t *testing.T) { }, { name: "returns error for missing already migrated target id", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions}, Actions: []*ActionPlan{ { @@ -174,7 +170,7 @@ func TestExecuteActions(t *testing.T) { }, handler: &mockExecutorHandler{}, wantErr: wantError(ErrMissingMigratedTarget, `action %q target %q`, "action-1", namespace1.GetFqn()), - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.Error(t, err) @@ -184,7 +180,7 @@ func TestExecuteActions(t *testing.T) { }, { name: "returns error for missing target namespace", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions}, Actions: []*ActionPlan{ { @@ -199,7 +195,7 @@ func TestExecuteActions(t *testing.T) { }, handler: &mockExecutorHandler{}, wantErr: wantError(ErrTargetNamespaceRequired, `action %q`, "action-1"), - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.Error(t, err) @@ -209,7 +205,7 @@ func TestExecuteActions(t *testing.T) { }, { name: "returns error for missing created target id", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions}, Actions: []*ActionPlan{ { @@ -231,7 +227,7 @@ func TestExecuteActions(t *testing.T) { }, }, wantErr: wantError(ErrMissingCreatedTargetID, `action %q target %q`, "action-1", namespace1.GetFqn()), - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.Error(t, err) @@ -243,7 +239,7 @@ func TestExecuteActions(t *testing.T) { }, { name: "returns error for unsupported target status", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions}, Actions: []*ActionPlan{ { @@ -265,7 +261,7 @@ func TestExecuteActions(t *testing.T) { namespace1.GetFqn(), TargetStatus("bogus"), ), - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.Error(t, err) @@ -280,13 +276,10 @@ func TestExecuteActions(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - executor, err := NewExecutor(tt.handler) + executor, err := NewMigrationExecutor(tt.handler) require.NoError(t, err) - if tt.runID != "" { - executor.runID = tt.runID - } - err = executor.Execute(t.Context(), tt.plan) + err = executor.ExecuteMigration(t.Context(), tt.plan) switch { case tt.wantErr != nil: require.Error(t, err) diff --git a/otdfctl/migrations/namespacedpolicy/backup_confirmation.go b/otdfctl/migrations/namespacedpolicy/backup_confirmation.go new file mode 100644 index 0000000000..499f2f0c88 --- /dev/null +++ b/otdfctl/migrations/namespacedpolicy/backup_confirmation.go @@ -0,0 +1,91 @@ +//nolint:forbidigo // interactive migration review requires terminal prompts +package namespacedpolicy + +import ( + "context" + "errors" + "fmt" + + "github.com/opentdf/platform/otdfctl/migrations" +) + +const ( + namespacedPolicyCommitConfirm = "confirm" + namespacedPolicyCommitSkip = "skip" + namespacedPolicyCommitAbort = "abort" + noneLabel = "(none)" + skippedByUserReason = "skipped by user" + + //nolint:gosec // user-facing backup prompt text, not credentials + backupWarningTitle = "WARNING: This operation will migrate namespaced policy objects and may create new policy objects." + backupWarningBody = "It is STRONGLY recommended to take a complete backup of your system before proceeding.\n" + backupConfirmTitle = "Have you taken a complete backup?" + backupConfirmDetail = "Commit mode will apply namespaced policy changes to the target system." + backupAbortDetail = "Choose abort if you have not created a backup yet." + backupConfirmLabel = "Yes, continue" + backupCancelLabel = "Abort" + + //nolint:gosec // user-facing backup prompt text, not credentials + pruneBackupWarningTitle = "WARNING: This operation will prune migrated namespaced policy and permanently delete legacy policy objects." + pruneBackupConfirmDetail = "Commit mode will delete legacy/global policy objects from the target system." + skipObjectLabel = "Skip this object" + skipObjectDescription = "leave this object untouched" +) + +var ( + ErrNamespacedPolicyBackupNotConfirmed = errors.New("user did not confirm backup") + errInteractiveSkipSelected = errors.New("interactive commit target skipped by user") +) + +func ConfirmNamespacedPolicyBackup(ctx context.Context, prompter InteractivePrompter) error { + return confirmNamespacedPolicyBackup(ctx, prompter, backupWarningTitle, backupConfirmDetail) +} + +func ConfirmNamespacedPolicyPruneBackup(ctx context.Context, prompter InteractivePrompter) error { + return confirmNamespacedPolicyBackup(ctx, prompter, pruneBackupWarningTitle, pruneBackupConfirmDetail) +} + +func confirmNamespacedPolicyBackup(ctx context.Context, prompter InteractivePrompter, warningTitle, confirmDetail string) error { + if prompter == nil { + prompter = &HuhPrompter{} + } + + styles := migrations.NewDisplayStyles() + fmt.Println(styles.Warning().Render(warningTitle)) + fmt.Println(styles.Warning().Render(backupWarningBody)) + + err := prompter.Confirm(ctx, ConfirmPrompt{ + Title: backupConfirmTitle, + Description: []string{ + confirmDetail, + backupAbortDetail, + }, + ConfirmLabel: backupConfirmLabel, + CancelLabel: backupCancelLabel, + }) + if err == nil { + return nil + } + if errors.Is(err, ErrInteractiveReviewAborted) { + return ErrNamespacedPolicyBackupNotConfirmed + } + return err +} + +func applyInteractiveDecision(ctx context.Context, prompter InteractivePrompter, prompt SelectPrompt) error { + choice, err := prompter.Select(ctx, prompt) + if err != nil { + return err + } + + switch choice { + case namespacedPolicyCommitConfirm: + return nil + case namespacedPolicyCommitSkip: + return errInteractiveSkipSelected + case namespacedPolicyCommitAbort: + return ErrInteractiveReviewAborted + default: + return fmt.Errorf("invalid interactive commit selection %q", choice) + } +} diff --git a/otdfctl/migrations/namespacedpolicy/interactive_commit.go b/otdfctl/migrations/namespacedpolicy/migration_commit_confirmation.go similarity index 75% rename from otdfctl/migrations/namespacedpolicy/interactive_commit.go rename to otdfctl/migrations/namespacedpolicy/migration_commit_confirmation.go index 521415bdfd..c716291193 100644 --- a/otdfctl/migrations/namespacedpolicy/interactive_commit.go +++ b/otdfctl/migrations/namespacedpolicy/migration_commit_confirmation.go @@ -1,4 +1,3 @@ -//nolint:forbidigo // interactive migration review requires terminal prompts package namespacedpolicy import ( @@ -7,93 +6,33 @@ import ( "fmt" "strings" - "github.com/opentdf/platform/otdfctl/migrations" "github.com/opentdf/platform/protocol/go/policy" ) const ( - namespacedPolicyCommitConfirm = "confirm" - namespacedPolicyCommitSkip = "skip" - namespacedPolicyCommitAbort = "abort" - noneLabel = "(none)" - skippedByUserReason = "skipped by user" - - //nolint:gosec // user-facing backup prompt text, not credentials - backupWarningTitle = "WARNING: This operation will migrate namespaced policy objects and may create new policy objects." - backupWarningBody = "It is STRONGLY recommended to take a complete backup of your system before proceeding.\n" - backupConfirmTitle = "Have you taken a complete backup?" - backupConfirmDetail = "Commit mode will apply namespaced policy changes to the target system." - backupAbortDetail = "Choose abort if you have not created a backup yet." - backupConfirmLabel = "Yes, continue" - backupCancelLabel = "Abort" - - //nolint:gosec // user-facing backup prompt text, not credentials - pruneBackupWarningTitle = "WARNING: This operation will prune migrated namespaced policy and permanently delete legacy policy objects." - pruneBackupConfirmDetail = "Commit mode will delete legacy/global policy objects from the target system." - sourceIDText = "Source ID: " - actionText = "Action: " - actionsText = "Actions: " - resourceText = "Resource: " - targetNamespaceText = "Target namespace: " - attributeValueText = "Attribute value: " - obligationValueText = "Obligation value: " - valuesText = "Values: " - actionBindingsText = "Action bindings: " - subjectSetsTextFmt = "Subject sets: %d" - scsSourceText = "Subject condition set source: " - + sourceIDText = "Source ID: " + actionText = "Action: " + actionsText = "Actions: " + resourceText = "Resource: " + targetNamespaceText = "Target namespace: " + attributeValueText = "Attribute value: " + obligationValueText = "Obligation value: " + valuesText = "Values: " + actionBindingsText = "Action bindings: " + 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." 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" - skipObjectLabel = "Skip this object" - skipObjectDescription = "leave this object untouched" - abortMigrationLabel = "Abort entire migration" - abortMigrationDescription = "stop without applying remaining changes" + confirmMigrationLabel = "Confirm migration" + confirmMigrationDescription = "apply this create operation" + abortMigrationLabel = "Abort entire migration" + abortMigrationDescription = "stop without applying remaining changes" ) -var ErrNamespacedPolicyBackupNotConfirmed = errors.New("user did not confirm backup") - -func ConfirmNamespacedPolicyBackup(ctx context.Context, prompter InteractivePrompter) error { - return confirmNamespacedPolicyBackup(ctx, prompter, backupWarningTitle, backupConfirmDetail) -} - -func ConfirmNamespacedPolicyPruneBackup(ctx context.Context, prompter InteractivePrompter) error { - return confirmNamespacedPolicyBackup(ctx, prompter, pruneBackupWarningTitle, pruneBackupConfirmDetail) -} - -func confirmNamespacedPolicyBackup(ctx context.Context, prompter InteractivePrompter, warningTitle, confirmDetail string) error { - if prompter == nil { - prompter = &HuhPrompter{} - } - - styles := migrations.NewDisplayStyles() - fmt.Println(styles.Warning().Render(warningTitle)) - fmt.Println(styles.Warning().Render(backupWarningBody)) - - err := prompter.Confirm(ctx, ConfirmPrompt{ - Title: backupConfirmTitle, - Description: []string{ - confirmDetail, - backupAbortDetail, - }, - ConfirmLabel: backupConfirmLabel, - CancelLabel: backupCancelLabel, - }) - if err == nil { - return nil - } - if errors.Is(err, ErrInteractiveReviewAborted) { - return ErrNamespacedPolicyBackupNotConfirmed - } - return err -} - -func ReviewNamespacedPolicyInteractiveCommit(ctx context.Context, plan *Plan, prompter InteractivePrompter) error { +func ConfirmMigrationPlan(ctx context.Context, plan *MigrationPlan, prompter InteractivePrompter) error { if plan == nil { return nil } @@ -207,8 +146,6 @@ func ReviewNamespacedPolicyInteractiveCommit(ctx context.Context, plan *Plan, pr return nil } -var errInteractiveSkipSelected = errors.New("interactive commit target skipped by user") - type interactiveCommitReviewState struct { skippedActions map[string]map[string]string skippedSCS map[string]map[string]string @@ -302,24 +239,6 @@ func interactiveReviewNamespaceKey(namespace *policy.Namespace) string { return strings.ToLower(strings.TrimSpace(namespace.GetFqn())) } -func applyInteractiveDecision(ctx context.Context, prompter InteractivePrompter, prompt SelectPrompt) error { - choice, err := prompter.Select(ctx, prompt) - if err != nil { - return err - } - - switch choice { - case namespacedPolicyCommitConfirm: - return nil - case namespacedPolicyCommitSkip: - return errInteractiveSkipSelected - case namespacedPolicyCommitAbort: - return ErrInteractiveReviewAborted - default: - return fmt.Errorf("invalid interactive commit selection %q", choice) - } -} - func actionPrompt(actionPlan *ActionPlan, target *ActionTargetPlan) SelectPrompt { return SelectPrompt{ Title: fmt.Sprintf("Migrate action %q to %s?", actionPlan.Source.GetName(), namespaceDisplay(target.Namespace)), @@ -346,7 +265,7 @@ func subjectConditionSetPrompt(scsPlan *SubjectConditionSetPlan, target *Subject } } -func subjectMappingPrompt(plan *Plan, mappingPlan *SubjectMappingPlan) SelectPrompt { +func subjectMappingPrompt(plan *MigrationPlan, mappingPlan *SubjectMappingPlan) SelectPrompt { return SelectPrompt{ Title: fmt.Sprintf("Migrate subject mapping %q to %s?", mappingPlan.Source.GetId(), namespaceDisplay(mappingPlan.Target.Namespace)), Description: []string{ @@ -361,7 +280,7 @@ func subjectMappingPrompt(plan *Plan, mappingPlan *SubjectMappingPlan) SelectPro } } -func registeredResourcePrompt(plan *Plan, resourcePlan *RegisteredResourcePlan) SelectPrompt { +func registeredResourcePrompt(plan *MigrationPlan, resourcePlan *RegisteredResourcePlan) SelectPrompt { description := []string{ sourceIDText + resourcePlan.Source.GetId(), resourceText + resourcePlan.Source.GetName(), @@ -378,7 +297,7 @@ func registeredResourcePrompt(plan *Plan, resourcePlan *RegisteredResourcePlan) } } -func obligationTriggerPrompt(plan *Plan, triggerPlan *ObligationTriggerPlan) SelectPrompt { +func obligationTriggerPrompt(plan *MigrationPlan, triggerPlan *ObligationTriggerPlan) SelectPrompt { return SelectPrompt{ Title: fmt.Sprintf("Migrate obligation trigger %q to %s?", triggerPlan.Source.GetId(), namespaceDisplay(triggerPlan.Target.Namespace)), Description: []string{ @@ -401,7 +320,7 @@ func confirmSkipAbortOptions() []PromptOption { } } -func plainActionNamesSummary(plan *Plan, sourceIDs []string) string { +func plainActionNamesSummary(plan *MigrationPlan, sourceIDs []string) string { names := make([]string, 0, len(sourceIDs)) seen := make(map[string]struct{}, len(sourceIDs)) for _, sourceID := range sourceIDs { @@ -444,7 +363,7 @@ func plainRegisteredResourceValueFQNsSummary(resource *RegisteredResourcePlan) s return strings.Join(values, ", ") } -func plainRegisteredResourceActionBindingsSummary(plan *Plan, resource *RegisteredResourcePlan) string { +func plainRegisteredResourceActionBindingsSummary(plan *MigrationPlan, resource *RegisteredResourcePlan) string { bindings := make([]string, 0) seen := make(map[string]struct{}) for _, valuePlan := range resource.Target.Values { diff --git a/otdfctl/migrations/namespacedpolicy/interactive_commit_test.go b/otdfctl/migrations/namespacedpolicy/migration_commit_confirmation_test.go similarity index 95% rename from otdfctl/migrations/namespacedpolicy/interactive_commit_test.go rename to otdfctl/migrations/namespacedpolicy/migration_commit_confirmation_test.go index d092099883..1eceba35b3 100644 --- a/otdfctl/migrations/namespacedpolicy/interactive_commit_test.go +++ b/otdfctl/migrations/namespacedpolicy/migration_commit_confirmation_test.go @@ -59,7 +59,7 @@ func TestConfirmNamespacedPolicyPruneBackupMapsAbortToBackupError(t *testing.T) assert.Equal(t, []string{pruneBackupConfirmDetail, backupAbortDetail}, prompter.lastConfirmPrompt.Description) } -func TestReviewNamespacedPolicyInteractiveCommitSkipsDependentsOfSkippedAction(t *testing.T) { +func TestConfirmMigrationPlanSkipsDependentsOfSkippedAction(t *testing.T) { t.Parallel() namespace := &policy.Namespace{ @@ -67,7 +67,7 @@ func TestReviewNamespacedPolicyInteractiveCommitSkipsDependentsOfSkippedAction(t Fqn: "https://example.com", } attributeValue := testAttributeValue("https://example.com/attr/classification/value/secret", namespace) - plan := &Plan{ + plan := &MigrationPlan{ Scopes: []Scope{ ScopeActions, ScopeSubjectConditionSets, @@ -156,7 +156,7 @@ func TestReviewNamespacedPolicyInteractiveCommitSkipsDependentsOfSkippedAction(t }, } - err := ReviewNamespacedPolicyInteractiveCommit(t.Context(), plan, prompter) + err := ConfirmMigrationPlan(t.Context(), plan, prompter) require.NoError(t, err) require.Equal(t, 2, prompter.selectCalls) @@ -188,7 +188,7 @@ func TestReviewNamespacedPolicyInteractiveCommitSkipsDependentsOfSkippedAction(t assert.Nil(t, triggerTarget.Execution) } -func TestReviewNamespacedPolicyInteractiveCommitSkipsMappingsDependentOnSkippedSCS(t *testing.T) { +func TestConfirmMigrationPlanSkipsMappingsDependentOnSkippedSCS(t *testing.T) { t.Parallel() namespace := &policy.Namespace{ @@ -196,7 +196,7 @@ func TestReviewNamespacedPolicyInteractiveCommitSkipsMappingsDependentOnSkippedS Fqn: "https://example.com", } attributeValue := testAttributeValue("https://example.com/attr/classification/value/secret", namespace) - plan := &Plan{ + plan := &MigrationPlan{ Scopes: []Scope{ ScopeActions, ScopeSubjectConditionSets, @@ -247,7 +247,7 @@ func TestReviewNamespacedPolicyInteractiveCommitSkipsMappingsDependentOnSkippedS }, } - err := ReviewNamespacedPolicyInteractiveCommit(t.Context(), plan, prompter) + err := ConfirmMigrationPlan(t.Context(), plan, prompter) require.NoError(t, err) require.Equal(t, 2, prompter.selectCalls) @@ -268,7 +268,7 @@ func TestReviewNamespacedPolicyInteractiveCommitSkipsMappingsDependentOnSkippedS assert.Nil(t, mappingTarget.Execution) } -func TestReviewNamespacedPolicyInteractiveCommitPropagatesAbort(t *testing.T) { +func TestConfirmMigrationPlanPropagatesAbort(t *testing.T) { t.Parallel() // The user aborts on the first action prompt. The reviewer must (a) return @@ -280,7 +280,7 @@ func TestReviewNamespacedPolicyInteractiveCommitPropagatesAbort(t *testing.T) { Fqn: "https://example.com", } attributeValue := testAttributeValue("https://example.com/attr/classification/value/secret", namespace) - plan := &Plan{ + plan := &MigrationPlan{ Scopes: []Scope{ ScopeActions, ScopeSubjectConditionSets, @@ -328,7 +328,7 @@ func TestReviewNamespacedPolicyInteractiveCommitPropagatesAbort(t *testing.T) { selectValues: []string{namespacedPolicyCommitAbort}, } - err := ReviewNamespacedPolicyInteractiveCommit(t.Context(), plan, prompter) + err := ConfirmMigrationPlan(t.Context(), plan, prompter) require.ErrorIs(t, err, ErrInteractiveReviewAborted) // Only the first prompt should have fired; abort must halt the walkthrough. diff --git a/otdfctl/migrations/namespacedpolicy/execute.go b/otdfctl/migrations/namespacedpolicy/migration_execute.go similarity index 89% rename from otdfctl/migrations/namespacedpolicy/execute.go rename to otdfctl/migrations/namespacedpolicy/migration_execute.go index 5313764f41..c6a33c2044 100644 --- a/otdfctl/migrations/namespacedpolicy/execute.go +++ b/otdfctl/migrations/namespacedpolicy/migration_execute.go @@ -5,7 +5,6 @@ import ( "errors" "strings" - "github.com/google/uuid" "github.com/opentdf/platform/protocol/go/common" "github.com/opentdf/platform/protocol/go/policy" "github.com/opentdf/platform/protocol/go/policy/registeredresources" @@ -23,13 +22,11 @@ var ( ErrMissingSubjectConditionSetTarget = errors.New("missing subject condition set target") ErrTargetNamespaceRequired = errors.New("target namespace is required") ErrMissingCreatedTargetID = errors.New("missing created target id") - ErrMissingPruneSourceID = errors.New("missing prune source id") ErrUnsupportedStatus = errors.New("unsupported status") ) const ( migrationLabelMigratedFrom = "migrated_from" - migrationLabelRun = "migration_run" unknownLabel = "" ) @@ -49,28 +46,26 @@ type ExecutorHandler interface { DeleteObligationTrigger(ctx context.Context, id string) (*policy.ObligationTrigger, error) } -type Executor struct { +type MigrationExecutor struct { handler ExecutorHandler - runID string actionTargets map[string]map[string]*ActionTargetPlan subjectConditionSets map[string]map[string]*SubjectConditionSetTargetPlan } -func NewExecutor(handler ExecutorHandler) (*Executor, error) { +func NewMigrationExecutor(handler ExecutorHandler) (*MigrationExecutor, error) { if handler == nil { return nil, ErrNilExecutorHandler } - return &Executor{ + return &MigrationExecutor{ handler: handler, - runID: uuid.NewString(), actionTargets: make(map[string]map[string]*ActionTargetPlan), subjectConditionSets: make(map[string]map[string]*SubjectConditionSetTargetPlan), }, nil } -func (e *Executor) Execute(ctx context.Context, plan *Plan) error { - if err := e.validatePlan(plan); err != nil { +func (e *MigrationExecutor) ExecuteMigration(ctx context.Context, plan *MigrationPlan) error { + if err := e.validateMigrationPlan(plan); err != nil { return err } @@ -93,7 +88,7 @@ func (e *Executor) Execute(ctx context.Context, plan *Plan) error { return nil } -func (e *Executor) validatePlan(plan *Plan) error { +func (e *MigrationExecutor) validateMigrationPlan(plan *MigrationPlan) error { if e == nil || e.handler == nil { return ErrNilExecutorHandler } @@ -104,14 +99,13 @@ func (e *Executor) validatePlan(plan *Plan) error { return nil } -func metadataForCreate(sourceID string, sourceLabels map[string]string, runID string) *common.MetadataMutable { +func metadataForCreate(sourceID string, sourceLabels map[string]string) *common.MetadataMutable { labels := map[string]string{} for key, value := range sourceLabels { labels[key] = value } labels[migrationLabelMigratedFrom] = sourceID - labels[migrationLabelRun] = runID return &common.MetadataMutable{ Labels: labels, diff --git a/otdfctl/migrations/namespacedpolicy/finalize_plan.go b/otdfctl/migrations/namespacedpolicy/migration_finalize_plan.go similarity index 97% rename from otdfctl/migrations/namespacedpolicy/finalize_plan.go rename to otdfctl/migrations/namespacedpolicy/migration_finalize_plan.go index e4723ecbc6..70de34844b 100644 --- a/otdfctl/migrations/namespacedpolicy/finalize_plan.go +++ b/otdfctl/migrations/namespacedpolicy/migration_finalize_plan.go @@ -6,9 +6,9 @@ import ( var ErrNilResolvedTargets = errors.New("planner resolved state is required") -// finalizePlan converts the fully resolved graph into the current Plan shape. +// finalizePlan converts the fully resolved graph into the current migration plan shape. // This is the last planner stage before artifact building/execution wiring. -func finalizePlan(resolved *ResolvedTargets) (*Plan, error) { +func finalizePlan(resolved *ResolvedTargets) (*MigrationPlan, error) { if resolved == nil { return nil, ErrNilResolvedTargets } @@ -70,8 +70,8 @@ func newPlanFinalizer(resolved *ResolvedTargets) *planFinalizer { } } -func (f *planFinalizer) build() *Plan { - return &Plan{ +func (f *planFinalizer) build() *MigrationPlan { + return &MigrationPlan{ Scopes: append([]Scope(nil), f.resolved.Scopes...), Actions: append([]*ActionPlan(nil), f.actions...), SubjectConditionSets: append([]*SubjectConditionSetPlan(nil), f.subjectConditionSets...), diff --git a/otdfctl/migrations/namespacedpolicy/finalize_plan_test.go b/otdfctl/migrations/namespacedpolicy/migration_finalize_plan_test.go similarity index 100% rename from otdfctl/migrations/namespacedpolicy/finalize_plan_test.go rename to otdfctl/migrations/namespacedpolicy/migration_finalize_plan_test.go diff --git a/otdfctl/migrations/namespacedpolicy/plan.go b/otdfctl/migrations/namespacedpolicy/migration_plan.go similarity index 97% rename from otdfctl/migrations/namespacedpolicy/plan.go rename to otdfctl/migrations/namespacedpolicy/migration_plan.go index f1f65e34ee..d90f910c13 100644 --- a/otdfctl/migrations/namespacedpolicy/plan.go +++ b/otdfctl/migrations/namespacedpolicy/migration_plan.go @@ -30,7 +30,7 @@ type Unresolved struct { Message string } -type Plan struct { +type MigrationPlan struct { Scopes []Scope `json:"scopes"` Actions []*ActionPlan `json:"actions"` SubjectConditionSets []*SubjectConditionSetPlan `json:"subject_condition_sets"` @@ -50,7 +50,6 @@ const ( ) type ExecutionResult struct { - RunID string `json:"run_id,omitempty"` Applied bool `json:"applied,omitempty"` CreatedTargetID string `json:"created_target_id,omitempty"` Failure string `json:"failure,omitempty"` @@ -271,7 +270,7 @@ func (t *ObligationTriggerTargetPlan) TargetID() string { return t.ExistingID } -func (p *Plan) LookupActionTarget(sourceID, namespaceID string) *ActionTargetPlan { +func (p *MigrationPlan) LookupActionTarget(sourceID, namespaceID string) *ActionTargetPlan { if p == nil || sourceID == "" || namespaceID == "" { return nil } @@ -290,7 +289,7 @@ func (p *Plan) LookupActionTarget(sourceID, namespaceID string) *ActionTargetPla return nil } -func (p *Plan) LookupSubjectConditionSetTarget(sourceID, namespaceID string) *SubjectConditionSetTargetPlan { +func (p *MigrationPlan) LookupSubjectConditionSetTarget(sourceID, namespaceID string) *SubjectConditionSetTargetPlan { if p == nil || sourceID == "" || namespaceID == "" { return nil } diff --git a/otdfctl/migrations/namespacedpolicy/plan_test.go b/otdfctl/migrations/namespacedpolicy/migration_plan_test.go similarity index 97% rename from otdfctl/migrations/namespacedpolicy/plan_test.go rename to otdfctl/migrations/namespacedpolicy/migration_plan_test.go index b11551b4de..2a5366051b 100644 --- a/otdfctl/migrations/namespacedpolicy/plan_test.go +++ b/otdfctl/migrations/namespacedpolicy/migration_plan_test.go @@ -26,7 +26,7 @@ func TestPlanLookupActionTarget(t *testing.T) { targetA := &ActionTargetPlan{Namespace: nsA, Status: TargetStatusCreate} targetB := &ActionTargetPlan{Namespace: nsB, Status: TargetStatusCreate} - plan := &Plan{ + plan := &MigrationPlan{ Actions: []*ActionPlan{ nil, {Source: nil, Targets: []*ActionTargetPlan{targetA}}, @@ -44,7 +44,7 @@ func TestPlanLookupActionTarget(t *testing.T) { tests := []struct { name string - plan *Plan + plan *MigrationPlan sourceID string namespaceID string want *ActionTargetPlan @@ -74,7 +74,7 @@ func TestPlanLookupSubjectConditionSetTarget(t *testing.T) { targetA := &SubjectConditionSetTargetPlan{Namespace: nsA, Status: TargetStatusCreate} targetB := &SubjectConditionSetTargetPlan{Namespace: nsB, Status: TargetStatusCreate} - plan := &Plan{ + plan := &MigrationPlan{ SubjectConditionSets: []*SubjectConditionSetPlan{ nil, {Source: nil, Targets: []*SubjectConditionSetTargetPlan{targetA}}, @@ -92,7 +92,7 @@ func TestPlanLookupSubjectConditionSetTarget(t *testing.T) { tests := []struct { name string - plan *Plan + plan *MigrationPlan sourceID string namespaceID string want *SubjectConditionSetTargetPlan diff --git a/otdfctl/migrations/namespacedpolicy/planner.go b/otdfctl/migrations/namespacedpolicy/migration_planner.go similarity index 91% rename from otdfctl/migrations/namespacedpolicy/planner.go rename to otdfctl/migrations/namespacedpolicy/migration_planner.go index edbc929394..f66fbec5c5 100644 --- a/otdfctl/migrations/namespacedpolicy/planner.go +++ b/otdfctl/migrations/namespacedpolicy/migration_planner.go @@ -28,14 +28,14 @@ type PolicyClient interface { ListNamespaces(ctx context.Context, state common.ActiveStateEnum, limit, offset int32, sort handlers.SortOption) (*namespaces.ListNamespacesResponse, error) } -type Planner struct { +type MigrationPlanner struct { retriever *Retriever requestedScopes scopeSet expandedScopes scopeSet reviewer InteractiveReviewer } -type Option func(*Planner) +type Option func(*MigrationPlanner) type Retrieved struct { Scopes []Scope @@ -59,7 +59,7 @@ type ExistingTargets struct { ObligationTriggers map[string][]*policy.ObligationTrigger } -func NewPlanner(handler PolicyClient, scopeCSV string, opts ...Option) (*Planner, error) { +func NewMigrationPlanner(handler PolicyClient, scopeCSV string, opts ...Option) (*MigrationPlanner, error) { if handler == nil { return nil, ErrNilPlannerHandler } @@ -74,7 +74,7 @@ func NewPlanner(handler PolicyClient, scopeCSV string, opts ...Option) (*Planner return nil, err } - planner := &Planner{ + planner := &MigrationPlanner{ retriever: newRetriever(handler, defaultPlannerPageSize), requestedScopes: normalizedScopes, expandedScopes: expandScopes(normalizedScopes), @@ -90,18 +90,18 @@ func NewPlanner(handler PolicyClient, scopeCSV string, opts ...Option) (*Planner } func WithPageSize(pageSize int32) Option { - return func(planner *Planner) { + return func(planner *MigrationPlanner) { planner.retriever.pageSize = pageSize } } func WithInteractiveReviewer(reviewer InteractiveReviewer) Option { - return func(planner *Planner) { + return func(planner *MigrationPlanner) { planner.reviewer = reviewer } } -func (p *Planner) Plan(ctx context.Context) (*Plan, error) { +func (p *MigrationPlanner) Plan(ctx context.Context) (*MigrationPlan, error) { resolved, err := p.resolve(ctx) if err != nil { return nil, err @@ -110,7 +110,7 @@ func (p *Planner) Plan(ctx context.Context) (*Plan, error) { return finalizePlan(resolved) } -func (p *Planner) resolve(ctx context.Context) (*ResolvedTargets, error) { +func (p *MigrationPlanner) resolve(ctx context.Context) (*ResolvedTargets, error) { retrieved, err := p.retrieve(ctx) if err != nil { return nil, err @@ -147,7 +147,7 @@ func (p *Planner) resolve(ctx context.Context) (*ResolvedTargets, error) { // Retrieve the candidate policy constructs for items within scope or dependent // on that scope. -func (p *Planner) retrieve(ctx context.Context) (*Retrieved, error) { +func (p *MigrationPlanner) retrieve(ctx context.Context) (*Retrieved, error) { if p == nil || p.retriever == nil || p.retriever.handler == nil { return nil, ErrNilPlannerHandler } diff --git a/otdfctl/migrations/namespacedpolicy/planner_test.go b/otdfctl/migrations/namespacedpolicy/migration_planner_test.go similarity index 97% rename from otdfctl/migrations/namespacedpolicy/planner_test.go rename to otdfctl/migrations/namespacedpolicy/migration_planner_test.go index a86f9bba9c..0582d913c5 100644 --- a/otdfctl/migrations/namespacedpolicy/planner_test.go +++ b/otdfctl/migrations/namespacedpolicy/migration_planner_test.go @@ -69,7 +69,7 @@ func TestPlannerPlanMarksActionAlreadyMigratedWithoutMetadata(t *testing.T) { }, } - planner, err := NewPlanner(handler, "actions") + planner, err := NewMigrationPlanner(handler, "actions") require.NoError(t, err) plan, err := planner.Plan(t.Context()) @@ -199,7 +199,7 @@ func TestPlannerPlanDoesNotLeakSupportSubjectMappingsIntoActionScope(t *testing. }, } - planner, err := NewPlanner(handler, "subject-condition-sets,registered-resources") + planner, err := NewMigrationPlanner(handler, "subject-condition-sets,registered-resources") require.NoError(t, err) plan, err := planner.Plan(t.Context()) @@ -361,7 +361,7 @@ func TestPlannerRetrieveUsesRequestedScopeBoundaries(t *testing.T) { }, } - planner, err := NewPlanner(handler, tt.scopeCSV) + planner, err := NewMigrationPlanner(handler, tt.scopeCSV) require.NoError(t, err) retrieved, err := planner.retrieve(t.Context()) @@ -488,7 +488,7 @@ func TestPlannerPlanAllScopesBuildsAllPlanSections(t *testing.T) { }, } - planner, err := NewPlanner(handler, "actions,subject-condition-sets,subject-mappings,registered-resources,obligation-triggers") + planner, err := NewMigrationPlanner(handler, "actions,subject-condition-sets,subject-mappings,registered-resources,obligation-triggers") require.NoError(t, err) plan, err := planner.Plan(t.Context()) @@ -638,7 +638,7 @@ func TestPlannerPlanCarriesMultiActionMappingsAndMultiBindingRegisteredResources }, } - planner, err := NewPlanner(handler, "subject-mappings,registered-resources") + planner, err := NewMigrationPlanner(handler, "subject-mappings,registered-resources") require.NoError(t, err) plan, err := planner.Plan(t.Context()) @@ -738,7 +738,7 @@ func TestPlannerPlanInvokesInteractiveReviewerWhenConfigured(t *testing.T) { }, } - planner, err := NewPlanner(handler, "actions", WithInteractiveReviewer(reviewer)) + planner, err := NewMigrationPlanner(handler, "actions", WithInteractiveReviewer(reviewer)) require.NoError(t, err) plan, err := planner.Plan(t.Context()) @@ -797,7 +797,7 @@ func TestPlannerPlanPropagatesInteractiveReviewerError(t *testing.T) { }, } - planner, err := NewPlanner(handler, "actions", WithInteractiveReviewer(reviewer)) + planner, err := NewMigrationPlanner(handler, "actions", WithInteractiveReviewer(reviewer)) require.NoError(t, err) _, err = planner.Plan(t.Context()) @@ -858,7 +858,7 @@ func TestPlannerPlanInteractiveReviewerLeavesCurrentUnresolvedPlanShapeUntouched }, } - planner, err := NewPlanner(handler, "registered-resources", WithInteractiveReviewer(reviewer)) + planner, err := NewMigrationPlanner(handler, "registered-resources", WithInteractiveReviewer(reviewer)) require.NoError(t, err) plan, err := planner.Plan(t.Context()) @@ -931,7 +931,7 @@ func TestPlannerPlanHuhInteractiveReviewerResolvesRegisteredResourceConflict(t * }, } - planner, err := NewPlanner(handler, "registered-resources", WithInteractiveReviewer(NewHuhInteractiveReviewer(handler, prompter))) + planner, err := NewMigrationPlanner(handler, "registered-resources", WithInteractiveReviewer(NewHuhInteractiveReviewer(handler, prompter))) require.NoError(t, err) plan, err := planner.Plan(t.Context()) diff --git a/otdfctl/migrations/namespacedpolicy/migration_summary.go b/otdfctl/migrations/namespacedpolicy/migration_summary.go index 7e96852236..6537e4ce52 100644 --- a/otdfctl/migrations/namespacedpolicy/migration_summary.go +++ b/otdfctl/migrations/namespacedpolicy/migration_summary.go @@ -26,22 +26,22 @@ const ( migrationPendingSectionLabel = "Will Create" ) -func RenderNamespacedPolicySummary(plan *Plan, commit bool) string { +func RenderNamespacedPolicySummary(plan *MigrationPlan, commit bool) string { return renderNamespacedPolicySummary(plan, commit, "success") } -func RenderNamespacedPolicySummaryWithResult(plan *Plan, commit bool, result string) string { +func RenderNamespacedPolicySummaryWithResult(plan *MigrationPlan, commit bool, result string) string { return renderNamespacedPolicySummary(plan, commit, result) } -func planScopes(plan *Plan) []Scope { +func planScopes(plan *MigrationPlan) []Scope { if plan == nil { return nil } return plan.Scopes } -func renderNamespacedPolicySummary(plan *Plan, commit bool, result string) string { +func renderNamespacedPolicySummary(plan *MigrationPlan, commit bool, result string) string { styles := migrations.NewDisplayStyles() return renderSummaryDocument(styles, summaryDocument{ plannedTitle: "Namespaced Policy Migration Plan", @@ -67,7 +67,7 @@ func appendMigrationSummaryCountParts(parts []string, counts summaryCounts) []st ) } -func summarizeActions(plan *Plan, commit bool, styles *migrations.DisplayStyles) constructSummary { +func summarizeActions(plan *MigrationPlan, commit bool, styles *migrations.DisplayStyles) constructSummary { summary := constructSummary{ label: "Actions", include: includesScope(planScopes(plan), ScopeActions), @@ -98,7 +98,7 @@ func summarizeActions(plan *Plan, commit bool, styles *migrations.DisplayStyles) return summary } -func summarizeSubjectConditionSets(plan *Plan, commit bool, styles *migrations.DisplayStyles) constructSummary { +func summarizeSubjectConditionSets(plan *MigrationPlan, commit bool, styles *migrations.DisplayStyles) constructSummary { summary := constructSummary{ label: "Subject Condition Sets", include: includesScope(planScopes(plan), ScopeSubjectConditionSets), @@ -129,7 +129,7 @@ func summarizeSubjectConditionSets(plan *Plan, commit bool, styles *migrations.D return summary } -func summarizeSubjectMappings(plan *Plan, commit bool, styles *migrations.DisplayStyles) constructSummary { +func summarizeSubjectMappings(plan *MigrationPlan, commit bool, styles *migrations.DisplayStyles) constructSummary { summary := constructSummary{ label: "Subject Mappings", include: includesScope(planScopes(plan), ScopeSubjectMappings), @@ -159,7 +159,7 @@ func summarizeSubjectMappings(plan *Plan, commit bool, styles *migrations.Displa return summary } -func summarizeRegisteredResources(plan *Plan, commit bool, styles *migrations.DisplayStyles) constructSummary { +func summarizeRegisteredResources(plan *MigrationPlan, commit bool, styles *migrations.DisplayStyles) constructSummary { summary := constructSummary{ label: "Registered Resources", include: includesScope(planScopes(plan), ScopeRegisteredResources), @@ -194,7 +194,7 @@ func summarizeRegisteredResources(plan *Plan, commit bool, styles *migrations.Di return summary } -func summarizeObligationTriggers(plan *Plan, commit bool, styles *migrations.DisplayStyles) constructSummary { +func summarizeObligationTriggers(plan *MigrationPlan, commit bool, styles *migrations.DisplayStyles) constructSummary { summary := constructSummary{ label: "Obligation Triggers", include: includesScope(planScopes(plan), ScopeObligationTriggers), @@ -313,7 +313,7 @@ func formatSubjectConditionSetCreatedLine(styles *migrations.DisplayStyles, scs ) } -func formatSubjectMappingCreatedLine(styles *migrations.DisplayStyles, plan *Plan, mapping *SubjectMappingPlan, commit bool) string { +func formatSubjectMappingCreatedLine(styles *migrations.DisplayStyles, plan *MigrationPlan, mapping *SubjectMappingPlan, commit bool) string { line := formatCreatedLine(styles, subjectMappingKind, mapping.Source.GetId(), mapping.Target.Namespace, mapping.Target.TargetID(), commit) return appendDetails(line, "attribute_value="+styles.Namespace().Render(valueFQN(mapping.Source.GetAttributeValue())), @@ -322,7 +322,7 @@ func formatSubjectMappingCreatedLine(styles *migrations.DisplayStyles, plan *Pla ) } -func formatRegisteredResourceCreatedLine(styles *migrations.DisplayStyles, plan *Plan, resource *RegisteredResourcePlan, commit bool) string { +func formatRegisteredResourceCreatedLine(styles *migrations.DisplayStyles, plan *MigrationPlan, resource *RegisteredResourcePlan, commit bool) string { line := formatCreatedLine(styles, registeredResourceKind, resource.Source.GetName(), resource.Target.Namespace, resource.Target.TargetID(), commit) return appendDetails(line, @@ -339,7 +339,7 @@ func formatRegisteredResourceFailedLine(styles *migrations.DisplayStyles, resour return line } -func formatObligationTriggerCreatedLine(styles *migrations.DisplayStyles, plan *Plan, trigger *ObligationTriggerPlan, commit bool) string { +func formatObligationTriggerCreatedLine(styles *migrations.DisplayStyles, plan *MigrationPlan, trigger *ObligationTriggerPlan, commit bool) string { line := formatCreatedLine(styles, obligationTriggerKind, trigger.Source.GetId(), trigger.Target.Namespace, trigger.Target.TargetID(), commit) return appendDetails(line, "action="+actionNamesSummary(styles, plan, []string{trigger.Target.ActionSourceID}), @@ -463,7 +463,7 @@ func registeredResourceFailedValue(resource *RegisteredResourcePlan) string { return "" } -func registeredResourceActionBindingsSummary(styles *migrations.DisplayStyles, plan *Plan, resource *RegisteredResourcePlan) string { +func registeredResourceActionBindingsSummary(styles *migrations.DisplayStyles, plan *MigrationPlan, resource *RegisteredResourcePlan) string { bindings := make([]string, 0) seen := make(map[string]struct{}) for _, valuePlan := range resource.Target.Values { @@ -494,7 +494,7 @@ func registeredResourceActionBindingsSummary(styles *migrations.DisplayStyles, p return strings.Join(bindings, ", ") } -func actionNamesSummary(styles *migrations.DisplayStyles, plan *Plan, sourceIDs []string) string { +func actionNamesSummary(styles *migrations.DisplayStyles, plan *MigrationPlan, sourceIDs []string) string { names := make([]string, 0, len(sourceIDs)) seen := make(map[string]struct{}, len(sourceIDs)) for _, sourceID := range sourceIDs { @@ -517,7 +517,7 @@ func actionNamesSummary(styles *migrations.DisplayStyles, plan *Plan, sourceIDs return strings.Join(names, ", ") } -func actionNameBySourceID(plan *Plan, sourceID string) string { +func actionNameBySourceID(plan *MigrationPlan, sourceID string) string { if plan == nil { return "" } diff --git a/otdfctl/migrations/namespacedpolicy/migration_summary_test.go b/otdfctl/migrations/namespacedpolicy/migration_summary_test.go index 410614afea..ca49af8572 100644 --- a/otdfctl/migrations/namespacedpolicy/migration_summary_test.go +++ b/otdfctl/migrations/namespacedpolicy/migration_summary_test.go @@ -14,7 +14,7 @@ func TestRenderNamespacedPolicySummaryCommitIncludesCountsAndCreatedDetails(t *t otherNamespace := &policy.Namespace{Id: "ns-2", Fqn: "https://example.org"} classificationValue := testAttributeValue("https://example.com/attr/classification/value/secret", namespace) - plan := &Plan{ + plan := &MigrationPlan{ Scopes: []Scope{ ScopeActions, ScopeSubjectConditionSets, @@ -164,7 +164,7 @@ func TestRenderNamespacedPolicySummaryDryRunUsesToCreateLabel(t *testing.T) { t.Parallel() namespace := &policy.Namespace{Id: "ns-1", Fqn: "https://example.com"} - plan := &Plan{ + plan := &MigrationPlan{ Scopes: []Scope{ScopeActions}, Actions: []*ActionPlan{ { @@ -195,7 +195,7 @@ func TestRenderNamespacedPolicySummaryDryRunUsesToCreateLabel(t *testing.T) { func TestRenderNamespacedPolicySummaryIncludesTargetlessUnresolvedEntries(t *testing.T) { t.Parallel() - plan := &Plan{ + plan := &MigrationPlan{ Scopes: []Scope{ ScopeActions, ScopeSubjectConditionSets, @@ -251,7 +251,7 @@ func TestRenderNamespacedPolicySummaryCommitFailureShowsFailedAndPendingCreates( namespace := &policy.Namespace{Id: "ns-1", Fqn: "https://example.com"} - plan := &Plan{ + plan := &MigrationPlan{ Scopes: []Scope{ ScopeActions, ScopeSubjectConditionSets, diff --git a/otdfctl/migrations/namespacedpolicy/obligation_triggers_execute.go b/otdfctl/migrations/namespacedpolicy/obligation_triggers_execute.go index 57a0295f30..773325f851 100644 --- a/otdfctl/migrations/namespacedpolicy/obligation_triggers_execute.go +++ b/otdfctl/migrations/namespacedpolicy/obligation_triggers_execute.go @@ -8,7 +8,7 @@ import ( "github.com/opentdf/platform/protocol/go/policy" ) -func (e *Executor) executeObligationTriggers(ctx context.Context, plans []*ObligationTriggerPlan) error { +func (e *MigrationExecutor) executeObligationTriggers(ctx context.Context, plans []*ObligationTriggerPlan) error { if len(plans) == 0 { return nil } @@ -30,7 +30,7 @@ func (e *Executor) executeObligationTriggers(ctx context.Context, plans []*Oblig return nil } -func (e *Executor) executeObligationTriggerTarget(ctx context.Context, triggerPlan *ObligationTriggerPlan, target *ObligationTriggerTargetPlan) error { +func (e *MigrationExecutor) executeObligationTriggerTarget(ctx context.Context, triggerPlan *ObligationTriggerPlan, target *ObligationTriggerTargetPlan) error { //nolint:exhaustive // Obligation-trigger execution only handles create and already-migrated explicitly; all other statuses are unsupported. switch target.Status { case TargetStatusAlreadyMigrated: @@ -49,7 +49,7 @@ func (e *Executor) executeObligationTriggerTarget(ctx context.Context, triggerPl } } -func (e *Executor) createObligationTriggerTarget(ctx context.Context, triggerPlan *ObligationTriggerPlan, target *ObligationTriggerTargetPlan) error { +func (e *MigrationExecutor) createObligationTriggerTarget(ctx context.Context, triggerPlan *ObligationTriggerPlan, target *ObligationTriggerTargetPlan) error { actionID, err := e.requireActionTargetID(target.ActionSourceID, target.Namespace, triggerPlan.Source.GetId()) if err != nil { return err @@ -64,26 +64,22 @@ func (e *Executor) createObligationTriggerTarget(ctx context.Context, triggerPla metadataForCreate( triggerPlan.Source.GetId(), metadataLabels(triggerPlan.Source.GetMetadata()), - e.runID, ), ) if err != nil { target.Execution = &ExecutionResult{ - RunID: e.runID, Failure: err.Error(), } return fmt.Errorf("create obligation trigger %q in namespace %q: %w", triggerPlan.Source.GetId(), namespaceLabel(target.Namespace), err) } if created.GetId() == "" { target.Execution = &ExecutionResult{ - RunID: e.runID, Failure: ErrMissingCreatedTargetID.Error(), } return fmt.Errorf("%w: obligation trigger %q target %q", ErrMissingCreatedTargetID, triggerPlan.Source.GetId(), namespaceLabel(target.Namespace)) } target.Execution = &ExecutionResult{ - RunID: e.runID, Applied: true, CreatedTargetID: created.GetId(), } @@ -92,7 +88,7 @@ func (e *Executor) createObligationTriggerTarget(ctx context.Context, triggerPla } // TODO: Eventually make this generic when we merge sm / rr -func (e *Executor) requireActionTargetID(sourceID string, targetNamespace *policy.Namespace, ownerID string) (string, error) { +func (e *MigrationExecutor) requireActionTargetID(sourceID string, targetNamespace *policy.Namespace, ownerID string) (string, error) { if sourceID == "" { return "", fmt.Errorf("%w: obligation trigger %q action source id is missing", ErrMissingMigratedTarget, ownerID) } diff --git a/otdfctl/migrations/namespacedpolicy/obligation_triggers_execute_test.go b/otdfctl/migrations/namespacedpolicy/obligation_triggers_execute_test.go index 42f934ea66..90cde505cc 100644 --- a/otdfctl/migrations/namespacedpolicy/obligation_triggers_execute_test.go +++ b/otdfctl/migrations/namespacedpolicy/obligation_triggers_execute_test.go @@ -19,15 +19,14 @@ func TestExecuteObligationTriggers(t *testing.T) { tests := []struct { name string - plan *Plan + plan *MigrationPlan handler *mockExecutorHandler - runID string wantErr *expectedError - assert func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, plan *Plan) + assert func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) }{ { name: "handles created and already migrated obligation trigger targets", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions, ScopeObligationTriggers}, Actions: []*ActionPlan{ { @@ -100,8 +99,7 @@ func TestExecuteObligationTriggers(t *testing.T) { }, }, }, - runID: "run-789", - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -117,14 +115,12 @@ func TestExecuteObligationTriggers(t *testing.T) { "owner": "policy-team", "env": "dev", migrationLabelMigratedFrom: "trigger-1", - migrationLabelRun: "run-789", }, createdCall.Metadata.GetLabels()) createdTarget := plan.ObligationTriggers[0].Target require.NotNil(t, createdTarget.Execution) assert.True(t, createdTarget.Execution.Applied) assert.Equal(t, "created-trigger-1", createdTarget.Execution.CreatedTargetID) - assert.Equal(t, "run-789", createdTarget.Execution.RunID) assert.Equal(t, "created-trigger-1", createdTarget.TargetID()) migratedTarget := plan.ObligationTriggers[1].Target @@ -134,7 +130,7 @@ func TestExecuteObligationTriggers(t *testing.T) { }, { name: "skips skipped obligation trigger targets", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeObligationTriggers}, ObligationTriggers: []*ObligationTriggerPlan{ { @@ -148,7 +144,7 @@ func TestExecuteObligationTriggers(t *testing.T) { }, }, handler: &mockExecutorHandler{}, - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -158,7 +154,7 @@ func TestExecuteObligationTriggers(t *testing.T) { }, { name: "ignores unresolved target status", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeObligationTriggers}, ObligationTriggers: []*ObligationTriggerPlan{ { @@ -172,7 +168,7 @@ func TestExecuteObligationTriggers(t *testing.T) { }, }, handler: &mockExecutorHandler{}, - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -181,7 +177,7 @@ func TestExecuteObligationTriggers(t *testing.T) { }, { name: "returns error when migrated action target is unavailable", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeObligationTriggers}, ObligationTriggers: []*ObligationTriggerPlan{ { @@ -201,7 +197,7 @@ func TestExecuteObligationTriggers(t *testing.T) { }, handler: &mockExecutorHandler{}, wantErr: wantError(ErrMissingMigratedTarget, `obligation trigger %q action %q target %q`, "trigger-1", "action-1", namespace1.GetFqn()), - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.Error(t, err) @@ -211,7 +207,7 @@ func TestExecuteObligationTriggers(t *testing.T) { }, { name: "returns error for missing already migrated trigger id", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeObligationTriggers}, ObligationTriggers: []*ObligationTriggerPlan{ { @@ -225,7 +221,7 @@ func TestExecuteObligationTriggers(t *testing.T) { }, handler: &mockExecutorHandler{}, wantErr: wantError(ErrMissingMigratedTarget, `obligation trigger %q target %q`, "trigger-1", namespace1.GetFqn()), - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.Error(t, err) @@ -234,7 +230,7 @@ func TestExecuteObligationTriggers(t *testing.T) { }, { name: "returns error for missing created target id", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions, ScopeObligationTriggers}, Actions: []*ActionPlan{ { @@ -276,7 +272,7 @@ func TestExecuteObligationTriggers(t *testing.T) { }, }, wantErr: wantError(ErrMissingCreatedTargetID, `obligation trigger %q target %q`, "trigger-1", namespace1.GetFqn()), - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.Error(t, err) @@ -287,7 +283,7 @@ func TestExecuteObligationTriggers(t *testing.T) { }, { name: "records create failure from handler", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions, ScopeObligationTriggers}, Actions: []*ActionPlan{ { @@ -332,7 +328,7 @@ func TestExecuteObligationTriggers(t *testing.T) { is: errBoom, message: `create obligation trigger "trigger-1" in namespace "https://example.com": boom`, }, - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.Error(t, err) @@ -343,7 +339,7 @@ func TestExecuteObligationTriggers(t *testing.T) { }, { name: "returns error for unsupported target status", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeObligationTriggers}, ObligationTriggers: []*ObligationTriggerPlan{ { @@ -363,7 +359,7 @@ func TestExecuteObligationTriggers(t *testing.T) { namespace1.GetFqn(), TargetStatus("bogus"), ), - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.Error(t, err) @@ -377,13 +373,10 @@ func TestExecuteObligationTriggers(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - executor, err := NewExecutor(tt.handler) + executor, err := NewMigrationExecutor(tt.handler) require.NoError(t, err) - if tt.runID != "" { - executor.runID = tt.runID - } - err = executor.Execute(t.Context(), tt.plan) + err = executor.ExecuteMigration(t.Context(), tt.plan) switch { case tt.wantErr != nil: require.Error(t, err) diff --git a/otdfctl/migrations/namespacedpolicy/prune_execute.go b/otdfctl/migrations/namespacedpolicy/prune_execute.go index 1ad45f1db2..b8fd4c3aea 100644 --- a/otdfctl/migrations/namespacedpolicy/prune_execute.go +++ b/otdfctl/migrations/namespacedpolicy/prune_execute.go @@ -2,6 +2,7 @@ package namespacedpolicy import ( "context" + "errors" "fmt" ) @@ -9,7 +10,24 @@ type ( pruneDeleteFunc[T prunePlanItem] func(context.Context, T, string) error ) -func (e *Executor) ExecutePrune(ctx context.Context, plan *PrunePlan) error { +var ( + ErrNilPruneExecutionPlan = errors.New("prune plan is required") + ErrMissingPruneSourceID = errors.New("missing prune source id") +) + +type PruneExecutor struct { + handler ExecutorHandler +} + +func NewPruneExecutor(handler ExecutorHandler) (*PruneExecutor, error) { + if handler == nil { + return nil, ErrNilExecutorHandler + } + + return &PruneExecutor{handler: handler}, nil +} + +func (e *PruneExecutor) ExecutePrune(ctx context.Context, plan *PrunePlan) error { if err := e.validatePrunePlan(plan); err != nil { return err } @@ -30,12 +48,12 @@ func (e *Executor) ExecutePrune(ctx context.Context, plan *PrunePlan) error { } } -func (e *Executor) validatePrunePlan(plan *PrunePlan) error { +func (e *PruneExecutor) validatePrunePlan(plan *PrunePlan) error { if e == nil || e.handler == nil { return ErrNilExecutorHandler } if plan == nil { - return ErrNilExecutionPlan + return ErrNilPruneExecutionPlan } if len(plan.Scopes) == 0 { return ErrEmptyPlannerScope @@ -47,32 +65,32 @@ func (e *Executor) validatePrunePlan(plan *PrunePlan) error { return nil } -func (e *Executor) executePruneActions(ctx context.Context, plans []*PruneActionPlan) error { +func (e *PruneExecutor) executePruneActions(ctx context.Context, plans []*PruneActionPlan) error { return executePruneItems(ctx, e, plans, "action", func(ctx context.Context, _ *PruneActionPlan, sourceID string) error { return e.handler.DeleteAction(ctx, sourceID) }) } -func (e *Executor) executePruneSubjectConditionSets(ctx context.Context, plans []*PruneSubjectConditionSetPlan) error { +func (e *PruneExecutor) executePruneSubjectConditionSets(ctx context.Context, plans []*PruneSubjectConditionSetPlan) error { return executePruneItems(ctx, e, plans, "subject condition set", func(ctx context.Context, _ *PruneSubjectConditionSetPlan, sourceID string) error { return e.handler.DeleteSubjectConditionSet(ctx, sourceID) }) } -func (e *Executor) executePruneSubjectMappings(ctx context.Context, plans []*PruneSubjectMappingPlan) error { +func (e *PruneExecutor) executePruneSubjectMappings(ctx context.Context, plans []*PruneSubjectMappingPlan) error { return executePruneItems(ctx, e, plans, "subject mapping", func(ctx context.Context, _ *PruneSubjectMappingPlan, sourceID string) error { _, err := e.handler.DeleteSubjectMapping(ctx, sourceID) return err }) } -func (e *Executor) executePruneRegisteredResources(ctx context.Context, plans []*PruneRegisteredResourcePlan) error { +func (e *PruneExecutor) executePruneRegisteredResources(ctx context.Context, plans []*PruneRegisteredResourcePlan) error { return executePruneItems(ctx, e, plans, "registered resource", func(ctx context.Context, _ *PruneRegisteredResourcePlan, sourceID string) error { return e.handler.DeleteRegisteredResource(ctx, sourceID) }) } -func (e *Executor) executePruneObligationTriggers(ctx context.Context, plans []*PruneObligationTriggerPlan) error { +func (e *PruneExecutor) executePruneObligationTriggers(ctx context.Context, plans []*PruneObligationTriggerPlan) error { return executePruneItems(ctx, e, plans, "obligation trigger", func(ctx context.Context, _ *PruneObligationTriggerPlan, sourceID string) error { _, err := e.handler.DeleteObligationTrigger(ctx, sourceID) return err @@ -81,7 +99,7 @@ func (e *Executor) executePruneObligationTriggers(ctx context.Context, plans []* func executePruneItems[T prunePlanItem]( ctx context.Context, - executor *Executor, + executor *PruneExecutor, items []T, kind string, deleteSource pruneDeleteFunc[T], @@ -101,7 +119,6 @@ func executePruneItems[T prunePlanItem]( } item.setExecution(&ExecutionResult{ - RunID: executor.runID, Applied: true, }) } @@ -109,9 +126,8 @@ func executePruneItems[T prunePlanItem]( return nil } -func (e *Executor) recordPruneFailure(item prunePlanItem, err error) error { +func (e *PruneExecutor) recordPruneFailure(item prunePlanItem, err error) error { item.setExecution(&ExecutionResult{ - RunID: e.runID, Failure: err.Error(), }) return err diff --git a/otdfctl/migrations/namespacedpolicy/prune_execute_test.go b/otdfctl/migrations/namespacedpolicy/prune_execute_test.go index 1b0005a5c9..d836d11ccc 100644 --- a/otdfctl/migrations/namespacedpolicy/prune_execute_test.go +++ b/otdfctl/migrations/namespacedpolicy/prune_execute_test.go @@ -53,7 +53,7 @@ func TestExecutePruneDispatchesOnlyPlanScope(t *testing.T) { handler := &mockExecutorHandler{} plan := mixedPrunePlan(tt.scope) - executor, err := NewExecutor(handler) + executor, err := NewPruneExecutor(handler) require.NoError(t, err) err = executor.ExecutePrune(t.Context(), plan) @@ -80,7 +80,7 @@ func TestExecutePruneRecordsFailureAndStops(t *testing.T) { }, } - executor, err := NewExecutor(handler) + executor, err := NewPruneExecutor(handler) require.NoError(t, err) err = executor.ExecutePrune(t.Context(), plan) @@ -96,7 +96,7 @@ func TestExecutePruneRecordsFailureAndStops(t *testing.T) { func TestExecutePruneRequiresSingleScope(t *testing.T) { handler := &mockExecutorHandler{} - executor, err := NewExecutor(handler) + executor, err := NewPruneExecutor(handler) require.NoError(t, err) err = executor.ExecutePrune(t.Context(), &PrunePlan{}) diff --git a/otdfctl/migrations/namespacedpolicy/prune_planner.go b/otdfctl/migrations/namespacedpolicy/prune_planner.go index e97c45d14c..155d3cd4ff 100644 --- a/otdfctl/migrations/namespacedpolicy/prune_planner.go +++ b/otdfctl/migrations/namespacedpolicy/prune_planner.go @@ -28,7 +28,7 @@ var ( // Each prune item is classified as delete, blocked, or unresolved and carries // the migrated target context that justified that decision. type PrunePlanner struct { - planner *Planner + planner *MigrationPlanner scopes scopeSet } @@ -82,7 +82,7 @@ func NewPrunePlanner(handler PolicyClient, scopeCSV string, opts ...PruneOption) config.pageSize = defaultPlannerPageSize } - planner, err := NewPlanner(handler, scopeCSV, WithPageSize(config.pageSize)) + planner, err := NewMigrationPlanner(handler, scopeCSV, WithPageSize(config.pageSize)) if err != nil { return nil, err } diff --git a/otdfctl/migrations/namespacedpolicy/registered_resources_execute.go b/otdfctl/migrations/namespacedpolicy/registered_resources_execute.go index 37d2a58fa6..668ec3c81a 100644 --- a/otdfctl/migrations/namespacedpolicy/registered_resources_execute.go +++ b/otdfctl/migrations/namespacedpolicy/registered_resources_execute.go @@ -10,7 +10,7 @@ import ( "github.com/opentdf/platform/protocol/go/policy/registeredresources" ) -func (e *Executor) executeRegisteredResources(ctx context.Context, plans []*RegisteredResourcePlan) error { +func (e *MigrationExecutor) executeRegisteredResources(ctx context.Context, plans []*RegisteredResourcePlan) error { if len(plans) == 0 { return nil } @@ -32,7 +32,7 @@ func (e *Executor) executeRegisteredResources(ctx context.Context, plans []*Regi return nil } -func (e *Executor) executeRegisteredResourceTarget(ctx context.Context, plan *RegisteredResourcePlan, target *RegisteredResourceTargetPlan) error { +func (e *MigrationExecutor) executeRegisteredResourceTarget(ctx context.Context, plan *RegisteredResourcePlan, target *RegisteredResourceTargetPlan) error { //nolint:exhaustive // Registered-resource execution only handles create and already-migrated explicitly; all other statuses are unsupported. switch target.Status { case TargetStatusAlreadyMigrated: @@ -51,7 +51,7 @@ func (e *Executor) executeRegisteredResourceTarget(ctx context.Context, plan *Re } } -func (e *Executor) createRegisteredResourceTarget(ctx context.Context, plan *RegisteredResourcePlan, target *RegisteredResourceTargetPlan) error { +func (e *MigrationExecutor) createRegisteredResourceTarget(ctx context.Context, plan *RegisteredResourcePlan, target *RegisteredResourceTargetPlan) error { namespace := namespaceIdentifier(target.Namespace) if namespace == "" { return fmt.Errorf("%w: registered resource %q", ErrTargetNamespaceRequired, plan.Source.GetId()) @@ -65,33 +65,28 @@ func (e *Executor) createRegisteredResourceTarget(ctx context.Context, plan *Reg 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)) } if created == nil { target.Execution = &ExecutionResult{ - RunID: e.runID, Failure: ErrMissingCreatedTargetID.Error(), } return fmt.Errorf("%w: registered resource %q target %q", ErrMissingCreatedTargetID, plan.Source.GetId(), namespaceLabel(target.Namespace)) } if created.GetId() == "" { target.Execution = &ExecutionResult{ - RunID: e.runID, Failure: ErrMissingCreatedTargetID.Error(), } return fmt.Errorf("%w: registered resource %q target %q", ErrMissingCreatedTargetID, plan.Source.GetId(), namespaceLabel(target.Namespace)) } target.Execution = &ExecutionResult{ - RunID: e.runID, Applied: true, CreatedTargetID: created.GetId(), } @@ -106,7 +101,6 @@ func (e *Executor) createRegisteredResourceTarget(ctx context.Context, plan *Reg // values that already exist on the chosen parent RR. if existingID := existingValues[registeredResourceValueKey(valuePlan.Source.GetValue())]; existingID != "" { valuePlan.Execution = &ExecutionResult{ - RunID: e.runID, Applied: true, CreatedTargetID: existingID, } @@ -121,11 +115,10 @@ func (e *Executor) createRegisteredResourceTarget(ctx context.Context, plan *Reg return nil } -func (e *Executor) createRegisteredResourceValue(ctx context.Context, target *RegisteredResourceTargetPlan, valuePlan *RegisteredResourceValuePlan) error { +func (e *MigrationExecutor) createRegisteredResourceValue(ctx context.Context, target *RegisteredResourceTargetPlan, valuePlan *RegisteredResourceValuePlan) error { actionAttributeValues, err := e.registeredResourceActionAttributeValues(target.Namespace, valuePlan) if err != nil { valuePlan.Execution = &ExecutionResult{ - RunID: e.runID, Failure: err.Error(), } return fmt.Errorf("%w: build registered resource value %q action bindings for namespace %q", err, valuePlan.Source.GetId(), namespaceLabel(target.Namespace)) @@ -139,26 +132,22 @@ func (e *Executor) createRegisteredResourceValue(ctx context.Context, target *Re metadataForCreate( valuePlan.Source.GetId(), metadataLabels(valuePlan.Source.GetMetadata()), - e.runID, ), ) if err != nil { valuePlan.Execution = &ExecutionResult{ - RunID: e.runID, Failure: err.Error(), } return fmt.Errorf("%w: create registered resource value %q for resource %q in namespace %q", err, valuePlan.Source.GetId(), target.TargetID(), namespaceLabel(target.Namespace)) } if created.GetId() == "" { valuePlan.Execution = &ExecutionResult{ - RunID: e.runID, Failure: ErrMissingCreatedTargetID.Error(), } return fmt.Errorf("%w: registered resource value %q for target %q", ErrMissingCreatedTargetID, valuePlan.Source.GetId(), namespaceLabel(target.Namespace)) } valuePlan.Execution = &ExecutionResult{ - RunID: e.runID, Applied: true, CreatedTargetID: created.GetId(), } @@ -166,7 +155,7 @@ func (e *Executor) createRegisteredResourceValue(ctx context.Context, target *Re return nil } -func (e *Executor) registeredResourceActionAttributeValues(namespace *policy.Namespace, valuePlan *RegisteredResourceValuePlan) ([]*registeredresources.ActionAttributeValue, error) { +func (e *MigrationExecutor) registeredResourceActionAttributeValues(namespace *policy.Namespace, valuePlan *RegisteredResourceValuePlan) ([]*registeredresources.ActionAttributeValue, error) { if valuePlan == nil { return nil, nil } diff --git a/otdfctl/migrations/namespacedpolicy/registered_resources_execute_test.go b/otdfctl/migrations/namespacedpolicy/registered_resources_execute_test.go index be5133d8f2..b61afb0011 100644 --- a/otdfctl/migrations/namespacedpolicy/registered_resources_execute_test.go +++ b/otdfctl/migrations/namespacedpolicy/registered_resources_execute_test.go @@ -18,15 +18,14 @@ func TestExecuteRegisteredResources(t *testing.T) { tests := []struct { name string - plan *Plan + plan *MigrationPlan handler *mockExecutorHandler - runID string wantErr *expectedError - assert func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, plan *Plan) + assert func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) }{ { name: "creates registered resource shell and values with authoritative action target ids", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions, ScopeRegisteredResources}, Actions: []*ActionPlan{ { @@ -112,8 +111,7 @@ func TestExecuteRegisteredResources(t *testing.T) { }, }, }, - runID: "run-rr-123", - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -127,7 +125,6 @@ func TestExecuteRegisteredResources(t *testing.T) { assert.Equal(t, map[string]string{ "owner": "policy-team", migrationLabelMigratedFrom: "rr-1", - migrationLabelRun: "run-rr-123", }, resourceCall.Metadata.GetLabels()) require.Contains(t, handler.createdRegisteredResourceValues, "rrv-1") @@ -138,7 +135,6 @@ func TestExecuteRegisteredResources(t *testing.T) { assert.Equal(t, map[string]string{ "classification": "secret", migrationLabelMigratedFrom: "rrv-1", - migrationLabelRun: "run-rr-123", }, valueCall.Metadata.GetLabels()) require.Len(t, valueCall.ActionAttributeValues, 2) assert.Equal(t, "created-action-1", valueCall.ActionAttributeValues[0].GetActionId()) @@ -150,14 +146,12 @@ func TestExecuteRegisteredResources(t *testing.T) { require.NotNil(t, resourceTarget.Execution) assert.True(t, resourceTarget.Execution.Applied) assert.Equal(t, "created-rr-1", resourceTarget.Execution.CreatedTargetID) - assert.Equal(t, "run-rr-123", resourceTarget.Execution.RunID) assert.Equal(t, "created-rr-1", resourceTarget.TargetID()) valueTarget := plan.RegisteredResources[0].Target.Values[0] require.NotNil(t, valueTarget.Execution) assert.True(t, valueTarget.Execution.Applied) assert.Equal(t, "created-rrv-1", valueTarget.Execution.CreatedTargetID) - assert.Equal(t, "run-rr-123", valueTarget.Execution.RunID) assert.Equal(t, "created-rrv-1", valueTarget.TargetID()) assert.Equal(t, "created-action-1", executor.cachedActionTargetID("action-1", namespace1)) @@ -166,7 +160,7 @@ func TestExecuteRegisteredResources(t *testing.T) { }, { name: "skips already migrated registered resource targets", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions, ScopeRegisteredResources}, RegisteredResources: []*RegisteredResourcePlan{ { @@ -185,7 +179,7 @@ func TestExecuteRegisteredResources(t *testing.T) { }, }, handler: &mockExecutorHandler{}, - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -198,7 +192,7 @@ func TestExecuteRegisteredResources(t *testing.T) { }, { name: "skips skipped registered resource targets", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeRegisteredResources}, RegisteredResources: []*RegisteredResourcePlan{ { @@ -217,7 +211,7 @@ func TestExecuteRegisteredResources(t *testing.T) { }, }, handler: &mockExecutorHandler{}, - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -229,7 +223,7 @@ func TestExecuteRegisteredResources(t *testing.T) { }, { name: "ignores unresolved target status", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions, ScopeRegisteredResources}, RegisteredResources: []*RegisteredResourcePlan{ { @@ -243,7 +237,7 @@ func TestExecuteRegisteredResources(t *testing.T) { }, }, handler: &mockExecutorHandler{}, - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -253,7 +247,7 @@ func TestExecuteRegisteredResources(t *testing.T) { }, { name: "ignores unresolved registered resource entry without target", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeRegisteredResources}, RegisteredResources: []*RegisteredResourcePlan{ { @@ -263,7 +257,7 @@ func TestExecuteRegisteredResources(t *testing.T) { }, }, handler: &mockExecutorHandler{}, - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -273,7 +267,7 @@ func TestExecuteRegisteredResources(t *testing.T) { }, { name: "records shell creation failures on the target", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions, ScopeRegisteredResources}, RegisteredResources: []*RegisteredResourcePlan{ { @@ -293,7 +287,7 @@ func TestExecuteRegisteredResources(t *testing.T) { }, }, wantErr: wantError(errBoom, `create registered resource %q in namespace %q`, "rr-1", namespace1.GetFqn()), - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.Error(t, err) @@ -304,7 +298,7 @@ func TestExecuteRegisteredResources(t *testing.T) { }, { name: "stops when a registered resource value cannot resolve its migrated action target", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions, ScopeRegisteredResources}, RegisteredResources: []*RegisteredResourcePlan{ { @@ -342,7 +336,7 @@ func TestExecuteRegisteredResources(t *testing.T) { "rrv-1", namespace1.GetFqn(), ), - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.Error(t, err) @@ -356,7 +350,7 @@ func TestExecuteRegisteredResources(t *testing.T) { }, { name: "records value creation failures on the value target", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions, ScopeRegisteredResources}, Actions: []*ActionPlan{ { @@ -408,7 +402,7 @@ func TestExecuteRegisteredResources(t *testing.T) { }, }, wantErr: wantError(errBoom, `create registered resource value %q for resource %q in namespace %q`, "rrv-1", "created-rr-1", namespace1.GetFqn()), - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.Error(t, err) @@ -426,13 +420,10 @@ func TestExecuteRegisteredResources(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - executor, err := NewExecutor(tt.handler) + executor, err := NewMigrationExecutor(tt.handler) require.NoError(t, err) - if tt.runID != "" { - executor.runID = tt.runID - } - err = executor.Execute(t.Context(), tt.plan) + err = executor.ExecuteMigration(t.Context(), tt.plan) switch { case tt.wantErr != nil: require.Error(t, err) diff --git a/otdfctl/migrations/namespacedpolicy/subject_condition_sets_execute.go b/otdfctl/migrations/namespacedpolicy/subject_condition_sets_execute.go index b54dbd9817..3b02fddef4 100644 --- a/otdfctl/migrations/namespacedpolicy/subject_condition_sets_execute.go +++ b/otdfctl/migrations/namespacedpolicy/subject_condition_sets_execute.go @@ -7,7 +7,7 @@ import ( "github.com/opentdf/platform/protocol/go/policy" ) -func (e *Executor) rememberSubjectConditionSetTarget(sourceID string, target *SubjectConditionSetTargetPlan) { +func (e *MigrationExecutor) rememberSubjectConditionSetTarget(sourceID string, target *SubjectConditionSetTargetPlan) { if e == nil || sourceID == "" || target == nil { return } @@ -27,7 +27,7 @@ func (e *Executor) rememberSubjectConditionSetTarget(sourceID string, target *Su e.subjectConditionSets[sourceID][namespaceKey] = target } -func (e *Executor) cachedScsTargetID(sourceID string, namespace *policy.Namespace) string { +func (e *MigrationExecutor) cachedScsTargetID(sourceID string, namespace *policy.Namespace) string { if e == nil || sourceID == "" { return "" } @@ -50,7 +50,7 @@ func (e *Executor) cachedScsTargetID(sourceID string, namespace *policy.Namespac return target.TargetID() } -func (e *Executor) executeSubjectConditionSets(ctx context.Context, plans []*SubjectConditionSetPlan) error { +func (e *MigrationExecutor) executeSubjectConditionSets(ctx context.Context, plans []*SubjectConditionSetPlan) error { if len(plans) == 0 { return nil } @@ -74,7 +74,7 @@ func (e *Executor) executeSubjectConditionSets(ctx context.Context, plans []*Sub return nil } -func (e *Executor) executeSubjectConditionSetTarget(ctx context.Context, scsPlan *SubjectConditionSetPlan, target *SubjectConditionSetTargetPlan) error { +func (e *MigrationExecutor) executeSubjectConditionSetTarget(ctx context.Context, scsPlan *SubjectConditionSetPlan, target *SubjectConditionSetTargetPlan) error { //nolint:exhaustive // SCS execution only handles create and already-migrated explicitly; all other statuses are unsupported. switch target.Status { case TargetStatusAlreadyMigrated: @@ -94,7 +94,7 @@ func (e *Executor) executeSubjectConditionSetTarget(ctx context.Context, scsPlan } } -func (e *Executor) createSubjectConditionSetTarget(ctx context.Context, scsPlan *SubjectConditionSetPlan, target *SubjectConditionSetTargetPlan) error { +func (e *MigrationExecutor) createSubjectConditionSetTarget(ctx context.Context, scsPlan *SubjectConditionSetPlan, target *SubjectConditionSetTargetPlan) error { namespace := namespaceIdentifier(target.Namespace) if namespace == "" { return fmt.Errorf("%w: subject condition set %q", ErrTargetNamespaceRequired, scsPlan.Source.GetId()) @@ -106,27 +106,23 @@ func (e *Executor) createSubjectConditionSetTarget(ctx context.Context, scsPlan metadataForCreate( scsPlan.Source.GetId(), metadataLabels(scsPlan.Source.GetMetadata()), - e.runID, ), namespace, ) if err != nil { target.Execution = &ExecutionResult{ - RunID: e.runID, Failure: err.Error(), } return fmt.Errorf("create subject condition set %q in namespace %q: %w", scsPlan.Source.GetId(), namespaceLabel(target.Namespace), err) } if created.GetId() == "" { target.Execution = &ExecutionResult{ - RunID: e.runID, Failure: ErrMissingCreatedTargetID.Error(), } return fmt.Errorf("%w: subject condition set %q target %q", ErrMissingCreatedTargetID, scsPlan.Source.GetId(), namespaceLabel(target.Namespace)) } target.Execution = &ExecutionResult{ - RunID: e.runID, Applied: true, CreatedTargetID: created.GetId(), } diff --git a/otdfctl/migrations/namespacedpolicy/subject_condition_sets_execute_test.go b/otdfctl/migrations/namespacedpolicy/subject_condition_sets_execute_test.go index 795eecf78f..c5d967d597 100644 --- a/otdfctl/migrations/namespacedpolicy/subject_condition_sets_execute_test.go +++ b/otdfctl/migrations/namespacedpolicy/subject_condition_sets_execute_test.go @@ -35,15 +35,14 @@ func TestExecuteSubjectConditionSets(t *testing.T) { tests := []struct { name string - plan *Plan + plan *MigrationPlan handler *mockExecutorHandler - runID string wantErr *expectedError - assert func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, plan *Plan) + assert func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) }{ { name: "handles created and already migrated subject condition set targets", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeSubjectConditionSets}, SubjectConditionSets: []*SubjectConditionSetPlan{ { @@ -78,8 +77,7 @@ func TestExecuteSubjectConditionSets(t *testing.T) { }, }, }, - runID: "run-456", - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -92,7 +90,6 @@ func TestExecuteSubjectConditionSets(t *testing.T) { "owner": "policy-team", "env": "dev", migrationLabelMigratedFrom: "scs-1", - migrationLabelRun: "run-456", }, handler.createdSubjectConditions["scs-1"]["ns-1"].Metadata.GetLabels()) createdTarget := plan.SubjectConditionSets[0].Targets[0] @@ -101,7 +98,6 @@ func TestExecuteSubjectConditionSets(t *testing.T) { require.NotNil(t, createdTarget.Execution) assert.True(t, createdTarget.Execution.Applied) assert.Equal(t, "created-scs-1", createdTarget.Execution.CreatedTargetID) - assert.Equal(t, "run-456", createdTarget.Execution.RunID) assert.Equal(t, "created-scs-1", createdTarget.TargetID()) migratedTarget := plan.SubjectConditionSets[0].Targets[1] @@ -114,7 +110,7 @@ func TestExecuteSubjectConditionSets(t *testing.T) { }, { name: "ignores unresolved target status", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeSubjectConditionSets}, SubjectConditionSets: []*SubjectConditionSetPlan{ { @@ -130,7 +126,7 @@ func TestExecuteSubjectConditionSets(t *testing.T) { }, }, handler: &mockExecutorHandler{}, - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -139,7 +135,7 @@ func TestExecuteSubjectConditionSets(t *testing.T) { }, { name: "returns error for missing already migrated target id", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeSubjectConditionSets}, SubjectConditionSets: []*SubjectConditionSetPlan{ { @@ -155,7 +151,7 @@ func TestExecuteSubjectConditionSets(t *testing.T) { }, handler: &mockExecutorHandler{}, wantErr: wantError(ErrMissingMigratedTarget, `subject condition set %q target %q`, "scs-1", namespace1.GetFqn()), - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.Error(t, err) @@ -164,7 +160,7 @@ func TestExecuteSubjectConditionSets(t *testing.T) { }, { name: "returns error for missing target namespace", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeSubjectConditionSets}, SubjectConditionSets: []*SubjectConditionSetPlan{ { @@ -179,7 +175,7 @@ func TestExecuteSubjectConditionSets(t *testing.T) { }, handler: &mockExecutorHandler{}, wantErr: wantError(ErrTargetNamespaceRequired, `subject condition set %q`, "scs-1"), - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.Error(t, err) @@ -188,7 +184,7 @@ func TestExecuteSubjectConditionSets(t *testing.T) { }, { name: "returns error for missing created target id", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeSubjectConditionSets}, SubjectConditionSets: []*SubjectConditionSetPlan{ { @@ -210,7 +206,7 @@ func TestExecuteSubjectConditionSets(t *testing.T) { }, }, wantErr: wantError(ErrMissingCreatedTargetID, `subject condition set %q target %q`, "scs-1", namespace1.GetFqn()), - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.Error(t, err) @@ -222,7 +218,7 @@ func TestExecuteSubjectConditionSets(t *testing.T) { }, { name: "returns error for unsupported target status", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeSubjectConditionSets}, SubjectConditionSets: []*SubjectConditionSetPlan{ { @@ -244,7 +240,7 @@ func TestExecuteSubjectConditionSets(t *testing.T) { namespace1.GetFqn(), TargetStatus("bogus"), ), - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.Error(t, err) @@ -253,7 +249,7 @@ func TestExecuteSubjectConditionSets(t *testing.T) { }, { name: "records create failures on the target", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeSubjectConditionSets}, SubjectConditionSets: []*SubjectConditionSetPlan{ { @@ -278,7 +274,7 @@ func TestExecuteSubjectConditionSets(t *testing.T) { is: errBoom, message: `create subject condition set "scs-1" in namespace "https://example.com": boom`, }, - assert: func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.Error(t, err) @@ -293,13 +289,10 @@ func TestExecuteSubjectConditionSets(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - executor, err := NewExecutor(tt.handler) + executor, err := NewMigrationExecutor(tt.handler) require.NoError(t, err) - if tt.runID != "" { - executor.runID = tt.runID - } - err = executor.Execute(t.Context(), tt.plan) + err = executor.ExecuteMigration(t.Context(), tt.plan) switch { case tt.wantErr != nil: require.Error(t, err) diff --git a/otdfctl/migrations/namespacedpolicy/subject_mappings_execute.go b/otdfctl/migrations/namespacedpolicy/subject_mappings_execute.go index 5cc786ea61..30b7a5aa5e 100644 --- a/otdfctl/migrations/namespacedpolicy/subject_mappings_execute.go +++ b/otdfctl/migrations/namespacedpolicy/subject_mappings_execute.go @@ -7,7 +7,7 @@ import ( "github.com/opentdf/platform/protocol/go/policy" ) -func (e *Executor) executeSubjectMappings(ctx context.Context, plans []*SubjectMappingPlan) error { +func (e *MigrationExecutor) executeSubjectMappings(ctx context.Context, plans []*SubjectMappingPlan) error { if len(plans) == 0 { return nil } @@ -29,7 +29,7 @@ func (e *Executor) executeSubjectMappings(ctx context.Context, plans []*SubjectM return nil } -func (e *Executor) executeSubjectMappingTarget(ctx context.Context, mappingPlan *SubjectMappingPlan, target *SubjectMappingTargetPlan) error { +func (e *MigrationExecutor) executeSubjectMappingTarget(ctx context.Context, mappingPlan *SubjectMappingPlan, target *SubjectMappingTargetPlan) error { //nolint:exhaustive // Subject mapping execution only handles create and already-migrated explicitly; all other statuses are unsupported. switch target.Status { case TargetStatusAlreadyMigrated: @@ -48,7 +48,7 @@ func (e *Executor) executeSubjectMappingTarget(ctx context.Context, mappingPlan } } -func (e *Executor) createSubjectMappingTarget(ctx context.Context, mappingPlan *SubjectMappingPlan, target *SubjectMappingTargetPlan) error { +func (e *MigrationExecutor) createSubjectMappingTarget(ctx context.Context, mappingPlan *SubjectMappingPlan, target *SubjectMappingTargetPlan) error { namespace := namespaceIdentifier(target.Namespace) if namespace == "" { return fmt.Errorf("%w: subject mapping %q", ErrTargetNamespaceRequired, mappingPlan.Source.GetId()) @@ -78,27 +78,23 @@ func (e *Executor) createSubjectMappingTarget(ctx context.Context, mappingPlan * metadataForCreate( mappingPlan.Source.GetId(), metadataLabels(mappingPlan.Source.GetMetadata()), - e.runID, ), namespace, ) if err != nil { target.Execution = &ExecutionResult{ - RunID: e.runID, Failure: err.Error(), } return fmt.Errorf("create subject mapping %q in namespace %q: %w", mappingPlan.Source.GetId(), namespaceLabel(target.Namespace), err) } if created.GetId() == "" { target.Execution = &ExecutionResult{ - RunID: e.runID, Failure: ErrMissingCreatedTargetID.Error(), } return fmt.Errorf("%w: subject mapping %q target %q", ErrMissingCreatedTargetID, mappingPlan.Source.GetId(), namespaceLabel(target.Namespace)) } target.Execution = &ExecutionResult{ - RunID: e.runID, Applied: true, CreatedTargetID: created.GetId(), } @@ -106,7 +102,7 @@ func (e *Executor) createSubjectMappingTarget(ctx context.Context, mappingPlan * return nil } -func (e *Executor) resolveSubjectMappingActions(mappingPlan *SubjectMappingPlan, target *SubjectMappingTargetPlan) ([]*policy.Action, error) { +func (e *MigrationExecutor) resolveSubjectMappingActions(mappingPlan *SubjectMappingPlan, target *SubjectMappingTargetPlan) ([]*policy.Action, error) { actions := make([]*policy.Action, 0, len(target.ActionSourceIDs)) for _, sourceID := range target.ActionSourceIDs { if sourceID == "" { @@ -124,7 +120,7 @@ func (e *Executor) resolveSubjectMappingActions(mappingPlan *SubjectMappingPlan, return actions, nil } -func (e *Executor) resolveSubjectMappingSubjectConditionSet(mappingPlan *SubjectMappingPlan, target *SubjectMappingTargetPlan) (string, error) { +func (e *MigrationExecutor) resolveSubjectMappingSubjectConditionSet(mappingPlan *SubjectMappingPlan, target *SubjectMappingTargetPlan) (string, error) { if target.SubjectConditionSetSourceID == "" { return "", fmt.Errorf("%w: subject mapping %q target %q", ErrMissingSubjectConditionSetTarget, mappingPlan.Source.GetId(), namespaceLabel(target.Namespace)) } diff --git a/otdfctl/migrations/namespacedpolicy/subject_mappings_execute_test.go b/otdfctl/migrations/namespacedpolicy/subject_mappings_execute_test.go index d4f30874b2..a1718e734f 100644 --- a/otdfctl/migrations/namespacedpolicy/subject_mappings_execute_test.go +++ b/otdfctl/migrations/namespacedpolicy/subject_mappings_execute_test.go @@ -18,15 +18,14 @@ func TestExecuteSubjectMappings(t *testing.T) { tests := []struct { name string - plan *Plan + plan *MigrationPlan handler *mockExecutorHandler - runID string wantErr *expectedError - assert func(t *testing.T, err error, executor *Executor, handler *mockExecutorHandler, plan *Plan) + assert func(t *testing.T, err error, executor *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) }{ { name: "creates subject mappings with migrated action and scs ids", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions, ScopeSubjectConditionSets, ScopeSubjectMappings}, Actions: []*ActionPlan{ { @@ -92,8 +91,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, }, }, - runID: "run-789", - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -110,7 +108,6 @@ func TestExecuteSubjectMappings(t *testing.T) { "owner": "policy-team", "env": "dev", migrationLabelMigratedFrom: "mapping-1", - migrationLabelRun: "run-789", }, call.Metadata.GetLabels()) target := plan.SubjectMappings[0].Target @@ -119,13 +116,12 @@ func TestExecuteSubjectMappings(t *testing.T) { require.NotNil(t, target.Execution) assert.True(t, target.Execution.Applied) assert.Equal(t, "mapping-target-1", target.Execution.CreatedTargetID) - assert.Equal(t, "run-789", target.Execution.RunID) assert.Equal(t, "mapping-target-1", target.TargetID()) }, }, { name: "skips already migrated subject mapping targets", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeSubjectMappings}, SubjectMappings: []*SubjectMappingPlan{ { @@ -139,7 +135,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, }, handler: &mockExecutorHandler{}, - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -150,7 +146,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, { name: "ignores unresolved target status", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeSubjectMappings}, SubjectMappings: []*SubjectMappingPlan{ { @@ -164,7 +160,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, }, handler: &mockExecutorHandler{}, - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.NoError(t, err) @@ -173,7 +169,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, { name: "returns error for missing already migrated target id", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeSubjectMappings}, SubjectMappings: []*SubjectMappingPlan{ { @@ -187,7 +183,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, handler: &mockExecutorHandler{}, wantErr: wantError(ErrMissingMigratedTarget, `subject mapping %q target %q`, "mapping-1", namespace1.GetFqn()), - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.Error(t, err) @@ -196,7 +192,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, { name: "returns error for missing action target id", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeSubjectMappings}, SubjectMappings: []*SubjectMappingPlan{ { @@ -217,7 +213,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, handler: &mockExecutorHandler{}, wantErr: wantError(ErrMissingActionTarget, `subject mapping %q action %q target %q`, "mapping-1", "action-1", namespace1.GetFqn()), - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.Error(t, err) @@ -226,7 +222,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, { name: "returns error for missing scs target id", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions, ScopeSubjectMappings}, Actions: []*ActionPlan{ { @@ -259,7 +255,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, handler: &mockExecutorHandler{}, wantErr: wantError(ErrMissingSubjectConditionSetTarget, `subject mapping %q subject condition set %q target %q`, "mapping-1", "scs-1", namespace1.GetFqn()), - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.Error(t, err) @@ -268,7 +264,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, { name: "returns error for missing target namespace", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeSubjectMappings}, SubjectMappings: []*SubjectMappingPlan{ { @@ -286,7 +282,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, handler: &mockExecutorHandler{}, wantErr: wantError(ErrTargetNamespaceRequired, `subject mapping %q`, "mapping-1"), - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.Error(t, err) @@ -295,7 +291,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, { name: "returns error for missing created target id", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions, ScopeSubjectConditionSets, ScopeSubjectMappings}, Actions: []*ActionPlan{ { @@ -346,7 +342,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, }, wantErr: wantError(ErrMissingCreatedTargetID, `subject mapping %q target %q`, "mapping-1", namespace1.GetFqn()), - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.Error(t, err) @@ -357,7 +353,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, { name: "returns error for unsupported target status", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeSubjectMappings}, SubjectMappings: []*SubjectMappingPlan{ { @@ -377,7 +373,7 @@ func TestExecuteSubjectMappings(t *testing.T) { namespace1.GetFqn(), TargetStatus("bogus"), ), - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, _ *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, _ *MigrationPlan) { t.Helper() require.Error(t, err) @@ -386,7 +382,7 @@ func TestExecuteSubjectMappings(t *testing.T) { }, { name: "records create failures on the target", - plan: &Plan{ + plan: &MigrationPlan{ Scopes: []Scope{ScopeActions, ScopeSubjectConditionSets, ScopeSubjectMappings}, Actions: []*ActionPlan{ { @@ -440,7 +436,7 @@ func TestExecuteSubjectMappings(t *testing.T) { is: errBoom, message: `create subject mapping "mapping-1" in namespace "https://example.com": boom`, }, - assert: func(t *testing.T, err error, _ *Executor, handler *mockExecutorHandler, plan *Plan) { + assert: func(t *testing.T, err error, _ *MigrationExecutor, handler *mockExecutorHandler, plan *MigrationPlan) { t.Helper() require.Error(t, err) @@ -456,13 +452,10 @@ func TestExecuteSubjectMappings(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - executor, err := NewExecutor(tt.handler) + executor, err := NewMigrationExecutor(tt.handler) require.NoError(t, err) - if tt.runID != "" { - executor.runID = tt.runID - } - err = executor.Execute(t.Context(), tt.plan) + err = executor.ExecuteMigration(t.Context(), tt.plan) switch { case tt.wantErr != nil: require.Error(t, err)