cockroachdb / terraform-provider-cockroach

Terraform provider for CockroachDB Cloud
Apache License 2.0
56 stars 10 forks source link

[CC-8987] Add Client CA Cert resource #89

Closed jason-crl closed 1 year ago

jason-crl commented 1 year ago

Add a new TF resource to manage custom Client CAs.

NOTE: Since the corresponding new CC/console API endpoints are currently still marked INTERNAL, I bumped these endpoints to PREVIEW on my local console, rebuilt cockroach-cloud-sdk-go pointing to my local console, and am temp using the rebuilt sdk so I can write against the new endpoints. Will only merge this after the new endpoints are actually bumped to PREVIEW.

Have a couple questions, will drop them below. I am new to TF, so please review aggressively 😆

jason-crl commented 1 year ago
jason-crl commented 1 year ago

@erademacher

one hiccup, the post-test destroy fails. appears TF is trying to delete the cluster while it's still locked from the previous cert UPDATE job. I tried adding a sleep function, but don't think that quite works. Any trick here to wait a bit before TF attempts the cluster deletion?

=== RUN   TestAccClientCACertResource
=== PAUSE TestAccClientCACertResource
=== CONT  TestAccClientCACertResource
    testing_new.go:84: Error running post-test destroy, there may be dangling resources: exit status 1

        Error: Error deleting cluster

        Could not delete cluster: internal call failed: cannot delete locked cluster
--- FAIL: TestAccClientCACertResource (1534.42s)
        ...
        Steps: []resource.TestStep{
            {
                Config: getTestClientCACertResourceConfig(clusterName, cert1),
                Check: resource.ComposeTestCheckFunc(
                    resource.TestCheckResourceAttr(clusterResourceName, "name", clusterName),
                    resource.TestCheckResourceAttr(cliCACertResourceName, "x509_pem_cert", cert1),
                    resource.TestCheckResourceAttr(cliCACertResourceName, "status", string(client.CLIENTCACERTSTATUS_IS_SET)),
                    waitSeconds(180), <<<<<<<<<
                ),
            },
            {
                Config: getTestClientCACertResourceConfig(clusterName, cert2),
                Check: resource.ComposeTestCheckFunc(
                    resource.TestCheckResourceAttr(clusterResourceName, "name", clusterName),
                    resource.TestCheckResourceAttr(cliCACertResourceName, "x509_pem_cert", cert2),
                    resource.TestCheckResourceAttr(cliCACertResourceName, "status", string(client.CLIENTCACERTSTATUS_IS_SET)),
                    waitSeconds(180), <<<<<<<<<
                ),
            },
        },
    })
}

// give the cluster some time to finish any in-flight operations
func waitSeconds(seconds int) resource.TestCheckFunc {
    return func(s *terraform.State) error {
        time.Sleep(time.Duration(seconds) * time.Second)
        return nil
    }
}
jason-crl commented 1 year ago

Reviewed 1 of 8 files at r2, 4 of 5 files at r3, all commit messages. Reviewable status: 9 of 10 files reviewed, 5 unresolved discussions (waiting on @jason-crl and @jenngeorge)

_internal/provider/client_ca_cert_resource.go line 219 at r4 (raw file):_

    }

    _, httpResp, err := r.provider.service.DeleteClientCACert(ctx, state.ID.ValueString())

If the delete operation locks the cluster, you need to poll until it's done, just like you do for create and update. That's probably why your acceptance test is failing.