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

New ssm parameters features #1295

Closed
wants to merge 44 commits into from

Conversation

jocgir
Copy link
Contributor

@jocgir jocgir commented Aug 1, 2017

Hi,

I made some changes on the aws_ssm_parameter resource.

I added support for the new features (description, tags and allowed pattern).
I also fix some problems with the current implementation. If a parameter was manually deleted from the parameter store, that causes problems with terraform plan, apply and destroy. A resource that no longer exists must not be considered as an error.

I also changed the default value for overwrite. Forcing the value to be false by default is not very useful in a terraform project. If we wish to preserve the value, we should use the default lifecycle rules to protect manually changed value. Setting this new parameter to false by default causes breaking change in the already written code since this parameter did not exists before and the values were overwritten by default.

jocgir added 4 commits July 29, 2017 12:19
… error if the parameter no longer exist. If a user manually delete a parameter, Terraform is no longer able to recreate it or destroy it.

The defautt value for overwrite should be true. Otherwise, this is causing a breaking change with previously written code where overwrite is not set and the behaviour was to overwrite the parameter by default. Moreover, terraform already provides a mechanism to handle the lifecycle of the values.
…meter. Destroy was not working when parameters are deleted.
@grubernaut grubernaut added the enhancement Requests to existing resources that expand the functionality or scope. label Aug 2, 2017
Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jocgir ! Thank you for the contribution here. It seems that at this time there are some additional things included? Or perhaps this simply needs a rebase from the master branch?

jocgir and others added 19 commits August 28, 2017 13:07
This reverts commit 4219326.
…new-ssm-parameters-features"

This reverts commit d2a7937, reversing
changes made to 9eeab57.
Rework of hashicorp#1093
Fixes: hashicorp#382

```
% make testacc TEST=./aws TESTARGS='-run=TestAccAWSRDSCluster_updateIamRoles'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSRDSCluster_updateIamRoles -timeout 120m
=== RUN   TestAccAWSRDSCluster_updateIamRoles
--- PASS: TestAccAWSRDSCluster_updateIamRoles (170.35s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	170.374s
```
The AWS docs say this:

> The instance state to which you want to attach the lifecycle hook. For
a list of lifecycle hook types, see describe-lifecycle-hook-types .
> This parameter is required for new lifecycle hooks, but optional when
updating existing hooks.

The actual behavior of the code (0.9.11) is to output this error
message at creation time and at update time.

```
1 error(s) occurred:

* aws_autoscaling_lifecycle_hook.fabio: "lifecycle_transition": required
field is not set
```

An example of a broken config would look like this:

```
resource "aws_autoscaling_group" "foobar" {
  availability_zones   = ["us-west-2a"]
  name                 = "terraform-test-foobar5"
  health_check_type    = "EC2"
  termination_policies = ["OldestInstance"]

  tag {
    key                 = "Foo"
    value               = "foo-bar"
    propagate_at_launch = true
  }
}

resource "aws_autoscaling_lifecycle_hook" "foobar" {
  name                   = "foobar"
  autoscaling_group_name = "${aws_autoscaling_group.foobar.name}"
  default_result         = "CONTINUE"
  heartbeat_timeout      = 2000
  # lifecycle_transition   = "autoscaling:EC2_INSTANCE_LAUNCHING"

  notification_metadata = <<EOF
{
  "foo": "bar"
}
EOF

  notification_target_arn = "arn:aws:sqs:us-east-1:444455556666:queue1*"
  role_arn                = "arn:aws:iam::123456789012:role/S3Access"
}
``
…nt panic

Fixes: hashicorp#581

This is just a simple guard clause to make sure that we are checking
UserData is not nil before we check that the UserData.Value is not nil
…change (hashicorp#1131)

* Issue hashicorp#459 - EBS information required during cluster configuration change

AWS demands to pass information about EBS volume during cluster configuration change like e.g. number of data nodes.

Issue: hashicorp#459
Debug data from issue:
```
  cluster_config.0.instance_count: "2" => "1"

2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:01 [DEBUG] [aws-sdk-go] DEBUG: Request es/UpdateElasticsearchDomainConfig Details:
2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: ---[ REQUEST POST-SIGN ]-----------------------------
2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: POST /2015-01-01/es/domain/daniel-test/config HTTP/1.1
2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: Host: es.us-east-1.amazonaws.com
2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: User-Agent: aws-sdk-go/1.10.8 (go1.8.1; linux; amd64) APN/1.0 HashiCorp/1.0 Terraform/0.9.8
2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: Content-Length: 150
2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: Authorization: AWS4-HMAC-SHA256 Credential=/20170712/us-east-1/es/aws4_request, SignedHeaders=content-length;host;x-amz-date, Signature=
2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: X-Amz-Date: 20170712T114701Z
2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: Accept-Encoding: gzip
2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws:
2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: {"ElasticsearchClusterConfig":{"DedicatedMasterEnabled":false,"InstanceCount":2,"InstanceType":"t2.small.elasticsearch","ZoneAwarenessEnabled":false}}
2017/07/12 12:47:01 [DEBUG] plugin: terraform-provider-aws: -----------------------------------------------------
2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:02 [DEBUG] [aws-sdk-go] DEBUG: Response es/UpdateElasticsearchDomainConfig Details:
2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: ---[ RESPONSE ]--------------------------------------
2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: HTTP/1.1 400 Bad Request
2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: Content-Length: 188
2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: Content-Type: application/json
2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: Date: Wed, 12 Jul 2017 11:47:01 GMT
2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: X-Amzn-Errortype: ValidationException
2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: X-Amzn-Requestid:
2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws:
2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws:
2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: -----------------------------------------------------
2017/07/12 12:47:02 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 12:47:02 [DEBUG] [aws-sdk-go] {"message":"New cluster configuration has insufficient storage capacity for your current data. The new configuration only supports up to 0.0GB. Your current Elasticsearch usage is 0.00GB"}
```

Debug data after fix:
```
cluster_config.0.instance_count: "2" => "1"
2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:35 [DEBUG] [aws-sdk-go] DEBUG: Request es/UpdateElasticsearchDomainConfig Details:
2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: ---[ REQUEST POST-SIGN ]-----------------------------
2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: POST /2015-01-01/es/domain/daniel-test/config HTTP/1.1
2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: Host: es.us-east-1.amazonaws.com
2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: User-Agent: aws-sdk-go/1.10.8 (go1.8.1; linux; amd64) APN/1.0 HashiCorp/1.0 Terraform/0.9.8
2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: Content-Length: 218
2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: Authorization: AWS4-HMAC-SHA256 Credential=/20170712/us-east-1/es/aws4_request, SignedHeaders=content-length;host;x-amz-date, Signature=
2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: X-Amz-Date: 20170712T124235Z
2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: Accept-Encoding: gzip
2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws:
2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: {"EBSOptions":{"EBSEnabled":true,"VolumeSize":10,"VolumeType":"gp2"},"ElasticsearchClusterConfig":{"DedicatedMasterEnabled":false,"InstanceCount":1,"InstanceType":"t2.small.elasticsearch","ZoneAwarenessEnabled":false}}
2017/07/12 13:42:35 [DEBUG] plugin: terraform-provider-aws: -----------------------------------------------------
2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:36 [DEBUG] [aws-sdk-go] DEBUG: Response es/UpdateElasticsearchDomainConfig Details:
2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: ---[ RESPONSE ]--------------------------------------
2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: HTTP/1.1 200 OK
2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: Content-Length: 1355
2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: Content-Type: application/json
2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: Date: Wed, 12 Jul 2017 12:42:36 GMT
2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: X-Amzn-Requestid:
2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws:
2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws:
2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: -----------------------------------------------------
2017/07/12 13:42:36 [DEBUG] plugin: terraform-provider-aws: 2017/07/12 13:42:36 [DEBUG] [aws-sdk-go] {"DomainConfig":{"AccessPo...<CUT>
```

Please verify.
Thanks

* fix formatting

* add test for hashicorp#459

Simple test, just to check if number of instances is correct.

* fix and adjust testing method

* fix function name for as an entrypoint for Config

* test: Randomize name of the cluster

* test: Use even number of data nodes

* test: Specify valid snapshot hours

* test: Fix test functions

* Fix tests
hakamadare and others added 17 commits August 28, 2017 14:06
`alb_arn` variable is interpolated twice; this seems inadvertent
Provided examples on how `run_command_targets` should be defined
Fixes hashicorp/terraform#15706. Fixes hashicorp#1314. A recent pull request hashicorp#899) accessed `lifecycleRule.Filter` without first ensuring that it is not a `nil` pointer.
[AWS docs](http://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_ACLs.html) state:

> We recommend that you start by creating rules with rule numbers that are multiples of 100, so that you can insert new rules where you need to later on.
In TF 0.10.0, the AWS provider fails validation from the typo in the
`resolution` attribute. As: `"resolution:"`, with Terraform v0.10.0 vendored, yields:

```
1 error(s) occurred:

* provider.aws: Internal validation of the provider failed! This is always a bug
with the provider itself, and not a user issue. Please report
this bug:

1 error(s) occurred:

* resource aws_elastictranscoder_preset: resolution:: Field name may only contain lowercase alphanumeric characters & underscores.
```

This is required to bump the vendored version of Terraform.
This reverts commit 4219326.
…new-ssm-parameters-features"

This reverts commit d2a7937, reversing
changes made to 9eeab57.
@jocgir
Copy link
Contributor Author

jocgir commented Aug 28, 2017

Sorry, I was on vacation. I mixup internal changes to make our fork buildable on the submitted branche and I am not able to cleanly reset the unwanted commit. I will close the branch and resubmit a new clean one. Sorry for the inconvenience.

@jocgir jocgir closed this Aug 28, 2017
@jocgir jocgir deleted the new-ssm-parameters-features branch August 28, 2017 18:11
@ghost
Copy link

ghost commented Apr 11, 2020

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!

@ghost ghost locked and limited conversation to collaborators Apr 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants