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

Extensions support in the operator #9785

Closed
vmuzikar opened this issue Jan 25, 2022 · 13 comments · Fixed by #10306
Closed

Extensions support in the operator #9785

vmuzikar opened this issue Jan 25, 2022 · 13 comments · Fixed by #10306
Assignees
Labels
area/operator Keycloak.X Operator kind/feature Categorizes a PR related to a new feature status/ready Ready to be merged
Milestone

Comments

@vmuzikar
Copy link
Contributor

vmuzikar commented Jan 25, 2022

  • Add a new field to Keycloak CRD for specifying a list of URLs to extensions.
  • The extensions will be downloaded and configured to be used in Keycloak.

Requires investigation on how this can work with the seamless augmentation.

See also:

@vmuzikar vmuzikar added area/operator Keycloak.X Operator kind/feature Categorizes a PR related to a new feature labels Jan 25, 2022
@vmuzikar vmuzikar added status/needs-discussion PR needs discussion on developer mailing list status/ready Ready to be merged and removed status/needs-discussion PR needs discussion on developer mailing list labels Feb 10, 2022
@andreaTP andreaTP self-assigned this Feb 11, 2022
@andreaTP
Copy link
Contributor

Context:
The current extensions mechanism is based on an init-container that simply runs curl to download jars directly into the Keycloak pod.

Consideration:
This mechanism needs to play well with Quarkus re/augmentation

Proposal:
Integrate the download of additional jars as a command line option for kc.sh that will be allowed only at build time.
E.g:

./kc.sh build --db=postgres --extensions="https://github.com/aerogear/keycloak-metrics-spi/releases/download/1.0.4/keycloak-metrics-spi-1.0.4.jar"

The behaviour will be to simply download the files in the providers directory before starting the augmentation process.

Pro:

  • Seamless workflow integration
  • Play nicely with "Custom Docker images"
  • Out of the box experience for the users
  • Same capabilities in K8s and on bare metal

Cons:

  • not strictly a CLI concern
  • increased code, maintenance etc...

Looking for feedback! @vmuzikar @pedroigor @DGuhr @stianst

@vmuzikar
Copy link
Contributor Author

@andreaTP I'm not 100% sure this belongs to the distribution (maybe it's more operator's job) but I agree it is convenient. But the param would definitely need a different naming. The directory is called providers, the param for downloading the providers is called extensions... Feels a bit confusing.

@andreaTP
Copy link
Contributor

Agreed that we should change the option name from extensions to (tentatively) providers.

@DGuhr
Copy link
Contributor

DGuhr commented Feb 15, 2022

@andreaTP @vmuzikar I like the approach in general. We also think about getting rid of "providers", "spi", "feature" etc. and provide a consistent "extension" approach overall, but this idea is not more than loosely in the heads for now, e.g. also spanning to wrap extensions using metadata and adding configuration options with this metadata.

So... in general I am with you, this would make it very convenient. And I also think the extension naming is more fitting/future proof. But not sure yet how this would fit in the overall extension approach.

Questions are e.g.

  • should this really be a param to the build command or a separate command instead? e.g. kc.sh extensions install ... && kc.sh build looks better for me at first sight,because the command could then also be used for e.g. searching kc.sh extensions search ... and it has a clearer separation from the build.

@stianst @pedroigor wdyt?

@vmuzikar
Copy link
Contributor Author

@DGuhr +1 for this "extensions manager" approach

@andreaTP
Copy link
Contributor

I think that implementing it as an additional command removes the value of the feature at all.

What would be the difference/added value then running a plain curl (or equivalent)? I do believe that integrating "package manager" functionalities (such as search) is out-of-scope from this proposal.

@vmuzikar
Copy link
Contributor Author

I agree a full implementation of "extension manager" is definitely out of scope for this. But we can at least align it with our future plans around it. AFAIK (@DGuhr and @pedroigor can correct me) in long term, we plan to have such manager for enabling/disabling extension in any case, so installing them is another logical step.

@pedroigor
Copy link
Contributor

The proposal is aligned with what we have been discussing about extensions. But how extensions will look like is not yet clear.

Until we don't decide how extensions will work, I'm afraid we can't include an option that will probably change in the future.

What Dominick mentioned is an example of how we plan to do it. A specific command and sub-commands to manage the different aspects of extensions.

@andreaTP
Copy link
Contributor

Since we have other long term plans on this subject we should be agnostic to it.
Next two options would be:

  • old approach: init container running curl + emptyDir shared with the main container
  • using a dedicated command for building and starting the image e.g.: /bin/bash -c curl ... && /opt/keycloak/bin/kc.sh build

@vmuzikar
Copy link
Contributor Author

@andreaTP I'm afraid that since we really can't make any long-term decision on how the "extension manager" should look like, what command it should be etc., and therefore we can't make any commitments in this area, the only non-breaking path forward is the init container. We can easily drop it in the future.

@andreaTP
Copy link
Contributor

@vmuzikar deal 👍

@stianst
Copy link
Contributor

stianst commented Feb 16, 2022

If we're doing a init container for this I reckon that perhaps we could at least see if we could get an agreement on how an extension would be installed using kc.sh. Honestly I'd say "kc.sh ext --install URL" is pretty future proof, and also easy to do. @DGuhr @pedroigor ?

By the way how does this align with any plans around optimised built image and static store stuff?

I'd still probably favour/recommend custom image though.

@andreaTP
Copy link
Contributor

Having to invoke an additional command have the same disadvantages as the "dedicated command building" along with the need to early commit to some design of the CLI.
We can proceed in steps:

  • use an init container as a temporary solution that give us the maximum freedom
  • switch to the keycloak CLI commands (even in an init container) in the future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/operator Keycloak.X Operator kind/feature Categorizes a PR related to a new feature status/ready Ready to be merged
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants