-
Notifications
You must be signed in to change notification settings - Fork 83
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
DXCDT-95 Stop ignoring errors when setting resource data within the connection #97
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a few comments to aid with reviewing.
@@ -243,6 +218,7 @@ var connectionSchema = map[string]*schema.Schema{ | |||
}, | |||
"allowed_audiences": { | |||
Type: schema.TypeSet, | |||
Computed: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue here is that we define all possible parameters of the options object for all the different connection types and with the legacy SDK (v1) if we don't set something up for a TypeSet initially it will be read as "" but then after a state update it will be automatically set to an empty []string making the tests fail cuz there's an unexpected plan change. To fix this we need to set computed on this property and several others.
@@ -77,7 +77,6 @@ func TestAccConnection(t *testing.T) { | |||
resource.TestCheckResourceAttr("auth0_connection.my_connection", "options.0.import_mode", "false"), | |||
resource.TestCheckResourceAttr("auth0_connection.my_connection", "options.0.disable_signup", "false"), | |||
resource.TestCheckResourceAttr("auth0_connection.my_connection", "options.0.requires_username", "true"), | |||
resource.TestCheckResourceAttr("auth0_connection.my_connection", "options.0.set_user_root_attributes", "on_each_login"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually not allowed and it was never set within the flatten for the Auth0 connection type. This property only works for enterprise connections.
@@ -216,6 +212,7 @@ resource "auth0_connection" "ad" { | |||
strategy = "ad" | |||
show_as_button = true | |||
options { | |||
brute_force_protection = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default by the API is to set brute force conection as enabled. Like this we remove the hack we introduced here https://github.com/auth0/terraform-provider-auth0/pull/97/files#diff-f2cec8e18d10bdff521bfda90a79b3154d267cccd5925b7ec9393cf7eeec7726L662-L666
"non_persistent_attrs": o.GetNonPersistentAttrs(), | ||
"scopes": o.Scopes(), | ||
} | ||
func optionsBlockIsNotDefined(d ResourceData) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This now allows to have a connection without options block defined in the terraform config.
Description
This PR stops ignoring errors when setting resource data within the connection.
Ultimately fixing these fixes several issues we have on github:
options
when importing aconnection
object #33auth0_connection
400 Bad request for google-apps strategy #23Checklist
Note: Checklist required to be completed before a PR is considered to be reviewable.
Auth0 Code of Conduct
Auth0 General Contribution Guidelines
Changes include test coverage?
Does the description provide the correct amount of context?
Have you updated the documentation?
Is this code ready for production?