diff --git a/otdfctl/docs/man/migrate/prune/namespaced-policy.md b/otdfctl/docs/man/migrate/prune/namespaced-policy.md index a61fcbcdf9..bf4aeff53b 100644 --- a/otdfctl/docs/man/migrate/prune/namespaced-policy.md +++ b/otdfctl/docs/man/migrate/prune/namespaced-policy.md @@ -33,8 +33,8 @@ The parent `migrate` command provides the shared `--commit` and `--interactive` An object is safe to delete only when prune can tie the legacy source object to the expected migrated target and prove the source is no longer needed. - `delete`: the source has the expected migrated target and prune found no remaining legacy dependency that still requires the source object. -- `blocked`: prune will not delete the source. Common reasons are that the source is still referenced by legacy policy or that the source object has not actually been migrated yet. -- `unresolved`: prune found something close to a migrated target, but it cannot prove the source and target match safely. Common reasons are missing or mismatched `migrated_from` labels, no matching labeled target, or a registered resource source that still contains values outside the resolved migration view. +- `blocked`: prune will not delete the source. Common reasons are that the source is still referenced by legacy policy, that the source object has not actually been migrated yet, or that an unmigrated registered resource spans multiple target namespaces and must be deleted manually. +- `unresolved`: prune found something close to a migrated target, but it cannot prove the source and target match safely. Common reasons are missing or mismatched `migrated_from` labels or no matching labeled target. In practice, prune relies on current legacy references plus `migrated_from` metadata on the namespaced targets. If that evidence is incomplete or inconsistent, the object is left in place instead of being deleted. @@ -63,10 +63,17 @@ otdfctl migrate prune namespaced-policy --scope=obligation-triggers --interactiv ## Other Information -1. Action / Subject-Condition-Set pruning +Action / Subject-Condition-Set pruning - 1a. Actions and subject-condition-sets are pruned a little differently from the other scopes. Instead of reusing the resolved migration view, prune classifies them directly from the current legacy objects, their current legacy references, and the canonical migrated targets it can find. +- Actions and subject-condition-sets are pruned a little differently from the other scopes. Instead of reusing the resolved migration view, prune classifies them directly from the current legacy objects, their current legacy references, and the canonical migrated targets it can find. +- They are expected to be pruned last. By the time you reach those scopes, their legacy dependents such as subject mappings, registered resources, and obligation triggers should already be gone, so the safest decision comes from checking the live legacy dependency graph at prune time. +- Some actions or subject-condition-sets that were never used by any other legacy policy object can still end up `blocked`. +- If prune cannot find a canonical migrated target for the source object, it leaves the source in place as `blocked` instead of assuming it is safe to delete. +- Example: if a custom action `decrypt` is no longer referenced by any legacy subject mapping, registered resource, or obligation trigger and prune finds a namespaced `decrypt` target with `metadata.labels.migrated_from=`, the source action is safe to delete. If no canonical namespaced `decrypt` target exists, the source action is reported as `blocked` because prune cannot prove that the object was actually migrated. - 1b. We do that because actions and subject-condition-sets are expected to be pruned last. By the time you reach those scopes, their legacy dependents such as subject mappings, registered resources, and obligation triggers should already be gone, so the safest decision comes from checking the live legacy dependency graph at prune time. That also means some actions or subject-condition-sets that were never used by any other legacy policy object can still end up `blocked`. +Registered-Resource manual deletion - 1c. If prune cannot find a canonical migrated target for the source object, it leaves the source in place as `blocked` instead of assuming it is safe to delete. For example, if a custom action `decrypt` is no longer referenced by any legacy subject mapping, registered resource, or obligation trigger and prune finds a namespaced `decrypt` target with `metadata.labels.migrated_from=`, the source action is safe to delete. If no canonical namespaced `decrypt` target exists, the source action is reported as `blocked` because prune cannot prove that the object was actually migrated. +- A legacy registered resource can only be auto-pruned when migration resolved it to one target namespace and prune can match it to the expected migrated target. +- If a registered resource spans multiple target namespaces and was never migrated, prune reports it as `blocked` with the `MultiNamespaceManualDelete` reason. +- In that case prune does not guess which namespace copy should have existed, and it does not delete the legacy source automatically. +- Clean this up manually after you verify the intended namespaced registered resources already exist and the legacy object is no longer needed. diff --git a/otdfctl/migrations/namespacedpolicy/prune_details.go b/otdfctl/migrations/namespacedpolicy/prune_details.go index ffdd469110..fc81d6c0b1 100644 --- a/otdfctl/migrations/namespacedpolicy/prune_details.go +++ b/otdfctl/migrations/namespacedpolicy/prune_details.go @@ -186,15 +186,11 @@ func (p *PruneSubjectMappingPlan) pruneDetails(styled bool, styles *migrations.D } func (p *PruneRegisteredResourcePlan) pruneDetails(styled bool, styles *migrations.DisplayStyles) []string { - details := []string{ + return []string{ formatPruneDetail("source_id", pruneIDValue(styled, styles, p.Source.GetId())), registeredResourceSourcePruneDetail("source", p.Source, styled, styles), targetRefPruneDetail("found_migrated_target", p.MigratedTarget, styled, styles), } - if !styled && p.Reason.Type == PruneStatusReasonTypeRegisteredResourceSourceMismatch { - details = append(details, formatPruneDetail("full_source", plainRegisteredResourceSourceSummary(p.FullSource))) - } - return details } func (p *PruneObligationTriggerPlan) pruneDetails(styled bool, styles *migrations.DisplayStyles) []string { diff --git a/otdfctl/migrations/namespacedpolicy/prune_execute_test.go b/otdfctl/migrations/namespacedpolicy/prune_execute_test.go index 40f76af64d..1b0005a5c9 100644 --- a/otdfctl/migrations/namespacedpolicy/prune_execute_test.go +++ b/otdfctl/migrations/namespacedpolicy/prune_execute_test.go @@ -173,24 +173,10 @@ func mixedPrunePlan(scope Scope) *PrunePlan { RegisteredResources: []*PruneRegisteredResourcePlan{ { Source: &policy.RegisteredResource{Id: "resource-delete-1"}, - FullSource: &policy.RegisteredResource{ - Id: "resource-delete-1", - Values: []*policy.RegisteredResourceValue{ - {Id: "value-1"}, - {Id: "value-2"}, - }, - }, Status: PruneStatusDelete, }, { Source: &policy.RegisteredResource{Id: "resource-delete-2"}, - FullSource: &policy.RegisteredResource{ - Id: "resource-delete-2", - Values: []*policy.RegisteredResourceValue{ - {Id: "value-3"}, - {Id: "value-4"}, - }, - }, Status: PruneStatusDelete, }, { diff --git a/otdfctl/migrations/namespacedpolicy/prune_plan.go b/otdfctl/migrations/namespacedpolicy/prune_plan.go index afbf18e87e..99dca8722c 100644 --- a/otdfctl/migrations/namespacedpolicy/prune_plan.go +++ b/otdfctl/migrations/namespacedpolicy/prune_plan.go @@ -20,23 +20,23 @@ const ( type PruneStatusReasonType string const ( - PruneStatusReasonTypeMigratedTargetNotFound PruneStatusReasonType = "MigratedTargetNotFound" - PruneStatusReasonTypeNoMatchingLabelsFound PruneStatusReasonType = "NoMatchingLabelsFound" - PruneStatusReasonTypeMismatchedMigrationLabel PruneStatusReasonType = "MismatchedMigrationLabel" - PruneStatusReasonTypeMissingMigrationLabel PruneStatusReasonType = "MissingMigrationLabel" - PruneStatusReasonTypeInUse PruneStatusReasonType = "InUse" - PruneStatusReasonTypeNeedsMigration PruneStatusReasonType = "NeedsMigration" - PruneStatusReasonTypeRegisteredResourceSourceMismatch PruneStatusReasonType = "RegisteredResourceSourceMismatch" - PruneStatusReasonTypeSkippedByUser PruneStatusReasonType = "SkippedByUser" - - pruneStatusReasonMessageMigratedTargetNotFound = "no migrated target was found for this source" - pruneStatusReasonMessageInUse = "source object is still referenced by legacy policy" - pruneStatusReasonMessageNoMatchingLabelsFound = "canonical migrated targets were found, but none carry migrated_from for this source" - pruneStatusReasonMessageMismatchedMigrationLabel = "migrated target carries migrated_from metadata for a different source" - pruneStatusReasonMessageMissingMigrationLabel = "migrated target is missing migrated_from metadata for this source" - pruneStatusReasonMessageNeedsMigration = "source object does not have a migrated target yet" - pruneStatusReasonMessageSkippedByUser = "skipped by user" - pruneStatusReasonMessageRegisteredResourceSourceMismatchFmt = "resolved registered resource view does not match the full source object for target namespace %q; source contains values outside the resolved migration view" + PruneStatusReasonTypeMigratedTargetNotFound PruneStatusReasonType = "MigratedTargetNotFound" + PruneStatusReasonTypeNoMatchingLabelsFound PruneStatusReasonType = "NoMatchingLabelsFound" + PruneStatusReasonTypeMismatchedMigrationLabel PruneStatusReasonType = "MismatchedMigrationLabel" + PruneStatusReasonTypeMissingMigrationLabel PruneStatusReasonType = "MissingMigrationLabel" + PruneStatusReasonTypeInUse PruneStatusReasonType = "InUse" + PruneStatusReasonTypeNeedsMigration PruneStatusReasonType = "NeedsMigration" + PruneStatusReasonTypeMultiNamespaceManualDelete PruneStatusReasonType = "MultiNamespaceManualDelete" + PruneStatusReasonTypeSkippedByUser PruneStatusReasonType = "SkippedByUser" + + pruneStatusReasonMessageMigratedTargetNotFound = "no migrated target was found for this source" + pruneStatusReasonMessageInUse = "source object is still referenced by legacy policy" + pruneStatusReasonMessageNoMatchingLabelsFound = "canonical migrated targets were found, but none carry migrated_from for this source" + pruneStatusReasonMessageMismatchedMigrationLabel = "migrated target carries migrated_from metadata for a different source" + pruneStatusReasonMessageMissingMigrationLabel = "migrated target is missing migrated_from metadata for this source" + pruneStatusReasonMessageNeedsMigration = "source object does not have a migrated target yet" + pruneStatusReasonMessageMultiNamespaceManualDelete = "registered resource spans multiple target namespaces and was not migrated; must be deleted manually" + pruneStatusReasonMessageSkippedByUser = "skipped by user" ) type PruneStatusReason struct { @@ -310,10 +310,8 @@ func (p *PruneSubjectMappingPlan) setExecution(execution *ExecutionResult) { // for deletion and the single migrated target RR matched to it by migration // metadata. type PruneRegisteredResourcePlan struct { - // Source is the resolved RR source from planning and may be filtered by interactive review. - Source *policy.RegisteredResource `json:"source"` - // FullSource is the authoritative RR source reloaded from the global namespace for prune verification. - FullSource *policy.RegisteredResource `json:"full_source,omitempty"` + // Source is the legacy RR source being considered for deletion. + Source *policy.RegisteredResource `json:"source"` Status PruneStatus `json:"status"` MigratedTarget TargetRef `json:"migrated_target,omitzero"` Reason PruneStatusReason `json:"reason,omitzero"` diff --git a/otdfctl/migrations/namespacedpolicy/prune_planner.go b/otdfctl/migrations/namespacedpolicy/prune_planner.go index 54949d0dbd..e97c45d14c 100644 --- a/otdfctl/migrations/namespacedpolicy/prune_planner.go +++ b/otdfctl/migrations/namespacedpolicy/prune_planner.go @@ -13,7 +13,6 @@ import ( var ( ErrMultiplePruneScopes = errors.New("prune planner accepts exactly one scope") ErrInvalidPruneResolvedTarget = errors.New("invalid prune resolved target") - ErrInvalidPruneResolvedSource = errors.New("invalid prune resolved source") ) // PrunePlanner classifies whether legacy policy objects can be deleted after @@ -35,7 +34,6 @@ type PrunePlanner struct { type prunePlannerConfig struct { pageSize int32 - reviewer InteractiveReviewer } type PruneOption func(*prunePlannerConfig) @@ -55,9 +53,9 @@ type pruneMigratedObject interface { } // NewPrunePlanner constructs a single-scope prune planner on top of the shared -// migration planner infrastructure. Interactive review is still supported for -// the resolved-object scopes, and direct-prune scopes reuse the same retriever -// and namespace discovery logic. +// migration planner infrastructure. Direct-prune scopes reuse the same +// retriever and namespace discovery logic, while resolved-object scopes reuse +// the resolver state without planner-time interactive review. func NewPrunePlanner(handler PolicyClient, scopeCSV string, opts ...PruneOption) (*PrunePlanner, error) { if handler == nil { return nil, ErrNilPlannerHandler @@ -84,12 +82,7 @@ func NewPrunePlanner(handler PolicyClient, scopeCSV string, opts ...PruneOption) config.pageSize = defaultPlannerPageSize } - plannerOpts := []Option{WithPageSize(config.pageSize)} - if config.reviewer != nil { - plannerOpts = append(plannerOpts, WithInteractiveReviewer(config.reviewer)) - } - - planner, err := NewPlanner(handler, scopeCSV, plannerOpts...) + planner, err := NewPlanner(handler, scopeCSV, WithPageSize(config.pageSize)) if err != nil { return nil, err } @@ -106,12 +99,6 @@ func WithPrunePageSize(pageSize int32) PruneOption { } } -func WithPruneInteractiveReviewer(reviewer InteractiveReviewer) PruneOption { - return func(config *prunePlannerConfig) { - config.reviewer = reviewer - } -} - // Plan produces a prune plan for the configured scope. // // Actions and subject condition sets bypass resolved migration output and are @@ -141,12 +128,7 @@ func (p *PrunePlanner) Plan(ctx context.Context) (*PrunePlan, error) { return nil, ErrNilResolvedTargets } - sourceRegisteredResources, err := p.sourceRegisteredResources(ctx) - if err != nil { - return nil, err - } - - return buildPrunePlanFromResolved(p.scopes, resolved, sourceRegisteredResources) + return buildPrunePlanFromResolved(p.scopes, resolved) } func (p *PrunePlanner) planActions(ctx context.Context) (*PrunePlan, error) { @@ -252,19 +234,6 @@ func (p *PrunePlanner) planSubjectConditionSets(ctx context.Context) (*PrunePlan return plan, nil } -func (p *PrunePlanner) sourceRegisteredResources(ctx context.Context) (map[string]*policy.RegisteredResource, error) { - if !p.scopes.has(ScopeRegisteredResources) { - return map[string]*policy.RegisteredResource{}, nil - } - - resources, err := p.planner.retriever.retrieveRegisteredResources(ctx) - if err != nil { - return nil, err - } - - return sourceRegisteredResourcesByID(resources), nil -} - func (p *PrunePlanner) usedLegacyActionsByID(ctx context.Context, sourceIDs map[string]struct{}) (map[string]struct{}, error) { used := make(map[string]struct{}, len(sourceIDs)) if len(sourceIDs) == 0 { @@ -351,26 +320,24 @@ func (p *PrunePlanner) usedLegacySubjectConditionSetsByID(ctx context.Context, s return used, nil } -func buildPrunePlanFromResolved(scopes scopeSet, resolved *ResolvedTargets, sourceRegisteredResources map[string]*policy.RegisteredResource) (*PrunePlan, error) { +func buildPrunePlanFromResolved(scopes scopeSet, resolved *ResolvedTargets) (*PrunePlan, error) { if resolved == nil { return &PrunePlan{}, nil } - builder := newPrunePlanBuilder(scopes, resolved, sourceRegisteredResources) + builder := newPrunePlanBuilder(scopes, resolved) return builder.build() } type prunePlanBuilder struct { - scopes scopeSet - resolved *ResolvedTargets - sourceRegisteredResources map[string]*policy.RegisteredResource + scopes scopeSet + resolved *ResolvedTargets } -func newPrunePlanBuilder(scopes scopeSet, resolved *ResolvedTargets, sourceRegisteredResources map[string]*policy.RegisteredResource) *prunePlanBuilder { +func newPrunePlanBuilder(scopes scopeSet, resolved *ResolvedTargets) *prunePlanBuilder { return &prunePlanBuilder{ - scopes: scopes, - resolved: resolved, - sourceRegisteredResources: sourceRegisteredResources, + scopes: scopes, + resolved: resolved, } } @@ -425,11 +392,10 @@ func (b *prunePlanBuilder) subjectMappings() ([]*PruneSubjectMappingPlan, error) return plans, nil } -// registeredResources verifies prune safety against the authoritative source RR. -// It first reloads the full source RR and marks the plan unresolved if the -// planner's resolved source is only a filtered view. For a full-source match, it -// then classifies the RR based on whether a migrated target exists and whether -// that target carries the expected migration metadata for the source RR. +// registeredResources classifies each resolved RR prune decision directly from +// the resolved migration state. Multi-namespace legacy RRs are blocked when no +// migrated target exists because the migration planner cannot determine a single +// target namespace for them and prune cannot safely auto-delete them. func (b *prunePlanBuilder) registeredResources() ([]*PruneRegisteredResourcePlan, error) { plans := make([]*PruneRegisteredResourcePlan, 0, len(b.resolved.RegisteredResources)) @@ -441,33 +407,26 @@ func (b *prunePlanBuilder) registeredResources() ([]*PruneRegisteredResourcePlan continue } - fullSource, err := b.registeredResourceSource(resource) - if err != nil { - return nil, err - } - - if !registeredResourceCanonicalEqual(resource.Source, fullSource) { + if pruneManualDeleteRequired(resource) { plans = append(plans, &PruneRegisteredResourcePlan{ Source: resource.Source, - FullSource: fullSource, - Status: PruneStatusUnresolved, + Status: PruneStatusBlocked, MigratedTarget: migratedTarget(resource.AlreadyMigrated, resource.Namespace), - Reason: newPruneReasonf( - PruneStatusReasonTypeRegisteredResourceSourceMismatch, - pruneStatusReasonMessageRegisteredResourceSourceMismatchFmt, - namespaceLabel(resource.Namespace)), + Reason: newPruneReason( + PruneStatusReasonTypeMultiNamespaceManualDelete, + pruneStatusReasonMessageMultiNamespaceManualDelete, + ), }) continue } - status, reason, err := pruneStatusForRegisteredResource(fullSource, resource.AlreadyMigrated) + status, reason, err := pruneStatusForRegisteredResource(resource.Source, resource.AlreadyMigrated) if err != nil { return nil, fmt.Errorf("registered resource %q: %w", resource.Source.GetId(), err) } plans = append(plans, &PruneRegisteredResourcePlan{ Source: resource.Source, - FullSource: fullSource, Status: status, MigratedTarget: migratedTarget(resource.AlreadyMigrated, resource.Namespace), Reason: reason, @@ -477,23 +436,11 @@ func (b *prunePlanBuilder) registeredResources() ([]*PruneRegisteredResourcePlan return plans, nil } -func (b *prunePlanBuilder) registeredResourceSource(resource *ResolvedRegisteredResource) (*policy.RegisteredResource, error) { - source := resource.Source - if b.sourceRegisteredResources == nil { - return nil, fmt.Errorf("%w: source registered resource verifier was not loaded", ErrInvalidPruneResolvedSource) - } - - sourceID := source.GetId() - if sourceID == "" { - return nil, fmt.Errorf("%w: registered resource source has empty id", ErrInvalidPruneResolvedSource) - } - - fullSource := b.sourceRegisteredResources[sourceID] - if fullSource == nil { - return nil, fmt.Errorf("%w: registered resource source %q not found during prune verification", ErrInvalidPruneResolvedSource, sourceID) - } - - return fullSource, nil +func pruneManualDeleteRequired(resource *ResolvedRegisteredResource) bool { + return resource != nil && + resource.AlreadyMigrated == nil && + resource.Unresolved != nil && + resource.Unresolved.Reason == UnresolvedReasonRegisteredResourceConflictingNamespaces } func (b *prunePlanBuilder) obligationTriggers() ([]*PruneObligationTriggerPlan, error) { @@ -519,17 +466,6 @@ func (b *prunePlanBuilder) obligationTriggers() ([]*PruneObligationTriggerPlan, return plans, nil } -func sourceRegisteredResourcesByID(resources []*policy.RegisteredResource) map[string]*policy.RegisteredResource { - byID := make(map[string]*policy.RegisteredResource, len(resources)) - for _, resource := range resources { - if resource == nil || resource.GetId() == "" { - continue - } - byID[resource.GetId()] = resource - } - return byID -} - func customLegacyActions(actions []*policy.Action) []*policy.Action { custom := make([]*policy.Action, 0, len(actions)) for _, action := range actions { @@ -699,10 +635,6 @@ func newPruneReason(reasonType PruneStatusReasonType, message string) PruneStatu } } -func newPruneReasonf(reasonType PruneStatusReasonType, format string, args ...any) PruneStatusReason { - return newPruneReason(reasonType, fmt.Sprintf(format, args...)) -} - func pruneStatusReasonForMigrationLabel(target pruneObject) PruneStatusReason { if migratedFromID(target) == "" { return newPruneReason(PruneStatusReasonTypeMissingMigrationLabel, pruneStatusReasonMessageMissingMigrationLabel) diff --git a/otdfctl/migrations/namespacedpolicy/prune_planner_test.go b/otdfctl/migrations/namespacedpolicy/prune_planner_test.go index 08d68b737d..fab92e804b 100644 --- a/otdfctl/migrations/namespacedpolicy/prune_planner_test.go +++ b/otdfctl/migrations/namespacedpolicy/prune_planner_test.go @@ -1,7 +1,6 @@ package namespacedpolicy import ( - "context" "testing" "github.com/opentdf/platform/protocol/go/common" @@ -700,7 +699,7 @@ func TestPrunePlannerPlanFailsWhenMigratedTargetIDIsEmpty(t *testing.T) { }, } - _, err := buildPrunePlanFromResolved(scopesFromSlice([]Scope{ScopeSubjectMappings}), resolved, nil) + _, err := buildPrunePlanFromResolved(scopesFromSlice([]Scope{ScopeSubjectMappings}), resolved) require.Error(t, err) require.ErrorIs(t, err, ErrInvalidPruneResolvedTarget) @@ -729,7 +728,7 @@ func TestPrunePlannerPlanClassifiesMismatchedMigrationLabelAsUnresolved(t *testi }, } - plan, err := buildPrunePlanFromResolved(scopesFromSlice([]Scope{ScopeSubjectMappings}), resolved, nil) + plan, err := buildPrunePlanFromResolved(scopesFromSlice([]Scope{ScopeSubjectMappings}), resolved) require.NoError(t, err) require.Len(t, plan.SubjectMappings, 1) @@ -749,7 +748,6 @@ func TestPrunePlannerPlanSkipsRegisteredResourceWhenResolvedSourceIsMissing(t *t plan, err := buildPrunePlanFromResolved( scopesFromSlice([]Scope{ScopeRegisteredResources}), resolved, - map[string]*policy.RegisteredResource{}, ) require.NoError(t, err) @@ -757,27 +755,6 @@ func TestPrunePlannerPlanSkipsRegisteredResourceWhenResolvedSourceIsMissing(t *t assert.Empty(t, plan.RegisteredResources) } -func TestPrunePlannerPlanFailsWhenRegisteredResourceSourceIsNotReloaded(t *testing.T) { - t.Parallel() - - source := testRegisteredResource("resource-1", "documents") - resolved := &ResolvedTargets{ - RegisteredResources: []*ResolvedRegisteredResource{ - {Source: source}, - }, - } - - _, err := buildPrunePlanFromResolved( - scopesFromSlice([]Scope{ScopeRegisteredResources}), - resolved, - map[string]*policy.RegisteredResource{}, - ) - - require.Error(t, err) - require.ErrorIs(t, err, ErrInvalidPruneResolvedSource) - assert.Contains(t, err.Error(), `registered resource source "resource-1" not found`) -} - func TestPrunePlannerPlanClassifiesUnmigratedRegisteredResourceAsNeedsMigration(t *testing.T) { t.Parallel() @@ -824,7 +801,7 @@ func TestPrunePlannerPlanClassifiesUnmigratedRegisteredResourceAsNeedsMigration( assert.True(t, plan.RegisteredResources[0].MigratedTarget.IsZero()) } -func TestPrunePlannerPlanMarksFilteredRegisteredResourceSourceAsUnresolved(t *testing.T) { +func TestPrunePlannerPlanBlocksUnmigratedMultiNamespaceRegisteredResourceForManualDelete(t *testing.T) { t.Parallel() leftNamespace := &policy.Namespace{Id: "ns-1", Fqn: "https://left.example.com"} @@ -847,11 +824,6 @@ func TestPrunePlannerPlanMarksFilteredRegisteredResourceSourceAsUnresolved(t *te ), ) legacyResource := testRegisteredResource("resource-1", "documents", leftValue, rightValue) - targetResource := &policy.RegisteredResource{ - Id: "target-resource-1", - Name: legacyResource.GetName(), - Metadata: migratedMetadata(legacyResource.GetId()), - } handler := &plannerTestHandler{ actionsByNamespace: map[string]*actions.ListActionsResponse{ "": { @@ -870,28 +842,21 @@ func TestPrunePlannerPlanMarksFilteredRegisteredResourceSourceAsUnresolved(t *te Pagination: emptyPageResponse(), }, } - reviewer := registeredResourceFilterReviewer{ - namespace: leftNamespace, - target: targetResource, - } - - planner, err := NewPrunePlanner(handler, "registered-resources", WithPruneInteractiveReviewer(reviewer)) + planner, err := NewPrunePlanner(handler, "registered-resources") require.NoError(t, err) plan, err := planner.Plan(t.Context()) require.NoError(t, err) require.Len(t, plan.RegisteredResources, 1) - assert.Equal(t, PruneStatusUnresolved, plan.RegisteredResources[0].Status) - assertPruneMigratedTarget(t, plan.RegisteredResources[0].MigratedTarget, leftNamespace, targetResource.GetId()) - assert.Equal(t, PruneStatusReasonTypeRegisteredResourceSourceMismatch, plan.RegisteredResources[0].Reason.Type) - assert.Contains(t, plan.RegisteredResources[0].Reason.Message, "resolved registered resource view does not match the full source object") - assert.Contains(t, plan.RegisteredResources[0].Reason.Message, leftNamespace.GetFqn()) - require.Len(t, plan.RegisteredResources[0].Source.GetValues(), 1) - require.Len(t, plan.RegisteredResources[0].FullSource.GetValues(), 2) + assert.Equal(t, PruneStatusBlocked, plan.RegisteredResources[0].Status) + assert.True(t, plan.RegisteredResources[0].MigratedTarget.IsZero()) + assert.Equal(t, PruneStatusReasonTypeMultiNamespaceManualDelete, plan.RegisteredResources[0].Reason.Type) + assert.Equal(t, pruneStatusReasonMessageMultiNamespaceManualDelete, plan.RegisteredResources[0].Reason.Message) + require.Len(t, plan.RegisteredResources[0].Source.GetValues(), 2) } -func TestPrunePlannerPlanDeletesRegisteredResourceWhenFullSourceMatchesMigratedTarget(t *testing.T) { +func TestPrunePlannerPlanDeletesRegisteredResourceWhenMigratedTargetMatches(t *testing.T) { t.Parallel() targetNamespace := &policy.Namespace{Id: "ns-1", Fqn: "https://example.com"} @@ -953,8 +918,6 @@ func TestPrunePlannerPlanDeletesRegisteredResourceWhenFullSourceMatchesMigratedT assertPruneMigratedTarget(t, plan.RegisteredResources[0].MigratedTarget, targetNamespace, targetResource.GetId()) assert.True(t, plan.RegisteredResources[0].Reason.IsZero()) require.NotNil(t, plan.RegisteredResources[0].Source) - require.NotNil(t, plan.RegisteredResources[0].FullSource) - assert.True(t, registeredResourceCanonicalEqual(plan.RegisteredResources[0].Source, plan.RegisteredResources[0].FullSource)) } // Scope: obligation triggers. @@ -1193,28 +1156,3 @@ func assertPruneMigratedTarget(t *testing.T, actual TargetRef, namespace *policy assert.Equal(t, namespace.GetId(), actual.NamespaceID) assert.Equal(t, namespace.GetFqn(), actual.NamespaceFQN) } - -type registeredResourceFilterReviewer struct { - namespace *policy.Namespace - target *policy.RegisteredResource -} - -func (r registeredResourceFilterReviewer) Review(_ context.Context, resolved *ResolvedTargets, _ []*policy.Namespace) error { - for _, resource := range resolved.RegisteredResources { - if resource == nil || resource.Source == nil { - continue - } - - filtered, err := filterRegisteredResourceToNamespace(resource.Source, r.namespace) - if err != nil { - return err - } - resource.Source = filtered - resource.Namespace = r.namespace - resource.Unresolved = nil - resource.AlreadyMigrated = r.target - resource.NeedsCreate = false - } - - return nil -} diff --git a/otdfctl/migrations/namespacedpolicy/prune_review_test.go b/otdfctl/migrations/namespacedpolicy/prune_review_test.go index 5891addb40..358a348768 100644 --- a/otdfctl/migrations/namespacedpolicy/prune_review_test.go +++ b/otdfctl/migrations/namespacedpolicy/prune_review_test.go @@ -1,7 +1,6 @@ package namespacedpolicy import ( - "fmt" "testing" "github.com/opentdf/platform/protocol/go/policy" @@ -163,55 +162,6 @@ func TestReviewPrunePlanSubjectMappingPromptIncludesActionNames(t *testing.T) { }, prompter.lastSelectPrompt.Description) } -func TestReviewPrunePlanRegisteredResourceMismatchPromptShowsFilteredAndFullSource(t *testing.T) { - t.Parallel() - - secretValue := testAttributeValue("https://example.com/attr/classification/value/secret", testNamespace("https://example.com")) - filteredSource := testRegisteredResource( - "resource-1", - "dataset", - testRegisteredResourceValue("prod", testActionAttributeValue("action-1", "read", secretValue)), - ) - fullSource := testRegisteredResource( - "resource-1", - "dataset", - testRegisteredResourceValue("prod", testActionAttributeValue("action-1", "read", secretValue)), - testRegisteredResourceValue("dev", testActionAttributeValue("action-2", "write", secretValue)), - ) - plan := &PrunePlan{ - RegisteredResources: []*PruneRegisteredResourcePlan{ - { - Source: filteredSource, - FullSource: fullSource, - Status: PruneStatusUnresolved, - MigratedTarget: TargetRef{ - ID: "target-resource-1", - NamespaceFQN: "https://example.com", - }, - Reason: newPruneReason( - PruneStatusReasonTypeRegisteredResourceSourceMismatch, - fmt.Sprintf(pruneStatusReasonMessageRegisteredResourceSourceMismatchFmt, "https://example.com"), - ), - }, - }, - } - prompter := &testInteractivePrompter{selectValue: pruneReviewSkip} - - err := ReviewPrunePlan(t.Context(), plan, prompter) - require.NoError(t, err) - - require.Equal(t, 1, prompter.selectCalls) - require.NotNil(t, prompter.lastSelectPrompt) - assert.Equal(t, `Delete unresolved registered resource "dataset"?`, prompter.lastSelectPrompt.Title) - assert.Equal(t, []string{ - "source_id=resource-1", - `source=values="prod" (action_bindings="read" -> https://example.com/attr/classification/value/secret)`, - `found_migrated_target=id: "target-resource-1" namespace: "https://example.com"`, - `full_source=values="prod", "dev" (action_bindings="read" -> https://example.com/attr/classification/value/secret, "write" -> https://example.com/attr/classification/value/secret)`, - `reason=RegisteredResourceSourceMismatch: resolved registered resource view does not match the full source object for target namespace "https://example.com"; source contains values outside the resolved migration view`, - }, prompter.lastSelectPrompt.Description) -} - func TestReviewPrunePlanObligationTriggerPromptIncludesTriggerContext(t *testing.T) { t.Parallel() diff --git a/otdfctl/migrations/namespacedpolicy/prune_summary_test.go b/otdfctl/migrations/namespacedpolicy/prune_summary_test.go index 73f70cd608..308a4e7dc8 100644 --- a/otdfctl/migrations/namespacedpolicy/prune_summary_test.go +++ b/otdfctl/migrations/namespacedpolicy/prune_summary_test.go @@ -311,27 +311,18 @@ func TestRenderNamespacedPolicyPruneSummaryRegisteredResourcesCoversEveryStatus( }) } -func TestRenderNamespacedPolicyPruneSummaryRegisteredResourceMismatchShowsSourceOnly(t *testing.T) { +func TestRenderNamespacedPolicyPruneSummaryRegisteredResourceManualDeleteReason(t *testing.T) { t.Parallel() - filteredSource := pruneSummaryRegisteredResource("resource-1", "dataset", "read") - fullSource := testRegisteredResource( - "resource-1", - "dataset", - testRegisteredResourceValue("prod", testActionAttributeValue("action-read", "read", pruneSummaryAttributeValue())), - testRegisteredResourceValue("dev", testActionAttributeValue("action-write", "write", pruneSummaryAttributeValue())), - ) plan := &PrunePlan{ Scopes: []Scope{ScopeRegisteredResources}, RegisteredResources: []*PruneRegisteredResourcePlan{ { - Source: filteredSource, - FullSource: fullSource, - Status: PruneStatusUnresolved, - MigratedTarget: pruneSummaryTarget("target-resource-1"), + Source: pruneSummaryRegisteredResource("resource-1", "dataset", "read"), + Status: PruneStatusBlocked, Reason: newPruneReason( - PruneStatusReasonTypeRegisteredResourceSourceMismatch, - "source mismatch", + PruneStatusReasonTypeMultiNamespaceManualDelete, + pruneStatusReasonMessageMultiNamespaceManualDelete, ), }, }, @@ -339,10 +330,8 @@ func TestRenderNamespacedPolicyPruneSummaryRegisteredResourceMismatchShowsSource summary := stripANSI(RenderNamespacedPolicyPruneSummary(plan, false, PruneSummaryResultSuccess)) - assert.Contains(t, summary, `registered resource "dataset" (source_id=resource-1, source=values="prod" (action_bindings="read" -> https://example.com/attr/classification/value/secret), found_migrated_target=id: "target-resource-1" namespace: "https://example.com"): reason=RegisteredResourceSourceMismatch: source mismatch`) - assert.NotContains(t, summary, "filtered_source=") + assert.Contains(t, summary, `registered resource "dataset" (source_id=resource-1, source=values="prod" (action_bindings="read" -> https://example.com/attr/classification/value/secret), found_migrated_target=(none)): reason=MultiNamespaceManualDelete: registered resource spans multiple target namespaces and was not migrated; must be deleted manually`) assert.NotContains(t, summary, "full_source=") - assert.NotContains(t, summary, `"dev"`) } func TestRenderNamespacedPolicyPruneSummaryObligationTriggersCoversEveryStatus(t *testing.T) {