Fix query parameters handling on API requests

[kgacheva]

This introduces a mechanism for resolving the query parameters stored in the args dictionary in all actions. These optional parameters were previously not passed along with the API request in the K8sClient and were not resolved. The Jinja template for the action script files has been modified to include a params dictionary within the args holding the query parameters key-value pairs. The k8s base client now consumes the params dictionary and passes it as data for the URL query string upon making the request to the API. All actions have been regenerated to reflect the changes in the template.

[Kami]

Thanks for your contribution :+1:

It would be a bit easier to review it if you could split it into two commits - one which makes change / fix to the code gen script and the other which re-generates the files.

In addition to that, it would be great if you could add a test case for it so we can avoid regressions and similar issues in the future.

[kgacheva]

Thanks for your quick response, @Kami! It is my bad that I did it in a single commit, I just wanted to make sure it will be easier if the changes need to be reverted. I will fix this and think about such a test case as well.

[kgacheva]

@Kami, I have reworked and force-pushed the commit to my forked repo and it now includes solely the changes to the action scripts' Jinja template and to the k8s client base class, which fix the passing of query parameters upon making requests to the k8s API. I hope it will be easier for you to review the proposed fix in this way.

However, in the process of testing those changes, I have identified an additional issue which has to do with how the responses from the API are handled. In certain actions, such as the one referenced below, the line https://github.com/StackStorm-Exchange/stackstorm-kubernetes/blob/c277fe0a3634f3c40927758ea9cccc26fbd6b9ce/actions/readCoreV1NamespacedPodLog.py#L70 fails upon receiving responses with Success status code as the response cannot be decoded as a JSON. This happens because the response from the API itself comes in plain text, as stated in the Kubernetes API Reference Documentation, with supported Accept headers text/plain, application/json, application/yaml, application/vnd.kubernetes.protobuf as extracted from the Swagger. Note that the reference here is to the newest version of the API but the same is observed in older versions as well.

With this in mind, I believe that it wouldn't hurt if we add exception handling for the JSON decoding directly to the template for the action scripts with the purpose of providing a fallback mechanism for similar situations. I will go on and push the change in the context of this same pull request and will then re-generate the actions to include the fixtures for both encountered issues.

Let me know what you think about those proposals.

[Kami]

@kgacheva Thanks.

This seems like a reasonable solution for that problem, indeed.

[Kami]

LGTM, thanks :+1:

As a future improvement and not directly related your change - we could do some refactoring in the future and move some of the common functionality in a base class so we wouldn't need to repeat / duplicate that much code in each auto-generated action.

[Kami]

Can you please also update CHANGELOG file and bump version in pack.yaml?

[kgacheva]

@Kami, thanks for your prompt review! Indeed refactoring the code to move out the common logic and eliminate duplication sounds like the reasonable step forward. For now, I will bump the version and update the CHANGELOG so that we can, hopefully, proceed with merging those changes soon. I have also noticed that the CHANGELOG has not been updated for the modifications in version 0.9.11 so I will add such a line as well for the sake of completeness.

[Kami]

Merged.

Thanks for your great contribution :+1: