dynatrace-oss / terraform-provider-dynatrace

Apache License 2.0
70 stars 33 forks source link

Impossible to migration built-in aws_credentials services to modern integration #541

Closed mattBaumBeneva closed 1 week ago

mattBaumBeneva commented 1 week ago

Describe the bug When applying the dynatrace_aws_service "lambda" to an existing dynatrace_aws_credentials, the following error is returned: Error: Invalid services configuration: you can't have lambda_builtin and lambda services turned on simultaneously

Setting https://registry.terraform.io/providers/dynatrace-oss/dynatrace/latest/docs/resources/aws_credentials#remove_defaults does not help, because this only applies at creation.

It is thus impossible to upgrade built-in services with Terraform. In our company, it would be preferable to update existing integrations rather than manually destroying them and recreating them with Terraform.

To Reproduce Steps to reproduce the behavior:

  1. Configure an aws_credentials with remove_defaults = false
  2. Modify the TF config to add aws_service "lambda" (optionally setting remove_defaults = true)
  3. See error

Expected behavior The provider should either handle replacing built-in services internally, or applying remove_defaults to existing integrations should remove thos services.

Reinhard-Pilz-Dynatrace commented 1 week ago

Hello @mattBaumBeneva,

Your ticket doesn't come entirely as a surprise. We've been aware of the fact, that the inability to modify remove_defaults might create an inconvenience sooner or later. It is indeed just getting taken into consideration when the resource is getting created. Any changes afterwards will be ignored.

We've decided against flagging the attribute as forceNew (i.e. ensure that Terraform destroys and recreates the resource in case the attribute gets changed) on purpose. That attribute isn't managed by the REST API. It is something the Terraform Provider needs to maintain solely within the state ... which comes with its own set of risks. There exists unfortunately no other way to "reinstate" the default services. That list of "default services" may change over time.

But now for the good news. What has changed since then is the fact, that the supported_services no longer need to be specified when creating or updating a resource dynatrace_aws_credentials. As a matter of fact that functionality has been deprecated. That allowed us to implement the resource dynatrace_aws_service in the first place.

With the upcoming release the resource dynatrace_aws_service will intercept error messages like the one you've encountered. If there is no room for ambiguities within that message (i.e. one of the service names mentioned ends with _builtin and the other one doesn't), the provider will reapply the resource without the builtin service.

What that solution will NOT be able to handle is reinstating that builtin service upon deletion of the resource dynatrace_aws_service that forced the provider to get rid of the builtin service.

I'm also hesitant to call that solution "bullet proof". Based on what I can see currently, it seems like every built in service indeed has a name that ends with _builtin, but that's just an observation. There is no guarantee that the REST API sticks to that naming convention forever.

Works for you?

mattBaumBeneva commented 1 week ago

Hi, @Reinhard-Pilz-Dynatrace . Yes, I think that will fix our issue. We'll test it once the release comes out. Thanks!