URL encode role name.

[cthiebault]

Fix #205

[mrparkers]

This change looks good, but would you mind adding a test case that uses the same (or similar) role name to verify this change?

[cthiebault]

I've just converted the test testing / to URL stuff.

[cthiebault]

I'm not sure to understand why some tests failed :(

--- FAIL: TestAccKeycloakLdapUserFederation_syncPeriodValidation (0.61s)
    testing.go:621: Step 0, no error received, but expected a match to:
        
        expected .+ to be either -1 \(disabled\), or greater than zero

I didn't touch these files...

[tomrutsaert]

Does the test work locally on 7.0.1?

[cthiebault]

yes

KEYCLOAK_CLIENT_ID=terraform KEYCLOAK_CLIENT_SECRET=884e0f95-0f42-4a63-9b1f-94274655669e KEYCLOAK_REALM=master KEYCLOAK_URL="http://localhost:8080" TF_ACC=1 go test ./provider -v --run TestAccKeycloakLdapUserFederation_syncPeriodValidation

=== RUN   TestAccKeycloakLdapUserFederation_syncPeriodValidation
--- PASS: TestAccKeycloakLdapUserFederation_syncPeriodValidation (0.78s)
PASS
ok  	github.com/mrparkers/terraform-provider-keycloak/provider	0.791s

[mrparkers]

Probably just a flaky test 😄

[tomrutsaert]

@cthiebault and @mrparkers

This still seem to give errors with keycloak 9.0.0 and higher.

the ':' char is not allowed in a role name.

{"name":"terraform-role-http://foo.bar?a=1\u0026b=2#3ryrbugxt9","description":"","clientRole":false,"containerId":"","composite":false}
2020/05/27 12:31:36 [DEBUG] Response: 400 Bad Request
2020/05/27 12:31:36 [DEBUG] Response body: {"error":"Character ':' not allowed."}
2020/05/27 12:31:36 [DEBUG] keycloak_role.role: apply errored, but we're indicating that via the Error pointer rather than returning it: error sending POST request to /auth/admin/realms/terraform-qpu6avjuzh/roles: 400 Bad Request
2020/05/27 12:31:36 [ERROR] <root>: eval: *terraform.EvalApplyPost, err: error sending POST request to /auth/admin/realms/terraform-qpu6avjuzh/roles: 400 Bad Request
2020/05/27 12:31:36 [ERROR] <root>: eval: *terraform.EvalSequence, err: error sending POST request to /auth/admin/realms/terraform-qpu6avjuzh/roles: 400 Bad Request
2020/05/27 12:31:36 [DEBUG] New state was assigned lineage "46aaa8ae-a7f8-80bc-3fe4-be118c3d1043"
    TestAccKeycloakRole_basicRealmUrlRoleName: testing.go:640: Step 0 error: errors during apply:
        
        Error: error sending POST request to /auth/admin/realms/terraform-qpu6avjuzh/roles: 400 Bad Request
        
          on /tmp/tf-test444599851/main.tf line 6:
          (source code not available)

Can I just remove the ':' fromthe test are do you really need ':' char in your role names?

@hawknewton for your information #294

[mrparkers]

Yeah, if Keycloak stopped supporting this, then it makes sense to remove it from the tests.

Really not sure why they would have changed this.

[tomrutsaert]

Ok I will make a PR to remove the ';' char in the test

[tomrutsaert]

Apparently a lot a chars are not allowed anymore:
more information and discussion here:
keycloak/keycloak#6798

so basically all special chars in an url are not allowed.
Imho this seems a very strange change, but is in all version 9 and 10 of keycloak
@cthiebault I will not block it in logic, in case you need it for older versions of keycloak, but i will remove it from the test

[mrparkers]

And it's going to be fixed in 10.0.2, nice 😄

[hawknewton]

This may not be the proper venue (or perhaps this conversation's already been had) but what do you folx think about surfacing the error message that comes back from keycloak in the event of a 400? It would go a long way to making the module a lot easier to use.

[mrparkers]

Yeah we should definitely be doing that. I think I didn't worry about it too much in the beginning stages of this provider since Keycloak rarely surfaced errors through its API, but I think they've made a lot of improvements on that front so we could do a better job of making that more clear to the user.

For reference, you can set the environment variable TF_LOG=DEBUG to get a bunch of information back if you're wondering why something might have failed.