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.

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

Refs #37601 - Refresh/deploy CA cert on hosts #3193

Merged
merged 26 commits into from
Sep 6, 2024

Conversation

Lennonka
Copy link
Contributor

@Lennonka Lennonka commented Aug 6, 2024

This is for refreshing the self-signed Foreman CA cert on hosts.

Redmine issue: #37601

  • I am okay with my commits getting squashed when you merge this PR.
  • I am familiar with the contributing guidelines.

Please cherry-pick my commits into: N/A

  • Foreman 3.12
  • Foreman 3.11/Katello 4.13
  • Foreman 3.10/Katello 4.12
  • Foreman 3.9/Katello 4.11 (Satellite 6.15; orcharhino 6.8/6.9)
  • Foreman 3.8/Katello 4.10
  • Foreman 3.7/Katello 4.9 (Satellite 6.14)
  • Foreman 3.6/Katello 4.8
  • Foreman 3.5/Katello 4.7 (Satellite 6.13; orcharhino 6.6/6.7)
  • We do not accept PRs for Foreman older than 3.5.

@Lennonka Lennonka force-pushed the refreshing-ca-certs-on-hosts branch 4 times, most recently from c06e32f to 484a8ab Compare August 8, 2024 18:45
@Lennonka Lennonka changed the title Refs #37601 - Refresh/deploy SSL certs Refs #37601 - Refresh/deploy CA cert on hosts Aug 8, 2024
@Lennonka Lennonka marked this pull request as ready for review August 8, 2024 18:48
@Lennonka Lennonka force-pushed the refreshing-ca-certs-on-hosts branch from 484a8ab to 661e4c0 Compare August 8, 2024 18:55
@Lennonka Lennonka force-pushed the refreshing-ca-certs-on-hosts branch 2 times, most recently from ff20ab3 to 6b5f7e2 Compare August 8, 2024 19:10
@Lennonka Lennonka requested a review from ekohl August 12, 2024 21:11
@Lennonka Lennonka force-pushed the refreshing-ca-certs-on-hosts branch from 44184de to 2e6e896 Compare August 12, 2024 21:25
@Lennonka
Copy link
Contributor Author

Rebased.

Co-authored-by: Eric Helms <eric.d.helms@gmail.com>
@Lennonka
Copy link
Contributor Author

@ehelms Since you haven't commented on the Planning section, I'm assuming that it's okay.

+
[options="nowrap" subs="+quotes,verbatim,attributes"]
----
https://_{foreman-example-com}_/unattended/public/foreman_ca_refresh
Copy link
Member

Choose a reason for hiding this comment

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

If you go this route, you must mention that for isolated hosts this must be retrieved via a Smart Proxy (AKA Capsule) because they may not be able to access Foreman.

It would be better to implement a dedicated REX job template and that the user selects. Then the user doesn't need to input any fields and we can rely on REX to properly transfer the script. Even if the certificate expired (because SSH push mode doesn't need them) and when the host is isolated.

The same goes for the Script REX procedure.

Copy link
Member

Choose a reason for hiding this comment

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

It would be better to implement a dedicated REX job template and that the user selects. Then the user doesn't need to input any fields and we can rely on REX to properly transfer the script. Even if the certificate expired (because SSH push mode doesn't need them) and when the host is isolated.
The same goes for the Script REX procedure.

Created a tracker to properly track it: https://projects.theforeman.org/issues/37773

Copy link
Member

Choose a reason for hiding this comment

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

@ekohl with our dropping of API access in the reverse proxy, will isolated hosts have access to this endpoint? do we need to add this endpoint to our list of allowed endpoints?

Copy link
Member

Choose a reason for hiding this comment

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

@ekohl with our dropping of API access in the reverse proxy, will isolated hosts have access to this endpoint? do we need to add this endpoint to our list of allowed endpoints?

We already have the templates module which proxies the unattended templates (also for kickstarts etc). I think the current API should cover it, but not 100% sure. This needs to be verified.

Copy link
Member

Choose a reason for hiding this comment

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

@ekohl Almost. We are missing get "/:kind/:template" do variant to get it working

Copy link
Member

Choose a reason for hiding this comment

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

@pr-processor pr-processor bot added the Waiting on contributor Requires an action from the author label Aug 29, 2024
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Sep 2, 2024
@Lennonka
Copy link
Contributor Author

Lennonka commented Sep 3, 2024

@ShimShtein @ehelms Are we happy now?

@Lennonka
Copy link
Contributor Author

Lennonka commented Sep 4, 2024

Unless there are further comments within the next 24 hours, I suggest we merge it.

Copy link
Contributor

@maximiliankolb maximiliankolb left a comment

Choose a reason for hiding this comment

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

Thanks Lena, LGTM.

@Lennonka
Copy link
Contributor Author

Lennonka commented Sep 5, 2024

The fact that it's for self-signed CA certs only seemed important, so I've added it to the titles of the Planning module and the assembly.

@maximiliankolb Can you please give it one more go?

Copy link
Contributor

@maximiliankolb maximiliankolb left a comment

Choose a reason for hiding this comment

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

Yes, still LGTM.

@Lennonka Lennonka merged commit 69b0498 into theforeman:master Sep 6, 2024
9 checks passed
@Lennonka Lennonka deleted the refreshing-ca-certs-on-hosts branch September 6, 2024 09:03
Lennonka added a commit that referenced this pull request Sep 6, 2024
Co-authored-by: Maximilian Kolb <mail@maximilian-kolb.de>
Co-authored-by: Shimon Shtein <sshtein@redhat.com>
Co-authored-by: Eric Helms <eric.d.helms@gmail.com>
@Lennonka
Copy link
Contributor Author

Lennonka commented Sep 6, 2024

Cherry-picked:

@Lennonka Lennonka mentioned this pull request Oct 2, 2024
10 tasks
.Procedure
. In the {ProjectWebUI}, navigate to *Monitor* > *Jobs*.
. Click *Run Job*.
. From the *Job category* list, select `Commands`.
Copy link
Contributor

@apinnick apinnick Nov 4, 2024

Choose a reason for hiding this comment

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

@Lennonka I realize that this has already been merged. I found this PR when I was trying to figure out the RN for this feature. UI elements and options should be marked up as bold, not with backticks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
style review done No issues from docs style/grammar perspective
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants