From 4dbd00b6cbee84415cffedb4c1baf6ae73a925c5 Mon Sep 17 00:00:00 2001 From: Anand Kumar Singh Date: Wed, 26 Feb 2025 12:48:33 +0530 Subject: [PATCH 1/3] fix CVE namespace-isolation break argocd adds cluster monitoring label if the ns contains openshift- prefix Signed-off-by: Anand Kumar Singh --- controllers/argocd_metrics_controller.go | 15 ++++++++++++--- controllers/argocd_metrics_controller_test.go | 17 ++++++++++------- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/controllers/argocd_metrics_controller.go b/controllers/argocd_metrics_controller.go index 41764161b..38fd95c9c 100644 --- a/controllers/argocd_metrics_controller.go +++ b/controllers/argocd_metrics_controller.go @@ -109,14 +109,23 @@ func (r *ArgoCDMetricsReconciler) Reconcile(ctx context.Context, request reconci } const clusterMonitoringLabel = "openshift.io/cluster-monitoring" - labelVal, exists := namespace.Labels[clusterMonitoringLabel] + const userDefinedMonitoringLabel = "openshift.io/user-monitoring" + var labelVal, monitoringLabel string + var exists bool + if strings.HasPrefix(namespace.Name, "openshift-") { + labelVal, exists = namespace.Labels[clusterMonitoringLabel] + monitoringLabel = clusterMonitoringLabel + } else { + labelVal, exists = namespace.Labels[userDefinedMonitoringLabel] + monitoringLabel = userDefinedMonitoringLabel + } if argocd.Spec.Monitoring.DisableMetrics == nil || !*argocd.Spec.Monitoring.DisableMetrics { if !exists || labelVal != "true" { if namespace.Labels == nil { namespace.Labels = make(map[string]string) } - namespace.Labels[clusterMonitoringLabel] = "true" + namespace.Labels[monitoringLabel] = "true" err = r.Client.Update(ctx, &namespace) if err != nil { reqLogger.Error(err, "Error updating namespace", @@ -178,7 +187,7 @@ func (r *ArgoCDMetricsReconciler) Reconcile(ctx context.Context, request reconci } } else { if exists { - namespace.Labels[clusterMonitoringLabel] = "false" + namespace.Labels[monitoringLabel] = "false" err = r.Client.Update(ctx, &namespace) if err != nil { reqLogger.Error(err, "Error updating namespace", diff --git a/controllers/argocd_metrics_controller_test.go b/controllers/argocd_metrics_controller_test.go index 70db47920..bbbeb2e53 100644 --- a/controllers/argocd_metrics_controller_test.go +++ b/controllers/argocd_metrics_controller_test.go @@ -82,16 +82,19 @@ func newMetricsReconciler(t *testing.T, namespace, name string, disableMetrics * func TestReconcile_add_namespace_label(t *testing.T) { testCases := []struct { - instanceName string - namespace string + instanceName string + namespace string + expectedLabel string }{ { - instanceName: argoCDInstanceName, - namespace: "openshift-gitops", + instanceName: argoCDInstanceName, + namespace: "openshift-gitops", + expectedLabel: "openshift.io/cluster-monitoring", }, { - instanceName: "instance-two", - namespace: "namespace-two", + instanceName: "instance-two", + namespace: "namespace-two", + expectedLabel: "openshift.io/user-monitoring", }, } for _, tc := range testCases { @@ -102,7 +105,7 @@ func TestReconcile_add_namespace_label(t *testing.T) { ns := corev1.Namespace{} err = r.Client.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, &ns) assert.NilError(t, err) - value := ns.Labels["openshift.io/cluster-monitoring"] + value := ns.Labels[tc.expectedLabel] assert.Equal(t, value, "true") } } From c3c96418aa9211b3d1290b87eaf65da16e3f7d8b Mon Sep 17 00:00:00 2001 From: Anand Kumar Singh Date: Mon, 24 Mar 2025 17:58:47 +0530 Subject: [PATCH 2/3] fix test Signed-off-by: Anand Kumar Singh --- .../05-argocd_login.yaml | 2 +- .../01-login_argocd_api_server.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/openshift/e2e/sequential/1-035_validate_argocd_secret_repopulate/05-argocd_login.yaml b/test/openshift/e2e/sequential/1-035_validate_argocd_secret_repopulate/05-argocd_login.yaml index c6165d115..6cfddc357 100644 --- a/test/openshift/e2e/sequential/1-035_validate_argocd_secret_repopulate/05-argocd_login.yaml +++ b/test/openshift/e2e/sequential/1-035_validate_argocd_secret_repopulate/05-argocd_login.yaml @@ -4,7 +4,7 @@ commands: - script: | api_server=$(oc get routes -n openshift-gitops --field-selector metadata.name=openshift-gitops-server -o jsonpath="{.items[*]['spec.host']}") password=$(oc get secret openshift-gitops-cluster -n openshift-gitops -o jsonpath='{.data.admin\.password}' | base64 -d) - output=$(argocd login $api_server --username admin --password $password --insecure) + output=$(argocd login $api_server --username admin --password $password --insecure --skip-test-tls)) if ! [[ "${output}" =~ "'admin:login' logged in successfully" ]]; then if [[ "${output}" == *"rpc error: code = Unknown desc = server.secretkey is missing" ]]; then diff --git a/test/openshift/e2e/sequential/1-040_validate_quoted_RBAC_group_names/01-login_argocd_api_server.yaml b/test/openshift/e2e/sequential/1-040_validate_quoted_RBAC_group_names/01-login_argocd_api_server.yaml index ca687cb6c..56de95bc7 100644 --- a/test/openshift/e2e/sequential/1-040_validate_quoted_RBAC_group_names/01-login_argocd_api_server.yaml +++ b/test/openshift/e2e/sequential/1-040_validate_quoted_RBAC_group_names/01-login_argocd_api_server.yaml @@ -5,7 +5,7 @@ commands: api_server=$(oc get routes -n openshift-gitops --field-selector metadata.name=openshift-gitops-server -o jsonpath="{.items[*]['spec.host']}") password=$(oc get secret openshift-gitops-cluster -n openshift-gitops -o jsonpath='{.data.admin\.password}' | base64 -d) - output=$(argocd login $api_server --username admin --password $password --insecure) + output=$(argocd login $api_server --username admin --password $password --insecure--skip-test-tls) if ! [[ "${output}" =~ "'admin:login' logged in successfully" ]]; then exit 1 From 2fa690575d4f834a0f3d562115823a4af3302ba0 Mon Sep 17 00:00:00 2001 From: Anand Kumar Singh Date: Tue, 25 Mar 2025 16:08:20 +0530 Subject: [PATCH 3/3] fix failing test Signed-off-by: Anand Kumar Singh --- .../01-login_argocd_api_server.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/openshift/e2e/sequential/1-040_validate_quoted_RBAC_group_names/01-login_argocd_api_server.yaml b/test/openshift/e2e/sequential/1-040_validate_quoted_RBAC_group_names/01-login_argocd_api_server.yaml index 56de95bc7..5d07c531f 100644 --- a/test/openshift/e2e/sequential/1-040_validate_quoted_RBAC_group_names/01-login_argocd_api_server.yaml +++ b/test/openshift/e2e/sequential/1-040_validate_quoted_RBAC_group_names/01-login_argocd_api_server.yaml @@ -5,7 +5,7 @@ commands: api_server=$(oc get routes -n openshift-gitops --field-selector metadata.name=openshift-gitops-server -o jsonpath="{.items[*]['spec.host']}") password=$(oc get secret openshift-gitops-cluster -n openshift-gitops -o jsonpath='{.data.admin\.password}' | base64 -d) - output=$(argocd login $api_server --username admin --password $password --insecure--skip-test-tls) + output=$(argocd login $api_server --username admin --password $password --insecure --skip-test-tls) if ! [[ "${output}" =~ "'admin:login' logged in successfully" ]]; then exit 1