search

getporter / helm2-mixin

Helm mixin for Porter
https://porter.sh/mixins/helm
Apache License 2.0
13 stars 7 forks source link

fix(schema): update set schema to allow for various types #30

Closed vdice closed 5 years ago

vdice commented 5 years ago

Updates the set portion of this mixin's schema to allow for various types.

Credit to @carolynvs for the change/fix. See https://github.com/deislabs/porter-helm/pull/28#issuecomment-478072977

Tested via make test-cli from https://github.com/deislabs/porter/pull/246

vdice commented 5 years ago

@carolynvs thanks -- updated with additional bool and integer inputs.

Though, I realized, our use of map[string]string for unmarshaling means the input yaml needs to concatenate keys into one line (that are usually multi-level in yaml). So, if in standard yaml,

livenessProbe:
  initialDelaySeconds: 30

currently needs to be input as

livenessProbe.initialDelaySeconds: 30

Are we good to leave this as is? Or do you think we should migrate to use of a map[string]interface{} (either in a future ticket or this changeset)?

carolynvs commented 5 years ago

Oops! I didn't realize that would limit nesting. Yeah okay we should go with what you had originally. I'm sorry for sending you off on a wild goose chase! 😞

vdice commented 5 years ago

@carolynvs no problem at all -- I didn't quite realize these most recent implications as well.

I'm going to propose we merge this PR as is (retaining same struct for unmarshal-ing) and then create a refactor ticket to change to a new struct. I started doing this and realize it may require quite a few changes... What do you think?

carolynvs commented 5 years ago

I'm happy with organizing the changes however you are comfortable.