Wire trusted-ca-bundle via /etc/pki#187
Wire trusted-ca-bundle via /etc/pki#187openshift-merge-robot merged 4 commits intoopenshift:masterfrom
Conversation
openshift-ci-robot
added
the
size/XXL
openshift-ci-robot
added
the
approved
| image: quay.io/openshift/origin-cluster-authentication-operator:v4.0 | ||
| imagePullPolicy: IfNotPresent | ||
| command: ["authentication-operator", "operator"] | ||
| command: ["/bin/bash", "ec"] |
| - "--config=/var/run/configmaps/config/operator-config.yaml" | ||
| - "-v=2" | ||
| - | | ||
| if [ -s /var/run/configmaps/trusted-ca-bundle/tls-ca-bundle.pem ]; then |
There was a problem hiding this comment.
I wish the operator does this during startup and we don't have to hack ways in bash.
There was a problem hiding this comment.
it cannot. Then it is too late. And this does not belong into a binary. It is system plumbing. It is just right here.
There was a problem hiding this comment.
if tls-ca-bundle.pem is empty, will that brick the operator?
There was a problem hiding this comment.
it is non-existent if not part of the configmap.
There was a problem hiding this comment.
The network operator validates it.
There was a problem hiding this comment.
Note that appending empty PEM files and such is fine - the parser only cares about the data in the cert blocks (comments, empty lines, etc are fine).
There was a problem hiding this comment.
@stlaz noticed that we must not copy empty files if I have understood him correctly.
sttts
force-pushed
the
sttts-file-observer
branch
from
395f9ee to
bd1d601
Compare
sttts
force-pushed
the
sttts-file-observer
branch
from
bd1d601 to
76a1c69
Compare
enj
left a comment
enj
left a comment
There was a problem hiding this comment.
nit. Why is the glide bump so large?
| trustedCABundleName = systemConfigPrefix + "trusted-ca-bundle" | ||
| trustedCABundleKey = "ca-bundle.crt" | ||
| trustedCABundleMountDir = "/etc/pki/ca-trust/extracted/pem" | ||
| trustedCABundleMountDir = systemTrustStoreDirPath |
There was a problem hiding this comment.
Just set to "/etc/pki/ca-trust/extracted/pem" and drop systemTrustStoreDirPath
There was a problem hiding this comment.
this is a revert. Would avoid to change anything not neccesary.
There was a problem hiding this comment.
... but its already set to trustedCABundleMountDir = "/etc/pki/ca-trust/extracted/pem" in master? AFAICT you did not do an actual git revert so I do not understand the comment. If I am missing something, just have someone tag as-is.
There was a problem hiding this comment.
I reverted the changes in that file, yes.
There was a problem hiding this comment.
Have removed the constant now.
|
After fixing the comment thing and making sure the bump size is expected, feel free to have anyone tag. |
because glide. |
Not sure I understand but do not feel strongly to block anything. /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enj, sttts The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
5 participants
Follow-up of #172 to make it more standard.