From 46c802bba4cacb23039bf5c057dd9467ace4b1e9 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Thu, 30 Jan 2025 14:51:51 -0500 Subject: [PATCH 1/3] UPSTREAM: : use projected volume for CAs to avoid subPath limitations Signed-off-by: Joe Lanford --- .../patches/manager_deployment_certs.yaml | 14 ++----- ...operator-controller-controller-manager.yml | 42 +++++++++---------- 2 files changed, 25 insertions(+), 31 deletions(-) diff --git a/openshift/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_certs.yaml b/openshift/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_certs.yaml index f439c1099..9166ed2d2 100644 --- a/openshift/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_certs.yaml +++ b/openshift/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_certs.yaml @@ -1,15 +1,9 @@ - op: add path: /spec/template/spec/volumes/- - value: {"name":"trusted-ca-bundle", "configMap":{"optional":false,"name":"trusted-ca-bundle", "items":[{"key":"ca-bundle.crt","path":"ca-bundle.crt"}]}} -- op: add - path: /spec/template/spec/volumes/- - value: {"name":"service-ca", "configMap":{"optional":false,"name":"openshift-service-ca.crt", "items":[{"key":"service-ca.crt","path":"service-ca.crt"}]}} -- op: add - path: /spec/template/spec/containers/0/volumeMounts/- - value: {"name":"trusted-ca-bundle", "mountPath":"/var/trusted-cas/ca-bundle.crt", "subPath":"ca-bundle.crt" } + value: {"name":"ca-certs", "projected": {"sources":[{"configMap":{"optional":false,"name":"trusted-ca-bundle", "items":[{"key":"ca-bundle.crt","path":"ca-bundle.crt"}]}},{"configMap":{"optional":false,"name":"openshift-service-ca.crt", "items":[{"key":"service-ca.crt","path":"service-ca.crt"}]}}]}} - op: add path: /spec/template/spec/containers/0/volumeMounts/- - value: {"name":"service-ca", "mountPath":"/var/trusted-cas/service-ca.crt", "subPath":"service-ca.crt" } + value: {"name":"ca-certs", "mountPath":"/var/ca-certs", "readOnly": true} - op: add - path: /spec/template/spec/containers/0/args/- - value: "--ca-certs-dir=/var/trusted-cas" + path: /spec/template/spec/containers/0/env + value: [{"name":"SSL_CERT_DIR", "value":"/var/ca-certs"}] diff --git a/openshift/manifests/20-deployment-openshift-operator-controller-operator-controller-controller-manager.yml b/openshift/manifests/20-deployment-openshift-operator-controller-operator-controller-controller-manager.yml index 22eb87e9e..9300276c5 100644 --- a/openshift/manifests/20-deployment-openshift-operator-controller-operator-controller-controller-manager.yml +++ b/openshift/manifests/20-deployment-openshift-operator-controller-operator-controller-controller-manager.yml @@ -43,11 +43,13 @@ spec: - --health-probe-bind-address=:8081 - --metrics-bind-address=127.0.0.1:8080 - --leader-elect - - --ca-certs-dir=/var/trusted-cas - --v=${LOG_VERBOSITY} - --global-pull-secret=openshift-config/pull-secret command: - /manager + env: + - name: SSL_CERT_DIR + value: /var/ca-certs image: ${OPERATOR_CONTROLLER_IMAGE} imagePullPolicy: IfNotPresent livenessProbe: @@ -76,12 +78,9 @@ spec: volumeMounts: - mountPath: /var/cache name: cache - - mountPath: /var/trusted-cas/ca-bundle.crt - name: trusted-ca-bundle - subPath: ca-bundle.crt - - mountPath: /var/trusted-cas/service-ca.crt - name: service-ca - subPath: service-ca.crt + - mountPath: /var/ca-certs + name: ca-certs + readOnly: true - mountPath: /etc/containers name: etc-containers readOnly: true @@ -131,20 +130,21 @@ spec: volumes: - emptyDir: {} name: cache - - configMap: - items: - - key: ca-bundle.crt - path: ca-bundle.crt - name: operator-controller-trusted-ca-bundle - optional: false - name: trusted-ca-bundle - - configMap: - items: - - key: service-ca.crt - path: service-ca.crt - name: openshift-service-ca.crt - optional: false - name: service-ca + - name: ca-certs + projected: + sources: + - configMap: + items: + - key: ca-bundle.crt + path: ca-bundle.crt + name: operator-controller-trusted-ca-bundle + optional: false + - configMap: + items: + - key: service-ca.crt + path: service-ca.crt + name: openshift-service-ca.crt + optional: false - hostPath: path: /etc/containers type: Directory From 40d48b346d442a6d856c84e9af4714d30ca4088f Mon Sep 17 00:00:00 2001 From: Todd Short Date: Thu, 30 Jan 2025 16:23:59 -0500 Subject: [PATCH 2/3] UPSTREAM: : Add support for SSL env vars to cert pool watcher (#1672) The SystemRoot store looks at the SSL_CERT_DIR and SSL_CERT_FILE environment variables for certificate locations. Because these variables are under control of the user, we should assume that the user wants to control the contents of the SystemRoot, and subsequently that those contents could change (as compared to certs located in the default /etc/pki location). Thus, we should watch those locations if they exist. Signed-off-by: Todd Short --- internal/httputil/certpoolwatcher.go | 24 +++++++++++++++++++++-- internal/httputil/certpoolwatcher_test.go | 4 ++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/internal/httputil/certpoolwatcher.go b/internal/httputil/certpoolwatcher.go index 2a250d069..0cce70312 100644 --- a/internal/httputil/certpoolwatcher.go +++ b/internal/httputil/certpoolwatcher.go @@ -4,6 +4,8 @@ import ( "crypto/x509" "fmt" "os" + "slices" + "strings" "sync" "time" @@ -44,8 +46,26 @@ func NewCertPoolWatcher(caDir string, log logr.Logger) (*CertPoolWatcher, error) if err != nil { return nil, err } - if err = watcher.Add(caDir); err != nil { - return nil, err + + // If the SSL_CERT_DIR or SSL_CERT_FILE environment variables are + // specified, this means that we have some control over the system root + // location, thus they may change, thus we should watch those locations. + watchPaths := strings.Split(os.Getenv("SSL_CERT_DIR"), ":") + watchPaths = append(watchPaths, caDir, os.Getenv("SSL_CERT_FILE")) + watchPaths = slices.DeleteFunc(watchPaths, func(p string) bool { + if p == "" { + return true + } + if _, err := os.Stat(p); err != nil { + return true + } + return false + }) + + for _, p := range watchPaths { + if err := watcher.Add(p); err != nil { + return nil, err + } } cpw := &CertPoolWatcher{ diff --git a/internal/httputil/certpoolwatcher_test.go b/internal/httputil/certpoolwatcher_test.go index bfebebd28..2ea3f862a 100644 --- a/internal/httputil/certpoolwatcher_test.go +++ b/internal/httputil/certpoolwatcher_test.go @@ -72,6 +72,10 @@ func TestCertPoolWatcher(t *testing.T) { t.Logf("Create cert file at %q\n", certName) createCert(t, certName) + // Update environment variables for the watcher - some of these should not exist + os.Setenv("SSL_CERT_DIR", tmpDir+":/tmp/does-not-exist.dir") + os.Setenv("SSL_CERT_FILE", "/tmp/does-not-exist.file") + // Create the cert pool watcher cpw, err := httputil.NewCertPoolWatcher(tmpDir, log.FromContext(context.Background())) require.NoError(t, err) From edc8033ef824a4f1905026e65a1fc00567c3f8ea Mon Sep 17 00:00:00 2001 From: Todd Short Date: Thu, 30 Jan 2025 16:55:40 -0500 Subject: [PATCH 3/3] UPSTREAM: : Separate CA configuration for pulls vs catalogd services (#1673) Rename the flags that provide CAs to image pulling to indicate the use. Keep the old flag around (for backward compatibility), but prefer the new flag(s). Signed-off-by: Todd Short --- cmd/manager/main.go | 12 +++++++----- .../tls/patches/manager_deployment_cert.yaml | 5 ++++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index c15db73f4..d81437efb 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -95,12 +95,14 @@ func main() { cachePath string operatorControllerVersion bool systemNamespace string - caCertDir string + catalogdCasDir string + pullCasDir string globalPullSecret string ) flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") - flag.StringVar(&caCertDir, "ca-certs-dir", "", "The directory of TLS certificate to use for verifying HTTPS connections to the Catalogd and docker-registry web servers.") + flag.StringVar(&catalogdCasDir, "catalogd-cas-dir", "", "The directory of TLS certificate authorities to use for verifying HTTPS connections to the Catalogd web service.") + flag.StringVar(&pullCasDir, "pull-cas-dir", "", "The directory of TLS certificate authorities to use for verifying HTTPS connections to image registries.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") @@ -221,7 +223,7 @@ func main() { os.Exit(1) } - certPoolWatcher, err := httputil.NewCertPoolWatcher(caCertDir, ctrl.Log.WithName("cert-pool")) + certPoolWatcher, err := httputil.NewCertPoolWatcher(catalogdCasDir, ctrl.Log.WithName("cert-pool")) if err != nil { setupLog.Error(err, "unable to create CA certificate pool") os.Exit(1) @@ -231,8 +233,8 @@ func main() { BaseCachePath: filepath.Join(cachePath, "unpack"), SourceContextFunc: func(logger logr.Logger) (*types.SystemContext, error) { srcContext := &types.SystemContext{ - DockerCertPath: caCertDir, - OCICertPath: caCertDir, + DockerCertPath: pullCasDir, + OCICertPath: pullCasDir, } if _, err := os.Stat(authFilePath); err == nil && globalPullSecretKey != nil { logger.Info("using available authentication information for pulling image") diff --git a/config/components/tls/patches/manager_deployment_cert.yaml b/config/components/tls/patches/manager_deployment_cert.yaml index 747979321..9eb0823c4 100644 --- a/config/components/tls/patches/manager_deployment_cert.yaml +++ b/config/components/tls/patches/manager_deployment_cert.yaml @@ -6,4 +6,7 @@ value: {"name":"olmv1-certificate", "readOnly": true, "mountPath":"/var/certs/"} - op: add path: /spec/template/spec/containers/0/args/- - value: "--ca-certs-dir=/var/certs" + value: "--catalogd-cas-dir=/var/certs" +- op: add + path: /spec/template/spec/containers/0/args/- + value: "--pull-cas-dir=/var/certs"