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

create-setter ignores array type in Kptfile #698

Closed
shawnzhu opened this issue Jun 6, 2020 · 3 comments
Closed

create-setter ignores array type in Kptfile #698

shawnzhu opened this issue Jun 6, 2020 · 3 comments
Assignees
Labels
bug Something isn't working p0 size/L 4 day triaged Issue has been triaged by adding an `area/` label

Comments

@shawnzhu
Copy link

shawnzhu commented Jun 6, 2020

$ kpt version
0.27.0

Problem

Given:

# input.yaml
apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  name: ml-pipeline-tensorboard-ui
spec:
  gateways:
  - "kubeflow-gateway"
  hosts:
  - '*'

And Kptfile:

kind: Kptfile
openAPI:
  definitions: {}

Run:

$ kpt cfg create-setter tmp/ \
  istio-gateway "kubeflow-gateway" \
  --type array --field "spec.gateways"
$ kpt cfg list-setters tmp/
Error: The input value doesn't validate against provided OpenAPI schema: validation failure list:
istio-gateway in body must be of type array: "string"

While the expected output is:

$ kpt cfg list-setters tmp/
      NAME                       VALUE                  SET BY            DESCRIPTION             COUNT                                  
  istiogateway    [istio-system/istio-ingressgateway]                                             2

Diagnose

When checking Kptfile, I've noticed that it uses value instead of listValues under setter:

# Kptfile
kind: Kptfile
openAPI:
  definitions: {io.k8s.cli.setters.istio-gateway: {type: array, x-k8s-cli: {setter: {
          name: istio-gateway, value: kubeflow-gateway}}}}

While the expected Kptfile should be:

openAPI:
  definitions:
    io.k8s.cli.setters.istiogateway:
      type: array
      x-k8s-cli:
        setter:
          name: istiogateway
          listValues:
          - "istio-system/istio-ingressgateway"

According to https://googlecontainertools.github.io/kpt/guides/producer/setters/#setting-lists

Note: Move the setter reference from the element (- "a") to the list (list: )

another problem is the field reference in updated input.yaml is not in the expected line (should be at the line of gateways:):

Actual result:

apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  name: ml-pipeline-tensorboard-ui
spec:
  gateways:
  - "kubeflow-gateway" # {"$ref":"#/definitions/io.k8s.cli.setters.istio-gateway"}
  hosts:
  - '*'
@shawnzhu
Copy link
Author

shawnzhu commented Jun 6, 2020

Discovered when trying to using kpt setter in kubeflow/manifests#1211

@mortent mortent added area/cfg bug Something isn't working p0 triaged Issue has been triaged by adding an `area/` label labels Jun 7, 2020
@mortent mortent added this to the Milestone 2 (Starts 2020-Q2) milestone Jun 7, 2020
@phanimarupaka
Copy link
Contributor

phanimarupaka commented Jun 7, 2020

@shawnzhu As mentioned here you should move the reference to the list key and it works fine.

kpt cfg create-setter /tmp istio-gateway "kubeflow-gateway" --type array --field "spec.gateways"

Move the reference and the input.yaml. This is a manual step currently for creating setter for list-item

apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  name: ml-pipeline-tensorboard-ui
spec:
  gateways: # {"$kpt-set":"istio-gateway"}
  - "kubeflow-gateway"
  hosts:
  - '*'

And list-setters works fine

kpt cfg list-setters /tmp
     NAME             VALUE         SET BY   DESCRIPTION   COUNT  
 istio-gateway   kubeflow-gateway                          0   

@shawnzhu
Copy link
Author

shawnzhu commented Jun 8, 2020

Note: Currently create-setter will not directly create a setter reference for a list field. The simplest way to create a list setter is to create a setter for one of the elements, and then move the reference to the list field.

@phanimarupaka notes taken, that's the workaround I'm using now. Thanks. But is this by design? b/c there will be lots of work that moves field references from the source YAML.

And list-setters works fine

Looks like the value becomes string instead of array. it's going to be a problem if there're multiple values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p0 size/L 4 day triaged Issue has been triaged by adding an `area/` label
Projects
None yet
Development

No branches or pull requests

3 participants