Skip to content

Auto-detect and normalize installer certificates#421

Open
ehelms wants to merge 3 commits into
theforeman:masterfrom
ehelms:installer-certificates-management
Open

Auto-detect and normalize installer certificates#421
ehelms wants to merge 3 commits into
theforeman:masterfrom
ehelms:installer-certificates-management

Conversation

@ehelms
Copy link
Copy Markdown
Member

@ehelms ehelms commented Mar 25, 2026

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

Migrating from a foreman-installer deployment to foremanctl requires converting
both the answer file parameters and the certificate layout. Previously this had
to be done manually. The migrate subcommand now handles this in a single step,
but the migrate playbook was running without elevated privileges, causing a
permission error when reading /etc/foreman-installer/ files.

What are the changes introduced in this pull request?

  • Add migrate_foreman_installer role that runs during foremanctl migrate to
    copy certificates from /root/ssl-build/ into /root/certificates/, persist
    the CA passphrase into parameters.yaml, and back up the original directory
  • Auto-detect custom server certificates by comparing the internal CA against
    the server CA; write certificates_source: custom_server to parameters.yaml
    when they differ so subsequent deploys do not overwrite the custom cert
  • Remove the installer certificate source — migrated certs use standard default
    source paths after normalization; mark certificate path parameters as IGNORE
    in answer file migration since the role handles cert files directly
  • Consolidate default_certificates.yml and custom_server_certificates.yml
    into a single certificates.yml (paths were identical after normalization)
  • Add a dedicated CI job that exercises the full migrate workflow: install
    foreman-installer, run foremanctl migrate, deploy, and test
  • Fix permission error in migrate playbook by adding become: true to the
    answer file migration play so the module can read files under
    /etc/foreman-installer/

How to test this pull request

Steps to reproduce:

  • Run the migration CI job and verify it passes end-to-end
  • On a host with an existing foreman-installer install, run foremanctl migrate
    and confirm a valid parameters.yaml is produced and certs are copied into place
  • Verify a host with a custom server cert gets certificates_source: custom_server
    written to parameters.yaml
  • Run foremanctl deploy after migration and confirm it succeeds without
    re-running certificate migration

Checklist

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

@ehelms ehelms force-pushed the installer-certificates-management branch 5 times, most recently from 252977b to 28bad69 Compare March 30, 2026 18:53
@ehelms ehelms force-pushed the installer-certificates-management branch from 28bad69 to da1cd29 Compare April 1, 2026 15:42
@evgeni
Copy link
Copy Markdown
Member

evgeni commented Apr 17, 2026

@ehelms could you please rebase this one

@ehelms ehelms force-pushed the installer-certificates-management branch 2 times, most recently from e96db39 to 5d8e7a7 Compare April 17, 2026 20:09

- name: 'Issue other certificates'
- name: Issue host certificates
ansible.builtin.include_tasks: issue.yml
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.

This fails for "installer" certs, as the CA password is not properly loaded.

OTOH, I don't see why it would re-generate the installer issued certs to begin with.

Comment thread src/roles/certificates/tasks/setup.yml Outdated
ansible.builtin.file:
path: "{{ certificates_ca_directory_keys }}"
state: directory
mode: '0755'
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 keys really should not be world readable

@evgeni
Copy link
Copy Markdown
Member

evgeni commented Apr 20, 2026

certs-check is right, /root/certificates/certs/quadlet.example.com.crt is signed by Foreman Self-signed CA, not by Custom Test CA - but why?!

"TASK [certificates : Sign server certificate] **********************************" did re-sign the cert, it really should not for "custom" certs, those can't be managed by us.

@evgeni evgeni force-pushed the installer-certificates-management branch 2 times, most recently from 552ddf4 to 5d317ef Compare April 20, 2026 15:06
Comment thread src/roles/certificates/tasks/main.yml Outdated
ansible.builtin.include_tasks: ca.yml
when: certificates_ca
when:
- certificates_ca
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.

At this point, will that ever be false?

Comment thread src/roles/certificates/tasks/main.yml Outdated
when: certificates_ca
when:
- certificates_ca
- not certificates_installer_ca.stat.exists
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.

normalize.yml deletes /root/ssl-build/katello-default-ca.crt after execution, so on the next run of the role not certificates_installer_ca.stat.exists will be true and ca.yml will run -- is that intentional?

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.

Comment thread src/playbooks/deploy/deploy.yaml Outdated
- "../../vars/defaults.yml"
- "../../vars/flavors/{{ flavor }}.yml"
- "../../vars/{{ certificate_source }}_certificates.yml"
- "../../vars/default_certificates.yml"
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.

arguably, this file now should be just certificates.yml, right?

@evgeni evgeni force-pushed the installer-certificates-management branch 2 times, most recently from a7444fc to dfde810 Compare April 21, 2026 07:53
@evgeni
Copy link
Copy Markdown
Member

evgeni commented Apr 21, 2026

The more I think about it, the more I think normalize should be part of the "one time migration from old installer" (as started in #446), not a thing that "continuously" runs as I think copying files is not sufficient for that step. We need to at least also make foremanctl aware of the passphrase of the CA (I hacked that up with slurp now, but I hate it) and ideally also import the old CA properties so that it does not get regenerated on follow up runs.

and the custom cert setup is a nice addition, but is also valuable outside the normalize flow, so IMHO this PR should be two?

@ehelms ehelms mentioned this pull request Apr 21, 2026
2 tasks
@evgeni evgeni mentioned this pull request Apr 21, 2026
@ehelms
Copy link
Copy Markdown
Member Author

ehelms commented Apr 21, 2026

I agree I can now pull out the custom certificates to their own PR. And that, installer certificates via migration handling makes more sense as a one time operation.

Do you generally agree on the direction of installer certs support? That is should be "converted" so foremanctl manages it like our default ones?

@ehelms ehelms marked this pull request as draft April 21, 2026 18:56
@ehelms
Copy link
Copy Markdown
Member Author

ehelms commented Apr 21, 2026

Custom certificate handling now exists at: #462
This will need to be incorporated into the migrate command once that's available. Setting this to draft for now as it will require re-work once those two conditions are met.

@evgeni
Copy link
Copy Markdown
Member

evgeni commented Apr 22, 2026

Do you generally agree on the direction of installer certs support? That is should be "converted" so foremanctl manages it like our default ones?

Yes, as the setup (well, at the very least the CA) need to be fully usable in foremanctl to deploy new proxies etc.

@evgeni evgeni mentioned this pull request Apr 22, 2026
2 tasks
@ehelms ehelms force-pushed the installer-certificates-management branch from dfde810 to 93f61e8 Compare April 27, 2026 13:14
@ehelms ehelms force-pushed the installer-certificates-management branch from 93f61e8 to 0807995 Compare May 5, 2026 20:21
@ehelms ehelms marked this pull request as ready for review May 5, 2026 20:22
@ehelms ehelms force-pushed the installer-certificates-management branch from 0807995 to b14f1a8 Compare May 7, 2026 01:14
@ehelms ehelms force-pushed the installer-certificates-management branch from b14f1a8 to 608ae7b Compare May 14, 2026 19:37
@ehelms ehelms force-pushed the installer-certificates-management branch 3 times, most recently from 6846c40 to 5d217fd Compare May 29, 2026 12:44
ehelms and others added 2 commits May 29, 2026 14:49
The default and custom_server certificate vars files defined identical
paths since custom certificates are normalized into the same directory
structure during deployment. Remove the vars file indirection and use a
single certificates.yml for all certificate sources.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a dedicated CI job that exercises the foremanctl migrate workflow:
install foreman-installer to create a realistic environment with an
answer file and certificates, run foremanctl migrate to convert the
answer file to foremanctl parameters, then deploy and test the result.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ehelms ehelms force-pushed the installer-certificates-management branch from 5d217fd to f4b868b Compare May 29, 2026 18:50
Move foreman-installer certificate normalization into the migrate
subcommand so it runs once during migration rather than on every deploy.
The migrate_certificates role copies certs from /root/ssl-build/ into
/root/certificates/, persists the CA passphrase into parameters.yaml,
and backs up the original directory.

Detect custom server certificates by comparing the internal CA with the
server CA. When they differ, persist certificates_source: custom_server
to prevent subsequent deploys from overwriting the custom server cert.

Remove the installer certificate source since migrated certs use the
default source paths after normalization. Mark certificate path
parameters as IGNORE in the answer file migration since the role handles
cert files directly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ehelms ehelms force-pushed the installer-certificates-management branch from f4b868b to c756108 Compare May 29, 2026 19:49
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.

2 participants