CyberArk(client): include cluster ID in discovery data

- Added `ClusterID` field to `DiscoveryData` to store the unique Kubernetes
  cluster identifier derived from the `kube-system` namespace UID.
- Updated `DataGathererDiscovery` to fetch and store the cluster ID during
  initialization.
- Refactored `extractServerVersionFromReading` to `extractClusterIDAndServerVersionFromReading`
  to handle both cluster ID and server version extraction.
- Removed `clusteruid` package as its functionality is now integrated into
  the `discovery` data gatherer.
- Updated unit tests to validate the inclusion of `ClusterID` in snapshots.

Signed-off-by: Richard Wall <richard.wall@cyberark.com>

Author: wallrj-cyberark

api/datareading.go

@@ -61,6 +61,11 @@ type DynamicData struct {
 // DiscoveryData is the DataReading.Data returned by the k8s.ConfigDiscovery
 // gatherer
 type DiscoveryData struct {
+	// ClusterID is the unique ID of the Kubernetes cluster which this snapshot was taken from.
+	// This is sourced from the kube-system namespace UID,
+	// which is assumed to be stable for the lifetime of the cluster.
+	// - https://github.com/kubernetes/kubernetes/issues/77487#issuecomment-489786023
+	ClusterID string `json:"cluster_id"`
 	// ServerVersion is the version information of the k8s apiserver
 	// See https://godoc.org/k8s.io/apimachinery/pkg/version#Info
 	ServerVersion *version.Info `json:"server_version"`

pkg/client/client_cyberark.go

@@ -49,9 +49,6 @@ func (o *CyberArkClient) PostDataReadingsWithOptions(ctx context.Context, readin
 		return fmt.Errorf("while converting data readings: %s", err)
 	}
 	snapshot.AgentVersion = version.PreflightVersion
-	// Temporary hard coded cluster ID.
-	// TODO(wallrj): The clusterID will eventually be extracted from the supplied readings.
-	snapshot.ClusterID = "success-cluster-id"
 
 	cfg, err := o.configLoader()
 	if err != nil {
@@ -69,9 +66,9 @@ func (o *CyberArkClient) PostDataReadingsWithOptions(ctx context.Context, readin
 	return nil
 }
 
-// extractServerVersionFromReading converts the opaque data from a DiscoveryData
+// extractClusterIDAndServerVersionFromReading converts the opaque data from a DiscoveryData
 // data reading to allow access to the Kubernetes version fields within.
-func extractServerVersionFromReading(reading *api.DataReading, target *string) error {
+func extractClusterIDAndServerVersionFromReading(reading *api.DataReading, target *dataupload.Snapshot) error {
 	if reading == nil {
 		return fmt.Errorf("programmer mistake: the DataReading must not be nil")
 	}
@@ -81,10 +78,10 @@ func extractServerVersionFromReading(reading *api.DataReading, target *string) e
 			"programmer mistake: the DataReading must have data type *api.DiscoveryData. "+
 				"This DataReading (%s) has data type %T", reading.DataGatherer, reading.Data)
 	}
-	if data.ServerVersion == nil {
-		return nil
+	target.ClusterID = data.ClusterID
+	if data.ServerVersion != nil {
+		target.K8SVersion = data.ServerVersion.GitVersion
 	}
-	*target = data.ServerVersion.GitVersion
 	return nil
 }
 
@@ -116,9 +113,7 @@ func extractResourceListFromReading(reading *api.DataReading, target *[]runtime.
 }
 
 var defaultExtractorFunctions = map[string]func(*api.DataReading, *dataupload.Snapshot) error{
-	"ark/discovery": func(r *api.DataReading, s *dataupload.Snapshot) error {
-		return extractServerVersionFromReading(r, &s.K8SVersion)
-	},
+	"ark/discovery": extractClusterIDAndServerVersionFromReading,
 	"ark/secrets": func(r *api.DataReading, s *dataupload.Snapshot) error {
 		return extractResourceListFromReading(r, &s.Secrets)
 	},

pkg/client/client_cyberark_convertdatareadings_test.go

@@ -18,10 +18,10 @@ import (
 // TestExtractServerVersionFromReading tests the extractServerVersionFromReading function.
 func TestExtractServerVersionFromReading(t *testing.T) {
 	type testCase struct {
-		name            string
-		reading         *api.DataReading
-		expectedVersion string
-		expectError     string
+		name             string
+		reading          *api.DataReading
+		expectedSnapshot dataupload.Snapshot
+		expectError      string
 	}
 	tests := []testCase{
 		{
@@ -50,32 +50,36 @@ func TestExtractServerVersionFromReading(t *testing.T) {
 				DataGatherer: "ark/discovery",
 				Data:         &api.DiscoveryData{},
 			},
-			expectedVersion: "",
+			expectedSnapshot: dataupload.Snapshot{},
 		},
 		{
 			name: "happy path",
 			reading: &api.DataReading{
 				DataGatherer: "ark/discovery",
 				Data: &api.DiscoveryData{
+					ClusterID: "success-cluster-id",
 					ServerVersion: &version.Info{
 						GitVersion: "v1.21.0",
 					},
 				},
 			},
-			expectedVersion: "v1.21.0",
+			expectedSnapshot: dataupload.Snapshot{
+				ClusterID:  "success-cluster-id",
+				K8SVersion: "v1.21.0",
+			},
 		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			var k8sVersion string
-			err := extractServerVersionFromReading(test.reading, &k8sVersion)
+			var snapshot dataupload.Snapshot
+			err := extractClusterIDAndServerVersionFromReading(test.reading, &snapshot)
 			if test.expectError != "" {
 				assert.EqualError(t, err, test.expectError)
-				assert.Equal(t, "", k8sVersion)
+				assert.Equal(t, dataupload.Snapshot{}, snapshot)
 				return
 			}
 			require.NoError(t, err)
-			assert.Equal(t, test.expectedVersion, k8sVersion)
+			assert.Equal(t, test.expectedSnapshot, snapshot)
 		})
 	}
 }
@@ -197,9 +201,7 @@ func TestExtractResourceListFromReading(t *testing.T) {
 // TestConvertDataReadings tests the convertDataReadings function.
 func TestConvertDataReadings(t *testing.T) {
 	simpleExtractorFunctions := map[string]func(*api.DataReading, *dataupload.Snapshot) error{
-		"ark/discovery": func(reading *api.DataReading, snapshot *dataupload.Snapshot) error {
-			return extractServerVersionFromReading(reading, &snapshot.K8SVersion)
-		},
+		"ark/discovery": extractClusterIDAndServerVersionFromReading,
 		"ark/secrets": func(reading *api.DataReading, snapshot *dataupload.Snapshot) error {
 			return extractResourceListFromReading(reading, &snapshot.Secrets)
 		},
@@ -208,6 +210,7 @@ func TestConvertDataReadings(t *testing.T) {
 		{
 			DataGatherer: "ark/discovery",
 			Data: &api.DiscoveryData{
+				ClusterID: "success-cluster-id",
 				ServerVersion: &version.Info{
 					GitVersion: "v1.21.0",
 				},
@@ -278,6 +281,7 @@ func TestConvertDataReadings(t *testing.T) {
 			extractorFunctions: simpleExtractorFunctions,
 			readings:           simpleReadings,
 			expectedSnapshot: dataupload.Snapshot{
+				ClusterID:  "success-cluster-id",
 				K8SVersion: "v1.21.0",
 				Secrets: []runtime.Object{
 					&corev1.Secret{

pkg/client/client_cyberark_test.go

@@ -109,6 +109,7 @@ func fakeReadings() []*api.DataReading {
 		{
 			DataGatherer: "ark/discovery",
 			Data: &api.DiscoveryData{
+				ClusterID: "success-cluster-id",
 				ServerVersion: &k8sversion.Info{
 					GitVersion: "v1.21.0",
 				},

pkg/clusteruid/clusteruid.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/discovery"
 
 	"github.com/jetstack/preflight/api"
@@ -33,19 +34,34 @@ func (c *ConfigDiscovery) UnmarshalYAML(unmarshal func(interface{}) error) error
 
 // NewDataGatherer constructs a new instance of the generic K8s data-gatherer for the provided
 // GroupVersionResource.
+// It gets the UID of the 'kube-system' namespace to use as the cluster ID, once at startup.
+// The UID is assumed to be stable for the lifetime of the cluster.
+// - https://github.com/kubernetes/kubernetes/issues/77487#issuecomment-489786023
 func (c *ConfigDiscovery) NewDataGatherer(ctx context.Context) (datagatherer.DataGatherer, error) {
 	cl, err := NewDiscoveryClient(c.KubeConfigPath)
 	if err != nil {
 		return nil, err
 	}
-
-	return &DataGathererDiscovery{cl: cl}, nil
+	cs, err := NewClientSet(c.KubeConfigPath)
+	if err != nil {
+		return nil, fmt.Errorf("while creating new clientset: %s", err)
+	}
+	kubesystemNS, err := cs.CoreV1().Namespaces().Get(ctx, "kube-system", metav1.GetOptions{})
+	if err != nil {
+		return nil, fmt.Errorf("while getting the kube-system namespace: %s", err)
+	}
+	return &DataGathererDiscovery{
+		cl:        cl,
+		clusterID: string(kubesystemNS.UID),
+	}, nil
 }
 
 // DataGathererDiscovery stores the config for a k8s-discovery datagatherer
 type DataGathererDiscovery struct {
 	// The 'discovery' client used for fetching data.
 	cl *discovery.DiscoveryClient
+	// The cluster ID, derived from the UID of the 'kube-system' namespace.
+	clusterID string
 }
 
 func (g *DataGathererDiscovery) Run(ctx context.Context) error {
@@ -65,6 +81,7 @@ func (g *DataGathererDiscovery) Fetch() (interface{}, int, error) {
 		return nil, -1, fmt.Errorf("failed to get server version: %v", err)
 	}
 	response := &api.DiscoveryData{
+		ClusterID:     g.clusterID,
 		ServerVersion: data,
 	}
 	return response, 1, nil