diff --git a/pkg/agent/run.go b/pkg/agent/run.go index 13d4401f..7691aed2 100644 --- a/pkg/agent/run.go +++ b/pkg/agent/run.go @@ -32,7 +32,6 @@ import ( "github.com/jetstack/preflight/api" "github.com/jetstack/preflight/pkg/client" - "github.com/jetstack/preflight/pkg/clusteruid" "github.com/jetstack/preflight/pkg/datagatherer" "github.com/jetstack/preflight/pkg/datagatherer/k8s" "github.com/jetstack/preflight/pkg/kubeconfig" @@ -79,28 +78,6 @@ func Run(cmd *cobra.Command, args []string) (returnErr error) { return fmt.Errorf("While evaluating configuration: %v", err) } - // We need the cluster UID before we progress further so it can be sent along with other data readings - - { - restCfg, err := kubeconfig.LoadRESTConfig("") - if err != nil { - return err - } - - clientset, err := kubernetes.NewForConfig(restCfg) - if err != nil { - return err - } - - ctx, err = clusteruid.GetClusterUID(ctx, clientset) - if err != nil { - return fmt.Errorf("failed to get cluster UID: %v", err) - } - - clusterUID := clusteruid.ClusterUIDFromContext(ctx) - log.V(logs.Debug).Info("Retrieved cluster UID", "clusterUID", clusterUID) - } - group, gctx := errgroup.WithContext(ctx) defer func() { cancel() diff --git a/pkg/clusteruid/clusteruid.go b/pkg/clusteruid/clusteruid.go deleted file mode 100644 index 2a5327f2..00000000 --- a/pkg/clusteruid/clusteruid.go +++ /dev/null @@ -1,45 +0,0 @@ -package clusteruid - -import ( - "context" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" -) - -// clusterUIDKey is the context key for storing the cluster UID -type clusterUIDKey struct{} - -// GetClusterUID retrieves the UID of the kube-system namespace using the given Kubernetes clientset. -// This UID can be used as a unique identifier for the Kubernetes cluster. -// The UID is stored in the given context for later retrieval; use ClusterUIDFromContext to get it. -func GetClusterUID(ctx context.Context, clientset kubernetes.Interface) (context.Context, error) { - namespace, err := clientset.CoreV1().Namespaces().Get(ctx, "kube-system", metav1.GetOptions{}) - if err != nil { - return ctx, err - } - - ctx = withClusterUID(ctx, string(namespace.ObjectMeta.UID)) - return ctx, nil -} - -// ClusterUIDFromContext retrieves the cluster UID from the context. -// Panics if the value is not found or if the value is not a string. -func ClusterUIDFromContext(ctx context.Context) string { - value := ctx.Value(clusterUIDKey{}) - if value == nil { - panic("cluster UID not found in context") - } - - uid, ok := value.(string) - if !ok { - panic("cluster UID in context is not a string") - } - - return uid -} - -// withClusterUID adds the given cluster UID to the context -func withClusterUID(ctx context.Context, clusterUID string) context.Context { - return context.WithValue(ctx, clusterUIDKey{}, clusterUID) -} diff --git a/pkg/clusteruid/clusteruid_test.go b/pkg/clusteruid/clusteruid_test.go deleted file mode 100644 index 1d1cacae..00000000 --- a/pkg/clusteruid/clusteruid_test.go +++ /dev/null @@ -1,39 +0,0 @@ -package clusteruid - -import ( - "testing" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/kubernetes/fake" -) - -func TestGetClusterUID(t *testing.T) { - client := fake.NewSimpleClientset() - - mockUID := "12345678-1234-5678-1234-567812345678" - - kubeSystemNS := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "kube-system", - UID: types.UID(mockUID), - }, - } - - _, err := client.CoreV1().Namespaces().Create(t.Context(), kubeSystemNS, metav1.CreateOptions{}) - if err != nil { - t.Fatalf("failed to create kube-system namespace with fake client: %v", err) - } - - ctx, err := GetClusterUID(t.Context(), client) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - uid := ClusterUIDFromContext(ctx) - - if uid != mockUID { - t.Fatalf("expected to get uid=%v, but got uid=%v", mockUID, uid) - } -} diff --git a/pkg/internal/cyberark/dataupload/dataupload.go b/pkg/internal/cyberark/dataupload/dataupload.go index 014aa85b..92cf7730 100644 --- a/pkg/internal/cyberark/dataupload/dataupload.go +++ b/pkg/internal/cyberark/dataupload/dataupload.go @@ -36,10 +36,6 @@ type CyberArkClient struct { authenticateRequest func(req *http.Request) error } -type Options struct { - ClusterName string -} - func NewCyberArkClient(trustedCAs *x509.CertPool, baseURL string, authenticateRequest func(req *http.Request) error) (*CyberArkClient, error) { cyberClient := &http.Client{} tr := http.DefaultTransport.(*http.Transport).Clone() @@ -55,14 +51,10 @@ func NewCyberArkClient(trustedCAs *x509.CertPool, baseURL string, authenticateRe }, nil } -// PostDataReadingsWithOptions PUTs the supplied payload to an [AWS presigned URL] which it obtains via the CyberArk inventory API. +// PostDataReadings PUTs the supplied payload to an [AWS presigned URL] which it obtains via the CyberArk inventory API. // // [AWS presigned URL]: https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-query-string-auth.html -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") - } - +func (c *CyberArkClient) PostDataReadings(ctx context.Context, readings []*api.DataReading) error { snapshot, err := convertDataReadingsToCyberarkSnapshot(readings) if err != nil { return fmt.Errorf("while converting datareadings to Cyberark snapshot format: %s", err) @@ -74,7 +66,7 @@ func (c *CyberArkClient) PostDataReadingsWithOptions(ctx context.Context, readin return err } - presignedUploadURL, err := c.retrievePresignedUploadURL(ctx, hex.EncodeToString(checksum.Sum(nil)), opts) + presignedUploadURL, err := c.retrievePresignedUploadURL(ctx, hex.EncodeToString(checksum.Sum(nil)), snapshot.ClusterID) if err != nil { return fmt.Errorf("while retrieving snapshot upload URL: %s", err) } @@ -104,7 +96,7 @@ func (c *CyberArkClient) PostDataReadingsWithOptions(ctx context.Context, readin return nil } -func (c *CyberArkClient) retrievePresignedUploadURL(ctx context.Context, checksum string, opts Options) (string, error) { +func (c *CyberArkClient) retrievePresignedUploadURL(ctx context.Context, checksum string, clusterID string) (string, error) { uploadURL, err := url.JoinPath(c.baseURL, apiPathSnapshotLinks) if err != nil { return "", err @@ -115,7 +107,7 @@ func (c *CyberArkClient) retrievePresignedUploadURL(ctx context.Context, checksu Checksum string `json:"checksum_sha3"` AgentVersion string `json:"agent_version"` }{ - ClusterID: opts.ClusterName, + ClusterID: clusterID, Checksum: checksum, AgentVersion: version.PreflightVersion, } diff --git a/pkg/internal/cyberark/dataupload/dataupload_test.go b/pkg/internal/cyberark/dataupload/dataupload_test.go index 4639029f..73d94c78 100644 --- a/pkg/internal/cyberark/dataupload/dataupload_test.go +++ b/pkg/internal/cyberark/dataupload/dataupload_test.go @@ -8,9 +8,10 @@ import ( "net/http" "os" "testing" - "time" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" "k8s.io/klog/v2" "k8s.io/klog/v2/ktesting" @@ -23,20 +24,41 @@ import ( _ "k8s.io/klog/v2/ktesting/init" ) -func TestCyberArkClient_PostDataReadingsWithOptions_MockAPI(t *testing.T) { - fakeTime := time.Unix(123, 0) - defaultDataReadings := []*api.DataReading{ - { - ClusterID: "success-cluster-id", - DataGatherer: "test-gatherer", - Timestamp: api.Time{Time: fakeTime}, - Data: map[string]interface{}{"test": "data"}, - SchemaVersion: "v1", +func genNamespace(name string) *unstructured.Unstructured { + o := &unstructured.Unstructured{} + o.SetAPIVersion("") + o.SetKind("Namespace") + o.SetName(name) + return o +} +func genArkNamespacesDataReading(clusterID types.UID) *api.DataReading { + kubeSystemNamespace := genNamespace("kube-system") + kubeSystemNamespace.SetUID(clusterID) + return &api.DataReading{ + ClusterID: "ignored-tlspk-cluster-id", + DataGatherer: "ark/namespaces", + Data: &api.DynamicData{ + Items: []*api.GatheredResource{ + { + Resource: kubeSystemNamespace, + }, + { + Resource: genNamespace("kube-public"), + }, + { + Resource: genNamespace("venafi"), + }, + { + Resource: genNamespace("cert-manager"), + }, + }, }, + SchemaVersion: "v1", } - - defaultOpts := dataupload.Options{ - ClusterName: "success-cluster-id", +} +func TestCyberArkClient_PostDataReadings_MockAPI(t *testing.T) { + defaultDataReadings := []*api.DataReading{ + genArkNamespacesDataReading("success-cluster-id"), } setToken := func(token string) func(*http.Request) error { @@ -50,31 +72,27 @@ func TestCyberArkClient_PostDataReadingsWithOptions_MockAPI(t *testing.T) { name string readings []*api.DataReading authenticate func(req *http.Request) error - opts dataupload.Options requireFn func(t *testing.T, err error) }{ { name: "successful upload", readings: defaultDataReadings, - opts: defaultOpts, authenticate: setToken("success-token"), requireFn: func(t *testing.T, err error) { require.NoError(t, err) }, }, { - name: "error when cluster name is empty", - readings: defaultDataReadings, - opts: dataupload.Options{ClusterName: ""}, + name: "error when cluster ID not found among data readings", + readings: nil, authenticate: setToken("success-token"), requireFn: func(t *testing.T, err error) { - require.ErrorContains(t, err, "programmer mistake: the cluster name") + require.ErrorContains(t, err, "while converting datareadings to Cyberark snapshot format: failed to compute a clusterID from the data-readings") }, }, { name: "error when bearer token is incorrect", 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") @@ -83,7 +101,6 @@ func TestCyberArkClient_PostDataReadingsWithOptions_MockAPI(t *testing.T) { { name: "error contains authenticate error", readings: defaultDataReadings, - opts: defaultOpts, authenticate: func(_ *http.Request) error { return errors.New("simulated-authenticate-error") }, @@ -92,18 +109,20 @@ func TestCyberArkClient_PostDataReadingsWithOptions_MockAPI(t *testing.T) { }, }, { - name: "invalid JSON from server (RetrievePresignedUploadURL step)", - readings: defaultDataReadings, - opts: dataupload.Options{ClusterName: "invalid-json-retrieve-presigned"}, + name: "invalid JSON from server (RetrievePresignedUploadURL step)", + readings: []*api.DataReading{ + genArkNamespacesDataReading("invalid-json-retrieve-presigned"), + }, authenticate: setToken("success-token"), requireFn: func(t *testing.T, err error) { require.ErrorContains(t, err, "while retrieving snapshot upload URL: rejecting JSON response from server as it was too large or was truncated") }, }, { - name: "500 from server (RetrievePresignedUploadURL step)", - readings: defaultDataReadings, - opts: dataupload.Options{ClusterName: "invalid-response-post-data"}, + name: "500 from server (RetrievePresignedUploadURL step)", + readings: []*api.DataReading{ + genArkNamespacesDataReading("invalid-response-post-data"), + }, authenticate: setToken("success-token"), requireFn: func(t *testing.T, err error) { require.ErrorContains(t, err, "while retrieving snapshot upload URL: received response with status code 500: mock error") @@ -128,13 +147,13 @@ func TestCyberArkClient_PostDataReadingsWithOptions_MockAPI(t *testing.T) { cyberArkClient, err := dataupload.NewCyberArkClient(certPool, server.Server.URL, tc.authenticate) require.NoError(t, err) - err = cyberArkClient.PostDataReadingsWithOptions(ctx, tc.readings, tc.opts) + err = cyberArkClient.PostDataReadings(ctx, tc.readings) tc.requireFn(t, err) }) } } -// TestCyberArkClient_PostDataReadingsWithOptions_RealAPI demonstrates that the dataupload code works with the real inventory API. +// TestCyberArkClient_PostDataReadings_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 @@ -142,8 +161,8 @@ func TestCyberArkClient_PostDataReadingsWithOptions_MockAPI(t *testing.T) { // 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) { +// -v -count 1 -run TestCyberArkClient_PostDataReadings_RealAPI -args -testing.v 6 +func TestCyberArkClient_PostDataReadings_RealAPI(t *testing.T) { platformDomain := os.Getenv("ARK_PLATFORM_DOMAIN") subdomain := os.Getenv("ARK_SUBDOMAIN") username := os.Getenv("ARK_USERNAME") @@ -183,12 +202,9 @@ func TestCyberArkClient_PostDataReadingsWithOptions_RealAPI(t *testing.T) { require.NoError(t, err) dataReadings := testutil.ParseDataReadings(t, testutil.ReadGZIP(t, "testdata/example-1/datareadings.json.gz")) - err = cyberArkClient.PostDataReadingsWithOptions( + err = cyberArkClient.PostDataReadings( ctx, dataReadings, - dataupload.Options{ - ClusterName: "bb068932-c80d-460d-88df-34bc7f3f3297", - }, ) require.NoError(t, err) } diff --git a/pkg/internal/cyberark/dataupload/mock.go b/pkg/internal/cyberark/dataupload/mock.go index 5887253c..c4933622 100644 --- a/pkg/internal/cyberark/dataupload/mock.go +++ b/pkg/internal/cyberark/dataupload/mock.go @@ -109,7 +109,7 @@ func (mds *mockDataUploadServer) handlePresignedUpload(w http.ResponseWriter, r } if req.ClusterID != successClusterID { - http.Error(w, "post body contains cluster ID", http.StatusInternalServerError) + http.Error(w, "post body does not contain cluster ID", http.StatusInternalServerError) return } diff --git a/pkg/internal/cyberark/dataupload/snapshot.go b/pkg/internal/cyberark/dataupload/snapshot.go index 41e2c9e5..8ed6ef35 100644 --- a/pkg/internal/cyberark/dataupload/snapshot.go +++ b/pkg/internal/cyberark/dataupload/snapshot.go @@ -1,6 +1,7 @@ package dataupload import ( + "errors" "fmt" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -67,12 +68,34 @@ func extractServerVersionFromReading(reading *api.DataReading) (string, error) { return data.ServerVersion.GitVersion, nil } +// extractClusterUIDFromReading converts the opaque data from a DynamicData +// reading to Unstructured Namespace resources, and finds the UID of the +// `kube-system` namespace. +// This UID can be used as a unique identifier for the Kubernetes cluster. +// - https://venafi.slack.com/archives/C04SQR5DAD7/p1747825325264979 +// - https://github.com/kubernetes/kubernetes/issues/77487#issuecomment-489786023 +func extractClusterUIDFromReading(reading *api.DataReading) (string, error) { + resources, err := extractResourceListFromReading(reading) + if err != nil { + return "", err + } + for _, resource := range resources { + if resource.GetName() == "kube-system" { + return string(resource.GetUID()), nil + } + } + return "", fmt.Errorf("kube-system namespace UID not found in data reading: %v", reading) +} + // convertDataReadingsToCyberarkSnapshot converts DataReadings to the Cyberark // Snapshot format. +// The ClusterUID is the UID of the kube-system namespace, which is assumed to +// be unique to the cluster and assumed to never change. func convertDataReadingsToCyberarkSnapshot( readings []*api.DataReading, ) (*snapshot, error) { k8sVersion := "" + clusterID := "" resourceData := resourceData{} for _, reading := range readings { if reading.DataGatherer == "ark/discovery" { @@ -82,6 +105,15 @@ func convertDataReadingsToCyberarkSnapshot( return nil, fmt.Errorf("while extracting server version from data-reading: %s", err) } } + + if reading.DataGatherer == "ark/namespaces" { + var err error + clusterID, err = extractClusterUIDFromReading(reading) + if err != nil { + return nil, fmt.Errorf("while extracting cluster UID from data-reading: %s", err) + } + } + if key, found := gathererNameToResourceDataKeyMap[reading.DataGatherer]; found { resources, err := extractResourceListFromReading(reading) if err != nil { @@ -90,10 +122,13 @@ func convertDataReadingsToCyberarkSnapshot( resourceData[key] = append(resourceData[key], resources...) } } - + if clusterID == "" { + return nil, errors.New("failed to compute a clusterID from the data-readings") + } return &snapshot{ AgentVersion: version.PreflightVersion, K8SVersion: k8sVersion, + ClusterID: clusterID, Secrets: resourceData["secrets"], ServiceAccounts: resourceData["serviceaccounts"], Roles: resourceData["roles"], diff --git a/pkg/internal/cyberark/dataupload/testdata/example-1/agent.yaml b/pkg/internal/cyberark/dataupload/testdata/example-1/agent.yaml index 27f4f38b..44bf3ef7 100644 --- a/pkg/internal/cyberark/dataupload/testdata/example-1/agent.yaml +++ b/pkg/internal/cyberark/dataupload/testdata/example-1/agent.yaml @@ -4,6 +4,12 @@ data-gatherers: # gather k8s apiserver version information - kind: k8s-discovery name: ark/discovery +- kind: k8s-dynamic + name: ark/namespaces + config: + resource-type: + version: v1 + resource: namespaces - kind: k8s-dynamic name: ark/serviceaccounts config: diff --git a/pkg/internal/cyberark/dataupload/testdata/example-1/datareadings.json.gz b/pkg/internal/cyberark/dataupload/testdata/example-1/datareadings.json.gz index 27d1e4ab..4e6b8c21 100644 Binary files a/pkg/internal/cyberark/dataupload/testdata/example-1/datareadings.json.gz and b/pkg/internal/cyberark/dataupload/testdata/example-1/datareadings.json.gz differ diff --git a/pkg/internal/cyberark/dataupload/testdata/example-1/snapshot.json.gz b/pkg/internal/cyberark/dataupload/testdata/example-1/snapshot.json.gz index 68ca44bb..506a3058 100644 Binary files a/pkg/internal/cyberark/dataupload/testdata/example-1/snapshot.json.gz and b/pkg/internal/cyberark/dataupload/testdata/example-1/snapshot.json.gz differ