Add a snapshot JSON format for uploads and convert DataReadings to Snapshot format

Signed-off-by: Richard Wall <[email protected]>

Author: wallrj

api/datareading.go

@@ -1,8 +1,12 @@
 package api
 
 import (
+	"bytes"
 	"encoding/json"
 	"time"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/version"
 )
 
 // DataReadingsPost is the payload in the upload request.
@@ -28,8 +32,8 @@ type DataReading struct {
 type GatheredResource struct {
 	// Resource is a reference to a k8s object that was found by the informer
 	// should be of type unstructured.Unstructured, raw Object
-	Resource  interface{}
-	DeletedAt Time
+	Resource  interface{} `json:"resource"`
+	DeletedAt Time        `json:"deleted_at,omitempty"`
 }
 
 func (v GatheredResource) MarshalJSON() ([]byte, error) {
@@ -48,3 +52,32 @@ func (v GatheredResource) MarshalJSON() ([]byte, error) {
 
 	return json.Marshal(data)
 }
+
+func (v *GatheredResource) UnmarshalJSON(data []byte) error {
+	var tmpResource struct {
+		Resource  *unstructured.Unstructured `json:"resource"`
+		DeletedAt Time                       `json:"deleted_at,omitempty"`
+	}
+
+	d := json.NewDecoder(bytes.NewReader(data))
+	d.DisallowUnknownFields()
+
+	if err := d.Decode(&tmpResource); err != nil {
+		return err
+	}
+	v.Resource = tmpResource.Resource
+	v.DeletedAt = tmpResource.DeletedAt
+	return nil
+}
+
+// DynamicData is the DataReading.Data returned by the k8s.DataGathererDynamic
+// gatherer
+type DynamicData struct {
+	Items []*GatheredResource `json:"items"`
+}
+
+// DiscoveryData is the DataReading.Data returned by the k8s.ConfigDiscovery
+// gatherer
+type DiscoveryData struct {
+	ServerVersion *version.Info `json:"server_version"`
+}

pkg/agent/run.go

@@ -305,6 +305,14 @@ func gatherAndOutputData(ctx context.Context, eventf Eventf, config CombinedConf
 	var readings []*api.DataReading
 
 	if config.InputPath != "" {
+		// TODO(wallrj): The datareadings read from disk can not yet be pushed
+		// to the CyberArk Discovery and Context API. Why? Because they have
+		// simple data types such as map[string]interface{}. In contrast, the
+		// data from data gatherers can be cast to rich types like DynamicData
+		// or DiscoveryData The CyberArk dataupload client requires the data to
+		// have rich types to convert it to the Discovery and Context snapshots
+		// format. Consider refactoring testutil.ParseDataReadings so that it
+		// can be used here.
 		log.V(logs.Debug).Info("Reading data from local file", "inputPath", config.InputPath)
 		data, err := os.ReadFile(config.InputPath)
 		if err != nil {

pkg/client/client_cyberark.go

@@ -38,7 +38,7 @@ func (o *CyberArkClient) PostDataReadingsWithOptions(ctx context.Context, readin
 		return err
 	}
 
-	err = datauploadClient.PostDataReadingsWithOptions(ctx, api.DataReadingsPost{}, dataupload.Options{
+	err = datauploadClient.PostDataReadingsWithOptions(ctx, readings, dataupload.Options{
 		ClusterName: "bb068932-c80d-460d-88df-34bc7f3f3297",
 	})
 	if err != nil {

pkg/datagatherer/k8s/discovery.go

@@ -6,6 +6,7 @@ import (
 
 	"k8s.io/client-go/discovery"
 
+	"github.com/jetstack/preflight/api"
 	"github.com/jetstack/preflight/pkg/datagatherer"
 )
 
@@ -59,15 +60,12 @@ func (g *DataGathererDiscovery) WaitForCacheSync(ctx context.Context) error {
 
 // Fetch will fetch discovery data from the apiserver, or return an error
 func (g *DataGathererDiscovery) Fetch() (interface{}, int, error) {
-	data, err := g.cl.ServerVersion()
+	serverVersion, err := g.cl.ServerVersion()
 	if err != nil {
 		return nil, -1, fmt.Errorf("failed to get server version: %v", err)
 	}
 
-	response := map[string]interface{}{
-		// data has type Info: https://godoc.org/k8s.io/apimachinery/pkg/version#Info
-		"server_version": data,
-	}
-
-	return response, len(response), nil
+	return &api.DiscoveryData{
+		ServerVersion: serverVersion,
+	}, 1, nil
 }

pkg/datagatherer/k8s/dynamic.go

@@ -314,7 +314,6 @@ func (g *DataGathererDynamic) Fetch() (interface{}, int, error) {
 		return nil, -1, fmt.Errorf("resource type must be specified")
 	}
 
-	var list = map[string]interface{}{}
 	var items = []*api.GatheredResource{}
 
 	fetchNamespaces := g.namespaces
@@ -344,10 +343,9 @@ func (g *DataGathererDynamic) Fetch() (interface{}, int, error) {
 		return nil, -1, err
 	}
 
-	// add gathered resources to items
-	list["items"] = items
-
-	return list, len(items), nil
+	return &api.DynamicData{
+		Items: items,
+	}, len(items), nil
 }
 
 // redactList removes sensitive and superfluous data from the supplied resource list.

pkg/datagatherer/k8s/dynamic_test.go

@@ -730,15 +730,12 @@ func TestDynamicGatherer_Fetch(t *testing.T) {
 			}
 
 			if tc.expected != nil {
-				items, ok := res.(map[string]interface{})
+				data, ok := res.(*api.DynamicData)
 				if !ok {
-					t.Errorf("expected result be an map[string]interface{} but wasn't")
+					t.Errorf("expected result be *api.DynamicData but wasn't")
 				}
 
-				list, ok := items["items"].([]*api.GatheredResource)
-				if !ok {
-					t.Errorf("expected result be an []*api.GatheredResource but wasn't")
-				}
+				list := data.Items
 				// sorting list of results by name
 				sortGatheredResources(list)
 				// sorting list of expected results by name
@@ -1045,10 +1042,9 @@ func TestDynamicGathererNativeResources_Fetch(t *testing.T) {
 			}
 
 			if tc.expected != nil {
-				res, ok := rawRes.(map[string]interface{})
-				require.Truef(t, ok, "expected result be an map[string]interface{} but wasn't")
-				actual := res["items"].([]*api.GatheredResource)
-				require.Truef(t, ok, "expected result be an []*api.GatheredResource but wasn't")
+				res, ok := rawRes.(*api.DynamicData)
+				require.Truef(t, ok, "expected result be an *api.DynamicData but wasn't")
+				actual := res.Items
 
 				// sorting list of results by name
 				sortGatheredResources(actual)

pkg/internal/cyberark/dataupload/dataupload.go

@@ -70,14 +70,19 @@ func NewCyberArkClient(trustedCAs *x509.CertPool, baseURL string, authenticateRe
 // If you omit that header, it is possible to PUT any data.
 // There is a work around listed in that issue which we have shared with the
 // CyberArk API team.
-func (c *CyberArkClient) PostDataReadingsWithOptions(ctx context.Context, payload api.DataReadingsPost, opts Options) error {
+func (c *CyberArkClient) PostDataReadingsWithOptions(ctx context.Context, readings []*api.DataReading, opts Options) error {
 	if opts.ClusterName == "" {
 		return fmt.Errorf("programmer mistake: the cluster name (aka `cluster_id` in the config file) cannot be left empty")
 	}
 
+	snapshot, err := convertDataReadingsToCyberarkSnapshot(readings)
+	if err != nil {
+		return fmt.Errorf("while converting datareadings to Cyberark snapshot format: %s", err)
+	}
+
 	encodedBody := &bytes.Buffer{}
 	hash := sha256.New()
-	if err := json.NewEncoder(io.MultiWriter(encodedBody, hash)).Encode(payload); err != nil {
+	if err := json.NewEncoder(io.MultiWriter(encodedBody, hash)).Encode(snapshot); err != nil {
 		return err
 	}
 	checksum := hash.Sum(nil)

pkg/internal/cyberark/dataupload/dataupload_test.go

@@ -3,35 +3,38 @@ package dataupload_test
 import (
 	"crypto/x509"
 	"encoding/pem"
+	"errors"
 	"fmt"
 	"net/http"
+	"os"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/require"
 
 	"github.com/jetstack/preflight/api"
 	"github.com/jetstack/preflight/pkg/internal/cyberark/dataupload"
+	"github.com/jetstack/preflight/pkg/internal/cyberark/identity"
+	"github.com/jetstack/preflight/pkg/internal/cyberark/servicediscovery"
+	"github.com/jetstack/preflight/pkg/testutil"
+
+	"k8s.io/klog/v2"
+	"k8s.io/klog/v2/ktesting"
+	_ "k8s.io/klog/v2/ktesting/init"
 )
 
-func TestCyberArkClient_PostDataReadingsWithOptions(t *testing.T) {
+func TestCyberArkClient_PostDataReadingsWithOptions_MockAPI(t *testing.T) {
 	fakeTime := time.Unix(123, 0)
-	defaultPayload := api.DataReadingsPost{
-		AgentMetadata: &api.AgentMetadata{
-			Version:   "test-version",
-			ClusterID: "test",
-		},
-		DataGatherTime: fakeTime,
-		DataReadings: []*api.DataReading{
-			{
-				ClusterID:     "success-cluster-id",
-				DataGatherer:  "test-gatherer",
-				Timestamp:     api.Time{Time: fakeTime},
-				Data:          map[string]interface{}{"test": "data"},
-				SchemaVersion: "v1",
-			},
+	defaultDataReadings := []*api.DataReading{
+		{
+			ClusterID:     "success-cluster-id",
+			DataGatherer:  "test-gatherer",
+			Timestamp:     api.Time{Time: fakeTime},
+			Data:          map[string]interface{}{"test": "data"},
+			SchemaVersion: "v1",
 		},
 	}
+
 	defaultOpts := dataupload.Options{
 		ClusterName: "success-cluster-id",
 	}
@@ -45,14 +48,14 @@ func TestCyberArkClient_PostDataReadingsWithOptions(t *testing.T) {
 
 	tests := []struct {
 		name         string
-		payload      api.DataReadingsPost
+		readings     []*api.DataReading
 		authenticate func(req *http.Request) error
 		opts         dataupload.Options
 		requireFn    func(t *testing.T, err error)
 	}{
 		{
 			name:         "successful upload",
-			payload:      defaultPayload,
+			readings:     defaultDataReadings,
 			opts:         defaultOpts,
 			authenticate: setToken("success-token"),
 			requireFn: func(t *testing.T, err error) {
@@ -61,7 +64,7 @@ func TestCyberArkClient_PostDataReadingsWithOptions(t *testing.T) {
 		},
 		{
 			name:         "error when cluster name is empty",
-			payload:      defaultPayload,
+			readings:     defaultDataReadings,
 			opts:         dataupload.Options{ClusterName: ""},
 			authenticate: setToken("success-token"),
 			requireFn: func(t *testing.T, err error) {
@@ -70,16 +73,27 @@ func TestCyberArkClient_PostDataReadingsWithOptions(t *testing.T) {
 		},
 		{
 			name:         "error when bearer token is incorrect",
-			payload:      defaultPayload,
+			readings:     defaultDataReadings,
 			opts:         defaultOpts,
 			authenticate: setToken("fail-token"),
 			requireFn: func(t *testing.T, err error) {
 				require.ErrorContains(t, err, "while retrieving snapshot upload URL: received response with status code 500: should authenticate using the correct bearer token")
 			},
 		},
+		{
+			name:     "error contains authenticate error",
+			readings: defaultDataReadings,
+			opts:     defaultOpts,
+			authenticate: func(_ *http.Request) error {
+				return errors.New("simulated-authenticate-error")
+			},
+			requireFn: func(t *testing.T, err error) {
+				require.ErrorContains(t, err, "while retrieving snapshot upload URL: failed to authenticate request: simulated-authenticate-error")
+			},
+		},
 		{
 			name:         "invalid JSON from server (RetrievePresignedUploadURL step)",
-			payload:      defaultPayload,
+			readings:     defaultDataReadings,
 			opts:         dataupload.Options{ClusterName: "invalid-json-retrieve-presigned"},
 			authenticate: setToken("success-token"),
 			requireFn: func(t *testing.T, err error) {
@@ -88,7 +102,7 @@ func TestCyberArkClient_PostDataReadingsWithOptions(t *testing.T) {
 		},
 		{
 			name:         "500 from server (RetrievePresignedUploadURL step)",
-			payload:      defaultPayload,
+			readings:     defaultDataReadings,
 			opts:         dataupload.Options{ClusterName: "invalid-response-post-data"},
 			authenticate: setToken("success-token"),
 			requireFn: func(t *testing.T, err error) {
@@ -99,6 +113,9 @@ func TestCyberArkClient_PostDataReadingsWithOptions(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
+			logger := ktesting.NewLogger(t, ktesting.DefaultConfig)
+			ctx := klog.NewContext(t.Context(), logger)
+
 			server := dataupload.MockDataUploadServer()
 			defer server.Close()
 
@@ -111,8 +128,67 @@ func TestCyberArkClient_PostDataReadingsWithOptions(t *testing.T) {
 			cyberArkClient, err := dataupload.NewCyberArkClient(certPool, server.Server.URL, tc.authenticate)
 			require.NoError(t, err)
 
-			err = cyberArkClient.PostDataReadingsWithOptions(t.Context(), tc.payload, tc.opts)
+			err = cyberArkClient.PostDataReadingsWithOptions(ctx, tc.readings, tc.opts)
 			tc.requireFn(t, err)
 		})
 	}
 }
+
+// TestCyberArkClient_PostDataReadingsWithOptions_RealAPI demonstrates that the dataupload code works with the real inventory API.
+// An API token is obtained by authenticating with the ARK_USERNAME and ARK_SECRET from the environment.
+// ARK_SUBDOMAIN should be your tenant subdomain.
+// ARK_PLATFORM_DOMAIN should be either integration-cyberark.cloud or cyberark.cloud
+//
+// To enable verbose request logging:
+//
+//	go test ./pkg/internal/cyberark/dataupload/... \
+//	  -v -count 1 -run TestCyberArkClient_PostDataReadingsWithOptions_RealAPI -args -testing.v 6
+func TestCyberArkClient_PostDataReadingsWithOptions_RealAPI(t *testing.T) {
+	platformDomain := os.Getenv("ARK_PLATFORM_DOMAIN")
+	subdomain := os.Getenv("ARK_SUBDOMAIN")
+	username := os.Getenv("ARK_USERNAME")
+	secret := os.Getenv("ARK_SECRET")
+
+	if platformDomain == "" || subdomain == "" || username == "" || secret == "" {
+		t.Skip("Skipping because one of the following environment variables is unset or empty: ARK_PLATFORM_DOMAIN, ARK_SUBDOMAIN, ARK_USERNAME, ARK_SECRET")
+		return
+	}
+
+	logger := ktesting.NewLogger(t, ktesting.DefaultConfig)
+	ctx := klog.NewContext(t.Context(), logger)
+
+	const (
+		discoveryContextServiceName = "inventory"
+		separator                   = "."
+	)
+
+	serviceURL := fmt.Sprintf("https://%s%s%s.%s", subdomain, separator, discoveryContextServiceName, platformDomain)
+
+	var (
+		identityClient *identity.Client
+		err            error
+	)
+	if platformDomain == "cyberark.cloud" {
+		identityClient, err = identity.New(ctx, subdomain)
+	} else {
+		discoveryClient := servicediscovery.New(servicediscovery.WithIntegrationEndpoint())
+		identityClient, err = identity.NewWithDiscoveryClient(ctx, discoveryClient, subdomain)
+	}
+	require.NoError(t, err)
+
+	err = identityClient.LoginUsernamePassword(ctx, username, []byte(secret))
+	require.NoError(t, err)
+
+	cyberArkClient, err := dataupload.NewCyberArkClient(nil, serviceURL, identityClient.AuthenticateRequest)
+	require.NoError(t, err)
+
+	dataReadings := testutil.ParseDataReadings(t, testutil.ReadGZIP(t, "testdata/example-1/datareadings.json.gz"))
+	err = cyberArkClient.PostDataReadingsWithOptions(
+		ctx,
+		dataReadings,
+		dataupload.Options{
+			ClusterName: "bb068932-c80d-460d-88df-34bc7f3f3297",
+		},
+	)
+	require.NoError(t, err)
+}