Merge pull request #762 from jetstack/oidc_upload

inteon · web-flow · commit 9df05b3c2bd2 · 2026-02-03T09:56:25.000+01:00
Upload OIDC discovery data to disco backend
diff --git a/api/datareading.go b/api/datareading.go
@@ -64,6 +64,7 @@ func (o *DataReading) UnmarshalJSON(data []byte) error {
 		target any
 		assign func(any)
 	}{
+		{&OIDCDiscoveryData{}, func(v any) { o.Data = v.(*OIDCDiscoveryData) }},
 		{&DiscoveryData{}, func(v any) { o.Data = v.(*DiscoveryData) }},
 		{&DynamicData{}, func(v any) { o.Data = v.(*DynamicData) }},
 	}
diff --git a/api/datareading_test.go b/api/datareading_test.go
@@ -75,6 +75,20 @@ func TestDataReading_UnmarshalJSON(t *testing.T) {
 			}`,
 			wantDataType: &DynamicData{},
 		},
+		{
+			name: "OIDCDiscoveryData type",
+			input: `{
+				"cluster_id": "11111111-2222-3333-4444-555555555555",
+				"data-gatherer": "oidc",
+				"timestamp": "2024-06-01T12:00:00Z",
+				"data": {
+					"openid_configuration": {"issuer": "https://example.com"},
+					"jwks": {"keys": []}
+				},
+				"schema_version": "v1"
+			}`,
+			wantDataType: &OIDCDiscoveryData{},
+		},
 		{
 			name:        "Invalid JSON",
 			input:       `not a json`,
diff --git a/deploy/charts/disco-agent/templates/configmap.yaml b/deploy/charts/disco-agent/templates/configmap.yaml
@@ -19,6 +19,8 @@ data:
       {{- . | toYaml | nindent 6 }}
     {{- end }}
     data-gatherers:
+    - kind: oidc
+      name: ark/oidc
     - kind: k8s-discovery
       name: ark/discovery
     - kind: k8s-dynamic
diff --git a/deploy/charts/disco-agent/tests/__snapshot__/configmap_test.yaml.snap b/deploy/charts/disco-agent/tests/__snapshot__/configmap_test.yaml.snap
@@ -7,6 +7,8 @@ custom-cluster-description:
         cluster_description: "A cloud hosted Kubernetes cluster hosting production workloads.\n\nteam: team-1\nemail: team-1@example.com\npurpose: Production workloads\n"
         period: "12h0m0s"
         data-gatherers:
+        - kind: oidc
+          name: ark/oidc
         - kind: k8s-discovery
           name: ark/discovery
         - kind: k8s-dynamic
@@ -114,6 +116,8 @@ custom-cluster-name:
         cluster_description: ""
         period: "12h0m0s"
         data-gatherers:
+        - kind: oidc
+          name: ark/oidc
         - kind: k8s-discovery
           name: ark/discovery
         - kind: k8s-dynamic
@@ -221,6 +225,8 @@ custom-period:
         cluster_description: ""
         period: "1m"
         data-gatherers:
+        - kind: oidc
+          name: ark/oidc
         - kind: k8s-discovery
           name: ark/discovery
         - kind: k8s-dynamic
@@ -328,6 +334,8 @@ defaults:
         cluster_description: ""
         period: "12h0m0s"
         data-gatherers:
+        - kind: oidc
+          name: ark/oidc
         - kind: k8s-discovery
           name: ark/discovery
         - kind: k8s-dynamic
diff --git a/examples/machinehub.yaml b/examples/machinehub.yaml
@@ -12,6 +12,10 @@
 #  go run . agent --one-shot --machine-hub -v 6 --agent-config-file ./examples/machinehub.yaml
 
 data-gatherers:
+# Gather Kubernetes OIDC information
+- name: ark/oidc
+  kind: oidc
+
 # Gather Kubernetes API server version information
 - name: ark/discovery
   kind: k8s-discovery
diff --git a/examples/machinehub/input.json b/examples/machinehub/input.json
@@ -1,4 +1,34 @@
 [
+    {
+        "data-gatherer": "ark/oidc",
+        "data": {
+            "openid_configuration": {
+                "id_token_signing_alg_values_supported": [
+                    "RS256"
+                ],
+                "issuer": "https://kubernetes.default.svc.cluster.local",
+                "jwks_uri": "https://10.10.1.2:6443/openid/v1/jwks",
+                "response_types_supported": [
+                    "id_token"
+                ],
+                "subject_types_supported": [
+                    "public"
+                ]
+            },
+            "jwks": {
+                "keys": [
+                    {
+                        "alg": "RS256",
+                        "e": "AQAB",
+                        "kid": "C-2916LkMJqepqULK2nqhq6uzVB6So_yyGnqyuor71Q",
+                        "kty": "RSA",
+                        "n": "sYh6rDpl5DyzBk8qlnYXo6Sf9WbplnXJv3tPxWTvhCFsVu9G5oWjknkafVDq5UOJrlybJJNjBmUyiEi1wbdnuhceJS7rZ3sRnNp3aNoS0omCR6iHJCOuoboSlcaPuRmYw4oWXlVUXlKyw8PYPVbNCcTLuq9nqf8y33mIqe7XJsf5-Z5P05WbK9Rzj-SJvlZLQ4dSFtIiwqLkm_2fpRLj0d8Af1F6vuztnhhUE2_PDsfIWdl_kJKkrK3B5x7k5tgTyFrNQPzlRBgK9jmK0HskwAFIDaLKb7FUWuUiQjn94rjKCED4iy201YPAoZBKIHFDlFVkQ_S3quwPcRyOS18r7w",
+                        "use": "sig"
+                    }
+                ]
+            }
+        }
+    },
     {
         "data-gatherer": "ark/discovery",
         "data": {
diff --git a/internal/cyberark/dataupload/dataupload.go b/internal/cyberark/dataupload/dataupload.go
@@ -57,6 +57,15 @@ type Snapshot struct {
 	ClusterDescription string `json:"cluster_description,omitempty"`
 	// K8SVersion is the version of Kubernetes which the cluster is running.
 	K8SVersion string `json:"k8s_version"`
+	// OIDCConfig contains OIDC configuration data from the API server's
+	// `/.well-known/openid-configuration` endpoint
+	OIDCConfig map[string]any `json:"openid_configuration,omitempty"`
+	// OIDCConfigError contains any error encountered while fetching the OIDC configuration
+	OIDCConfigError string `json:"openid_configuration_error,omitempty"`
+	// JWKS contains JWKS data from the API server's `/openid/v1/jwks` endpoint
+	JWKS map[string]any `json:"jwks,omitempty"`
+	// JWKSError contains any error encountered while fetching the JWKS
+	JWKSError string `json:"jwks_error,omitempty"`
 	// Secrets is a list of Secret resources in the cluster. Not all Secret
 	// types are included and only a subset of the Secret data is included.
 	Secrets []runtime.Object `json:"secrets"`
diff --git a/pkg/client/client_cyberark.go b/pkg/client/client_cyberark.go
@@ -104,6 +104,25 @@ func baseSnapshotFromOptions(opts Options) dataupload.Snapshot {
 	}
 }
 
+// extractOIDCFromReading converts the opaque data from a OIDCDiscoveryData
+// data reading to allow access to the OIDC fields within.
+func extractOIDCFromReading(reading *api.DataReading, target *dataupload.Snapshot) error {
+	if reading == nil {
+		return fmt.Errorf("programmer mistake: the DataReading must not be nil")
+	}
+	data, ok := reading.Data.(*api.OIDCDiscoveryData)
+	if !ok {
+		return fmt.Errorf(
+			"programmer mistake: the DataReading must have data type *api.OIDCDiscoveryData. "+
+				"This DataReading (%s) has data type %T", reading.DataGatherer, reading.Data)
+	}
+	target.OIDCConfig = data.OIDCConfig
+	target.OIDCConfigError = data.OIDCConfigError
+	target.JWKS = data.JWKS
+	target.JWKSError = data.JWKSError
+	return nil
+}
+
 // extractClusterIDAndServerVersionFromReading converts the opaque data from a DiscoveryData
 // data reading to allow access to the Kubernetes version fields within.
 func extractClusterIDAndServerVersionFromReading(reading *api.DataReading, target *dataupload.Snapshot) error {
@@ -161,6 +180,7 @@ func extractResourceListFromReading(reading *api.DataReading, target *[]runtime.
 // and populates the relevant field(s) of the Snapshot based on the DataReading's data.
 // Deleted resources are excluded from the snapshot because they are not needed by CyberArk.
 var defaultExtractorFunctions = map[string]func(*api.DataReading, *dataupload.Snapshot) error{
+	"ark/oidc":      extractOIDCFromReading,
 	"ark/discovery": extractClusterIDAndServerVersionFromReading,
 	"ark/secrets": func(r *api.DataReading, s *dataupload.Snapshot) error {
 		return extractResourceListFromReading(r, &s.Secrets)
diff --git a/pkg/client/client_cyberark_convertdatareadings_test.go b/pkg/client/client_cyberark_convertdatareadings_test.go
@@ -126,6 +126,70 @@ func TestExtractServerVersionFromReading(t *testing.T) {
 	}
 }
 
+// TestExtractOIDCFromReading tests the extractOIDCFromReading function.
+func TestExtractOIDCFromReading(t *testing.T) {
+	type testCase struct {
+		name             string
+		reading          *api.DataReading
+		expectedSnapshot dataupload.Snapshot
+		expectError      string
+	}
+	tests := []testCase{
+		{
+			name:        "nil reading",
+			expectError: `programmer mistake: the DataReading must not be nil`,
+		},
+		{
+			name: "nil data",
+			reading: &api.DataReading{
+				DataGatherer: "ark/oidc",
+				Data:         nil,
+			},
+			expectError: `programmer mistake: the DataReading must have data type *api.OIDCDiscoveryData. This DataReading (ark/oidc) has data type <nil>`,
+		},
+		{
+			name: "wrong data type",
+			reading: &api.DataReading{
+				DataGatherer: "ark/oidc",
+				Data:         &api.DiscoveryData{},
+			},
+			expectError: `programmer mistake: the DataReading must have data type *api.OIDCDiscoveryData. This DataReading (ark/oidc) has data type *api.DiscoveryData`,
+		},
+		{
+			name: "happy path",
+			reading: &api.DataReading{
+				DataGatherer: "ark/oidc",
+				Data: &api.OIDCDiscoveryData{
+					OIDCConfig:      map[string]any{"issuer": "https://example.com"},
+					OIDCConfigError: "oidc-err",
+					JWKS:            map[string]any{"keys": []any{}},
+					JWKSError:       "jwks-err",
+				},
+			},
+			expectedSnapshot: dataupload.Snapshot{
+				OIDCConfig:      map[string]any{"issuer": "https://example.com"},
+				OIDCConfigError: "oidc-err",
+				JWKS:            map[string]any{"keys": []any{}},
+				JWKSError:       "jwks-err",
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var snapshot dataupload.Snapshot
+			err := extractOIDCFromReading(test.reading, &snapshot)
+			if test.expectError != "" {
+				assert.EqualError(t, err, test.expectError)
+				assert.Equal(t, dataupload.Snapshot{}, snapshot)
+				return
+			}
+			require.NoError(t, err)
+			assert.Equal(t, test.expectedSnapshot, snapshot)
+		})
+	}
+}
+
 // TestExtractResourceListFromReading tests the extractResourceListFromReading function.
 func TestExtractResourceListFromReading(t *testing.T) {
 	type testCase struct {
diff --git a/pkg/client/client_cyberark_test.go b/pkg/client/client_cyberark_test.go
@@ -104,6 +104,13 @@ func fakeReadings() []*api.DataReading {
 	}
 
 	return append([]*api.DataReading{
+		{
+			DataGatherer: "ark/oidc",
+			Data: &api.OIDCDiscoveryData{
+				OIDCConfigError: "Failed to fetch /.well-known/openid-configuration: 404 Not Found",
+				JWKSError:       "Failed to fetch /openid/v1/jwks: 404 Not Found",
+			},
+		},
 		{
 			DataGatherer: "ark/discovery",
 			Data: &api.DiscoveryData{
diff --git a/pkg/datagatherer/oidc/oidc.go b/pkg/datagatherer/oidc/oidc.go
@@ -4,15 +4,20 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"net/url"
+	"strings"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/rest"
+	"k8s.io/klog/v2"
 
 	"github.com/jetstack/preflight/api"
 	"github.com/jetstack/preflight/pkg/datagatherer"
 	"github.com/jetstack/preflight/pkg/kubeconfig"
 )
 
-// OIDCDiscovery contains the configuration for the k8s-discovery data-gatherer
+// OIDCDiscovery contains the configuration for the oidc data-gatherer.
 type OIDCDiscovery struct {
 	// KubeConfigPath is the path to the kubeconfig file. If empty, will assume it runs in-cluster.
 	KubeConfigPath string `yaml:"kubeconfig"`
@@ -44,7 +49,7 @@ func (c *OIDCDiscovery) NewDataGatherer(ctx context.Context) (datagatherer.DataG
 	}, nil
 }
 
-// DataGathererOIDC stores the config for a k8s-discovery datagatherer
+// DataGathererOIDC stores the config for an oidc datagatherer.
 type DataGathererOIDC struct {
 	cl rest.Interface
 }
@@ -74,7 +79,14 @@ func (g *DataGathererOIDC) Fetch() (any, int, error) {
 		return ""
 	}
 
-	return api.OIDCDiscoveryData{
+	if oidcErr != nil {
+		klog.FromContext(ctx).V(4).Error(oidcErr, "Failed to fetch OIDC configuration")
+	}
+	if jwksErr != nil {
+		klog.FromContext(ctx).V(4).Error(jwksErr, "Failed to fetch JWKS")
+	}
+
+	return &api.OIDCDiscoveryData{
 		OIDCConfig:      oidcResponse,
 		OIDCConfigError: errToString(oidcErr),
 		JWKS:            jwksResponse,
@@ -84,14 +96,15 @@ func (g *DataGathererOIDC) Fetch() (any, int, error) {
 
 func (g *DataGathererOIDC) fetchOIDCConfig(ctx context.Context) (map[string]any, error) {
 	// Fetch the OIDC discovery document from the well-known endpoint.
-	bytes, err := g.cl.Get().AbsPath("/.well-known/openid-configuration").Do(ctx).Raw()
-	if err != nil {
-		return nil, fmt.Errorf("failed to get OIDC discovery document: %v", err)
+	result := g.cl.Get().AbsPath("/.well-known/openid-configuration").Do(ctx)
+	if err := result.Error(); err != nil {
+		return nil, fmt.Errorf("failed to get /.well-known/openid-configuration: %s", k8sErrorMessage(err))
 	}
 
+	bytes, _ := result.Raw() // we already checked result.Error(), so there is no error here
 	var oidcResponse map[string]any
 	if err := json.Unmarshal(bytes, &oidcResponse); err != nil {
-		return nil, fmt.Errorf("failed to unmarshal OIDC discovery document: %v", err)
+		return nil, fmt.Errorf("failed to unmarshal OIDC discovery document: %v (raw: %q)", err, stringFirstN(string(bytes), 80))
 	}
 
 	return oidcResponse, nil
@@ -104,15 +117,54 @@ func (g *DataGathererOIDC) fetchJWKS(ctx context.Context) (map[string]any, error
 	//  - on fully private AWS EKS clusters, the URL is still public and might not
 	//    be reachable from within the cluster (https://github.com/aws/containers-roadmap/issues/2038)
 	// So we are using the default path instead, which we think should work in most cases.
-	bytes, err := g.cl.Get().AbsPath("/openid/v1/jwks").Do(ctx).Raw()
-	if err != nil {
-		return nil, fmt.Errorf("failed to get JWKS from jwks_uri: %v", err)
+	result := g.cl.Get().AbsPath("/openid/v1/jwks").Do(ctx)
+	if err := result.Error(); err != nil {
+		return nil, fmt.Errorf("failed to get /openid/v1/jwks: %s", k8sErrorMessage(err))
 	}
 
+	bytes, _ := result.Raw() // we already checked result.Error(), so there is no error here
 	var jwksResponse map[string]any
 	if err := json.Unmarshal(bytes, &jwksResponse); err != nil {
-		return nil, fmt.Errorf("failed to unmarshal JWKS response: %v", err)
+		return nil, fmt.Errorf("failed to unmarshal JWKS response: %v (raw: %q)", err, stringFirstN(string(bytes), 80))
 	}
 
 	return jwksResponse, nil
 }
+
+func stringFirstN(s string, n int) string {
+	if len(s) <= n {
+		return s
+	}
+	return s[:n]
+}
+
+// based on https://github.com/kubernetes/kubectl/blob/a64ceaeab69eed1f11a9e1bd91cf2c1446de811c/pkg/cmd/util/helpers.go#L244
+func k8sErrorMessage(err error) string {
+	if status, isStatus := err.(apierrors.APIStatus); isStatus {
+		switch s := status.Status(); {
+		case s.Reason == metav1.StatusReasonUnauthorized:
+			return fmt.Sprintf("error: You must be logged in to the server (%s)", s.Message)
+		case len(s.Reason) > 0:
+			return fmt.Sprintf("Error from server (%s): %s", s.Reason, err.Error())
+		default:
+			return fmt.Sprintf("Error from server: %s", err.Error())
+		}
+	}
+
+	if apierrors.IsUnexpectedObjectError(err) {
+		return fmt.Sprintf("Server returned an unexpected response: %s", err.Error())
+	}
+
+	if t, isURL := err.(*url.Error); isURL {
+		if strings.Contains(t.Err.Error(), "connection refused") {
+			host := t.URL
+			if server, err := url.Parse(t.URL); err == nil {
+				host = server.Host
+			}
+			return fmt.Sprintf("The connection to the server %s was refused - did you specify the right host or port?", host)
+		}
+		return fmt.Sprintf("Unable to connect to the server: %v", t.Err)
+	}
+
+	return fmt.Sprintf("error: %v", err)
+}
diff --git a/pkg/datagatherer/oidc/oidc_test.go b/pkg/datagatherer/oidc/oidc_test.go

Original file line number	Diff line number	Diff line change
`@@ -64,6 +64,7 @@ func (o *DataReading) UnmarshalJSON(data []byte) error {`
`64`	`64`	`target any`
`65`	`65`	`assign func(any)`
`66`	`66`	`}{`
	`67`	`+ {&OIDCDiscoveryData{}, func(v any) { o.Data = v.(*OIDCDiscoveryData) }},`
`67`	`68`	`{&DiscoveryData{}, func(v any) { o.Data = v.(*DiscoveryData) }},`
`68`	`69`	`{&DynamicData{}, func(v any) { o.Data = v.(*DynamicData) }},`
`69`	`70`	`}`
Original file line number	Diff line number	Diff line change
`@@ -104,6 +104,13 @@ func fakeReadings() []*api.DataReading {`
`104`	`104`	`}`
`105`	`105`
`106`	`106`	`return append([]*api.DataReading{`
	`107`	`+ {`
	`108`	`+ DataGatherer: "ark/oidc",`
	`109`	`+ Data: &api.OIDCDiscoveryData{`
	`110`	`+ OIDCConfigError: "Failed to fetch /.well-known/openid-configuration: 404 Not Found",`
	`111`	`+ JWKSError: "Failed to fetch /openid/v1/jwks: 404 Not Found",`
	`112`	`+ },`
	`113`	`+ },`
`107`	`114`	`{`
`108`	`115`	`DataGatherer: "ark/discovery",`
`109`	`116`	`Data: &api.DiscoveryData{`