Giter Club home page Giter Club logo

Comments (6)

Langleu avatar Langleu commented on July 20, 2024 1

@jessesimpson36 so you changed the Helm chart's behavior by accident compared to previous versions and now deem it a feature?
What for do you need KeyCloak as part of the Helm chart in a non identity environment?
it will block anything since it isn't configured, as that happens through identity and it's not like we have any docs that tell you how to configure keycloak without identity.
Last but not least, I would probably not use the Camunda Helm Chart to deploy just KeyCloak.

from camunda-platform-helm.

jessesimpson36 avatar jessesimpson36 commented on July 20, 2024 1

The following is for extra context on why my previous PR is closed, what I tried to do was change the conditions part of the helm subchart dependency

dependencies:
  # Identity Dependencies.
  - name: keycloak
    alias: identityKeycloak
    repository: https://charts.bitnami.com/bitnami
    version: 19.4.1
-     condition: "identityKeycloak.enabled"
+     condition: "identity.enabled,identityKeycloak.enabled"

After playing around with helm template commands to see whether keycloak would actually be rendered or not, what I found was that this "condition" is neither an AND operation nor an OR operation. The behavior with the comma is unintuitive. But what it does is it determines whether the first variable left-most of the commas is non-null. If it's non-null, (so true or false), then that condition is what's used to evaluate whether it should render the subchart. If it's null, it looks at the next argument after the comma.

if someone specified a values.yaml like

identity:
  enabled: false
identityKeycloak:
  enabled: true

I personally would expect that keycloak would be deployed, because my view of enabled when directly under a top-level key is that it should render or not render whatever is beneath it, regardless of whether it makes sense. However, if we took the old syntax for keycloak

identity:
  enabled: false
  keycloak:
    enabled: true

I would expect that neither would be rendered because in here keycloak is not a top-level key.

My expectations also differ when you take into account that some of these keys might be enabled by default in our values.yaml, and other keys may be explicitly set. So...

identity:
  enabled: false

expresses an intent to disable identity, so I would expect keycloak to be disabled. but the default values.yaml enables identityKeycloak by default. Meanwhile, despite being equivalent,

identity:
  enabled: false
identityKeycloak:
  enabled: true

This expresses intent to enable keycloak despite identity being false.

from camunda-platform-helm.

aabouzaid avatar aabouzaid commented on July 20, 2024 1

Given how Helm deals with dependencies conditions, the way to handle this should be using a constraint to fail if Identity is disabled and Keycloak is enabled.

So I put that in backlog till we work on it.

from camunda-platform-helm.

jessesimpson36 avatar jessesimpson36 commented on July 20, 2024

Now that I think about it, there are situations where one could want identity disabled with keycloak enabled, or for identity enabled and keycloak disabled. so I don't think this ticket is as much of a bug as we think it is.

Identity enabled + Keycloak disabled = OIDC enabled environment

Identity disabled + Keycloak enabled = self-configured keycloak instance

both enabled = typical install

both disabled = each app managing their own auth.

from camunda-platform-helm.

jessesimpson36 avatar jessesimpson36 commented on July 20, 2024

so you changed the Helm chart's behavior by accident compared to previous versions and now deem it a feature?

I wouldn't describe it like that. The old behavior where keycloak was disabled when identity was disabled was an implicit side-effect of identity being a subchart. This is no longer the case anymore, since we've moved identity to be in our templates.

So what we're discussing is whether the implicit functionality was intended or not, and whether we should have an explicit code fix to maintain that behavior. And one of the technical constraints we have, is that the conditions section of Chart.yaml has some pretty confusing side-effects if we try to use it to get that old behavior back. So needless to say, the PR where I was investigating a quick fix is not sufficient, and fixing without introducing more bugs is going to be more than a 1 day task.

What for do you need KeyCloak as part of the Helm chart in a non identity environment?

Customers sometimes choose to not enable identity, due to feeling like it is insecure to give a service access to control authentication services. Often this will be the case in situations where Keycloak is used for more than simply authenticating against camunda, such as if keycloak was also allowing you to authenticate to a JFrog registry.

it will block anything since it isn't configured, as that happens through identity and it's not like we have any docs that tell you how to configure keycloak without identity.

Customers do configure keycloak themselves despite there not being Camunda documentation on the subject. I'm personally pretty impressed when community members are able to figure this sort of thing out, because Keycloak is quite difficult for me to work with.

I will also discuss this topic with @theburi .

from camunda-platform-helm.

Langleu avatar Langleu commented on July 20, 2024

Thank you for the in-depth explanation of why you changed your mind.

If it's technically not easy to implement, then maybe adding the information about the behaviour change to the release notes / upgrade notes may be sufficient already.
Otherwise, customers may naturally assume it to be a bug since it diverges from what they have known in previous releases.

from camunda-platform-helm.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.