terraform icon indicating copy to clipboard operation
terraform copied to clipboard

Add more context to Resource type mismatch error

Open makarov-roman opened this issue 2 years ago • 9 comments

Terraform Version

Terraform v1.4.6
on linux_amd64

Use Cases

I'm trying to move s3 definition to the module which utilize that. And I'm getting the following error:

╷
│ Error: Resource type mismatch
│ 
│ This statement declares a move from aws_s3_bucket.logs to module.logging.s3.bucket, which is a resource of a different type.

Proposal

If we provide resource types for both modules it would be much easier to understand what's actually wrong. Here is the example error message:

╷
│ Error: Resource type mismatch
│ 
│This statement declares a transition from aws_s3_bucket.logs (Type X) to module.logging.s3.bucket (Type Y), which represent two different types of resources.

References

No response

makarov-roman avatar Jun 16 '23 09:06 makarov-roman

In my case one of modules had incorrect reference.

makarov-roman avatar Jun 16 '23 09:06 makarov-roman

Hi @makarov-roman,

A resource type should be included because it is part of the resource's address, i.e. the type of an aws_s3_bucket is aws_s3_bucket. The destination resource type in the error though is only s3, which doesn't look correct. Can you show an example of the move block and configuration used?

Thanks!

jbardin avatar Jun 16 '23 13:06 jbardin

Hi @jbardin thanks for the quick response. the correct code in my case is

moved {
  from = aws_s3_bucket.logs
  to     = module.logging.module.logs-storage-s3[0].aws_s3_bucket.logs
}

This feature request is not about how to correctly write moved instruction, but I'm saying that if we had better Error message it would be clear from the first glance what's not correct. E.g. better error message in my case could be

╷
│ Error: Resource type mismatch
│ 
│This statement declares a transition from aws_s3_bucket.logs (aws_s3_bucket type) to module.logging.s3.bucket (null type), which represent two different types of resources.

makarov-roman avatar Jun 16 '23 15:06 makarov-roman

Hi @makarov-roman,

The resource type of module.logging.s3.bucket would be "s3", just as the resource type of aws_s3_bucket.logs is "aws_s3_bucket". There isn't any situation where the resource type isn't already included as part of the resource address, because the resource type is an inherent part of the address of the module.

Is the feedback you're intending here that it isn't clear that the resource type is part of the address? I can see that this error message does rely on some prior knowledge about Terraform -- which part of the address is the resource type -- and if that is your concern then I think I would propose to keep the message concise by removing the full resource address altogether, since that's clearly available in the configuration already anyway.

Error: Resource type mismatch

This statement declares a transition from a resource of type "aws_s3_bucket" to a
resource of type "s3". The source and destination resource type must match.

This formulation omits the irrelevant information (the module path, resource name, and instance key) and focused only on the information that is relevant to the problem being described.

Would this updated error message meet your needs better? Thanks!

apparentlymart avatar Jun 16 '23 16:06 apparentlymart

Hi @apparentlymart , I would say - yes, partially at least. Does it cover scenario if address is incorrect? My initial idea was that I can use modules outputs definition to access the resource, that's why it looks like that in the first snippet. e.g. logging/outputs.tf

output "s3" {
  value       = module.logs-storage-s3
}

logging/modules/logs-storage-s3/outputs.tf

output "bucket" {
  value = aws_s3_bucket.logs
}

That was what I expected: module.logging.s3(resolves to module.logs-storage-s3).bucket(resolves to aws_s3_bucket.logs) That's not a big deal that it is not working that way, but it's confusing that it compares valid address with invalid address and tells you that it's resource mismatch. Maybe we should check if the address match any definition? it's like valid_object.property === null.property will throw an exception in most languages because null doesn't have a property in the first place.

makarov-roman avatar Jun 16 '23 17:06 makarov-roman

Thanks for that extra context!

Unfortunately module.logging.s3.bucket is a valid address, but it has a different meaning than you expected: it means a resource "s3" "bucket" block inside module.logging, rather than an output value called s3 of that module. moved is exclusively for rebinding resources and resource instances, so it has no syntax for interacting with output values.

For misunderstandings like this we need to be careful in our error messages because we cannot be certain that a particular user had a particular misunderstanding. For situations like that, if they come up sufficiently often to justify the extra logic, we tend to add "Did you mean ...?" hints to the error message. In the case you described we could potentially notice that module.logging has an output "s3" block and that it doesn't have a resource "s3" "bucket", and if so add an extra clause that asks if the author was intending to use that output value. For example:

Error: Resource type mismatch

This statement declares a transition from a resource of type "aws_s3_bucket" to a
resource of type "s3". The source and destination resource type must match.

Did you intend to refer to the output "s3" block in module.logging? Moving is
only for resources, so there is no syntax for moving an output value.

Ideally I'd like to suggest an alternative as part of the second paragraph, but it's not clear to me what we'd suggest in this case because there's no way for us to understand what the author might have meant by declaring that a resource as moved to an output value. So I wrote it to just state that it's not possible, which is a lower-quality error message than I'd typically prefer but seems like the best we could do in this case, to avoid making too many assumptions that could cause confusion if they are incorrect assumptions.

apparentlymart avatar Jun 16 '23 17:06 apparentlymart

Agree. This heuristic isn't worth it. Is there a way to check if there is any resource defined under the address? Something similar to that, but for to. https://github.com/hashicorp/terraform/blob/66e3c20b181407fd93b787a359f97a2ae6aa66ac/internal/refactoring/move_validate.go#L99

makarov-roman avatar Jun 16 '23 18:06 makarov-roman

I received this error message today when I made a typo in the from, so I was saying aws_route_table_assocation.private_subnet instead of aws_route_table_association.private_subnet. Since there wasn't actually any such resource that existed, could the error message reflect that? In other wards, rather than saying it's a type mismatch error, if the from doesn't exist in the state file, say so? Perhaps I'm saying the same thing as @makarov-roman here.

ncabatoff avatar May 01 '24 17:05 ncabatoff

Hi @ncabatoff! Thanks for that feedback.

I think a tension here is that it's expected (and in fact required) for from to refer to something that isn't declared. Terraform will return a different error if you set from to something that's also still declared elsewhere in the configuration.

The original report in this issue was about a mistake in the to argument, and I think we could generate an error message for that in principle, but moved blocks have some extra constraints compared to other constructs in the Terraform language: a moved block must become inert once it's been acted on (so that you don't need to immediately remove the moved block after you've applied it) and also a moved block in one module mustn't block the evolution of another module.

Today Terraform allows to to refer to something that doesn't exist because if the destination is in a different module then a future version of that module might not declare that target resource anymore, with the intention that it would be deleted. At that point, the correct behavior is for Terraform to notice if there's an object bound to the original address, move it to the new address, and then plan to destroy it because it's no longer present in the configuration.

I think there's a plausible compromise though: in any situation where Terraform would produce the error about the resource types not matching, it could first check whether the to argument refers to something that exists. If not, then it could check to see if there is something that matches when replacing the to resource type with the from resource type, and in that case say "Did you mean to = ...?" where ... is the resource that matched after adjusting the resource type.

(Current versions of Terraform do allow moving between resources of different types as long as the destination provider has a migration rule for translating from the original object type, so if we make a change here we must make sure not to break the case where an automatic migration is available.)

apparentlymart avatar May 01 '24 18:05 apparentlymart