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

Fix --var variables merging into config file variables #97

Merged
merged 2 commits into from
Mar 11, 2019
Merged

Fix --var variables merging into config file variables #97

merged 2 commits into from
Mar 11, 2019

Conversation

KevinFUA
Copy link
Contributor

@KevinFUA KevinFUA commented Mar 9, 2019

Context

Note: Run with ruby 2.5.3

We're planning to have a series of configuration files (shot.yaml, shot-prod.yaml, etc.) and then override some values at deploy time using the --var option.

So trying this out with:

mortar --fire -c shot.yaml --var components.server.image.tag=finalfinalv4

# shot.yaml
variables:
  components:
    server:
      replicaCount: 3
      image:
        ..
        tag: finalfinalv3
        ..

What we would expect is that the tag value gets updated to finalfinalv4 while other values stay the same

However what we get is:

ERROR: NoMethodError : undefined method `replicaCount' for nil:NilClass (...yaml:14)

See: 'mortar fire --help'

Issue

This seems to be a bug caused by a discrepancy in between the outputs of YAML.safe_load and variables_hash

YAML outputs a hash of string keys
variables_hash outputs a hash with symbol keys

Which means in Config.variables when it runs deep_merge! we get:

yaml_vars = { 'a' => { 'b' => 3 } }
var_vars = { :a => { :c => 3 } }
merged_vars = yaml_vars.dup.deep_merge!(var_vars)
# { 'a' => { 'b' => 3 }, :a => { :c => 3 } }

When finally passed to RecursiveOpenStruct, it'll read from either the string or symbol version (typically the latter it seems like) which means it drops the values from the other.

Fix

This PR forces variables_hash to use string keys so the keys match with YAML.safe_load. This will result in deep_merge! properly merging the keys.

@jakolehm jakolehm requested review from jnummelin and kke March 9, 2019 10:25
@jakolehm jakolehm added the bug Something isn't working label Mar 9, 2019
@jakolehm jakolehm merged commit f0e7b24 into kontena:master Mar 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants