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

provider/aws: Support "custom JSON" for OpsWorks layers #4272

Merged
merged 1 commit into from
Apr 16, 2016
Merged

provider/aws: Support "custom JSON" for OpsWorks layers #4272

merged 1 commit into from
Apr 16, 2016

Conversation

mrjefftang
Copy link
Contributor

No description provided.

@cmcarthur
Copy link

👍

1 similar comment
@b-ryan
Copy link
Contributor

b-ryan commented Dec 16, 2015

👍

@mrjefftang
Copy link
Contributor Author

Updated this PR to complete the following:

  • Rebased to current master to remove merge conflicts
  • Acceptance test (also added the documentation for custom_json in opsworks_stack since it was implemented from the beginning but not documented`)
  • Documentation update
  • Gofmt

@radeksimko Anything else I need to do to get this merged?

@wsh
Copy link

wsh commented Mar 29, 2016

+1 this would be super helpful for us. 😄

@@ -377,6 +392,7 @@ func (lt *opsworksLayerType) Update(d *schema.ResourceData, client *opsworks.Ops
CustomInstanceProfileArn: aws.String(d.Get("custom_instance_profile_arn").(string)),
CustomRecipes: lt.CustomRecipes(d),
CustomSecurityGroupIds: expandStringSet(d.Get("custom_security_group_ids").(*schema.Set)),
CustomJson: aws.String(d.Get("custom_json").(string)),
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens here if CustomJson is not defined? I think it will have to change to being outside the struct declaration and then check if it is defined

e.g. https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/resource_aws_opsworks_stack.go#L318

Thoughts?

@stack72
Copy link
Contributor

stack72 commented Apr 5, 2016

Hi @mrjefftang

Thanks so much for the PR here. I have left a couple of comments about empty params

1 other point is that the both tests have been updated to have custom_json. This means we have no longer got a test inplace to make sure that custom_json is truly optional

Please can you modify this [test[(https://github.com/mrjefftang/terraform/blob/opsworks_custom_json/builtin/providers/aws/resource_aws_opsworks_custom_layer_test.go#L18) to remove the custom_json param and rerun your tests?

It may be used to make an additional test cover the functionality of custom_json. This will ensure that only the basic config necessary is covered in our most basic test. This is a pattern that a lot of the resources follow

Thanks

Paul

@stack72 stack72 added the waiting-response An issue/pull request is waiting for a response from the community label Apr 5, 2016
@stack72 stack72 self-assigned this Apr 5, 2016
@mrjefftang
Copy link
Contributor Author

Hi @stack72 , I've implemented your suggestions.

@stack72
Copy link
Contributor

stack72 commented Apr 5, 2016

Thanks @mrjefftang

Currently running the tests to make sure all looks ok

Paul

@stack72
Copy link
Contributor

stack72 commented Apr 5, 2016

Hi @mrjefftang

So on running the tests, I get state changes

TF_LOG=1 make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSOpsworksCustomLayer' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
/Users/pstack/code/go/bin/stringer
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSOpsworksCustomLayer -timeout 120m
=== RUN   TestAccAWSOpsworksCustomLayer
--- FAIL: TestAccAWSOpsworksCustomLayer (15.54s)
    testing.go:154: Step 0 error: After applying this step, the plan was not empty:

        DIFF:

        UPDATE: aws_opsworks_stack.tf-acc
          default_subnet_id: "subnet-c0f30fb6" => ""
          vpc_id:            "vpc-b645c2d2" => ""

        STATE:

        aws_iam_instance_profile.opsworks_instance:
          ID = tf-4920485386043202654_opsworks_instance
          arn = arn:aws:iam::881237884953:instance-profile/tf-4920485386043202654_opsworks_instance
          name = tf-4920485386043202654_opsworks_instance
          path = /
          roles.# = 1
          roles.379301246 = tf-4920485386043202654_opsworks_instance

          Dependencies:
            aws_iam_role.opsworks_instance
        aws_iam_role.opsworks_instance:
          ID = tf-4920485386043202654_opsworks_instance
          arn = arn:aws:iam::881237884953:role/tf-4920485386043202654_opsworks_instance
          assume_role_policy = {
          "Version": "2008-10-17",
          "Statement": [
            {
              "Sid": "",
              "Effect": "Allow",
              "Principal": {
                "Service": "ec2.amazonaws.com"
              },
              "Action": "sts:AssumeRole"
            }
          ]
        }

          name = tf-4920485386043202654_opsworks_instance
          path = /
          unique_id = AROAIAJ32FXPLQX5ADRXO
        aws_iam_role.opsworks_service:
          ID = tf-4920485386043202654_opsworks_service
          arn = arn:aws:iam::881237884953:role/tf-4920485386043202654_opsworks_service
          assume_role_policy = {
          "Version": "2008-10-17",
          "Statement": [
            {
              "Sid": "",
              "Effect": "Allow",
              "Principal": {
                "Service": "opsworks.amazonaws.com"
              },
              "Action": "sts:AssumeRole"
            }
          ]
        }

          name = tf-4920485386043202654_opsworks_service
          path = /
          unique_id = AROAJIPRMGENLQCJBK2X2
        aws_iam_role_policy.opsworks_service:
          ID = tf-4920485386043202654_opsworks_service:tf-4920485386043202654_opsworks_service
          name = tf-4920485386043202654_opsworks_service
          policy = {
          "Statement": [
            {
              "Action": [
                "ec2:*",
                "iam:PassRole",
                "cloudwatch:GetMetricStatistics",
                "elasticloadbalancing:*",
                "rds:*"
              ],
              "Effect": "Allow",
              "Resource": ["*"]
            }
          ]
        }

          role = tf-4920485386043202654_opsworks_service

          Dependencies:
            aws_iam_role.opsworks_service
        aws_opsworks_custom_layer.tf-acc:
          ID = 98da61c7-1292-4838-ac73-a54d5c5452d0
          auto_assign_elastic_ips = false
          auto_assign_public_ips = true
          auto_healing = true
          custom_configure_recipes.# = 0
          custom_deploy_recipes.# = 0
          custom_instance_profile_arn =
          custom_json =
          custom_security_group_ids.# = 2
          custom_security_group_ids.4172965597 = sg-f0d1f088
          custom_security_group_ids.671131585 = sg-ffd1f087
          custom_setup_recipes.# = 0
          custom_shutdown_recipes.# = 0
          custom_undeploy_recipes.# = 0
          drain_elb_on_shutdown = true
          ebs_volume.# = 1
          ebs_volume.3575749636.iops = 0
          ebs_volume.3575749636.mount_point = /home
          ebs_volume.3575749636.number_of_disks = 2
          ebs_volume.3575749636.raid_level = 0
          ebs_volume.3575749636.size = 100
          ebs_volume.3575749636.type = gp2
          elastic_load_balancer =
          install_updates_on_boot = true
          instance_shutdown_timeout = 300
          name = tf-4920485386043202654
          short_name = tf-ops-acc-custom-layer
          stack_id = 0495ac71-3e8e-400d-a552-3bb0dbd5714a
          system_packages.# = 2
          system_packages.1368285564 = git
          system_packages.2937857443 = golang
          use_ebs_optimized_instances = false

          Dependencies:
            aws_opsworks_stack.tf-acc
            aws_security_group.tf-ops-acc-layer1
            aws_security_group.tf-ops-acc-layer2
        aws_opsworks_stack.tf-acc:
          ID = 0495ac71-3e8e-400d-a552-3bb0dbd5714a
          berkshelf_version = 3.2.0
          color =
          configuration_manager_name = Chef
          configuration_manager_version = 11.10
          custom_cookbooks_source.# = 1
          custom_json = {"key": "value"}
          default_availability_zone = us-east-1c
          default_instance_profile_arn = arn:aws:iam::881237884953:instance-profile/tf-4920485386043202654_opsworks_instance
          default_os = Amazon Linux 2014.09
          default_root_device_type = ebs
          default_ssh_key_name =
          default_subnet_id = subnet-c0f30fb6
          hostname_theme = Layer_Dependent
          manage_berkshelf = false
          name = tf-4920485386043202654
          region = us-east-1
          service_role_arn = arn:aws:iam::881237884953:role/tf-4920485386043202654_opsworks_service
          use_custom_cookbooks = false
          use_opsworks_security_groups = false
          vpc_id = vpc-b645c2d2

          Dependencies:
            aws_iam_instance_profile.opsworks_instance
            aws_iam_role.opsworks_service
        aws_security_group.tf-ops-acc-layer1:
          ID = sg-f0d1f088
          description = Managed by Terraform
          egress.# = 0
          ingress.# = 1
          ingress.490428346.cidr_blocks.# = 1
          ingress.490428346.cidr_blocks.0 = 0.0.0.0/0
          ingress.490428346.from_port = 8
          ingress.490428346.protocol = icmp
          ingress.490428346.security_groups.# = 0
          ingress.490428346.self = false
          ingress.490428346.to_port = -1
          name = tf-4920485386043202654-layer1
          owner_id = 881237884953
          tags.# = 0
          vpc_id = vpc-b645c2d2
        aws_security_group.tf-ops-acc-layer2:
          ID = sg-ffd1f087
          description = Managed by Terraform
          egress.# = 0
          ingress.# = 1
          ingress.490428346.cidr_blocks.# = 1
          ingress.490428346.cidr_blocks.0 = 0.0.0.0/0
          ingress.490428346.from_port = 8
          ingress.490428346.protocol = icmp
          ingress.490428346.security_groups.# = 0
          ingress.490428346.self = false
          ingress.490428346.to_port = -1
          name = tf-4920485386043202654-layer2
          owner_id = 881237884953
          tags.# = 0
          vpc_id = vpc-b645c2d2
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    15.554s

This is saying that a plan -> apply - > plan will always show that changes are needed to be made

Paul

@mrjefftang
Copy link
Contributor Author

@stack72 I am unable to reproduce that failure.

That output indicates that it's trying to update default_subnet_id and vpc_id on the aws_opsworks_stack module which wasn't even touched with this PR.

@apparentlymart apparentlymart removed the waiting-response An issue/pull request is waiting for a response from the community label Apr 16, 2016
@apparentlymart apparentlymart merged commit 41a8220 into hashicorp:master Apr 16, 2016
@apparentlymart
Copy link
Contributor

apparentlymart commented Apr 16, 2016

@mrjefftang sorry for the really long turnaround time on this one, and thanks for your time and patience!

I've just merged your change, with the addition of a minor follow-up change to set the req.CustomJson field even if the configured custom_json is empty, since in my testing I found that previously it was impossible to "unset" custom_json for a layer once it was already set.

@apparentlymart
Copy link
Contributor

@stack72 I was also unable to reproduce that test failure you saw there, and manual testing indicated this worked, so I proceeded with the merge. Can you still repro that test failure?

I'm wondering if there's something different about your AWS account compared to mine... in earlier opsworks changes it's seemed that the Opsworks console sometimes "secretly" creates or updates objects in ways that the raw API does not, so someone who has used the UI before gets different results than someone who hasn't. If you're still seeing the error maybe we can make a new issue for that, since I agree with @mrjefftang that it doesn't seem related to this change at first glance.

@stack72
Copy link
Contributor

stack72 commented Apr 16, 2016

Hi @apparentlymart

I can reproduce this on every test run

Paul

@ghost
Copy link

ghost commented Apr 26, 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 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.

@ghost ghost locked and limited conversation to collaborators Apr 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants