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

packaging: unify ovirt admin user email #508

Merged
merged 1 commit into from
Jul 4, 2022

Conversation

arso
Copy link
Contributor

@arso arso commented Jun 30, 2022

This patch changes default ovirt admin email address to one required
(hardcoded) by grafana so that oVirt SSO works when Keycloak integration
is disabled.

@didib
Copy link
Member

didib commented Jun 30, 2022

As discussed in private, I can see value in this patch, but do not have the full picture - can't say if it's risking upgrades or other flows, or how hard it is to achieve good-enough convenience without it - e.g. by (automatically?) creating another admin user in grafana (which IIRC is what we did until keycloak).

@arso
Copy link
Contributor Author

arso commented Jun 30, 2022

As discussed in private, I can see value in this patch, but do not have the full picture - can't say if it's risking upgrades or other flows, or how hard it is to achieve good-enough convenience without it - e.g. by (automatically?) creating another admin user in grafana (which IIRC is what we did until keycloak).

@didib You are absolutely right - I created that patch mostly to run it against OST.

@arso arso marked this pull request as draft June 30, 2022 13:38
@arso arso marked this pull request as draft June 30, 2022 13:38
@didib
Copy link
Member

didib commented Jun 30, 2022

If we do merge it, perhaps we should have a doc bug to update the text around "For this option, you need an operational local mail server configured on the Grafana machine", in the admin guide. I guess I phrased it generally enough, at the time, to accommodate also current patch, not mentioning that you should make mail to root@localhost approachable somehow (e.g. alias it to your own address or have a mail reader on the machine).

@mwperina
Copy link
Member

AFAIK nothing uses email address for admin@internal. This email address has been added only because of grafana, as it requires all users to contain valid address

So I'm OK to change to admin@localhost if needed, but I'd like to know why root@localhost is not working properly on grafana SSO

This patch changes default ovirt admin email address to one required
(hardcoded) by grafana so that oVirt SSO works when Keycloak integration
is disabled.
Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@mwperina
Copy link
Member

mwperina commented Jul 4, 2022

Let's convert it to regular patch and merge

@arso arso marked this pull request as ready for review July 4, 2022 15:11
@arso
Copy link
Contributor Author

arso commented Jul 4, 2022

This is the related OST patch: oVirt/ovirt-system-tests#208

@arso arso requested a review from avlitman July 4, 2022 15:12
@mwperina mwperina merged commit d5fda64 into oVirt:master Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants