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

Embed default provider at compile time #29

Closed
tlvenn opened this issue Nov 5, 2019 · 14 comments · Fixed by #34
Closed

Embed default provider at compile time #29

tlvenn opened this issue Nov 5, 2019 · 14 comments · Fixed by #34
Labels
enhancement New feature or request question Further information is requested

Comments

@tlvenn
Copy link

tlvenn commented Nov 5, 2019

Right now, google is the default provider set in the code unless told otherwise with a cli flag. Just like we can embed credentials at compile time, it would be great to be able to set the default provider as well.

@dkerwin
Copy link
Contributor

dkerwin commented Nov 7, 2019

Hi @tlvenn! Is this issue still something you would want to have after #30 is merged?

@dkerwin
Copy link
Contributor

dkerwin commented Nov 25, 2019

Now that each provider has its own sub command I close this issue. Please reopen if you think this feature is still worth implementing. Thanks!

@dkerwin dkerwin closed this as completed Nov 25, 2019
@tlvenn
Copy link
Author

tlvenn commented Nov 25, 2019

Hi @dkerwin, i think it's important when you have to provide a custom build, for example in the case of dex integration.

Whenever you provide a custom build to your end users, you should be able to alter the default provider so the cli command is as simple as possible. Right now it forces me to alter the code which is relatively trivial but would prefer to have a declarative approach.

@dkerwin
Copy link
Contributor

dkerwin commented Nov 25, 2019

Hi @tlvenn!

I understand your point. How would that look like now that providers are split into individual sub commands? Have a new subcommand (eg. default) that is just a wrapper around the compile time configured provider?

@dkerwin dkerwin reopened this Nov 25, 2019
@tlvenn
Copy link
Author

tlvenn commented Nov 25, 2019

Hi @dkerwin,

I though google was still implicit ? If there is no implicit provider anymore, I think having some kind of fallback would be great, I don't really expect that dexter auth default could work but dexter auth could / should fallback to a predefined provider that can be changed at compile time.

@phidlipus
Copy link

It would be great if we can set the default provider during the compile time and still use simpler form of command dexter auth instead of new form dexter auth google.

@tlvenn
Copy link
Author

tlvenn commented Nov 26, 2019

@Filip-Havlicek my though exactly

@dkerwin
Copy link
Contributor

dkerwin commented Nov 26, 2019

The problem I see here is that auth would show unpredictable behaviour. If a provider is predefined, it would run that and if not it would just show the help screen. Wouldn't it be exactly as helpful to have a new command (eg. default, builtin, xxx)? Is the driver for the both of you to give your users a consistent experience even if you decide to switch providers?

@tlvenn
Copy link
Author

tlvenn commented Nov 26, 2019

I think it's more about trying to keep it as simple / stupid as possible for end users.
If a given org is only going to use a single provider at a time, why should it forces its users to specify it all the time, it should be transparent / invisible.

Maybe the official release of dexter should not favor one provider over another so effectively there is no fallback defined and the provider has to be explicit. And for custom build being released by orgs, they can define the fallback provider at compile time so that it's invisible to their users if they want to.

It's the same reasoning for the a potential Dex integration and its endpoint which is by nature custom. I would rather tell people in my org to do: dexter auth rather than dexter auth dex https://dex.mycompany.com

Of course the tradeoff being that now someone needs to build and distribute a custom build. Some orgs might choose the simpler approach on leveraging the official distribution and pushing the concern of oidc provider and its endpoint to their users and some org will choose the other path but I believe both should exist and be facilitated without forcing people to create a fork of the project and to maintain it.

@phidlipus
Copy link

We are only using google, so we don't need other providers (at the moment). I understand why you reworked provider code. I just liked old behaviour when I simply wrote dexter auth.
New additional command for default provider doesn't make sense (for me). In that case, it's not problem use the real provider name (google in my case).
By the way, I like dexter and thanks for it.

@dkerwin dkerwin added enhancement New feature or request question Further information is requested labels Nov 27, 2019
@dkerwin
Copy link
Contributor

dkerwin commented Nov 27, 2019

Thank you both for the clarifications. I'll have a closer look at the problem see if I can come up with a good solution. I'm glad dexter is helpful to you @Filip-Havlicek

@dkerwin
Copy link
Contributor

dkerwin commented Nov 27, 2019

Hey @tlvenn & @Filip-Havlicek,

would be great if you could give the PR a spin and see if it matches your expectations.

@phidlipus
Copy link

@dkerwin I built it and it works as expected and it matches my expectations. You are fast.

@tlvenn
Copy link
Author

tlvenn commented Nov 27, 2019

Awesome, that's perfect, thanks @dkerwin !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants