RFC0055 Identity-Aware Routing#3758
Conversation
|
Don't worry about this PR just yet, just doing some more POC work on the RFC: cloudfoundry/community#1438 |
a0ffff8 to
9409dbb
Compare
glad we all agree =) one nit: i think it will be much more fluent to have |
|
Issue: add-route-policy and remove-route-policy do not have matching flags. This works: There is no matching –source-app for the remove policy. |
|
I was testing all of this and I accidentally deployed without updating capi, which resulted in this error message: Is there a way to give a clearer error message? Maybe checking the capi version? Or saying something like "Route Policies are not supported in this version of CAPI"? |
|
@cppforlife — circling back on this since some time has passed.
The During RFC review, @ameowlia explicitly pushed for this feature to follow the network policy CLI pattern:
And @Gerg noted the family resemblance:
Using |
Fixed in 0ab3e02. The source flags ( |
|
Done in fba97d8 — here's what was added:
Failing test — Version checks added:
Let me know what version number to drop in once CAPI has it. |
2a42b90 to
4b6445b
Compare
This commit improves the user experience for the add-access-rule command by replacing the positional GUID-based SELECTOR argument with intuitive flags that accept human-readable names and support cross-space/org resolution. Changes: **Command Interface:** - Remove positional SELECTOR argument (breaking change, acceptable for unreleased feature) - Add new flags: --source-app, --source-space, --source-org, --source-any, --selector - Support hierarchical name resolution: - --source-app APP_NAME (looks in current space) - --source-app APP_NAME --source-space SPACE (cross-space in current org) - --source-app APP_NAME --source-space SPACE --source-org ORG (cross-org) - --source-space SPACE (space-level rule) - --source-org ORG (org-level rule) - --source-any (allow any authenticated app) - --selector SELECTOR (raw GUID-based selector for advanced users) - Validate exactly one primary source is specified - Display verbose output showing resolved selector for transparency **Terminology Update:** - Rename all "target" terminology to "source" throughout codebase - Access rules specify the source (who can access), not the target - Update AccessRuleWithRoute.TargetName → SourceName - Update resolveAccessRuleTarget() → resolveAccessRuleSource() - Update access-rules list command table header: "target" → "source" **Error Handling:** - Provide helpful error messages when app not found in current space - Suggest using --source-space and --source-org flags for cross-space/org access - Follow CF CLI patterns from add-network-policy command **Testing:** - Add 17 comprehensive test cases for add-access-rule command - Update 19 actor tests to use new SourceName field - All tests passing (36/36) **Domain Integration:** - Add enforce_access_rules support to create-shared-domain and create-private-domain - Add --enforce-access-rules and --access-rules-scope flags - Update domain resource with new fields Examples: # Simple case - app in current space cf add-access-rule allow-frontend apps.identity --source-app frontend-app --hostname backend # Cross-space access cf add-access-rule allow-other apps.identity --source-app api-client --source-space other-space --hostname backend # Cross-org access cf add-access-rule allow-prod apps.identity --source-app client --source-space prod-space --source-org prod-org --hostname api # Space-level rule cf add-access-rule allow-monitoring apps.identity --source-space monitoring --hostname api # Org-level rule cf add-access-rule allow-platform apps.identity --source-org platform --hostname shared-api # Any authenticated app cf add-access-rule allow-all apps.identity --source-any --hostname public-api Related to: cloudfoundry/community#1438
Per RFC commits 882b69a and 11752f2, access rules no longer have user-provided names. They are identified by their selector only, with labels/annotations used for metadata instead. Changes: - Removed RULE_NAME argument from add-access-rule command - Removed Name field from AccessRule API resource - Updated access-rules list to show 4 columns (route, selector, scope, source) - SourceName now represents resolved app/space/org name from selector - Updated remove-access-rule to use --selector flag instead of rule name - Renamed DeleteAccessRule() to DeleteAccessRuleBySelector() - Updated all tests to remove Name field references All tests passing.
…lumns Changed table format from: route selector scope source backend.apps.identity ... app frontend-app To: host domain path selector scope source backend apps.identity ... app frontend-app api apps.identity /metrics ... space monitoring This provides better clarity by separating the route components into individual columns, making it easier to scan and filter visually.
…urce Complete terminology shift for identity-aware routing RFC implementation: **Access Rules → Route Policies** - API: /v3/access_rules → /v3/route_policies - CLI commands: - cf access-rules → cf route-policies - cf add-access-rule → cf add-route-policy - cf remove-access-rule → cf remove-route-policy - Domain flags: --enforce-access-rules → --enforce-route-policies - Domain fields: enforce_access_rules → enforce_route_policies, access_rules_scope → route_policies_scope **Selector → Source** - API field: "selector" → "source" - CLI flag: --selector → --source - Query params: selectors → sources, selector_resource_guids → source_guids - Table column headers: "selector/source" → "source/name" - Internal types: AccessRule → RoutePolicy, AccessRuleWithRoute → RoutePolicyWithRoute - Error types: AccessRuleNotFoundError → RoutePolicyNotFoundError **Rationale (per RFC)** - "Route policies" aligns with existing CF "network policies" terminology - "Source" matches C2C network policy convention (source → destination) - Improves clarity: policies define allowed sources that can reach routes - Better mental model for users familiar with CF networking concepts This is a breaking change but acceptable since RFC is pre-GA with only POC/lab implementations. Clean terminology is preferred over backward compatibility at this stage. Co-authored-by: RFC Community <cloudfoundry/community#1438> Aligns-with: cloudfoundry/community@be8d74c1
Extract source resolution flags (--source-app, --source-space, --source-org, --source-any, --source) into a shared RoutePolicySourceFlags struct embedded in both add-route-policy and remove-route-policy commands. Previously remove-route-policy only accepted --source with a raw GUID-format value (cf:app:<guid>, etc.), while add-route-policy supported name-based resolution. The two commands now have matching flag sets.
Guard add-route-policy, remove-route-policy, and route-policies with an unconditional MinimumCCAPIVersionCheck against MinVersionRoutePolicies. Guard create-shared-domain and create-private-domain conditionally when --enforce-route-policies is passed. MinVersionRoutePolicies is currently a placeholder (3.999.0); a failing test in ccversion/minimum_version_test.go keeps the TODO visible until the real CAPI version is confirmed and the constant is updated.
Show a single 'route policies' column when the CAPI version supports it. The column is blank for plain domains, 'enforced' when enforcement is on with no scope, and 'enforced (org/space/any)' when a scope is set. The column is gated on MinVersionRoutePolicies so it silently disappears on older CAPI targets — no hard error, cf domains still works everywhere.
…sing When a user specifies --source-org with --source-app but omits --source-space, validateSourceFlags() previously passed (treating --source-app as the sole primary flag), and resolveSource() silently ignored --source-org, resolving the app in the currently targeted space. Add a pre-check in validateSourceFlags() that returns RequiredFlagsError (--source-org and --source-space must be used together) whenever --source-org is combined with --source-app but --source-space is absent.
4b6445b to
cc26579
Compare
…st.go Refactor CreatePrivateDomain describe block to use JustBeforeEach pattern and add Context block for enforceAccessRules=true with non-empty scope, mirroring the existing coverage in CreateSharedDomain.
…cyArgs Three identical one-field structs replaced with a single shared type. Description aligned with the existing convention in arguments.go.
…ivate-domain new flags - route_policy_source_flags_test.go: covers all validateSourceFlags branches (no flags, single flags, qualifier combinations, RequiredFlagsError, ArgumentCombinationError) and all resolveSource paths (raw --source, --source-any, --source-app with/without cross-space/org, --source-space, --source-org, error propagation from each actor call) - create_private_domain_command_test.go: adds coverage for --scope without --enforce-route-policies, invalid --scope values, API version check failure, --enforce-route-policies success (identity-aware TIP), and --scope forwarding
…red-domain test Mirrors the coverage added to create_private_domain_command_test.go: - --scope without --enforce-route-policies returns an error - invalid --scope value returns an error - --enforce-route-policies with old API version returns MinimumCFAPIVersionNotMetError - --enforce-route-policies success: identity-aware TIP, enforce=true passed to actor - --enforce-route-policies + --scope: scope forwarded to actor - default path now explicitly asserts enforce=false, scope empty
…viour Introduce EnforceRoutePoliciesBehavior and ItEnforcesRoutePolicies in enforce_route_policies_shared_test.go. Both create-shared-domain and create-private-domain tests now delegate the duplicate When blocks to the shared helper, parameterised only by TIPAdjective and the actor-specific arg-extraction closures. ccversion and translatableerror imports removed from both individual test files.
Bulk find-and-replace tooling from the rebrand commit introduced spaces→tabs indentation fixes and Invocations() mutex-lock removals in files completely unrelated to the route-policies feature. Restore all of them to origin/main to keep the feature diff focused.
| // version that introduces /v3/route_policies and the enforce_route_policies / | ||
| // route_policies_scope domain fields. Replace "3.999.0" with the real version | ||
| // once known. The test in minimum_version_test.go will keep failing until then. | ||
| MinVersionRoutePolicies = "3.999.0" |
There was a problem hiding this comment.
The version should be changed once: cloudfoundry/cloud_controller_ng#4910 has been merged and released.
|
This PR is ready for review. The specs are intentionally failing until the correct CAPI version has been filled in. But let's try and get this PR in a reviewed state, so the changes in this PR can get released closely after the CAPI changes get published. |
Summary
This PR implements the identity-aware routing RFC (RFC#1438) in the CF CLI. It provides the complete implementation from a prior POC, with two significant improvements:
add-network-policyconventions) instead of raw GUID selectorsTerminology Update (Breaking Change):
access-rules→route-policies,add-access-rule→add-route-policy,remove-access-rule→remove-route-policy--enforce-access-rules→--enforce-route-policies,--selector→--source/v3/access_rules→/v3/route_policiesAccessRule→RoutePolicyThis is an acceptable breaking change since the feature has not been GA-released.
Motivation
The original POC implementation used "access rules" terminology, but the RFC was updated to use "route policies" for better alignment with existing CF concepts. Additionally, the commands required users to manually construct GUID-based selectors (e.g.,
cf:app:d76446a1-f429-4444-8797-be2f78b75b08), which was cumbersome and error-prone.This PR follows CF CLI conventions from commands like
add-network-policyto provide a user-friendly interface with name-based flags.Changes
1. Terminology Rebrand
Complete rebrand across all affected files:
route-policies,add-route-policy,remove-route-policy--enforce-route-policies,--source,--source-app,--source-space,--source-org,--source-any/v3/route_policiessources,source_guidsRoutePolicy.Source,Domain.EnforceRoutePolicies,Domain.RoutePoliciesScopeRationale (per RFC commit be8d74c):
2. Command Interface Improvements
Before:
After:
3. New Flags
add-route-policy:
--source-app APP_NAME- Specify source app by name (resolves to GUID)--source-space SPACE_NAME- Specify space context for app lookup or create space-level policy--source-org ORG_NAME- Specify org context for space/app lookup or create org-level policy--source-any- Allow any authenticated app--source SOURCE- Raw GUID-based source for advanced usersremove-route-policy:
--source SOURCE- Required. Specify the source to removecreate-shared-domain / create-private-domain:
--enforce-route-policies- Require route policies for all routes on domain--route-policies-scope [app|space|org]- Minimum granularity for policies4. RFC Alignment: No User-Provided Names
Per RFC commits 882b69a and 11752f2, route policies no longer have user-provided names. They are identified by their source only, with labels/annotations available for metadata.
Key Changes:
RULE_NAMEpositional argument fromadd-route-policycommandNamefield from RoutePolicy API resourceroute-policieslist command to show 4 columns: route, source, scope, source/namesource/namecolumn shows the resolved name of the app/space/org from the sourceremove-route-policyto use--sourceflag instead of nameDeleteAccessRule()→DeleteRoutePolicyBySource()5. Enhanced Output
add-route-policy:
route-policies:
6. Validation & Error Handling
--source-spaceand--source-orgflags for cross-space/org accessadd-network-policycommand7. Code Quality
RoutePolicyArgsconsolidates the positional-argument struct that was independently defined foradd-route-policyandremove-route-policyinto a single shared type incommand/flag/arguments.goTesting
New and updated test files:
command/v7/add_route_policy_command_test.go— flags, validation, error handling, success pathcommand/v7/remove_route_policy_command_test.go— flags, validation, error handling, success pathcommand/v7/route_policies_command_test.go— listing, column outputcommand/v7/route_policy_source_flags_test.go— 23 specs covering all source flag validation and resolution paths (internal package tests, no generated fakes needed)command/v7/create_shared_domain_command_test.go— extended with--enforce-route-policies/--scopecoverage, CAPI version check, shared behaviour helpercommand/v7/create_private_domain_command_test.go— same coverage as shared domaincommand/v7/enforce_route_policies_shared_test.go— shared Ginkgo behaviour helper for domain command testsactor/v7action/route_policy_test.go— actor-layer testsapi/cloudcontroller/ccv3/route_policy_test.go— ccv3 client testsactor/v7action/domain_test.go— extended with scope/enforce contexts usingJustBeforeEachpatternresources/route_policy_resource_test.go— marshal/unmarshal tests for the resource typeCoverage: flag validation, source resolution, GUID lookup, error handling, cross-space/org scenarios, CAPI version checks, domain flags.
Breaking Changes
This is a complete rebrand with no backward compatibility:
This is acceptable since:
Files Changed
route_policy_resource.goroute_policy.go,api_routes.go,query.goroute_policy.go, error types, interfacesroute_policies_command.go,add_route_policy_command.go,remove_route_policy_command.gocreate_shared_domainandcreate_private_domainfake_cloud_controller_client.go,fake_actor.goregenerated for new methods only — bulkInvocations()cleanup is in PR Regenerate counterfeiter fakes with v6.12.2 #3799Checklist
Invocations()cleanup in separate PR Regenerate counterfeiter fakes with v6.12.2 #3799)Pending
Related
Co-authored-by: Ruben Koster rkoster@starkandwayne.com