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

consul_config: Remove default values #447

Merged
merged 1 commit into from
Jun 11, 2017

Conversation

legal90
Copy link
Contributor

@legal90 legal90 commented May 31, 2017

We don't need to set these defaults values because:

  1. These default values are rendered to the consul.json config file only if an appropriate argument was called in the consul_config resource source code. For example, currently we have verify_incoming and verify_outgoing always rendered in the config file because they are called here: https://github.com/johnbellone/consul-cookbook/blob/master/libraries/consul_config.rb#L208

  2. Consul can handle default values by itself.
    We just don't need to render them in the config file unless we really need to set them explicitly. It makes a big sense for consul config options which have different default values in different Consul agent versions. For example, acl_enforce_version_8 defaults to false in versions of Consul prior to 0.8, and defaults to true in Consul 0.8 and later.

This PR doesn't break the backward compatibility. No explicitly defined variables will be removed.

@legal90 legal90 requested review from johnbellone and Ginja May 31, 2017 08:31
@legal90 legal90 modified the milestone: v3.0.0 May 31, 2017
We don't need to set these defaults values because:
1) These default values are rendered to the `consul.json` config file only if an appropriate argument was called in the `consul_config` resource source code. For example, currently we have `verify_incoming` and `verify_outgoing` always rendered in the config file because they are called there, in #tls? helper.

2) Consul can handle default values by itself.
We just don't need to render them in the config file unless we really need to set them explicitly. It makes a big sense for consul config options which have different default values in different Consul agent versions. For example, `acl_enforce_version_8` defaults to _false_ in versions of Consul prior to 0.8, and defaults to _true_ in Consul 0.8 and later.
@legal90
Copy link
Contributor Author

legal90 commented May 31, 2017

I've fixed the spec as well.
As we see, now the config file will contain only those options, which have been explicitly set by resource param and/or node's attributes ['consul']['config'][<opt_name>]

@legal90
Copy link
Contributor Author

legal90 commented May 31, 2017

@johnbellone Could you please disable codecov integration for Pull-Requests in this repo?
We can still use it for a badge in README.md, but let's keep PRs green.

@legal90 legal90 merged commit 3ffc256 into sous-chefs:master Jun 11, 2017
@legal90 legal90 deleted the config-delete-defaults branch June 11, 2017 12:14
@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

Successfully merging this pull request may close these issues.

1 participant