kubernetes: Network Policy accepts 2 cidrs in one ipBlock

yeap, just concluding, this is the correct behavior of the API, as an example, when I create the following deployment:

apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    run: nginx
  name: nginx
spec:
  replicas: 1
  replicas: 2 
  replicas: 3
  selector:
    matchLabels:
      run: nginx
  template:
    metadata:
      labels:
        run: nginx
    spec:
      containers:
      - image: nginx
        image: nginx1
        name: nginx
        resources: {}

I repeat replicas and the image fields twice, so the last value the field have is the one that counts:

kubectl  get deploy
NAME    READY   UP-TO-DATE   AVAILABLE   AGE
nginx   0/3     3            0           55s

IMO the behavior is right, and there’s no correction that can be made on validation as the part is a struct containing a string. Anyway, waiting for some more thoughts here 😃

@jayunit100 @rikatz this seems a good one for the netpol crew 😉

It appears --dry-run=server is using the last value, so different to --dry-run=client. Yes, probably this is the way library generates the information on client side vs server side. I don’t know under the hood of the api machinery if there’s some reason why this happens, but I guess…it’s partially right per the nature of the struct right?

Yes, I would investigate the unmarshalling process, I think that was why @mattfenwick made the statement below earlier

Ah good call @dean-alphafx , I made a boneheaded mistake and put a different policy into kubeval!!

Retrying with the right policy, I got the same result as you. I think we need to report a kubeval bug too!

For solving this, yaml.UnmarshalStrict does handle this correctly in one of my projects, maybe we could use that.

@solomonope The main problem here seems in the network policy validation -> https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/networking/validation/validation.go#L190

Definition of an IPBlock: https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/networking/types.go#L158-L161

If you need some guidance, please let me know as well 😃

@mattfenwick I’m thinking a bit here, I don’t know actually what’s the behavior of the API should be if you insert multiple strings in a struct that should receive only one.

Like:

type Blo struct {
    MyString string
}

For what I can see here, what’s happening actually is that the double declaration of the CIDR (which is a single field inside the IPBlock struct) is being overwritten because you can only declare once.

Do you remember other case in Kubernetes API that accepts a string, and you do a double declaration? Like, what happens if I create a Pod object, and insert the ‘image’ field twice? Should test before assume that a validation can solve this problem 😃

@solomonope will hand off to you 😄 😄 lmk if you need anything from me !!!

I would like to work on this

Odd @mattfenwick, kubeval --strict doesn’t even complain for me. Version: 0.15.0:

root@cks-master:~# cat broke_np.yaml
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
  name: backend
  namespace: default
spec:
  podSelector:
    matchLabels:
      run: backend
  policyTypes:
  - Egress
  - Ingress
  egress:
  - to:
    - ipBlock:
        cidr: 8.8.4.4/32
        cidr: 1.1.1.1/32
        cidr: 8.8.8.8/32
        cidr: 2.2.2.2/32
    ports:
    - protocol: TCP
      port: 53
root@cks-master:~# ./kubeval --strict broke_np.yaml
PASS - broke_np.yaml contains a valid NetworkPolicy (default.backend)