joan38 / kubernetes-client

A Kubernetes client for Scala
Apache License 2.0
122 stars 41 forks source link

Incorrect `AuthInfoExec` model #242

Closed jchapuis closed 1 year ago

jchapuis commented 1 year ago

I believe that the env field in

case class AuthInfoExec(
    ...,
    env: Option[Map[String, String]],
    ...
)

should rather by typed with smth like

List[Map[String, String]]

as this is a list in the config API.

At the moment, parsing a valid config with

 env:
      - name: AWS_PROFILE
        value: Dev

leads to the following decoding error:

DecodingFailure at .users[0].user.exec.env: Got value '[{"name":"AWS_PROFILE","value":"Dev"}]' with wrong type, expecting object

yurique commented 1 year ago

Interesting. I must have misread the spec when implementing this initially.

For reference, here's the spec: https://kubernetes.io/docs/reference/config-api/kubeconfig.v1/#ExecConfig

It does say that env is an array of ExecEnvVar, with ExecEnvVar being an object with two fields: name and value.

I'll prepare a PR with a fix later today.

yurique commented 1 year ago

@jchapuis: I just opened a PR addressing this (https://github.com/joan38/kubernetes-client/pull/243).

I don't have a kubeconfig that has env vars specified, so not sure how to test this.

Would you be willing to publish-local and test it out? :)

./mill kubernetes-client[2.13.10].publishLocal
jchapuis commented 1 year ago

@yurique great thanks for the quick answer and fix! will try it now

jchapuis commented 1 year ago

perfect it's working, i've tested it with my config which has the pattern I mentioned here. Thanks for the super quick fix!

yurique commented 1 year ago

Awesome! I can't release it, so we'll have to wait for @joan38 to get some time to spare :)

There's also a couple of other PRs that would be nice to have released as well:

joan38 commented 1 year ago

Hi guys, Sorry for the late answer here, I've been busy upgrading projects at work to Scala 3 😃 Thanks @jchapuis for reporting the bug and thanks @yurique for the fix and all other PRs too.

jchapuis commented 1 year ago

@joan38 any chance for a release including this fix? 🙏

joan38 commented 1 year ago

Looking at the 2 PRs from @yurique and will make a release