Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 1 deletion .github/template_gitref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2021.08.26-425-gfdd7d2d
2021.08.26-426-g3a3f8a1
2 changes: 1 addition & 1 deletion .github/workflows/scripts/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ VARSYAML

cat >> vars/main.yaml << VARSYAML
pulp_env: {}
pulp_settings: null
pulp_settings: {"smart_proxy_rhsm_url": "https://rhsm.example.com/rhsm"}
pulp_scheme: https
pulp_default_container: ghcr.io/pulp/pulp-ci-centos9:latest
VARSYAML
Expand Down
4 changes: 4 additions & 0 deletions pulp_smart_proxy/app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,7 @@ class PulpSmartProxyPluginAppConfig(PulpPluginAppConfig):
version = "0.0.0.dev"
python_package_name = "pulp_smart_proxy"
domain_compatible = True

def ready(self):
super().ready()
from . import checks
15 changes: 15 additions & 0 deletions pulp_smart_proxy/app/checks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from django.conf import settings
from django.core.checks import Error as CheckError, register


@register(deploy=True)
def smart_proxy_rhsm_url_check(app_configs, **kwargs):
messages = []
if getattr(settings, "SMART_PROXY_RHSM_URL", "UNREACHABLE") == "UNREACHABLE":
messages.append(
CheckError(
"SMART_PROXY_RHSM_URL is a required setting but it was not configured.",
id="pulp_smart_proxy.E001",
)
)
return messages
10 changes: 4 additions & 6 deletions pulp_smart_proxy/app/settings.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
"""
Check `Plugin Writer's Guide`_ for more details.

.. _Plugin Writer's Guide:
https://pulpproject.org/pulpcore/docs/dev/
"""
SMART_PROXY_MIRROR = False
SMART_PROXY_AUTH_USERNAME = None
SMART_PROXY_AUTH_PASSWORD = None
SMART_PROXY_AUTH_METHODS = ["client_certificate"]
19 changes: 19 additions & 0 deletions pulp_smart_proxy/app/urls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
from django.conf import settings
from django.urls import path, include

from .views import FeaturesView, FeaturesV2View, VersionView

if settings.DOMAIN_ENABLED:
V3_API_ROOT = settings.V3_DOMAIN_API_ROOT_NO_FRONT_SLASH
else:
V3_API_ROOT = settings.V3_API_ROOT_NO_FRONT_SLASH

smart_proxy_patterns = [
path("features", FeaturesView.as_view()),
path("v2/features", FeaturesV2View.as_view()),
path("version", VersionView.as_view()),
]

urlpatterns = [
path(f"{V3_API_ROOT}smart_proxy/", include(smart_proxy_patterns)),
]
73 changes: 73 additions & 0 deletions pulp_smart_proxy/app/views.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
from urllib.parse import urljoin

from django.conf import settings
from drf_spectacular.utils import extend_schema
from rest_framework.response import Response
from rest_framework.views import APIView

from pulpcore.app.apps import pulp_plugin_configs, get_plugin_config


class FeaturesView(APIView):
"""
Returns features of the smart_proxy
"""

@extend_schema(
summary="Inspect features",
operation_id="features_read",
)
def get(self, request):
data = ["pulpcore"]
return Response(data)


class FeaturesV2View(APIView):
"""
Returns features of the smart_proxy in v2 format
"""

@extend_schema(
summary="Inspect features",
operation_id="featuresv2_read",
)
def get(self, request):
# there is no setting for the API url
# not adding /pulp/api/v3 here as Katello does so on its own
pulp_url = request.build_absolute_uri("/")
# CONTENT_ORIGIN can be None, guess based on the API url then
content_origin = settings.CONTENT_ORIGIN or pulp_url
capabilities = [app.label for app in pulp_plugin_configs()]
data = {
"pulpcore": {
"http_enabled": False,
"https_enabled": True,
"settings": {
"pulp_url": pulp_url,
"mirror": settings.SMART_PROXY_MIRROR,
"content_app_url": urljoin(content_origin, settings.CONTENT_PATH_PREFIX),
"username": settings.SMART_PROXY_AUTH_USERNAME,
"password": settings.SMART_PROXY_AUTH_PASSWORD,
"client_authentication": settings.SMART_PROXY_AUTH_METHODS,
Copy link
Member

Choose a reason for hiding this comment

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

I was wondering if we could derive these values from settings. We set some values (https://github.com/theforeman/puppet-pulpcore/blob/8288f3736a1543417eba6322e7013d0f230bd85e/templates/settings.py.erb#L52-L59) so perhaps that works.

I think you should check if settings.AUTHENTICATION_BACKENDS and derive values based on that.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about that, but then wasn't sure how to get from pulpcore.app.authentication.PulpNoCreateRemoteUserBackend to client_certificate -- does the one imply the other?

Looking at https://pulpproject.org/pulpcore/docs/admin/guides/auth/external/, PulpNoCreateRemoteUserBackend does imply external auth, but the kind of external is up to the user. (OTOH, we're in the Foreman-domain here, and we probably can imply cert?)

Copy link
Member

Choose a reason for hiding this comment

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

The non-cert option is mostly interesting for development setups, but I think it's highly questionable if it makes sense in this plugin.

In the Ruby based implementation we assume a trusted (authenticated) channel from Foreman to Foreman Proxy. The v2 features API (which exposes the settings) is normally only available if you are authenticated (typically using client certs).

If you take that to this, you either make the username/password available without authentication (effectively disabling authentication) or you have authentication but then don't need the credentials.

What you can do is to return [] if pulpcore.app.authentication.PulpNoCreateRemoteUserBackend isn't in there, but realistically the current implementation is probably OK.

Copy link
Member

Choose a reason for hiding this comment

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

Is this related to pulp/pulpcore#5438 which we could not bring forward until openapi 3.1.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorta, but also not really :)

We have a Pulp running somewhere, and we need to tell the "client" (Katello) how to get there and how to auth there.
Today we do this by talking to the smart proxy and that returns a mostly static setting hash (pulp_url, client_authentication are the most important here) that gets used in other parts of the app.

So yeah, if the APIdoc would announce "you can use certs here", that would get rid of that particular line, the whole concept of "someone tells katello how pulp is configured" remains (I'd argue the way we do discovery of services, like pulp, is not optimal)

"rhsm_url": settings.SMART_PROXY_RHSM_URL,
},
"state": "running",
"capabilities": capabilities,
}
}

return Response(data)


class VersionView(APIView):
"""
Returns version of the smart_proxy plugin
"""

@extend_schema(
summary="Inspect version",
operation_id="version_read",
)
def get(self, request):
data = {"version": get_plugin_config("smart_proxy").version}
Copy link
Member Author

Choose a reason for hiding this comment

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

This makes Foreman issue a warning as the version obviously doesn't match the Foreman version…

Copy link
Member

Choose a reason for hiding this comment

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

I thought Foreman is supposed to work with "older" versions of the smart proxy...

Copy link
Member Author

Choose a reason for hiding this comment

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

It does! Doesn't forbid it to issue useless warnings ;)

Copy link
Member

Choose a reason for hiding this comment

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

This makes Foreman issue a warning as the version obviously doesn't match the Foreman version…

Yes, I've been complaining about that a few times: we need a better way to determine compatibility than version numbers.

return Response(data)
17 changes: 17 additions & 0 deletions pulp_smart_proxy/tests/functional/api/test_smart_proxy_api.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import requests
from urllib.parse import urljoin


def test_version(pulp_api_v3_url):
r = requests.get(urljoin(pulp_api_v3_url, "smart_proxy/version"))
assert "version" in r.json()


def test_features(pulp_api_v3_url):
r = requests.get(urljoin(pulp_api_v3_url, "smart_proxy/features"))
assert ["pulpcore"] == r.json()


def test_features_v2(pulp_api_v3_url):
r = requests.get(urljoin(pulp_api_v3_url, "smart_proxy/v2/features"))
assert "pulpcore" in r.json()
3 changes: 2 additions & 1 deletion template_config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ pulp_env_azure: {}
pulp_env_gcp: {}
pulp_env_s3: {}
pulp_scheme: https
pulp_settings: null
pulp_settings:
smart_proxy_rhsm_url: https://rhsm.example.com/rhsm
pulp_settings_azure:
domain_enabled: true
pulp_settings_gcp: null
Expand Down