From 4a40570e55fae2e143d268c0ec5d1a0a05d6b364 Mon Sep 17 00:00:00 2001 From: Fabio Bertinatto Date: Thu, 7 Jan 2021 17:34:06 -0300 Subject: [PATCH 1/2] csi: add configobserver to watch proxy info. Cloud CSI drivers perform all sorts of operations using the cloud API. However, that's not possible in network-restricted clusters, where requests need to go through a given proxy, unless they target the API server. This patch adds a CSI Config Observer controller to observe the proxy resource and record the information in the operator CR. It also adds a hook function to inject the observed proxy config into the deployment's containers. --- .../csi_config_observer_controller.go | 81 +++++++++++++++++++ .../csicontrollerset/csi_controller_set.go | 17 ++++ ...si_driver_controller_service_controller.go | 11 +-- ...iver_controller_service_controller_test.go | 2 +- .../helpers.go | 26 ++++++ .../csi_driver_node_service_controller.go | 4 +- ...csi_driver_node_service_controller_test.go | 2 +- .../csidrivernodeservicecontroller/helpers.go | 29 +++++++ pkg/operator/v1helpers/helpers.go | 37 +++++++++ 9 files changed, 200 insertions(+), 9 deletions(-) create mode 100644 pkg/operator/csi/csiconfigobservercontroller/csi_config_observer_controller.go create mode 100644 pkg/operator/csi/csidrivercontrollerservicecontroller/helpers.go create mode 100644 pkg/operator/csi/csidrivernodeservicecontroller/helpers.go diff --git a/pkg/operator/csi/csiconfigobservercontroller/csi_config_observer_controller.go b/pkg/operator/csi/csiconfigobservercontroller/csi_config_observer_controller.go new file mode 100644 index 0000000000..634db32f86 --- /dev/null +++ b/pkg/operator/csi/csiconfigobservercontroller/csi_config_observer_controller.go @@ -0,0 +1,81 @@ +package csiconfigobservercontroller + +import ( + "strings" + + "k8s.io/client-go/tools/cache" + + configinformers "github.com/openshift/client-go/config/informers/externalversions" + configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" + + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/configobserver" + "github.com/openshift/library-go/pkg/operator/configobserver/proxy" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/resourcesynccontroller" + "github.com/openshift/library-go/pkg/operator/v1helpers" +) + +// ProxyConfigPath returns the path for the observed proxy config. This is a +// function to avoid exposing a slice that could potentially be appended. +func ProxyConfigPath() []string { + return []string{"targetcsiconfig", "proxy"} +} + +// Listers implement the configobserver.Listers interface. +type Listers struct { + ProxyLister_ configlistersv1.ProxyLister + + ResourceSync resourcesynccontroller.ResourceSyncer + PreRunCachesSynced []cache.InformerSynced +} + +func (l Listers) ProxyLister() configlistersv1.ProxyLister { + return l.ProxyLister_ +} + +func (l Listers) ResourceSyncer() resourcesynccontroller.ResourceSyncer { + return l.ResourceSync +} + +func (l Listers) PreRunHasSynced() []cache.InformerSynced { + return l.PreRunCachesSynced +} + +// CISConfigObserverController watches information that's relevant to CSI driver operators. +// For now it only observes proxy information, (through the proxy.config.openshift.io/cluster +// object), but more will be added. +type CSIConfigObserverController struct { + factory.Controller +} + +// NewCSIConfigObserverController returns a new CSIConfigObserverController. +func NewCSIConfigObserverController( + name string, + operatorClient v1helpers.OperatorClient, + configinformers configinformers.SharedInformerFactory, + eventRecorder events.Recorder, +) *CSIConfigObserverController { + informers := []factory.Informer{ + operatorClient.Informer(), + configinformers.Config().V1().Proxies().Informer(), + } + + c := &CSIConfigObserverController{ + Controller: configobserver.NewConfigObserver( + operatorClient, + eventRecorder.WithComponentSuffix("csi-config-observer-controller-"+strings.ToLower(name)), + Listers{ + ProxyLister_: configinformers.Config().V1().Proxies().Lister(), + PreRunCachesSynced: append([]cache.InformerSynced{}, + operatorClient.Informer().HasSynced, + configinformers.Config().V1().Proxies().Informer().HasSynced, + ), + }, + informers, + proxy.NewProxyObserveFunc(ProxyConfigPath()), + ), + } + + return c +} diff --git a/pkg/operator/csi/csicontrollerset/csi_controller_set.go b/pkg/operator/csi/csicontrollerset/csi_controller_set.go index 599c4f231d..03ed8190c8 100644 --- a/pkg/operator/csi/csicontrollerset/csi_controller_set.go +++ b/pkg/operator/csi/csicontrollerset/csi_controller_set.go @@ -9,6 +9,7 @@ import ( "k8s.io/client-go/kubernetes" configinformers "github.com/openshift/client-go/config/informers/externalversions" + "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/loglevel" @@ -18,6 +19,7 @@ import ( "github.com/openshift/library-go/pkg/operator/v1helpers" "github.com/openshift/library-go/pkg/operator/csi/credentialsrequestcontroller" + "github.com/openshift/library-go/pkg/operator/csi/csiconfigobservercontroller" "github.com/openshift/library-go/pkg/operator/csi/csidrivercontrollerservicecontroller" "github.com/openshift/library-go/pkg/operator/csi/csidrivernodeservicecontroller" ) @@ -28,6 +30,7 @@ type CSIControllerSet struct { managementStateController factory.Controller staticResourcesController factory.Controller credentialsRequestController factory.Controller + csiConfigObserverController factory.Controller csiDriverControllerServiceController factory.Controller csiDriverNodeServiceController factory.Controller @@ -42,6 +45,7 @@ func (c *CSIControllerSet) Run(ctx context.Context, workers int) { c.managementStateController, c.staticResourcesController, c.credentialsRequestController, + c.csiConfigObserverController, c.csiDriverControllerServiceController, c.csiDriverNodeServiceController, } { @@ -109,6 +113,19 @@ func (c *CSIControllerSet) WithCredentialsRequestController( return c } +func (c *CSIControllerSet) WithCSIConfigObserverController( + name string, + configinformers configinformers.SharedInformerFactory, +) *CSIControllerSet { + c.csiConfigObserverController = csiconfigobservercontroller.NewCSIConfigObserverController( + name, + c.operatorClient, + configinformers, + c.eventRecorder, + ) + return c +} + func (c *CSIControllerSet) WithCSIDriverControllerService( name string, assetFunc func(string) []byte, diff --git a/pkg/operator/csi/csidrivercontrollerservicecontroller/csi_driver_controller_service_controller.go b/pkg/operator/csi/csidrivercontrollerservicecontroller/csi_driver_controller_service_controller.go index c4781f53e4..5789e39045 100644 --- a/pkg/operator/csi/csidrivercontrollerservicecontroller/csi_driver_controller_service_controller.go +++ b/pkg/operator/csi/csidrivercontrollerservicecontroller/csi_driver_controller_service_controller.go @@ -8,10 +8,14 @@ import ( "strings" "time" + appsv1 "k8s.io/api/apps/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + appsinformersv1 "k8s.io/client-go/informers/apps/v1" + "k8s.io/client-go/kubernetes" opv1 "github.com/openshift/api/operator/v1" configinformers "github.com/openshift/client-go/config/informers/externalversions" + "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/loglevel" @@ -19,9 +23,6 @@ import ( "github.com/openshift/library-go/pkg/operator/resource/resourcemerge" "github.com/openshift/library-go/pkg/operator/resource/resourceread" "github.com/openshift/library-go/pkg/operator/v1helpers" - appsv1 "k8s.io/api/apps/v1" - appsinformersv1 "k8s.io/client-go/informers/apps/v1" - "k8s.io/client-go/kubernetes" ) const ( @@ -36,7 +37,7 @@ const ( ) // DeploymentHookFunc is a hook function to modify the Deployment. -type DeploymentHookFunc func(*appsv1.Deployment) error +type DeploymentHookFunc func(*opv1.OperatorSpec, *appsv1.Deployment) error // CSIDriverControllerServiceController is a controller that deploys a CSI Controller Service to a given namespace. // @@ -162,7 +163,7 @@ func (c *CSIDriverControllerServiceController) sync(ctx context.Context, syncCon required := resourceread.ReadDeploymentV1OrDie(manifest) for i := range c.optionalDeploymentHooks { - err := c.optionalDeploymentHooks[i](required) + err := c.optionalDeploymentHooks[i](opSpec, required) if err != nil { return fmt.Errorf("error running hook function (index=%d): %w", i, err) } diff --git a/pkg/operator/csi/csidrivercontrollerservicecontroller/csi_driver_controller_service_controller_test.go b/pkg/operator/csi/csidrivercontrollerservicecontroller/csi_driver_controller_service_controller_test.go index 4cfc2f7d54..a9d4f37425 100644 --- a/pkg/operator/csi/csidrivercontrollerservicecontroller/csi_driver_controller_service_controller_test.go +++ b/pkg/operator/csi/csidrivercontrollerservicecontroller/csi_driver_controller_service_controller_test.go @@ -367,7 +367,7 @@ func addGenerationReactor(client *fakecore.Clientset) { }) } -func deploymentAnnotationHook(instance *appsv1.Deployment) error { +func deploymentAnnotationHook(opSpec *opv1.OperatorSpec, instance *appsv1.Deployment) error { if instance.Annotations == nil { instance.Annotations = map[string]string{} } diff --git a/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers.go b/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers.go new file mode 100644 index 0000000000..d175009b19 --- /dev/null +++ b/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers.go @@ -0,0 +1,26 @@ +package csidrivercontrollerservicecontroller + +import ( + "strings" + + appsv1 "k8s.io/api/apps/v1" + + opv1 "github.com/openshift/api/operator/v1" + + "github.com/openshift/library-go/pkg/operator/csi/csiconfigobservercontroller" + "github.com/openshift/library-go/pkg/operator/v1helpers" +) + +// WithObservedProxyDeploymentHook creates a deployment hook that injects into the deployment's containers the observed proxy config. +func WithObservedProxyDeploymentHook() DeploymentHookFunc { + return func(opSpec *opv1.OperatorSpec, deployment *appsv1.Deployment) error { + containerNamesString := deployment.Annotations["config.openshift.io/inject-proxy"] + err := v1helpers.InjectObservedProxyIntoContainers( + &deployment.Spec.Template.Spec, + strings.Split(containerNamesString, ","), + opSpec.ObservedConfig.Raw, + csiconfigobservercontroller.ProxyConfigPath()..., + ) + return err + } +} diff --git a/pkg/operator/csi/csidrivernodeservicecontroller/csi_driver_node_service_controller.go b/pkg/operator/csi/csidrivernodeservicecontroller/csi_driver_node_service_controller.go index 6de64b8e05..7c0e5d45bf 100644 --- a/pkg/operator/csi/csidrivernodeservicecontroller/csi_driver_node_service_controller.go +++ b/pkg/operator/csi/csidrivernodeservicecontroller/csi_driver_node_service_controller.go @@ -30,7 +30,7 @@ const ( ) // DaemonSetHookFunc is a hook function to modify the DaemonSet. -type DaemonSetHookFunc func(*appsv1.DaemonSet) error +type DaemonSetHookFunc func(*opv1.OperatorSpec, *appsv1.DaemonSet) error // CSIDriverNodeServiceController is a controller that deploys a CSI Node Service to a given namespace. // @@ -128,7 +128,7 @@ func (c *CSIDriverNodeServiceController) sync(ctx context.Context, syncContext f required := resourceread.ReadDaemonSetV1OrDie(manifest) for i := range c.optionalDaemonSetHooks { - err := c.optionalDaemonSetHooks[i](required) + err := c.optionalDaemonSetHooks[i](opSpec, required) if err != nil { return fmt.Errorf("error running hook function (index=%d): %w", i, err) } diff --git a/pkg/operator/csi/csidrivernodeservicecontroller/csi_driver_node_service_controller_test.go b/pkg/operator/csi/csidrivernodeservicecontroller/csi_driver_node_service_controller_test.go index a0d752a5b3..bccef6266d 100644 --- a/pkg/operator/csi/csidrivernodeservicecontroller/csi_driver_node_service_controller_test.go +++ b/pkg/operator/csi/csidrivernodeservicecontroller/csi_driver_node_service_controller_test.go @@ -297,7 +297,7 @@ func addGenerationReactor(client *fakecore.Clientset) { }) } -func daemonSetAnnotationHook(instance *appsv1.DaemonSet) error { +func daemonSetAnnotationHook(opSpec *opv1.OperatorSpec, instance *appsv1.DaemonSet) error { if instance.Annotations == nil { instance.Annotations = map[string]string{} } diff --git a/pkg/operator/csi/csidrivernodeservicecontroller/helpers.go b/pkg/operator/csi/csidrivernodeservicecontroller/helpers.go new file mode 100644 index 0000000000..8b4a24f49f --- /dev/null +++ b/pkg/operator/csi/csidrivernodeservicecontroller/helpers.go @@ -0,0 +1,29 @@ +package csidrivernodeservicecontroller + +import ( + "strings" + + appsv1 "k8s.io/api/apps/v1" + + opv1 "github.com/openshift/api/operator/v1" + + "github.com/openshift/library-go/pkg/operator/csi/csiconfigobservercontroller" + "github.com/openshift/library-go/pkg/operator/v1helpers" +) + +// WithObservedProxyDaemonSetHook creates a hook that injects into the daemonSet's containers the observed proxy config. +func WithObservedProxyDaemonSetHook() DaemonSetHookFunc { + return func(opSpec *opv1.OperatorSpec, daemonSet *appsv1.DaemonSet) error { + containerNamesString := daemonSet.Annotations["config.openshift.io/inject-proxy"] + err := v1helpers.InjectObservedProxyIntoContainers( + &daemonSet.Spec.Template.Spec, + strings.Split(containerNamesString, ","), + opSpec.ObservedConfig.Raw, + csiconfigobservercontroller.ProxyConfigPath()..., + ) + if err != nil { + return err + } + return nil + } +} diff --git a/pkg/operator/v1helpers/helpers.go b/pkg/operator/v1helpers/helpers.go index ac680e27b6..40a46f96da 100644 --- a/pkg/operator/v1helpers/helpers.go +++ b/pkg/operator/v1helpers/helpers.go @@ -2,6 +2,7 @@ package v1helpers import ( "errors" + "fmt" "sort" "strings" "time" @@ -14,6 +15,8 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/util/retry" + "github.com/ghodss/yaml" + configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" ) @@ -310,3 +313,37 @@ func MapToEnvVars(mapEnvVars map[string]string) []corev1.EnvVar { sort.Slice(envVars, func(i, j int) bool { return envVars[i].Name < envVars[j].Name }) return envVars } + +// InjectObservedProxyIntoContainers injects proxy environment variables in containers specified in containerNames. +func InjectObservedProxyIntoContainers(podSpec *corev1.PodSpec, containerNames []string, observedConfig []byte, fields ...string) error { + var config map[string]interface{} + if err := yaml.Unmarshal(observedConfig, &config); err != nil { + return fmt.Errorf("failed to unmarshal the observedConfig: %w", err) + } + + proxyConfig, found, err := unstructured.NestedStringMap(config, fields...) + if err != nil { + return fmt.Errorf("couldn't get the proxy config from observedConfig: %w", err) + } + + proxyEnvVars := MapToEnvVars(proxyConfig) + if !found || len(proxyEnvVars) < 1 { + // There's no observed proxy config, we should tolerate that + return nil + } + + for _, containerName := range containerNames { + for i := range podSpec.InitContainers { + if podSpec.InitContainers[i].Name == containerName { + podSpec.InitContainers[i].Env = append(podSpec.InitContainers[i].Env, proxyEnvVars...) + } + } + for i := range podSpec.Containers { + if podSpec.Containers[i].Name == containerName { + podSpec.Containers[i].Env = append(podSpec.Containers[i].Env, proxyEnvVars...) + } + } + } + + return nil +} From be29e99d45053a7448fe34cddce976d130f07f16 Mon Sep 17 00:00:00 2001 From: Fabio Bertinatto Date: Mon, 11 Jan 2021 10:59:06 -0300 Subject: [PATCH 2/2] csi: add hook that annotates deployment with secret's hash --- .../csicontrollerset/csi_controller_set.go | 30 ++++++++++---- .../helpers.go | 39 +++++++++++++++++++ .../csidrivernodeservicecontroller/helpers.go | 8 +--- 3 files changed, 63 insertions(+), 14 deletions(-) diff --git a/pkg/operator/csi/csicontrollerset/csi_controller_set.go b/pkg/operator/csi/csicontrollerset/csi_controller_set.go index 03ed8190c8..505753754d 100644 --- a/pkg/operator/csi/csicontrollerset/csi_controller_set.go +++ b/pkg/operator/csi/csicontrollerset/csi_controller_set.go @@ -2,26 +2,26 @@ package csicontrollerset import ( "context" + "fmt" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/dynamic" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/cache" configinformers "github.com/openshift/client-go/config/informers/externalversions" - "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/csi/credentialsrequestcontroller" + "github.com/openshift/library-go/pkg/operator/csi/csiconfigobservercontroller" + "github.com/openshift/library-go/pkg/operator/csi/csidrivercontrollerservicecontroller" + "github.com/openshift/library-go/pkg/operator/csi/csidrivernodeservicecontroller" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/loglevel" "github.com/openshift/library-go/pkg/operator/management" "github.com/openshift/library-go/pkg/operator/resource/resourceapply" "github.com/openshift/library-go/pkg/operator/staticresourcecontroller" "github.com/openshift/library-go/pkg/operator/v1helpers" - - "github.com/openshift/library-go/pkg/operator/csi/credentialsrequestcontroller" - "github.com/openshift/library-go/pkg/operator/csi/csiconfigobservercontroller" - "github.com/openshift/library-go/pkg/operator/csi/csidrivercontrollerservicecontroller" - "github.com/openshift/library-go/pkg/operator/csi/csidrivernodeservicecontroller" ) // CSIControllerSet contains a set of controllers that are usually used to deploy CSI Drivers. @@ -34,12 +34,17 @@ type CSIControllerSet struct { csiDriverControllerServiceController factory.Controller csiDriverNodeServiceController factory.Controller - operatorClient v1helpers.OperatorClient - eventRecorder events.Recorder + preRunCachesSynced []cache.InformerSynced + operatorClient v1helpers.OperatorClient + eventRecorder events.Recorder } // Run starts all controllers initialized in the set. func (c *CSIControllerSet) Run(ctx context.Context, workers int) { + if !cache.WaitForCacheSync(ctx.Done(), c.preRunCachesSynced...) { + utilruntime.HandleError(fmt.Errorf("caches did not sync")) + return + } for _, ctrl := range []factory.Controller{ c.logLevelController, c.managementStateController, @@ -170,6 +175,15 @@ func (c *CSIControllerSet) WithCSIDriverNodeService( return c } +// WithExtraInformers adds informers that individual controllers don't wait for. These are typically +// informers used by hook functions in csidrivercontrollerservicecontroller and csidrivernodeservicecontroller. +func (c *CSIControllerSet) WithExtraInformers(informers ...cache.SharedIndexInformer) *CSIControllerSet { + for i := range informers { + c.preRunCachesSynced = append(c.preRunCachesSynced, informers[i].HasSynced) + } + return c +} + // New returns a basic *ControllerSet without any controller. func NewCSIControllerSet(operatorClient v1helpers.OperatorClient, eventRecorder events.Recorder) *CSIControllerSet { return &CSIControllerSet{ diff --git a/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers.go b/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers.go index d175009b19..fd8c26c163 100644 --- a/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers.go +++ b/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers.go @@ -1,13 +1,17 @@ package csidrivercontrollerservicecontroller import ( + "crypto/sha256" + "fmt" "strings" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/client-go/informers/core/v1" opv1 "github.com/openshift/api/operator/v1" "github.com/openshift/library-go/pkg/operator/csi/csiconfigobservercontroller" + "github.com/openshift/library-go/pkg/operator/resource/resourcehash" "github.com/openshift/library-go/pkg/operator/v1helpers" ) @@ -24,3 +28,38 @@ func WithObservedProxyDeploymentHook() DeploymentHookFunc { return err } } + +// With SecretHashAnnotationHook creates a deployment hook that annotates a Deployment with a secret's hash. +func WithSecretHashAnnotationHook( + namespace string, + secretName string, + secretInformer corev1.SecretInformer, +) DeploymentHookFunc { + return func(opSpec *opv1.OperatorSpec, deployment *appsv1.Deployment) error { + inputHashes, err := resourcehash.MultipleObjectHashStringMapForObjectReferenceFromLister( + nil, + secretInformer.Lister(), + resourcehash.NewObjectRef().ForSecret().InNamespace(namespace).Named(secretName), + ) + if err != nil { + return fmt.Errorf("invalid dependency reference: %w", err) + } + if deployment.Annotations == nil { + deployment.Annotations = map[string]string{} + } + if deployment.Spec.Template.Annotations == nil { + deployment.Spec.Template.Annotations = map[string]string{} + } + for k, v := range inputHashes { + annotationKey := fmt.Sprintf("operator.openshift.io/dep-%s", k) + if len(annotationKey) > 63 { + hash := sha256.Sum256([]byte(k)) + annotationKey = fmt.Sprintf("operator.openshift.io/dep-%x", hash) + annotationKey = annotationKey[:63] + } + deployment.Annotations[annotationKey] = v + deployment.Spec.Template.Annotations[annotationKey] = v + } + return nil + } +} diff --git a/pkg/operator/csi/csidrivernodeservicecontroller/helpers.go b/pkg/operator/csi/csidrivernodeservicecontroller/helpers.go index 8b4a24f49f..f32825d864 100644 --- a/pkg/operator/csi/csidrivernodeservicecontroller/helpers.go +++ b/pkg/operator/csi/csidrivernodeservicecontroller/helpers.go @@ -3,9 +3,8 @@ package csidrivernodeservicecontroller import ( "strings" - appsv1 "k8s.io/api/apps/v1" - opv1 "github.com/openshift/api/operator/v1" + appsv1 "k8s.io/api/apps/v1" "github.com/openshift/library-go/pkg/operator/csi/csiconfigobservercontroller" "github.com/openshift/library-go/pkg/operator/v1helpers" @@ -21,9 +20,6 @@ func WithObservedProxyDaemonSetHook() DaemonSetHookFunc { opSpec.ObservedConfig.Raw, csiconfigobservercontroller.ProxyConfigPath()..., ) - if err != nil { - return err - } - return nil + return err } }