zitadel / terraform-provider-zitadel

Official Terraform provider for ZITADEL
https://zitadel.com
Apache License 2.0
21 stars 13 forks source link

Refer to smtp_config by ID #185

Open cimnine opened 1 week ago

cimnine commented 1 week ago

This updates the terraform provider to refer to the SMTP Configuration by their IDs. This is a breaking change! Previous SMTP configs need to be removed from the state and re-imported with their proper resource ID.

Fixes #180

Todo

Definition of Ready

cimnine commented 1 week ago

The test currently fails as follows:

=== RUN   TestAccSMTPConfig
=== PAUSE TestAccSMTPConfig
=== CONT  TestAccSMTPConfig
    lifecyletest.go:73: Step 5/7 error running import: exit status 1

        Error: Cannot import non-existent remote object

        While attempting to import an existing object to
        "zitadel_smtp_config.default", the provider detected that no object exists
        with the given id. Only pre-existing objects can be imported; check that the
        id is correct and that it is associated with the provider's configured region
        or endpoint, or use "terraform apply" to create a new remote object for this
        resource.

--- FAIL: TestAccSMTPConfig (4.62s)

I'm not sure how to proceed. There is probably no default SMTP config anymore, so to import a default SMTP config, one would have to be created beforehand. Is this the correct assumption and how to do that?

cimnine commented 6 days ago

For reference: Via Discord Migual A. C suggested that this resource should also activate the SMTP configuration:

For your code I'd add the followint to the smtpconfig/funcs.go to activate the provider after it has been created:

    _, err = client.ActivateSMTPConfig(ctx, &admin.ActivateSMTPConfigRequest{Id: resp.Id})
    if err != nil {
        return diag.Errorf("failed to activate smtp config: %v", err)
    }

However, I don't believe that this is a good idea:

I believe the activate part should be a new independent resource. Otherwise, if there are multiple parallel configurations, it will never be deterministic which configuration is currently active.

There could also be an is_active flag on the zitadel_smtp_config resource. But it would be hard to enforce that only one zitadel_smtp_config defines is_active = true:

The benefit of an independent resource compared to an is_active flag (or similar) is, that with the resource we can use the internal terraform ID to ensure that there is only one zitadel_active_smtp_config (or whatever the name shall be).