OCPBUGS-98986: [4.20] Stabilize telemetry config to prevent continuous console pod rollouts

pkg/console/operator/sync_v400.go

@@ -476,14 +476,13 @@ func (co *consoleOperator) GetTelemetryConfiguration(ctx context.Context, operat
 	if err != nil {
 		return telemetryConfig, err
 	}
-	if !telemeterClientIsAvailable {
-		telemetryConfig["TELEMETER_CLIENT_DISABLED"] = "true"
-		return telemetryConfig, nil
-	}
+	telemetryConfig["TELEMETER_CLIENT_DISABLED"] = fmt.Sprintf("%t", !telemeterClientIsAvailable)
 
-	accessToken, err := telemetry.GetAccessToken(co.configNSSecretLister)
+	var accessToken string
+	accessToken, err = telemetry.GetAccessToken(co.configNSSecretLister)
 	if err != nil {
-		return nil, err
+		klog.V(4).Infof("telemetry config: failed to get access token, proceeding without organization metadata: %v", err)
+		accessToken = ""
 	}
 	organizationID, accountMail, refreshCache := telemetry.GetOrganizationMeta(telemetryConfig, co.trackables.organizationID, co.trackables.accountMail, clusterID, accessToken)
 	// cache fetched ORGANIZATION_ID and ACCOUNT_MAIL

pkg/console/operator/sync_v400_test.go

@@ -1,12 +1,25 @@
 package operator
 
 import (
+	"context"
+	"encoding/json"
+	"sort"
 	"testing"
 
 	"github.com/go-test/deep"
-	"github.com/openshift/console-operator/pkg/api"
+
+	configv1 "github.com/openshift/api/config/v1"
+	operatorv1 "github.com/openshift/api/operator/v1"
+	configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
+	appsv1 "k8s.io/api/apps/v1"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	appsv1listers "k8s.io/client-go/listers/apps/v1"
+	corev1listers "k8s.io/client-go/listers/core/v1"
+	"k8s.io/client-go/tools/cache"
+
+	"github.com/openshift/console-operator/pkg/api"
+	"github.com/openshift/console-operator/pkg/console/telemetry"
 )
 
 func TestGetNodeComputeEnvironments(t *testing.T) {
@@ -210,3 +223,246 @@ func TestGetNodeComputeEnvironments(t *testing.T) {
 		})
 	}
 }
+
+func newIndexer(keyFunc cache.KeyFunc) cache.Indexer {
+	return cache.NewIndexer(keyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
+}
+
+func newTestConsoleOperator(t *testing.T, telemeterAvailable bool, pullSecretHasCloudAuth bool) *consoleOperator {
+	t.Helper()
+
+	// telemetry-config ConfigMap in operator namespace
+	telemetryConfigCM := &v1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      telemetry.TelemetryConfigMapName,
+			Namespace: api.OpenShiftConsoleOperatorNamespace,
+		},
+		Data: map[string]string{
+			"SEGMENT_API_HOST":       "https://segment.example.com",
+			"SEGMENT_JS_HOST":        "https://segment-js.example.com",
+			"SEGMENT_PUBLIC_API_KEY": "test-key",
+		},
+	}
+	operatorNSCMIndexer := newIndexer(cache.MetaNamespaceKeyFunc)
+	if err := operatorNSCMIndexer.Add(telemetryConfigCM); err != nil {
+		t.Fatalf("failed to add telemetry configmap to indexer: %v", err)
+	}
+
+	// ClusterVersion
+	cv := &configv1.ClusterVersion{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: api.VersionResourceName,
+		},
+		Spec: configv1.ClusterVersionSpec{
+			ClusterID: "test-cluster-id",
+		},
+	}
+	cvIndexer := newIndexer(func(obj interface{}) (string, error) {
+		meta := obj.(metav1.ObjectMetaAccessor).GetObjectMeta()
+		return meta.GetName(), nil
+	})
+	if err := cvIndexer.Add(cv); err != nil {
+		t.Fatalf("failed to add clusterversion to indexer: %v", err)
+	}
+
+	// telemeter-client deployment
+	var replicas int32
+	if telemeterAvailable {
+		replicas = 1
+	}
+	telemeterDeploy := &appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      telemetry.TelemeterClientDeploymentName,
+			Namespace: telemetry.TelemeterClientDeploymentNamespace,
+		},
+		Status: appsv1.DeploymentStatus{
+			AvailableReplicas: replicas,
+		},
+	}
+	deployIndexer := newIndexer(cache.MetaNamespaceKeyFunc)
+	if err := deployIndexer.Add(telemeterDeploy); err != nil {
+		t.Fatalf("failed to add telemeter deployment to indexer: %v", err)
+	}
+
+	// pull-secret
+	pullSecret := &v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      telemetry.PullSecretName,
+			Namespace: api.OpenShiftConfigNamespace,
+		},
+		Data: map[string][]byte{},
+	}
+	if pullSecretHasCloudAuth {
+		dockerConfig := telemetry.DockerConfig{
+			Auths: map[string]telemetry.DockerAuthEntry{
+				"cloud.openshift.com": {Auth: "test-token"},
+			},
+		}
+		configBytes, err := json.Marshal(dockerConfig)
+		if err != nil {
+			t.Fatalf("failed to marshal docker config: %v", err)
+		}
+		pullSecret.Data[".dockerconfigjson"] = configBytes
+	} else {
+		dockerConfig := telemetry.DockerConfig{
+			Auths: map[string]telemetry.DockerAuthEntry{
+				"quay.io": {Auth: "quay-token"},
+			},
+		}
+		configBytes, err := json.Marshal(dockerConfig)
+		if err != nil {
+			t.Fatalf("failed to marshal docker config: %v", err)
+		}
+		pullSecret.Data[".dockerconfigjson"] = configBytes
+	}
+	secretIndexer := newIndexer(cache.MetaNamespaceKeyFunc)
+	if err := secretIndexer.Add(pullSecret); err != nil {
+		t.Fatalf("failed to add pull-secret to indexer: %v", err)
+	}
+
+	return &consoleOperator{
+		operatorNSConfigMapLister:  corev1listers.NewConfigMapLister(operatorNSCMIndexer),
+		clusterVersionLister:       configlistersv1.NewClusterVersionLister(cvIndexer),
+		monitoringDeploymentLister: appsv1listers.NewDeploymentLister(deployIndexer),
+		configNSSecretLister:       corev1listers.NewSecretLister(secretIndexer),
+	}
+}
+
+func TestGetTelemetryConfiguration_StableKeySet(t *testing.T) {
+	tests := []struct {
+		name               string
+		telemeterAvailable bool
+		hasCloudAuth       bool
+		expectDisabledVal  string
+	}{
+		{
+			name:               "telemeter available with cloud auth",
+			telemeterAvailable: true,
+			hasCloudAuth:       true,
+			expectDisabledVal:  "false",
+		},
+		{
+			name:               "telemeter unavailable with cloud auth",
+			telemeterAvailable: false,
+			hasCloudAuth:       true,
+			expectDisabledVal:  "true",
+		},
+		{
+			name:               "telemeter unavailable without cloud auth (disconnected)",
+			telemeterAvailable: false,
+			hasCloudAuth:       false,
+			expectDisabledVal:  "true",
+		},
+		{
+			name:               "telemeter available without cloud auth (disconnected)",
+			telemeterAvailable: true,
+			hasCloudAuth:       false,
+			expectDisabledVal:  "false",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			co := newTestConsoleOperator(t, tt.telemeterAvailable, tt.hasCloudAuth)
+			operatorConfig := &operatorv1.Console{
+				ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
+			}
+
+			config, err := co.GetTelemetryConfiguration(context.Background(), operatorConfig)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			if _, ok := config["CLUSTER_ID"]; !ok {
+				t.Error("expected CLUSTER_ID key to be present")
+			}
+			if _, ok := config["ORGANIZATION_ID"]; !ok {
+				t.Error("expected ORGANIZATION_ID key to always be present")
+			}
+			if _, ok := config["ACCOUNT_MAIL"]; !ok {
+				t.Error("expected ACCOUNT_MAIL key to always be present")
+			}
+
+			disabledVal, ok := config["TELEMETER_CLIENT_DISABLED"]
+			if !ok {
+				t.Error("expected TELEMETER_CLIENT_DISABLED key to always be present")
+			}
+			if disabledVal != tt.expectDisabledVal {
+				t.Errorf("expected TELEMETER_CLIENT_DISABLED=%q, got %q", tt.expectDisabledVal, disabledVal)
+			}
+		})
+	}
+}
+
+func TestGetTelemetryConfiguration_KeySetStableAcrossAvailabilityChange(t *testing.T) {
+	operatorConfig := &operatorv1.Console{
+		ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
+	}
+
+	coUnavailable := newTestConsoleOperator(t, false, true)
+	configUnavailable, err := coUnavailable.GetTelemetryConfiguration(context.Background(), operatorConfig)
+	if err != nil {
+		t.Fatalf("unexpected error (unavailable): %v", err)
+	}
+
+	coAvailable := newTestConsoleOperator(t, true, true)
+	configAvailable, err := coAvailable.GetTelemetryConfiguration(context.Background(), operatorConfig)
+	if err != nil {
+		t.Fatalf("unexpected error (available): %v", err)
+	}
+
+	keysUnavailable := sortedKeys(configUnavailable)
+	keysAvailable := sortedKeys(configAvailable)
+
+	// All keys must be present in both states — no key-set difference allowed.
+	sharedKeys := []string{"CLUSTER_ID", "ORGANIZATION_ID", "ACCOUNT_MAIL", "TELEMETER_CLIENT_DISABLED", "SEGMENT_API_HOST", "SEGMENT_JS_HOST", "SEGMENT_PUBLIC_API_KEY"}
+	for _, key := range sharedKeys {
+		if _, ok := configUnavailable[key]; !ok {
+			t.Errorf("key %q missing from unavailable config, keys present: %v", key, keysUnavailable)
+		}
+		if _, ok := configAvailable[key]; !ok {
+			t.Errorf("key %q missing from available config, keys present: %v", key, keysAvailable)
+		}
+	}
+
+	if len(keysUnavailable) != len(keysAvailable) {
+		t.Fatalf("key set sizes differ: unavailable=%v, available=%v", keysUnavailable, keysAvailable)
+	}
+	for i := range keysUnavailable {
+		if keysUnavailable[i] != keysAvailable[i] {
+			t.Errorf("key set mismatch at index %d: unavailable has %q, available has %q", i, keysUnavailable[i], keysAvailable[i])
+		}
+	}
+}
+
+func sortedKeys(m map[string]string) []string {
+	keys := make([]string, 0, len(m))
+	for k := range m {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	return keys
+}
+
+// Verify that GetAccessToken failure does not cause GetTelemetryConfiguration to error
+func TestGetTelemetryConfiguration_DisconnectedClusterNoError(t *testing.T) {
+	co := newTestConsoleOperator(t, false, false)
+	operatorConfig := &operatorv1.Console{
+		ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
+	}
+
+	config, err := co.GetTelemetryConfiguration(context.Background(), operatorConfig)
+	if err != nil {
+		t.Fatalf("expected no error on disconnected cluster, got: %v", err)
+	}
+
+	if config["TELEMETER_CLIENT_DISABLED"] != "true" {
+		t.Error("expected TELEMETER_CLIENT_DISABLED=true on disconnected cluster")
+	}
+	if _, ok := config["ORGANIZATION_ID"]; !ok {
+		t.Error("expected ORGANIZATION_ID key to be present even on disconnected cluster")
+	}
+	if _, ok := config["ACCOUNT_MAIL"]; !ok {
+		t.Error("expected ACCOUNT_MAIL key to be present even on disconnected cluster")
+	}
+}

pkg/console/telemetry/telemetry.go

@@ -82,12 +82,12 @@ func GetAccessToken(secretsLister v1.SecretLister) (string, error) {
 	var config DockerConfig
 	err = json.Unmarshal(configBytes, &config)
 	if err != nil {
-		fmt.Println("Error decoding JSON:", err)
-		return "", err
+		klog.Errorf("error decoding pull-secret JSON: %v", err)
+		return "", fmt.Errorf("error decoding pull-secret JSON: %w", err)
 	}
 	authsBytes, ok := config.Auths["cloud.openshift.com"]
 	if !ok {
-		return "", fmt.Errorf("failed to parse 'cloud.openshift.com' field from pull-secret")
+		return "", nil
 	}
 	return authsBytes.Auth, nil
 }

pkg/console/telemetry/telemetry_test.go

@@ -3,6 +3,13 @@ package telemetry
 import (
 	"testing"
 	"time"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	corev1listers "k8s.io/client-go/listers/core/v1"
+	"k8s.io/client-go/tools/cache"
+
+	"github.com/openshift/console-operator/pkg/api"
 )
 
 // Helpers to set rate-limit timestamps and restore after test
@@ -18,6 +25,49 @@ func withLastSuccessTime(t *testing.T, ts time.Time) {
 	t.Cleanup(func() { lastSuccessTime = prev })
 }
 
+func newFakeSecretLister(t *testing.T, secrets ...*corev1.Secret) corev1listers.SecretLister {
+	t.Helper()
+	indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
+	for _, s := range secrets {
+		if err := indexer.Add(s.DeepCopy()); err != nil {
+			t.Fatalf("failed to add secret to indexer: %v", err)
+		}
+	}
+	return corev1listers.NewSecretLister(indexer)
+}
+
+func TestGetAccessToken_MissingCloudEntry(t *testing.T) {
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{Name: PullSecretName, Namespace: api.OpenShiftConfigNamespace},
+		Data:       map[string][]byte{".dockerconfigjson": []byte(`{"auths":{}}`)},
+	}
+	lister := newFakeSecretLister(t, secret)
+
+	token, err := GetAccessToken(lister)
+	if err != nil {
+		t.Fatalf("expected no error for missing cloud.openshift.com, got: %v", err)
+	}
+	if token != "" {
+		t.Fatalf("expected empty token, got %q", token)
+	}
+}
+
+func TestGetAccessToken_PresentCloudEntry(t *testing.T) {
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{Name: PullSecretName, Namespace: api.OpenShiftConfigNamespace},
+		Data:       map[string][]byte{".dockerconfigjson": []byte(`{"auths":{"cloud.openshift.com":{"auth":"my-token"}}}`)},
+	}
+	lister := newFakeSecretLister(t, secret)
+
+	token, err := GetAccessToken(lister)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if token != "my-token" {
+		t.Fatalf("expected %q, got %q", "my-token", token)
+	}
+}
+
 func TestGetOrganizationMeta_UsesCustomOverrides(t *testing.T) {
 	telemetryConfig := map[string]string{
 		"ORGANIZATION_ID": "org-custom",