v0.12 testing: A mysterious extra entry

[jtopjian]

Tests for the openstack_compute_secgroup_v2 resource are failing.

You can see the test output here:

http://logs.openlabtesting.org/logs/59/559/5f55582c3e249ab0d93ef4bf37ffc85e6cfeb384/check/terraform-provider-openstack-acceptance-test/1fdae5a/

via the job-output.txt.gz file.

For example, the test TestAccComputeV2SecGroup_importBasic has the error message:

--- FAIL: TestAccComputeV2SecGroup_importBasic (4.04s)
    testing.go:568: Step 0 error: errors during apply:

        Error: Error creating openstack_compute_secgroup_v2 sg_1 rule: Missing input for argument [IPProtocol]

          on /tmp/tf-test366550116/main.tf line 23:
          (source code not available)

The error message is from the Gophercloud library, denoting that the required IPProtocol field was omitted. This threw me for a loop because all test fixtures are setting this field (not to mention it passes on v0.11).

After further debugging, I found that there is an empty entry in the rules set. Adding a debug line below this line produces:

v0.11.x:

2019/02/26 20:37:33 [DEBUG] FOO: map[string]interface {}{"from_port":1, "to_port":65535, "ip_protocol":"udp", "cidr":"0.0.0.0/0", "from_group_id":"", "self":false, "id":""}
2019/02/26 20:37:33 [DEBUG] FOO: map[string]interface {}{"cidr":"0.0.0.0/0", "from_group_id":"", "self":false, "id":"", "from_port":-1, "to_port":-1, "ip_protocol":"icmp"}
2019/02/26 20:37:33 [DEBUG] FOO: map[string]interface {}{"cidr":"0.0.0.0/0", "from_group_id":"", "self":false, "id":"", "from_port":22, "to_port":22, "ip_protocol":"tcp"}

v0.12:

2019/02/26 20:38:30 [DEBUG] FOO: map[string]interface {}{"to_port":65535, "ip_protocol":"udp", "cidr":"0.0.0.0/0", "from_group_id":"", "self":false, "id":"", "from_port":1}
2019/02/26 20:38:30 [DEBUG] FOO: map[string]interface {}{"ip_protocol":"icmp", "cidr":"0.0.0.0/0", "from_group_id":"", "self":false, "id":"", "from_port":-1, "to_port":-1}
2019/02/26 20:38:30 [DEBUG] FOO: map[string]interface {}{"id":"", "from_port":0, "to_port":0, "ip_protocol":"", "cidr":"0.0.0.0/0", "from_group_id":"", "self":false}
2019/02/26 20:38:30 [DEBUG] FOO: map[string]interface {}{"cidr":"0.0.0.0/0", "from_group_id":"", "self":false, "id":"", "from_port":22, "to_port":22, "ip_protocol":"tcp"}

This seems to be the only resource that is seeing this issue. openstack_compute_secgroup_v2 is one of the oldest and largely unused resources in this provider, so there's probably some bad practices left in over time.

Let me know if I can provide any further details on this one.

[jbardin]

Hi @jtopjian,

Thanks, I've managed to reproduce this issue. It looks like it's a helper/schema bug of some sort that 0.12 is surfacing, because all the data going through the new shims looks correct, but the extra element shows up in only when you Get it from the ResourceData.

Strangely enough, the StateFunc on cidr seems to be the trigger, even though it's not gong to change anything in the test cases.

[trentrosenbaum]

Hi @jbardin

I am currently working on upgrading the fastly terraform provider to work with Terraform 0.12 and I have noticed that terraform, (version 0.12.1) has been returning extra entries as described in this issue. When I removed the two StateFunc entries from the schema then I get the expected output, but as in this case the required functions then would not be executed to generate a checksum to be stored in the terraform.state file.

2019/06/06 14:19:50 [DEBUG] CREATING S3LOGGING - *Set(map[string]interface {}{

"1389522020":map[string]interface {}{"bucket_name":"fastlytestlogging", "domain":"s3-us-west-2.amazonaws.com", "format":"%h %l %u %t %r %>s", "format_version":1, "gzip_level":0, "message_type":"classic", "name":"somebucketlog", "path":"", "period":3600, "placement":"", "redundancy":"", "response_condition":"", "s3_access_key":"60285b36eac3782ac220ddd227fdd23af194783046494251a4c40d0d7445fa205fc6a32f072d4fc4e6603fcde4b521d8f93854d4acd2b13d9d1e8868cdd867ef", "s3_secret_key":"9f609070bc050dcd9cb245bccfc8cd5a8e2d197eb2ff35570becaad0979be6e92e0ebd85bd1c42f5842195c6fa197adc1ca6016d17e31042148bd136ef53df37", "timestamp_format":"%Y-%m-%dT%H:%M:%S.000"}, 

"180572385":map[string]interface {}{"bucket_name":"", "domain":"", "format":"", "format_version":0, "gzip_level":0, "message_type":"", "name":"", "path":"", "period":0, "placement":"", "redundancy":"", "response_condition":"", "s3_access_key":"XXXXXXXXXXXXXXXXXXXX", "s3_secret_key":"XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX", "timestamp_format":""}})

In the example above using the StateFunc enabled provider creates two entries. The first a fully populated map with the required checksum for the fields "s3_access_key" and "s3_secret_key". The second entry contains blank values for all fields except the two previously mentioned and they are stored with the original value before being processed by the functions. I would expect a single entry with the original non checksum values and the terraform.state file to hold the checksum values.

Any thoughts about how to proceed?

[jbardin]

Hi @trentrosenbaum,

I don't think I've encountered use of the StateFunc in a set before, but I'm guessing that the plan operation is returning a different value from what is being applied. I would need a more complete example to try and sort this out. I can probably replicate it without the fastly provider, but a sample of the configuration used would help.

I'm also not sure I understand the use of the StateFunc in this case, as the access keys are not particularly large, and the StateFunc is not really meant to be used as a security mechanism (nor is a simple sha512 hash). Can the Fastly API accept an update only using the hash of the access key?

[trentrosenbaum]

Hi @jbardin

Thanks for coming back to me it is really appreciated. I have a bit of a limited history with the provider and I am going to try and find out more about the use of the StateFunc in this case.

It is my understanding that the StateFunc is used to store a value in the terraform.state file while the original values are then communicated to the appropriate API. In the same provider there is the an example where a set of vcl entries use the StateFunc to hash a code snippet. I think this is the more intended use of the function? Again this is a set and fails for the same reason.

This is the example configuration I am using to test the s3logging nest block in the service.

resource "fastly_service_v1" "foo" {
  name = "test1"

  domain {
    name    = "tftestingdomain.com"
    comment = "tf-testing-domain"
  }

  backend {
    name    = "docs"
    address = "aws.amazon.com"
  }

  s3logging {
    name               = "somebucketlog"
    bucket_name        = "fastlytestlogging"
    domain             = "s3-us-west-2.amazonaws.com"
    s3_access_key      = "XXXXXXXXXXXXXXXXXXXX"
    s3_secret_key      = "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"
  }
  force_destroy = true
}

[jbardin]

Thanks for the config @trentrosenbaum.
I'm having trouble reproducing this. Are you using the SDK from master? The go.mod in the main repo is referencing a relatively old commit from the first beta.

[trentrosenbaum]

Hi @jbardin
I have using this fork of the fastly repo to investigate.
https://github.com/trentrosenbaum/terraform-provider-fastly/tree/upgrade-provider-to-0.12.0

This one is using the 0.12.0 SDK and not 0.12.1 SDK, (which I just realized) and I was using the 0.12.1 terraform cli. I did have the same issue with 0.12.0 SDK and 0.12.0 terraform cli.

[jbardin]

Got it, I was looking at the final output, but the extra elements show up in the provider itself.
I'll take a look at this, but I need to first figure out if using a StateFunc within a set makes sense at all. Set values are identified by their value, but since you can't really correlate the value stored in the state with the value in the config there may be no way to avoid perpetually seeing extra new values.

[jbardin]

@trentrosenbaum,

Unfortunately the issue was what I thought, which doesn't leave us with a good solution.
While there doesn't appear to be a good reason for the StateFunc, migrating away from that is hard to do automatically, because you can't determine the value to migrate to from the hash. There however doesn't appear to be any reason for this to be a TypeSet either, so you may be able to convert it to a TypeList without issue.

I opened #21641 to help any other providers that may have used this pattern diagnose the issue.

[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.