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

KeycloakResourceBuilderExtensions.WithRealmImport is unnecessarily constraining the bind to a directory #5086

Closed
paulomorgado opened this issue Jul 26, 2024 · 6 comments · Fixed by #7121
Labels
area-integrations Issues pertaining to Aspire Integrations packages keycloak Issues related to keycloack integrations
Milestone

Comments

@paulomorgado
Copy link
Contributor

paulomorgado commented Jul 26, 2024

Background and Motivation

KeycloakResourceBuilderExtensions.WithRealmImport is unnecessarily constraining the bind to a single directory, constraining the user choices.

Proposed API

Consider something like this, instead:

    public static IResourceBuilder<KeycloakResource> WithRealmImport(
        this IResourceBuilder<KeycloakResource> builder,
        string import,
        bool isReadOnly = false)
    {
        ArgumentNullException.ThrowIfNull(importDirectory);

        if (Directory.Exists(import))
        {
            builder.WithBindMount(import, "/opt/keycloak/data/import", isReadOnly);
        }
        else
        {
            var fileName = Path.GetFileName(import);

            builder.WithBindMount(import, $"/opt/keycloak/data/import/{fileName}", isReadOnly);
        }

        return builder;
    }

Do not test if the source file or directory exists, because the user might create them later.

Usage Examples

var keycloak = builder.AddKeycloak(...);
// ...
keycloak.WithRealmImport(<path to file>);
// ...
keycloak.WithRealmImport(<path to another file>);
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Jul 26, 2024
@davidfowl davidfowl added area-integrations Issues pertaining to Aspire Integrations packages bug and removed area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication labels Sep 7, 2024
@davidfowl davidfowl added the keycloak Issues related to keycloack integrations label Sep 29, 2024
@davidfowl davidfowl removed the bug label Oct 16, 2024
@eerhardt eerhardt added this to the Backlog milestone Jan 14, 2025
@eerhardt
Copy link
Member

@julioct @DamianEdwards - any thoughts on this?

@paulomorgado
Copy link
Contributor Author

The user might want to bind a single file at a time or the whole directory.

@DamianEdwards
Copy link
Member

Seems fine at first glance. I admit I've only used this pattern in containers by binding whole directories so far (it's a similar pattern to how many of the DB container support init SQL files).

What happens if a user calls it with a directory first, and then with a file? Does the right thing just happen?

@paulomorgado
Copy link
Contributor Author

Not any different than what happened with a container resource bind.

I have an Aspire host where I, by host configuration, use local or remote identity providers. To avoid load unused realms, I conditionally load realms by conditionally binding to the realm configuration files.

@DamianEdwards
Copy link
Member

So this ends up being a tweak to the existing API and shouldn't be a compatibility issue AFAICT. @paulomorgado feel free to send a PR for this change and we can review there.

@paulomorgado
Copy link
Contributor Author

@DamianEdwards, PR #7121 created!

@github-actions github-actions bot locked and limited conversation to collaborators Feb 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-integrations Issues pertaining to Aspire Integrations packages keycloak Issues related to keycloack integrations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants