diff --git a/cmd/secrets-store-csi-driver/main.go b/cmd/secrets-store-csi-driver/main.go index 1d0c2a591..2acaf1a2e 100644 --- a/cmd/secrets-store-csi-driver/main.go +++ b/cmd/secrets-store-csi-driver/main.go @@ -23,6 +23,7 @@ import ( "net/http" _ "net/http/pprof" // #nosec "os" + "strings" "time" secretsstorev1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1" @@ -56,6 +57,10 @@ var ( nodeID = flag.String("nodeid", "", "node id") logFormatJSON = flag.Bool("log-format-json", false, "set log formatter to json") providerVolumePath = flag.String("provider-volume", "/etc/kubernetes/secrets-store-csi-providers", "Volume path for provider") + // Check in additional paths for providers. Added to support migration from /etc/ to /var/ as part of + // https://github.com/kubernetes-sigs/secrets-store-csi-driver/issues/823. + // The default should be moved to /var/ in https://github.com/kubernetes-sigs/secrets-store-csi-driver/issues/870 + additionalProviderPaths = flag.String("additional-provider-volume-paths", "/var/run/secrets-store-csi-providers", "Comma separated list of additional paths to communicate with providers") // this will be removed in a future release metricsAddr = flag.String("metrics-addr", ":8095", "The address the metric endpoint binds to") enableSecretRotation = flag.Bool("enable-secret-rotation", false, "Enable secret rotation feature [alpha]") @@ -159,7 +164,9 @@ func main() { ctx := withShutdownSignal(context.Background()) // create provider clients - providerClients := secretsstore.NewPluginClientBuilder(*providerVolumePath, grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(*maxCallRecvMsgSize))) + providerPaths := strings.Split(strings.TrimSpace(*additionalProviderPaths), ",") + providerPaths = append(providerPaths, *providerVolumePath) + providerClients := secretsstore.NewPluginClientBuilder(providerPaths, grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(*maxCallRecvMsgSize))) defer providerClients.Cleanup() // enable provider health check diff --git a/manifest_staging/charts/secrets-store-csi-driver/templates/secrets-store-csi-driver-windows.yaml b/manifest_staging/charts/secrets-store-csi-driver/templates/secrets-store-csi-driver-windows.yaml index ba39b86f1..4ce4fec83 100644 --- a/manifest_staging/charts/secrets-store-csi-driver/templates/secrets-store-csi-driver-windows.yaml +++ b/manifest_staging/charts/secrets-store-csi-driver/templates/secrets-store-csi-driver-windows.yaml @@ -76,7 +76,8 @@ spec: {{- end }} - "--endpoint=$(CSI_ENDPOINT)" - "--nodeid=$(KUBE_NODE_NAME)" - - "--provider-volume=C:\\k\\secrets-store-csi-providers" + - "--provider-volume={{ .Values.windows.providersDir }}" + - "--additional-provider-volume-paths={{ join "," .Values.windows.additionalProvidersDirs }}" {{- if and (semverCompare ">= v0.0.9-0" .Values.windows.image.tag) .Values.minimumProviderVersions }} - "--min-provider-version={{ .Values.minimumProviderVersions }}" {{- end }} @@ -131,7 +132,11 @@ spec: - name: mountpoint-dir mountPath: {{ .Values.windows.kubeletRootDir }}\pods - name: providers-dir - mountPath: C:\k\secrets-store-csi-providers + mountPath: "{{ .Values.windows.providersDir }}" + {{- range $i, $path := .Values.windows.additionalProvidersDirs }} + - name: providers-dir-{{ $i }} + mountPath: "{{ $path }}" + {{- end }} {{- if .Values.windows.volumeMounts }} {{- toYaml .Values.windows.volumeMounts | nindent 12}} {{- end }} @@ -174,8 +179,14 @@ spec: type: DirectoryOrCreate - name: providers-dir hostPath: - path: {{ .Values.windows.providersDir }} + path: "{{ .Values.windows.providersDir }}" + type: DirectoryOrCreate + {{- range $i, $path := .Values.windows.additionalProvidersDirs }} + - name: providers-dir-{{ $i }} + hostPath: + path: "{{ $path }}" type: DirectoryOrCreate + {{- end }} {{- if .Values.windows.volumes }} {{- toYaml .Values.windows.volumes | nindent 8}} {{- end }} diff --git a/manifest_staging/charts/secrets-store-csi-driver/templates/secrets-store-csi-driver.yaml b/manifest_staging/charts/secrets-store-csi-driver/templates/secrets-store-csi-driver.yaml index 0184ee007..48a4f4028 100644 --- a/manifest_staging/charts/secrets-store-csi-driver/templates/secrets-store-csi-driver.yaml +++ b/manifest_staging/charts/secrets-store-csi-driver/templates/secrets-store-csi-driver.yaml @@ -76,7 +76,8 @@ spec: {{- end }} - "--endpoint=$(CSI_ENDPOINT)" - "--nodeid=$(KUBE_NODE_NAME)" - - "--provider-volume=/etc/kubernetes/secrets-store-csi-providers" + - "--provider-volume={{ .Values.linux.providersDir }}" + - "--additional-provider-volume-paths={{ join "," .Values.linux.additionalProvidersDirs }}" {{- if and (semverCompare ">= v0.0.8-0" .Values.linux.image.tag) .Values.minimumProviderVersions }} - "--min-provider-version={{ .Values.minimumProviderVersions }}" {{- end }} @@ -134,7 +135,11 @@ spec: mountPath: {{ .Values.linux.kubeletRootDir }}/pods mountPropagation: Bidirectional - name: providers-dir - mountPath: /etc/kubernetes/secrets-store-csi-providers + mountPath: {{ .Values.linux.providersDir }} + {{- range $i, $path := .Values.linux.additionalProvidersDirs }} + - name: providers-dir-{{ $i }} + mountPath: "{{ $path }}" + {{- end }} {{- if .Values.linux.volumeMounts }} {{- toYaml .Values.linux.volumeMounts | nindent 12}} {{- end }} @@ -179,6 +184,12 @@ spec: hostPath: path: {{ .Values.linux.providersDir }} type: DirectoryOrCreate + {{- range $i, $path := .Values.linux.additionalProvidersDirs }} + - name: providers-dir-{{ $i }} + hostPath: + path: "{{ $path }}" + type: DirectoryOrCreate + {{- end }} {{- if .Values.linux.volumes }} {{- toYaml .Values.linux.volumes | nindent 8}} {{- end }} diff --git a/manifest_staging/charts/secrets-store-csi-driver/values.yaml b/manifest_staging/charts/secrets-store-csi-driver/values.yaml index 14dee94d9..3033e7baf 100644 --- a/manifest_staging/charts/secrets-store-csi-driver/values.yaml +++ b/manifest_staging/charts/secrets-store-csi-driver/values.yaml @@ -4,7 +4,7 @@ linux: repository: k8s.gcr.io/csi-secrets-store/driver tag: v1.0.1 pullPolicy: IfNotPresent - + crds: image: repository: k8s.gcr.io/csi-secrets-store/driver-crds @@ -13,15 +13,15 @@ linux: annotations: {} ## Prevent the CSI driver from being scheduled on virtual-kubelet nodes - affinity: + affinity: nodeAffinity: requiredDuringSchedulingIgnoredDuringExecution: nodeSelectorTerms: - - matchExpressions: - - key: type - operator: NotIn - values: - - virtual-kubelet + - matchExpressions: + - key: type + operator: NotIn + values: + - virtual-kubelet driver: resources: @@ -61,7 +61,6 @@ linux: cpu: 10m memory: 20Mi - updateStrategy: type: RollingUpdate rollingUpdate: @@ -69,6 +68,8 @@ linux: kubeletRootDir: /var/lib/kubelet providersDir: /etc/kubernetes/secrets-store-csi-providers + additionalProvidersDirs: + - /var/run/secrets-store-csi-providers nodeSelector: {} tolerations: [] metricsAddr: ":8095" @@ -97,15 +98,15 @@ windows: pullPolicy: IfNotPresent ## Prevent the CSI driver from being scheduled on virtual-kubelet nodes - affinity: + affinity: nodeAffinity: requiredDuringSchedulingIgnoredDuringExecution: nodeSelectorTerms: - - matchExpressions: - - key: type - operator: NotIn - values: - - virtual-kubelet + - matchExpressions: + - key: type + operator: NotIn + values: + - virtual-kubelet driver: resources: @@ -151,7 +152,8 @@ windows: maxUnavailable: 1 kubeletRootDir: C:\var\lib\kubelet - providersDir: C:\k\secrets-store-csi-providers + providersDir: C:\\k\\secrets-store-csi-providers + additionalProvidersDirs: nodeSelector: {} tolerations: [] metricsAddr: ":8095" diff --git a/manifest_staging/deploy/secrets-store-csi-driver.yaml b/manifest_staging/deploy/secrets-store-csi-driver.yaml index 52dc8a58a..3c68c5576 100644 --- a/manifest_staging/deploy/secrets-store-csi-driver.yaml +++ b/manifest_staging/deploy/secrets-store-csi-driver.yaml @@ -55,6 +55,7 @@ spec: - "--endpoint=$(CSI_ENDPOINT)" - "--nodeid=$(KUBE_NODE_NAME)" - "--provider-volume=/etc/kubernetes/secrets-store-csi-providers" + - "--additional-provider-volume-paths=/var/run/secrets-store-csi-providers" - "--metrics-addr=:8095" - "--enable-secret-rotation=false" - "--rotation-poll-interval=2m" @@ -94,6 +95,8 @@ spec: mountPropagation: Bidirectional - name: providers-dir mountPath: /etc/kubernetes/secrets-store-csi-providers + - name: providers-dir-0 + mountPath: /var/run/secrets-store-csi-providers resources: limits: cpu: 200m @@ -136,5 +139,9 @@ spec: hostPath: path: /etc/kubernetes/secrets-store-csi-providers type: DirectoryOrCreate + - name: providers-dir-0 + hostPath: + path: /var/run/secrets-store-csi-providers + type: DirectoryOrCreate nodeSelector: kubernetes.io/os: linux diff --git a/pkg/rotation/reconciler_test.go b/pkg/rotation/reconciler_test.go index 12d21d769..916312a05 100644 --- a/pkg/rotation/reconciler_test.go +++ b/pkg/rotation/reconciler_test.go @@ -71,7 +71,7 @@ func newTestReconciler(client client.Reader, s *runtime.Scheme, kubeClient kuber return &Reconciler{ providerVolumePath: socketPath, rotationPollInterval: rotationPollInterval, - providerClients: secretsstore.NewPluginClientBuilder(socketPath), + providerClients: secretsstore.NewPluginClientBuilder([]string{socketPath}), queue: workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()), reporter: newStatsReporter(), eventRecorder: fakeRecorder, diff --git a/pkg/secrets-store/nodeserver_test.go b/pkg/secrets-store/nodeserver_test.go index 40ec1c0f0..a246f1f3d 100644 --- a/pkg/secrets-store/nodeserver_test.go +++ b/pkg/secrets-store/nodeserver_test.go @@ -42,7 +42,7 @@ import ( func testNodeServer(t *testing.T, tmpDir string, mountPoints []mount.MountPoint, client client.Client, reporter StatsReporter) (*nodeServer, error) { t.Helper() - providerClients := NewPluginClientBuilder(tmpDir) + providerClients := NewPluginClientBuilder([]string{tmpDir}) return newNodeServer(tmpDir, "testnode", mount.NewFakeMounter(mountPoints), providerClients, client, client, reporter, k8s.NewTokenClient(fakeclient.NewSimpleClientset(), "test-driver", 1*time.Second)) } diff --git a/pkg/secrets-store/provider_client.go b/pkg/secrets-store/provider_client.go index 68c575682..313b9f2c6 100644 --- a/pkg/secrets-store/provider_client.go +++ b/pkg/secrets-store/provider_client.go @@ -22,6 +22,7 @@ import ( "fmt" "net" "os" + "path/filepath" "regexp" "strings" "sync" @@ -72,11 +73,11 @@ var ( // PluginClientBuilder builds and stores grpc clients for communicating with // provider plugins. type PluginClientBuilder struct { - clients map[string]v1alpha1.CSIDriverProviderClient - conns map[string]*grpc.ClientConn - socketPath string - lock sync.RWMutex - opts []grpc.DialOption + clients map[string]v1alpha1.CSIDriverProviderClient + conns map[string]*grpc.ClientConn + socketPaths []string + lock sync.RWMutex + opts []grpc.DialOption } // NewPluginClientBuilder creates a PluginClientBuilder that will connect to @@ -89,12 +90,12 @@ type PluginClientBuilder struct { // // Additional grpc dial options can also be set through opts and will be used // when creating all clients. -func NewPluginClientBuilder(path string, opts ...grpc.DialOption) *PluginClientBuilder { +func NewPluginClientBuilder(paths []string, opts ...grpc.DialOption) *PluginClientBuilder { return &PluginClientBuilder{ - clients: make(map[string]v1alpha1.CSIDriverProviderClient), - conns: make(map[string]*grpc.ClientConn), - socketPath: path, - lock: sync.RWMutex{}, + clients: make(map[string]v1alpha1.CSIDriverProviderClient), + conns: make(map[string]*grpc.ClientConn), + socketPaths: paths, + lock: sync.RWMutex{}, opts: append(opts, []grpc.DialOption{ grpc.WithInsecure(), // the interface is only secured through filesystem ACLs grpc.WithContextDialer(func(ctx context.Context, target string) (net.Conn, error) { @@ -124,12 +125,22 @@ func (p *PluginClientBuilder) Get(ctx context.Context, provider string) (v1alpha return nil, fmt.Errorf("%w: provider %q", ErrInvalidProvider, provider) } - if _, err := os.Stat(fmt.Sprintf("%s/%s.sock", p.socketPath, provider)); os.IsNotExist(err) { + // check all paths + socketPath := "" + for k := range p.socketPaths { + tryPath := filepath.Join(p.socketPaths[k], provider+".sock") + if _, err := os.Stat(tryPath); err == nil { + socketPath = tryPath + break + } + } + + if socketPath == "" { return nil, fmt.Errorf("%w: provider %q", ErrProviderNotFound, provider) } conn, err := grpc.Dial( - fmt.Sprintf("%s/%s.sock", p.socketPath, provider), + socketPath, p.opts..., ) if err != nil { diff --git a/pkg/secrets-store/provider_client_test.go b/pkg/secrets-store/provider_client_test.go index 90882128e..90890e912 100644 --- a/pkg/secrets-store/provider_client_test.go +++ b/pkg/secrets-store/provider_client_test.go @@ -178,7 +178,7 @@ func TestMountContent(t *testing.T) { socketPath := tmpdir.New(t, "", "ut") targetPath := tmpdir.New(t, "", "ut") - pool := NewPluginClientBuilder(socketPath) + pool := NewPluginClientBuilder([]string{socketPath}) defer pool.Cleanup() server, cleanup := fakeServer(t, socketPath, "provider1") @@ -230,7 +230,7 @@ func TestMountContent_TooLarge(t *testing.T) { targetPath := tmpdir.New(t, "", "ut") // set a very small max message size - pool := NewPluginClientBuilder(socketPath, grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(5))) + pool := NewPluginClientBuilder([]string{socketPath}, grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(5))) defer pool.Cleanup() server, cleanup := fakeServer(t, socketPath, "provider1") @@ -328,7 +328,7 @@ func TestMountContentError(t *testing.T) { t.Run(test.name, func(t *testing.T) { socketPath := tmpdir.New(t, "", "ut") - pool := NewPluginClientBuilder(socketPath) + pool := NewPluginClientBuilder([]string{socketPath}) defer pool.Cleanup() providerName := "provider1" @@ -365,7 +365,40 @@ func TestMountContentError(t *testing.T) { func TestPluginClientBuilder(t *testing.T) { path := tmpdir.New(t, "", "ut") - cb := NewPluginClientBuilder(path) + cb := NewPluginClientBuilder([]string{path}) + ctx := context.Background() + + for i := 0; i < 10; i++ { + server, cleanup := fakeServer(t, path, fmt.Sprintf("server-%d", i)) + defer cleanup() + if err := server.Start(); err != nil { + t.Fatalf("expected err to be nil, got: %+v", err) + } + } + + var wg sync.WaitGroup + + for i := 0; i < 10; i++ { + wg.Add(1) + provider := fmt.Sprintf("server-%d", i) + go func() { + defer wg.Done() + if _, err := cb.Get(ctx, provider); err != nil { + t.Errorf("Get(%q) = %v, want nil", provider, err) + } + }() + } + + wg.Wait() +} + +func TestPluginClientBuilderMultiPath(t *testing.T) { + emptyPath := tmpdir.New(t, "", "ut") + path := tmpdir.New(t, "", "ut") + + // Ensure that if the path containing the listening socket is not the first + // path checked that the operation still succeeds. + cb := NewPluginClientBuilder([]string{emptyPath, path}) ctx := context.Background() for i := 0; i < 10; i++ { @@ -395,7 +428,7 @@ func TestPluginClientBuilder(t *testing.T) { func TestPluginClientBuilder_ConcurrentGet(t *testing.T) { path := tmpdir.New(t, "", "ut") - cb := NewPluginClientBuilder(path) + cb := NewPluginClientBuilder([]string{path}) ctx := context.Background() provider := "server" @@ -423,7 +456,7 @@ func TestPluginClientBuilder_ConcurrentGet(t *testing.T) { func TestPluginClientBuilderErrorNotFound(t *testing.T) { path := tmpdir.New(t, "", "ut") - cb := NewPluginClientBuilder(path) + cb := NewPluginClientBuilder([]string{path}) ctx := context.Background() if _, err := cb.Get(ctx, "notfoundprovider"); !errors.Is(err, ErrProviderNotFound) { @@ -445,7 +478,7 @@ func TestPluginClientBuilderErrorNotFound(t *testing.T) { func TestPluginClientBuilderErrorInvalid(t *testing.T) { path := tmpdir.New(t, "", "ut") - cb := NewPluginClientBuilder(path) + cb := NewPluginClientBuilder([]string{path}) ctx := context.Background() if _, err := cb.Get(ctx, "bad/provider/name"); !errors.Is(err, ErrInvalidProvider) { @@ -468,7 +501,7 @@ func TestVersion(t *testing.T) { t.Run(test.name, func(t *testing.T) { socketPath := tmpdir.New(t, "", "ut") - pool := NewPluginClientBuilder(socketPath) + pool := NewPluginClientBuilder([]string{socketPath}) defer pool.Cleanup() server, cleanup := fakeServer(t, socketPath, "provider1") @@ -499,7 +532,7 @@ func TestPluginClientBuilder_HealthCheck(t *testing.T) { // HealthCheck() method work as expected path := tmpdir.New(t, "", "ut") - cb := NewPluginClientBuilder(path) + cb := NewPluginClientBuilder([]string{path}) ctx := context.Background() healthCheckInterval := 1 * time.Millisecond diff --git a/test/e2eprovider/e2e-provider-installer.yaml b/test/e2eprovider/e2e-provider-installer.yaml index b4f08ac6a..63903e61a 100644 --- a/test/e2eprovider/e2e-provider-installer.yaml +++ b/test/e2eprovider/e2e-provider-installer.yaml @@ -57,6 +57,6 @@ spec: volumes: - name: providervol hostPath: - path: "/etc/kubernetes/secrets-store-csi-providers" + path: "/var/run/secrets-store-csi-providers" nodeSelector: kubernetes.io/os: linux