Closes #150: EB diffs settings correctly

[artburkart]

All this commit does is add a call to normalizeJsonString, so the
settings returned by the AWS API and the settings set by users in the tf
config don't indicate diffs in the tf state whenever there's extra
whitespace in the JSON. I added a case to the structure_test.go file, to
verify that normalizeJsonString treats regular strings the way I'd
expect.

I would have added an acceptance test, but the 15-20 minute feedback loop
is far too long to be practical. I tried figuring out a way to spoof the
flow instead, but had no luck.

Fixes #150

[artburkart]

Alternatively, I think I could've used DiffSuppressFunc for this purpose?

[artburkart]

@Ninir - General question: what distinguishes an enhancement vs a bug? Thanks! :)

[Ninir]

Hey @artburkart,

In this case, I understood that the diff was working, but needed improvements. If it had been a bug, like the diff was not correct, I would have put this one instead.
Not arbitrary, just a personal thought on this 😄

[artburkart]

Fair enough. I didn't think it was arbitrary at all because I'd noticed you'd tagged other things as bugs that I'd maybe consider enhancements and vice versa. It's all about perspective. Thanks for enlightening me 😊

[radeksimko]

Hi @artburkart
thanks for submitting this PR.

I read through the official docs and I couldn't find any option that would accept JSON: https://docs.aws.amazon.com/elasticbeanstalk/latest/dg/command-options-general.html

I'm happy to help cobbling the acceptance test, but can you at least provide some hints - e.g. plan output with the affected resource & option so I can reproduce it?

[artburkart]

@radeksimko - Thanks for the offer. Here are some links for it. Please let me know if you'd like more:
Original Issue: hashicorp/terraform#6257
New issue in this repo: #150
AWS docs: https://docs.aws.amazon.com/elasticbeanstalk/latest/dg/command-options-general.html#command-options-general-elasticbeanstalkhealthreporting

Look for "configdocument" in the AWS docs.

Cheers

[radeksimko]

@artburkart That's very useful, thanks!

I'll cobble an acceptance test together and push that to your branch once ready.

[radeksimko]

I gave it a few hours of experimenting and I could not find a working example with configdocument. There's clearly a bunch of other options that need to be aligned with this one. It is also possible that there's another bug in that resource.

Here's my example:

resource "aws_elastic_beanstalk_application" "default" {
  name = "tf-test-name-example"
  description = "tf-test-desc"
}

resource "aws_elastic_beanstalk_application_version" "default" {
  application = "${aws_elastic_beanstalk_application.default.name}"
  name = "tf-test-version-label-example"
  bucket = "elasticbeanstalk-samples-us-west-2"
  key = "python-sample-20150402.zip"
}

resource "aws_elastic_beanstalk_environment" "default" {
  name = "tf-test-name-example"
  application = "${aws_elastic_beanstalk_application.default.name}"
  version_label = "${aws_elastic_beanstalk_application_version.default.name}"
  solution_stack_name = "64bit Amazon Linux 2017.03 v2.4.1 running Python 3.4"

  setting {
    name = "IamInstanceProfile"
    resource = "AWSEBAutoScalingLaunchConfiguration",
    namespace = "aws:autoscaling:launchconfiguration",
    value = "aws-elasticbeanstalk-ec2-role"
  }

  setting {
    name = "ELBScheme"
    namespace = "aws:ec2:vpc",
    value = "public"
  }

  setting {
    name = "ServiceRole"
    namespace = "aws:elasticbeanstalk:environment",
    value = "aws-elasticbeanstalk-service-role"
  }

  setting {
    name = "ConfigDocument"
    namespace = "aws:elasticbeanstalk:healthreporting:system",
    value = "{\"Version\":1,\"CloudWatchMetrics\":{\"Instance\":{\"CPUIrq\":null,\"LoadAverage5min\":null,\"ApplicationRequests5xx\":null,\"ApplicationRequests4xx\":null,\"CPUUser\":null,\"LoadAverage1min\":null,\"ApplicationLatencyP50\":null,\"CPUIdle\":null,\"InstanceHealth\":null,\"ApplicationLatencyP95\":null,\"ApplicationLatencyP85\":null,\"RootFilesystemUtil\":null,\"ApplicationLatencyP90\":null,\"CPUSystem\":null,\"ApplicationLatencyP75\":null,\"CPUSoftirq\":null,\"ApplicationLatencyP10\":null,\"ApplicationLatencyP99\":null,\"ApplicationRequestsTotal\":null,\"ApplicationLatencyP99.9\":null,\"ApplicationRequests3xx\":null,\"ApplicationRequests2xx\":null,\"CPUIowait\":null,\"CPUNice\":null},\"Environment\":{\"InstancesSevere\":null,\"InstancesDegraded\":null,\"ApplicationRequests5xx\":null,\"ApplicationRequests4xx\":null,\"ApplicationLatencyP50\":null,\"ApplicationLatencyP95\":null,\"ApplicationLatencyP85\":null,\"InstancesUnknown\":null,\"ApplicationLatencyP90\":null,\"InstancesInfo\":null,\"InstancesPending\":null,\"ApplicationLatencyP75\":null,\"ApplicationLatencyP10\":null,\"ApplicationLatencyP99\":null,\"ApplicationRequestsTotal\":null,\"InstancesNoData\":null,\"ApplicationLatencyP99.9\":null,\"ApplicationRequests3xx\":null,\"ApplicationRequests2xx\":null,\"InstancesOk\":null,\"InstancesWarning\":null}}}"
  }

  setting {
    name = "HealthCheckSuccessThreshold"
    namespace = "aws:elasticbeanstalk:healthreporting:system",
    value = "Ok"
  }

  setting {
    name = "SystemType"
    namespace = "aws:elasticbeanstalk:healthreporting:system",
    value = "enhanced"
  }
}

which Beanstalk API apparently "accepts" (in the sense that it returns 200), but ignores some settings like IamInstanceProfile when returning it back from the API which causes spurious diffs.
If I leave it out I get an error instead:

* aws_elastic_beanstalk_environment.default: Error waiting for Elastic Beanstalk Environment (e-csxfwjgrxv) to become ready: 2 error(s) occurred:

		* 2017-07-19 16:25:02.326 +0000 UTC (e-csxfwjgrxv) : You must specify an Instance Profile for your EC2 instance in this region. See [Managing Elastic Beanstalk Instance Profiles](http://docs.aws.amazon.com/elasticbeanstalk/latest/dg/iam-instanceprofile.html) for more information.
		* 2017-07-19 16:25:02.437 +0000 UTC (e-csxfwjgrxv) : Failed to launch environment.

---

I do genuinely believe this needs fixing, but we need a working example which we can turn into a test. Do you mind providing one @artburkart please?

[Ninir]

@radeksimko I faced the exact same issue as the author in a few projects.
Parameters ignored: RDS-ones, subnets order, ...

If you folks want, I could easily share reproducible configurations :).
Also, just to let you know: you can't leave an empty parameter: if you don't want to set it, just omit it.

[artburkart]

Since nothing has changed in the code as far as I could tell when I looked at it, I think the full config I provided in the terraform repo should still work too.

hashicorp/terraform#6257

[radeksimko]

Thanks for all the pointers and patience. Test added, so we can now merge this 🎉

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!