[wip] Basic AWS OpsWorks support

[apparentlymart]

This merge request is still a work-in-progress. Pushing it up early so others can see that this is being worked on, and to gather any early feedback anyone might want to give.

• Stacks
• Layers
• Instances
• Applications
• Deployments

[nabeken]

Ah, I'm also working on implementing basic OpsWorks support and I found this PR when I supposed to open PR.

You can find my works at https://github.com/nabeken/terraform/tree/aws-opsworks

I have finished implementing 3 resources that has acc tests passing:

• aws_opsworks_custom_layer
• aws_opsworks_custom_recipes
• aws_opsworks_stack

Currently, I have no interest in implementing builtin layer resources but I think we can merge our efforts.

[apparentlymart]

@nabeken I'm sorry that we've done some duplicated work here. Hopefully we can take a bit of both of our work and merge them together.

It looks like you made custom recipes a separate resource than the layer itself, which was a different direction than I took. In my current implementation the custom recipes are just nested inside the layer, with a separate property for each type:

resource "aws_opsworks_custom_layer" "foo" {
    stack_id = "${aws_opsworks_stack.bar.id}"
    name = "foo"
    short_name = "foo"

    custom_undeploy_recipes = [
        "x", "y", "x"
    ]
    custom_configure_recipes = [
        "foo", "bar", "baz"
    ]
}

What was your reason for making the custom recipes a separate resource? It feels a little unnatural to do that since each layer can have only one set of recipes for each type, and so it wouldn't make sense to have more than one aws_opsworks_custom_recipes resource on the same layer.

I have support for all of the different layer types in my changesets, as well as the "custom" one.

Our stack designs look very similar, with only internal implementation differences between them.

I was planning to finish up my work here with the following extra parts:

• Resource type for OpsWorks apps
• Resource type for OpsWorks instances
• A special resource type for OpsWorks deployments. Deployments are not convergent in OpsWorks, so I was planning to implement this by adding an arbitrary unique id on the deployment resource, and trigger a new deployment each time that unique id changes. The idea here would be that the user can put in the unique id whatever value will change each time the application version changes, such as the package URL set on the application.
• Acceptance tests for all of it

I've not written any acceptance tests yet, since I've just been testing manually with a local configuration. Do you mind if I incorporate your acceptance tests into my patchset as a starting point? Then I will also write more acceptance tests for the other resource types mentioned above once I get to them. (I hope to be able to wrap this whole thing up later in the week.)

[nabeken]

@apparentlymart Thanks for your comment.

What was your reason for making the custom recipes a separate resource?

I tried to implement a layer resource as close as API request object so it looked like this:

resource "aws_opsworks_custom_layer" "foo" {
    stack_id = "${aws_opsworks_stack.bar.id}"
    name = "foo"
    short_name = "foo"

    custom_recipes {
        configure = [ "x", "y", "x"]
        ...
    }
}

I found schema.TypeList in schema.TypeSet just makes implementation complicated and its resource diffs also doesn't look good when recipes are changed. My solution was moving custom recipes resource to the top level resource.

You made custom recipes into flatten schema.TypeList. I think it makes more sense for me:+1:

Do you mind if I incorporate your acceptance tests into my patchset as a starting point?

No, I don't mind. Feel free to take any my changes:smile:

I'll do some manual testing with your changes if I have a chance.

I was planning to finish up my work here with the following extra parts:

It is very interesting to me but I guess we need more time to land. This PR has already over 1000 lines and it will become more larger:smile:

I think having at least two aws_opsworks_stack and aws_opsworks_foo_layer resources will help us. Other resources can be added later so we can focus on implementing basic resources as a starting point for Terraform.

Once basic OpsWorks support lands on Terraform, more user can start using OpsWorks resources and we can get more feedbacks. What do you think?

[apparentlymart]

Sure, we could try to just land the changes so far, and I'll attack the rest as a separate changeset. For the use-case I have in mind (using Terraform to deploy an app to OpsWorks) I need the rest of the functionality, but no reason we can't get this first set of resources polished and merged first.

[Fodoj]

What is already possible to do? Would love to see it merged and contribute, so I will probably use your fork for now.

[nabeken]

@apparentlymart I tested aws_opsworks_stack resources (not yet for layer resources).

Here is my thoughts.

around aws.awsResourceData

You tried to wrap *schema.ResourceData with aws.awsResourceData to reduce boilerplates.
I like the idea but unfortunately, in some case, it does not work as we expected.

awsResourceData uses GetOk(...) to decide whether to return aws-go-sdk-aware value using aws.{String,Bool,Long} or nil but
GetOk(...) will return (false, false) even though we have set a value explicitly.

For example:

resource "aws_opsworks_stack" "opsworks" {
  ...
  use_opsworks_security_groups = false
  ...
}

In this case, TF tries to set UseOpsWorksSecurityGroups to nil so it have no effect on actual resource because false is zero-value (empty value) for bool.

A solution here is just putting d.Get("use_opsworks_security_groups").(bool) (d is resource.ResourceData) because we already have default value for ResourceData.

The solution results in mixing up aws.awsResourceData and resource.ResourceData. Instead of having awsResourceData, we can have some useful utility functions like https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/structure.go. Going with utility functions make more sense to me.

If we really want to have aws.awsResourceData, I think aws.awsResourceData needs to be discussed separately from OpsWorks resources.

Region

region value can be filled automatically from AWS provider.
You can see the example at https://github.com/nabeken/terraform/blob/aws-opsworks/builtin/providers/aws/resource_aws_opsworks_stack.go#L137

Create and Update

We can reduce some duplications between Create and Update.

In Create, we can set values at minimum like this:

createOpts := &opsworks.CreateStackInput{
    DefaultInstanceProfileARN: aws.String(d.Get("default_instance_profile_arn").(string)),
    Name:           aws.String(d.Get("name").(string)),
    Region:         aws.String(stackRegion),
    ServiceRoleARN: aws.String(d.Get("service_role_arn").(string)),
}

... conn.CreateStack(createOpts)

...

Then, we can return with calling return resourceAwsOpsWorksStackUpdate(d, meta).

When we need to add a new value, we can just put a value only in Update.

That's all for now and sorry for long reply. I really want this to be landed. I'll test layer resources if I have a change. Thanks!

[apparentlymart]

Thanks for the testing and feedback, @nabeken.

I will see how the patch looks without the awsResourceData helper.

Given that OpsWorks is a "global" service but that its stack resources must in fact be attached to regions, I concluded that it was better for usability to not expose this detail and just use the provider region, so that the opsworks resources work the same way as most other AWS resources. It sounds like you disagree and would like the ability to override it per-resource. Given that it is easy to support (as you showed) I will include the ability to override it as you did.

Unfortunately some other work came up for me that prevented me from completing this last week, but I will work on this again once I am finished with my other project.

[nabeken]

@apparentlymart I tested aws_opsworks_custom_layer resource.

here is my thoughts (cont):

raid_level

I think raid_level should be optional and string. AWS's document says it's Number but it is simply wrong because we need to distinguish between 0, 1, 10 and 01 even though they don't support currently.

Another point is here: Go's zero-value for int is also 0 so we can't say whether it is default value or not. If we have raid_level as a string, we can easily find it's default when value is empty string.

ebs_volume

I guess ebs_volume support is still work in progress.
FYI, it prevents us from applying after the first time because we have only default value in state file.

custom_json

I forgot to mention in previous but I'm really missing custom_json in stack resource.

stack in VPC vs security groups and IAM

There are some wired problems in AWS's end.

For Security Groups: OpsWorks tries to create builtin security groups in VPC and it will happen in async.
We need to wait for its complete. I added some workarounds here and here.

Without this workarounds, acceptance tests in VPC will become harder.

For IAM: IAM's change seems not to propagate immediately. In some cases, especially with TF's IAM resources, we might get ValidationException error from API. I also added a workaround here.

That's all from me. Thank you for your hard work.

[apparentlymart]

@nabeken thanks again for the feedback. I will be working on this patch some more today.

I should have mentioned custom_json before... I was reluctant to implement it yet since it seems like there's a missing feature in terraform to have attributes that are just arbitrary data structures. I'd like to be able to write something like this:

resource "opsworks_stack" "foo" {
    custom_json = {
        something = {
            arbitrary = true
        }
    }
}

This seems much more natural than what I presume we'd need to do today:

resource "opsworks_stack" "foo" {
    custom_json = <<EOF
{
    "something": {
            "arbitrary": true
        }
    }
}
EOF
}

So I skipped over that attribute with the intention of discussing its implementation, but didn't then start the discussion. I'll just implement it as in the second example above (for which there is already a precedent in aws_iam_role) and then the other discussion can happen separately.

[apparentlymart]

I have rewritten the aws_opsworks_stack resource to not use aws.ResourceData. Something has come up here so I don't have time yet to rewrite the layer implementation, but I will get to that when I next have time.

[apparentlymart]

I created #2162 to submit just the stack and layer support, as suggested earlier.

I'm going to keep this PR open to represent the work to implement the remaining OpsWorks objects, since it's been linked from a few places like the AWS support checklist spreadsheet.

[apparentlymart]

Right now I don't see myself doing any further work on Opsworks so I'm closing this so I don't scare others away from working on it. #2162 represents a subset of what this PR originally set out to do, and I'm not expecting to go beyond that since it takes a long time to get these things merged and maintaining my branches against ongoing other changes to the AWS provider is becoming a timesuck.

[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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.