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

Add application credentials support to OS for JS2 #260

Open
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 4 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

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

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

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

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
65 changes: 46 additions & 19 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,44 @@ 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_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')}"
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
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': os_user,
'os_password': os_pass,
'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
}
else:
domain_entry = f"domain-name={creds.get('os_user_domain_name')}"
# Assuming app credentials if no user and pass set
conf_template = OPENSTACK_CLOUD_CONF_APP_CRED
os_ignore_az = self._os_ignore_az(
almahmoud marked this conversation as resolved.
Show resolved Hide resolved
zone.get('zone_id'),
zone.get('region', {}).get('cloudbridge_settings'))
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'),
'os_auth_url': zone.get('cloud', {}).get('auth_url'),
'os_region': zone.get('region', {}).get('name'),
'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
4 changes: 2 additions & 2 deletions requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
git+https://github.com/encode/django-rest-framework
# install edge till this is released: https://github.com/celery/django-celery-results/issues/157
git+https://github.com/celery/django-celery-results
git+https://github.com/CloudVE/cloudbridge#egg=cloudbridge
git+https://github.com/CloudVE/djcloudbridge#egg=djcloudbridge
git+https://github.com/CloudVE/cloudbridge@os_app_creds_support#egg=cloudbridge[full]
git+https://github.com/almahmoud/djcloudbridge@os_app_creds
-e ".[prod]"
6 changes: 3 additions & 3 deletions setup.py
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',
almahmoud marked this conversation as resolved.
Show resolved Hide resolved
# ======== Celery =========
'celery>=5.0',
# celery results backend which uses the django DB
Expand All @@ -63,8 +63,8 @@ def get_version(*file_paths):
# Provides REST API schema
'coreapi>=2.2.3',
# ======== CloudBridge =========
'cloudbridge',
'djcloudbridge',
'cloudbridge@git+https://github.com/CloudVE/cloudbridge@os_app_creds_support',
'djcloudbridge@git+https://github.com/almahmoud/djcloudbridge@os_app_creds',
# ======== Django =========
# Provides better inheritance support for django models
'django-model-utils',
Expand Down