Repository navigation
fix(helm): refuse NetworkPolicy peers the kubernetes api would refuse - #3500
Merged
Merged
Conversation
gatewayRouting.sandbox.ingressPeers checked only matchLabels and openshellGit.clients only that both selectors were present; ipBlock CIDRs, matchExpressions and unknown keys were pasted with toYaml, so an invalid peer failed when Helm applied the NetworkPolicy, after other release objects had changed. A shared openclaw.networkPolicy.peer helper now checks each peer: ipBlock or selectors, CIDRs (the remoteNodeCidrs parser, moved into a shared helper), matchLabels (existing helper), matchExpressions key/operator/values, and no unknown fields. egressAuthority.clients is left as is (Credential Proxy area). Finding 2336.
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review in progressClawSweeper is reviewing this revision. This supersedes any previous blocked status. |
Review follow-ups: a peer field set to null (ipBlock: null next to selectors) is unset for the API server, so the peer helper no longer refuses it, and a peer whose fields are all null is refused. matchExpressions values that are not strings get a --set-string hint. The test now matches each refusal on Helm's stderr (the error message also carries the --set command line, which let the operator case pass for the wrong reason), covers unknown ipBlock and selector fields, invalid and non-string expression values and the null shapes, and checks that valid peers render as written.
# Conflicts: # docs/guides/deploy/breaking-changes.md
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.
Problem
Two chart values become NetworkPolicy peers almost verbatim:
gatewayRouting.sandbox.ingressPeerschecked onlymatchLabels;ipBlock.cidr,ipBlock.exceptandmatchExpressionswere pasted withtoYaml.openshellGit.clientschecked only that both selectors carriedmatchLabels, not their keys or values.helm templatetherefore rendered, for example,ipBlock.cidr: 10.0.0.0/33,operator: Equalswith keybad key!, ormatchLabels: {kubernetes.io/metadata.name: "openshell system"}. The Kubernetes API (ValidateLabelSelector,ParseCIDR) refuses each one when Helm applies the NetworkPolicy, after other objects of the release have already changed. An unknown key such aspodSelectorswas rendered as written. Finding 2336.Change
templates/_network-policies.tpl:openclaw.networkPolicy.peer: the peer must be eitheripBlockor selectors, with no unknown fields. A field set to null counts as unset, as the API server reads it.openclaw.networkPolicy.selector:matchLabelsgoes through the existingopenclaw.networkPolicy.matchLabels. FormatchExpressionsit checks the key, the operator (In/NotIn/Exists/DoesNotExist) and that the values fit the operator.openclaw.networkPolicy.cidr: the IPv4/IPv6 parser thatgatewayRouting.remoteNodeCidrsalready used, moved out so it can be shared. It produces the same messages for that field.openclaw.networkPolicy.labelKey: split out of thematchLabelshelper.gateway-routing.yamlandopenshell-git.yamlrun each peer throughopenclaw.networkPolicy.peer. The rendered YAML does not change for valid input.docs/guides/deploy/native-admin.mdand a breaking-change notice.egressAuthority.clientshas the same gap but belongs to the credential proxy listener, so it is left for a separate change.Verification
tests/integration/production-kubernetes-packaging.test.mjs:openshellGit.clientsshapes, plus valid controls:0.0.0.0/0withexcept,::/0, andmatchExpressionsusingNotInandExists.--setcommand line), and checks that valid peers, including a null sibling field, render exactly as written.--set-stringhint for non-string expression values, leading-zero octets named in the notice. Mutants for the operator, unknownipBlockfield and null-field checks are caught. This includes the existingremoteNodeCidrsand peer-label tests, which exercise the moved CIDR parser and label-key helper.tests/integration/installation-change-preflight.test.mjs: 7 pass. eslint and prettier are clean on the touched files.Risks
ipBlock.exceptcontainment insidecidris not checked.breaking-changes.md. Whichever lands second needs a trivial merge of main.