Skip to content

fix(cli): Pull namespace for OTs from attr val #3725

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
c-r33d merged 2 commits into from
Jul 6, 2026
Merged

fix(cli): Pull namespace for OTs from attr val #3725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4c75af
fix(cli): Namespace migration too restrictive for OTs.
c-r33d Jul 6, 2026
a26f4c6
fix
c-r33d Jul 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions otdfctl/e2e/migrate-namespaced-policy.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1911,6 +1911,7 @@ teardown_file() {
local obligation_b_value="${TEST_PREFIX}-notify-b-default"
local trigger_a_client_id="${TEST_PREFIX}-client-a"
local trigger_b_client_id="${TEST_PREFIX}-client-b"
local trigger_attr_b_obligation_a_client_id="${TEST_PREFIX}-client-attr-b-obligation-a"
local custom_action_labels=(--label "test_case=obligation-triggers" --label "fixture=${TEST_PREFIX}-custom-action")
local trigger_a_labels=(--label "test_case=obligation-triggers" --label "fixture=${TEST_PREFIX}-trigger-a")
local custom_action_id
Expand All @@ -1920,6 +1921,7 @@ teardown_file() {
local obligation_b_value_id
local trigger_a_id
local trigger_b_id
local trigger_attr_b_obligation_a_id
local ns_b_read_action_id
local existing_trigger_b_id
local ns_a_state_before
Expand All @@ -1935,6 +1937,7 @@ teardown_file() {

create_legacy_obligation_trigger trigger_a_id "$ATTR_A_VAL_1_ID" "$custom_action_id" "$obligation_a_value_id" --client-id "$trigger_a_client_id" "${trigger_a_labels[@]}"
create_legacy_obligation_trigger trigger_b_id "$ATTR_B_VAL_1_ID" "$GLOBAL_READ_ID" "$obligation_b_value_id" --client-id "$trigger_b_client_id" --label "test_case=obligation-triggers" --label "fixture=${TEST_PREFIX}-trigger-b"
create_legacy_obligation_trigger trigger_attr_b_obligation_a_id "$ATTR_B_VAL_1_ID" "$custom_action_id" "$obligation_a_value_id" --client-id "$trigger_attr_b_obligation_a_client_id" --label "test_case=obligation-triggers" --label "fixture=${TEST_PREFIX}-trigger-attr-b-obligation-a"

lookup_namespaced_action_id ns_b_read_action_id "read" "$NS_B_ID"

Expand All @@ -1953,23 +1956,29 @@ teardown_file() {
ns_b_state_after=$(namespace_state_json "$NS_B_ID")

assert_namespace_state_delta "$ns_a_state_before" "$ns_a_state_after" 1 0 0 0 1
assert_namespace_state_delta "$ns_b_state_before" "$ns_b_state_after" 0 0 0 0 0
assert_namespace_state_delta "$ns_b_state_before" "$ns_b_state_after" 1 0 0 0 1

assert_obligation_trigger_created_in_namespace "$trigger_a_id" "$NS_A_ID" "$ATTR_A_VAL_1_ID" "$obligation_a_value_id" "$custom_action_name" "$custom_action_id" "create" "$trigger_a_client_id"
assert_obligation_trigger_created_in_namespace "$trigger_attr_b_obligation_a_id" "$NS_B_ID" "$ATTR_B_VAL_1_ID" "$obligation_a_value_id" "$custom_action_name" "$custom_action_id" "create" "$trigger_attr_b_obligation_a_client_id"

assert_obligation_trigger_already_migrated_in_namespace "$trigger_b_id" "$NS_B_ID" "$existing_trigger_b_id"
assert_standard_action_resolved_in_namespace "read" "$NS_B_ID"

assert_legacy_obligation_trigger_still_exists "$trigger_a_id" "$NS_A_ID" "$ATTR_A_VAL_1_ID" "$custom_action_id" "$obligation_a_value_id" "$trigger_a_client_id"
assert_legacy_obligation_trigger_still_exists "$trigger_b_id" "$NS_B_ID" "$ATTR_B_VAL_1_ID" "$GLOBAL_READ_ID" "$obligation_b_value_id" "$trigger_b_client_id"
assert_legacy_obligation_trigger_still_exists "$trigger_attr_b_obligation_a_id" "$NS_B_ID" "$ATTR_B_VAL_1_ID" "$custom_action_id" "$obligation_a_value_id" "$trigger_attr_b_obligation_a_client_id"

# Re-running the same migration should be idempotent. The previously created
# custom action target and trigger target should resolve as already_migrated,
# while the pre-existing canonical trigger remains already_migrated.
local custom_action_target_id
local custom_action_target_a_id
local custom_action_target_b_id
local trigger_a_target_id
custom_action_target_id=$(action_id_by_name_in_namespace "$custom_action_name" "$NS_A_ID")
local trigger_attr_b_obligation_a_target_id
custom_action_target_a_id=$(action_id_by_name_in_namespace "$custom_action_name" "$NS_A_ID")
custom_action_target_b_id=$(action_id_by_name_in_namespace "$custom_action_name" "$NS_B_ID")
trigger_a_target_id=$(obligation_trigger_id_by_migrated_from "$NS_A_ID" "$trigger_a_id")
trigger_attr_b_obligation_a_target_id=$(obligation_trigger_id_by_migrated_from "$NS_B_ID" "$trigger_attr_b_obligation_a_id")

ns_a_state_before="$ns_a_state_after"
ns_b_state_before="$ns_b_state_after"
Expand All @@ -1983,9 +1992,11 @@ teardown_file() {
assert_namespace_state_delta "$ns_a_state_before" "$ns_a_state_after" 0 0 0 0 0
assert_namespace_state_delta "$ns_b_state_before" "$ns_b_state_after" 0 0 0 0 0

assert_action_already_migrated_in_namespace "$custom_action_name" "$NS_A_ID" "$custom_action_target_id"
assert_action_already_migrated_in_namespace "$custom_action_name" "$NS_A_ID" "$custom_action_target_a_id"
assert_action_already_migrated_in_namespace "$custom_action_name" "$NS_B_ID" "$custom_action_target_b_id"
assert_standard_action_resolved_in_namespace "read" "$NS_B_ID"
assert_obligation_trigger_already_migrated_in_namespace "$trigger_a_id" "$NS_A_ID" "$trigger_a_target_id"
assert_obligation_trigger_already_migrated_in_namespace "$trigger_attr_b_obligation_a_id" "$NS_B_ID" "$trigger_attr_b_obligation_a_target_id"
assert_obligation_trigger_already_migrated_in_namespace "$trigger_b_id" "$NS_B_ID" "$existing_trigger_b_id"
}

Expand Down Expand Up @@ -2632,7 +2643,7 @@ teardown_file() {
create_global_action delete_a_action_id "$delete_a_action_name" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-delete-a-action"
create_global_action delete_b_action_id "$delete_b_action_name" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-delete-b-action"
create_namespaced_obligation delete_a_obligation_id "$NS_A_ID" "${TEST_PREFIX}-prune-trigger-delete-a" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-delete-a-obligation"
create_namespaced_obligation delete_b_obligation_id "$NS_A_ID" "${TEST_PREFIX}-prune-trigger-delete-b" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-delete-b-obligation"
create_namespaced_obligation delete_b_obligation_id "$NS_B_ID" "${TEST_PREFIX}-prune-trigger-delete-b" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-delete-b-obligation"
create_obligation_value delete_a_value_id "$delete_a_obligation_id" "${TEST_PREFIX}-delete-a-value" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-delete-a-value"
create_obligation_value delete_b_value_id "$delete_b_obligation_id" "${TEST_PREFIX}-delete-b-value" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-delete-b-value"
create_legacy_obligation_trigger delete_a_trigger_id "$ATTR_A_VAL_1_ID" "$delete_a_action_id" "$delete_a_value_id" --client-id "${TEST_PREFIX}-delete-a-client" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-delete-a-trigger"
Expand All @@ -2645,7 +2656,7 @@ teardown_file() {
delete_b_trigger_target_id=$(obligation_trigger_id_by_migrated_from "$NS_A_ID" "$delete_b_trigger_id")

create_global_action not_migrated_global_action_id "$not_migrated_global_action_name" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-not-migrated-action"
create_namespaced_obligation not_migrated_source_obligation_id "$NS_A_ID" "${TEST_PREFIX}-prune-trigger-not-migrated" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-not-migrated-obligation"
create_namespaced_obligation not_migrated_source_obligation_id "$NS_B_ID" "${TEST_PREFIX}-prune-trigger-not-migrated" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-not-migrated-obligation"
create_obligation_value not_migrated_source_value_id "$not_migrated_source_obligation_id" "${TEST_PREFIX}-not-migrated-value" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-not-migrated-value"
create_legacy_obligation_trigger not_migrated_source_trigger_id "$ATTR_A_VAL_1_ID" "$not_migrated_global_action_id" "$not_migrated_source_value_id" --client-id "${TEST_PREFIX}-not-migrated-client" --label "test_case=prune-obligation-triggers" --label "fixture=${TEST_PREFIX}-not-migrated-trigger"

Expand Down
2 changes: 1 addition & 1 deletion otdfctl/migrations/namespacedpolicy/derived.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func (d *targetDeriver) deriveRegisteredResource(resource *policy.RegisteredReso

func (d *targetDeriver) deriveObligationTrigger(trigger *policy.ObligationTrigger) (*DerivedObligationTrigger, error) {
item := &DerivedObligationTrigger{Source: trigger}
namespace, err := d.resolveNamespace(namespaceFromObligationValue(trigger.GetObligationValue()))
namespace, err := d.resolveNamespace(namespaceFromAttributeValue(trigger.GetAttributeValue()))
if err != nil {
return nil, fmt.Errorf("obligation trigger %q: %w", trigger.GetId(), err)
}
Expand Down
47 changes: 43 additions & 4 deletions otdfctl/migrations/namespacedpolicy/derived_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ func TestDeriveTargetsCollectsTargetsAndReferencesFromDependencies(t *testing.T)
},
ObligationTriggers: []*policy.ObligationTrigger{
{
Id: "trigger-1",
Action: &policy.Action{Id: "action-1", Name: "decrypt"},
Id: "trigger-1",
Action: &policy.Action{Id: "action-1", Name: "decrypt"},
AttributeValue: testAttributeValue("https://example.com/attr/classification/value/secret", namespace),
ObligationValue: &policy.ObligationValue{
Id: "ov-1",
Fqn: "https://example.com/obl/notify/value/email",
Expand Down Expand Up @@ -89,6 +90,43 @@ func TestDeriveTargetsCollectsTargetsAndReferencesFromDependencies(t *testing.T)
assert.Equal(t, namespace.GetId(), derived.ObligationTriggers[0].Target.GetId())
}

func TestDeriveTargetsDerivesObligationTriggerNamespaceFromAttributeValue(t *testing.T) {
t.Parallel()

attributeNamespace := &policy.Namespace{
Id: "ns-attribute",
Fqn: "https://attributes.example.com",
}
obligationNamespace := &policy.Namespace{
Id: "ns-obligation",
Fqn: "https://obligations.example.com",
}

derived, err := deriveTargets(
&Retrieved{
Scopes: []Scope{ScopeObligationTriggers},
Candidates: Candidates{
ObligationTriggers: []*policy.ObligationTrigger{
{
Id: "trigger-1",
Action: &policy.Action{Id: "action-1", Name: "decrypt"},
AttributeValue: testAttributeValue("https://attributes.example.com/attr/classification/value/secret", attributeNamespace),
ObligationValue: &policy.ObligationValue{
Id: "ov-1",
Fqn: "https://obligations.example.com/obl/notify/value/email",
Obligation: &policy.Obligation{Namespace: obligationNamespace},
},
},
},
},
},
[]*policy.Namespace{attributeNamespace, obligationNamespace},
)
require.NoError(t, err)
require.Len(t, derived.ObligationTriggers, 1)
assert.Equal(t, attributeNamespace.GetId(), derived.ObligationTriggers[0].Target.GetId())
}

func TestDeriveTargetsFailsWhenSubjectMappingNamespaceCannotBeDerived(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -230,8 +268,9 @@ func TestDeriveTargetsSkipsNilObligationTriggerCandidate(t *testing.T) {
ObligationTriggers: []*policy.ObligationTrigger{
nil,
{
Id: "trigger-1",
Action: &policy.Action{Id: "action-1", Name: "decrypt"},
Id: "trigger-1",
Action: &policy.Action{Id: "action-1", Name: "decrypt"},
AttributeValue: testAttributeValue("https://example.com/attr/classification/value/secret", namespace),
ObligationValue: &policy.ObligationValue{
Id: "ov-1",
Fqn: "https://example.com/obl/notify/value/email",
Expand Down
7 changes: 0 additions & 7 deletions otdfctl/migrations/namespacedpolicy/migration_plan.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,6 @@ func namespaceFromAttributeValue(value *policy.Value) *policy.Namespace {
}
}

func namespaceFromObligationValue(value *policy.ObligationValue) *policy.Namespace {
if value == nil {
return nil
}
return value.GetObligation().GetNamespace()
}

func hasRegisteredResourceActionAttributeValues(resource *policy.RegisteredResource) bool {
if resource == nil {
return false
Expand Down
10 changes: 6 additions & 4 deletions otdfctl/migrations/namespacedpolicy/migration_planner_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,9 @@ func TestPlannerRetrieveUsesRequestedScopeBoundaries(t *testing.T) {
),
)
legacyTrigger := &policy.ObligationTrigger{
Id: "trigger-1",
Action: &policy.Action{Id: legacyAction.GetId(), Name: legacyAction.GetName()},
Id: "trigger-1",
Action: &policy.Action{Id: legacyAction.GetId(), Name: legacyAction.GetName()},
AttributeValue: testAttributeValue("https://example.com/attr/classification/value/secret", targetNamespace),
ObligationValue: &policy.ObligationValue{
Id: "ov-1",
Fqn: "https://example.com/obl/notify/value/email",
Expand Down Expand Up @@ -425,8 +426,9 @@ func TestPlannerPlanAllScopesBuildsAllPlanSections(t *testing.T) {
),
)
legacyTrigger := &policy.ObligationTrigger{
Id: "trigger-1",
Action: &policy.Action{Id: legacyAction.GetId(), Name: legacyAction.GetName()},
Id: "trigger-1",
Action: &policy.Action{Id: legacyAction.GetId(), Name: legacyAction.GetName()},
AttributeValue: testAttributeValue("https://example.com/attr/classification/value/secret", targetNamespace),
ObligationValue: &policy.ObligationValue{
Id: "ov-1",
Fqn: "https://example.com/obl/notify/value/email",
Expand Down
Loading