i like the idea.

Importing by <realmId>/<clientId>/<roleId> would be perfect, especially if there are no weird keycloak UUIDS involved.

Changing this might break this function:

https://gitlab.com/corewire/images/crossplane/function-keycloak-builtin-objects

but we have to test it

@Breee I stumbled upon an case where the change lead to probably unwanted/unexpected behavior:

When one of the identifying properties are changed i.e. realmId or clientId in the OidcClient.
and
the provider pod is not currently running and started after that change (or the tf-state in workspace is renewed for any different reason)
then
a new OidcClient will be created and the old is left over.

Examples:

1. when provider pod is running and therefore tf-state is persisted in workspace. Renaming ClientId property from abc to 123 will result in an update of the existing client
2. when provider pod is not running. Renaming ClientId property from abc to 123. Then starting the provider pod will result in an newly created client 123 and the old client abc still exists.

What if we still maintain the UUID´s in external-name, but if UUID is empty or resource with that UUID is not existing, then utilize the identifying properties to retrieve the UUID?

We could apply the following logic:

1. Check if external-name is set
2. If external-name is set: try to resolve resource by UUID (get UUID from external-name)
3. If resource can be resolved by UUID: return UUID
4. If resource can NOT be resolved by UUID OR external-name is NOT set: try to resolve resource by identifying properties (realmId, clientId, etc.)
5. If resource can be resolved by identifying properties: return UUID
6. return indicator to create resource, because it does not exists

yes

@Breee I stumbled upon an case where the change lead to probably unwanted/unexpected behavior:

When one of the identifying properties are changed i.e. realmId or clientId in the OidcClient. and the provider pod is not currently running and started after that change (or the tf-state in workspace is renewed for any different reason) then a new OidcClient will be created and the old is left over.

Examples:

1. when provider pod is running and therefore tf-state is persisted in workspace. Renaming ClientId property from abc to 123 will result in an update of the existing client
2. when provider pod is not running. Renaming ClientId property from abc to 123. Then starting the provider pod will result in an newly created client 123 and the old client abc still exists.

What if we still maintain the UUID´s in external-name, but if UUID is empty or resource with that UUID is not existing, then utilize the identifying properties to retrieve the UUID?

We could apply the following logic:

1. Check if external-name is set
2. If external-name is set: try to resolve resource by UUID (get UUID from external-name)
3. If resource can be resolved by UUID: return UUID
4. If resource can NOT be resolved by UUID OR external-name is NOT set: try to resolve resource by identifying properties (realmId, clientId, etc.)
5. If resource can be resolved by identifying properties: return UUID
6. return indicator to create resource, because it does not exists

Yes, good catch. If people mess with the objects in keycloak that's probably in more cases a problem (it's a shame that they can not be made immutable on the UI)

I like the logic you proposed.

We then also have to test, how this affects existing deployments

@Breee seems to be that make generate throws errors when I am using generated types in config module:

	r := x.RoleParameters{}
	err = utils.UnmarshalTerraformParamsToObject(parameters, &r)
	if err != nil {
		return "", err
	}

	if r.RealmID == nil {
		return "", errors.New("realmId not set")
	}

	if r.Name == nil {
		return "", errors.New("name not set")
	}

	clientId := ""
	if r.ClientID != nil {
		clientId = *r.ClientID
	}

Any ideas how to solve that? Else I will switch back to my previous approach:

        realmID, realmIdExists := parameters["realm_id"]
	if !realmIdExists {
		return "", errors.New("realmId not set")
	}

	name, nameExists := parameters["name"]
	if !nameExists {
		return "", errors.New("name not set")
	}

	clientID, clientIdExists := parameters["client_id"]
	if !clientIdExists {
		clientID = ""
	}

...then we won´t get any compiler errors once those used attribute names are changed.

1. But I guess that is a very rare case for identifying properties.
2. Thats already the case in provider.go.KnownReferencers()

@Breee
I am getting closer...Only the ldap-resources are missing.

Can you already have a look at it?
BTW: It was necessary to access a private KeycloakClient method, because not every exposed API is implemented by the client. I would like to tackle this once we have a final version. Maybe terraform-keycloak-provider is willing to accept those additional methods.

How should we proceed? Does it make sense to release a new major version along with that feature to highlight the change?
Furthermore we could release it as ReleaseCandidate/PreRelease i.e "2.0.0-rc.1" and improve it further, but already ask for input from the community and offer users an opportunity to experiment with that changed approach.

What do you think?