From 960a47a9d70c1ebbd63551bb4bcf1ab7b9d9223a Mon Sep 17 00:00:00 2001 From: Mykola Morhun Date: Wed, 11 Nov 2020 14:25:01 +0200 Subject: [PATCH 1/6] Draft implementation Signed-off-by: Mykola Morhun --- pkg/controller/che/che_controller.go | 16 +++- pkg/deploy/configmap.go | 3 + pkg/deploy/tls.go | 109 +++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 1 deletion(-) diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index 4efc06a4b8..ca0de535fb 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -457,6 +457,13 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e } } + // Make sure that CA certificates from all marked config maps are merged into single config map to be propageted to Che components + _, err = deploy.SyncAdditionalCACertsConfigMapToCluster(instance, deployContext) + if err != nil { + logrus.Errorf("Error updating additional CA config map: %v", err) + return reconcile.Result{}, err + } + // Get custom ConfigMap // if it exists, add the data into CustomCheProperties customConfigMap := &corev1.ConfigMap{} @@ -1033,6 +1040,7 @@ func getServerExposingServiceName(cr *orgv1.CheCluster) string { return deploy.CheServiceName } +// isTrustedBundleConfigMap detects whether given config map is the config map with additional CA certificates to be trusted by Che func isTrustedBundleConfigMap(mgr manager.Manager, obj handler.MapObject) (bool, reconcile.Request) { checlusters := &orgv1.CheClusterList{} if err := mgr.GetClient().List(context.TODO(), checlusters, &client.ListOptions{}); err != nil { @@ -1043,8 +1051,14 @@ func isTrustedBundleConfigMap(mgr manager.Manager, obj handler.MapObject) (bool, return false, reconcile.Request{} } + // Check if config map is the config map from CR if checlusters.Items[0].Spec.Server.ServerTrustStoreConfigMapName != obj.Meta.GetName() { - return false, reconcile.Request{} + // No, it is not form CR + // Check for labels + if value, exists := obj.Meta.GetLabels()[deploy.CheCACertsConfigMapLabelKey]; !exists || value != deploy.CheCACertsConfigMapLabelValue { + // Labels do not match + return false, reconcile.Request{} + } } return true, reconcile.Request{ diff --git a/pkg/deploy/configmap.go b/pkg/deploy/configmap.go index ab71f0fbfa..a4d233da66 100644 --- a/pkg/deploy/configmap.go +++ b/pkg/deploy/configmap.go @@ -26,6 +26,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) +// SyncConfigMapToCluster makes sure that given config map spec is actual func SyncConfigMapToCluster(deployContext *DeployContext, specConfigMap *corev1.ConfigMap) (*corev1.ConfigMap, error) { clusterConfigMap, err := GetClusterConfigMap(specConfigMap.Name, specConfigMap.Namespace, deployContext.ClusterAPI.Client) if err != nil { @@ -50,6 +51,7 @@ func SyncConfigMapToCluster(deployContext *DeployContext, specConfigMap *corev1. return clusterConfigMap, nil } +// GetSpecConfigMap returns config map spec template func GetSpecConfigMap( deployContext *DeployContext, name string, @@ -79,6 +81,7 @@ func GetSpecConfigMap( return configMap, nil } +// GetClusterConfigMap reads config map from cluster func GetClusterConfigMap(name string, namespace string, client runtimeClient.Client) (*corev1.ConfigMap, error) { configMap := &corev1.ConfigMap{} namespacedName := types.NamespacedName{ diff --git a/pkg/deploy/tls.go b/pkg/deploy/tls.go index 2a2412478f..7a6cd776d8 100644 --- a/pkg/deploy/tls.go +++ b/pkg/deploy/tls.go @@ -18,9 +18,11 @@ import ( "encoding/pem" stderrors "errors" "net/http" + "reflect" "strings" "time" + orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1" "github.com/eclipse/che-operator/pkg/util" routev1 "github.com/openshift/api/route/v1" "github.com/sirupsen/logrus" @@ -28,7 +30,10 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/api/extensions/v1beta1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -42,6 +47,15 @@ const ( CheTLSJobComponentName = "che-create-tls-secret-job" CheTLSSelfSignedCertificateSecretName = "self-signed-certificate" DefaultCheTLSSecretName = "che-tls" + + // CheAllCACertsConfigMapName is the name of config map which contains all additional trusted by Che TLS CA certificates + CheAllCACertsConfigMapName = "che-ca-certs-merged" + // CheCACertsConfigMapLabelKey is the label key which marks config map with additional CA certificates + CheCACertsConfigMapLabelKey = "che-ca-certs" + // CheCACertsConfigMapLabelKey is the label value which marks config map with additional CA certificates + CheCACertsConfigMapLabelValue = "true" + // CheMergedCAConfigMapRevisionsLabelKey is label name which holds versions of included config maps in format: cm-name1=ver1,cm-name2=ver2 + CheMergedCAConfigMapRevisionsLabelKey = "included-cm" ) // IsSelfSignedCertificateUsed detects whether endpoints are/should be secured by self-signed certificate. @@ -460,3 +474,98 @@ func deleteJob(deployContext *DeployContext, job *batchv1.Job) { logrus.Errorf("Error deleting job: '%s', error: %v", CheTLSJobName, err) } } + +// SyncAdditionalCACertsConfigMapToCluster makes sure that additional CA certs config map is up to date if any +func SyncAdditionalCACertsConfigMapToCluster(cr *orgv1.CheCluster, deployContext *DeployContext) (*corev1.ConfigMap, error) { + // Get all source config maps, if any + caConfigMaps, err := getCACertsConfigMaps(deployContext) + if err != nil { + return nil, err + } + if len(cr.Spec.Server.ServerTrustStoreConfigMapName) > 0 { + crConfigMap := &corev1.ConfigMap{} + err := deployContext.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Namespace: deployContext.CheCluster.Namespace, Name: cr.Spec.Server.ServerTrustStoreConfigMapName}, crConfigMap) + if err != nil { + return nil, err + } + caConfigMaps = append(caConfigMaps, *crConfigMap) + } + + mergedCAConfigMap := &corev1.ConfigMap{} + err = deployContext.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Namespace: deployContext.CheCluster.Namespace, Name: CheAllCACertsConfigMapName}, mergedCAConfigMap) + if err == nil { + // Merged config map exists. Check if it up to date. + caConfigMapsCurrentRevisions := make(map[string]string) + for _, cm := range caConfigMaps { + caConfigMapsCurrentRevisions[cm.Name] = cm.ResourceVersion + } + + caConfigMapsCachedRevisions := make(map[string]string) + if revisions, exists := mergedCAConfigMap.ObjectMeta.Labels[CheMergedCAConfigMapRevisionsLabelKey]; exists { + for _, cmNameRevision := range strings.Split(revisions, ",") { + nameRevision := strings.Split(cmNameRevision, "=") + if len(nameRevision) != 2 { + // The label value is invalid, recreate merged config map + break + } + caConfigMapsCachedRevisions[nameRevision[0]] = nameRevision[1] + } + } + + if reflect.DeepEqual(caConfigMapsCurrentRevisions, caConfigMapsCachedRevisions) { + // Existing merged config map is up to date, do nothing + return mergedCAConfigMap, nil + } + } else { + if !errors.IsNotFound(err) { + return nil, err + } + // Merged config map doesn't exist. Create it. + } + + // Merged config map is out of date or doesn't exist + // Merge all config maps into single one to mount inside Che components and workspaces + data := make(map[string]string) + revisions := "" + for _, cm := range caConfigMaps { + // Copy data + for key, dataRecord := range cm.Data { + data[cm.ObjectMeta.Name+"."+key] = dataRecord + } + + // Save source config map revision + if revisions != "" { + revisions += "," + } + revisions += cm.ObjectMeta.Name + "=" + cm.ObjectMeta.ResourceVersion + } + + mergedCAConfigMapSpec, err := GetSpecConfigMap(deployContext, CheAllCACertsConfigMapName, data) + if err != nil { + return nil, err + } + mergedCAConfigMapSpec.ObjectMeta.Labels[CheMergedCAConfigMapRevisionsLabelKey] = revisions + + logrus.Infof("Updating additional CA certs config map: %s", CheAllCACertsConfigMapName) + mergedCAConfigMap, err = SyncConfigMapToCluster(deployContext, mergedCAConfigMapSpec) + if err != nil { + return nil, err + } + return mergedCAConfigMap, nil +} + +// getCACertsConfigMaps returns list of config maps with additional CA certificates that should be trusted by Che +// The selection is based on the specific label +func getCACertsConfigMaps(deployContext *DeployContext) ([]corev1.ConfigMap, error) { + CACertsConfigMapList := &corev1.ConfigMapList{} + + labelSelectorRequirement, _ := labels.NewRequirement(CheCACertsConfigMapLabelKey, selection.Equals, []string{CheCACertsConfigMapLabelValue}) + listOptions := &client.ListOptions{ + LabelSelector: labels.NewSelector().Add(*labelSelectorRequirement), + } + if err := deployContext.ClusterAPI.Client.List(context.TODO(), listOptions, CACertsConfigMapList); err == nil { + return nil, err + } + + return CACertsConfigMapList.Items, nil +} From 73c36d55dbe55172e4032cef5625f647ef0df260 Mon Sep 17 00:00:00 2001 From: Mykola Morhun Date: Thu, 12 Nov 2020 12:56:17 +0200 Subject: [PATCH 2/6] Implement CA certs sources merge and propagate resulting config map to Che server Signed-off-by: Mykola Morhun --- pkg/controller/che/che_controller.go | 7 +++++- pkg/controller/che/proxy.go | 4 +-- pkg/deploy/configmap.go | 12 ++++++--- .../identity-provider/deployment_keycloak.go | 14 +++++------ pkg/deploy/server/che_configmap.go | 2 +- pkg/deploy/server/configmap_cert.go | 11 -------- pkg/deploy/server/deployment_che.go | 14 +++++------ pkg/deploy/tls.go | 25 +++++++++++++++---- 8 files changed, 49 insertions(+), 40 deletions(-) diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index ca0de535fb..47d08a32ef 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -458,11 +458,16 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e } // Make sure that CA certificates from all marked config maps are merged into single config map to be propageted to Che components - _, err = deploy.SyncAdditionalCACertsConfigMapToCluster(instance, deployContext) + cm, err := deploy.SyncAdditionalCACertsConfigMapToCluster(instance, deployContext) if err != nil { logrus.Errorf("Error updating additional CA config map: %v", err) return reconcile.Result{}, err } + if cm == nil && !tests { + // Config map update is in progress + // Return and do not force reconcile. When update finishes it will trigger reconcile loop. + return reconcile.Result{}, err + } // Get custom ConfigMap // if it exists, add the data into CustomCheProperties diff --git a/pkg/controller/che/proxy.go b/pkg/controller/che/proxy.go index 6c89e0152d..82672a74a8 100644 --- a/pkg/controller/che/proxy.go +++ b/pkg/controller/che/proxy.go @@ -13,10 +13,10 @@ package che import ( "context" - "github.com/eclipse/che-operator/pkg/deploy/server" orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1" "github.com/eclipse/che-operator/pkg/deploy" + "github.com/eclipse/che-operator/pkg/deploy/server" "github.com/eclipse/che-operator/pkg/util" configv1 "github.com/openshift/api/config/v1" "k8s.io/apimachinery/pkg/types" @@ -36,7 +36,7 @@ func (r *ReconcileChe) getProxyConfiguration(checluster *orgv1.CheCluster) (*dep // If proxy configuration exists in CR then cluster wide proxy configuration is ignored // otherwise cluster wide proxy configuration is used and non proxy hosts - // are merted with defined ones in CR + // are merged with defined ones in CR if proxy.HttpProxy == "" && clusterProxy.Status.HTTPProxy != "" { proxy, err = deploy.ReadClusterWideProxyConfiguration(clusterProxy, proxy.NoProxy) if err != nil { diff --git a/pkg/deploy/configmap.go b/pkg/deploy/configmap.go index a4d233da66..ff209225f3 100644 --- a/pkg/deploy/configmap.go +++ b/pkg/deploy/configmap.go @@ -26,7 +26,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) -// SyncConfigMapToCluster makes sure that given config map spec is actual +// SyncConfigMapToCluster makes sure that given config map spec is actual. +// It compares config map data and labels. +// If returned config map is nil then it means that the config map update is in progress and reconcile loop probably should be restarted. func SyncConfigMapToCluster(deployContext *DeployContext, specConfigMap *corev1.ConfigMap) (*corev1.ConfigMap, error) { clusterConfigMap, err := GetClusterConfigMap(specConfigMap.Name, specConfigMap.Namespace, deployContext.ClusterAPI.Client) if err != nil { @@ -39,11 +41,13 @@ func SyncConfigMapToCluster(deployContext *DeployContext, specConfigMap *corev1. return nil, err } - diff := cmp.Diff(clusterConfigMap.Data, specConfigMap.Data) - if len(diff) > 0 { + dataDiff := cmp.Diff(clusterConfigMap.Data, specConfigMap.Data) + labelsDiff := cmp.Diff(clusterConfigMap.ObjectMeta.Labels, specConfigMap.ObjectMeta.Labels) + if len(dataDiff) > 0 || len(labelsDiff) > 0 { logrus.Infof("Updating existing object: %s, name: %s", specConfigMap.Kind, specConfigMap.Name) - fmt.Printf("Difference:\n%s", diff) + fmt.Printf("Difference:\n%s\n%s", dataDiff, labelsDiff) clusterConfigMap.Data = specConfigMap.Data + clusterConfigMap.ObjectMeta.Labels = specConfigMap.ObjectMeta.Labels err := deployContext.ClusterAPI.Client.Update(context.TODO(), clusterConfigMap) return nil, err } diff --git a/pkg/deploy/identity-provider/deployment_keycloak.go b/pkg/deploy/identity-provider/deployment_keycloak.go index 53d298cab8..0938a05ac7 100644 --- a/pkg/deploy/identity-provider/deployment_keycloak.go +++ b/pkg/deploy/identity-provider/deployment_keycloak.go @@ -104,7 +104,7 @@ func getSpecKeycloakDeployment( } } - cmResourceVersions := server.GetTrustStoreConfigMapVersion(deployContext) + cmResourceVersions := deploy.GetAdditionalCACertsConfigMapVersion(deployContext) terminationGracePeriodSeconds := int64(30) cheCertSecretVersion := getSecretResourceVersion("self-signed-certificate", deployContext.CheCluster.Namespace, deployContext.ClusterAPI) openshiftApiCertSecretVersion := getSecretResourceVersion("openshift-api-crt", deployContext.CheCluster.Namespace, deployContext.ClusterAPI) @@ -134,14 +134,12 @@ func getSpecKeycloakDeployment( customPublicCertsDir := "/public-certs" customPublicCertsVolumeSource := corev1.VolumeSource{} - if deployContext.CheCluster.Spec.Server.ServerTrustStoreConfigMapName != "" { - customPublicCertsVolumeSource = corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: deployContext.CheCluster.Spec.Server.ServerTrustStoreConfigMapName, - }, + customPublicCertsVolumeSource = corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: deploy.CheAllCACertsConfigMapName, }, - } + }, } customPublicCertsVolume := corev1.Volume{ Name: "che-public-certs", diff --git a/pkg/deploy/server/che_configmap.go b/pkg/deploy/server/che_configmap.go index 1e52e6a6ba..8eb84b65f6 100644 --- a/pkg/deploy/server/che_configmap.go +++ b/pkg/deploy/server/che_configmap.go @@ -213,7 +213,7 @@ func GetCheConfigMapData(deployContext *deploy.DeployContext) (cheEnv map[string CheServerSecureExposerJwtProxyImage: deploy.DefaultCheServerSecureExposerJwtProxyImage(deployContext.CheCluster), CheJGroupsKubernetesLabels: cheLabels, CheMetricsEnabled: cheMetrics, - CheTrustedCABundlesConfigMap: deployContext.CheCluster.Spec.Server.ServerTrustStoreConfigMapName, + CheTrustedCABundlesConfigMap: deploy.CheAllCACertsConfigMapName, ServerStrategy: ingressStrategy, WorkspaceExposure: workspaceExposure, SingleHostGatewayConfigMapLabels: singleHostGatewayConfigMapLabels, diff --git a/pkg/deploy/server/configmap_cert.go b/pkg/deploy/server/configmap_cert.go index ab816fb0c8..74f16d953b 100644 --- a/pkg/deploy/server/configmap_cert.go +++ b/pkg/deploy/server/configmap_cert.go @@ -54,14 +54,3 @@ func SyncTrustStoreConfigMapToCluster(deployContext *deploy.DeployContext) (*cor return clusterConfigMap, nil } - -func GetTrustStoreConfigMapVersion(deployContext *deploy.DeployContext) string { - if deployContext.CheCluster.Spec.Server.ServerTrustStoreConfigMapName != "" { - trustStoreConfigMap, _ := deploy.GetClusterConfigMap(deployContext.CheCluster.Spec.Server.ServerTrustStoreConfigMapName, deployContext.CheCluster.Namespace, deployContext.ClusterAPI.Client) - if trustStoreConfigMap != nil { - return trustStoreConfigMap.ResourceVersion - } - } - - return "" -} diff --git a/pkg/deploy/server/deployment_che.go b/pkg/deploy/server/deployment_che.go index 2ad6c73171..692bb0b864 100644 --- a/pkg/deploy/server/deployment_che.go +++ b/pkg/deploy/server/deployment_che.go @@ -57,7 +57,7 @@ func getSpecCheDeployment(deployContext *deploy.DeployContext) (*appsv1.Deployme } cmResourceVersions := GetCheConfigMapVersion(deployContext) - cmResourceVersions += "," + GetTrustStoreConfigMapVersion(deployContext) + cmResourceVersions += "," + deploy.GetAdditionalCACertsConfigMapVersion(deployContext) terminationGracePeriodSeconds := int64(30) cheFlavor := deploy.DefaultCheFlavor(deployContext.CheCluster) @@ -69,14 +69,12 @@ func getSpecCheDeployment(deployContext *deploy.DeployContext) (*appsv1.Deployme Value: "", } customPublicCertsVolumeSource := corev1.VolumeSource{} - if deployContext.CheCluster.Spec.Server.ServerTrustStoreConfigMapName != "" { - customPublicCertsVolumeSource = corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: deployContext.CheCluster.Spec.Server.ServerTrustStoreConfigMapName, - }, + customPublicCertsVolumeSource = corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: deploy.CheAllCACertsConfigMapName, }, - } + }, } customPublicCertsVolume := corev1.Volume{ Name: "che-public-certs", diff --git a/pkg/deploy/tls.go b/pkg/deploy/tls.go index 7a6cd776d8..5c48abbc7c 100644 --- a/pkg/deploy/tls.go +++ b/pkg/deploy/tls.go @@ -56,6 +56,10 @@ const ( CheCACertsConfigMapLabelValue = "true" // CheMergedCAConfigMapRevisionsLabelKey is label name which holds versions of included config maps in format: cm-name1=ver1,cm-name2=ver2 CheMergedCAConfigMapRevisionsLabelKey = "included-cm" + + // Local constants + labelEqualSign = "-" + labelCommaSign = "." ) // IsSelfSignedCertificateUsed detects whether endpoints are/should be secured by self-signed certificate. @@ -502,8 +506,8 @@ func SyncAdditionalCACertsConfigMapToCluster(cr *orgv1.CheCluster, deployContext caConfigMapsCachedRevisions := make(map[string]string) if revisions, exists := mergedCAConfigMap.ObjectMeta.Labels[CheMergedCAConfigMapRevisionsLabelKey]; exists { - for _, cmNameRevision := range strings.Split(revisions, ",") { - nameRevision := strings.Split(cmNameRevision, "=") + for _, cmNameRevision := range strings.Split(revisions, labelCommaSign) { + nameRevision := strings.Split(cmNameRevision, labelEqualSign) if len(nameRevision) != 2 { // The label value is invalid, recreate merged config map break @@ -535,9 +539,9 @@ func SyncAdditionalCACertsConfigMapToCluster(cr *orgv1.CheCluster, deployContext // Save source config map revision if revisions != "" { - revisions += "," + revisions += labelCommaSign } - revisions += cm.ObjectMeta.Name + "=" + cm.ObjectMeta.ResourceVersion + revisions += cm.ObjectMeta.Name + labelEqualSign + cm.ObjectMeta.ResourceVersion } mergedCAConfigMapSpec, err := GetSpecConfigMap(deployContext, CheAllCACertsConfigMapName, data) @@ -545,6 +549,7 @@ func SyncAdditionalCACertsConfigMapToCluster(cr *orgv1.CheCluster, deployContext return nil, err } mergedCAConfigMapSpec.ObjectMeta.Labels[CheMergedCAConfigMapRevisionsLabelKey] = revisions + mergedCAConfigMapSpec.ObjectMeta.Labels["warning"] = "do-not-edit-manually" logrus.Infof("Updating additional CA certs config map: %s", CheAllCACertsConfigMapName) mergedCAConfigMap, err = SyncConfigMapToCluster(deployContext, mergedCAConfigMapSpec) @@ -563,9 +568,19 @@ func getCACertsConfigMaps(deployContext *DeployContext) ([]corev1.ConfigMap, err listOptions := &client.ListOptions{ LabelSelector: labels.NewSelector().Add(*labelSelectorRequirement), } - if err := deployContext.ClusterAPI.Client.List(context.TODO(), listOptions, CACertsConfigMapList); err == nil { + if err := deployContext.ClusterAPI.Client.List(context.TODO(), CACertsConfigMapList, listOptions); err != nil { return nil, err } return CACertsConfigMapList.Items, nil } + +// GetAdditionalCACertsConfigMapVersion returns revision of merged additional CA certs config map +func GetAdditionalCACertsConfigMapVersion(deployContext *DeployContext) string { + trustStoreConfigMap, _ := GetClusterConfigMap(CheAllCACertsConfigMapName, deployContext.CheCluster.Namespace, deployContext.ClusterAPI.Client) + if trustStoreConfigMap != nil { + return trustStoreConfigMap.ResourceVersion + } + + return "" +} From c9c1190c0c21799f5758a803f2f18d2d95ef8906 Mon Sep 17 00:00:00 2001 From: Mykola Morhun Date: Mon, 23 Nov 2020 12:31:17 +0200 Subject: [PATCH 3/6] Change CA config maps label name. Require second app.kubernetes.io/part-of=che.eclipse.org label Signed-off-by: Mykola Morhun --- pkg/deploy/tls.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/deploy/tls.go b/pkg/deploy/tls.go index 5c48abbc7c..2066aa2b3e 100644 --- a/pkg/deploy/tls.go +++ b/pkg/deploy/tls.go @@ -48,12 +48,12 @@ const ( CheTLSSelfSignedCertificateSecretName = "self-signed-certificate" DefaultCheTLSSecretName = "che-tls" - // CheAllCACertsConfigMapName is the name of config map which contains all additional trusted by Che TLS CA certificates - CheAllCACertsConfigMapName = "che-ca-certs-merged" // CheCACertsConfigMapLabelKey is the label key which marks config map with additional CA certificates - CheCACertsConfigMapLabelKey = "che-ca-certs" + CheCACertsConfigMapLabelKey = "app.kubernetes.io/component" // CheCACertsConfigMapLabelKey is the label value which marks config map with additional CA certificates - CheCACertsConfigMapLabelValue = "true" + CheCACertsConfigMapLabelValue = "ca-bundle" + // CheAllCACertsConfigMapName is the name of config map which contains all additional trusted by Che TLS CA certificates + CheAllCACertsConfigMapName = "che-ca-certs-merged" // CheMergedCAConfigMapRevisionsLabelKey is label name which holds versions of included config maps in format: cm-name1=ver1,cm-name2=ver2 CheMergedCAConfigMapRevisionsLabelKey = "included-cm" @@ -564,9 +564,10 @@ func SyncAdditionalCACertsConfigMapToCluster(cr *orgv1.CheCluster, deployContext func getCACertsConfigMaps(deployContext *DeployContext) ([]corev1.ConfigMap, error) { CACertsConfigMapList := &corev1.ConfigMapList{} - labelSelectorRequirement, _ := labels.NewRequirement(CheCACertsConfigMapLabelKey, selection.Equals, []string{CheCACertsConfigMapLabelValue}) + caBundleLabelSelectorRequirement, _ := labels.NewRequirement(CheCACertsConfigMapLabelKey, selection.Equals, []string{CheCACertsConfigMapLabelValue}) + cheComponetLabelSelectorRequirement, _ := labels.NewRequirement("app.kubernetes.io/part-of", selection.Equals, []string{"che.eclipse.org"}) listOptions := &client.ListOptions{ - LabelSelector: labels.NewSelector().Add(*labelSelectorRequirement), + LabelSelector: labels.NewSelector().Add(*cheComponetLabelSelectorRequirement).Add(*caBundleLabelSelectorRequirement), } if err := deployContext.ClusterAPI.Client.List(context.TODO(), CACertsConfigMapList, listOptions); err != nil { return nil, err From 1803085450c3596d7ae7fde62ed59f030656001f Mon Sep 17 00:00:00 2001 From: Mykola Morhun Date: Tue, 24 Nov 2020 09:09:08 +0200 Subject: [PATCH 4/6] Fix imports Signed-off-by: Mykola Morhun --- pkg/deploy/identity-provider/deployment_keycloak.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/deploy/identity-provider/deployment_keycloak.go b/pkg/deploy/identity-provider/deployment_keycloak.go index 0938a05ac7..e5e22290e6 100644 --- a/pkg/deploy/identity-provider/deployment_keycloak.go +++ b/pkg/deploy/identity-provider/deployment_keycloak.go @@ -17,12 +17,9 @@ import ( "strconv" "strings" - "github.com/eclipse/che-operator/pkg/deploy/server" - + orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1" "github.com/eclipse/che-operator/pkg/deploy" "github.com/eclipse/che-operator/pkg/deploy/postgres" - - orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1" "github.com/eclipse/che-operator/pkg/util" "github.com/google/go-cmp/cmp" "github.com/sirupsen/logrus" From 03bb5ceac410ef87bc53444ba96d2da4ad1b6ab8 Mon Sep 17 00:00:00 2001 From: Mykola Morhun Date: Tue, 24 Nov 2020 12:33:04 +0200 Subject: [PATCH 5/6] Fixes according to review Signed-off-by: Mykola Morhun --- pkg/controller/che/che_controller.go | 8 ++++++++ pkg/deploy/defaults.go | 6 +++++- pkg/deploy/tls.go | 8 +++++--- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index 47d08a32ef..b691c7013a 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -1060,6 +1060,14 @@ func isTrustedBundleConfigMap(mgr manager.Manager, obj handler.MapObject) (bool, if checlusters.Items[0].Spec.Server.ServerTrustStoreConfigMapName != obj.Meta.GetName() { // No, it is not form CR // Check for labels + + // Check for part of Che label + if value, exists := obj.Meta.GetLabels()[deploy.PartOfCheLabelKey]; !exists || value != deploy.PartOfCheLabelValue { + // Labels do not match + return false, reconcile.Request{} + } + + // Check for CA bundle label if value, exists := obj.Meta.GetLabels()[deploy.CheCACertsConfigMapLabelKey]; !exists || value != deploy.CheCACertsConfigMapLabelValue { // Labels do not match return false, reconcile.Request{} diff --git a/pkg/deploy/defaults.go b/pkg/deploy/defaults.go index bdc92689ab..2df8d5502e 100644 --- a/pkg/deploy/defaults.go +++ b/pkg/deploy/defaults.go @@ -14,11 +14,12 @@ package deploy import ( "fmt" - "gopkg.in/yaml.v2" "io/ioutil" "os" "strings" + "gopkg.in/yaml.v2" + "github.com/eclipse/che-operator/pkg/util" "github.com/sirupsen/logrus" appsv1 "k8s.io/api/apps/v1" @@ -97,6 +98,9 @@ const ( OldDefaultCodeReadyServerImageRepo = "registry.redhat.io/codeready-workspaces/server-rhel8" OldDefaultCodeReadyServerImageTag = "1.2" OldCrwPluginRegistryUrl = "https://che-plugin-registry.openshift.io" + + PartOfCheLabelKey = "app.kubernetes.io/part-of" + PartOfCheLabelValue = "che.eclipse.org" ) func InitDefaults(defaultsPath string) { diff --git a/pkg/deploy/tls.go b/pkg/deploy/tls.go index 2066aa2b3e..1ca247c1d5 100644 --- a/pkg/deploy/tls.go +++ b/pkg/deploy/tls.go @@ -55,10 +55,12 @@ const ( // CheAllCACertsConfigMapName is the name of config map which contains all additional trusted by Che TLS CA certificates CheAllCACertsConfigMapName = "che-ca-certs-merged" // CheMergedCAConfigMapRevisionsLabelKey is label name which holds versions of included config maps in format: cm-name1=ver1,cm-name2=ver2 - CheMergedCAConfigMapRevisionsLabelKey = "included-cm" + CheMergedCAConfigMapRevisionsLabelKey = "cm_revision" // Local constants + // labelEqualSign consyant is used as a replacement for '=' symbol in labels because '=' is not allowed there labelEqualSign = "-" + // labelCommaSign consyant is used as a replacement for ',' symbol in labels because ',' is not allowed there labelCommaSign = "." ) @@ -549,7 +551,7 @@ func SyncAdditionalCACertsConfigMapToCluster(cr *orgv1.CheCluster, deployContext return nil, err } mergedCAConfigMapSpec.ObjectMeta.Labels[CheMergedCAConfigMapRevisionsLabelKey] = revisions - mergedCAConfigMapSpec.ObjectMeta.Labels["warning"] = "do-not-edit-manually" + mergedCAConfigMapSpec.ObjectMeta.Labels[PartOfCheLabelKey] = PartOfCheLabelValue logrus.Infof("Updating additional CA certs config map: %s", CheAllCACertsConfigMapName) mergedCAConfigMap, err = SyncConfigMapToCluster(deployContext, mergedCAConfigMapSpec) @@ -565,7 +567,7 @@ func getCACertsConfigMaps(deployContext *DeployContext) ([]corev1.ConfigMap, err CACertsConfigMapList := &corev1.ConfigMapList{} caBundleLabelSelectorRequirement, _ := labels.NewRequirement(CheCACertsConfigMapLabelKey, selection.Equals, []string{CheCACertsConfigMapLabelValue}) - cheComponetLabelSelectorRequirement, _ := labels.NewRequirement("app.kubernetes.io/part-of", selection.Equals, []string{"che.eclipse.org"}) + cheComponetLabelSelectorRequirement, _ := labels.NewRequirement(PartOfCheLabelKey, selection.Equals, []string{PartOfCheLabelValue}) listOptions := &client.ListOptions{ LabelSelector: labels.NewSelector().Add(*cheComponetLabelSelectorRequirement).Add(*caBundleLabelSelectorRequirement), } From b9bc48e78ce300e793d3c54a537ec2b4a01bf398 Mon Sep 17 00:00:00 2001 From: Mykola Morhun Date: Thu, 26 Nov 2020 18:19:39 +0200 Subject: [PATCH 6/6] Fixes according to review Signed-off-by: Mykola Morhun --- pkg/deploy/tls.go | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/pkg/deploy/tls.go b/pkg/deploy/tls.go index 1ca247c1d5..94fa4b1d82 100644 --- a/pkg/deploy/tls.go +++ b/pkg/deploy/tls.go @@ -53,9 +53,9 @@ const ( // CheCACertsConfigMapLabelKey is the label value which marks config map with additional CA certificates CheCACertsConfigMapLabelValue = "ca-bundle" // CheAllCACertsConfigMapName is the name of config map which contains all additional trusted by Che TLS CA certificates - CheAllCACertsConfigMapName = "che-ca-certs-merged" - // CheMergedCAConfigMapRevisionsLabelKey is label name which holds versions of included config maps in format: cm-name1=ver1,cm-name2=ver2 - CheMergedCAConfigMapRevisionsLabelKey = "cm_revision" + CheAllCACertsConfigMapName = "ca-certs-merged" + // CheMergedCAConfigMapRevisionsAnnotationKey is annotation name which holds versions of included config maps in format: cm-name1=ver1,cm-name2=ver2 + CheMergedCAConfigMapRevisionsAnnotationKey = "che.eclipse.org/included-configmaps" // Local constants // labelEqualSign consyant is used as a replacement for '=' symbol in labels because '=' is not allowed there @@ -500,21 +500,23 @@ func SyncAdditionalCACertsConfigMapToCluster(cr *orgv1.CheCluster, deployContext mergedCAConfigMap := &corev1.ConfigMap{} err = deployContext.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Namespace: deployContext.CheCluster.Namespace, Name: CheAllCACertsConfigMapName}, mergedCAConfigMap) if err == nil { - // Merged config map exists. Check if it up to date. + // Merged config map exists. Check if it is up to date. caConfigMapsCurrentRevisions := make(map[string]string) for _, cm := range caConfigMaps { caConfigMapsCurrentRevisions[cm.Name] = cm.ResourceVersion } caConfigMapsCachedRevisions := make(map[string]string) - if revisions, exists := mergedCAConfigMap.ObjectMeta.Labels[CheMergedCAConfigMapRevisionsLabelKey]; exists { - for _, cmNameRevision := range strings.Split(revisions, labelCommaSign) { - nameRevision := strings.Split(cmNameRevision, labelEqualSign) - if len(nameRevision) != 2 { - // The label value is invalid, recreate merged config map - break + if mergedCAConfigMap.ObjectMeta.Annotations != nil { + if revisions, exists := mergedCAConfigMap.ObjectMeta.Annotations[CheMergedCAConfigMapRevisionsAnnotationKey]; exists { + for _, cmNameRevision := range strings.Split(revisions, labelCommaSign) { + nameRevision := strings.Split(cmNameRevision, labelEqualSign) + if len(nameRevision) != 2 { + // The label value is invalid, recreate merged config map + break + } + caConfigMapsCachedRevisions[nameRevision[0]] = nameRevision[1] } - caConfigMapsCachedRevisions[nameRevision[0]] = nameRevision[1] } } @@ -550,9 +552,13 @@ func SyncAdditionalCACertsConfigMapToCluster(cr *orgv1.CheCluster, deployContext if err != nil { return nil, err } - mergedCAConfigMapSpec.ObjectMeta.Labels[CheMergedCAConfigMapRevisionsLabelKey] = revisions mergedCAConfigMapSpec.ObjectMeta.Labels[PartOfCheLabelKey] = PartOfCheLabelValue + if mergedCAConfigMapSpec.ObjectMeta.Annotations == nil { + mergedCAConfigMapSpec.ObjectMeta.Annotations = make(map[string]string) + } + mergedCAConfigMapSpec.ObjectMeta.Annotations[CheMergedCAConfigMapRevisionsAnnotationKey] = revisions + logrus.Infof("Updating additional CA certs config map: %s", CheAllCACertsConfigMapName) mergedCAConfigMap, err = SyncConfigMapToCluster(deployContext, mergedCAConfigMapSpec) if err != nil {