-
Notifications
You must be signed in to change notification settings - Fork 637
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
Fix custom CA certificates for task/web/migration #1846
Conversation
Thank you for the nice work done on this PR @JoelKle . |
@rooftopcellist @fosterseth Could you have a look on this PR? Thx :) |
securityContext: | ||
runAsUser: 0 | ||
runAsGroup: 0 |
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.
@JoelKle This will break on Openshift. Can we template this in only if is_k8s=true?
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.
Did update-ca-trust extract
require more permissions than just update-ca-trust
? I am trying to understand why the securityContext bit was added.
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.
I removed the whole securityContext
block from this PR. Now the init-container behaves as before.
@rooftopcellist Is this fine for you?
TL;DR
Withouth the runAsUser: 0
parameter the init-container runs the update-ca-trust command as uid=1000(awx) gid=0(root)
. That means the files in the folder /etc/pki/ca-trust/extracted
are owned by awx:root
With the runAsUser: 0
parameter all files are owned by root:root
which is more accurate.
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.
The update-ca-trust extract
behaves the same as withouth the extract
. However with the extract
the command may print more warnings, ...
Check the manpage of update-ca-trust extract > "COMMANDS"
@rooftopcellist Are there any other blocker for you? |
@JoelKle thanks for following up, and sorry for the delay here. Could you please rebase to resolve the conflicts? We'll see if CI passes on the run kicked off by pushing the rebased branch. I just checked the CI failure, and it looks like it was flake, so once rebased I think we will be good. |
- added a new init container init-bundle-ca-trust - added volume ca-trust-extracted to the migration job - added volume ca-trust-extracted to the init container init-database - removed volume bundle-ca from all follow-up containers
@rooftopcellist Rebase done. Look like the CI run needs your approval? |
SUMMARY
This PR fixes the usage of custom CA certificates in the migration job and improves the existing implementation in the task & web deployment.
During the upgrade to awx 24.0.0, a new pod is created for migration. This pod doesnt contain the custom ca certificate and fails when using external postgres with verify enabled.
PR fixes: #1782
Follow up / Improves: #1800
ISSUE TYPE
ADDITIONAL INFORMATION
This PR is based on the great work of @YassineFadhlaoui in #1782 (comment) and @akkaba23 in #1800 (comment)
The following has changed:
Added a new init container
init-bundle-ca-trust
to thetask
+web
deployment and to themigration
jobupdate-ca-trust extract
ifbundle_ca_crt
is set.runAsUser: 0
Removed the
update-ca-trust
command from the init containerinit-receptor
because it will run once in the new init containerinit-bundle-ca-trust
Removed the mounting of the volume
{{ ansible_operator_meta.name }}-bundle-cacert
from containers that really do not need itAdded the whole
bundle_ca_crt
logic to themigration
jobI've successfully tested that change during my upgrade from awx-operator v2.12.1 to v2.15.0