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

allow specification of parentId in customer_federation #325

Merged

Conversation

dmeyerholt
Copy link
Contributor

@dmeyerholt dmeyerholt commented Jun 18, 2020

This is a follow up of PR #270 and Issue #216
User Federations are internally represented by components. Those components are linked to its realm using the parent_id attribute which is the realms' id and not necessarily its name. Using solely this provider everything is fine because realm-name == realm-internald.
However, when existing realms are imported into the provider's state, the realm's internal_id can be different from the realm's name.
Since PR #270 we have the realms' internal id at hand. This PR extends the behavior of the userfederation resource, so it is possible to use the realm's internal id and not the realm name as parent_id. For this an additional attribute parent_id can be added to the respective resources if they need this feature. Unfortunately we need to specify both: realm name for the keycloak api calls and the parent_id to reference the realm's internal id.
This PR uses the realm name as parent_id if it is not specified so nothing should break here.

  • Code
  • Tests
  • Docs
  • Rebase & tidy up

Note: This should also be fixed similarly at least for the ldap user federation provider. if time permits I could have a look into that resource as well.

Copy link
Owner

@mrparkers mrparkers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code LGTM, thanks for taking a look at this.

I think for the test case, we'll have to create a realm using the API client (not using terraform) so it has a random internal ID, then attempt to create this resource using parent_id instead of realm_id.

@dmeyerholt dmeyerholt force-pushed the userfederation-internal-realmid branch 5 times, most recently from edda73b to ad8aeac Compare June 25, 2020 09:38
@dmeyerholt dmeyerholt marked this pull request as ready for review June 25, 2020 10:10
@dmeyerholt
Copy link
Contributor Author

Hello @mrparkers should be fine now :)

@dmeyerholt dmeyerholt force-pushed the userfederation-internal-realmid branch from ad8aeac to 7946e29 Compare June 29, 2020 11:17
@dmeyerholt
Copy link
Contributor Author

rebased to current master

Copy link
Owner

@mrparkers mrparkers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mrparkers mrparkers merged commit b720742 into mrparkers:master Jun 29, 2020
This pull request was closed.
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