design-proposal: source-IP restriction for externally published applications - #87
Draft
IvanHunters wants to merge 1 commit into
Draft
IvanHunters wants to merge 1 commit into
IvanHunters wants to merge 1 commit into
Conversation
…ations Adds sourceRanges beside external in each application's values, routed per-chart to the Service the chart renders, the operator CR field that already accepts it, or a nested chart's values. No CRD, no controller. The routing table is the proposal: sixteen rows for fourteen applications, because two publish two Services at once, two publish a variable number, and three choose the carrying object from a value other than external. Four rows have no usable route and say so. States the platform precondition the feature is dishonest without: loadBalancerSourceRanges filters the LoadBalancer VIP and not the node ports the same Service answers on. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A tenant sets
external: trueon a managed database, gets a public address, and cannot saywho may connect to it. There is no field for it on any application, and the object whose
name promises one —
SecurityGroup— cannot deliver: it selects pods and only widens abaseline that already admits
fromEntities: [world].This adds
sourceRangesbesideexternalin the application's own values. Each chartroutes it to whatever object already carries its external Service: the Service the chart
renders, the operator CR field that exists for this purpose, or the values of a nested
chart. No CRD, no controller, no label convention, no new RBAC.
The routing table in §4 is the proposal. Sixteen rows for fourteen applications, because
two publish two Services at once, two publish a variable number, and three choose the
carrying object from a value other than
external. Twelve rows have a route today; four donot and say so rather than accepting a value nothing enforces — openbao (its UI has a route
but its API does not, and one chart cannot refuse for one Service only), qdrant (upstream
offers
loadBalancerIPalone), rabbitmq (only via the override its own chart warnsagainst), vm-instance (served by cozy-proxy, which has no such field).
It leads with a platform precondition rather than the API, because the feature is
dishonest without it:
loadBalancerSourceRangesfilters the LoadBalancer VIP and not thenode ports the same Service answers on, verified on a live cluster. Without
bpf.lbSourceRangeAllTypes: truea "restricted" endpoint stays reachable on every node.Three consequences of that flag are stated, including that 1.19 removed the kill switch.
Deliberately not #29 again: that proposal was anchored to "the chart renders one additive
LoadBalancer Service per target", which its review found false for most engines. This
renders no Service and creates no object.
Known limits, all stated in the document rather than implied: in-cluster clients bypass the
filter entirely (Socket LB),
healthCheckNodePortis not covered and that touches six ofthe seven applications in the first wave, RobotLB clusters are unsupported and deliberately
not detected, and refusal from a genuinely external client has not been measured yet —
Phase 1 is that fixture and nothing ships before it.
Before review
DCO
git commit --signoff).