Skip to content
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.

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

refactor(policy): generalize route types in inbound index #12677

Conversation

the-wondersmith
Copy link
Contributor

Subject

Prepare to expand the subset of Gateway API route types linkerd supports driving outbound traffic with in linkerd-policy-controller-k8s-index.

Problem

Currently, the policy controller's index component is written with HTTPRoute support (effectively) exclusively, both in its structure/organization as well as its naming (e.g. HttpRoute as a primary type name, convert_http_route as a method name, etc...).

In order to expand the subset of route types defined by the Gateway API that linkerd supports for driving inbound traffic policy, the policy controller's inbound index component needs to be made more generic in both respects.

Solution

NOTE: PR is branched from the policy-refactor-core-grpc-generalize-route-types branch, so the diff will be appear larger than it actually is until after #12662 has been merged.

PR introduces structural and naming changes making the codebase generic with respect to the type of route being handled (e.g. HTTPRoute -> Route). Changes are almost entirely cosmetic but may still appear rather extreme at first glance. Even so, PR should not be introducing any meaningful behavioral changes in refactored functions and methods.

Validation

  • maintainer review
  • tests pass

Screenshot 2024-06-03 at 2 27 01 PM

Fixes Lays Groundwork For Addressing

Signed-off-by: Mark S <the@wondersmith.dev>
Signed-off-by: Mark S <the@wondersmith.dev>
Signed-off-by: Mark S <the@wondersmith.dev>
Signed-off-by: Mark S <the@wondersmith.dev>
Signed-off-by: Mark S <the@wondersmith.dev>
Signed-off-by: Mark S <the@wondersmith.dev>
Signed-off-by: Mark S <the@wondersmith.dev>
Signed-off-by: Mark S <the@wondersmith.dev>
@the-wondersmith the-wondersmith requested a review from a team as a code owner June 3, 2024 18:27
Signed-off-by: Mark S <the@wondersmith.dev>
…efactor-index-inbound-generalize-route-types

Signed-off-by: Mark S <the@wondersmith.dev>

# Conflicts:
#	policy-controller/grpc/src/inbound.rs
#	policy-controller/k8s/index/src/http_route.rs
#	policy-controller/k8s/index/src/inbound/index.rs
#	policy-controller/k8s/index/src/inbound/routes.rs
#	policy-controller/k8s/index/src/inbound/tests/authorization_policy.rs
#	policy-controller/k8s/index/src/outbound/index.rs
#	policy-controller/k8s/status/src/index.rs
#	policy-controller/k8s/status/src/resource_id.rs
#	policy-controller/k8s/status/src/tests/http_routes.rs
Signed-off-by: Mark S <the@wondersmith.dev>
Signed-off-by: Mark S <the@wondersmith.dev>
…efactor-index-inbound-generalize-route-types

Signed-off-by: Mark S <the@wondersmith.dev>

# Conflicts:
#	policy-controller/core/src/routes.rs
#	policy-controller/k8s/index/src/inbound/index.rs
#	policy-controller/k8s/index/src/inbound/routes.rs
#	policy-controller/k8s/index/src/inbound/tests/routes/http.rs
#	policy-controller/k8s/index/src/outbound/index.rs
Signed-off-by: Mark S <the@wondersmith.dev>
policy-controller/grpc/src/inbound.rs Outdated Show resolved Hide resolved
policy-controller/core/src/inbound.rs Outdated Show resolved Hide resolved
policy-controller/k8s/index/src/inbound/index.rs Outdated Show resolved Hide resolved
policy-controller/core/src/routes.rs Outdated Show resolved Hide resolved
policy-controller/k8s/index/src/inbound/routes.rs Outdated Show resolved Hide resolved
Signed-off-by: Mark S <the@wondersmith.dev>
Signed-off-by: Mark S <the@wondersmith.dev>
Signed-off-by: Mark S <the@wondersmith.dev>
@the-wondersmith the-wondersmith requested a review from adleong June 6, 2024 19:34
policy-controller/k8s/index/src/inbound/index.rs Outdated Show resolved Hide resolved
}
}

fn inbound_server<'p>(
&self,
name: String,
name: &str,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the purpose this change? String seems more honest than &str since we're going to to_string() it anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It saves pushes the clone to a single place (inside the function itself) instead of forcing callers to provide an owned value to begin with

Screenshot 2024-06-10 at 1 06 13 PM

policy-controller/k8s/index/src/inbound/index.rs Outdated Show resolved Hide resolved
…e type pattern

Signed-off-by: Mark S <the@wondersmith.dev>
@the-wondersmith the-wondersmith requested a review from adleong June 10, 2024 17:09
@adleong adleong closed this Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants