Add trusted CA bundle support for OpenShift #1079
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| package ca | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "strings" | ||
|
|
||
| "github.com/spf13/viper" | ||
| corev1 "k8s.io/api/core/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
|
|
||
| v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1" | ||
| "github.com/jaegertracing/jaeger-operator/pkg/util" | ||
| ) | ||
|
|
||
| // Get returns a trusted CA bundle configmap if platform is OpenShift | ||
| func Get(jaeger *v1.Jaeger) *corev1.ConfigMap { | ||
| // Only configure the trusted CA if running in OpenShift | ||
| if viper.GetString("platform") != v1.FlagPlatformOpenShift { | ||
| return nil | ||
| } | ||
|
|
||
| if !deployTrustedCA(jaeger) { | ||
| jaeger.Logger().Debug("CA: Skip deploying the Jaeger instance's trustedCABundle configmap") | ||
| return nil | ||
| } | ||
|
|
||
| jaeger.Logger().Debug("CA: Creating the trustedCABundle configmap") | ||
| trueVar := true | ||
|
|
||
| name := TrustedCAName(jaeger) | ||
| labels := util.Labels(name, "ca-configmap", *jaeger) | ||
| labels["config.openshift.io/inject-trusted-cabundle"] = "true" | ||
|
|
||
| // See https://docs.openshift.com/container-platform/4.4/networking/configuring-a-custom-pki.html#certificate-injection-using-operators_configuring-a-custom-pki | ||
| return &corev1.ConfigMap{ | ||
| TypeMeta: metav1.TypeMeta{ | ||
| APIVersion: "v1", | ||
| Kind: "ConfigMap", | ||
| }, | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: name, | ||
| Namespace: jaeger.Namespace, | ||
| Labels: labels, | ||
| OwnerReferences: []metav1.OwnerReference{{ | ||
| APIVersion: jaeger.APIVersion, | ||
| Kind: jaeger.Kind, | ||
| Name: jaeger.Name, | ||
| UID: jaeger.UID, | ||
| Controller: &trueVar, | ||
| }}, | ||
| }, | ||
| Data: map[string]string{ | ||
| "ca-bundle.crt": "", | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| // Update will modify the supplied common spec, to include | ||
| // trusted CA bundle volume and volumeMount, if running on OpenShift | ||
| func Update(jaeger *v1.Jaeger, commonSpec *v1.JaegerCommonSpec) { | ||
| // Only configure the trusted CA if running in OpenShift | ||
| if viper.GetString("platform") != v1.FlagPlatformOpenShift { | ||
| return | ||
| } | ||
|
|
||
| if !deployTrustedCA(jaeger) { | ||
| jaeger.Logger().Debug("CA: Skip adding the Jaeger instance's trustedCABundle volume") | ||
| return | ||
| } | ||
|
|
||
| volume := corev1.Volume{ | ||
| Name: TrustedCAName(jaeger), | ||
| VolumeSource: corev1.VolumeSource{ | ||
| ConfigMap: &corev1.ConfigMapVolumeSource{ | ||
| LocalObjectReference: corev1.LocalObjectReference{ | ||
| Name: TrustedCAName(jaeger), | ||
| }, | ||
| Items: []corev1.KeyToPath{{ | ||
| Key: "ca-bundle.crt", | ||
| Path: "tls-ca-bundle.pem", | ||
| }}, | ||
| }, | ||
| }, | ||
| } | ||
| commonSpec.Volumes = append(commonSpec.Volumes, volume) | ||
|
|
||
| volumeMount := corev1.VolumeMount{ | ||
| Name: TrustedCAName(jaeger), | ||
| MountPath: "/etc/pki/ca-trust/extracted/pem", | ||
| ReadOnly: true, | ||
| } | ||
| commonSpec.VolumeMounts = append(commonSpec.VolumeMounts, volumeMount) | ||
| } | ||
|
|
||
| func deployTrustedCA(jaeger *v1.Jaeger) bool { | ||
| for _, vm := range jaeger.Spec.JaegerCommonSpec.VolumeMounts { | ||
| if strings.HasPrefix(vm.MountPath, "/etc/pki/ca-trust/extracted/pem") { | ||
|
There was a problem hiding this comment. @jpkrohling Is this a reason approach to disable adding the custom CA bundle/volumes/mounts, if (for example) Service Mesh explicitly adds them into the Jaeger CR? Or do we need a better approach? There was a problem hiding this comment. I'll think about it some more, but I think it's good enough. In any case, there's no way to be 100% sure that there won't be a clash, as the path inside the volumes themselves might end up conflicting ( Edit: actually, we could compare the paths, and see if the CA bundle volume mount would clash with an existing volume mount ( |
||
| // Volume Mount already exists, so don't create specific | ||
| // one for this Jaeger instance | ||
| return false | ||
| } | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| // TrustedCAName returns the name of the trusted CA | ||
| func TrustedCAName(jaeger *v1.Jaeger) string { | ||
| return fmt.Sprintf("%s-trusted-ca", jaeger.Name) | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| package ca | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/spf13/viper" | ||
| "github.com/stretchr/testify/assert" | ||
| corev1 "k8s.io/api/core/v1" | ||
| "k8s.io/apimachinery/pkg/types" | ||
|
|
||
| v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1" | ||
| ) | ||
|
|
||
| func TestGetWithoutTrustedCA(t *testing.T) { | ||
| viper.Set("platform", "other") | ||
| defer viper.Reset() | ||
|
|
||
| jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestGetWithoutTrustedCA"}) | ||
|
There was a problem hiding this comment. I think we started using a generic name some time ago. Was it changed recently, to use individual names for the instances in each test? If not, there's no need to change this now. |
||
|
|
||
| cm := Get(jaeger) | ||
| assert.Nil(t, cm) | ||
| } | ||
|
|
||
| func TestGetWithTrustedCA(t *testing.T) { | ||
| viper.Set("platform", v1.FlagPlatformOpenShift) | ||
| defer viper.Reset() | ||
|
|
||
| jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestGetWithTrustedCA"}) | ||
|
|
||
| cm := Get(jaeger) | ||
| assert.NotNil(t, cm) | ||
| assert.Equal(t, "true", cm.Labels["config.openshift.io/inject-trusted-cabundle"]) | ||
| assert.Equal(t, "", cm.Data["ca-bundle.crt"]) | ||
| } | ||
|
|
||
| func TestGetWithExistingTrustedCA(t *testing.T) { | ||
| viper.Set("platform", v1.FlagPlatformOpenShift) | ||
| defer viper.Reset() | ||
|
|
||
| jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestGetWithExistingTrustedCA"}) | ||
| jaeger.Spec.JaegerCommonSpec.VolumeMounts = []corev1.VolumeMount{{ | ||
| MountPath: "/etc/pki/ca-trust/extracted/pem", | ||
| Name: "ExistingTrustedCA", | ||
| }} | ||
|
|
||
| cm := Get(jaeger) | ||
| assert.Nil(t, cm) | ||
| } | ||
|
|
||
| func TestUpdateWithoutTrustedCA(t *testing.T) { | ||
| viper.Set("platform", "other") | ||
| defer viper.Reset() | ||
|
|
||
| jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestUpdateWithoutTrustedCA"}) | ||
|
|
||
| commonSpec := v1.JaegerCommonSpec{} | ||
|
|
||
| Update(jaeger, &commonSpec) | ||
| assert.Len(t, commonSpec.Volumes, 0) | ||
| assert.Len(t, commonSpec.VolumeMounts, 0) | ||
| } | ||
|
|
||
| func TestUpdateWithTrustedCA(t *testing.T) { | ||
| viper.Set("platform", v1.FlagPlatformOpenShift) | ||
| defer viper.Reset() | ||
|
|
||
| jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestUpdateWithTrustedCA"}) | ||
|
|
||
| commonSpec := v1.JaegerCommonSpec{} | ||
|
|
||
| Update(jaeger, &commonSpec) | ||
| assert.Len(t, commonSpec.Volumes, 1) | ||
| assert.Equal(t, commonSpec.Volumes[0].Name, TrustedCAName(jaeger)) | ||
| assert.Len(t, commonSpec.VolumeMounts, 1) | ||
| assert.Equal(t, commonSpec.VolumeMounts[0].Name, TrustedCAName(jaeger)) | ||
| } | ||
|
|
||
| func TestUpdateWithExistingTrustedCA(t *testing.T) { | ||
| viper.Set("platform", v1.FlagPlatformOpenShift) | ||
| defer viper.Reset() | ||
|
|
||
| jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestUpdateWithExistingTrustedCA"}) | ||
| jaeger.Spec.JaegerCommonSpec.VolumeMounts = []corev1.VolumeMount{{ | ||
| MountPath: "/etc/pki/ca-trust/extracted/pem", | ||
| Name: "ExistingTrustedCA", | ||
| }} | ||
|
|
||
| commonSpec := v1.JaegerCommonSpec{} | ||
|
|
||
| Update(jaeger, &commonSpec) | ||
| assert.Len(t, commonSpec.Volumes, 0) | ||
| assert.Len(t, commonSpec.VolumeMounts, 0) | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests are showing that this is fine, but I wonder if there's a reason to have this as empty first? Also, would the reconciliation phase revert this to empty? If so, OpenShift seems to be refilling this quite fast (possibly with a webhook, before changes are actually applied).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be an idea to check with
kubectl get -w configmap -o yaml- could be a potencial race?