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

helper/schema: Normalize bools to "true"/"false" in diffs #6499

Merged
merged 1 commit into from
May 8, 2016
Merged

Commits on May 5, 2016

  1. helper/schema: Normalize bools to "true"/"false" in diffs

    For a long time now, the diff logic has relied on the behavior of
    `mapstructure.WeakDecode` to determine how various primitives are
    converted into strings.  The `schema.DiffString` function is used for
    all primitive field types: TypeBool, TypeInt, TypeFloat, and TypeString.
    
    The `mapstructure` library's string representation of booleans is "0"
    and "1", which differs from `strconv.FormatBool`'s "false" and "true"
    (which is used in writing out boolean fields to the state).
    
    Because of this difference, diffs have long had the potential for
    cosmetically odd but semantically neutral output like:
    
        "true" => "1"
        "false" => "0"
    
    So long as `mapstructure.Decode` or `strconv.ParseBool` are used to
    interpret these strings, there's no functional problem.
    
    We had our first clear functional problem with #6005 and friends, where
    users noticed diffs like the above showing up unexpectedly and causing
    troubles when `ignore_changes` was in play.
    
    This particular bug occurs down in Terraform core's EvalIgnoreChanges.
    There, the diff is modified to account for ignored attributes, and
    special logic attempts to handle properly the situation where the
    ignored attribute was going to trigger a resource replacement. That
    logic relies on the string representations of the Old and New fields in
    the diff to be the same so that it filters properly.
    
    So therefore, we now get a bug when a diff includes `Old: "0", New:
    "false"` since the strings do not match, and `ignore_changes` is not
    properly handled.
    
    Here, we introduce `TypeBool`-specific normalizing into `finalizeDiff`.
    I spiked out a full `diffBool` function, but figuring out which pieces
    of `diffString` to duplicate there got hairy. This seemed like a simpler
    and more direct solution.
    
    Fixes #6005 (and potentially others!)
    phinze committed May 5, 2016
    Configuration menu
    Copy the full SHA
    b4df304 View commit details
    Browse the repository at this point in the history