AUTH-6588 Support Access apps `destinations` field #4661

veggiedefender · 2024-11-25T18:04:19Z

Adds support for the Access app destinations field, but still keeps the deprecated self_hosted_domains field around for backwards compatibility. Since cloudflare-go now only supports the new field, everything is implemented in terms of destinations under the hood.

veggiedefender · 2024-11-25T18:07:57Z

internal/sdkv2provider/schema_cloudflare_access_application.go

+				return oldValue == newValue
+			},
+		},
+		"destinations": {


Should this be named destination (singular instead of plural) instead? It'll be used like this:

destinations { uri = "example.com" } destinations { uri = "foo.example.com" } destinations { type = "private" uri = "10.116.0.4" }

I saw the Attributes as Blocks page but it's kind of confusing to me.

destinations makes sense. in v5, this makes a lot more sense as a collection of objects - https://github.com/cloudflare/terraform-provider-cloudflare/blob/next/internal/services/zero_trust_access_application/schema.go#L225-L244

github-actions · 2024-11-25T18:17:38Z

This project handles dependency version bumps (including upstream changes from cloudflare-go) independently of the standard PR process using automation. This allows the dependency upgrades to land without causing merge conflicts in multiple branches and handled in a consistent way. The exception to this is security related dependency upgrades but they should be co-ordinated with the maintainer team privately.

Please remove the changes to the go.mod or go.sum files from this PR in order to proceed with review and merging.

github-actions · 2024-11-25T18:21:26Z

changelog detected ✅

veggiedefender · 2024-11-25T19:04:16Z

internal/sdkv2provider/schema_cloudflare_access_application.go

+		},
+		"destinations": {
+			Type:          schema.TypeList,
+			Optional:      true,


Should this be Computed as well? It's unclear what that actually does 🧐

https://developer.hashicorp.com/terraform/plugin/sdkv2/schemas/schema-behaviors#computed

Looks like it probably shouldn't be computed then, thanks!

jacobbednarz · 2024-11-25T23:54:27Z

go.mod

@@ -4,7 +4,7 @@ go 1.23.3

 require (
 	github.com/agext/levenshtein v1.2.3 // indirect
-	github.com/cloudflare/cloudflare-go v0.110.0
+	github.com/cloudflare/cloudflare-go v0.110.1-0.20241125012349-7c091bc8c0dd


we should remove this and just shim it in locally.

jacobbednarz · 2024-11-25T23:54:51Z

internal/consts/provider.go

@@ -110,4 +110,10 @@ const (

 	// Schema description for all ID fields.
 	IDSchemaDescription = "The identifier of this resource."
+
+	// Schema key for access app destination.
+	DestinationsSchemaKey = "destinations"


we only use this in one place -- perhaps it's fine just as a string?

jacobbednarz · 2024-11-26T00:54:33Z

we've got some failing tests but they are also happening on master too so i suspect something else is off there.

TF_ACC=1 go test ./internal/sdkv2provider -v -run "^TestAccCloudflareAccessApplication_WithSCIMConfigOAuthBearerToken" -count 1 -timeout 120m -parallel 1
=== RUN   TestAccCloudflareAccessApplication_WithSCIMConfigOAuthBearerToken
    resource_cloudflare_access_application_test.go:281: Step 1/1 error: After applying this test step, the non-refresh plan was not empty.
        stdout:
        
        
        Terraform used the selected providers to generate the following execution
        plan. Resource actions are indicated with the following symbols:
          ~ update in-place
        
        Terraform will perform the following actions:
        
          # cloudflare_zero_trust_access_identity_provider.ixngfmfbhl will be updated in-place
          ~ resource "cloudflare_zero_trust_access_identity_provider" "ixngfmfbhl" {
                id         = "9acedaaf-2ccf-4188-8ddd-a6e95eb97327"
                name       = "ixngfmfbhl"
                # (2 unchanged attributes hidden)
        
              ~ scim_config {
                  - identity_update_behavior = "reauth" -> null
                    # (5 unchanged attributes hidden)
                }
        
                # (1 unchanged block hidden)
            }
        
        Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccCloudflareAccessApplication_WithSCIMConfigOAuthBearerToken (3.66s)
FAIL
FAIL	github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider	4.850s
FAIL
make: *** [testacc] Error 1

…or nil) in order for the association to be destroyed. Setting this to false keeps the association in a disabled state instead of destroying the resource. Documentation on this API endpoint can be found here: https://developers.cloudflare.com/api/operations/per-hostname-authenticated-origin-pull-enable-or-disable-a-hostname-for-client-authentication Referencing issue raised on provider: cloudflare#4648

veggiedefender · 2024-12-04T14:52:24Z

cherry-picked the commit from #4649 so that this compiles

jacobbednarz · 2024-12-04T21:44:34Z

acceptance tests all passing

TF_ACC=1 go test ./internal/sdkv2provider -v -run "^TestAccCloudflareAccessApplication_" -count 1 -timeout 120m -parallel 1
=== RUN   TestAccCloudflareAccessApplication_BasicZone
--- PASS: TestAccCloudflareAccessApplication_BasicZone (7.86s)
=== RUN   TestAccCloudflareAccessApplication_BasicAccount
--- PASS: TestAccCloudflareAccessApplication_BasicAccount (2.73s)
=== RUN   TestAccCloudflareAccessApplication_WithSCIMConfigHttpBasic
--- PASS: TestAccCloudflareAccessApplication_WithSCIMConfigHttpBasic (4.42s)
=== RUN   TestAccCloudflareAccessApplication_UpdateSCIMConfig
--- PASS: TestAccCloudflareAccessApplication_UpdateSCIMConfig (9.59s)
=== RUN   TestAccCloudflareAccessApplication_WithSCIMConfigInvalidMappingSchema
--- PASS: TestAccCloudflareAccessApplication_WithSCIMConfigInvalidMappingSchema (1.36s)
=== RUN   TestAccCloudflareAccessApplication_WithSCIMConfigHttpBasicMissingRequired
--- PASS: TestAccCloudflareAccessApplication_WithSCIMConfigHttpBasicMissingRequired (1.39s)
=== RUN   TestAccCloudflareAccessApplication_WithSCIMConfigOAuthBearerToken
--- PASS: TestAccCloudflareAccessApplication_WithSCIMConfigOAuthBearerToken (6.38s)
=== RUN   TestAccCloudflareAccessApplication_WithSCIMConfigOAuth2
--- PASS: TestAccCloudflareAccessApplication_WithSCIMConfigOAuth2 (4.28s)
=== RUN   TestAccCloudflareAccessApplication_WithSCIMConfigOAuth2MissingRequired
--- PASS: TestAccCloudflareAccessApplication_WithSCIMConfigOAuth2MissingRequired (1.42s)
=== RUN   TestAccCloudflareAccessApplication_WithSCIMConfigAuthenticationInvalid
--- PASS: TestAccCloudflareAccessApplication_WithSCIMConfigAuthenticationInvalid (1.35s)
=== RUN   TestAccCloudflareAccessApplication_WithCORS
--- PASS: TestAccCloudflareAccessApplication_WithCORS (2.82s)
=== RUN   TestAccCloudflareAccessApplication_WithSAMLSaas
--- PASS: TestAccCloudflareAccessApplication_WithSAMLSaas (7.89s)
=== RUN   TestAccCloudflareAccessApplication_WithSAMLSaas_Import
=== PAUSE TestAccCloudflareAccessApplication_WithSAMLSaas_Import
=== RUN   TestAccCloudflareAccessApplication_WithOIDCSaas
--- PASS: TestAccCloudflareAccessApplication_WithOIDCSaas (2.87s)
=== RUN   TestAccCloudflareAccessApplication_WithOIDCSaas_Import
=== PAUSE TestAccCloudflareAccessApplication_WithOIDCSaas_Import
=== RUN   TestAccCloudflareAccessApplication_WithAutoRedirectToIdentity
--- PASS: TestAccCloudflareAccessApplication_WithAutoRedirectToIdentity (4.15s)
=== RUN   TestAccCloudflareAccessApplication_WithEnableBindingCookie
--- PASS: TestAccCloudflareAccessApplication_WithEnableBindingCookie (2.75s)
=== RUN   TestAccCloudflareAccessApplication_WithCustomDenyFields
--- PASS: TestAccCloudflareAccessApplication_WithCustomDenyFields (2.87s)
=== RUN   TestAccCloudflareAccessApplication_WithADefinedIdps
--- PASS: TestAccCloudflareAccessApplication_WithADefinedIdps (3.94s)
=== RUN   TestAccCloudflareAccessApplication_WithMultipleIdpsReordered
--- PASS: TestAccCloudflareAccessApplication_WithMultipleIdpsReordered (8.33s)
=== RUN   TestAccCloudflareAccessApplication_WithHttpOnlyCookieAttribute
--- PASS: TestAccCloudflareAccessApplication_WithHttpOnlyCookieAttribute (2.85s)
=== RUN   TestAccCloudflareAccessApplication_WithHTTPOnlyCookieAttributeSetToFalse
--- PASS: TestAccCloudflareAccessApplication_WithHTTPOnlyCookieAttributeSetToFalse (3.43s)
=== RUN   TestAccCloudflareAccessApplication_WithSameSiteCookieAttribute
--- PASS: TestAccCloudflareAccessApplication_WithSameSiteCookieAttribute (2.82s)
=== RUN   TestAccCloudflareAccessApplication_WithLogoURL
--- PASS: TestAccCloudflareAccessApplication_WithLogoURL (2.83s)
=== RUN   TestAccCloudflareAccessApplication_WithSkipInterstitial
--- PASS: TestAccCloudflareAccessApplication_WithSkipInterstitial (3.43s)
=== RUN   TestAccCloudflareAccessApplication_WithAppLauncherVisible
--- PASS: TestAccCloudflareAccessApplication_WithAppLauncherVisible (2.74s)
=== RUN   TestAccCloudflareAccessApplication_WithTargetContexts
--- PASS: TestAccCloudflareAccessApplication_WithTargetContexts (7.59s)
=== RUN   TestAccCloudflareAccessApplication_WithDestinations
--- PASS: TestAccCloudflareAccessApplication_WithDestinations (7.01s)
=== RUN   TestAccCloudflareAccessApplication_WithSelfHostedDomains
--- PASS: TestAccCloudflareAccessApplication_WithSelfHostedDomains (4.86s)
=== RUN   TestAccCloudflareAccessApplication_WithDefinedTags
--- PASS: TestAccCloudflareAccessApplication_WithDefinedTags (4.01s)
=== RUN   TestAccCloudflareAccessApplication_WithReusablePolicies
--- PASS: TestAccCloudflareAccessApplication_WithReusablePolicies (5.00s)
=== RUN   TestAccCloudflareAccessApplication_WithAppLauncherCustomization
--- PASS: TestAccCloudflareAccessApplication_WithAppLauncherCustomization (2.65s)
=== RUN   TestAccCloudflareAccessApplication_AuthTypeForcesNewResource
--- PASS: TestAccCloudflareAccessApplication_AuthTypeForcesNewResource (10.63s)
=== CONT  TestAccCloudflareAccessApplication_WithSAMLSaas_Import
--- PASS: TestAccCloudflareAccessApplication_WithSAMLSaas_Import (3.78s)
=== CONT  TestAccCloudflareAccessApplication_WithOIDCSaas_Import
--- PASS: TestAccCloudflareAccessApplication_WithOIDCSaas_Import (3.70s)
PASS
ok  	github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider	144.993s

github-actions · 2024-12-11T04:58:46Z

This functionality has been released in v4.48.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!

veggiedefender requested a review from jacobbednarz as a code owner November 25, 2024 18:04

veggiedefender force-pushed the jesse/destinations branch from a82ba3c to f76e7b3 Compare November 25, 2024 18:05

veggiedefender commented Nov 25, 2024

View reviewed changes

veggiedefender force-pushed the jesse/destinations branch 3 times, most recently from 69985d4 to a1e55e5 Compare November 25, 2024 18:42

veggiedefender commented Nov 25, 2024

View reviewed changes

jacobbednarz reviewed Nov 25, 2024

View reviewed changes

veggiedefender force-pushed the jesse/destinations branch 2 times, most recently from b295366 to 9c23f84 Compare November 26, 2024 20:27

Michael9127 mentioned this pull request Nov 26, 2024

Mike/ztia-355 Adds allow_email_alias connection rule boolean to access infra policy #4665

Merged

veggiedefender mentioned this pull request Nov 26, 2024

Access IDP scim_config.identity_update_behavior is computed #4667

Merged

jacobbednarz force-pushed the jesse/destinations branch from 9c23f84 to 3d90298 Compare November 27, 2024 00:35

veggiedefender mentioned this pull request Dec 3, 2024

Fix Access app domain and self_hosted_domains import #4708

Draft

AUTH-6588 Support Access apps destinations field

818bd23

veggiedefender force-pushed the jesse/destinations branch from 3d90298 to 818bd23 Compare December 4, 2024 14:32

jacobbednarz merged commit 6576989 into cloudflare:master Dec 4, 2024
7 checks passed

github-actions bot added this to the v4.48.0 milestone Dec 4, 2024

jacobbednarz mentioned this pull request Dec 4, 2024

Fixes destroy on cloudflare_authenticated_origin_pulls #4649

Closed

github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AUTH-6588 Support Access apps `destinations` field #4661

AUTH-6588 Support Access apps `destinations` field #4661

veggiedefender commented Nov 25, 2024 •

edited

Loading

veggiedefender Nov 25, 2024 •

edited

Loading

veggiedefender Nov 25, 2024

jacobbednarz Nov 25, 2024

github-actions bot commented Nov 25, 2024

github-actions bot commented Nov 25, 2024

veggiedefender Nov 25, 2024

jacobbednarz Nov 25, 2024

veggiedefender Nov 26, 2024

jacobbednarz Nov 25, 2024

jacobbednarz Nov 25, 2024

jacobbednarz commented Nov 26, 2024

veggiedefender commented Dec 4, 2024

jacobbednarz commented Dec 4, 2024

github-actions bot commented Dec 11, 2024

AUTH-6588 Support Access apps destinations field #4661

AUTH-6588 Support Access apps destinations field #4661

Conversation

veggiedefender commented Nov 25, 2024 • edited Loading

veggiedefender Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

veggiedefender Nov 25, 2024

Choose a reason for hiding this comment

jacobbednarz Nov 25, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 25, 2024

github-actions bot commented Nov 25, 2024

veggiedefender Nov 25, 2024

Choose a reason for hiding this comment

jacobbednarz Nov 25, 2024

Choose a reason for hiding this comment

veggiedefender Nov 26, 2024

Choose a reason for hiding this comment

jacobbednarz Nov 25, 2024

Choose a reason for hiding this comment

jacobbednarz Nov 25, 2024

Choose a reason for hiding this comment

jacobbednarz commented Nov 26, 2024

veggiedefender commented Dec 4, 2024

jacobbednarz commented Dec 4, 2024

github-actions bot commented Dec 11, 2024

AUTH-6588 Support Access apps `destinations` field #4661

AUTH-6588 Support Access apps `destinations` field #4661

veggiedefender commented Nov 25, 2024 •

edited

Loading

veggiedefender Nov 25, 2024 •

edited

Loading