Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CIDR json tag in CNP CIDRRule #25617

Merged
merged 1 commit into from May 24, 2023

Conversation

pippolo84
Copy link
Member

Without the omitempty tag, the following valid policy:

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "echo-ingress-from-cidr-group-ref"
  namespace: "default"
spec:
  description: "Allow echo pods to receive ingress traffic from a specific CIDR Group"
  endpointSelector:
    matchLabels:
      kind: echo
  ingress:
  - fromCIDRSet:
    - cidrGroupRef: "connectivity-test-cidr-group"

cannot be correctly decoded from YAML using an UniversalDeserializer from runtime/serializer.

Without the omitempty tag, the following valid policy:

```
apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "echo-ingress-from-cidr-group-ref"
  namespace: "default"
spec:
  description: "Allow echo pods to receive ingress traffic from a specific CIDR Group"
  endpointSelector:
    matchLabels:
      kind: echo
  ingress:
  - fromCIDRSet:
    - cidrGroupRef: "connectivity-test-cidr-group"
```

cannot be correctly decoded from YAML:

```
CiliumNetworkPolicy.cilium.io "echo-ingress-from-cidr-group-ref" is invalid: [spec.ingress.fromCIDRSet.cidr: Invalid value: "": spec.ingress.fromCIDRSet.cidr in body should match '^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\/([0-9]|[1-2][0-9]|3[0-2])$|^s*((([0-9A-Fa-f]{1,4}:){7}(:|([0-9A-Fa-f]{1,4})))|(([0-9A-Fa-f]{1,4}:){6}:([0-9A-Fa-f]{1,4})?)|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){0,1}):([0-9A-Fa-f]{1,4})?))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){0,2}):([0-9A-Fa-f]{1,4})?))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){0,3}):([0-9A-Fa-f]{1,4})?))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){0,4}):([0-9A-Fa-f]{1,4})?))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){0,5}):([0-9A-Fa-f]{1,4})?))|(:(:|((:[0-9A-Fa-f]{1,4}){1,7}))))(%.+)?s*/([0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8])$', : Invalid value: "": "spec.ingress.fromCIDRSet" must validate one and only one schema (oneOf). Found 2 valid alternatives]
```

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
@pippolo84 pippolo84 added sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/agent Cilium agent related. labels May 23, 2023
@pippolo84 pippolo84 requested a review from a team as a code owner May 23, 2023 10:15
@christarazi christarazi added the affects/v1.13 This issue affects v1.13 branch label May 23, 2023
@christarazi
Copy link
Member

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 24, 2023
@squeed squeed merged commit 50e2f87 into cilium:main May 24, 2023
59 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects/v1.13 This issue affects v1.13 branch ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/agent Cilium agent related. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants