Skip to content

Commit a67c967

Browse files
ThomasK33ThomasK33
committed
fix: watch envFrom sources and normalize cert mount collisions
1 parent f4d0af8 commit a67c967

3 files changed

Lines changed: 222 additions & 20 deletions

File tree

‎config/rbac/role.yaml‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ rules:
88
- ""
99
resources:
1010
- configmaps
11-
- pods/log
11+
- namespaces
1212
verbs:
1313
- get
14+
- list
15+
- watch
1416
- apiGroups:
1517
- ""
1618
resources:
@@ -21,14 +23,6 @@ rules:
2123
- list
2224
- patch
2325
- watch
24-
- apiGroups:
25-
- ""
26-
resources:
27-
- namespaces
28-
verbs:
29-
- get
30-
- list
31-
- watch
3226
- apiGroups:
3327
- ""
3428
resources:
@@ -43,6 +37,12 @@ rules:
4337
- patch
4438
- update
4539
- watch
40+
- apiGroups:
41+
- ""
42+
resources:
43+
- pods/log
44+
verbs:
45+
- get
4646
- apiGroups:
4747
- ""
4848
resources:

‎internal/controller/codercontrolplane_controller.go‎

Lines changed: 161 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,10 @@ const (
6060
workspaceRoleNameSuffix = "-workspace-perms"
6161
kubernetesObjectNameMaxLength = 253
6262

63-
// #nosec G101 -- this is a field index key, not a credential.
64-
licenseSecretNameFieldIndex = ".spec.licenseSecretRef.name"
63+
// #nosec G101 -- these are field index keys, not credentials.
64+
licenseSecretNameFieldIndex = ".spec.licenseSecretRef.name"
65+
envFromConfigMapNameFieldIndex = ".spec.envFrom.configMapRef.name"
66+
envFromSecretNameFieldIndex = ".spec.envFrom.secretRef.name" // #nosec G101 -- this is a field index key, not a credential.
6567

6668
licenseConditionReasonApplied = "Applied"
6769
licenseConditionReasonPending = "Pending"
@@ -199,7 +201,7 @@ type CoderControlPlaneReconciler struct {
199201
// +kubebuilder:rbac:groups=coder.com,resources=codercontrolplanes/status,verbs=get;update;patch
200202
// +kubebuilder:rbac:groups=coder.com,resources=codercontrolplanes/finalizers,verbs=update
201203
// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete
202-
// +kubebuilder:rbac:groups="",resources=configmaps,verbs=get
204+
// +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch
203205
// +kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch;create;update;patch;delete
204206
// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;update;patch;delete
205207
// +kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch;create;update;patch;delete
@@ -1057,13 +1059,13 @@ func (r *CoderControlPlaneReconciler) reconcileDeployment(ctx context.Context, c
10571059
if certSecretNameCounts[secret.Name] > 1 {
10581060
mountFileBase = fmt.Sprintf("%s-%s", secret.Name, secret.Key)
10591061
}
1060-
mountFileCount := certMountFileCount[mountFileBase]
1061-
certMountFileCount[mountFileBase] = mountFileCount + 1
1062-
10631062
mountFileName := mountFileBase
10641063
if !strings.HasSuffix(mountFileName, ".crt") {
10651064
mountFileName += ".crt"
10661065
}
1066+
1067+
mountFileCount := certMountFileCount[mountFileName]
1068+
certMountFileCount[mountFileName] = mountFileCount + 1
10671069
if mountFileCount > 0 {
10681070
mountFileName = strings.TrimSuffix(mountFileName, ".crt")
10691071
mountFileName = fmt.Sprintf("%s-%d.crt", mountFileName, mountFileCount+1)
@@ -2568,30 +2570,135 @@ func indexByLicenseSecretName(obj client.Object) []string {
25682570
return []string{licenseSecretName}
25692571
}
25702572

2571-
func (r *CoderControlPlaneReconciler) reconcileRequestsForLicenseSecret(
2573+
func indexByEnvFromConfigMapName(obj client.Object) []string {
2574+
coderControlPlane, ok := obj.(*coderv1alpha1.CoderControlPlane)
2575+
if !ok {
2576+
return nil
2577+
}
2578+
2579+
configMapNames := map[string]struct{}{}
2580+
for i := range coderControlPlane.Spec.EnvFrom {
2581+
configMapRef := coderControlPlane.Spec.EnvFrom[i].ConfigMapRef
2582+
if configMapRef == nil {
2583+
continue
2584+
}
2585+
configMapName := strings.TrimSpace(configMapRef.Name)
2586+
if configMapName == "" {
2587+
continue
2588+
}
2589+
configMapNames[configMapName] = struct{}{}
2590+
}
2591+
2592+
indexedNames := make([]string, 0, len(configMapNames))
2593+
for configMapName := range configMapNames {
2594+
indexedNames = append(indexedNames, configMapName)
2595+
}
2596+
2597+
return indexedNames
2598+
}
2599+
2600+
func indexByEnvFromSecretName(obj client.Object) []string {
2601+
coderControlPlane, ok := obj.(*coderv1alpha1.CoderControlPlane)
2602+
if !ok {
2603+
return nil
2604+
}
2605+
2606+
secretNames := map[string]struct{}{}
2607+
for i := range coderControlPlane.Spec.EnvFrom {
2608+
secretRef := coderControlPlane.Spec.EnvFrom[i].SecretRef
2609+
if secretRef == nil {
2610+
continue
2611+
}
2612+
secretName := strings.TrimSpace(secretRef.Name)
2613+
if secretName == "" {
2614+
continue
2615+
}
2616+
secretNames[secretName] = struct{}{}
2617+
}
2618+
2619+
indexedNames := make([]string, 0, len(secretNames))
2620+
for secretName := range secretNames {
2621+
indexedNames = append(indexedNames, secretName)
2622+
}
2623+
2624+
return indexedNames
2625+
}
2626+
2627+
func (r *CoderControlPlaneReconciler) reconcileRequestsForEnvFromConfigMap(
2628+
ctx context.Context,
2629+
obj client.Object,
2630+
) []reconcile.Request {
2631+
configMap, ok := obj.(*corev1.ConfigMap)
2632+
if !ok {
2633+
return nil
2634+
}
2635+
2636+
return r.reconcileRequestsForIndexedControlPlanes(ctx, configMap.Namespace, envFromConfigMapNameFieldIndex, configMap.Name)
2637+
}
2638+
2639+
func (r *CoderControlPlaneReconciler) reconcileRequestsForEnvFromSecret(
25722640
ctx context.Context,
25732641
obj client.Object,
25742642
) []reconcile.Request {
25752643
secret, ok := obj.(*corev1.Secret)
25762644
if !ok {
25772645
return nil
25782646
}
2579-
if strings.TrimSpace(secret.Name) == "" || strings.TrimSpace(secret.Namespace) == "" {
2647+
2648+
return r.reconcileRequestsForIndexedControlPlanes(ctx, secret.Namespace, envFromSecretNameFieldIndex, secret.Name)
2649+
}
2650+
2651+
func mergeReconcileRequests(requestGroups ...[]reconcile.Request) []reconcile.Request {
2652+
if len(requestGroups) == 0 {
2653+
return nil
2654+
}
2655+
2656+
seen := map[string]struct{}{}
2657+
merged := make([]reconcile.Request, 0)
2658+
for i := range requestGroups {
2659+
for j := range requestGroups[i] {
2660+
request := requestGroups[i][j]
2661+
if strings.TrimSpace(request.Name) == "" || strings.TrimSpace(request.Namespace) == "" {
2662+
continue
2663+
}
2664+
key := namespacedResourceKey(request.Namespace, request.Name)
2665+
if _, exists := seen[key]; exists {
2666+
continue
2667+
}
2668+
seen[key] = struct{}{}
2669+
merged = append(merged, request)
2670+
}
2671+
}
2672+
2673+
return merged
2674+
}
2675+
2676+
func (r *CoderControlPlaneReconciler) reconcileRequestsForIndexedControlPlanes(
2677+
ctx context.Context,
2678+
namespace string,
2679+
indexField string,
2680+
indexValue string,
2681+
) []reconcile.Request {
2682+
namespace = strings.TrimSpace(namespace)
2683+
indexField = strings.TrimSpace(indexField)
2684+
indexValue = strings.TrimSpace(indexValue)
2685+
if namespace == "" || indexField == "" || indexValue == "" {
25802686
return nil
25812687
}
25822688

25832689
var coderControlPlanes coderv1alpha1.CoderControlPlaneList
25842690
if err := r.List(
25852691
ctx,
25862692
&coderControlPlanes,
2587-
client.InNamespace(secret.Namespace),
2588-
client.MatchingFields{licenseSecretNameFieldIndex: secret.Name},
2693+
client.InNamespace(namespace),
2694+
client.MatchingFields{indexField: indexValue},
25892695
); err != nil {
25902696
return nil
25912697
}
25922698

25932699
requests := make([]reconcile.Request, 0, len(coderControlPlanes.Items))
2594-
for _, coderControlPlane := range coderControlPlanes.Items {
2700+
for i := range coderControlPlanes.Items {
2701+
coderControlPlane := coderControlPlanes.Items[i]
25952702
if strings.TrimSpace(coderControlPlane.Name) == "" || strings.TrimSpace(coderControlPlane.Namespace) == "" {
25962703
continue
25972704
}
@@ -2604,6 +2711,29 @@ func (r *CoderControlPlaneReconciler) reconcileRequestsForLicenseSecret(
26042711
return requests
26052712
}
26062713

2714+
func (r *CoderControlPlaneReconciler) reconcileRequestsForLicenseSecret(
2715+
ctx context.Context,
2716+
obj client.Object,
2717+
) []reconcile.Request {
2718+
secret, ok := obj.(*corev1.Secret)
2719+
if !ok {
2720+
return nil
2721+
}
2722+
if strings.TrimSpace(secret.Name) == "" || strings.TrimSpace(secret.Namespace) == "" {
2723+
return nil
2724+
}
2725+
2726+
licenseSecretRequests := r.reconcileRequestsForIndexedControlPlanes(
2727+
ctx,
2728+
secret.Namespace,
2729+
licenseSecretNameFieldIndex,
2730+
secret.Name,
2731+
)
2732+
envFromSecretRequests := r.reconcileRequestsForEnvFromSecret(ctx, secret)
2733+
2734+
return mergeReconcileRequests(licenseSecretRequests, envFromSecretRequests)
2735+
}
2736+
26072737
func isDuplicateLicenseUploadError(err error) bool {
26082738
var sdkErr *codersdk.Error
26092739
if !errors.As(err, &sdkErr) {
@@ -2775,6 +2905,22 @@ func (r *CoderControlPlaneReconciler) SetupWithManager(mgr ctrl.Manager) error {
27752905
); err != nil {
27762906
return fmt.Errorf("index coder control planes by license secret name: %w", err)
27772907
}
2908+
if err := mgr.GetFieldIndexer().IndexField(
2909+
context.Background(),
2910+
&coderv1alpha1.CoderControlPlane{},
2911+
envFromConfigMapNameFieldIndex,
2912+
indexByEnvFromConfigMapName,
2913+
); err != nil {
2914+
return fmt.Errorf("index coder control planes by envFrom ConfigMap name: %w", err)
2915+
}
2916+
if err := mgr.GetFieldIndexer().IndexField(
2917+
context.Background(),
2918+
&coderv1alpha1.CoderControlPlane{},
2919+
envFromSecretNameFieldIndex,
2920+
indexByEnvFromSecretName,
2921+
); err != nil {
2922+
return fmt.Errorf("index coder control planes by envFrom Secret name: %w", err)
2923+
}
27782924

27792925
return ctrl.NewControllerManagedBy(mgr).
27802926
For(&coderv1alpha1.CoderControlPlane{}).
@@ -2789,6 +2935,10 @@ func (r *CoderControlPlaneReconciler) SetupWithManager(mgr ctrl.Manager) error {
27892935
&corev1.Secret{},
27902936
handler.EnqueueRequestsFromMapFunc(r.reconcileRequestsForLicenseSecret),
27912937
).
2938+
Watches(
2939+
&corev1.ConfigMap{},
2940+
handler.EnqueueRequestsFromMapFunc(r.reconcileRequestsForEnvFromConfigMap),
2941+
).
27922942
Named("codercontrolplane").
27932943
Complete(r)
27942944
}

‎internal/controller/codercontrolplane_controller_test.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3715,6 +3715,58 @@ func TestReconcile_CertSecretsWithSharedSecretNameReuseVolume(t *testing.T) {
37153715
}
37163716
}
37173717

3718+
func TestReconcile_CertSecretMountFileNormalizationAvoidsPathCollisions(t *testing.T) {
3719+
ensureGatewaySchemeRegistered(t)
3720+
ctx := context.Background()
3721+
3722+
cp := &coderv1alpha1.CoderControlPlane{
3723+
ObjectMeta: metav1.ObjectMeta{Name: "test-cert-normalized-mount-collision", Namespace: "default"},
3724+
Spec: coderv1alpha1.CoderControlPlaneSpec{
3725+
Image: "test-certs-mount-collision:latest",
3726+
Certs: coderv1alpha1.CertsSpec{
3727+
Secrets: []coderv1alpha1.CertSecretSelector{
3728+
{Name: "foo", Key: "ca.crt"},
3729+
{Name: "foo.crt", Key: "ca.crt"},
3730+
},
3731+
},
3732+
},
3733+
}
3734+
if err := k8sClient.Create(ctx, cp); err != nil {
3735+
t.Fatalf("create control plane: %v", err)
3736+
}
3737+
t.Cleanup(func() {
3738+
_ = k8sClient.Delete(ctx, cp)
3739+
})
3740+
3741+
r := &controller.CoderControlPlaneReconciler{Client: k8sClient, Scheme: scheme}
3742+
if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: cp.Name, Namespace: cp.Namespace}}); err != nil {
3743+
t.Fatalf("reconcile control plane: %v", err)
3744+
}
3745+
3746+
deployment := &appsv1.Deployment{}
3747+
if err := k8sClient.Get(ctx, types.NamespacedName{Name: cp.Name, Namespace: cp.Namespace}, deployment); err != nil {
3748+
t.Fatalf("get deployment: %v", err)
3749+
}
3750+
container := deployment.Spec.Template.Spec.Containers[0]
3751+
3752+
mountPaths := map[string]struct{}{}
3753+
for _, mount := range container.VolumeMounts {
3754+
if !strings.HasPrefix(mount.MountPath, "/etc/ssl/certs/foo") {
3755+
continue
3756+
}
3757+
mountPaths[mount.MountPath] = struct{}{}
3758+
}
3759+
if len(mountPaths) != 2 {
3760+
t.Fatalf("expected two unique normalized cert mount paths, got %d: %#v", len(mountPaths), mountPaths)
3761+
}
3762+
if _, ok := mountPaths["/etc/ssl/certs/foo.crt"]; !ok {
3763+
t.Fatalf("expected normalized mount path %q, got %#v", "/etc/ssl/certs/foo.crt", mountPaths)
3764+
}
3765+
if _, ok := mountPaths["/etc/ssl/certs/foo-2.crt"]; !ok {
3766+
t.Fatalf("expected deduplicated mount path %q, got %#v", "/etc/ssl/certs/foo-2.crt", mountPaths)
3767+
}
3768+
}
3769+
37183770
func TestReconcile_PassThroughConfiguration(t *testing.T) {
37193771
ensureGatewaySchemeRegistered(t)
37203772
ctx := context.Background()

0 commit comments

Comments
 (0)