Skip to content

Commit bde2afd

Browse files
committed
Fix a reconciliation issue with mount_ca_trust
[noissue]
1 parent 1d7b3a3 commit bde2afd

File tree

13 files changed

+201
-321
lines changed

13 files changed

+201
-321
lines changed

apis/repo-manager.pulpproject.org/v1/pulp_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ type PulpSpec struct {
328328
// Required on vanilla Kubernetes when mount_trusted_ca is true. Optional on OpenShift.
329329
// +kubebuilder:validation:Optional
330330
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Trusted CA ConfigMap Key",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:advanced","urn:alm:descriptor:com.tectonic.ui:fieldDependency:mount_trusted_ca:true"}
331-
TrustedCaConfigMapKey string `json:"mount_trusted_ca_configmap_key,omitempty"`
331+
TrustedCaConfigMapKey *string `json:"mount_trusted_ca_configmap_key,omitempty"`
332332

333333
// Job to reset pulp admin password
334334
AdminPasswordJob PulpJob `json:"admin_password_job,omitempty"`

apis/repo-manager.pulpproject.org/v1/zz_generated.deepcopy.go

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

controllers/deployment.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,10 @@ func (d *CommonDeployment) setVolumes(resources any, pulpcoreType settings.Pulpc
554554
}
555555
volumes = append(volumes, containerTokenSecretVolume)
556556
}
557+
558+
// append the CA configmap to the volumes
559+
volumes = SetCAVolumes(&pulp, volumes)
560+
557561
d.volumes = append([]corev1.Volume(nil), volumes...)
558562
}
559563

@@ -724,6 +728,10 @@ func (d *CommonDeployment) setVolumeMounts(pulp pulpv1.Pulp, pulpcoreType settin
724728
}
725729
volumeMounts = append(volumeMounts, containerTokenSecretMount...)
726730
}
731+
732+
// append the CA configmap to the volumeMounts
733+
volumeMounts = SetCAVolumeMounts(&pulp, volumeMounts)
734+
727735
d.volumeMounts = append([]corev1.VolumeMount(nil), volumeMounts...)
728736
}
729737

controllers/ocp/deployment.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,6 @@ import (
2828
func defaultsForOCPDeployment(deployment *appsv1.Deployment, pulp *pulpv1.Pulp) {
2929
// in OCP we use SCC so there is no need to define PodSecurityContext
3030
deployment.Spec.Template.Spec.SecurityContext = &corev1.PodSecurityContext{}
31-
32-
// get the current volume mount points
33-
volumes := deployment.Spec.Template.Spec.Volumes
34-
volumeMounts := deployment.Spec.Template.Spec.Containers[0].VolumeMounts
35-
36-
// append the CA configmap to the volumes/volumemounts slice
37-
volumes, volumeMounts = MountCASpec(pulp, volumes, volumeMounts)
38-
deployment.Spec.Template.Spec.Volumes = volumes
39-
deployment.Spec.Template.Spec.Containers[0].VolumeMounts = volumeMounts
4031
}
4132

4233
// DeploymentAPIOCP is the pulpcore-api Deployment definition for common OCP clusters

controllers/ocp/utils.go

Lines changed: 0 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package ocp
1818

1919
import (
2020
"context"
21-
"strings"
2221

2322
"github.com/go-logr/logr"
2423
pulpv1 "github.com/pulp/pulp-operator/apis/repo-manager.pulpproject.org/v1"
@@ -102,72 +101,6 @@ func CreateEmptyConfigMap(r client.Client, scheme *runtime.Scheme, ctx context.C
102101
return ctrl.Result{}, nil
103102
}
104103

105-
// MountCASpec adds the trusted-ca bundle into []volume and []volumeMount if pulp.Spec.TrustedCA is true
106-
// On OpenShift: uses the operator-created ConfigMap with CNO injection
107-
// On vanilla K8s: uses a user-specified ConfigMap (which can be managed manually or by trust-manager)
108-
func MountCASpec(pulp *pulpv1.Pulp, volumes []corev1.Volume, volumeMounts []corev1.VolumeMount) ([]corev1.Volume, []corev1.VolumeMount) {
109-
110-
if pulp.Spec.TrustedCa {
111-
var configMapName string
112-
var configMapKey string
113-
114-
// Determine ConfigMap name and key based on configuration
115-
if len(pulp.Spec.TrustedCaConfigMapKey) > 0 {
116-
// Vanilla K8s mode: parse "configmap-name:key" format
117-
// If no separator, assume it's just the ConfigMap name and use the first key in the map
118-
parts := strings.Split(pulp.Spec.TrustedCaConfigMapKey, ":")
119-
if len(parts) == 2 {
120-
configMapName = parts[0]
121-
configMapKey = parts[1]
122-
} else {
123-
// Just ConfigMap name provided, use empty key to get first key in map
124-
configMapName = parts[0]
125-
configMapKey = ""
126-
}
127-
} else {
128-
// OpenShift mode: use the operator-created ConfigMap with CNO injection
129-
configMapName = settings.EmptyCAConfigMapName(pulp.Name)
130-
configMapKey = "ca-bundle.crt"
131-
}
132-
133-
// trustedCAVolume contains the configmap with the custom ca bundle
134-
defaultMode := int32(420)
135-
configMapVolumeSource := &corev1.ConfigMapVolumeSource{
136-
LocalObjectReference: corev1.LocalObjectReference{
137-
Name: configMapName,
138-
},
139-
DefaultMode: &defaultMode,
140-
}
141-
142-
// If a specific key is provided, map it to the expected path
143-
// Otherwise, mount all keys from the ConfigMap (first key will be used)
144-
if configMapKey != "" {
145-
configMapVolumeSource.Items = []corev1.KeyToPath{
146-
{Key: configMapKey, Path: "tls-ca-bundle.pem"},
147-
}
148-
}
149-
150-
trustedCAVolume := corev1.Volume{
151-
Name: "trusted-ca",
152-
VolumeSource: corev1.VolumeSource{
153-
ConfigMap: configMapVolumeSource,
154-
},
155-
}
156-
volumes = append(volumes, trustedCAVolume)
157-
158-
// trustedCAMount defines the mount point of the configmap
159-
// with the custom ca bundle
160-
trustedCAMount := corev1.VolumeMount{
161-
Name: "trusted-ca",
162-
MountPath: "/etc/pki/ca-trust/extracted/pem",
163-
ReadOnly: true,
164-
}
165-
volumeMounts = append(volumeMounts, trustedCAMount)
166-
}
167-
168-
return volumes, volumeMounts
169-
}
170-
171104
// GetRouteHost defines route host based on ingress default cluster domain if no .spec.route_host defined
172105
func GetRouteHost(pulp *pulpv1.Pulp) string {
173106
if len(pulp.Spec.RouteHost) == 0 {

controllers/ocp/utils_test.go

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"testing"
2121

2222
pulpv1 "github.com/pulp/pulp-operator/apis/repo-manager.pulpproject.org/v1"
23+
"github.com/pulp/pulp-operator/controllers"
2324
"github.com/pulp/pulp-operator/controllers/settings"
2425
corev1 "k8s.io/api/core/v1"
2526
)
@@ -35,7 +36,8 @@ func TestMountCASpec_Disabled(t *testing.T) {
3536
volumes := []corev1.Volume{}
3637
volumeMounts := []corev1.VolumeMount{}
3738

38-
resultVolumes, resultVolumeMounts := MountCASpec(pulp, volumes, volumeMounts)
39+
resultVolumes := controllers.SetCAVolumes(pulp, volumes)
40+
resultVolumeMounts := controllers.SetCAVolumeMounts(pulp, volumeMounts)
3941

4042
if len(resultVolumes) != 0 {
4143
t.Errorf("Expected 0 volumes when TrustedCa is false, got %d", len(resultVolumes))
@@ -52,15 +54,16 @@ func TestMountCASpec_OpenShiftMode(t *testing.T) {
5254
pulp := &pulpv1.Pulp{
5355
Spec: pulpv1.PulpSpec{
5456
TrustedCa: true,
55-
TrustedCaConfigMapKey: "", // Empty means OpenShift mode
57+
TrustedCaConfigMapKey: nil, // Empty means OpenShift mode
5658
},
5759
}
5860
pulp.Name = pulpName
5961

6062
volumes := []corev1.Volume{}
6163
volumeMounts := []corev1.VolumeMount{}
6264

63-
resultVolumes, resultVolumeMounts := MountCASpec(pulp, volumes, volumeMounts)
65+
resultVolumes := controllers.SetCAVolumes(pulp, volumes)
66+
resultVolumeMounts := controllers.SetCAVolumeMounts(pulp, volumeMounts)
6467

6568
// Verify we got exactly one volume and one mount
6669
if len(resultVolumes) != 1 {
@@ -117,15 +120,16 @@ func TestMountCASpec_TrustManagerMode(t *testing.T) {
117120
pulp := &pulpv1.Pulp{
118121
Spec: pulpv1.PulpSpec{
119122
TrustedCa: true,
120-
TrustedCaConfigMapKey: configMapName, // Just ConfigMap name, no ":"
123+
TrustedCaConfigMapKey: &configMapName, // Just ConfigMap name, no ":"
121124
},
122125
}
123126
pulp.Name = pulpName
124127

125128
volumes := []corev1.Volume{}
126129
volumeMounts := []corev1.VolumeMount{}
127130

128-
resultVolumes, resultVolumeMounts := MountCASpec(pulp, volumes, volumeMounts)
131+
resultVolumes := controllers.SetCAVolumes(pulp, volumes)
132+
resultVolumeMounts := controllers.SetCAVolumeMounts(pulp, volumeMounts)
129133

130134
// Verify we got exactly one volume and one mount
131135
if len(resultVolumes) != 1 {
@@ -176,15 +180,13 @@ func TestMountCASpec_CustomKey(t *testing.T) {
176180
pulp := &pulpv1.Pulp{
177181
Spec: pulpv1.PulpSpec{
178182
TrustedCa: true,
179-
TrustedCaConfigMapKey: separatorFormat,
183+
TrustedCaConfigMapKey: &separatorFormat,
180184
},
181185
}
182186
pulp.Name = pulpName
183187

184188
volumes := []corev1.Volume{}
185-
volumeMounts := []corev1.VolumeMount{}
186-
187-
resultVolumes, _ := MountCASpec(pulp, volumes, volumeMounts)
189+
resultVolumes := controllers.SetCAVolumes(pulp, volumes)
188190

189191
if len(resultVolumes) != 1 {
190192
t.Fatalf("Expected 1 volume, got %d", len(resultVolumes))
@@ -206,10 +208,11 @@ func TestMountCASpec_CustomKey(t *testing.T) {
206208

207209
// TestMountCASpec_PreservesExistingVolumes verifies that existing volumes are preserved
208210
func TestMountCASpec_PreservesExistingVolumes(t *testing.T) {
211+
configMapName := "my-bund:ca-bundle.crt"
209212
pulp := &pulpv1.Pulp{
210213
Spec: pulpv1.PulpSpec{
211214
TrustedCa: true,
212-
TrustedCaConfigMapKey: "my-bundle:ca-bundle.crt",
215+
TrustedCaConfigMapKey: &configMapName,
213216
},
214217
}
215218
pulp.Name = "test-pulp"
@@ -224,7 +227,8 @@ func TestMountCASpec_PreservesExistingVolumes(t *testing.T) {
224227
{Name: "existing-mount-2"},
225228
}
226229

227-
resultVolumes, resultVolumeMounts := MountCASpec(pulp, existingVolumes, existingVolumeMounts)
230+
resultVolumes := controllers.SetCAVolumes(pulp, existingVolumes)
231+
resultVolumeMounts := controllers.SetCAVolumeMounts(pulp, existingVolumeMounts)
228232

229233
// Should have 3 volumes (2 existing + 1 new)
230234
if len(resultVolumes) != 3 {
@@ -269,15 +273,16 @@ func TestMountCASpec_SeparatorFormat(t *testing.T) {
269273
pulp := &pulpv1.Pulp{
270274
Spec: pulpv1.PulpSpec{
271275
TrustedCa: true,
272-
TrustedCaConfigMapKey: separatorFormat,
276+
TrustedCaConfigMapKey: &separatorFormat,
273277
},
274278
}
275279
pulp.Name = pulpName
276280

277281
volumes := []corev1.Volume{}
278282
volumeMounts := []corev1.VolumeMount{}
279283

280-
resultVolumes, resultVolumeMounts := MountCASpec(pulp, volumes, volumeMounts)
284+
resultVolumes := controllers.SetCAVolumes(pulp, volumes)
285+
resultVolumeMounts := controllers.SetCAVolumeMounts(pulp, volumeMounts)
281286

282287
// Verify we got exactly one volume and one mount
283288
if len(resultVolumes) != 1 {
@@ -361,12 +366,12 @@ func TestMountCASpec_MountPathConsistency(t *testing.T) {
361366
pulp := &pulpv1.Pulp{
362367
Spec: pulpv1.PulpSpec{
363368
TrustedCa: tc.trustedCa,
364-
TrustedCaConfigMapKey: tc.trustedCaConfigMapKey,
369+
TrustedCaConfigMapKey: &tc.trustedCaConfigMapKey,
365370
},
366371
}
367372
pulp.Name = pulpName
368373

369-
_, resultVolumeMounts := MountCASpec(pulp, []corev1.Volume{}, []corev1.VolumeMount{})
374+
resultVolumeMounts := controllers.SetCAVolumeMounts(pulp, []corev1.VolumeMount{})
370375

371376
if len(resultVolumeMounts) != 1 {
372377
t.Fatalf("Expected 1 volumeMount, got %d", len(resultVolumeMounts))

controllers/repo_manager/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ PulpSpec defines the desired state of Pulp
251251
| sa_labels | ServiceAccount.metadata.labels that will be used in Pulp pods. | map[string]string | false |
252252
| sso_secret | Secret where Single Sign-on configuration can be found | string | false |
253253
| mount_trusted_ca | Enable mounting of custom CA certificates. On OpenShift, mounts CA certificates added to the cluster via cluster-wide proxy config. On vanilla Kubernetes with cert-manager's trust-manager, requires mount_trusted_ca_configmap_key to specify the ConfigMap and key containing the CA bundle. Default: false | bool | false |
254-
| mount_trusted_ca_configmap_key | Specifies the ConfigMap and key containing the CA bundle for vanilla Kubernetes clusters. The ConfigMap can be managed manually or kept up to date using cert-manager's trust-manager. Format: \"configmap-name:key\" (e.g., \"vault-ca-defaults-bundle:ca.crt\") Required on vanilla Kubernetes when mount_trusted_ca is true. Optional on OpenShift. | string | false |
254+
| mount_trusted_ca_configmap_key | Specifies the ConfigMap and key containing the CA bundle for vanilla Kubernetes clusters. The ConfigMap can be managed manually or kept up to date using cert-manager's trust-manager. Format: \"configmap-name:key\" (e.g., \"vault-ca-defaults-bundle:ca.crt\") Required on vanilla Kubernetes when mount_trusted_ca is true. Optional on OpenShift. | *string | false |
255255
| admin_password_job | Job to reset pulp admin password | [PulpJob](#pulpjob) | false |
256256
| migration_job | Job to run django migrations | [PulpJob](#pulpjob) | false |
257257
| signing_job | Job to store signing metadata scripts | [PulpJob](#pulpjob) | false |
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package repo_manager
2+
3+
import (
4+
"context"
5+
6+
pulpv1 "github.com/pulp/pulp-operator/apis/repo-manager.pulpproject.org/v1"
7+
"github.com/pulp/pulp-operator/controllers"
8+
corev1 "k8s.io/api/core/v1"
9+
"k8s.io/apimachinery/pkg/types"
10+
ctrl "sigs.k8s.io/controller-runtime"
11+
)
12+
13+
func (r *RepoManagerReconciler) configMapTasks(ctx context.Context, pulp *pulpv1.Pulp) (*ctrl.Result, error) {
14+
15+
needsPulpcoreRestart := false
16+
17+
// if mount_trusted_ca_configmap_key is defined, check if it was modified
18+
if pulp.Spec.TrustedCaConfigMapKey != nil {
19+
trustedCAConfigMap := &corev1.ConfigMap{}
20+
caConfigMapName, _ := controllers.SplitCAConfigMapNameKey(*pulp)
21+
22+
r.Get(ctx, types.NamespacedName{Name: caConfigMapName, Namespace: pulp.Namespace}, trustedCAConfigMap)
23+
if r.caConfigMapChanged(ctx, trustedCAConfigMap) {
24+
needsPulpcoreRestart = true
25+
}
26+
}
27+
28+
// TODO: check pulp-web configmap change
29+
30+
// restart pulpcore pods if any of the configmaps changed
31+
if needsPulpcoreRestart {
32+
r.restartPulpCorePods(ctx, pulp)
33+
return &ctrl.Result{Requeue: true}, nil
34+
}
35+
return nil, nil
36+
}
37+
38+
func (r *RepoManagerReconciler) caConfigMapChanged(ctx context.Context, cm *corev1.ConfigMap) bool {
39+
currentHash := controllers.GetCurrentHash(cm)
40+
calculatedHash := controllers.CalculateHash(cm.Data)
41+
42+
if currentHash == calculatedHash {
43+
return false
44+
}
45+
46+
controllers.SetHashLabel(calculatedHash, cm)
47+
if err := r.Update(ctx, cm); err != nil {
48+
r.RawLogger.Error(err, "Failed to update "+cm.Name+" ConfigMap label!")
49+
}
50+
return true
51+
}

controllers/repo_manager/controller.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,11 @@ func pulpCoreTasks(ctx context.Context, pulp *pulpv1.Pulp, r RepoManagerReconcil
194194
return pulpController, err
195195
}
196196

197+
log.V(1).Info("Running configMap tasks ...")
198+
if pulpController, err := r.configMapTasks(ctx, pulp); pulpController != nil || err != nil {
199+
return pulpController, err
200+
}
201+
197202
log.V(1).Info("Running API tasks")
198203
if pulpController, err := r.pulpApiController(ctx, pulp, log); needsRequeue(err, pulpController) {
199204
return &pulpController, err
@@ -329,6 +334,10 @@ func indexerFunc(obj client.Object) []string {
329334
if customSettings := pulp.Spec.CustomPulpSettings; customSettings != "" {
330335
keys = append(keys, customSettings)
331336
}
337+
if pulp.Spec.TrustedCaConfigMapKey != nil {
338+
caConfigMap, _ := controllers.SplitCAConfigMapNameKey(*pulp)
339+
keys = append(keys, caConfigMap)
340+
}
332341

333342
return keys
334343
}

0 commit comments

Comments
 (0)