Skip to content

Commit

Permalink
fix: inject direct response for sidecar acme-challenge
Browse files Browse the repository at this point in the history
When the sidecar is running in the cluster and no acme challenge
route is found then it will inject one. Previously, it would point
the injected route at an existing sidecar cluster.

When the cluster route names are larger than 60 characters they
are shortened and a new name is passed to envoy but the injected
route cluster was being populated by the IR.sidecar_cluster_name
which wasn't aware of the truncated cluster name. This would
cause diagd to produce invalid envoy configuration for the
injected acme challenge route.

The route will now be injected with an envoy direct_response
with a 404 status code. We return a 404 because in the proper
acme flow of the sidecar it is already handled in the AuthService
and this matches the existing behavior.

Signed-off-by: Lance Austin <[email protected]>
  • Loading branch information
Lance Austin committed Jul 29, 2022
1 parent 7fa0c7b commit 9bfc82f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 19 deletions.
26 changes: 8 additions & 18 deletions python/ambassador/envoy/v3/v3listener.py
Original file line number Diff line number Diff line change
Expand Up @@ -868,33 +868,23 @@ def compute_routes(self) -> None:

# If we're on Edge Stack and we don't already have an ACME route, add one.
if self.config.ir.edge_stack_allowed and not found_acme:
# The target cluster doesn't actually matter -- the auth service grabs the
# challenge and does the right thing. But we do need a cluster that actually
# exists, so use the sidecar cluster.

if not self.config.ir.sidecar_cluster_name:
# Uh whut? how is Edge Stack running exactly?
raise Exception(
"Edge Stack claims to be running, but we have no sidecar cluster??"
)
# This route is needed to trigger an ExtAuthz request for the AuthService.
# The auth service grabs the challenge and does the right thing.
# Rather than try to route to some existing cluster we can just return a
# direct response. What we return doesn't really matter but
# to match existing Edge Stack behavior we return a 404 response.

if self._log_debug:
logger.debug(" punching a hole for ACME")

# Make sure to include _host_constraints in here for now.
#
# XXX This is needed only because we're dictifying the V3Route too early.

# Make sure to include _host_constraints in here for now so it can be
# applied to the correct vhost during future proccessing
chain.routes.insert(
0,
{
"_host_constraints": set(),
"match": {"case_sensitive": True, "prefix": "/.well-known/acme-challenge/"},
"route": {
"cluster": self.config.ir.sidecar_cluster_name,
"prefix_rewrite": "/.well-known/acme-challenge/",
"timeout": "3.000s",
},
"direct_response": {"status": 404},
},
)

Expand Down
2 changes: 1 addition & 1 deletion python/ambassador/ir/ir.py
Original file line number Diff line number Diff line change
Expand Up @@ -1025,7 +1025,7 @@ def add_cluster(self, cluster: IRCluster) -> IRCluster:
self.clusters[cluster.name] = cluster

if cluster.is_edge_stack_sidecar():
# self.logger.debug(f"IR: cluster {cluster.name} is the sidecar")
self.logger.debug(f"IR: cluster {cluster.name} is the sidecar cluster name")
self.sidecar_cluster_name = cluster.name
else:
self.logger.debug(
Expand Down

0 comments on commit 9bfc82f

Please sign in to comment.