-
Notifications
You must be signed in to change notification settings - Fork 91
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
Fix missing options when importing a connection object #142
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.
Leaving a few comments to aid with reviewing.
@@ -8,7 +8,7 @@ import ( | |||
) | |||
|
|||
func flattenConnectionOptions(d ResourceData, options interface{}) []interface{} { | |||
if options == nil || optionsBlockIsNotDefined(d) { |
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 where Issue #33 gets fixed.
After checking the terraform plugin SDK docs over here, even though the options might be omitted from the terraform config we should always record it within the state in the READ function as the Auth0 API assigns a couple of default values to certain elements.
This is also the reason why importing failed to get the options assigned within the state, although it was working fine after creating a new resource.
resource.TestCheckResourceAttr("auth0_connection.my_connection", "strategy", "samlp"), | ||
resource.TestCheckResourceAttr("auth0_connection.my_connection", "show_as_button", "false"), | ||
random.TestCheckResourceAttr("auth0_connection.my_connection", "display_name", "Acceptance-Test-SAML-{{.random}}", rand), | ||
resource.TestCheckResourceAttr("auth0_connection.my_connection", "options.#", "1"), |
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.
We complement the test by making sure we also check the options are set as previously it was omitted.
@@ -56,6 +56,7 @@ var connectionSchema = map[string]*schema.Schema{ | |||
}, | |||
"options": { | |||
Type: schema.TypeList, | |||
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.
Because the API assigns a couple of default values after creation if none are specified, this should be a computed property.
@@ -789,8 +790,6 @@ func readConnection(ctx context.Context, d *schema.ResourceData, m interface{}) | |||
return diag.FromErr(err) | |||
} | |||
|
|||
d.SetId(auth0.StringValue(connection.ID)) |
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.
From my understanding so far we should only set the ID of a resource when we create it, so no read method should set any ids as they are already set or if importing the d.Id()
will be the id of the connection already.
Description
Fixes #33.
Checklist
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?