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

Dangerous reordering of Carbon .ini sections #241

Closed
satiani opened this issue Jan 27, 2016 · 3 comments
Closed

Dangerous reordering of Carbon .ini sections #241

satiani opened this issue Jan 27, 2016 · 3 comments

Comments

@satiani
Copy link

satiani commented Jan 27, 2016

Right now in cookbooks/graphite/libraries/chef_graphite.rb the INI file sections are generated as such:

     def generate_conf_data(data)
      tuples = sort_tuples(section_tuples(data))

       result = Hash.new
       tuples.each { |tuple| result[tuple.first] = tuple.last }
       result
     end

    def sort_tuples(tuples)
      tuples.sort { |a, b| a.first <=> b.first }
    end

Which means that sections are reordered by section name alphabetically rather than by the order they are defined in the file. This is dangerous since carbon is order-sensitive when it parses the rules in INI files. For example, in single_node.rb example in the cookbook, you have the following:

graphite_storage_schema "carbon" do
  config ({
            pattern: "^carbon\.",
            retentions: "60:90d"
          })
end

graphite_storage_schema "default_1min_for_1day" do
  config ({
            pattern: ".*",
            retentions: "60s:1d"
          })
end

In this case, the insertion order is the same as the alphabetic order ("carbon" then "default_1min_for_1day"). If you switch the names around the pattern .* will catch everything and the "carbon" rule will never be matched, resulting in completely different behavior and expensively unexpected results.

@satiani
Copy link
Author

satiani commented Jan 27, 2016

The solution for ruby 1.9+ is simple: remove sort_tuples function. For earlier versions, you'll have to use some ordered hash implementation throughout or to keep the order in an independent list.

@cwjohnston
Copy link
Contributor

Duplicates #218

@lock
Copy link

lock bot commented Apr 25, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants