kubernetes-client/haskell

Issue with (-&-) optional argument operator and Command optional argument for exec

polygonhell opened this issue · 18 comments

In the Exec entry point

Command has to be supplied multiple times in the query string in order to pass parameters

(CoreV1.connectGetNamespacedPodExec (Accept MimeAny) name namespace) -&- 
                (Command "ls") -&- 
                (Command "/") -&- 
                (Stdout True) -&- 
                (Stderr True)

should result in a query string of
?command=ls&command=/&stdout=true&stderr=true

but actually results in a query string of
?command=/&stdout=true&stderr=true

It looks like the (-&-) operator currently only allows setting a single value.

I might change the generator to have (-&-) default to appending values instead of replacing values, but will have to think about the the right default is.

In the meantime, you can use the setQuery function directly to set a list of parameters with the same name at the same time, and they will all be output in the final querystring.

CoreV1.connectGetNamespacedPodExec (Accept MimeAny) name namespace
  `setQuery` (toQuery ("command", Just "ls") ++ toQuery ("command", Just "/"))
  -&- (Stdout True)
  -&- (Stderr True) 

Thanks, no real rush on this.
I already had a workaround, in order to deal with the transition to web socket in the exec query I have to add a bunch of headers to the query anyway.
I just noticed this as dealing with that.

One additional point, that might influence which way you decide to go with a solution this came up again with label selectors.

ah, good to know. I think part of what exacerbates this is that in the kubernetes swagger spec it looks like labelSelector is typed as a string query parameter for many of these operations, so the generator doesn't know that this can accept multiple values

but i think it may make more sense to append rather than replace, since the use-case for replacing doesn't seem like something that would come up very often

Yes, the problem is one common way collections are passed in query strings is to just repeat them in query.
Which leaves the swagger specs under defined.
One option if your not comfortable changing (-&-) would be to add a second operator (-&&-) that has the add's rather than replaces.
Yes I can't imagine when the replace would matter.

@polygonhell Is the spec actually asking for a CSV list instead of multiple instances of the query parameter here? or do both work?

https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#list-and-watch-filtering

that would be compatible with the string type in the swagger spec.

Still might change the generator to append, but I wonder if building the CSV list as a string and setting that as a single parameter also works for you.

I think both work, my test seemed to work with multiple instances.
But it wasn't exactly exhaustive, I'll try again later when I have access to the machine

My error it looks like if you pass 2 labelSelectors it's just ignoring the second one, the correct syntax is as you state comma separated.

/close

@jonschoning: Closing this issue.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Please re-open if the problem still persists

/re-open
Sorry to clarify the command problem still exists, the labelselector issue does not.

/reopen

@akshaymankar: Reopened this issue.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ok i'll put in an update to the generator

/close
resolved with #79

@jonschoning: Closing this issue.

In response to this:

/close
resolved with OpenAPITools/openapi-generator#7805

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.