feat(template-chart)!: ambientMesh becomes an object, and the CI exception gets a pinned expectation
Gitea #143, maintainer's instruction. Both numbers this switch renders are ISTIO CONSTANTS rather than standards, and a value with a documented default is the better home for a vendor constant than a template literal: an operator can correct it when Istio changes the number, without waiting for a chart release. networkPolicy: ingress: ambientMesh: enabled: false port: 15008 redirectedProbeSources: - 169.254.7.127/32 One object under one switch, so the coupling argument survives the change instead of being undermined by it - two separate values could drift apart, which is the thing this design exists to prevent. redirectedProbeSources is a LIST because a dual-stack cluster needs BOTH of Istio's addresses at once, not one of two; that is a nameable case rather than speculation, so it does not contradict reason 4 of the issue. Istio's IPv6 counterpart is documented as the dual-stack addition, with the reason it is absent today unchanged: SingleStack IPv4, and an ipBlock of the wrong family is accepted and matches nothing. Named redirectedProbeSources, not probeSources, because the confusion risk with networkPolicy.probeSources one level up is larger than the ingress/ ingressController one: that value holds YOUR worker node CIDRs, is empty by default and means OPEN when empty; this one holds ISTIO's rewrite address and ships filled, because unlike the node IPs it is a constant the chart can know. The distinction is written where both appear. PROVENANCE, which the previous round explained the coupling of but never attributed. 15008 is Istio's HBONE port - not IANA-registered for this, not prescribed by any protocol, unknown to Kubernetes. 169.254.7.127 is link-local (169.254.0.0/16, RFC 3927, never routed - the range one picks vendor magic out of); Istio's IPv6 counterpart fd16:9254:7127:1337:ffff:ffff:ffff:ffff carries the same digits plus 1337, which is what vendor constants look like; and Kubernetes knows the address in neither the NetworkPolicy API nor the kubelet - only Istio's data plane rewrites the probe source onto it. Both read for ztunnel 1.30.6. If either changes, the failure is SILENT: a pod unreachable in the mesh, or probes dropped where probeSources is non-empty, which is nowhere today. NULL-SAFE, per #99 and because #136 shipped this exact defect one value away (its NOTES.txt branch dereferenced .networkPolicy.egress directly): every read goes through a fallback reproducing the chart's own declared defaults, so a deleted object or one carrying only "enabled: true" renders the defaults rather than failing. Clearing the probe-source list restores the constant instead of removing the ipBlock, deliberately - the halves are not separable, and enabled: false is how you open nothing. THE CONSEQUENCE, and it is the point of the CI half. With the port configurable, the exception can no longer be checked against the rendered value: "whatever the value says" is design A's hole wearing design C's name, and a mistyped port would pass by sitting in the configurable field. So the expectation is PINNED IN THE TEST - AMBIENT_PORT and AMBIENT_CIDR - and assertion 7 now checks two things instead of one: that the render at chart defaults produces exactly that port and that ipBlock, and that every chart's values.yaml DECLARES the same defaults, read back through ci/yaml2json rather than by regex. A chart that drifts is red. Changing an Istio constant means touching 30 values files AND the pinned lines, and the pinned lines are what force someone to look. That block is therefore also the single place saying what we hold Istio's numbers to be, so it carries the provenance too. The renders set only enabled=true and never a port override, on purpose: the pin is a statement about the defaults we ship, not about what a release may choose - an operator overriding a value is outside this test's reach, as for every value here. The declared-defaults check is conditional on the chart offering probeSources, like the inseparability check and assertion 4: the five charts whose probe rule is never source-restricted render no ipBlock, so for them the key must be ABSENT - a value reaching no rule reads as a control that does nothing - and that absence is asserted rather than tolerated. Their schemas drop it too. No version change: nothing of this is published, the switch stays additive with its default off, and the render at defaults is byte-identical - re-diffed per chart against origin/main, not assumed.
This commit is contained in:
4 files changed
+358
-98
No files matched your search
+152
-35
@@ -1261,17 +1261,26 @@ done
|
||||
# around, so the exception is held to five things at once, on TWO renders of
|
||||
# every chart:
|
||||
#
|
||||
# the chart's DECLARED DEFAULTS (read back through ci/yaml2json)
|
||||
# - enabled must default to false, and the port and the probe source must be
|
||||
# exactly the numbers pinned below. A chart whose default drifted would
|
||||
# otherwise ship a port no other chart ships while the exception kept being
|
||||
# granted to the pinned one.
|
||||
# switch OFF (the main loop's render, reused)
|
||||
# - 15008 must be ABSENT. Without this half the exception is a hole: a
|
||||
# - the pinned port must be ABSENT. Without this half the exception is a hole: a
|
||||
# chart could render the port unconditionally and nothing would notice,
|
||||
# because assertion 7 would find it present and assertion 2 is where it
|
||||
# would have failed.
|
||||
# - 169.254.7.127/32 must be absent too, for the same reason.
|
||||
# switch ON
|
||||
# - 15008/TCP must be PRESENT, or the value is decorative.
|
||||
# - 15008 must be the ONLY ingress port that is not a containerPort. This is
|
||||
# the line between a named exception and design A of #143: every other
|
||||
# mistyped or NodePort number still fails here, with the switch on.
|
||||
# switch ON (only "enabled=true" is set, never a port override - the pin is a
|
||||
# statement about the defaults WE ship)
|
||||
# - the PINNED port must be PRESENT, or the value is decorative.
|
||||
# - the PINNED port must be the ONLY ingress port that is not a
|
||||
# containerPort. This is the line between a named exception and design A
|
||||
# of #143: every other mistyped or NodePort number still fails here, with
|
||||
# the switch on. Note "pinned": since #143 made the port a VALUE, checking
|
||||
# the exception against the rendered value would permit whatever the value
|
||||
# said, which is design A's hole under design C's name.
|
||||
# - wherever the rule carrying 15008 restricts sources, that rule's "from"
|
||||
# must contain 169.254.7.127/32. This is the inseparability of the two
|
||||
# halves, asserted rather than trusted - and it is REACHABLE, because
|
||||
@@ -1314,6 +1323,45 @@ done
|
||||
# mechanism as assertion 0: the mistake arrives by copy into a NEW chart whose
|
||||
# own diff nobody reads for it, which is exactly how garage (#81) and keycloak
|
||||
# (#83) arrived with no policy at all after #5 had been filed.
|
||||
# THE PINNED EXPECTATION, and it is pinned HERE rather than read from the chart
|
||||
# on purpose (Gitea #143). Both numbers are values with documented defaults, so
|
||||
# that an operator can correct an Istio constant without waiting for a chart
|
||||
# release - but an exception that permits "whatever the value says" is design A's
|
||||
# hole wearing design C's name: a mistyped port would pass simply by sitting in
|
||||
# the configurable field, and that is the single reason design A was rejected.
|
||||
#
|
||||
# So the test states what WE believe Istio's numbers to be, and asserts two
|
||||
# things against that statement: the render at CHART DEFAULTS produces exactly
|
||||
# this port and exactly this ipBlock, AND every chart's values.yaml DECLARES
|
||||
# these same defaults. A chart that drifts is red.
|
||||
#
|
||||
# Which makes this block the one place in the repository that says what we hold
|
||||
# Istio's constants to be - and therefore also the answer to "where does
|
||||
# 169.254.7.127 come from":
|
||||
#
|
||||
# 15008 Istio's HBONE port. Not registered with IANA for this,
|
||||
# prescribed by no protocol, unknown to Kubernetes. Istio
|
||||
# picked it and Istio can change it.
|
||||
# 169.254.7.127/32 the address Istio's data plane rewrites the kubelet's probe
|
||||
# source onto. LINK-LOCAL (169.254.0.0/16, RFC 3927), a range
|
||||
# reserved for node-local use and never routed. Istio's IPv6
|
||||
# counterpart fd16:9254:7127:1337:ffff:ffff:ffff:ffff carries
|
||||
# the SAME DIGITS plus 1337, which is what vendor constants
|
||||
# look like. Neither the NetworkPolicy API nor the kubelet has
|
||||
# any notion of it.
|
||||
#
|
||||
# BOTH READ FOR ztunnel 1.30.6. If Istio changes either, the failure is silent -
|
||||
# a pod unreachable in the mesh, or probes dropped where probeSources is
|
||||
# non-empty, which is nowhere today. Changing them means 30 values files AND
|
||||
# these two lines, and these two lines are what force someone to look.
|
||||
#
|
||||
# An operator who overrides the value in their own release is outside this test's
|
||||
# reach. That is true of every value in this repository and is not a new gap -
|
||||
# and it is why these renders set ONLY "enabled=true" and never a port override:
|
||||
# the pin is a statement about the defaults WE ship, not about what a release may
|
||||
# choose.
|
||||
AMBIENT_PORT=15008
|
||||
AMBIENT_CIDR="169.254.7.127/32"
|
||||
AMBIENT_VALUE="networkPolicy.ingress.ambientMesh"
|
||||
for CHART in "${CHARTS[@]}"; do
|
||||
# A chart whose main-loop render already failed has nothing to compare
|
||||
@@ -1326,10 +1374,10 @@ for CHART in "${CHARTS[@]}"; do
|
||||
ROW_FAIL=""
|
||||
ON_JSON="$WORK/ambient-$CHART-on.json"
|
||||
if ! RCJ_WITH_OVERLAY=1 render_chart_json "$CHART" "$ON_JSON" \
|
||||
--set "$AMBIENT_VALUE=true"; then
|
||||
--set "$AMBIENT_VALUE.enabled=true"; then
|
||||
sed 's/^/ /' "$WORK/xerr"
|
||||
echo " FAIL $CHART does not render with $AMBIENT_VALUE=true"
|
||||
echo "::error title=ambient-mesh $CHART::$CHART does not render with $AMBIENT_VALUE=true. Since Gitea #143 every chart in this repository offers that value, and networkPolicy carries additionalProperties: false in all of them - so an unknown key does not get ignored, it fails the render. Add the key to values.yaml AND to values.schema.json (schema and template belong together per chart), and render the ingress port 15008 plus, inside the source-restricted branch, the ipBlock 169.254.7.127/32. Copy template-chart/templates/xxx.networkpolicy.yaml."
|
||||
echo " FAIL $CHART does not render with $AMBIENT_VALUE.enabled=true"
|
||||
echo "::error title=ambient-mesh $CHART::$CHART does not render with $AMBIENT_VALUE.enabled=true. Since Gitea #143 every chart in this repository offers that value, and networkPolicy carries additionalProperties: false in all of them - so an unknown key does not get ignored, it fails the render. Add the object to values.yaml AND to values.schema.json (schema and template belong together per chart), with enabled: false, port: \$AMBIENT_PORT and redirectedProbeSources: [\$AMBIENT_CIDR], and render that port plus - inside the source-restricted branch - that ipBlock. Copy template-chart/templates/xxx.networkpolicy.yaml."
|
||||
ROW_FAIL=1
|
||||
fi
|
||||
|
||||
@@ -1345,17 +1393,36 @@ for CHART in "${CHARTS[@]}"; do
|
||||
ROW_FAIL=1
|
||||
fi
|
||||
|
||||
# The chart's DECLARED defaults, read with the same parser as everything else
|
||||
# (ci/yaml2json prints a values file back as JSON). Not a regex over
|
||||
# values.yaml: the pinned expectation is about numbers and a list, and a
|
||||
# substring test over a commented file is how the probeSources check in
|
||||
# assertion 4 was once fooled by a deleted key whose comment survived.
|
||||
VALUES_JSON="$WORK/$CHART.values.json"
|
||||
if [ -z "$ROW_FAIL" ]; then
|
||||
if ! helm template y2j "$YAML2JSON" -f "$ROOT/$CHART/values.yaml" 2>"$WORK/xerr" \
|
||||
| awk '/^\{/ { print; exit }' >"$VALUES_JSON" || [ ! -s "$VALUES_JSON" ]; then
|
||||
sed 's/^/ /' "$WORK/xerr"
|
||||
echo " FAIL $CHART/values.yaml could not be re-read as JSON"
|
||||
echo "::error title=ambient-mesh $CHART::$CHART/values.yaml could not be re-read by ci/yaml2json, so the chart's DECLARED ambient-mesh defaults could not be compared with the numbers this test pins. That is a parse failure, not a policy finding."
|
||||
ROW_FAIL=1
|
||||
fi
|
||||
fi
|
||||
|
||||
if [ -z "$ROW_FAIL" ]; then
|
||||
set +e
|
||||
python3 - "$CHART" "$RELEASE" "$AMBIENT_VALUE" "$WORK/$CHART.json" "$ON_JSON" \
|
||||
"$ROOT/$CHART" "$CHART_NAME" \
|
||||
"$ROOT/$CHART" "$CHART_NAME" "$AMBIENT_PORT" "$AMBIENT_CIDR" "$VALUES_JSON" \
|
||||
>"$WORK/ambient-$CHART.out" 2>"$WORK/ambient-$CHART.pyerr" <<'PY'
|
||||
import json, re, sys
|
||||
|
||||
chart, release, gate, off_path, on_path, chart_dir, chart_name = sys.argv[1:8]
|
||||
(chart, release, gate, off_path, on_path, chart_dir, chart_name,
|
||||
pinned_port, pinned_cidr, values_json) = sys.argv[1:11]
|
||||
|
||||
PORT = 15008
|
||||
CIDR = "169.254.7.127/32"
|
||||
# Pinned by the caller, never read from the chart - see the long note above
|
||||
# AMBIENT_PORT in this script.
|
||||
PORT = int(pinned_port)
|
||||
CIDR = pinned_cidr
|
||||
|
||||
lines = []
|
||||
def ok(msg): lines.append("OK|%s: %s" % (chart, msg))
|
||||
@@ -1451,27 +1518,77 @@ def cidrs_of(rule):
|
||||
return out
|
||||
|
||||
|
||||
# --- The value is declared, in BOTH places it has to be --------------------
|
||||
# The render with the switch on already proves the SCHEMA declares it - an
|
||||
# --- The chart DECLARES the pinned defaults -------------------------------
|
||||
# The render with the switch on already proves the SCHEMA declares the key - an
|
||||
# undeclared key cannot pass additionalProperties: false. What that render
|
||||
# cannot prove is that values.yaml ships the documented default, and a value
|
||||
# only reachable through --set is not a value an operator finds.
|
||||
# cannot prove is what the chart's own defaults SAY, and since #143 made both
|
||||
# Istio constants configurable, that is the half the pinned exception rests on:
|
||||
# the exception below is granted to the number THIS SCRIPT names, so a chart
|
||||
# whose default has drifted away from it must be red here rather than quietly
|
||||
# carrying a different port than every other chart.
|
||||
values_yaml = ""
|
||||
try:
|
||||
values_yaml = open("%s/values.yaml" % chart_dir, encoding="utf-8").read()
|
||||
except IOError:
|
||||
pass
|
||||
if re.search(r"^\s*ambientMesh\s*:", values_yaml, re.M):
|
||||
ok("values.yaml ships the ambientMesh key with its default, and the render "
|
||||
"with %s=true succeeded - which is itself the proof that "
|
||||
"values.schema.json declares it, since networkPolicy carries "
|
||||
"additionalProperties: false" % gate)
|
||||
|
||||
declared = json.load(open(values_json))
|
||||
node = (((declared.get("networkPolicy") or {}).get("ingress") or {})
|
||||
.get("ambientMesh"))
|
||||
if not isinstance(node, dict):
|
||||
fail("renders with %s.enabled=true - so the schema knows the key - but the "
|
||||
"chart's own values.yaml declares no ambientMesh OBJECT (found %r). A "
|
||||
"value that exists only in the schema is reachable through --set and "
|
||||
"findable nowhere: the defaults are what an operator reads. Ship the "
|
||||
"object with enabled: false, port: %d and redirectedProbeSources: "
|
||||
"[%s], as in template-chart." % (gate, node, PORT, CIDR))
|
||||
else:
|
||||
fail("renders with %s=true - so the schema knows the key - but values.yaml "
|
||||
"contains no ambientMesh key at all. A value that exists only in the "
|
||||
"schema is reachable through --set and findable nowhere: the defaults "
|
||||
"are what an operator reads. Add it with its comment, default false, "
|
||||
"as in template-chart." % gate)
|
||||
problems = []
|
||||
if node.get("enabled") not in (False, None):
|
||||
problems.append("enabled is %r and must default to false - the switch is "
|
||||
"additive and no release may get it by accident"
|
||||
% node.get("enabled"))
|
||||
if node.get("port") != PORT:
|
||||
problems.append("port defaults to %r, but this test pins Istio's HBONE "
|
||||
"port at %d" % (node.get("port"), PORT))
|
||||
# Conditional on the chart offering probeSources, exactly as the
|
||||
# inseparability check below and assertion 4 are. A chart whose probe rule
|
||||
# is never source-restricted renders no ipBlock, so this key would reach no
|
||||
# rule - and a value that silently does nothing reads as a control, which is
|
||||
# worse than one that is absent (README, "Values, and shipping only the live
|
||||
# ones"). So for those charts the key must be ABSENT, and that is asserted
|
||||
# rather than tolerated.
|
||||
sources = node.get("redirectedProbeSources")
|
||||
offers_ps = bool(re.search(r"^\s*probeSources\s*:", values_yaml, re.M))
|
||||
if offers_ps and sources != [CIDR]:
|
||||
problems.append("redirectedProbeSources defaults to %r, but this test "
|
||||
"pins Istio's redirected probe source at [%r]"
|
||||
% (sources, CIDR))
|
||||
if not offers_ps and sources is not None:
|
||||
problems.append("redirectedProbeSources is declared (%r) although this "
|
||||
"chart offers no networkPolicy.probeSources, so its probe "
|
||||
"rule is never source-restricted and no ipBlock is ever "
|
||||
"rendered - the key would reach no rule and read as a "
|
||||
"control that does nothing" % sources)
|
||||
if problems:
|
||||
fail("the chart's declared ambient-mesh defaults do not match the "
|
||||
"numbers ci/run-networkpolicy-test.sh pins: %s. Both are ISTIO "
|
||||
"CONSTANTS, and they are values precisely so an operator can "
|
||||
"correct them without a chart release - but the exception assertion "
|
||||
"2 grants the HBONE port is pinned to the number THIS TEST knows, "
|
||||
"never to whatever the value says, because a value-driven exception "
|
||||
"would let a mistyped port ride in through a configurable field. "
|
||||
"That is the hole design A of #143 was rejected for. So: if Istio "
|
||||
"changed the constant, change it in all 30 charts AND in the pinned "
|
||||
"expectation here - the drift is meant to be loud."
|
||||
% "; ".join(problems))
|
||||
else:
|
||||
ok("declares the pinned ambient-mesh defaults verbatim - enabled false, "
|
||||
"port %d, %s - so the named exception below is granted to a number "
|
||||
"this chart really ships, and not to whatever a value happened to say"
|
||||
% (PORT, ("redirectedProbeSources [%s]" % CIDR) if offers_ps else
|
||||
"and no redirectedProbeSources, correctly, since this chart "
|
||||
"renders no ipBlock at all"))
|
||||
|
||||
off_policies = own_policies(off_path)
|
||||
on_policies = own_policies(on_path)
|
||||
@@ -1496,20 +1613,20 @@ else:
|
||||
if cidr == CIDR:
|
||||
off_cidr_hits.append("%s ingress[%d]" % (name, i))
|
||||
if off_hits:
|
||||
fail("allows ingress port %d with %s at its default (false): %s. The "
|
||||
fail("allows ingress port %d with %s.enabled at its default (false): %s. The "
|
||||
"exception assertion 2 grants for %d is gated on that value, and "
|
||||
"this is the half that keeps it from being a hole - a port rendered "
|
||||
"unconditionally would otherwise be found present by this assertion "
|
||||
"and never held against the pod spec by assertion 2. Gate the port "
|
||||
"on the value." % (PORT, gate, ", ".join(off_hits), PORT))
|
||||
elif off_cidr_hits:
|
||||
fail("carries the ipBlock %s with %s at its default (false): %s. The "
|
||||
fail("carries the ipBlock %s with %s.enabled at its default (false): %s. The "
|
||||
"probe-source half of the switch is for the mesh only, where the "
|
||||
"kubelet's probes are redirected; outside it the address matches "
|
||||
"nothing and makes the rule read as if it did something."
|
||||
% (CIDR, gate, ", ".join(off_cidr_hits)))
|
||||
else:
|
||||
ok("with %s at its default the policy allows neither port %d nor the "
|
||||
ok("with %s.enabled at its default the policy allows neither port %d nor the "
|
||||
"ipBlock %s - the negative half of assertion 2's named exception, "
|
||||
"without which the exception would be a hole" % (gate, PORT, CIDR))
|
||||
|
||||
@@ -1544,18 +1661,18 @@ else:
|
||||
"the selected pod spec" % (name, i, value))
|
||||
|
||||
if not found_port:
|
||||
fail("renders no ingress port %d with %s=true, so the value is "
|
||||
fail("renders no ingress port %d with %s.enabled=true, so the value is "
|
||||
"decorative: the chart accepts it, the schema declares it, and the "
|
||||
"policy that comes out is the same one as before. A pod whose "
|
||||
"namespace has joined the ambient mesh is then unreachable, because "
|
||||
"ambient does not bypass NetworkPolicy enforcement." % (PORT, gate))
|
||||
else:
|
||||
ok("with %s=true the policy allows %s - ztunnel's HBONE tunnel, the one "
|
||||
ok("with %s.enabled=true the policy allows %s - ztunnel's HBONE tunnel, the one "
|
||||
"port assertion 2 grants a named exception for"
|
||||
% (gate, ", ".join(found_port)))
|
||||
|
||||
if bad_ports:
|
||||
fail("with %s=true the policy allows ingress port(s) that are NOT "
|
||||
fail("with %s.enabled=true the policy allows ingress port(s) that are NOT "
|
||||
"containerPorts of the selected pod spec and are NOT the one number "
|
||||
"the exception covers (%d): %s. This is the line between a named "
|
||||
"exception and the free port key that was rejected as design A of "
|
||||
@@ -1563,7 +1680,7 @@ else:
|
||||
"including a NodePort number written where a pod port belongs."
|
||||
% (gate, PORT, "; ".join(bad_ports)))
|
||||
elif found_port:
|
||||
ok("with %s=true, port %d is still the ONLY ingress port of this chart "
|
||||
ok("with %s.enabled=true, port %d is still the ONLY ingress port of this chart "
|
||||
"that is not a containerPort of the selected pod spec - the exception "
|
||||
"covers exactly one number and nothing rode in beside it"
|
||||
% (gate, PORT))
|
||||
@@ -1599,7 +1716,7 @@ else:
|
||||
"%s ingress[%d] (peers: %s)"
|
||||
% (name, i, ", ".join(cidrs_of(rule)) or "no ipBlock at all"))
|
||||
if restricted_without_cidr:
|
||||
fail("with %s=true the rule carrying port %d restricts its sources but "
|
||||
fail("with %s.enabled=true the rule carrying port %d restricts its sources but "
|
||||
"does not include the ipBlock %s: %s. Inside the mesh the kubelet's "
|
||||
"probes are redirected and arrive from that address instead of from "
|
||||
"the worker node's, so a restricted rule without it drops every "
|
||||
|
||||
Reference in new issue
Block a user