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

Setting default idle_session_lifetime on tenant import #849

Merged
merged 4 commits into from
Sep 28, 2023

Conversation

willvedd
Copy link
Contributor

@willvedd willvedd commented Sep 25, 2023

🔧 Changes

Sometimes when importing a auth0_tenant resource, the idle_session_lifetime field gets set to zero. Normally this isn't such a problem because subsequent plans or applies will have this value set to the default value of 72. However, the Auth0 CLI's auth0 tf generate command will get hung up on the zero value because it violates the schema's validation. This default is necessary because not all tenants will have the idle_session_lifetime value set, particularly if it is a new tenant.

📚 References

🔬 Testing

Manual verification on a new tenant. Difficult to add to acceptance tests because our testing tenants will always have this value set; it is important to have it not set for 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)

@willvedd willvedd requested a review from a team as a code owner September 25, 2023 14:37
internal/auth0/tenant/flatten.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Merging #849 (730ec82) into main (b4d4962) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #849      +/-   ##
==========================================
+ Coverage   89.89%   89.92%   +0.02%     
==========================================
  Files         101      101              
  Lines       13785    13793       +8     
==========================================
+ Hits        12392    12403      +11     
+ Misses        989      987       -2     
+ Partials      404      403       -1     
Files Coverage Δ
internal/auth0/tenant/flatten.go 100.00% <100.00%> (+4.91%) ⬆️
internal/auth0/tenant/resource.go 98.08% <100.00%> (ø)

internal/auth0/tenant/flatten_test.go Outdated Show resolved Hide resolved
internal/auth0/tenant/flatten.go Outdated Show resolved Hide resolved
@willvedd willvedd enabled auto-merge (squash) September 28, 2023 19:20
@willvedd willvedd merged commit 7f04a8e into main Sep 28, 2023
4 checks passed
@willvedd willvedd deleted the ESD-31284-use-default-tenant-idle-session-lifetime branch September 28, 2023 19:26
@dangbert
Copy link

dangbert commented Oct 3, 2023

Thank you for the fix, I just ran into this issue with auth0 tf generate (I'm on auth0 version 1.1.2 7646242b292c208470ebd4c78360a695bbf56f1a and "1.0.0" of this provider).

If idle_session_lifetime and session_lifetime are both 0 for data "auth0_tenant" "this" {} (as well as my imported tenant) does that indicate the value is actually set to 0 or this is part of the problem addressed here?

I'm importing an existing tenant that has been functioning fine for our application, but I'm weary about changing these values.

@willvedd
Copy link
Contributor Author

willvedd commented Oct 3, 2023

If idle_session_lifetime and session_lifetime are both 0 for data "auth0_tenant" "this" {} (as well as my imported tenant) does that indicate the value is actually set to 0 or this is part of the problem addressed here?

@dangbert both session lifetime values are required to be positive, non-zero values. The zeros you're seeing are the Go defaults being used in absence of the explicit values being returned from the tenant. In the Auth0 Management API, if the session lifetime values haven't been adjusted, they'll inherit the defaults and not be explicitly defined in your specific tenant object. The tenant that you're exporting to will behave the same as the existing tenant. Though I recommend to review the session lifetime values with your team to make sure they work for your particular business case.

@dangbert
Copy link

dangbert commented Oct 3, 2023

Thanks for the clarification @willvedd. That is indeed consistent with a later discussion I had with my team, so I've omitted the values as you suggested. Thanks for your fix and guidance, the timing was perfect 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants