Use stringData when creating root password secret #920
Conversation
When using data `NodeRootPassword` value should be base64 encoded. If using a normal string for $EDPM_ROOT_PASSWORD secret creation fails. Signed-off-by: rabi <[email protected]>
| @@ -24,7 +24,7 @@ metadata: | |||
| name: baremetalset-password-secret | |||
| namespace: ${NAMESPACE} | |||
| type: Opaque | |||
| data: | |||
| stringData: | |||
There was a problem hiding this comment.
Ok I see everything starts with BM_ROOT_PASSWORD passed as input, that is propagated here.
I don't have big concerns to prefer data over stringData (as we're delegating k8s to encode the secret for us), so I'm ok with the patch if it ease the development workflow.
There was a problem hiding this comment.
Does our CI automation currently use this? I just wonder if they're already passing base64-encoded values through the env var (BM_ROOT_PASSWORD), such that if it was injected into stringData, k8s would try to base64-encode something that was already encoded by the user.
There was a problem hiding this comment.
Good question, in that case we need to update how CI pass this info as well. @rabi I'm wondering if we want to clarify this aspect before merging.
There was a problem hiding this comment.
I had checked it before proposing. I didn't find it being used in any of the openstack-k8s-operator repos. Unless there is some other places that we should look for.
There was a problem hiding this comment.
ack, feel free to remove the hold when you're comfortable merging the patch then.
There was a problem hiding this comment.
@abays Do you see this breaking something? If at all something is broken it can be fixed when reported I guess.
There was a problem hiding this comment.
I really don't know if something in CI is using this already or not. It just seems possible to me that some existing job automation might be setting BM_ROOT_PASSWORD as an encoded string already. Maybe we should ask someone on the CI side to comment? @raukadah, maybe you know something about this?
There was a problem hiding this comment.
I don't see it being used in upstream/downstream CI.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fmount, rabi 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 |
openshift-merge-bot
bot
merged commit 60827c3
into
openstack-k8s-operators:main
When using
data,NodeRootPasswordvalue should be base64 encoded. If using a normal string for $EDPM_ROOT_PASSWORD secret creation fails.