-
Notifications
You must be signed in to change notification settings - Fork 254
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
⚠️ Store htpasswd files in Secrets instead of ConfigMaps #1241
Conversation
The htpasswd files for Ironic and Inspector contains clear text usernames and hashed passwords so it is better to store them in Secrets. Depending on how exactly Ironic is deployed this could be a breaking change that requires manual action from the user. I have tested this with the [deploy.sh](https://github.com/metal3-io/baremetal-operator/blob/main/tools/deploy.sh) script and confirmed that it is working. Re-deploying Ironic, with the updated kustomization using the script, automatically creates the new Secrets and configures Ironic and Inspector to use them instead of the ConfigMaps. Note that the ConfigMaps are **not** automatically removed. Ideally, the user should remove the ConfigMaps and change the credentials. The ConfigMaps in question are named `baremetal-operator-ironic-htpasswd-<random-hash>` and `baremetal-operator-ironic-inspector-htpasswd-<random-hash>` and are located in the `baremetal-operator-system` Namespace by default. Note that if the credentials are changed, they must also be updated for BMO. This can be done in the same way by re-deploying using the script.
/test-centos-e2e-integration-main |
/lgtm |
/hold |
/hold cancel I have added manual steps for live environments in the description. It is hard to make something fool proof without making too many assumptions, so it is unfortunately quite general. The users will need to adapt to their specific environments. |
Thanks @lentzi90, the steps are detailed enough and quite OK. Anyone installing BMO should be able to follow those. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Prow/Tide test, please disregard |
/test unit |
@kashifest @zaneb @dtantsur please take a look when you have the time. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kashifest, tuminoid The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold |
Rerunning the tests since the previous runs were 2 weeks old. Unhold it when the tests pass. |
/hold cancel |
What this PR does / why we need it:
The htpasswd files for Ironic and Inspector contains clear text usernames and hashed passwords so it is better to store them in Secrets.
Depending on how exactly Ironic is deployed this could be a breaking change that requires manual action from the user.
I have tested this with the deploy.sh script and confirmed that it is working. Re-deploying Ironic, with the updated kustomization using the script, automatically creates the new Secrets and configures Ironic and Inspector to use them instead of the ConfigMaps.
Note that the ConfigMaps are not automatically removed. Ideally, the user should remove the ConfigMaps and change the credentials. The ConfigMaps in question are named
baremetal-operator-ironic-htpasswd-<random-hash>
andbaremetal-operator-ironic-inspector-htpasswd-<random-hash>
and are located in thebaremetal-operator-system
Namespace by default.Note that if the credentials are changed, they must also be updated for BMO. This can be done in the same way by re-deploying using the script.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #
Steps for changing ConfigMap to Secret
Before you start, confirm if the environment is affected or not.
This can be done with the following commands:
Affected environments would have
configMapRef
s for the htpasswd files in the output.If it instead shows
secretRef
s for the htpasswd files it is not affected.Start by listing the current ConfigMaps and Secrets for reference, so it is easy to check which are new and which are old later.
Generate new credentials and configuration files, for example like this:
Create new secrets from htpasswd files, authentication configuration files and credentials for BareMetalOperator.
The exact names of these do not matter and the ones provided here are just examples.
Edit the Ironic Deployment to make it use the Secrets instead of the ConfigMaps.
Unfortunately, it is not easy to generate a fool proof patch (for
kubectl patch
) for this, since theenvFrom
field is an array.It is only possible to replace the whole array or do index based replacements, which cannot really be used without knowing the exact details of the specific environment.
For this reason we only show the steps using
kubectl edit
instead of providing a patch that would likely not work for everyone.Edit the Deployment with this command:
kubectl -n baremetal-operator-system edit deployment baremetal-operator-ironic
.Note that the names of the ConfigMaps may differ depending on version and how it was deployed (e.g. the
baremetal-operator-
prefix may not be included in v0.1.2).The order of the containers and other fields can also differ.
The necessary changes are described here:
Now patch the BareMetalOperator Deployment to make use of the new credentials.
Check that the new Pods become
Running
and then delete the old ConfigMaps and Secrets (optional).Confirm the fix by again checking the output of these commands:
They should now show only
secretRef
s for htpasswd files.Clean up temporary files: