Skip to content

Add apache config support for /pypi#458

Open
pavanshekar wants to merge 1 commit intotheforeman:masterfrom
pavanshekar:add-pypi-support
Open

Add apache config support for /pypi#458
pavanshekar wants to merge 1 commit intotheforeman:masterfrom
pavanshekar:add-pypi-support

Conversation

@pavanshekar
Copy link
Copy Markdown

Why are you introducing these changes? (Problem description, related links)

Add Apache config support for /pypi in foremanctl

Fixes SAT-44475

What are the changes introduced in this pull request?

  • Added <Location "/pypi"> block in foreman-ssl-vhost.conf.j2 to proxy requests to Pulp API backend
  • Configured required headers: X-CLIENT-CERT, X-FORWARDED-PROTO
  • Set timeout to 600 seconds (matching other Pulp API routes)

How to test this pull request

Steps to reproduce:

  • Deploy the changes
  • SSH into the VM
  • Verify the Apache configuration was generated:
    grep -A 6 'Location "/pypi"' /etc/httpd/conf.d/foreman-ssl.conf
  • Expected: Should show the /pypi Location block with correct headers and proxy settings

Checklist

  • Tests added/updated (if applicable)
  • Documentation updated (if applicable)

ProxyPassReverse {{ httpd_pulp_content_backend }}/pulp/content
</Location>

<Location "/pypi">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In foreman-installer this was added conditionally if python content type was enabled. I think we should do the same here so as not to expose routes that have no effect.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I've updated the template to conditionally include the /pypi Location block only when pulp_python is in the enabled plugins list.

ProxyPassReverse {{ httpd_pulp_content_backend }}/pulp/content
</Location>

{% if 'pulp_python' in (pulp_enabled_plugins | default([])) %}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The problem is that this variable is from the pulp role, and this is the httpd role.

@evgeni -- do you think we should go with role variables httpd_pulp_python or we should have Pulp role require/expect httpd, and use our drop in directory to have the pulp role drop in chunks?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Just checking in on this when you have a moment. Happy to proceed based on your suggestion.

Copy link
Copy Markdown
Member

@evgeni evgeni Apr 28, 2026

Choose a reason for hiding this comment

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

Jinja has an include tag, so we could do something like:

# httpd/defaults/main.yml
httpd_enabled_pulp_snippets: []

# vhost.conf.j2:
{% for httpd_pulp_snippet in httpd_enabled_pulp_snippets %}
{% include httpd_pulp_snippet+'.j2' %}
{% endfor %}

and then feed httpd_enabled_pulp_snippets with the right list of enabled pulp plugins at the foremanctl level (and of course provide those snippets)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks! Implemented using httpd_enabled_pulp_snippets with includes as suggested.

Comment thread src/vars/base.yaml
httpd_client_ca_certificate: "{{ client_ca_certificate }}"
httpd_server_certificate: "{{ server_certificate }}"
httpd_server_key: "{{ server_key }}"
httpd_enabled_pulp_snippets: "{{ ['pypi'] if 'pulp_python' in pulp_enabled_plugins else [] }}"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think here we need to tie this into our features. For example, see

- content/ansible

And you can look down at line 32 in this file to see it being referenced.

@evgeni -- did we have an intent to define "meta" features within the features.yaml? e.g. content/* features

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.

3 participants