Skip to content

CLO-117: reject AWS PrivateLink SERVICE NAME that is not a service name - #38185

Open
jubrad wants to merge 4 commits into
MaterializeInc:mainfrom
jubrad:justin/clo-117-invalid-service-name-used-during-connection-reports
Open

CLO-117: reject AWS PrivateLink SERVICE NAME that is not a service name#38185
jubrad wants to merge 4 commits into
MaterializeInc:mainfrom
jubrad:justin/clo-117-invalid-service-name-used-during-connection-reports

Conversation

@jubrad

@jubrad jubrad commented Aug 13, 2026

Copy link
Copy Markdown
Member

Fixes CLO-117.

Motivation

A user created an AWS PrivateLink connection with an ELB DNS hostname in
SERVICE NAME instead of a VPC endpoint service name:

CREATE CONNECTION c TO AWS PRIVATELINK (
  SERVICE NAME 'my-db-lb-0123456789abcdef.elb.eu-central-1.amazonaws.com',
  AVAILABILITY ZONES ()
);
-- CREATE CONNECTION

CREATE CONNECTION accepted it, and the mistake only surfaced later as

Error: The Endpoint cannot be created due to missing availability zones

which points at the wrong option. The environment controller cannot parse that
value as a cross-region service name, so it treats the endpoint as same-region
and reports the empty AVAILABILITY ZONES list as the failure.

Changes

  • AwsPrivatelinkConnection::check_service_name rejects values that cannot be
    AWS VPC endpoint service names. It is deliberately permissive: it requires
    only the com.amazonaws. prefix, so both customer-owned
    (com.amazonaws.vpce.<region>.vpce-svc-<id>) and AWS-managed
    (com.amazonaws.<region>.<service>) services pass.
  • A new PlanError::InvalidPrivatelinkServiceName carries a hint that gives the
    expected shape and says the value is not the DNS name of the target.
  • CREATE CONNECTION and ALTER CONNECTION now fail up front with that error.
  • AwsPrivatelinkConnection::validate performs the same check before reading
    endpoint status, so VALIDATE CONNECTION on a connection that already stores
    an offending value reports the true cause instead of the availability zone
    message. This is the exact symptom in the report, and it also removes the need
    for a new VpcEndpointState variant in the cloud repo.
  • Docs: the SERVICE NAME syntax element now says what the value is and is not.

Why the sequencer and not the planner

The obvious home for this is plan_connection, next to the existing
AVAILABILITY ZONES validation. That is not safe. Catalog items are re-planned
on boot via CatalogState::deserialize_item, and a failure there panics
(src/adapter/src/catalog/apply.rs). Any environment that already stores an
offending service name would crash-loop on upgrade, which is precisely the
population this change is about. with_enable_for_item_parsing does not help
either, since it force-enables flags during item parsing, so a flag-gated
planner rule would still fire on boot.

The sequencer runs only for statements a client issues, so the rule cannot stop
an existing item from loading. For CREATE the check precedes id allocation,
secret creation, resource-limit validation, and the catalog transaction, so
nothing leaks before the rejection.

Two accepted behavior changes for environments that already store an offending
value:

  • CREATE CONNECTION IF NOT EXISTS with the offending name now errors instead
    of being a no-op. This matches how existing option validation already behaves
    (an invalid AVAILABILITY ZONES entry errors at plan time regardless of
    IF NOT EXISTS).
  • ALTER CONNECTION re-plans the full create_sql, so it reports the stored
    offending name even when the statement does not touch SERVICE NAME. Setting
    a valid SERVICE NAME in the same ALTER clears it.

Tips for reviewer

The permissiveness of the rule is the main thing to sanity check. It is
prefix-only on purpose. Worth confirming with the cloud team that no live
connection uses a service name outside com.amazonaws.* before this ships,
since such a connection would start failing VALIDATE CONNECTION and ALTER.

Checklist

  • This PR has adequate test coverage / QA involvement has been duly
    considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • This PR includes the following user-facing behavior changes:
    • CREATE CONNECTION ... TO AWS PRIVATELINK and ALTER CONNECTION now reject
      a SERVICE NAME that is not an AWS VPC endpoint service name, and
      VALIDATE CONNECTION reports that as the cause instead of a missing
      availability zone.

🤖 Generated with Claude Code

jubrad and others added 4 commits August 12, 2026 22:12
CLO-117. A `SERVICE NAME` that is not an AWS VPC endpoint service name, most
often the DNS name of the target such as a load balancer hostname, was accepted
by `CREATE CONNECTION` and only surfaced later as
"The Endpoint cannot be created due to missing availability zones". The
environment controller cannot parse such a value as a cross-region service
name, so it treats the endpoint as same-region and blames the availability
zone list, which points at the wrong option.

`AwsPrivatelinkConnection::check_service_name` now rejects values that cannot
be endpoint service names, and adds a hint that calls out the DNS name mistake
specifically. `VALIDATE CONNECTION` reports it too, so a connection that
already stores an offending value gets the true cause instead of the
availability zone message.

The check runs in the sequencer rather than the planner. The planner also runs
against the `create_sql` of items already in the catalog on boot, where a
failure panics, so a planner-level rule would turn an environment that already
stores an offending value into a crash loop.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`VALIDATE CONNECTION` on an AWS PrivateLink connection whose SERVICE NAME
cannot name a VPC endpoint service folded the hint into the error message,
because `AwsPrivatelinkConnection::validate` returned an `anyhow::Error` and
every such error lands in `ConnectionValidationError::Other`, whose `hint()`
is `None`.

Give the shape failure its own `ConnectionValidationError` variant, matching
how the other connection types carry a typed validation error. The hint now
travels in the pgwire HINT field, the same way it already does on the CREATE
path via `PlanError::hint`.

Part of CLO-117.
Drop the DNS-hostname heuristic. Varying the hint on whether the value
parses as a hostname needed a scheme/port/TLD parser and its own tests to
change one sentence. The hint now always says a service name is not the DNS
name of the target, which covers the mistake without the machinery.

Also trim the comments to what the code does not already say.
The hint and the `SERVICE NAME` docs already give the expected shape, which is
enough to tell a user their value is wrong.
@jubrad
jubrad marked this pull request as ready for review August 13, 2026 14:34
@jubrad
jubrad requested review from a team as code owners August 13, 2026 14:34
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.

1 participant