Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MGMT-20207: avoid adding system CA bundle to AdditionalTrustBundle #7448

Merged
merged 1 commit into from
Apr 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion internal/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ const (

consoleUrlPrefix = "https://console-openshift-console.apps"

MirrorRegistriesCertificateFile = "tls-ca-bundle.pem"
SystemCertificateBundle = "tls-ca-bundle.pem"
SystemCertificateBundlePath = "/etc/pki/ca-trust/extracted/pem/" + SystemCertificateBundle

MirrorRegistriesCertificateFile = "user-registry-ca-bundle.pem"
MirrorRegistriesCertificatePath = "/etc/pki/ca-trust/extracted/pem/" + MirrorRegistriesCertificateFile
MirrorRegistriesConfigDir = "/etc/containers"
MirrorRegistriesConfigFile = "registries.conf"
Expand Down
31 changes: 26 additions & 5 deletions internal/controller/controllers/agentserviceconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1854,7 +1854,7 @@ func newAssistedServiceDeployment(ctx context.Context, log logrus.FieldLogger, a
ConfigMap: &corev1.ConfigMapVolumeSource{
Items: []corev1.KeyToPath{{
Key: caBundleKey,
Path: common.MirrorRegistriesCertificateFile,
Path: common.SystemCertificateBundle,
}},
LocalObjectReference: corev1.LocalObjectReference{
Name: assistedCAConfigMapName,
Expand All @@ -1868,8 +1868,8 @@ func newAssistedServiceDeployment(ctx context.Context, log logrus.FieldLogger, a
corev1.VolumeMount{Name: "tls-certs", MountPath: "/etc/assisted-tls-config"},
corev1.VolumeMount{
Name: "trusted-ca-certs",
MountPath: common.MirrorRegistriesCertificatePath,
SubPath: common.MirrorRegistriesCertificateFile,
MountPath: common.SystemCertificateBundlePath,
SubPath: common.SystemCertificateBundle,
},
)
healthCheckScheme = corev1.URISchemeHTTPS
Expand Down Expand Up @@ -1983,7 +1983,7 @@ func newAssistedServiceDeployment(ctx context.Context, log logrus.FieldLogger, a
return nil, nil, pkgerror.Wrapf(err, "Unable to mark mirror configmap for backup")
}

volume := corev1.Volume{
registriesConfVolume := corev1.Volume{
Name: mirrorRegistryConfigVolume,
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
Expand All @@ -1999,16 +1999,37 @@ func newAssistedServiceDeployment(ctx context.Context, log logrus.FieldLogger, a
},
}

caBundleVolume := corev1.Volume{
Name: mirrorRegistryCertBundleVolume,
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: *asc.spec.MirrorRegistryRef,
DefaultMode: swag.Int32(420),
Items: []corev1.KeyToPath{
{
Key: mirrorRegistryRefCertKey,
Path: common.MirrorRegistriesCertificateFile,
},
},
},
},
}

serviceContainer.VolumeMounts = append(
serviceContainer.VolumeMounts,
corev1.VolumeMount{
Name: mirrorRegistryConfigVolume,
MountPath: common.MirrorRegistriesConfigDir,
},
corev1.VolumeMount{
Name: mirrorRegistryCertBundleVolume,
MountPath: common.MirrorRegistriesCertificatePath,
SubPath: common.MirrorRegistriesCertificateFile,
},
)

// add our mirror registry config to volumes
volumes = append(volumes, volume)
volumes = append(volumes, registriesConfVolume, caBundleVolume)
}

deploymentLabels := map[string]string{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1699,7 +1699,7 @@ var _ = Describe("ensureAssistedServiceDeployment", func() {

found := &appsv1.Deployment{}
Expect(ascr.Client.Get(ctx, types.NamespacedName{Name: serviceName, Namespace: testNamespace}, found)).To(Succeed())
Expect(found.Spec.Template.Spec.Volumes).To(HaveLen(6))
Expect(found.Spec.Template.Spec.Volumes).To(HaveLen(7))
Expect(found.Spec.Template.Spec.Containers[0].VolumeMounts).Should(ContainElement(
corev1.VolumeMount{
Name: mirrorRegistryConfigVolume,
Expand Down Expand Up @@ -1759,15 +1759,15 @@ var _ = Describe("ensureAssistedServiceDeployment", func() {

found := &appsv1.Deployment{}
Expect(ascr.Client.Get(ctx, types.NamespacedName{Name: serviceName, Namespace: testNamespace}, found)).To(Succeed())
Expect(found.Spec.Template.Spec.Volumes).To(HaveLen(6))
Expect(found.Spec.Template.Spec.Volumes).To(HaveLen(7))
Expect(found.Spec.Template.Spec.Volumes).To(ContainElement(
corev1.Volume{
Name: "trusted-ca-certs",
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
Items: []corev1.KeyToPath{{
Key: caBundleKey,
Path: common.MirrorRegistriesCertificateFile,
Path: common.SystemCertificateBundle,
}},
LocalObjectReference: corev1.LocalObjectReference{
Name: assistedCAConfigMapName,
Expand All @@ -1777,6 +1777,23 @@ var _ = Describe("ensureAssistedServiceDeployment", func() {
},
},
))
Expect(found.Spec.Template.Spec.Volumes).To(ContainElement(
corev1.Volume{
Name: mirrorRegistryCertBundleVolume,
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
Items: []corev1.KeyToPath{{
Key: mirrorRegistryRefCertKey,
Path: common.MirrorRegistriesCertificateFile,
}},
LocalObjectReference: corev1.LocalObjectReference{
Name: testMirrorRegConfigmapName,
},
DefaultMode: swag.Int32(420),
},
},
},
))

Expect(found.Spec.Template.Spec.Containers[0].VolumeMounts).Should(ContainElement(
corev1.VolumeMount{
Expand All @@ -1787,6 +1804,13 @@ var _ = Describe("ensureAssistedServiceDeployment", func() {
Expect(found.Spec.Template.Spec.Containers[0].VolumeMounts).Should(ContainElement(
corev1.VolumeMount{
Name: "trusted-ca-certs",
MountPath: common.SystemCertificateBundlePath,
SubPath: common.SystemCertificateBundle,
}),
)
Expect(found.Spec.Template.Spec.Containers[0].VolumeMounts).Should(ContainElement(
corev1.VolumeMount{
Name: mirrorRegistryCertBundleVolume,
MountPath: common.MirrorRegistriesCertificatePath,
SubPath: common.MirrorRegistriesCertificateFile,
}),
Expand Down
1 change: 1 addition & 0 deletions internal/controller/controllers/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const (
mirrorRegistryRefCertKey = caBundleKey
mirrorRegistryRefRegistryConfKey = "registries.conf"
mirrorRegistryConfigVolume = "mirror-registry-config"
mirrorRegistryCertBundleVolume = "mirror-registry-ca-bundle"
WatchResourceLabel = "agent-install.openshift.io/watch"
WatchResourceValue = "true"
BackupLabel = "cluster.open-cluster-management.io/backup"
Expand Down
9 changes: 8 additions & 1 deletion pkg/mirrorregistries/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ type ServiceMirrorRegistriesConfigBuilder interface {
type mirrorRegistriesConfigBuilder struct {
MirrorRegistriesConfigPath string
MirrorRegistriesCertificatePath string
SystemCertificateBundlePath string
}

func New() ServiceMirrorRegistriesConfigBuilder {
return &mirrorRegistriesConfigBuilder{
MirrorRegistriesConfigPath: common.MirrorRegistriesConfigPath,
MirrorRegistriesCertificatePath: common.MirrorRegistriesCertificatePath,
SystemCertificateBundlePath: common.SystemCertificateBundlePath,
}
}

Expand Down Expand Up @@ -59,7 +61,12 @@ func (m *mirrorRegistriesConfigBuilder) IsMirrorRegistriesConfigured() bool {
// the mirror registries are not configured.
// empty dir is due to the way we mao configmap in the assisted-service pod
func (m *mirrorRegistriesConfigBuilder) GetMirrorCA() ([]byte, error) {
return os.ReadFile(m.MirrorRegistriesCertificatePath)
bytes, err := os.ReadFile(m.MirrorRegistriesCertificatePath)
if err != nil {
// fallback to tls-ca-bundle.pem (used by ABI)
return os.ReadFile(m.SystemCertificateBundlePath)
}
return bytes, nil
}

// GetMirrorRegistries returns error if the file is not present, which will also indicate that
Expand Down
50 changes: 50 additions & 0 deletions pkg/mirrorregistries/mirror_registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,56 @@ var _ = Describe("Generator tests", func() {
})
})

var _ = Describe("GetMirrorCA", func() {
var (
tempRegistryCA *os.File
tempSystemCA *os.File
m mirrorRegistriesConfigBuilder
err error
)

BeforeEach(func() {
tempRegistryCA, err = os.CreateTemp(os.TempDir(), "user-registry-ca-bundle.pem")
Expect(err).NotTo(HaveOccurred())
_, _ = tempRegistryCA.WriteString("registry CA bundle")

tempSystemCA, err = os.CreateTemp(os.TempDir(), "tls-ca-bundle.pem")
Expect(err).NotTo(HaveOccurred())
_, _ = tempSystemCA.WriteString("system CA bundle")
})

It("should return registry bundle when registry bundle exists", func() {
m = mirrorRegistriesConfigBuilder{
MirrorRegistriesCertificatePath: tempRegistryCA.Name(),
}

result, err := m.GetMirrorCA()
Expect(err).ShouldNot(HaveOccurred())
Expect(result).To(Equal([]byte("registry CA bundle")))
})

It("should return the system CA bundle when registry bundle doesn't exist", func() {
m = mirrorRegistriesConfigBuilder{
MirrorRegistriesCertificatePath: "/tmp/does-not-exist",
SystemCertificateBundlePath: tempSystemCA.Name(),
}

result, err := m.GetMirrorCA()
Expect(err).ShouldNot(HaveOccurred())
Expect(result).To(Equal([]byte("system CA bundle")))
})

It("should return an error if both files don't exist", func() {
m = mirrorRegistriesConfigBuilder{
MirrorRegistriesCertificatePath: "/tmp/does-not-exist",
SystemCertificateBundlePath: "/tmp/does-not-exist",
}

result, err := m.GetMirrorCA()
Expect(err).Should(HaveOccurred())
Expect(result).To(BeNil())
})
})
})

const (
Expand Down