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

New script: Local Certificate Authority based upon Smallstep's step-ca #1655

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

fwiegerinck
Copy link

✍️ Description

New script to create an Alpine-based container running a local Certificate Authority based upon Smallstep's OS "step-ca" (url: https://smallstep.com/docs/step-ca/).

This release provides - next to the default config GUI - a GUI to configure:

  • name of the CA
  • DNS hostnames to access the CA
  • X509 policies for the allowed DNS and IP addresses
  • optionally enable ACME service (at port 443)

  • Related Issue: #
  • Related PR: #
  • Related Discussion: #

✅ Prerequisites

The following steps must be completed for the pull request to be considered:

  • Self-review performed (I have reviewed my code to ensure it follows established patterns and conventions.)
  • Testing performed (I have thoroughly tested my changes and verified expected functionality.)

🛠️ Type of Change

Please check the relevant options:

  • [] Bug fix (non-breaking change that resolves an issue)
  • [] New feature (non-breaking change that adds functionality)
  • [] Breaking change (fix or feature that would cause existing functionality to change unexpectedly)
  • New script (a fully functional and thoroughly tested script or set of scripts)

📋 Additional Information (optional)

Provide any extra context or screenshots about the feature or fix here.

@fwiegerinck fwiegerinck requested a review from a team as a code owner January 21, 2025 20:38
@github-actions github-actions bot added new script A change that adds a new script website A change to the website labels Jan 21, 2025
Copy link
Member

@michelroegl-brunner michelroegl-brunner left a comment

Choose a reason for hiding this comment

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

This script breaks a few standards. First, why have you divided the installation in two parts?
Then remove the comments.
and you can remove the part with motd file. This is no longer used this way.

@github-actions github-actions bot added maintenance Code maintenance or general upkeep of the project high risk A change that can affect many scripts labels Jan 21, 2025
@fwiegerinck
Copy link
Author

fwiegerinck commented Jan 21, 2025

@michelroegl-brunner Thanks for the feedback.

First, why have you divided the installation in two parts?

Assuming you refer to alpine-step-ca-install.sh, during troubleshooting I found it more stable /easier to complete the steps of the OS first (incl customizations) and amend with the installation and configuration of step-ca. in case the configuration fails, at least the OS is accessible.

Then remove the comments

Done

and you can remove the part with motd file. This is no longer used this way.

I switched to .profile avoiding motd, while the user can still quickly access the fingerprint to onboard new clients to the ACME service.

@michelroegl-brunner
Copy link
Member

Please have a look at our Guides. Please move all install parts int the *-install.sh file. Also, are all these inputs really necessary? Cant you use some sort of default values wich can get changed later in a config file. Maybe write a short guide for this? Also you are doing many things not the way our standard is, have a look at the guides.
The scripts should be easy to install and with as little input as possible. We can not merge it in this state.

@michelroegl-brunner michelroegl-brunner removed maintenance Code maintenance or general upkeep of the project high risk A change that can affect many scripts labels Jan 24, 2025
@michelroegl-brunner
Copy link
Member

Any progress?

@fwiegerinck
Copy link
Author

Please have a look at our Guides. Please move all install parts int the *-install.sh file. Also, are all these inputs really necessary? Cant you use some sort of default values wich can get changed later in a config file. Maybe write a short guide for this? Also you are doing many things not the way our standard is, have a look at the guides. The scripts should be easy to install and with as little input as possible. We can not merge it in this state.

Apologies having troubles to understand the install parts outside of the -install.sh you refer to. As far as I'm aware I separated the dialog for input with the customer to be in the ct/.sh and the actual install and configuration in the install/-install.sh. The only 'install'/update element in the ct/.sh is related to the update_script() which follows the example of the contribution guide.

I will have another check on the contribution guide. So far I guess my environment variable handling and usage of $STD could be improved.

However the main question for me would be around your feedback whether the inputs are really necessary. Typically with a certificate authority you don't want to regenerate your root certificate for obvious reasons. Hence this step of the configuration process should be done only once. I included the input dialog to have an one-off flow with the user to deliver a turnkey ready-to-go local certificate authority. Moving the dialog/configuration steps into separate scripts blocks the ability to start the service at the end of the script as it needs to be initialize first. Using defaults forces the user to regenerate the root certificate (or at least for the naming of the CA part).
Respecting the desire to have an easy to install and little input as possible, I was wondering myself how much value the user would have from a ProxmoxVE script/image that would install of 2 packages, and a few scripts but doesn't bring a (basic) running CA once completed. Perhaps scripts outside of the ProxmoxVE eco-space would be as useful and allows the user to use the AlpineOS base script and than run the step-ca install-&-config script?

Looking forward to your thoughts whether it makes sense to pursue a (integrated) ProxmoxVE script? or would be better to split allowing the user to use the existing AlpineOS script use, and install/configurre the CA with another script?

(should we close this PR and continue the chat elsewhere?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new script A change that adds a new script website A change to the website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants