Skip to content

Add application credentials support to OS for JS2 #260

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

Open
almahmoud wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add application credentials support to OS for JS2 #260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f54c9b0
Add application credentials support
almahmoud Jun 17, 2022
7b8d2aa
Tmp working
almahmoud Jun 21, 2022
5938f37
Update django-cloudlaunch/cloudlaunch/backend_plugins/cloudman2/cloud…
Jun 21, 2022
00b8612
Update django-cloudlaunch/cloudlaunch/backend_plugins/cloudman2/cloud…
Jun 21, 2022
d74ae6f
Refactor common dict keys
Sep 15, 2022
3ece862
Merge branch 'master' into os_app_creds
Sep 15, 2022
0a0a485
Merge branch 'master' of https://github.com/galaxyproject/cloudlaunch…
almahmoud Sep 30, 2022
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
49 changes: 37 additions & 12 deletions django-cloudlaunch/cloudlaunch/backend_plugins/cloudman2/cloudman2_app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ def delete_iam():
GCP_CLOUD_CONF = \
"[Global]\n"

OPENSTACK_CLOUD_CONF = \
OPENSTACK_CLOUD_CONF_USER = \
"[Global]\n" \
"username=\"$os_username\"\n" \
"password=\"$os_password\"\n" \
Expand All @@ -349,6 +349,16 @@ def delete_iam():
"ignore-volume-az=$os_ignore_volume_az\n"


OPENSTACK_CLOUD_CONF_APP_CRED = \
"[Global]\n" \
"application-credential-id=\"$os_app_cred_id\"\n" \
"application-credential-secret=\"$os_app_cred_secret\"\n" \
"auth-url=$os_auth_url\n" \
"region=$os_region\n" \
"[BlockStorage]\n" \
"ignore-volume-az=$os_ignore_volume_az\n"


class CloudMan2AnsibleAppConfigurer(AnsibleAppConfigurer):
"""Add CloudMan2 specific vars to playbook."""

Expand Down Expand Up @@ -391,26 +401,41 @@ def _gen_cloud_conf(self, provider_id, cloud_config):
conf_template = GCP_CLOUD_CONF
values = {}
elif provider_id == "openstack":
# http://henriquetruta.github.io/openstack-cloud-provider/
conf_template = OPENSTACK_CLOUD_CONF
os_user = creds.get('os_username')
Copy link
Contributor

@ksuderman ksuderman Sep 14, 2022

Choose a reason for hiding this comment

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

Will os_username and os_password still work on Jetstream2? In my openrc file they have been replaced with
OS_APPLICATION_CREDENTIAL_ID and OS_APPLICATION_CREDENTIAL_SECRET

Copy link
Member Author

@almahmoud almahmoud Sep 15, 2022

Choose a reason for hiding this comment

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

They will not on JS2, the point of this PR is to add support for application credentials. However, given that this is openstack in general, not just for JS2, it's good to leave both options, so other places, eg: Nectar, would still work with user/pass. A few lines below, there is a check for whether both username and password are passed, and otherwise it falls back on application credentials.

Copy link
Member

@nuwang nuwang Sep 15, 2022

Choose a reason for hiding this comment

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

NeCTAR also supports app creds. What if we drop the legacy creds in cloudlaunch, and just stick to app creds only? The UI would certainly become simpler. We can leave things as is in cloudbridge, since we don't want an interface change, but for cloudlaunch, we may as well? Would need to test complete flow through cloudman including adding nodes though. Something we may need to do regardless?

Copy link
Member Author

@almahmoud almahmoud Sep 16, 2022

Choose a reason for hiding this comment

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

Ok I don't mind sticking to only app creds then

os_pass = creds.get('os_password')
os_ignore_az = self._os_ignore_az(
zone.get('zone_id'),
zone.get('region', {}).get('cloudbridge_settings'))
if creds.get('os_user_domain_id'):
domain_entry = f"domain-id={creds.get('os_user_domain_id')}"
else:
domain_entry = f"domain-name={creds.get('os_user_domain_name')}"

values = {
'os_username': creds.get('os_username'),
'os_password': creds.get('os_password'),
'domain_entry': domain_entry,
'os_tenant_name': creds.get('os_project_name'),
'os_auth_url': zone.get('cloud', {}).get('auth_url'),
'os_region': zone.get('region', {}).get('name'),
# https://github.com/kubernetes/kubernetes/issues/53488
'os_ignore_volume_az': os_ignore_az
}

if os_user and os_pass:
# http://henriquetruta.github.io/openstack-cloud-provider/
conf_template = OPENSTACK_CLOUD_CONF_USER
if creds.get('os_user_domain_id'):
domain_entry = f"domain-id={creds.get('os_user_domain_id')}"
else:
domain_entry = f"domain-name={creds.get('os_user_domain_name')}"

values.update({
'os_username': os_user,
'os_password': os_pass,
'domain_entry': domain_entry,
'os_tenant_name': creds.get('os_project_name'),
})
else:
# Assuming app credentials if no user and pass set
conf_template = OPENSTACK_CLOUD_CONF_APP_CRED
values.update({
# https://github.com/kubernetes/cloud-provider-openstack/blob/master/manifests/controller-manager/cloud-config
'os_app_cred_id': creds.get('os_application_credential_id'),
'os_app_cred_secret': creds.get('os_application_credential_secret'),
})

return string.Template(conf_template).substitute(values)

def _get_kube_cloud_settings(self, provider_config, cloud_config):
Expand Down
56 changes: 39 additions & 17 deletions django-cloudlaunch/cloudlaunch/backend_plugins/cloudman2_app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ def delete_iam():
GCP_CLOUD_CONF = \
"[Global]\n"

OPENSTACK_CLOUD_CONF = \
OPENSTACK_CLOUD_CONF_USER = \
"[Global]\n" \
"username=\"$os_username\"\n" \
"password=\"$os_password\"\n" \
Expand All @@ -348,6 +348,15 @@ def delete_iam():
"ignore-volume-az=$os_ignore_volume_az\n"


OPENSTACK_CLOUD_CONF_APP_CRED = \
"[Global]\n" \
"application-credential-id=\"$os_app_cred_id\"\n" \
"application-credential-secret=\"$os_app_cred_secret\"\n" \
"auth-url=$os_auth_url\n" \
"region=$os_region\n" \
"[BlockStorage]\n" \
"ignore-volume-az=$os_ignore_volume_az\n"

class CloudMan2AnsibleAppConfigurer(AnsibleAppConfigurer):
"""Add CloudMan2 specific vars to playbook."""

Expand Down Expand Up @@ -390,26 +399,39 @@ def _gen_cloud_conf(self, provider_id, cloud_config):
conf_template = GCP_CLOUD_CONF
values = {}
elif provider_id == "openstack":
# http://henriquetruta.github.io/openstack-cloud-provider/
conf_template = OPENSTACK_CLOUD_CONF
os_user = creds.get('os_username')
os_pass = creds.get('os_password')
if os_user and os_pass:
# http://henriquetruta.github.io/openstack-cloud-provider/
conf_template = OPENSTACK_CLOUD_CONF_USER
if creds.get('os_user_domain_id'):
domain_entry = f"domain-id={creds.get('os_user_domain_id')}"
else:
domain_entry = f"domain-name={creds.get('os_user_domain_name')}"

values = {
'os_username': os_user,
'os_password': os_pass,
'domain_entry': domain_entry,
'os_tenant_name': creds.get('os_project_name')
}
else:
# Assuming app credentials if no user and pass set
conf_template = OPENSTACK_CLOUD_CONF_APP_CRED
values = {
# https://github.com/kubernetes/cloud-provider-openstack/blob/master/manifests/controller-manager/cloud-config
'os_app_cred_id': creds.get('os_application_credential_id'),
'os_app_cred_secret': creds.get('os_application_credential_secret')
}

# https://github.com/kubernetes/kubernetes/issues/53488
os_ignore_az = self._os_ignore_az(
zone.get('zone_id'),
zone.get('region', {}).get('cloudbridge_settings'))
if creds.get('os_user_domain_id'):
domain_entry = f"domain-id={creds.get('os_user_domain_id')}"
else:
domain_entry = f"domain-name={creds.get('os_user_domain_name')}"
values['os_auth_url'] = zone.get('cloud', {}).get('auth_url')
values['os_region'] = zone.get('region', {}).get('name')
values['os_ignore_volume_az'] = os_ignore_az

values = {
'os_username': creds.get('os_username'),
'os_password': creds.get('os_password'),
'domain_entry': domain_entry,
'os_tenant_name': creds.get('os_project_name'),
'os_auth_url': zone.get('cloud', {}).get('auth_url'),
'os_region': zone.get('region', {}).get('name'),
# https://github.com/kubernetes/kubernetes/issues/53488
'os_ignore_volume_az': os_ignore_az
}
return string.Template(conf_template).substitute(values)

def _get_kube_cloud_settings(self, provider_config, cloud_config):
Expand Down
2 changes: 1 addition & 1 deletion setup.py
100755 → 100644
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def get_version(*file_paths):
history = open('HISTORY.rst').read().replace('.. :changelog:', '')

REQS_BASE = [
'Django>=3.0',
'Django<4.0',
# ======== Celery =========
'celery>=5.0',
# celery results backend which uses the django DB
Expand Down