Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GIN-341: add teams certificates to Gateway and Gateway account configuration #3547

Merged
merged 4 commits into from
Sep 4, 2024

Conversation

alyssamw
Copy link
Contributor

@alyssamw alyssamw commented Aug 1, 2024

Adds support for Zero Trust certificates in Gateway and Gateway account configuration. The custom /activate api path is treated as a terraform field within the Teams certificate resource.

Relies on merged cloudflare-go change cloudflare/cloudflare-go#2754

Copy link
Contributor

github-actions bot commented Aug 1, 2024

changelog detected ✅

@alyssamw alyssamw force-pushed the master branch 6 times, most recently from 9db634c to c54b045 Compare August 2, 2024 16:03
@@ -263,6 +263,7 @@ func New(version string) func() *schema.Provider {
"cloudflare_static_route": resourceCloudflareStaticRoute(),
"cloudflare_bot_management": resourceCloudflareBotManagement(),
"cloudflare_teams_account": resourceCloudflareTeamsAccount(),
"cloudflare_teams_certificate": resourceCloudflareTeamsCertificate(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

best have a chat with kenny on this one to get the naming right. we've just done a mass transition migration to get these better named and make more sense in the current landscape.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot about the rename, teams_ is replaced with zero_trust. I'll see if cloudflare_zero_trust_gateway_certificate is kosher with Kenny and Ankur

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Jacob, James and Ankur have approved in the ticket, let me know if you have any other blockers on this PR.

@alyssamw alyssamw force-pushed the master branch 2 times, most recently from 14f7dd7 to 7af5eee Compare August 9, 2024 17:37
d.Set("enabled", Certificate.Enabled)
d.Set("binding_status", Certificate.BindingStatus)
d.Set("qs_pack_id", Certificate.QsPackId)
d.Set("type", Certificate.Type)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is erroring as the field doesn't exist; should it be in the schema? i'm not seeing anything related to it.

=== RUN   TestAccCloudflareTeamsCertificate_Basic
panic: Invalid address to set: []string{"type"}

goroutine 182 [running]:
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*ResourceData).Set(0x14000d8a280, {0x10310a57d, 0x4}, {0x1034eaf80, 0x104362d00})
	/Users/jacob/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/schema/resource_data.go:233 +0x24c
github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider.resourceCloudflareTeamsCertificateRead({0x103776ab0, 0x14000bdf490}, 0x14000d8a280, {0x103756380?, 0x14000451080})
	/Users/jacob/src/github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider/resource_cloudflare_teams_certificates.go:108 +0x51c
github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider.resourceCloudflareTeamsCertificateCreate({0x103776ab0, 0x14000bdf490}, 0x14000d8a280, {0x103756380, 0x14000451080})
	/Users/jacob/src/github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider/resource_cloudflare_teams_certificates.go:62 +0x500
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).create(0x14000866ee0, {0x103776a08, 0x14000b6fbf0}, 0x14000d8a280, {0x103756380, 0x14000451080})
	/Users/jacob/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/schema/resource.go:806 +0xe4
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).Apply(0x14000866ee0, {0x103776a08, 0x14000b6fbf0}, 0x14000b56680, 0x14000d8a100, {0x103756380, 0x14000451080})
	/Users/jacob/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/schema/resource.go:937 +0x884
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ApplyResourceChange(0x14000704210, {0x103776a08?, 0x14000b6fb60?}, 0x14000bfc000)
	/Users/jacob/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/schema/grpc_provider.go:1153 +0xaa4
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ApplyResourceChange(0x14000dae500, {0x103776a08?, 0x14000b6f170?}, 0x14000bdee70)
	/Users/jacob/go/pkg/mod/github.com/hashicorp/terraform-plugin-go@v0.23.0/tfprotov5/tf5server/server.go:865 +0x2b4
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ApplyResourceChange_Handler({0x10371aca0, 0x14000dae500}, {0x103776a08, 0x14000b6f170}, 0x14000bc1780, 0x0)
	/Users/jacob/go/pkg/mod/github.com/hashicorp/terraform-plugin-go@v0.23.0/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:518 +0x1c0
google.golang.org/grpc.(*Server).processUnaryRPC(0x14000435600, {0x103776a08, 0x14000b6f0e0}, {0x10377c940, 0x14000e0fb00}, 0x14000b50900, 0x140008084b0, 0x1042e35f8, 0x0)
	/Users/jacob/go/pkg/mod/google.golang.org/grpc@v1.63.2/server.go:1369 +0xb58
google.golang.org/grpc.(*Server).handleStream(0x14000435600, {0x10377c940, 0x14000e0fb00}, 0x14000b50900)
	/Users/jacob/go/pkg/mod/google.golang.org/grpc@v1.63.2/server.go:1780 +0xb20
google.golang.org/grpc.(*Server).serveStreams.func2.1()
	/Users/jacob/go/pkg/mod/google.golang.org/grpc@v1.63.2/server.go:1019 +0x8c
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 181
	/Users/jacob/go/pkg/mod/google.golang.org/grpc@v1.63.2/server.go:1030 +0x13c
FAIL	github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider	2.298s
FAIL
make: *** [testacc] Error 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching, changed to assign to the gateway_managed / custom fields accordingly

d.Set("binding_status", Certificate.BindingStatus)
d.Set("qs_pack_id", Certificate.QsPackId)
d.Set("type", Certificate.Type)
d.Set("uploaded_on", Certificate.UploadedOn.Format(time.RFC3339Nano))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar issue with these timestamps; they are all throwing nil pointer dereferences.

TF_ACC=1 go test ./internal/sdkv2provider -v -run "^TestAccCloudflareTeamsCertificate_" -count 1 -timeout 120m -parallel 1
=== RUN   TestAccCloudflareTeamsCertificate_Basic
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x101757524]

goroutine 152 [running]:
github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider.resourceCloudflareTeamsCertificateRead({0x1020d2ab0, 0x140007d72d0}, 0x140007dce00, {0x1020b2380?, 0x14000b1e000})
	/Users/jacob/src/github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider/resource_cloudflare_teams_certificates.go:110 +0x4f4
github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider.resourceCloudflareTeamsCertificateCreate({0x1020d2ab0, 0x140007d72d0}, 0x140007dce00, {0x1020b2380, 0x14000b1e000})
	/Users/jacob/src/github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider/resource_cloudflare_teams_certificates.go:62 +0x500
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).create(0x140009d87e0, {0x1020d2a08, 0x140006d1050}, 0x140007dce00, {0x1020b2380, 0x14000b1e000})
	/Users/jacob/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/schema/resource.go:806 +0xe4
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).Apply(0x140009d87e0, {0x1020d2a08, 0x140006d1050}, 0x14000fd6410, 0x140007dcc80, {0x1020b2380, 0x14000b1e000})
	/Users/jacob/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/schema/resource.go:937 +0x884
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ApplyResourceChange(0x14000990138, {0x1020d2a08?, 0x140006d0fc0?}, 0x14000a809b0)
	/Users/jacob/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/schema/grpc_provider.go:1153 +0xaa4
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ApplyResourceChange(0x14000f9a8c0, {0x1020d2a08?, 0x140006d0450?}, 0x140007d6000)
	/Users/jacob/go/pkg/mod/github.com/hashicorp/terraform-plugin-go@v0.23.0/tfprotov5/tf5server/server.go:865 +0x2b4
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ApplyResourceChange_Handler({0x102076ca0, 0x14000f9a8c0}, {0x1020d2a08, 0x140006d0450}, 0x140007dc000, 0x0)
	/Users/jacob/go/pkg/mod/github.com/hashicorp/terraform-plugin-go@v0.23.0/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:518 +0x1c0
google.golang.org/grpc.(*Server).processUnaryRPC(0x14000acf600, {0x1020d2a08, 0x140006d03c0}, {0x1020d8940, 0x14000ae5c80}, 0x14000b42480, 0x1400090d620, 0x102c3f5f8, 0x0)
	/Users/jacob/go/pkg/mod/google.golang.org/grpc@v1.63.2/server.go:1369 +0xb58
google.golang.org/grpc.(*Server).handleStream(0x14000acf600, {0x1020d8940, 0x14000ae5c80}, 0x14000b42480)
	/Users/jacob/go/pkg/mod/google.golang.org/grpc@v1.63.2/server.go:1780 +0xb20
google.golang.org/grpc.(*Server).serveStreams.func2.1()
	/Users/jacob/go/pkg/mod/google.golang.org/grpc@v1.63.2/server.go:1019 +0x8c
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 162
	/Users/jacob/go/pkg/mod/google.golang.org/grpc@v1.63.2/server.go:1030 +0x13c
FAIL	github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider	1.721s
FAIL
make: *** [testacc] Error 1

are you able to run the acceptance tests locally and get these passing?

Copy link
Contributor Author

@alyssamw alyssamw Aug 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am now getting

    provider_test.go:82: [{0 You've attempted to add a new resource to the `terraform-plugin-sdkv2` 
which is no longer considered suitable for use. Due the number of known internal issues with 
`terraform-plugin-sdkv2` (most notably handling of zero values), we are no longer recommending using it 
and instead, advise using `terraform-plugin-framework` exclusively. If you must use terraform-plugin-sdkv2
 for this new resource you should first discuss it with a maintainer to fully understand the impact and 
potential ramifications. Only then should you bump MAXIMUM_ALLOWED_SDKV2_RESOURCES to include 
your resource. []}]

Does this mean I should add the resource to /internal/framework/service, and not allow users of older TF versions to interact with certificates?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initially, this should have been added as a framework based resource however, let's leave this one as is seeing how it's already got to this point. it doesn't restrict end users on what resources they can use; only which internals it relies on.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even after bumping the number (which you can do here too), there is still an acceptance test failure.

TF_ACC=1 go test ./internal/sdkv2provider -v -run "^TestAccCloudflareTeamsCertificate_" -count 1 -timeout 120m -parallel 1
=== RUN   TestAccCloudflareTeamsCertificate_Basic
    resource_cloudflare_teams_certificate_test.go:26: Step 1/2 error: Error running apply: exit status 1
        
        Error: error reading Teams Certificate type "": %!w(<nil>)
        
          with cloudflare_zero_trust_gateway_certificate.xzcchpxrwj,
          on terraform_plugin_test.tf line 12, in resource "cloudflare_zero_trust_gateway_certificate" "xzcchpxrwj":
          12: resource "cloudflare_zero_trust_gateway_certificate" "xzcchpxrwj" {
        
--- FAIL: TestAccCloudflareTeamsCertificate_Basic (1.11s)
FAIL
FAIL	github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider	2.069s
FAIL
make: *** [testacc] Error 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, there's a bug on my api. Will reopen this PR when ready

.changelog/3547.txt Outdated Show resolved Hide resolved
@jacobbednarz
Copy link
Member

acceptance tests all passing

TF_ACC=1 go test ./internal/sdkv2provider -v -run "^(TestAccCloudflareTeamsAccounts|TestAccCloudflareTeamsCertificate)_" -count 1 -timeout 120m -parallel 1
=== RUN   TestAccCloudflareTeamsAccounts_ConfigurationBasic
--- PASS: TestAccCloudflareTeamsAccounts_ConfigurationBasic (6.31s)
=== RUN   TestAccCloudflareTeamsCertificate_Basic
--- PASS: TestAccCloudflareTeamsCertificate_Basic (2.67s)
PASS
ok  	github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider	10.032s 

@jacobbednarz jacobbednarz merged commit 46a1935 into cloudflare:master Sep 4, 2024
8 checks passed
@github-actions github-actions bot added this to the v4.42.0 milestone Sep 4, 2024
Copy link
Contributor

This functionality has been released in v4.42.0 of the Terraform Cloudflare Provider.

Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants