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

config_hash converts strings to integers => breaks port mappings #119

Closed
weitzj opened this issue May 18, 2015 · 6 comments
Closed

config_hash converts strings to integers => breaks port mappings #119

weitzj opened this issue May 18, 2015 · 6 comments

Comments

@weitzj
Copy link
Contributor

weitzj commented May 18, 2015

In the consul config_hash it is possible to disable the http port by setting it to -1, and enabling the https service instead by setting it to the appropiate port, 8500:

      'ports' =>              {
        'http'  =>  -1,
        'https' => 8500,
      },

The generated config.json results in:

  "ports": {
    "https": "8500",
    "http": -1
  },

Current behaviour (Release 1.0.0)

-1 is looking good, and is still an integer, whilst the https ports was converted to a string. The consul agent does not support strings as port numbers and therefore fails to launch.

Expected behaviour

-1 as well as 8500 should be integers.

Workaround

Tell puppet to treat "8500" as an integer by multiplying with 1 *:

      'ports' =>              {
        'http'  =>  -1,
        'https' =>  1 * 8500,
      },
@solarkennedy
Copy link
Contributor

Yea, we explicitly try to "cast" these into ints for service:
https://github.com/solarkennedy/puppet-consul/blob/master/manifests/service.pp#L49

Then we do it on a bit more keys here:
https://github.com/solarkennedy/puppet-consul/blob/master/manifests/config.pp#L82-L98

I'm... up for suggestions?

We could take a advantage of the fact that we have a custom function to write the json down:
https://github.com/solarkennedy/puppet-consul/blob/master/lib/puppet/parser/functions/consul_sorted_json.rb

In ruby we could look at each key, and if it looks like an int, make it an int?
Seems a little dangerous, but a tad better than whitelisting keys?

@aj-jester
Copy link

Something like this?

irb(main):001:0> iffy_integers = [-1, '+1', '-goats', 'moats-', '8500', '2+3', '-3454', '-this is A-']
=> [-1, "+1", "-goats", "moats-", "8500", "2+3", "-3454", "-this is A-"]

irb(main):002:0> iffy_integers.map { |i| (i.kind_of?(String) and i.match(/\A[-]?[0-9]+\z/)) ? i.to_i : i }
=> [-1, "+1", "-goats", "moats-", 8500, "2+3", -3454, "-this is A-"]

@solarkennedy
Copy link
Contributor

Yea, that looks like it achieves the desired effect.
@aj-jester want me to whip that into a PR or did you want to?

@aj-jester
Copy link

@solarkennedy #156

@solarkennedy
Copy link
Contributor

@weitzj this should be fixed on the latest master thanks to @aj-jester

@weitzj
Copy link
Contributor Author

weitzj commented Jul 17, 2015

Thank you. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants