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

upgrading old config fails with unsupported type error #1585

Closed
cpoole opened this issue Feb 1, 2019 · 1 comment
Closed

upgrading old config fails with unsupported type error #1585

cpoole opened this issue Feb 1, 2019 · 1 comment
Labels
area/config deploy/helm good first issue Good for newcomers kind/bug Something isn't working

Comments

@cpoole
Copy link

cpoole commented Feb 1, 2019

Expected behavior

my file should correctly parse and update

Actual behavior

parsing fails but file is valid and works if we just manually change the apiVersion.

Information

  • Skaffold version: 0.21.0 to 0.22.0
  • Operating system: linux - ubuntu 18.04
  • Contents of skaffold.yaml:
apiVersion: skaffold/v1beta3
kind: Config
build:
  artifacts:
  - image: local/app1
    context: .
  - image: local/redis-cl
    context: ./redis-local/
deploy:
  helm:
    releases:
    - name: app1
      chartPath: app1
      values:
        image: local/app1
        redisImage: local/redis-cl
      overrides:
        global:
          localstack:
            enabled: true
          environment: "local"
      namespace: "default"
      wait: false
      recreatePods: false

Steps to reproduce the behavior

run skaffold fix and the following is printed:

WARN[0000] config version (skaffold/v1beta3) out of date: upgrading to latest (skaffold/v1beta4) 
FATA[0000] creating runner: parsing skaffold config: transforming skaffold config: converting deploy config: marshalling old: json: unsupported type: map[interface {}]interface {} 

but if I simply update the apiVersion to beta4 it works fine and skaffold is totally happy actually parsing the config

@dgageot dgageot added kind/bug Something isn't working deploy/helm area/config labels Feb 4, 2019
@nkubala
Copy link
Contributor

nkubala commented Feb 4, 2019

I think this has actually been broken for a long time. Golang's json package only supports maps with string keys, so the Overrides field in the HelmRelease breaks this because it's a map[string]interface{}.

func convert(old interface{}, new interface{}) error {
o, err := json.Marshal(old)
if err != nil {
return errors.Wrap(err, "marshalling old")
}

skaffold fix won't work on any config that has overrides set for helm releases.

@balopat balopat added the good first issue Good for newcomers label Feb 5, 2019
corneliusweig pushed a commit to corneliusweig/skaffold that referenced this issue Feb 12, 2019
…1585)

Helm overrides have type `map[string]interface{}` which cannot be serialized to json.

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
#####
corneliusweig pushed a commit to corneliusweig/skaffold that referenced this issue Feb 12, 2019
…tainerTools#1585)

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
corneliusweig pushed a commit to corneliusweig/skaffold that referenced this issue Feb 12, 2019
…rrides (GoogleContainerTools#1585)

Helm overrides have type `map[string]interface{}` which cannot be serialized to json. Implement a custom serialization which includes the overrides as a yaml-string in json. This implementation is forwards-compatible, so that it will automatically work for future apiVersions.

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
corneliusweig pushed a commit to corneliusweig/skaffold that referenced this issue Feb 12, 2019
Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
corneliusweig pushed a commit to corneliusweig/skaffold that referenced this issue Mar 10, 2019
…1585)

Helm overrides have type `map[string]interface{}` which cannot be serialized to json.

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
#####
corneliusweig pushed a commit to corneliusweig/skaffold that referenced this issue Mar 10, 2019
…tainerTools#1585)

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
corneliusweig pushed a commit to corneliusweig/skaffold that referenced this issue Mar 10, 2019
…rrides (GoogleContainerTools#1585)

Helm overrides have type `map[string]interface{}` which cannot be serialized to json. Implement a custom serialization which includes the overrides as a yaml-string in json. This implementation is forwards-compatible, so that it will automatically work for future apiVersions.

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
corneliusweig pushed a commit to corneliusweig/skaffold that referenced this issue Mar 10, 2019
Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
balopat pushed a commit that referenced this issue Mar 15, 2019
* config upgrade: handle helm overrides manually (#1585)

Helm overrides have type `map[string]interface{}` which cannot be serialized to json. Implement a custom serialization which includes the overrides as a yaml-string in json. This implementation is forwards-compatible, so that it will automatically work for future apiVersions.

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config deploy/helm good first issue Good for newcomers kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants