Add custom_client_ip_header and tls_policy to custom_domain resource #335

cv711 · 2022-10-06T15:12:16Z

🔧 Changes

This PR is exposing the custom_client_ip_header option for custom domains

🔬 Testing

📝 Checklist

All new/changed/fixed functionality is covered by tests (or N/A)
I have added documentation for all new/changed functionality (or N/A)

cv711 · 2022-10-10T13:46:30Z

So I'm trying to record some new http interactions and I'm facing a couple of problems:

The documentation doesn't describe that this check has to be commented out in order to record http interations for other domains.
After doing that and trying to proceed with recording a new http interaction I get the following error:

--- FAIL: TestAccCustomDomain (5.77s)
    resource_auth0_custom_domain_test.go:56: Step 1/1 error: Error running apply: exit status 1
        
        Error: failed to send the request: Post "https://domain.eu.auth0.com/api/v2/custom-domains": requested interaction not found
        
          with auth0_custom_domain.my_custom_domain,
          on terraform_plugin_test.tf line 2, in resource "auth0_custom_domain" "my_custom_domain":
           2: resource "auth0_custom_domain" "my_custom_domain" {
        
FAIL
FAIL    github.com/auth0/terraform-provider-auth0/internal/provider     6.517s
FAIL
make: *** [test-acc] Error 1

So I went ahead and manually added the missing field in the recording, since I don't have access to the domain the tests where created with.
If someone could update the recording for me with the correct process we should be ready to go with this one.
Thanks!

sergiught · 2022-10-10T17:03:46Z

Hey @cv711 👋🏻 thanks for the contribution! ⭐

I'll be able to help get this in the right place tomorrow, no worries about the conflicts either. If I can push to your branch I'll fix them as well.

sergiught · 2022-10-12T10:03:43Z

Hey @cv711, I set this branch to draft mode while we get this in the right place.

sergiught

Adding a few comments to ease up reviewing, also to give you an overview of all the extra changes that were still needed @cv711 . Hope it helps for future contributions from your part! ☘️ Looking forward to reviewing some more of your work:)

sergiught · 2022-10-12T12:59:26Z

internal/provider/resource_auth0_custom_domain.go

@@ -18,6 +17,7 @@ func newCustomDomain() *schema.Resource {
 	return &schema.Resource{
 		CreateContext: createCustomDomain,
 		ReadContext:   readCustomDomain,
+		UpdateContext: updateCustomDomain,


We're adding an update command as well because the custom_client_ip_header and tls_policy fields can be updated without recreating the custom domain entity.

sergiught · 2022-10-12T13:00:04Z

internal/provider/resource_auth0_custom_domain.go

+				Type:     schema.TypeString,
+				Optional: true,
+				ValidateFunc: validation.StringInSlice([]string{
+					"cf-connecting-ip", "x-forwarded-for", "true-client-ip", "",


We're adding as well validation as this field can only have certain values.

sergiught · 2022-10-12T13:00:24Z

internal/provider/resource_auth0_custom_domain.go

+				Description: "The HTTP header to fetch the client's IP address. " +
+					"Cannot be set on auth0_managed domains.",
+			},
+			"tls_policy": {


We're adding this field as well for completeness as the PATCH endpoint accepts it as well.

I was planning to add this field in my next PR but good to have them both here!

sergiught · 2022-10-12T13:01:03Z

internal/provider/resource_auth0_custom_domain_test.go

-	if os.Getenv("AUTH0_DOMAIN") != recorder.RecordingsDomain {
-		t.Skip()
-	}
+const testAccCreateSelfManagedCustomDomain = `


Refactoring the tests so they're covering more cases.

sergiught · 2022-10-12T13:01:39Z

internal/provider/resource_auth0_custom_domain_test.go

@@ -47,35 +46,135 @@ func init() {
 	})
 }

-func TestAccCustomDomain(t *testing.T) {
-	if os.Getenv("AUTH0_DOMAIN") != recorder.RecordingsDomain {


It turns out we didn't really need to run these tests only on the predefined recordings, we can run them on the live tenant on demand as well without anything breaking as we're not going through the verification.

sergiught · 2022-10-12T13:03:28Z

internal/provider/resource_auth0_custom_domain.go

 	}
+
+	if d.IsNewResource() {
+		customDomain.Domain = value.String(config.GetAttr("domain"))


We still wanna make sure the domain and type can only be set on a new resource but are not getting sent on the PATCH.

sergiught · 2022-10-12T13:08:14Z

@cv711 If you wanna test this as well on your side before we merge this and cut a release you can use the make install VERSION=XX.XX.XX command and then simply update your terraform config with the correct installed version. It will fetch it from local and you can use it.

e.g.

terraform {
  required_providers {
    auth0 = {
      source  = "auth0/auth0"
      version = "XX.XX.XX"
    }
  }
}

cv711 · 2022-10-12T15:22:31Z

🚀 all good from my side! Thank you very much for your help!

cv711 marked this pull request as ready for review October 6, 2022 15:19

cv711 requested a review from a team as a code owner October 6, 2022 15:19

sergiught force-pushed the feature/custom_client_ip_header branch from 9038dd1 to 10196d2 Compare October 12, 2022 09:52

sergiught marked this pull request as draft October 12, 2022 10:03

Add custom_client_ip_header and tls_policy to custom_domain resource

7bc0cd3

sergiught changed the title ~~add custom_client_ip_header~~ Add custom_client_ip_header and tls_policy to custom_domain resource Oct 12, 2022

sergiught force-pushed the feature/custom_client_ip_header branch from 10196d2 to 7bc0cd3 Compare October 12, 2022 12:55

sergiught reviewed Oct 12, 2022

View reviewed changes

sergiught requested a review from a team October 12, 2022 13:05

sergiught marked this pull request as ready for review October 12, 2022 13:05

sergiught approved these changes Oct 12, 2022

View reviewed changes

sergiught merged commit c48755f into auth0:main Oct 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add custom_client_ip_header and tls_policy to custom_domain resource #335

Add custom_client_ip_header and tls_policy to custom_domain resource #335

cv711 commented Oct 6, 2022 •

edited

Loading

cv711 commented Oct 10, 2022 •

edited by sergiught

Loading

sergiught commented Oct 10, 2022

sergiught commented Oct 12, 2022

sergiught left a comment

sergiught Oct 12, 2022

sergiught Oct 12, 2022

sergiught Oct 12, 2022

cv711 Oct 12, 2022

sergiught Oct 12, 2022

sergiught Oct 12, 2022

sergiught Oct 12, 2022

sergiught commented Oct 12, 2022

cv711 commented Oct 12, 2022

Add custom_client_ip_header and tls_policy to custom_domain resource #335

Add custom_client_ip_header and tls_policy to custom_domain resource #335

Conversation

cv711 commented Oct 6, 2022 • edited Loading

🔧 Changes

🔬 Testing

📝 Checklist

cv711 commented Oct 10, 2022 • edited by sergiught Loading

sergiught commented Oct 10, 2022

sergiught commented Oct 12, 2022

sergiught left a comment

Choose a reason for hiding this comment

sergiught Oct 12, 2022

Choose a reason for hiding this comment

sergiught Oct 12, 2022

Choose a reason for hiding this comment

sergiught Oct 12, 2022

Choose a reason for hiding this comment

cv711 Oct 12, 2022

Choose a reason for hiding this comment

sergiught Oct 12, 2022

Choose a reason for hiding this comment

sergiught Oct 12, 2022

Choose a reason for hiding this comment

sergiught Oct 12, 2022

Choose a reason for hiding this comment

sergiught commented Oct 12, 2022

cv711 commented Oct 12, 2022

cv711 commented Oct 6, 2022 •

edited

Loading

cv711 commented Oct 10, 2022 •

edited by sergiught

Loading