Skip to content

Conversation

@huntergregory
Copy link
Contributor

@huntergregory huntergregory commented Dec 12, 2022

Issue

Kubernetes doesn't validate matchExpression label values in NetworkPolicies. Currently, NPM may log an uninformative error for non-standard matchExpression labels.

We should validate matchExpression labels during translation and error with a clear message if necessary.

Details

The issue can occur for any use of matchExpression values (used with the In or NotIn operator). This includes:

  • the target PodSelector
  • a rule PodSelector
  • a rule NamespaceSelector

Example NetworkPolicy (passes kubectl validation):

apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
  name: test-policy
  namespace: test
spec:
  ingress:
  - from:
    - podSelector:
        matchExpressions:
        - key: X
          operator: In
          values:
          - "bad space"
  podSelector:
    matchLabels:
      pod: "a"
  policyTypes:
  - Ingress

Typical k8s validation for labels in matchLabels:

The NetworkPolicy "test-policy" is invalid: spec.podSelector.matchLabels: Invalid value: "bad space": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')

Current, uninformative error from NPM:

E1212 20:30:11.921121       1 restore.go:145] on try number 1, failed to run command [iptables-restore -w 60 -T filter --noflush]. Rerunning with same file. err: [line-number error for line [-A AZURE-NPM-INGRESS-972688392 -j AZURE-NPM-INGRESS-ALLOW-MARK -m set ! --match-set azure-npm-2409537903 src -m set --match-set azure-npm-3631277798 src -m comment --comment ALLOW-FROM-!podlabel-X:bad space-AND-ns-test]: error running command [iptables-restore -w 60 -T filter --noflush] with err [exit status 2] and stdErr [Bad argument `space'
Error occurred at line: 3
Try `iptables-restore -h' or 'iptables-restore --help' for more information.
]]
E1212 20:30:11.923799       1 networkPolicyController.go:195] error syncing 'test/test-policy': [syncNetPol] error due to  [syncAddAndUpdateNetPol] Error: failed to update translated NPMNetworkPolicy into Dataplane due to [DataPlane] error while adding policy: Operation [AddNetworkPolicy] failed with error code [999], full cmd [], full error failed to add policy: failed to restore iptables with updated policies: failed to restore iptables file: Operation [RunCommandWithFile] failed with error code [999], full cmd [], full error after 2 tries, failed to run command [iptables-restore -w 60 -T filter --noflush] with error: error running command [iptables-restore -w 60 -T filter --noflush] with err [exit status 2] and stdErr [Bad argument `space'
Error occurred at line: 3
Try `iptables-restore -h' or 'iptables-restore --help' for more information.
], requeuing

@huntergregory huntergregory requested a review from a team as a code owner December 12, 2022 23:15
@huntergregory huntergregory requested review from matmerr and removed request for a team December 12, 2022 23:15
@huntergregory huntergregory added npm Related to NPM. bug labels Dec 12, 2022
@huntergregory
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@vakalapa
Copy link
Contributor

vakalapa commented Jan 4, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@vakalapa vakalapa merged commit 8bbb99e into master Jan 19, 2023
@vakalapa vakalapa deleted the hgregory/bad-iptables-arg branch January 19, 2023 17:43
rjdenney pushed a commit to rjdenney/azure-container-networking that referenced this pull request Jan 19, 2023
…zure#1717)

* add validation for matchExpression values

* fix lint

* make regex identical to kubectl validation and include more test cases

* remove debugging lines
smittal22 pushed a commit to smittal22/azure-container-networking that referenced this pull request Feb 3, 2023
…zure#1717)

* add validation for matchExpression values

* fix lint

* make regex identical to kubectl validation and include more test cases

* remove debugging lines
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug npm Related to NPM.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants