-
Notifications
You must be signed in to change notification settings - Fork 112
Inject payload's system store with proxy CA when specified #172
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,10 +3,13 @@ package operator2 | |
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
| "io/ioutil" | ||
| "net/http" | ||
| "net/url" | ||
| "strings" | ||
|
|
||
| "k8s.io/klog" | ||
|
|
||
| corev1 "k8s.io/api/core/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/runtime" | ||
|
|
@@ -293,7 +296,7 @@ func (c *authOperator) discoverOpenIDURLs(issuer, key string, ca configv1.Config | |
|
|
||
| func (c *authOperator) transportForCARef(ca configv1.ConfigMapNameReference, key string) (http.RoundTripper, error) { | ||
| if len(ca.Name) == 0 { | ||
| return transportFor("", nil, nil, nil) | ||
| return transportFor("", trustedCABytes(), nil, nil) | ||
| } | ||
| cm, err := c.configMaps.ConfigMaps(userConfigNamespace).Get(ca.Name, metav1.GetOptions{}) | ||
| if err != nil { | ||
|
|
@@ -343,3 +346,12 @@ func encodeOrDie(obj runtime.Object) []byte { | |
| } | ||
| return bytes | ||
| } | ||
|
|
||
| func trustedCABytes() []byte { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are these additional to the system trust store? The file only includes some CAs, not all. Especially not possibly required intermediate ones. |
||
| caData, err := ioutil.ReadFile(operatorTrustedCAFile) | ||
| if err != nil { | ||
| klog.Infof("could not read %s, it won't be used in transport", operatorTrustedCAFile) | ||
| return nil | ||
| } | ||
| return caData | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,7 +62,8 @@ const ( | |
| kasServiceAndEndpointName = "kubernetes" | ||
| kasServiceFullName = kasServiceAndEndpointName + "." + corev1.NamespaceDefault + ".svc" | ||
|
|
||
| rootCAFile = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" | ||
| rootCAFile = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" | ||
| operatorTrustedCAFile = "/var/run/configmaps/trusted-ca-bundle/ca-bundle.crt" | ||
|
|
||
| systemConfigPath = "/var/config/system" | ||
| systemConfigPathConfigMaps = systemConfigPath + "/configmaps" | ||
|
|
@@ -109,6 +110,12 @@ const ( | |
| consoleConfigMapLocalName = systemConfigPrefix + consoleConfigMapSharedName | ||
| consoleConfigKey = consoleConfigMapSharedName + ".yaml" | ||
|
|
||
| // trustedCABundleName part of manifests, if changing this, need to change that, too | ||
| trustedCABundleName = systemConfigPrefix + "trusted-ca-bundle" | ||
| trustedCABundleKey = "ca-bundle.crt" | ||
| trustedCABundleMountDir = "/etc/pki/ca-trust/extracted/pem" | ||
| trustedCABundleMountFile = "tls-ca-bundle.pem" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a plain yaml text file would me much more readable than 35 constansts. |
||
|
|
||
| ocpBrandingSecretName = systemConfigPrefix + "ocp-branding-template" | ||
| ocpBrandingSecretMount = systemConfigPathSecrets + "/" + ocpBrandingSecretName | ||
| ocpBrandingLoginPath = ocpBrandingSecretMount + "/" + configv1.LoginTemplateKey | ||
|
|
@@ -525,7 +532,8 @@ func (c *authOperator) checkDeploymentReady(deployment *appsv1.Deployment, opera | |
| func (c *authOperator) checkRouteHealthy(route *routev1.Route, routerSecret *corev1.Secret, ingress *configv1.Ingress) (ready bool, msg, reason string, err error) { | ||
| caData := routerSecretToCA(route, routerSecret, ingress) | ||
|
|
||
| rt, err := transportFor("", caData, nil, nil) | ||
| // merge trustedCA data with router cert in case TLS intercept proxy is in place | ||
| rt, err := transportFor("", append(caData, trustedCABytes()...), nil, nil) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this reads the file on every request, not good.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we sure these bytes always append cleanly, i.e. that we have a trailing newline in caData? Looks fragile.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ic, we probably reread to get the latest version (which can change in the background). |
||
| if err != nil { | ||
| return false, "", "FailedTransport", fmt.Errorf("failed to build transport for route: %v", err) | ||
| } | ||
|
|
||
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.
won't CVO stomp this configmap after network operator added the CA ?
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.
same below of course
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.
@danehans ^^
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.
presumably not, because of the
create-onlyannotation...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.
nvmd. The create-only label will make sure it is not overwritten.