Merge pull request #1747 from JoaoBraveCoding/2114721

Bug 2114721: Adds telemeter token hash to Deployment annotation

Author: openshift-merge-robot

pkg/manifests/manifests.go

@@ -16,6 +16,7 @@ package manifests
 
 import (
 	"crypto/md5"
+	"crypto/sha256"
 	"crypto/tls"
 	"encoding/base64"
 	"encoding/json"
@@ -3513,12 +3514,18 @@ func (f *Factory) TelemeterClientPrometheusRule() (*monv1.PrometheusRule, error)
 // TelemeterClientDeployment generates a new Deployment for Telemeter client.
 // If the passed ConfigMap is not empty it mounts the Trusted CA Bundle as a VolumeMount to
 // /etc/pki/ca-trust/extracted/pem/ location.
-func (f *Factory) TelemeterClientDeployment(proxyCABundleCM *v1.ConfigMap) (*appsv1.Deployment, error) {
+func (f *Factory) TelemeterClientDeployment(proxyCABundleCM *v1.ConfigMap, s *v1.Secret) (*appsv1.Deployment, error) {
 	d, err := f.NewDeployment(f.assets.MustNewAssetReader(TelemeterClientDeployment))
 	if err != nil {
 		return nil, err
 	}
 
+	// Set annotation on deployment to trigger redeployments
+	if s != nil {
+		hash := sha256.New()
+		d.Spec.Template.Annotations["telemeter-token-hash"] = string(hash.Sum(s.Data["token"]))
+	}
+
 	for i, container := range d.Spec.Template.Spec.Containers {
 		switch container.Name {
 		case "telemeter-client":
@@ -3602,7 +3609,6 @@ func (f *Factory) TelemeterClientServiceAccount() (*v1.ServiceAccount, error) {
 	return s, nil
 }
 
-// TelemeterClientSecret generates a new Secret for Telemeter client.
 func (f *Factory) TelemeterClientSecret() (*v1.Secret, error) {
 	s, err := f.NewSecret(f.assets.MustNewAssetReader(TelemeterClientSecret))
 	if err != nil {

pkg/manifests/manifests_test.go

@@ -16,6 +16,7 @@ package manifests
 
 import (
 	"context"
+	"crypto/sha256"
 	"errors"
 	"fmt"
 	"net/url"
@@ -628,7 +629,7 @@ func TestUnconfiguredManifests(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	_, err = f.TelemeterClientDeployment(nil)
+	_, err = f.TelemeterClientDeployment(nil, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -3125,7 +3126,7 @@ func TestTelemeterConfiguration(t *testing.T) {
 		t.Fatal(err)
 	}
 	f := NewFactory("openshift-monitoring", "openshift-user-workload-monitoring", c, defaultInfrastructureReader(), &fakeProxyReader{}, NewAssets(assetsPath), &APIServerConfig{}, &configv1.Console{})
-	d, err := f.TelemeterClientDeployment(&v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "foo"}})
+	d, err := f.TelemeterClientDeployment(&v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, &v1.Secret{Data: map[string][]byte{"token": []byte("test")}})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -3148,6 +3149,15 @@ func TestTelemeterConfiguration(t *testing.T) {
 		}
 	}
 
+	hash := sha256.New()
+	expectedTokenHash := string(hash.Sum([]byte("test")))
+
+	if tokenHash, ok := d.Spec.Template.Annotations["telemeter-token-hash"]; !ok {
+		t.Fatalf("telemeter-token-hash annotation not set in telemeter-client deployment")
+	} else if expectedTokenHash != tokenHash {
+		t.Fatalf("incorrect token hash on telemeter-token-hash annotation, \n got %s, \nwant %s", tokenHash, expectedTokenHash)
+	}
+
 	expectedKubeRbacProxyTLSCipherSuitesArg := fmt.Sprintf("%s%s",
 		KubeRbacProxyTLSCipherSuitesFlag,
 		strings.Join(crypto.OpenSSLToIANACipherSuites(APIServerDefaultTLSCiphers), ","))
@@ -3163,6 +3173,87 @@ func TestTelemeterConfiguration(t *testing.T) {
 	}
 }
 
+func TestTelemeterClientSecret(t *testing.T) {
+	for _, tc := range []struct {
+		name                 string
+		config               string
+		existingData         map[string][]byte
+		expectedData         map[string][]byte
+		updateToSaltExpected bool
+	}{
+		{
+			name: "No existing secret",
+			config: `telemeterClient:
+  token: mySecretToken
+`,
+			existingData: map[string][]byte{},
+			expectedData: map[string][]byte{
+				"token": []byte("mySecretToken"),
+			},
+			updateToSaltExpected: true,
+		},
+		{
+			name: "Existing secret, salt gets deleted",
+			config: `telemeterClient:
+  token: mySecretToken
+`,
+			existingData: map[string][]byte{
+				"token": []byte("mySecretToken"),
+			},
+			expectedData: map[string][]byte{
+				"token": []byte("mySecretToken"),
+			},
+			updateToSaltExpected: true,
+		},
+		{
+			name: "Existing secret, secret changes",
+			config: `telemeterClient:
+  token: myNewSecretToken
+`,
+			existingData: map[string][]byte{
+				"token": []byte("mySecretToken"),
+				"salt":  []byte("1234456789ABCDEF"),
+			},
+			expectedData: map[string][]byte{
+				"token": []byte("myNewSecretToken"),
+			},
+			updateToSaltExpected: true,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			c, err := NewConfigFromString(tc.config)
+			if err != nil {
+				t.Fatal(err)
+			}
+			c.UserWorkloadConfiguration = NewDefaultUserWorkloadMonitoringConfig()
+			f := NewFactory("openshift-monitoring", "openshift-user-workload-monitoring", c, defaultInfrastructureReader(), &fakeProxyReader{}, NewAssets(assetsPath), &APIServerConfig{}, &configv1.Console{})
+			generatedS, err := f.TelemeterClientSecret()
+			if err != nil {
+				t.Fatal(err)
+			}
+			byteT, exists := generatedS.Data["token"]
+			newToken := string(byteT)
+			if !exists {
+				t.Fatalf("generated TelemeterClientSecret does not contain a token")
+			}
+			byteS, exists := generatedS.Data["salt"]
+			newSalt := string(byteS)
+			if !exists {
+				t.Fatalf("generated TelemeterClientSecret does not contain a salt")
+			}
+			if string(tc.expectedData["token"]) != newToken {
+				t.Fatalf("generated token is different from expected, expected %s, got %s", tc.expectedData["token"], newToken)
+			}
+			if tc.updateToSaltExpected && string(tc.existingData["salt"]) == newSalt {
+				t.Fatalf("generated salt remain the same expected it to be different, got %s", newSalt)
+			} else if !tc.updateToSaltExpected && string(tc.expectedData["salt"]) != newSalt {
+				t.Fatalf("generated salt is different from expected, expected %s, got %s", tc.expectedData["salt"], newSalt)
+			}
+		})
+	}
+
+}
+
 func TestThanosRulerConfiguration(t *testing.T) {
 	c, err := NewConfigFromString(``)
 	uwc, err := NewUserConfigFromString(`thanosRuler:

pkg/tasks/telemeter.go

@@ -21,6 +21,7 @@ import (
 	"github.com/openshift/cluster-monitoring-operator/pkg/client"
 	"github.com/openshift/cluster-monitoring-operator/pkg/manifests"
 	"github.com/openshift/cluster-monitoring-operator/pkg/promqlgen"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 
 	"github.com/pkg/errors"
 )
@@ -117,6 +118,14 @@ func (t *TelemeterClientTask) create(ctx context.Context) error {
 		return errors.Wrap(err, "initializing Telemeter client Secret failed")
 	}
 
+	oldS, err := t.client.GetSecret(ctx, s.Namespace, s.Name)
+	if err != nil && !apierrors.IsNotFound(err) {
+		return errors.Wrap(err, "getting Telemeter Client Secret failed")
+	}
+	if oldS != nil && string(oldS.Data["token"]) == t.config.ClusterMonitoringConfiguration.TelemeterClientConfig.Token {
+		s.Data = oldS.Data
+	}
+
 	err = t.client.CreateOrUpdateSecret(ctx, s)
 	if err != nil {
 		return errors.Wrap(err, "reconciling Telemeter client Secret failed")
@@ -149,7 +158,7 @@ func (t *TelemeterClientTask) create(ctx context.Context) error {
 			return errors.Wrap(err, "syncing Telemeter client CA bundle ConfigMap failed")
 		}
 
-		dep, err := t.factory.TelemeterClientDeployment(trustedCA)
+		dep, err := t.factory.TelemeterClientDeployment(trustedCA, s)
 		if err != nil {
 			return errors.Wrap(err, "initializing Telemeter client Deployment failed")
 		}
@@ -180,7 +189,7 @@ func (t *TelemeterClientTask) create(ctx context.Context) error {
 }
 
 func (t *TelemeterClientTask) destroy(ctx context.Context) error {
-	dep, err := t.factory.TelemeterClientDeployment(nil)
+	dep, err := t.factory.TelemeterClientDeployment(nil, nil)
 	if err != nil {
 		return errors.Wrap(err, "initializing Telemeter client Deployment failed")
 	}

test/e2e/config_test.go

@@ -453,6 +453,50 @@ func TestClusterMonitorTelemeterClientConfig(t *testing.T) {
 	}
 }
 
+func TestTelemeterClientSecret(t *testing.T) {
+	for _, tc := range []struct {
+		name         string
+		oldC         string
+		newC         string
+		tokenChanged bool
+	}{
+		{
+			name: "Existing Secret",
+			oldC: `telemeterClient:
+  token: mySecretToken
+`,
+			newC: `telemeterClient:
+  token: mySecretToken
+`,
+			tokenChanged: false,
+		},
+		{
+			name: "Existing Secret, new token",
+			oldC: `telemeterClient:
+  token: mySecretToken
+`,
+			newC: `telemeterClient:
+  token: myNewSecretToken
+`,
+			tokenChanged: true,
+		},
+	} {
+
+		t.Run(tc.name, func(t *testing.T) {
+			f.MustCreateOrUpdateConfigMap(t, configMapWithData(t, tc.oldC))
+			oldS := f.MustGetSecret(t, "telemeter-client", f.Ns)
+			f.MustCreateOrUpdateConfigMap(t, configMapWithData(t, tc.newC))
+			if tc.tokenChanged {
+				f.AssertValueInSecretNotEquals(oldS.GetName(), oldS.GetNamespace(), "token", string(oldS.Data["token"]))
+				f.AssertValueInSecretNotEquals(oldS.GetName(), oldS.GetNamespace(), "salt", string(oldS.Data["salt"]))
+				return
+			}
+			f.AssertValueInSecretEquals(oldS.GetName(), oldS.GetNamespace(), "token", string(oldS.Data["token"]))
+			f.AssertValueInSecretEquals(oldS.GetName(), oldS.GetNamespace(), "salt", string(oldS.Data["salt"]))
+		})
+	}
+}
+
 func TestClusterMonitorK8sPromAdapterConfig(t *testing.T) {
 	const (
 		deploymentName = "prometheus-adapter"

test/e2e/framework/assertions.go

@@ -434,6 +434,24 @@ func (f *Framework) AssertValueInConfigMapNotEquals(name, namespace, key, compar
 	}
 }
 
+func (f *Framework) AssertValueInSecretEquals(name, namespace, key, compareWith string) func(t *testing.T) {
+	return func(t *testing.T) {
+		s := f.MustGetSecret(t, name, namespace)
+		if string(s.Data[key]) != compareWith {
+			t.Fatalf("wanted value %s for key %s but got %s", compareWith, key, string(s.Data[key]))
+		}
+	}
+}
+
+func (f *Framework) AssertValueInSecretNotEquals(name, namespace, key, compareWith string) func(t *testing.T) {
+	return func(t *testing.T) {
+		s := f.MustGetSecret(t, name, namespace)
+		if string(s.Data[key]) == compareWith {
+			t.Fatalf("did not want value %s for key %s", compareWith, key)
+		}
+	}
+}
+
 type getResourceFunc func() (metav1.Object, error)
 
 func assertResourceExists(t *testing.T, getResource getResourceFunc) {

test/e2e/framework/helpers.go

@@ -60,6 +60,25 @@ func (f *Framework) MustGetConfigMap(t *testing.T, name, namespace string) *v1.C
 	return clusterCm
 }
 
+// MustGetSecret `name` from `namespace` within 5 minutes or fail
+func (f *Framework) MustGetSecret(t *testing.T, name, namespace string) *v1.Secret {
+	t.Helper()
+	var secret *v1.Secret
+	err := wait.Poll(time.Second, 5*time.Minute, func() (bool, error) {
+		s, err := f.KubeClient.CoreV1().Secrets(namespace).Get(ctx, name, metav1.GetOptions{})
+		if err != nil {
+			return false, nil
+		}
+
+		secret = s
+		return true, nil
+	})
+	if err != nil {
+		t.Fatalf("failed to get secret %s in namespace %s - %s", name, namespace, err.Error())
+	}
+	return secret
+}
+
 // MustGetStatefulSet `name` from `namespace` within 5 minutes or fail
 func (f *Framework) MustGetStatefulSet(t *testing.T, name, namespace string) *appsv1.StatefulSet {
 	t.Helper()