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.

Sign up for GitHub

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

Jump to bottom

packaging: fix ovirt ovn provider for keycloak #379

Merged
merged 1 commit into from
May 31, 2022
Merged

packaging: fix ovirt ovn provider for keycloak #379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packaging/setup/ovirt_engine_setup/engine/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,8 @@ def OVIRT_PROVIDER_OVN_PASSWORD(self):

@osetupattrs(
is_secret=True,
answerfile=True,
postinstallfile=True,
)
def OVIRT_PROVIDER_OVN_SECRET(self):
return 'OVESETUP_OVN/ovirtProviderOvnSecret'
Expand Down
41 changes: 38 additions & 3 deletions packaging/setup/ovirt_engine_setup/engine_common/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,9 +395,44 @@ class RPMDistroEnv(object):
@util.codegen
@osetupattrsclass
class KeycloakEnv(object):
KEYCLOAK_ENABLED = 'OVESETUP_CONFIG/keycloakEnabled'
KEYCLOAK_OVIRT_ADMIN_USER = 'OVESETUP_CONFIG/keycloakOvirtAdmin'
KEYCLOAK_OVIRT_ADMIN_PASSWD = 'OVESETUP_CONFIG/keycloakOvirtAdminPasswd'
@osetupattrs(
answerfile=True,
postinstallfile=True,
summary=True,
reconfigurable=True,
description=_('Keycloak installation'),
)
def ENABLE(self):
return 'OVESETUP_CONFIG/keycloakEnable'

@osetupattrs(
is_secret=True,
answerfile=True,
postinstallfile=True,
)
def KEYCLOAK_OVIRT_INTERNAL_CLIENT_SECRET(self):
return 'OVESETUP_CONFIG/keycloakOvirtClientSecret'

@osetupattrs(
postinstallfile=True,
)
def KEYCLOAK_OVIRT_ADMIN_USER_WITH_PROFILE(self):
return 'OVESETUP_CONFIG/keycloakOvirtAdminWithProfile'

@osetupattrs(
is_secret=True,
)
def ADMIN_PASSWORD(self):
return 'OVESETUP_CONFIG/keycloakAdminPasswd'

CONFIGURED = 'OVESETUP_CONFIG/keycloakConfigured'
KEYCLOAK_AUTH_URL = 'OVESETUP_CONFIG/keycloakAuthUrl'
KEYCLOAK_TOKEN_URL = 'OVESETUP_CONFIG/keycloakTokenUrl'
KEYCLOAK_USERINFO_URL = 'OVESETUP_CONFIG/keycloakUserinfoUrl'
KEYCLOAK_GRAFANA_ADMIN_ROLE = 'OVESETUP_CONFIG/keycloakGrafanaAdminRole'
KEYCLOAK_GRAFANA_EDITOR_ROLE = 'OVESETUP_CONFIG/keycloakGrafanaEditorRole'
KEYCLOAK_GRAFANA_VIEWER_ROLE = 'OVESETUP_CONFIG/keycloakGrafanaViewerRole'
KEYCLOAK_OVIRT_INTERNAL_CLIENT_ID = 'OVESETUP_CONFIG/keycloakOvirtClientId'


# vim: expandtab tabstop=4 shiftwidth=4
4 changes: 2 additions & 2 deletions packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/aaa.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def _init(self):
None
)
self.environment.setdefault(
oengcommcons.KeycloakEnv.KEYCLOAK_ENABLED,
oengcommcons.KeycloakEnv.ENABLE,
False
)

Expand Down Expand Up @@ -280,7 +280,7 @@ def _attach_group_to_role(self):
oenginecons.ConfigEnv.ADMIN_PASSWORD
] is not None and
self.environment[
oengcommcons.KeycloakEnv.KEYCLOAK_ENABLED
oengcommcons.KeycloakEnv.ENABLE
] is False
)
)
Expand Down
122 changes: 99 additions & 23 deletions packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/network/ovirtproviderovn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,19 @@ def _init(self):
Defaults.DEFAULT_OVN_FIREWALLD_SERVICES
)
self.environment.setdefault(
oengcommcons.KeycloakEnv.KEYCLOAK_OVIRT_ADMIN_USER,
oengcommcons.KeycloakEnv.KEYCLOAK_OVIRT_ADMIN_USER_WITH_PROFILE,
None
)
self.environment.setdefault(
oengcommcons.KeycloakEnv.KEYCLOAK_OVIRT_ADMIN_PASSWD,
oengcommcons.KeycloakEnv.ADMIN_PASSWORD,
None
)
self.environment.setdefault(
oengcommcons.KeycloakEnv.KEYCLOAK_ENABLED,
oengcommcons.KeycloakEnv.ENABLE,
False
)
self.environment.setdefault(
oengcommcons.KeycloakEnv.CONFIGURED,
False
)

Expand Down Expand Up @@ -467,26 +471,36 @@ def _get_provider_credentials(self):
return user, password

keycloak_enabled = self.environment.get(
oengcommcons.KeycloakEnv.KEYCLOAK_ENABLED
oengcommcons.KeycloakEnv.ENABLE
)
if keycloak_enabled:
keycloak_configured = self.environment.get(
oengcommcons.KeycloakEnv.CONFIGURED
)
if keycloak_enabled and not keycloak_configured:
self.logger.debug(
'Setting up OVN provider credentials '
'for Keycloak based authentication'
)
user = self.environment[
oengcommcons.KeycloakEnv.KEYCLOAK_OVIRT_ADMIN_USER
]
password = self.environment[
oengcommcons.KeycloakEnv.KEYCLOAK_OVIRT_ADMIN_PASSWD
]
else:
user = self.environment[
oenginecons.ConfigEnv.ADMIN_USER
oengcommcons.KeycloakEnv.KEYCLOAK_OVIRT_ADMIN_USER_WITH_PROFILE
]
password = self.environment[
oenginecons.ConfigEnv.ADMIN_PASSWORD
oengcommcons.KeycloakEnv.ADMIN_PASSWORD
]
self.environment[
OvnEnv.OVIRT_PROVIDER_OVN_USER
] = user
self.environment[
OvnEnv.OVIRT_PROVIDER_OVN_PASSWORD
] = password
return user, password

user = self.environment[
oenginecons.ConfigEnv.ADMIN_USER
]
password = self.environment[
oenginecons.ConfigEnv.ADMIN_PASSWORD
]

use_default_credentials = False

Expand Down Expand Up @@ -772,11 +786,11 @@ def _create_config_content(self):
}

keycloak_enabled = self.environment.get(
oengcommcons.KeycloakEnv.KEYCLOAK_ENABLED
oengcommcons.KeycloakEnv.ENABLE
)
if keycloak_enabled:
keycloak_admin = self.environment[
oengcommcons.KeycloakEnv.KEYCLOAK_OVIRT_ADMIN_USER
oengcommcons.KeycloakEnv.KEYCLOAK_OVIRT_ADMIN_USER_WITH_PROFILE
]

self.logger.debug(
Expand Down Expand Up @@ -940,10 +954,23 @@ def _print_commands(self, message, commands):
after=(
oengcommcons.Stages.ADMIN_PASSWORD_SET,
),
condition=lambda self: self._enabled,
)
def _customization_credentials(self):
self._user, self._password = self._get_provider_credentials()
if self._enabled:
self._user, self._password = self._get_provider_credentials()
return

ovnprovider_installed = self._is_provider_installed()
keycloak_enabled = self.environment.get(
oengcommcons.KeycloakEnv.ENABLE
)
keycloak_configured = self.environment.get(
oengcommcons.KeycloakEnv.CONFIGURED
)

if ovnprovider_installed and \
keycloak_enabled and not keycloak_configured:
self._user, self._password = self._get_provider_credentials()

@plugin.event(
stage=plugin.Stages.STAGE_MISC,
Expand Down Expand Up @@ -1088,13 +1115,18 @@ def _misc_configure_ovn_timeout(self):
oenginecons.Stages.CA_AVAILABLE,
oenginecons.Stages.OVN_SERVICES_RESTART,
),
condition=lambda self:
self._enabled
)
def _misc_configure_provider(self):
self._generate_client_secret()
self._configure_ovirt_provider_ovn()
self._upate_external_providers_keystore()
if self._enabled:
self._generate_client_secret()
self._configure_ovirt_provider_ovn()
self._upate_external_providers_keystore()
elif self._provider_installed:
keycloak_enabled = self.environment.get(
oengcommcons.KeycloakEnv.ENABLE
)
if keycloak_enabled:
Copy link
Member

Choose a reason for hiding this comment

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

Same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, when subsequent engine-setup is executed on environment with already enabled keycloak, the ovirt provider ovn config /etc/ovirt-provider-ovn/conf.d/10-setup-ovirt-provider-ovn.conf will be simply rewritten. This is not the prettiest approach, but it is harmless and I think I saw similar cases elsewhere in setup code. No additional credentials related questions are supposed to be asked.

Copy link
Member

Choose a reason for hiding this comment

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

You are right that there are many other such cases, as I noted in the dwh patch. It will not even be rewritten, because filetransaction won't rewrite it if it already has the content we want. And the secret key is saved in postinstall, and AFAICT with this the content indeed will be the same. Didn't test.

However, if you do accept my suggestion to add a var CONFIGURED, perhaps better use it here too. Or not, if you think it's better to overwrite if needed. In the original place we added CONFIGURED, we actually eventually had to partially ignore it, because we wanted to make further changes to the ssl conf file (and later also had to revert that, due to wanting to rely on crypto-policies, but that's a different story...).

self._configure_ovirt_provider_ovn()

@plugin.event(
stage=plugin.Stages.STAGE_MISC,
Expand Down Expand Up @@ -1151,6 +1183,50 @@ def _misc_db_entries(self):
self._add_client_secret_to_db()
self._set_default_network_provider_in_db()

@plugin.event(
stage=plugin.Stages.STAGE_MISC,
after=(
oenginecons.Stages.OVN_PROVIDER_OVN_DB,
),
condition=lambda self: self._provider_installed,
Copy link
Member

Choose a reason for hiding this comment

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

Again. Not terribly bad, as I hope it will be harmless, but perhaps better check if we need to update.

Not sure how to best do this, exactly. If you do indeed add a 'KeycloakEnv.CONFIGURED' as discussed in the dwh patch, perhaps you can condition on this - update ovn only if keycloak is ENABLEd but not yet CONFIGURED.

And, to clarify (as I didn't write this in the dwh patch, sorry): I think they should both be in your current KeycloakEnv in engine_common/constants.py and used only from there everywhere - not duplicate ENABLE/D or CONFIGURED in dwh or ovirt-engine-keycloak.

Copy link
Contributor Author

@arso arso May 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

That makes sense. Originally I planned to make that unification after those patches/fixes, but I can see the benefits of having it much earlier. I am going to give it a try now, when the patches are, more or less, doing what they are supposed to do.

)
def _update_db_credentials(self):
keycloak_enabled = self.environment.get(
oengcommcons.KeycloakEnv.ENABLE
)
keycloak_configured = self.environment.get(
oengcommcons.KeycloakEnv.CONFIGURED
)
if keycloak_enabled and not keycloak_configured:
provider_id = self.environment.get(OvnEnv.OVIRT_PROVIDER_ID)
keycloak_admin = self.environment[
oengcommcons.KeycloakEnv.KEYCLOAK_OVIRT_ADMIN_USER_WITH_PROFILE
]
keycloak_password = self.environment[
oengcommcons.KeycloakEnv.ADMIN_PASSWORD
]
encrypted_password = self._encrypt_password(
keycloak_password
).decode()

self.environment[
oenginecons.EngineDBEnv.STATEMENT
].execute(
statement="""
UPDATE providers SET
auth_username = %(auth_username)s,
auth_password = %(auth_password)s
WHERE
id = %(provider_id)s
""",
args=dict(
provider_id=provider_id,
auth_username=keycloak_admin,
auth_password=encrypted_password
),
)
self._add_client_secret_to_db()

@plugin.event(
stage=plugin.Stages.STAGE_CLOSEUP,
before=(
Expand Down