crossplane-contrib / provider-keycloak

Apache License 2.0
25 stars 16 forks source link

Executions within Flows are created in a non-deterministic order. #149

Open mtrejo opened 3 months ago

mtrejo commented 3 months ago

When creating a Flow with multiple Execution steps, there is no way to specify the step order, and it seems they are created in a non-deterministic order.

Example:

apiVersion: authenticationflow.keycloak.crossplane.io/v1alpha1
kind: Flow
metadata:
  name: test
spec:
  deletionPolicy: Orphan
  forProvider:
    alias: Test Flow
    realmId: foo
  providerConfigRef:
    name: keycloak-provider
---
apiVersion: authenticationflow.keycloak.crossplane.io/v1alpha1
kind: Execution
metadata:
  name: foo-cookie
spec:
  deletionPolicy: Orphan
  forProvider:
    authenticator: auth-cookie
    parentFlowAliasRef:
      name: test
    realmId: foo
    requirement: ALTERNATIVE
  providerConfigRef:
    name: keycloak-provider
---
apiVersion: authenticationflow.keycloak.crossplane.io/v1alpha1
kind: Execution
metadata:
  name: foo-kerberos
spec:
  deletionPolicy: Orphan
  forProvider:
    authenticator: auth-spnego
    parentFlowAliasRef:
      name: test
    realmId: foo
    requirement: DISABLED
  providerConfigRef:
    name: keycloak-provider
---
apiVersion: authenticationflow.keycloak.crossplane.io/v1alpha1
kind: Execution
metadata:
  name: foo-identity-provider-redirector
spec:
  deletionPolicy: Orphan
  forProvider:
    authenticator: identity-provider-redirector
    parentFlowAliasRef:
      name: test
    realmId: foo
    requirement: ALTERNATIVE
  providerConfigRef:
    name: keycloak-provider

It seems the Terraform provider circumvented this issues by using the depends_on directive, but there's nothing like it in Kubernetes.

Breee commented 3 months ago

Why do you need a specific order?

mtrejo commented 3 months ago

@Breee, instead of me trying to explain why the order matters, here's the official documentation about authentication flows https://www.keycloak.org/docs/latest/server_development/#algorithm-overview, TLDR; the order in which the executions are listed is the order in which they will be executed.

Breee commented 3 months ago

I see. I don't think k8s controllers can solve that natively. But I have to Check that out. Maybe we can just add a reference to a "parentExecution" or so to Model the dependency

Some Tools like argocd or flux Support ordering or resources creation.

However, we can also Design a composition or function for that I guess. Or we try it with https://github.com/crossplane-contrib/function-sequencer

mtrejo commented 3 months ago

We are using argocd, I think this might be the feature you're referring to https://argo-cd.readthedocs.io/en/stable/user-guide/sync-waves/#how-do-i-configure-waves, I'll give it a try, thank you.

b509 commented 2 months ago

Hi @mtrejo,

I was aware of that when adding it, we need it for our usecase as well. It's an issue - as you mentioned - in the terraform provider, but even more gravely an issue in Keycloak's API itself. Because an execution step or subflow does not accept a "priority" at creation, only on update (or rather - it seems to accept it, but the priority value you send in the API has no effect, because in the backend it is hardcodedly set to last-execution-step's priority + 10)

I remember that I saw a commit that changed that in an upcoming Keycloak release, but cannot find it right now. The only thing I can find is this open issue https://github.com/keycloak/keycloak/issues/8726

Once this is resolved in Keycloak, it needs to be extended in the terraform provider for it to take effect in this provider.

Really bad workaround: Create the resources (execution steps, subflows) in the order that you need them to be and take care that crossplane runs a reconciliation after each kubectl apply.

EDIT: Found the other issue concerning this and also the fix for it in Keycloak 25 https://github.com/keycloak/keycloak/issues/20747 with PR https://github.com/keycloak/keycloak/pull/27751

and in the Terraform Provider the still open issue https://github.com/mrparkers/terraform-provider-keycloak/issues/296

mtrejo commented 2 months ago

Thank you @b509, that's pretty good to know, I wasn't aware this was an issue in Keycloak itself. Looks like a fix for the terraform provider is already in review, so hopefully we'll have a fix in the crossplane provider soon, as it so happens, we are just migrating to Keycloak 25.