access: add support for allowed_idps #734

xens · 2020-07-14T08:17:55Z

cloudflare/resource_cloudflare_access_application_test.go

website/docs/r/access_application.html.markdown

Goal of this commit is to add support for multiple IdPs selection inside Cloudflare Access.

jacobbednarz · 2020-07-14T20:41:32Z

@xens as a bit side note, continually rebasing after you've opened a PR and got review is somewhat frowned upon as it makes comparing commits and previous changes impossible. I'd recommend that you keep rebasing and rewriting history only local to your machine but after you've pushed it, add commits for changes/merges. Commits in Git are lightweight and don't need to be compacted into another in order to save space or similar arguments which you can make in other VCS. There is also nothing wrong with multiple commits in a PR or showing your journey to getting it merged 😃

If the maintainers would like a single, squashed PR before merge they will generally ask you or do it themselves on merge saving you having to guess. Thanks!

jacobbednarz · 2020-07-14T20:56:58Z

Running this branch locally with these new changes are causing panics.

Test ended in panic.
------- Stdout: -------
=== RUN   TestAccCloudflareAccessApplicationBasic
------- Stderr: -------
panic: interface conversion: interface {} is []interface {}, not []string
goroutine 127 [running]:
github.com/terraform-providers/terraform-provider-cloudflare/cloudflare.resourceCloudflareAccessApplicationCreate(0xc0003e7340, 0x1426bc0, 0xc000116900, 0x2, 0x21e1380)
  /opt/teamcity-agent/work/962a4ee8807683e3/src/github.com/terraform-providers/terraform-provider-cloudflare/cloudflare/resource_cloudflare_access_application.go:121 +0x6da
github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*Resource).Apply(0xc00046f170, 0xc0001d78b0, 0xc00029fea0, 0x1426bc0, 0xc000116900, 0xc000296c01, 0xc00022f758, 0x1263a60)
  /opt/teamcity-agent/work/962a4ee8807683e3/src/github.com/terraform-providers/terraform-provider-cloudflare/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/schema/resource.go:310 +0x3b4
github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*Provider).Apply(0xc0004c0100, 0xc0002b7a60, 0xc0001d78b0, 0xc00029fea0, 0xc0001a6aa8, 0xc0004403e0, 0x12664c0)
  /opt/teamcity-agent/work/962a4ee8807683e3/src/github.com/terraform-providers/terraform-provider-cloudflare/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/schema/provider.go:294 +0x18f
github.com/hashicorp/terraform-plugin-sdk/internal/helper/plugin.(*GRPCProviderServer).ApplyResourceChange(0xc00044c018, 0x17a74c0, 0xc00028f500, 0xc0003e69a0, 0xc00044c018, 0xc00028f500, 0xc000271bc8)
  /opt/teamcity-agent/work/962a4ee8807683e3/src/github.com/terraform-providers/terraform-provider-cloudflare/vendor/github.com/hashicorp/terraform-plugin-sdk/internal/helper/plugin/grpc_provider.go:885 +0x8b5
github.com/hashicorp/terraform-plugin-sdk/internal/tfplugin5._Provider_ApplyResourceChange_Handler(0x13dc3e0, 0xc00044c018, 0x17a74c0, 0xc00028f500, 0xc0004866c0, 0x0, 0x17a74c0, 0xc00028f500, 0xc00014a540, 0x1b8)
  /opt/teamcity-agent/work/962a4ee8807683e3/src/github.com/terraform-providers/terraform-provider-cloudflare/vendor/github.com/hashicorp/terraform-plugin-sdk/internal/tfplugin5/tfplugin5.pb.go:3305 +0x23e
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0003e4000, 0x17b8140, 0xc0003e4300, 0xc0004c2900, 0xc0003dc300, 0x21b3d20, 0x0, 0x0, 0x0)
  /opt/teamcity-agent/work/962a4ee8807683e3/src/github.com/terraform-providers/terraform-provider-cloudflare/vendor/google.golang.org/grpc/server.go:1024 +0x4d2
google.golang.org/grpc.(*Server).handleStream(0xc0003e4000, 0x17b8140, 0xc0003e4300, 0xc0004c2900, 0x0)
  /opt/teamcity-agent/work/962a4ee8807683e3/src/github.com/terraform-providers/terraform-provider-cloudflare/vendor/google.golang.org/grpc/server.go:1313 +0xda6

Pointing to https://github.com/terraform-providers/terraform-provider-cloudflare/pull/734/files#diff-bd4433060b42fc5a3d9c565f8d75adedR121

xens · 2020-07-14T21:11:03Z

Running this branch locally with these new changes are causing panics.

Test ended in panic.
------- Stdout: -------
=== RUN   TestAccCloudflareAccessApplicationBasic
------- Stderr: -------
panic: interface conversion: interface {} is []interface {}, not []string
goroutine 127 [running]:
github.com/terraform-providers/terraform-provider-cloudflare/cloudflare.resourceCloudflareAccessApplicationCreate(0xc0003e7340, 0x1426bc0, 0xc000116900, 0x2, 0x21e1380)
  /opt/teamcity-agent/work/962a4ee8807683e3/src/github.com/terraform-providers/terraform-provider-cloudflare/cloudflare/resource_cloudflare_access_application.go:121 +0x6da
github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*Resource).Apply(0xc00046f170, 0xc0001d78b0, 0xc00029fea0, 0x1426bc0, 0xc000116900, 0xc000296c01, 0xc00022f758, 0x1263a60)
  /opt/teamcity-agent/work/962a4ee8807683e3/src/github.com/terraform-providers/terraform-provider-cloudflare/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/schema/resource.go:310 +0x3b4
github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*Provider).Apply(0xc0004c0100, 0xc0002b7a60, 0xc0001d78b0, 0xc00029fea0, 0xc0001a6aa8, 0xc0004403e0, 0x12664c0)
  /opt/teamcity-agent/work/962a4ee8807683e3/src/github.com/terraform-providers/terraform-provider-cloudflare/vendor/github.com/hashicorp/terraform-plugin-sdk/helper/schema/provider.go:294 +0x18f
github.com/hashicorp/terraform-plugin-sdk/internal/helper/plugin.(*GRPCProviderServer).ApplyResourceChange(0xc00044c018, 0x17a74c0, 0xc00028f500, 0xc0003e69a0, 0xc00044c018, 0xc00028f500, 0xc000271bc8)
  /opt/teamcity-agent/work/962a4ee8807683e3/src/github.com/terraform-providers/terraform-provider-cloudflare/vendor/github.com/hashicorp/terraform-plugin-sdk/internal/helper/plugin/grpc_provider.go:885 +0x8b5
github.com/hashicorp/terraform-plugin-sdk/internal/tfplugin5._Provider_ApplyResourceChange_Handler(0x13dc3e0, 0xc00044c018, 0x17a74c0, 0xc00028f500, 0xc0004866c0, 0x0, 0x17a74c0, 0xc00028f500, 0xc00014a540, 0x1b8)
  /opt/teamcity-agent/work/962a4ee8807683e3/src/github.com/terraform-providers/terraform-provider-cloudflare/vendor/github.com/hashicorp/terraform-plugin-sdk/internal/tfplugin5/tfplugin5.pb.go:3305 +0x23e
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0003e4000, 0x17b8140, 0xc0003e4300, 0xc0004c2900, 0xc0003dc300, 0x21b3d20, 0x0, 0x0, 0x0)
  /opt/teamcity-agent/work/962a4ee8807683e3/src/github.com/terraform-providers/terraform-provider-cloudflare/vendor/google.golang.org/grpc/server.go:1024 +0x4d2
google.golang.org/grpc.(*Server).handleStream(0xc0003e4000, 0x17b8140, 0xc0003e4300, 0xc0004c2900, 0x0)
  /opt/teamcity-agent/work/962a4ee8807683e3/src/github.com/terraform-providers/terraform-provider-cloudflare/vendor/google.golang.org/grpc/server.go:1313 +0xda6

Pointing to https://github.com/terraform-providers/terraform-provider-cloudflare/pull/734/files#diff-bd4433060b42fc5a3d9c565f8d75adedR121

Thanks I'll fix that tomorrow, I guess that I should be able to run the AcceptanceTests against a dummy Cloudflare account right ? Sorry for that I was sure that these tests were running part of the automated tests.

jacobbednarz · 2020-07-14T21:28:53Z

There are a couple of different types of tests. The ones linked to GitHub are unit tests whereas the majority of our suite is in integration tests that can't be linked here from TeamCity.

xens · 2020-07-14T22:07:00Z

Regarding the panic I'm wondering if I made a mistake in the cloudflare-go library https://github.com/cloudflare/cloudflare-go/blob/master/access_application.go#L23 where AllowedIdps should be of type []interface {} instead of []string, that's not what's in the API documentation but that what I read from the error message. Any idea ?

jacobbednarz · 2020-07-14T22:21:42Z

I think you're good; Terraform passes complex structures around as interface{}s and you need to convert them to use them. []string for the API is 👍. Let me pull this locally and I'll push up a fix to demonstrate.

jacobbednarz · 2020-07-14T22:43:37Z

I've pushed up 32bdd74 which addresses the following:

Makes []interface{} into usable []string using a helper we wrote
Only send the allowed_idps to the create/update endpoint if we have something to send
Fixes test expression (no need for ${} interpolation in 0.12)
Assert allowed_idps.# = 1 as that covers ensuring the structure is populated with the expected data at a high enough level. We could use regex matches but we get about the same value this way without the 18x slow down of parsing a UUID.

With this, I've kicked off an integration test run and we should be good to go!

xens · 2020-07-15T06:05:37Z

Awesome thanks for your extended support on that one !

…arly-hints custom_hostname: add support for early_hints

ghost added size/XS size/S and removed size/XS labels Jul 14, 2020

xens mentioned this pull request Jul 14, 2020

Add support for allowed_idps feature on Cloudflare Access #720

Closed

xens commented Jul 14, 2020

View reviewed changes

cloudflare/resource_cloudflare_access_application_test.go Outdated Show resolved Hide resolved

ghost added size/M kind/documentation Categorizes issue or PR as related to documentation. and removed size/S labels Jul 14, 2020

jacobbednarz reviewed Jul 14, 2020

View reviewed changes

website/docs/r/access_application.html.markdown Show resolved Hide resolved

Access: add support for allowed_idps

dfb0050

Goal of this commit is to add support for multiple IdPs selection inside Cloudflare Access.

Access: allowed_idps: modify AccTests

fe34527

Expand []interface{} into usable []string

32bdd74

jacobbednarz approved these changes Jul 15, 2020

View reviewed changes

jacobbednarz merged commit 127e8c6 into cloudflare:master Jul 15, 2020

boekkooi-lengoo pushed a commit to boekkooi-lengoo/terraform-provider-cloudflare that referenced this pull request Feb 28, 2022

Merge pull request cloudflare#734 from jacobbednarz/custom-hostname-e…

2cd368a

…arly-hints custom_hostname: add support for early_hints

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

access: add support for allowed_idps #734

access: add support for allowed_idps #734

xens commented Jul 14, 2020 •

edited

Loading

jacobbednarz commented Jul 14, 2020

jacobbednarz commented Jul 14, 2020

xens commented Jul 14, 2020

jacobbednarz commented Jul 14, 2020

xens commented Jul 14, 2020

jacobbednarz commented Jul 14, 2020

jacobbednarz commented Jul 14, 2020 •

edited

Loading

xens commented Jul 15, 2020

access: add support for allowed_idps #734

access: add support for allowed_idps #734

Conversation

xens commented Jul 14, 2020 • edited Loading

jacobbednarz commented Jul 14, 2020

jacobbednarz commented Jul 14, 2020

xens commented Jul 14, 2020

jacobbednarz commented Jul 14, 2020

xens commented Jul 14, 2020

jacobbednarz commented Jul 14, 2020

jacobbednarz commented Jul 14, 2020 • edited Loading

xens commented Jul 15, 2020

xens commented Jul 14, 2020 •

edited

Loading

jacobbednarz commented Jul 14, 2020 •

edited

Loading