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

User Defined Injections - does not take into account json path operation #93

Open
MichalGuzieniuk opened this issue Apr 12, 2021 · 2 comments
Labels
enhancement New feature or request

Comments

@MichalGuzieniuk
Copy link
Contributor

While defining custom data in ConfigMap it is possible to define JSON path operation (add, remove, replace, copy, move). Those operation are not taken into account by NRI.

For instance for ConfigMap

apiVersion: v1
kind: ConfigMap
metadata:
  name: nri-user-defined-injections
  namespace: kube-system
data:
  "customInjection": '{"op": "remove", "path": "/metadata/annotations", "value": {"k8s.v1.cni.cncf.io/networks": "foo-network"}}'

Original POD specification is not modified

apiVersion: v1
kind: Pod
metadata:
  name: testpod
  labels:
    customInjection: "true"
  annotations:
    k8s.v1.cni.cncf.io/networks: foo-network
spec:
  containers:
  - name: app
    image: alpine
    command: [ "/bin/sh", "-c", "sleep INF" ]

Expected to remove foo-network from POD specification.

Second use case, for ConfigMap, operation add

apiVersion: v1
kind: ConfigMap
metadata:
  name: nri-user-defined-injections
  namespace: kube-system
data:
  "customInjection": '{"op": "add", "path": "/metadata/annotations", "value": {"k8s.v1.cni.cncf.io/networks": "sriov-net-attach-def"}}'

and above POD definition, I would expect for operation add to have after modification two networks

 k8s.v1.cni.cncf.io/networks: foo-network, sriov-net-attach-def

instead for given key, values are replaced. Current state:

 k8s.v1.cni.cncf.io/networks: sriov-net-attach-def
@zshi-redhat
Copy link
Collaborator

@MichalGuzieniuk Thanks for the continued testing!

While defining custom data in ConfigMap it is possible to define JSON path operation (add, remove, replace, copy, move). Those operation are not taken into account by NRI.

For instance for ConfigMap

apiVersion: v1
kind: ConfigMap
metadata:
  name: nri-user-defined-injections
  namespace: kube-system
data:
  "customInjection": '{"op": "remove", "path": "/metadata/annotations", "value": {"k8s.v1.cni.cncf.io/networks": "foo-network"}}'

Original POD specification is not modified

apiVersion: v1
kind: Pod
metadata:
  name: testpod
  labels:
    customInjection: "true"
  annotations:
    k8s.v1.cni.cncf.io/networks: foo-network
spec:
  containers:
  - name: app
    image: alpine
    command: [ "/bin/sh", "-c", "sleep INF" ]

Expected to remove foo-network from POD specification.

I only added add operation in the initial implemention as that was the use-case I'd like to use.
I think we can take this as a feature enhancement for future release.

Second use case, for ConfigMap, operation add

apiVersion: v1
kind: ConfigMap
metadata:
  name: nri-user-defined-injections
  namespace: kube-system
data:
  "customInjection": '{"op": "add", "path": "/metadata/annotations", "value": {"k8s.v1.cni.cncf.io/networks": "sriov-net-attach-def"}}'

and above POD definition, I would expect for operation add to have after modification two networks

 k8s.v1.cni.cncf.io/networks: foo-network, sriov-net-attach-def

instead for given key, values are replaced. Current state:

 k8s.v1.cni.cncf.io/networks: sriov-net-attach-def

Another good catch!
I didn't think of such case, but it sounds reasonable to expect the networks be appended to existing list.
I will consider implementing this along with the first use case.

@zshi-redhat zshi-redhat added the enhancement New feature or request label Apr 13, 2021
@MichalGuzieniuk
Copy link
Contributor Author

@zshi-redhat Thank you for comment, sound good for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants