Exercise connections, etc. in zone tests #45
Conversation
While there are still better ways to DRY up the repeated strings used throughout the zone tests, this is at least a step in the right direction.
mdb
changed the title
This improves the recordset resource acceptance tests to exercise more functionality.
| zConName = "vinyldns." | ||
| zConKey = "nzisn+4G2ldMn0q1CV3vsg==" | ||
| zConKeyName = "vinyldns." | ||
| zConPrimaryServer = "vinyldns-bind9" |
There was a problem hiding this comment.
If I understand the spec correctly, then I believe that a fully qualified domain name, IP address, or IPAddr:Port pair would more clearly communicate the type of data desired for primary server. While a simple unqualified host name might work, it seems like an uncommon case. https://www.vinyldns.io/api/zone-model.html#zone-conn-attr
There was a problem hiding this comment.
@rhenning Are you referring to vinyldns-bind9? If so, I agree, but this is the docker-compose service name. If it's not clear, these acceptance tests are running against a docker-compose'd VinylDNS setup so I believe it must be vinyldns-bind9, IIUC.
There was a problem hiding this comment.
@rhenning just the host name OR the IP address is the common case, as 99% of DNS servers talk on port 53. Actually, having the port specifier is the uncommon case (because you cannot bind to lower ports on most linux machines) and we need to do uncommon things for our tests
There was a problem hiding this comment.
Got it. I hadn't realized that the tests were running against a docker-compose container. Totally makes sense why it's an unqualified hostname now. Disregard last transmission!
| zConKey = "nzisn+4G2ldMn0q1CV3vsg==" | ||
| zConKeyName = "vinyldns." | ||
| zConPrimaryServer = "vinyldns-bind9" | ||
| // NOTE: If ever the test config HCL is changed, these zACLRule*Hash var |
There was a problem hiding this comment.
HCL is "Hashicorp Configuration Language" - The Terraform DSL itself.
| resource.TestCheckResourceAttr("vinyldns_zone.test_zone", "name", zName), | ||
| resource.TestCheckResourceAttr("vinyldns_zone.test_zone", "email", zEmail), | ||
| resource.TestCheckResourceAttr("vinyldns_zone.test_zone", "zone_connection.0.name", zConName), | ||
| resource.TestCheckResourceAttr("vinyldns_zone.test_zone", "zone_connection.0.key", zConKey), |
There was a problem hiding this comment.
fun facts, when hitting a "real" environment (not docker), the key is encrypted. I.e., it is impossible to verify the value of the key in a test against a real environment unless you know the secret and the encryption algorithm, so very likely not.
This works against docker because the default cryptography is NoOp
vinyldns/resource_zone_test.go
Outdated
|
|
||
| accessLev := rs.Attributes[fmt.Sprintf("acl_rule.%s.access_level", zACLRuleUpdatedHash)] | ||
| if accessLev != "Delete" { | ||
| return fmt.Errorf("expected acl_rule access_level attribute to have value; got %s from state: %s", accessLev, rs.Attributes) |
There was a problem hiding this comment.
this error message is a little off yea? "expected acl_rule access_level attribute to have value Delete; got ...."
It indicates that it doesn't have a value, but the if check is whether it is Delete
|
@pauljamescleary @rhenning Thanks for the feedback! |
This PR seeks to exercise more functionality in the zone resource tests towards the goal of more robust testing and addressing issue #21.