Ability to hide config vars from Plan Output

[heroku]

Ability to hide config vars from Plan Output

[bernerdschaefer]

I like the proposal for separate attributes.

Joe's last comment about having two separate attributes, one for sensitive & non-sensitive vars, could prove to be a good compromise. However, I wonder about a scenario where a user moves a config var between non-sensitive/sensitive. This might cause this config var to be deleted/recreated and restart the application. I'm not sure if this overheard is a good thing to have for the sake of having granular control for config var sensitivity.

I'd guess we'd still make a single API call back to Heroku with all of the config vars. We would make a call where we technically wouldn't need one, but Heroku's API actually handles that fine -- you can set config vars as often as you want but if there's no diff it won't create a new release.

[davidji99]

Heroku's API actually handles that fine -- you can set config vars as often as you want but if there's no diff it won't create a new release. That's good to know then.

[davidji99]

One way would be this on heroku_app:

[joestump]

@talbright config_vars are per-app and providers are for an entire account.

@drn @joslynesser @davidji99 the only way that I know of to hide this from the Terraform output is to mark them all as Sensitive: true. We could do sensitive_config_vars, but that feels a bit awkward. I tend to agree with @drn that the vast majority of these will be sensitive.

How about this? We add DEBUG logs that output the raw config_vars. By default the TF output will continue to be Sensitive: true, but if you need to in a pinch you can force TF to show all config_vars.

[joslynesser]

Thanks @davidji99!

This is definitely a highly subjective change, but it solved our immediate problem of having secrets shown in plan/apply output. Ideally we'd have something like config_vars vs sensitive_config_vars or some other notion of separating sensitive values from the set, as it makes reviewing plans more difficult. Currently we check the heroku app (or current state) vs the code diff to determine what the value is changing from/to, but ideally that would only be for changing secrets and not every single config var. Maybe related to how #47 pans out?

[drn]

👍for this change.

Our team would love to switch our staging environment creation process over to Terraform. However, having terraform print the DATABASE_URL in plain text and have it captured by CI logs is definitely not ideal. We consider our terraform state secure, so we are less concerned about it having it stored there. However, having our database credentials show up in CI logs is a blocker for us.

Would love to hear of a solution for this!

[joslynesser]

I could see folks wanting to see non-sensitive config vars in the diff output. Maybe a separate sensitive_config_vars attribute to differentiate while still keeping the current design? Or a redesign like #47 that allows individual specification on sensitivity?

[drn]

@davidji99 / @joslynesser - most of the config variables i have are sensitive (database url, session secret, api keys, etc). While I understand potentially wanting to log some of the of the config in output, would we be able to change the default config_vars sensitivity before implementing the sensitive_config_vars option?

My team is currently currently blocked on leveraging the heroku provider because we don't want to any CI logs to contain sensitive data.

I've created a pull based on your suggestion, @davidji99. https://github.com/terraform-providers/terraform-provider-heroku/pull/92 Thoughts?

Would love to be able to start using this provider in CI!

[joslynesser]

@drn we went with that exact same solution of marking everything sensitive in our private fork as well, but my guess is that some users won't want all of their heroku_app.config_vars changes hidden if they are used to seeing diffs and do not currently have any sensitive values being managed. It makes it harder to discern what the value is changing to in the terraform plan review when everything is marked as sensitive.

We definitely went for the short term route of flagging everything sensitive like #92 for now until a better solution could get brainstormed, but I'm not sure that everyone else is managing additional secrets via heroku_app.config_vars like some of us are. @joestump any thoughts?

[talbright]

Just a thought -- can we pass options to the provider? Let the user choose to hide all config vars or not, and set the schema accordingly. Not sure yet how that would look if a user toggled that switch back and forth, just brainstorming here.

[talbright]

Yes, that's true and I was assuming that would affect all apps. That being said, your suggestion is not a bad compromise IMO.

[drn]

That sounds like a great plan to me, @joestump! Let me know if there's anything I can help out with to facilitate this

[bernerdschaefer]

Could we set the value for Sensitive based on the environment with something like Sensitive: os.Getenv("HEROKU_SENSITIVE_CONFIG_VARS") == "1"?

@joslynesser Could you share an example of what a plan looks like with config_vars as sensitive? I'm a bit concerned that reviewing plans will be roughly as confusing as they are with this bug hashicorp/terraform#15962.

[talbright]

@bernerdschaefer using an env var to affect behavior crossed my mind initially as well, but when thinking about it I prefer making it an option at the provider level, because its much more explicit, can be documented with the provider, and has plenty of visibility (less surprises.) @joestump was concerned about doing this at the provider/global level, though I'm not 💯 understanding the concern there. Seems like most solutions we have come up with so far affect behavior globally.

If we do end up going the route @joestump suggested, lets make sure to communicate that well in the docs and logging.

[joestump]

Yet another option would be to create the heroku_app_config_vars resource requested in #47 and mark the variables as Sensitive: os.Getenv("HEROKU_SENSITIVE_CONFIG_VARS") == "1" there. We could have two types of variables in that resource as well:

resource "heroku_app_config_vars" "foobar" {
  app = "${heroku_app.foobar.name}"

  var {
    name = "TF_LOG"
    value = "DEBUG"
  }

  secret {
    name = "GITHUB_TOKEN"
    value = "super-secret-123"
  }
}

We'd then mark the secret attribute as Sensitive: true.

[davidji99]

+1 for the separate resource for config vars.

It seems from this conversation, globally defined solutions would be more explicit instead of relying on an env variable which could be set/unset without a proper audit log in place. This might prove disastrous if private keys/tokens/etc were shown in plaintext in a CI system.

Joe's last comment about having two separate attributes, one for sensitive & non-sensitive vars, could prove to be a good compromise. However, I wonder about a scenario where a user moves a config var between non-sensitive/sensitive. This might cause this config var to be deleted/recreated and restart the application. I'm not sure if this overhead is a good thing to have for the sake of having granular control for config var sensitivity.

But if more people want granular control, then they would have to live which the possibility of that overhead I mentioned above.