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

Upgrade to TF 0.12 #107

Merged
merged 4 commits into from
May 14, 2019
Merged

Upgrade to TF 0.12 #107

merged 4 commits into from
May 14, 2019

Conversation

ctrox
Copy link
Contributor

@ctrox ctrox commented May 8, 2019

This upgrades the vendored TF version to v0.12.0-rc1 to make the provider compatible with Terraform 0.12.

Two acceptance tests still fail and I'm not sure how to handle them:

--- FAIL: TestAccKeycloakOpenIdAudienceProtocolMapper_validateClientConflictsWithClientScope (0.72s)
    testing.go:549: Step 0, no error received, but expected a match to:

        .+ conflicts with .+


=== RUN   TestAccKeycloakOpenIdAudienceProtocolMapper_validateClientAudienceConflictsWithCustomAudience
--- FAIL: TestAccKeycloakOpenIdAudienceProtocolMapper_validateClientAudienceConflictsWithCustomAudience (0.72s)
    testing.go:549: Step 0, no error received, but expected a match to:

        .+ conflicts with .+

It looks like behaviour of the ConflictsWith has changed if the values are unknown. So with Terraform 0.12 the "conflicts with x" error will only appear if the values are known so for example this will not result in an error:

resource "keycloak_openid_audience_protocol_mapper" "audience_mapper" {
  name            = "foo"
  realm_id        = "${keycloak_realm.realm.id}"
  client_id       = "${keycloak_openid_client.openid_client.id}"
  client_scope_id = "${keycloak_openid_client_scope.client_scope.id}"

  included_custom_audience = "foo"
}

But this will:

resource "keycloak_openid_audience_protocol_mapper" "audience_mapper" {
  name            = "foo"
  realm_id        = "bar"
  client_id       = "bar"
  client_scope_id = "${keycloak_openid_client_scope.client_scope.id}"

  included_custom_audience = "foo"
}

I'm actually not sure if this is a bug in TF 0.12 since it also does not catch the ConflictsWith once the values are known (during apply and subsequent plans).

@mrparkers
Copy link
Contributor

Hi @ctrox, thanks for the PR! I agree that it seems like a bug, or at least a major change in the behavior of ConflictsWith. I don't think this will be hard to get around though - the audience protocol mapper has a validation function where this logic can be moved to.

Let me know if you need some assistance. This has been on my todo list for a while, so as soon as this is merged I will cut a release for you 😄

@ctrox
Copy link
Contributor Author

ctrox commented May 10, 2019

@mrparkers Thanks for the pointers. I added two additional validations, is this what you meant? Maybe I will still open an issue in the TF repo about this as i think the ConflictsWith should still work with computed values during apply.

@ctrox ctrox changed the title [WIP] Upgrade to TF 0.12 Upgrade to TF 0.12 May 10, 2019
@mrparkers
Copy link
Contributor

Looks perfect! I can merge this as soon as the merge conflicts are fixed.

I also agree that opening an issue in the Terraform core repo would be a good idea. It would be nice to know if they intend on restoring the original behavior of ConflictsWith.

Thanks again for the PR!

@ctrox
Copy link
Contributor Author

ctrox commented May 10, 2019

Great, thanks! I rebased on master.

@mrparkers mrparkers merged commit 37a550d into keycloak:master May 14, 2019
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.

2 participants