Bug 1958790: Store translation table in a secret (#419)

* First draft

* Working prototype

* Fixes for initial reviews

* Adds tests and docs

* Replace apply with create/delete

* Minor corrections

* Adds necessary permissions for gather service account

* Small doc fixes

Author: 0sewa0

manifests/03-clusterrole.yaml

@@ -179,7 +179,7 @@ rules:
   verbs:
     - get
     - list
-    - watch 
+    - watch
 - apiGroups:
     - operators.coreos.com
   resources:
@@ -214,6 +214,38 @@ subjects:
   namespace: openshift-insights
   name: gather
 
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: insights-operator-obfuscation-secret
+  namespace: openshift-insights
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - list
+  - create
+  - update
+  - delete
+
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: insights-operator-obfuscation-secret
+  namespace: openshift-insights
+roleRef:
+  kind: Role
+  name: insights-operator-obfuscation-secret
+subjects:
+- kind: ServiceAccount
+  name: gather
+  namespace: openshift-insights
+
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding

pkg/anonymization/anonymizer.go

@@ -30,8 +30,10 @@ import (
 	"strings"
 
 	configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
+	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/rest"
 	"k8s.io/klog/v2"
 	k8snet "k8s.io/utils/net"
@@ -50,6 +52,13 @@ const (
 		"some data won't be anonymized(ipv4 and cluster base domain). The error is %v"
 )
 
+var (
+	TranslationTableSecretName = "obfuscation-translation-table" //nolint: gosec
+	secretAPIVersion           = "v1"
+	secretKind                 = "Secret"
+	secretNamespace            = "openshift-insights"
+)
+
 type subnetInformation struct {
 	network net.IPNet
 	lastIP  net.IP
@@ -63,14 +72,15 @@ type Anonymizer struct {
 	networks          []subnetInformation
 	translationTable  map[string]string
 	ipNetworkRegex    *regexp.Regexp
+	secretsClient     corev1client.SecretInterface
 }
 
 type ConfigProvider interface {
 	Config() *config.Controller
 }
 
-// NewAnonymizer creates a new instance of anonymizer
-func NewAnonymizer(clusterBaseDomain string, networks []string) (*Anonymizer, error) {
+// NewAnonymizer creates a new instance of anonymizer with a provided config observer and sensitive data
+func NewAnonymizer(clusterBaseDomain string, networks []string, secretsClient corev1client.SecretInterface) (*Anonymizer, error) {
 	networks = append(networks, "127.0.0.1/8")
 
 	cidrs, err := k8snet.ParseCIDRs(networks)
@@ -92,6 +102,7 @@ func NewAnonymizer(clusterBaseDomain string, networks []string) (*Anonymizer, er
 		networks:          networksInformation,
 		translationTable:  make(map[string]string),
 		ipNetworkRegex:    regexp.MustCompile(Ipv4AddressOrNetworkRegex),
+		secretsClient:     secretsClient,
 	}, nil
 }
 
@@ -123,6 +134,8 @@ func NewAnonymizerFromConfigClient(
 		return nil, err
 	}
 
+	secretsClient := kubeClient.CoreV1().Secrets(secretNamespace)
+
 	if installConfig, exists := clusterConfigV1.Data["install-config"]; exists {
 		networkRegex := regexp.MustCompile(Ipv4NetworkRegex)
 		networks = append(networks, networkRegex.FindAllString(installConfig, -1)...)
@@ -145,7 +158,7 @@ func NewAnonymizerFromConfigClient(
 		return network1[0] > network2[0]
 	})
 
-	return NewAnonymizer(baseDomain, networks)
+	return NewAnonymizer(baseDomain, networks, secretsClient)
 }
 
 // NewAnonymizerFromConfig creates a new instance of anonymizer with a provided kubeconfig
@@ -249,6 +262,48 @@ func (anonymizer *Anonymizer) ObfuscateIP(ipStr string) string {
 	return "::"
 }
 
+// Stores the translation table in a Secret in the openshift-insights namespace.
+// The actual data is stored in the StringData portion of the Secret.
+func (anonymizer *Anonymizer) StoreTranslationTable() *corev1.Secret {
+	if len(anonymizer.translationTable) == 0 {
+		return nil
+	}
+	defer anonymizer.ResetTranslationTable()
+
+	err := anonymizer.secretsClient.Delete(context.TODO(), TranslationTableSecretName, metav1.DeleteOptions{})
+	if err != nil {
+		klog.V(4).Infof("Failed to delete translation table secret. err: %s", err)
+	}
+
+	secret := corev1.Secret{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       secretKind,
+			APIVersion: secretAPIVersion,
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name: TranslationTableSecretName,
+		},
+		StringData: anonymizer.translationTable,
+	}
+
+	createOptions := metav1.CreateOptions{
+		FieldManager: "insights-operator",
+	}
+
+	result, err := anonymizer.secretsClient.Create(context.TODO(), &secret, createOptions)
+	if err != nil {
+		klog.Errorf("Failed to create the translation table secret. err: %s", err)
+		return nil
+	}
+	klog.V(3).Infof("Created/Updated %s secret in %s namespace", TranslationTableSecretName, secretNamespace)
+	return result
+}
+
+// Resets the translation table, so that the translation table of multiple gathers wont mix toghater.
+func (anonymizer *Anonymizer) ResetTranslationTable() {
+	anonymizer.translationTable = make(map[string]string)
+}
+
 // IsObfuscationEnabled returns true if obfuscation(hiding IP and domain names) is enabled and false otherwise
 func IsObfuscationEnabled(configObserver ConfigProvider) bool {
 	if configObserver == nil {

pkg/anonymization/anonymizer_test.go

@@ -5,7 +5,13 @@ import (
 	"net"
 	"testing"
 
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+
 	"github.com/stretchr/testify/assert"
+	kubefake "k8s.io/client-go/kubernetes/fake"
+	corefake "k8s.io/client-go/kubernetes/typed/core/v1/fake"
+	clienttesting "k8s.io/client-go/testing"
 
 	"github.com/openshift/insights-operator/pkg/record"
 )
@@ -107,8 +113,7 @@ func getAnonymizer(t *testing.T) *Anonymizer {
 		"127.0.0.0/8",
 		"192.168.0.0/16",
 	}
-
-	anonymizer, err := NewAnonymizer(clusterBaseDomain, networks)
+	anonymizer, err := NewAnonymizer(clusterBaseDomain, networks, kubefake.NewSimpleClientset().CoreV1().Secrets(secretNamespace))
 	assert.NoError(t, err)
 
 	return anonymizer
@@ -190,4 +195,49 @@ func Test_Anonymizer_TranslationTableTest(t *testing.T) {
 	}).Data)
 
 	assert.Equal(t, "192.168.1.1", obfuscatedData)
+
+	assert.Equal(t, 257, len(anonymizer.translationTable))
+	anonymizer.ResetTranslationTable()
+	assert.Equal(t, 0, len(anonymizer.translationTable))
+}
+
+func Test_Anonymizer_StoreTranslationTable(t *testing.T) {
+	anonymizer := getAnonymizer(t)
+
+	// Empty translation table == No call made to
+	assert.Nil(t, anonymizer.StoreTranslationTable())
+
+	// Mock the client to react/check Apply calls
+	kube := kubefake.Clientset{}
+	client := kube.CoreV1().Secrets(secretNamespace)
+	client.(*corefake.FakeSecrets).Fake.AddReactor("create", "secrets",
+		func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) {
+			if createAction, ok := action.(clienttesting.CreateAction); ok {
+				assert.Equal(t, secretNamespace, createAction.GetNamespace())
+				assert.Equal(t, secretAPIVersion, createAction.GetResource().Version)
+				var secret *corev1.Secret
+				secret, ok = createAction.GetObject().(*corev1.Secret)
+				if !ok {
+					t.Errorf("Failed to convert sent Secret.")
+				}
+				return true, secret, nil
+			}
+			t.Errorf("Incorrect action, expected patch got %s", action)
+			return false, nil, nil
+		})
+	anonymizer.secretsClient = client
+
+	// Fill translation table
+	for i := 0; i < 10; i++ {
+		obfuscatedData := string(anonymizer.AnonymizeMemoryRecord(&record.MemoryRecord{
+			Data: []byte(fmt.Sprintf("192.168.0.%v", 255-i)),
+		}).Data)
+
+		assert.Equal(t, fmt.Sprintf("192.168.0.%v", i+1), obfuscatedData)
+	}
+	// Store translation table, then check
+	secret := anonymizer.StoreTranslationTable()
+	for i := 0; i < 10; i++ {
+		assert.Equal(t, secret.StringData[fmt.Sprintf("192.168.0.%v", 255-i)], fmt.Sprintf("192.168.0.%v", i+1))
+	}
 }

pkg/gather/gather_test.go

@@ -251,7 +251,7 @@ func Test_CollectAndRecord(t *testing.T) {
 		Gather:                  []string{AllGatherersConst},
 		EnableGlobalObfuscation: true,
 	}}
-	anonymizer, err := anonymization.NewAnonymizer("", nil)
+	anonymizer, err := anonymization.NewAnonymizer("", nil, nil)
 	assert.NoError(t, err)
 
 	functionReports, err := CollectAndRecordGatherer(context.Background(), gatherer, mockRecorder, mockConfigurator)

pkg/recorder/recorder.go

@@ -92,6 +92,9 @@ func (r *Recorder) Record(rec record.Record) error {
 
 // Flush and save the reports using recorder driver
 func (r *Recorder) Flush() error {
+	if r.anonymizer != nil {
+		defer r.anonymizer.StoreTranslationTable()
+	}
 	records := r.copy()
 	if len(records) == 0 {
 		return nil

pkg/recorder/recorder_test.go

@@ -62,7 +62,7 @@ func newRecorder(maxArchiveSize int64) Recorder {
 	driver := driverMock{}
 	driver.On("Save").Return(nil, nil)
 
-	anonymizer, _ := anonymization.NewAnonymizer("", nil)
+	anonymizer, _ := anonymization.NewAnonymizer("", nil, nil)
 
 	interval, _ := time.ParseDuration("1m")
 	return Recorder{